Add missing CURLOPT constants
Add the 52 missing CURLOPT_* constants and their descriptions to the cURL predefined constant list.
Reference used: https://curl.se/libcurl/c/symbols-in-versions.html
Cursory glance, main question is if the extension handles boolean options with a bool or not.
If I understand the source correctly, the extension converts all values to long before passing them to the appropriate C function. I tested it locally and bool CURLOPT options accepted bool, int and float values too.
@Girgias I've made some updates to the constant list based on your review comments. Could you check my last commit and let me know whether the general direction of the changes will work or not? If it does, I'll update the entire file. If it doesn't, I didn't waste too much time on it yet. :-)
@Girgias I've made some updates to the constant list based on your review comments. Could you check my last commit and let me know whether the general direction of the changes will work or not? If it does, I'll update the entire file. If it doesn't, I didn't waste too much time on it yet. :-)
I quite like what you did in the last commit :)
A note in case someone would like to tackle this in php-src: values passed to some of the CURLOPT_* constants are not type checked and these constants should probably be added to the appropriate parts of the huge switch statement of _php_curl_setopt() in ext/curl's interface.c.
The constants (so far):
CURLOPT_ENCODING
CURLOPT_FNMATCH_FUNCTION
CURLOPT_FTPAPPEND
CURLOPT_FTPLISTONLY
CURLOPT_FTP_RESPONSE_TIMEOUT
CURLOPT_FTP_SSL
CURLOPT_HEADERFUNCTION
CURLOPT_KEYPASSWD
CURLOPT_KRB4LEVEL
CURLOPT_MAIL_RCPT_ALLOWFAILS (replaces mistyped CURLOPT_MAIL_RCPT_ALLLOWFAILS)
CURLOPT_READDATA (replaces CURLOPT_INFILE)
CURLOPT_READFUNCTION
CURLOPT_FNMATCH_FUNCTION and CURLOPT_HEADERFUNCTION are already properly handled there (see the "callables" section at the top of that switch (macro magic). The others are aliases for already handled cases, and there must be no duplicates (adding a comment might make sense, though).
It would be more important, I think, to declare the missing @aliases in curl.stub.php.
CURLOPT_FNMATCH_FUNCTIONandCURLOPT_HEADERFUNCTIONare already properly handled there (see the "callables" section at the top of thatswitch(macro magic).
@cmb69 Sorry that I wasn't specific enough but I meant that these constants are not being type checked on master anymore (https://github.com/php/php-src/pull/13291 removed the two you have mentioned). But you're right, these were there before https://github.com/php/php-src/pull/13291 so I'm really suggesting to take a look at these on master only.
@haszi, I've actually debugged CURLOPT_FNMATCH_FUNCTION and CURLOPT_HEADERFUNCTION on master, and both are properly type checked. As of https://github.com/php/php-src/pull/13291, this happens due to HANDLE_CURL_OPTION_CALLABLE(ch, CURLOPT_FNMATCH_, handlers.fnmatch, curl_fnmatch); and HANDLE_CURL_OPTION_CALLABLE_PHP_CURL_USER(ch, CURLOPT_HEADER, write_header);. You just won't be able to seek for these constants, due to the macro magic.
@haszi, I've actually debugged
CURLOPT_FNMATCH_FUNCTIONandCURLOPT_HEADERFUNCTIONon master, and both are properly type checked. As of php/php-src#13291, this happens due toHANDLE_CURL_OPTION_CALLABLE(ch, CURLOPT_FNMATCH_, handlers.fnmatch, curl_fnmatch);andHANDLE_CURL_OPTION_CALLABLE_PHP_CURL_USER(ch, CURLOPT_HEADER, write_header);. You just won't be able to seek for these constants, due to the macro magic.
You're right. How did I even miss that part? Sorry for the noise and thanks for checking on this. :-)
@Girgias I've added some applicable tags, information on what type of value each option accepts and (where known) what the default value is.
Not sure if the acronym tag comments are that useful as it would be adding a lot of them
At least adding some would be great! And others might be added later; in my opinion, our current list of acronyms is really meager.
Not sure if the acronym tag comments are that useful as it would be adding a lot of them
At least adding some would be great! And others might be added later; in my opinion, our current list of acronyms is really meager.
I took a look at acronyms.xml in doc-base and we only have 70 acronyms listed there. While I agree that more need to be added there and used on the CURLOPT_* constants page, I think I'd like to tackle that as a separate set of PRs (ie. adding them in doc-base and probably a large number of PRs adding the <acronym> tags for a large part of the doc-en files).
Another (selfish) reason is that I have a PR ready with the same type of tags/value type/default value changes made to all CURLOPT_* constants that I've added to ones in this PR and that PR is already humongous. :-)
One more thing in regards to acronyms: we should probably add an ENTITY for at least the most frequently used ones (eg.: HTTP, FTP, SSL/TLS, etc.) so that we can use something like &HTTP; instead of the unnecessarily long <acronym>HTTP</acronym>.
One more thing in regards to acronyms: we should probably add an
ENTITYfor at least the most frequently used ones (eg.: HTTP, FTP, SSL/TLS, etc.) so that we can use something like&HTTP;instead of the unnecessarily long<acronym>HTTP</acronym>.
That is a good point, could added in doc-base/entities/global.ent