Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:115635
Message-ID: <A4C46EDC-5D29-4F3C-8F88-25C86D49B4B8@newclarity.net>
Content-Type: multipart/alternative;
	boundary="Apple-Mail=_855C6250-F649-43B3-A949-7FE8BB02B90D"
Mime-Version: 1.0 (Mac OS X Mail 13.4 \(3608.120.23.2.7\))
Date: Fri, 6 Aug 2021 03:45:53 -0400
In-Reply-To: <CAGBsUrfoAT4CeiJvbNDczjiqRLLikdvmCfYe2BdUQDZudZ2w4w@mail.gmail.com>
Cc: Internals <internals@lists.php.net>,
 ignace nyamagana butera <nyamsprod@gmail.com>
To: Kamil Tekiela <tekiela246@gmail.com>
References: <7DA18959-3463-4AAE-ABE3-B9330FEE83D2@gmail.com>
 <CAGBsUrfoAT4CeiJvbNDczjiqRLLikdvmCfYe2BdUQDZudZ2w4w@mail.gmail.com>
Subject: Re: [PHP-DEV] [RFC] Add parse_query_string as an alternative to
 parse_str
From: mike@newclarity.net (Mike Schinkel)

--Apple-Mail=_855C6250-F649-43B3-A949-7FE8BB02B90D
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=us-ascii

> On Aug 5, 2021, at 6:21 PM, Kamil Tekiela <tekiela246@gmail.com> =
wrote:
>=20
> I have added implementation for
> https://wiki.php.net/rfc/parse_str_alternative. If there are no other
> comments, I would like to start voting on Saturday.


I too would appreciate having a function in the PHP library that returns =
an array and that is named more intuitively than parse_str().

However, I would suggest naming it `parse_query()` instead of =
`parse_query_string()` as `_string()` is redundant and I see shorter =
function names being preferable when the intent of the function is =
clear.

I searched for prior art and it appears that Guzzle's PSR7 helper =
library v1.x had a namespaced parse_query() function:

https://github.com/guzzle/psr7/blob/1.x/src/functions.php#L299 =
<https://github.com/guzzle/psr7/blob/1.x/src/functions.php#L299>

I found Psr7\parse_query() being called on GitHub in 840 places using =
SourceGraph code search:

=
https://sourcegraph.com/search?q=3Dcontext:global+lang:php+Psr7%5Cparse_qu=
ery%28+count:all&patternType=3Dliteral =
<https://sourcegraph.com/search?q=3Dcontext:global+lang:php+Psr7%5Cparse_q=
uery%28+count:all&patternType=3Dliteral>

There was only one (1) place with SourceGraph code search where someone =
named a function parse_query_string(), and that example did not use =
parse_query_string() in the same way as your RFC:

=
https://sourcegraph.com/github.com/k0a1a/hotglue2/-/blob/controller.inc.ph=
p?L344:1 =
<https://sourcegraph.com/github.com/k0a1a/hotglue2/-/blob/controller.inc.p=
hp?L344:1>

My takeaway is that PHP developers will easily be able to understand the =
intent if named `parse_query()`, especially those who might be familiar =
with it from Guzzle, and thus there is no need for the redundant =
`_string()`. =20

And, of course, a parse_query() in the global namespace won't conflict =
with Guzzle's use because their function is namespaced with Guzzle\Psr7.

> On Aug 6, 2021, at 3:17 AM, ignace nyamagana butera =
<nyamsprod@gmail.com> wrote:
>=20
> I feel that we are missing a chance to also improve how parse_str
> algorithm is currently used, we could or should (?) use this =
opportunity
> to fix some long shortcomings around parse_str.

Also, +1 to this.

-Mike

P.S. WordPress has a parse_query() *method* on their WP_Query() class, =
but as someone who has worked with WordPress for 10+ years even I don't =
see these two in conflict. One is a method scoped to the solution domain =
of its class and the other would be a function in PHP's global =
namespace.=

--Apple-Mail=_855C6250-F649-43B3-A949-7FE8BB02B90D--