Add undocumented FFI\CType methods #1227
Conversation
kocsismate
force-pushed
the
ffi-ctype-stub
branch
from
200d01f to
8aabfd5
Compare
|
Oh, indeed, documenting php/php-src#7217 has apparently been forgotten so far. I think we should note that in the migration guide as well, and maybe someone who has already been working with these new APIs might want to fill in some of documentation gaps right away. I'm going to add a help-wanted label, and suggest to wait a while; otherwise committing as is should be okay. |
XPointer evaluation failed: #xmlns(db=http://docbook.org/ns/docbook) xpointer(id('class.ffi-ctype')/db:refentry/db:refsect1[@ROLE='description']/descendant::db:methodsynopsis[not(@ROLE='procedural')]) According to stub file, method entries will be added. https://github.com/php/php-src/blob/97cdf62a6a1d8491f8d1ef1580f344400eb51f1d/ext/ffi/ffi.stub.php#L77-L100
|
This PR will fix the following build warning. |
|
I just noticed that the FFI\CType reflection API also introduced a bunch of class constants. Apparently, we don't have class constants anywhere in the stub files; is that deliberate? If there is no particular reason to not have them there, I suggest to add them for documentation purposes; although, of course, someone would have to implement that. And also, the transition for the docs might be quite some work, since (most?) exisiting documentation puts the class constants in the predefined constants sections, e.g. https://www.php.net/manual/en/zip.constants.php. |
Yes, unfortunately, this is due to a technical limitation: we can't autogenerate constants which have a C constant value. However, I made a POC in order to add simple constant autogeneration support in stubs: php/php-src#7434 As Nikita wasn't really impressed, I put this PR on hold. However, I think extensions could benefit from this feature, and I agree that it would be advantageous for class synopsis generation as well.
Oh, I wasn't aware of this. Thanks for bringing this to my attention! |
|
Thanks for pointing out that PR! Actually, I'm not in favor of documenting all constants with their value; while it may make sense for some (e.g. error constants, where one wants to quickly look up what the result of And note that I'm actually in favor of moving class constants from the predefined constants sections to the classs synopses, although I'm not sure whether this is currently supported by PhD. |
kocsismate
force-pushed
the
ffi-ctype-stub
branch
from
8aabfd5 to
7154953
Compare
I would be more happy if constants in stubs could be used/validated by php-src itself in any way. Otherwise we'll likely end up with outdated information in a while. That's why my PR only adds "simple" constant (~ constants whose value is a literal rather than a C constant) support so that we don't have to refer to C constants in the stubs, but we can still benefit from a more complete class entry and class synopsis page autogeneration. E.g. JSON error codes (
|
One does not necessarily exclude the other. In the stubs, constants that refer to C constants could have an UNKNOWN value, and for the docs we could still decide whether we want to display the values or not.
Ah, right. Thanks!
Well, in my opinion, that should be done anyway. The only drawback generating this from a stub file would be that grouping would not be supported (at least not out of the box). |
I like this idea! :) Believe it or not, there are only a handful of class synopses left which still needs synchronization :) And the rest of the diff is due to the missing constants. So adding support for constants would improve the situation of class synopsis generation much more.
Yeah, I think we could add support for grouping by using |
|
How much should we wait with the merge? Can we go ahead now? I doubt there will be any volunteers to help with this PR :S |
Currently, the FFI\CType class page warns due to the missing documentation of its methods.
Pages are generated by docgen.php