Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:118186
MIME-Version: 1.0
References: <7d72b5c1-d19b-8734-57e0-f02c402b09c2@gmail.com>
 <CAEKnhAEUPn48mOEFPUfYCp23F_PjtwtEjOUY0V2vOwom6rPjwQ@mail.gmail.com>
 <64e73280-83aa-c60d-4c01-54a617c5468f@gmail.com> <CAEKnhAF21rdAARVMjXq44+rjLej9rEGNigr-iPXP4YWNKkTfmQ@mail.gmail.com>
In-Reply-To: <CAEKnhAF21rdAARVMjXq44+rjLej9rEGNigr-iPXP4YWNKkTfmQ@mail.gmail.com>
Date: Tue, 5 Jul 2022 01:04:25 +0100
Message-ID: <CAPg3Xx+d1jkpCG_iV464hJWdC_UqEpns-F-Phtog4bA2DbJEJg@mail.gmail.com>
To: Jakub Zelenka <bukka@php.net>
Cc: Timon de Groot <tdegroot96@gmail.com>, PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="0000000000009e0cb605e3039a2b"
Subject: Re: [PHP-DEV] [RFC] Add json_encode indent parameter
From: petercowburn@gmail.com (Peter Cowburn)

--0000000000009e0cb605e3039a2b
Content-Type: text/plain; charset="UTF-8"

On Mon, 4 Jul 2022 at 11:11, Jakub Zelenka <bukka@php.net> wrote:

> On Mon, Jul 4, 2022 at 8:38 AM Timon de Groot <tdegroot96@gmail.com>
> wrote:
>
> > Hi internals,
> >
> > If the rest also thinks the RFC is good to go, I suggest we start a vote
> > coming week.
> > As this is my first RFC, I'm not so sure how this typically gets kicked
> > off, so I'd like to know what I need to do!
> >
>
> You just need to change status in RFC to Voting and in voting section set
> the date range and add doodle poll - you can basically copy it from one of
> the open RFC's (see for example code in
> https://wiki.php.net/rfc/fetch_property_in_const_expressions ). Then you
> just need to send email to internals with subject prefixed with [RFC][VOTE]
> or similar. As noted you need to do it today or latest tomorrow. Feel free
> to let me know if you are too busy or something is not clear.
>
> I would recommend not to do any last time changes as in such case you
> should probably give an extra time for dicusion which means it won't make
> the feature freeze and will have to wait for another year. In my opinion it
> is good as it is. The tabs can be later added as an extra flag. If the flag
> is set, we could just change default value for indent to 1 and use tabs but
> it would be still possible for users to set indent size if they wish. I
> think that's something that makes most sense to me and doesn't impact the
> current RFC in any way.
>
> Regards
>
> Jakub
>

My thoughts might be firmly in the realms of "too little, too late" since
the vote is open already, so by all means choose to ignore.

Things that I would have liked to have seen in the RFC document:
* the mention/consideration of a user-specified indent string
  (even if under a "Rejected Features" section with some details about the
rejection)
* details on the new parameter's interaction with / dependency on
JSON_PRETTY_PRINT.
* related, details on what happens when the new parameter is used without
JSON_PRETTY_PRINT.
  (I'd personally like to have JSON_PRETTY_PRINT be implicitly added if the
new parameter is used, or it could
   raise an error, or it could be ignored as seems to be the case but isn't
explicltly mentioned)
* details on allowed values of the new parameter, e.g. what happens with
negative, or stupidly huge, values?
* some summary (more than none!) of discussion topics / design decisions
resulting from the mailing list thread(s)

The RFC currently states (the quote is two-thirds of the proposal text,
minus examples):

> By default, an indentation of 4 spaces will be applied, just like the
original json_encode behaviour with the JSON_PRETTY_PRINT option.
>
> When the indent parameter is passed a different value, an indentation of
N spaces will be applied.

Even after several readings of the proposal, I thought that you were
proposing that json_encode() always be pretty-printed and indented.

Even the examples weren't particularly helpful in correcting
that mis-reading. It took me far too long to understand (hopefully
correctly) that:
* "By default..." means "When the flags include JSON_PRETTY_PRINT and the
indent parameter is not passed (or 4 is passed)...".
* "When the indent parameter is passed..." really means "When the flags
include JSON_PRETTY_PRINT and the indent parameter is passed...".
* Any calls to json_encode() without JSON_PRETTY_PRINT, with or without the
indent parameter, behave identically to now.

The "Unaffected PHP Functionality" section only muddied the waters further:
    "Normal usage ... of the json_encode function will not be affected, as
the default of 4 spaces will still be in effect."

My point being, I don't think this document was anywhere near ready... but
feel free to disagree.

Just to finish up, wouldn't it be nice to have the following?..

    json_encode($value, indent: 2)

Regards,

Peter

--0000000000009e0cb605e3039a2b--