Re: Discussion about new Curl URL API and ext/curl improvements

Hi Rowan, all, Thanks for your responses and comments. > I think it's perfectly reasonable for CurlUrl to be designed the same way, > regardless of what else we do with the extension. > As suggested by most of you, I created a new version of the proposed new URL API with a friendlier interface [1]. A quick descriptions of the changes I made : - The curl_url_* procedural API was removed - The CurlUrl::set() and ::get() methods with the CURLUPART_ constants were replaced by more explicit setters and getters (ex: setHost() and getHost()). - All the CURLU_* constants are now CurlUrl class constants (ex: CURLU_GUESS_SCHEME => CurlUrl::GUESS_SCHEME) - All the CURLUE_* constants are now CurlUrlException class constants (ex: CURLUE_MALFORMED_INPUT => CurlUrlException::MALFORMED_INPUT) Does it look more like what you were expecting ? Any other improvements that should be made ? This feels like a very small gain, since users can already type "curl_" and > get a list of all curl functions. It also has real costs: users may be > confused why there are two ways of writing the same thing; and it sets the > precedent that methods on CurlHandle should mirror curl_* functions, even > though we know those have terrible usability. > This is not the only change that I would like to introduce with a new OO API. One other change that I would like to introduce is a better type checking of CurlHandle::setOpt() when declaring strict_types to 1 (I know Sara once opened a PR about this [2]). The current signature of the procedural API is `curl_setopt(CurlHandle $handle, int $option, mixed $value): bool`. The 3rd argument being mixed never throws any TypeError, and adding this to the current procedural API would be a breaking change. If we add a new OOP API this kind of change is possible and we can improve the error message like : "Argument 2 passed to CurlHandle::setOpt() for option CURLOPT_BUFFERSIZE must be of type int, string given" However, knowing that the feature freeze for 8.2 is in a month, I will put more emphasis on the Curl URL API than on a new OO API for existing Curl API. Thanks again for your feedback Pierrick [1] https://github.com/adoy/php-src/pull/1/files [2] https://github.com/php/php-src/pull/2495