On Mon, Mar 30, 2026, at 3:40 PM, Máté Kocsis wrote: > Hi Ignace, Everyone, > > I have recently clarified/updated a few things in the RFC: > > - How Uri\Rfc3986\Uri::getDecodedPathSegments() and > Uri\WhatWg\Url::getDecodedPathSegments() exactly work > - The exact list of percent-encoding modes and their behavior > - Exactly how the percentDecode() methods work > > Please have a look at these changes, because I'd like to bring this RFC > to a vote soonish, since there's not too much debate > going on for a while. > > Regards, > Máté
My apologies for taking so long to review this. It's been a hectic few weeks Chez Crell. :-) >From the builder examples: "echo $url->toAsciiString; " Is that missing () ? I'm not sure I agree that one should always reset() a builder before using it. There's plenty of good reasons to make a builder part way, then fill the rest in separately. But I do agree that reset() should exist, so the non-normative advice there is not a blocker for me. For host type detection, what does a null return signify? UrlHostType includes an Empty case, which I'd assume would be used in that situation. And I'd rather have an Empty case on UriHostType as well rather than null, unless someone can make a good argument for using null instead... LeadingSlashPolicy::AddForNonEmtpyRelative is... a mouthful. Self-documenting is fine, but as the example demonstrates it quickly creates super long lines. That could be a problem in, say, a match() statement, inside a method, where with that enum value as a case you'd be more than halfway across the screen before you get to the executable code for that case. Is there no way to make that whole thing shorter? I will also agree with Tim's comments in a separate message: The encode/decode operations should not be static methods. They do not relate to the *type*/*class*, therefore they should not be on the type/class. Functions in namespaces are totally fine. I'm not sure how I feel about object methods on the enum. That would be something like: use Uri\Rfc3986\PercentEncoder; PercentEncoder::Path->encode($someval); Right? That.. could work, but also feels quite convoluted. use Uri\Rfc3986\encode; encode($someval, PercentEncoder::Path); Is about the same length, and with PFA easily pre-configurable, and closer to what is typically seen in the wild today. I think I'd lean toward "just a function", but I wouldn't vote against the enum method approach just for that. Aside from those (overall minor) points, this all looks great, and I appreciate how much research has clearly gone into it! --Larry Garfield
