Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow JSON_PRETTY_PRINT #46

Open
williamdes opened this issue May 19, 2021 · 5 comments
Open

Allow JSON_PRETTY_PRINT #46

williamdes opened this issue May 19, 2021 · 5 comments

Comments

@williamdes
Copy link
Contributor

protected function calculateEncodeOptions()

@williamdes
Copy link
Contributor Author

At phpMyAdmin while adding your lib on our tests I had to hack this to enable pretty print because reading diffs on PRs is very important for us.

        $serializer = new JsonSerializer();
        $encoded = $serializer->serialize($test);
        $encoded = json_encode(json_decode($encoded), JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE | JSON_PRESERVE_ZERO_FRACTION);

williamdes added a commit to phpmyadmin/sql-parser that referenced this issue May 19, 2021
Ref: zumba/json-serializer#46 (for why the JSON pretty print hack is needed)

Signed-off-by: William Desportes <[email protected]>
@williamdes
Copy link
Contributor Author

@williamdes
Copy link
Contributor Author

Hi @jrbasso
Should I make a PR ?

@jrbasso
Copy link
Member

jrbasso commented Aug 30, 2021

@williamdes Sorry, I totally missed this issue. I'm sorry.

I think just adding the JSON_PRETTY_PRINT on that list would cause backward incompatibility and a possibility of break some systems that expect always one line. Also, it goes inverse as optimization.
However, I think the case is valid and maybe adding a new method to specify the encoding modifiers is a good way to handle it, so it becomes optional and whoever set these flags will expect a different output format.

@williamdes
Copy link
Contributor Author

I think just adding the JSON_PRETTY_PRINT on that list would cause backward incompatibility and a possibility of break some systems that expect always one line. Also, it goes inverse as optimization.
However, I think the case is valid and maybe adding a new method to specify the encoding modifiers is a good way to handle it, so it becomes optional and whoever set these flags will expect a different output format.

Sure !
We will be happy to use this new function as for us the diff between JSON files needs to be visible

Here is the old diffs, totally un-readable PHP serialize data: phpmyadmin/sql-parser@b60d7e7

And here you can see that with you lib and pretty json it make quite a difference in changes tracking !
See: phpmyadmin/sql-parser@e0ba76f#diff-5223b8b0039f0b42219f7e98df65f2f0b2595a3a3880a5376502a25d1679be73

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants