Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:124066
Precedence: bulk
MIME-Version: 1.0
References: <CAH5C8xUb1O20ZDrOQNC=ckFxHUUWSK7sw_njQQzFBd0qgQqoww@mail.gmail.com>
 <dc040b64-028d-49ff-a177-23e9f054fdec@app.fastmail.com>
In-Reply-To: <dc040b64-028d-49ff-a177-23e9f054fdec@app.fastmail.com>
Date: Sun, 30 Jun 2024 00:42:06 +0200
Message-ID: <CAH5C8xVe8kPEbyQCw8cX1VTbi_W_aNRqkYU=C266irMXv7wdhw@mail.gmail.com>
Subject: Re: [PHP-DEV] [RFC] [Discussion] Add WHATWG compliant URL parsing API
To: Larry Garfield <larry@garfieldtech.com>
Cc: php internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="0000000000007f6141061c0f130f"
From: kocsismate90@gmail.com (=?UTF-8?B?TcOhdMOpIEtvY3Npcw==?=)

--0000000000007f6141061c0f130f
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Larry,

Thank you very much for your feedback! I think I have already partially
answered some of your questions in my previous email to Niels,
but let me answer your other questions below:

* I... do not understand the point of having public properties AND
> getters/withers.  A readonly class with withers, OK, a bit clunky to
> implement but it would be your problem in C, not mine, so I don't care.
> :-)  But why getters AND public properties?  If going that far, why not
> finish up clone-with and then we don't need the withers, either? :-)


I know it's disappointing, but the public modifiers are just a typo which
were forgotten there from the very first iteration of the API :) However,
I'm fine with having public readonly properties without getters as well, as
long as we declare this a policy that we are going to adopt... Withers are
indeed a must for now (and their implementation indeed requires some magic
in C...).

* Making all the parameters to Url required except port makes little sense
> to me. User/pass is more likely to be omitted 99% of the time than port.
> In practice, most components are optional, in which case it would be
> inaccurate to not make them nullable.  Empty string wouldn't be quite the
> same, as that is still a value and code that knows to skip empty string
> when doing something is basically the same as code that knows to skip
> nulls.  We should assume people are going to instantiate this class
> themselves often, not just get it from the parser, so it should be design=
ed
> to support that.


I may have misunderstood what you wrote, but all the parameters - including
port - are required. If you really meant "nullable" instead of "required",
then you are right. Apart from this, I'm completely fine with making these
parameters optional, especially if we decide not to have the UrlParser (my
initial assumption was that the Url class is going to be instantiated via
UrlParser::parseUrl() calls).

* I would not make Url final.  "OMG but then people can extend it!"
> Exactly.  I can absolutely see a case for an HttpUrl subclass that enforc=
es
> scheme as http/https, or an FtpUrl that enforces a scheme of ftp, etc.  O=
r
> even an InternalUrl that assumes the host is one particular company, or
> something.  (If this sounds like scope creep, it's because I am confident
> that people will want to creep this direction and we should plan ahead fo=
r
> it.)


Without having thought much about its consequences on the implementation,
I'm fine with removing the final modifier.

* If the intent of the withers is to mimic PSR-7, I don't think it does so
> effectively.  Without the interface, it couldn't be a drop-in replacement
> for UriInterface anyway.  And we cannot extend it to add the interface if
> it's final.  Widening the parameters in PSR-7 interfaces to support both
> wouldn't work, as that would be a hard-BC break for any existing
> implementations.  So I don't really see what the goal is here.


I've just answered this to Ben, but let me reiterate: PSR-7's UriInterface
is only needed because PHP doesn't have a Url internal class. :)

M=C3=A1t=C3=A9

--0000000000007f6141061c0f130f
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr">Hi Larry,</div><div dir=3D"ltr"><br></div=
><div>Thank you very much for your feedback! I think I have already partial=
ly answered some of your questions in my=C2=A0previous email to Niels,</div=
><div>but let me answer your other questions below:</div><div><br></div><bl=
ockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-lef=
t:1px solid rgb(204,204,204);padding-left:1ex">* I... do not understand the=
 point of having public properties AND getters/withers.=C2=A0 A readonly cl=
ass with withers, OK, a bit clunky to implement but it would be your proble=
m in C, not mine, so I don&#39;t care. :-)=C2=A0 But why getters AND public=
 properties?=C2=A0 If going that far, why not finish up clone-with and then=
 we don&#39;t need the withers, either? :-)</blockquote><div><br></div><div=
>I know it&#39;s disappointing, but the public modifiers are just a typo wh=
ich were forgotten there from the very first iteration of the API :) Howeve=
r, I&#39;m fine with having public readonly properties without getters as w=
ell, as long as we declare this a policy that we are going to adopt... With=
ers are indeed a must for now (and their implementation indeed requires som=
e magic in C...).</div><div><br></div><blockquote class=3D"gmail_quote" sty=
le=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);paddi=
ng-left:1ex">* Making all the parameters to Url required except port makes =
little sense to me. User/pass is more likely to be omitted 99% of the time =
than port.=C2=A0 In practice, most components are optional, in which case i=
t would be inaccurate to not make them nullable.=C2=A0 Empty string wouldn&=
#39;t be quite the same, as that is still a value and code that knows to sk=
ip empty string when doing something is basically the same as code that kno=
ws to skip nulls.=C2=A0 We should assume people are going to instantiate th=
is class themselves often, not just get it from the parser, so it should be=
 designed to support that.</blockquote><div><br></div><div>I may have misun=
derstood what you wrote, but all the parameters - including port - are requ=
ired. If you really meant &quot;nullable&quot; instead of &quot;required&qu=
ot;, then you are right. Apart from this, I&#39;m completely fine with maki=
ng these parameters optional, especially if we decide not to have the UrlPa=
rser (my initial assumption was that the Url class is going to be instantia=
ted via UrlParser::parseUrl() calls).</div><div><br></div><blockquote class=
=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rg=
b(204,204,204);padding-left:1ex">* I would not make Url final.=C2=A0 &quot;=
OMG but then people can extend it!&quot;=C2=A0 Exactly.=C2=A0 I can absolut=
ely see a case for an HttpUrl subclass that enforces scheme as http/https, =
or an FtpUrl that enforces a scheme of ftp, etc.=C2=A0 Or even an InternalU=
rl that assumes the host is one particular company, or something.=C2=A0 (If=
 this sounds like scope creep, it&#39;s because I am confident that people =
will want to creep this direction and we should plan ahead for it.)</blockq=
uote><div><br></div><div>Without having thought much about its consequences=
 on the implementation, I&#39;m fine with removing the final modifier.</div=
><div><br></div><blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0=
px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">* If the =
intent of the withers is to mimic PSR-7, I don&#39;t think it does so effec=
tively.=C2=A0 Without the interface, it couldn&#39;t be a drop-in replaceme=
nt for UriInterface anyway.=C2=A0 And we cannot extend it to add the interf=
ace if it&#39;s final.=C2=A0 Widening the parameters in PSR-7 interfaces to=
 support both wouldn&#39;t work, as that would be a hard-BC break for any e=
xisting implementations.=C2=A0 So I don&#39;t really see what the goal is h=
ere.</blockquote><div><br></div><div>I&#39;ve just answered this to Ben, bu=
t let me reiterate: PSR-7&#39;s UriInterface is only needed because PHP=C2=
=A0doesn&#39;t have a Url internal class. :)</div><div><br></div><div>M=C3=
=A1t=C3=A9</div><div><br></div></div>

--0000000000007f6141061c0f130f--