Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:129568
Feedback-ID: i8414410d:Fastmail
Precedence: list
MIME-Version: 1.0
Date: Sat, 06 Dec 2025 10:24:13 -0600
To: "php internals" <internals@lists.php.net>
Message-ID: <67f338b0-5e82-4d51-a8a4-28cf1e06629d@app.fastmail.com>
In-Reply-To: 
 <CAH5C8xWoaum6_te-trQKf6uFJjJVxF9vdxA47v9RbfmhX8QuFQ@mail.gmail.com>
References: 
 <CAH5C8xXUPKgd0wWwLC3G2Ye=kRZN58gK3DY82v5GoduBNNtb5A@mail.gmail.com>
 <9873b03e-1260-44b3-8285-af9511d8766e@app.fastmail.com>
 <CAH5C8xWoaum6_te-trQKf6uFJjJVxF9vdxA47v9RbfmhX8QuFQ@mail.gmail.com>
Subject: Re: [PHP-DEV] [RFC] [Discussion] Followup Improvements for ext/uri
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
From: larry@garfieldtech.com ("Larry Garfield")

On Sat, Dec 6, 2025, at 5:45 AM, M=C3=A1t=C3=A9 Kocsis wrote:
> Hi Larry,
>
>> - I really, really hate the "set" prefix on all the methods.  It's a =
builder object, surely the "set" is implied?=20
>
> I like that the "set" prefix makes all setters grouped together. For=20
> example, should we have to add some extra methods (like authority()),
> the build() method would appear between authority() and fragment() in=20
> IDE autocomplete lists without the set() prefix.

That seems like a very minor point.  Code will be read hundreds of times=
 more than it is auto-completed...

>> - It really feels like there's an interface to extract here from the =
Url/UriBuilder classes.  There's literally only one type-specific method=
 (build()).
>
> It's the very same thing that we discussed for a very long time last=20
> time... What would be the purpose of the interface? To make the two=20
> builders
> interchangeable? But they produce fundamentally different URIs. Even i=
f=20
> we don't include build(), then we still have some differences between
> the two implementations:

I guess what's bugging me here is that in the typical case, URLs and URI=
s are interchangeable.  If you're just using a standard ASCII domain and=
 path, which is the vast majority of URLs, then either one gets you the =
same result.  So it feels grating to have to deal with two different ver=
sions of that logic; say, if I just want to pull the path off of one, or=
 set the path on one.

I 1000% realize that's not your fault, nor PHP's fault, it's the fault o=
f the two competing standards that don't talk to each other.  The theore=
tical Venn diagram overlap of URLs and URIs is relatively small.  The pr=
actical overlap is quite large.  So that makes ignoring the overlap very=
 grating.  Hence why I keep looking for places to codify the safe overla=
p.

>> - UriQueryParams::hasWithValue(), could that be just hasValue()?  You=
 still need to specify the key anyway, and that's self-evident from the =
signature.
>
> Yes, I was already considering updating this name, but I was sure that=20
> someone (with 99% confidence of you) will point this out and suggest a=20
> better one.

Nice to know I'm predictable? :-)

> I agree that hasValue() is probably the right choice, although=20
> hasNameAndValue() would be the most technically correct name...
>
>> - There's a count() method, so shouldn't Ur{i|l]QueryParams implement=
 Countable?
>
> Yes, it can. I thought that implementing Countable on its own (without=20
> IteratorAggregate) was less useful, so I omitted it. Ignace suggested=20
> another approach that
> would allow implementing IteratorAggregate: if it happens then I'm=20
> totally fine with also implementing Countable.

I don't feel strongly about IteratorAggregate.  I'm not entirely sure I =
see the value for that.  But Countable seems like a no-brainer to includ=
e.

>> - Why both Uri getRawQueryParams() and getQueryParams()?  It looks li=
ke they would return the same value, no?  (If not, that should be explai=
ned.)
>
> This is actually already briefly explained in the RFC (but thanks to=20
> Ignace how also described this part):
>
>> The difference between Uri\Rfc3986\Uri::getRawQueryParams() and Uri\R=
fc3986\Uri::getQueryParams() is that the former one passes the =E2=80=9C=
raw=E2=80=9D (non-normalized) query string as an input when instantiatin=
g Uri\Rfc3986\Uri\UriQueryParams.

I have read that sentence 3 times and it's still not making sense in my =
head.  Can you clarify with an example (in the RFC)?

>> - The sort() method... should it take an optional user callback, or d=
o we lock people in to lexical ordering?
>
> Only WHATWG URL specifies its behavior, and it uses basic alphanumeric=20
> sorting. Even though there's nothing that could stop us from=20
> implementing fancier
> sorting ways, I think it's already fine as-is. Sort() can be used to=20
> guarantee that the query components are in deterministic order, and I=20
> think that's all that we need.

Fair enough.

>> - It would be quite convenient of set() and append() returned $this, =
allowing them to be chained.
>
> That's fine for me. WHATWG URL specifies their return type as void, so=20
> I went with this, but there's nothing wrong with returning $this.

It would certainly make for cleaner code.  Possibly sort() should also r=
eturn $this.

My thinking is that half the time we'll just be inlining a builder call =
somewhere and directly passing it to the appropriate UR object, so the m=
ore we can avoid "ugh, I must have a temp variable here for some damned =
reason", the better.

>> - The fromArray() logic is... totally weird and unexpected and I hate=
 it. :-)  Why can't you support repeated query parameters using nested a=
rrays rather than gumming up all calls with a wonky format?
>
> Do you mean something like ["foo" =3D> [0, 1, 2, 3]])"? I think it is=20
> indeed possible to implement what you suggested. Whenever the basic=20
> structure of the proposal settles a little bit,
> I'll update the implementation, and I'll try to find out a sensible=20
> behavior for arrays/objects.

Yes, that's more what I was thinking.  Thanks.

>> - It's not clear how one would start a new query from scratch, with t=
he private constructor. There doesn't seem to be a justification for the=
 private. I can't see why new UriQueryParams()->set('foo', 'bar') is a b=
ad thing.
>
> Yes, starting from scratch is only possible by using=20
> UriQueryParams::fromArray([]) or UriQueryParams::parse(""). But I don'=
t=20
> have any fundamental issue with
> adding support for the empty constructor variant.

An empty constructor would help make it more ergonomic, yes.  Or, heck, =
UriQueryParams::new() would also work, and avoid the edge cases of const=
ructor calls. :-)  Just some more natural way to "start from scratch."
=20
>> - Url::isSpecial() Could we come up with a better name here?  "Specia=
l" could mean anything unless you know the RFC; it feels like "real esca=
pe string" all over again.
>
> The "special URL" is indeed the technicus terminus that WHATWG URL=20
> uses. The RFC explains the concept briefly:
>
>> The WHATWG URL specification defines some special schemes (http, http=
s, ftp, file, ws, wss), which have distinct parsing and serialization ru=
les.
>
> I don't have any issues with the current name, but the only alternativ=
e=20
> I could imagine is Uri\WhatWg\Url::isSpecialScheme().

That would be imperfect, but still a major improvement from just `isSpec=
ial()`, as it specifies that it's the scheme that's special, not the who=
le URL.  What "special" means is still unclear, but at least the scope i=
s reduced.  IOW, yes please.

--Larry Garfield