Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:118077
MIME-Version: 1.0
References: <CAOfKkdTjPiZHnSHuag_+fmFK-1Advh2-vfDgbJBXEmK=72NB0g@mail.gmail.com>
In-Reply-To: <CAOfKkdTjPiZHnSHuag_+fmFK-1Advh2-vfDgbJBXEmK=72NB0g@mail.gmail.com>
Reply-To: Levi Morrison <levi.morrison@datadoghq.com>
Date: Thu, 23 Jun 2022 10:48:57 -0600
Message-ID: <CAAKU0um7-7KZnBibwQxkvm_Kq8a264BZYOPc8G2jfax_43odjQ@mail.gmail.com>
To: Pierrick Charron <pierrick@php.net>
Cc: PHP internals <internals@lists.php.net>
Content-Type: text/plain; charset="UTF-8"
Subject: Re: [PHP-DEV] [RFC] [Under Discussion] New Curl URL API
From: internals@lists.php.net ("Levi Morrison via internals")

On Tue, Jun 21, 2022 at 10:38 PM Pierrick Charron <pierrick@php.net> wrote:
>
> Hi,
>
> Following our discussions we had on the subject of the new Curl URL API,
> and other curl improvements. I decided to only focus on adding the new Curl
> URL API and put aside all other improvements. Here is the RFC that reflects
> our current conversations.
>
> https://wiki.php.net/rfc/curl-url-api
>
> Feel free to give any feedback, concern or support :-)
>
> Regards,
> Pierrick

IMO, this should mirror the low-level curl url API very directly. The
basis of my opinion is that we do not own libcurl; we are merely
adapting it for use in PHP. We cannot anticipate changes in their
design, nor do we have authority to do so if we feel something should
change. Touching it as little as possible makes it easier to track
upstream changes, etc.

Based on that, I think the naming should be closer to libcurl.:
  - CurlUrl::URL_ENCODE should be CurlUrl::URLENCODE
  - CurlUrl::URL_DECODE should be CurlUrl::URLDECODE

And so on, only differing if necessary because something is a reserved
word. The API should be as exact as possible to what libcurl provides.
The "helpers" getHost, getPassword, etc should be removed and should
expose `curl_url_get` more directly.

Of course, it should be object based instead of resource based, but that's it.

A nicer API can be built on top of it, but I don't think that's the
role this particular API should play.