Adapt Json\Json for better prettyPrint support #7016
Conversation
add "prettyPrint" option to Json::encode
|
|
||
| $encodeOptions = JSON_HEX_TAG | JSON_HEX_APOS | JSON_HEX_QUOT | JSON_HEX_AMP; | ||
|
|
||
| if (version_compare(phpversion(), '5.4', '>=') && $prettyPrint) { |
There was a problem hiding this comment.
And now this can't be cached/optimezed into bytecode.
Use PHP_VERSION_ID instead.
Or leave version and simply check if constant is defined.
There was a problem hiding this comment.
In addition to that, phpversion() is very unreliable on some distros :-(
There was a problem hiding this comment.
thx, I fixed the version check
| $encodeOptions = JSON_HEX_TAG | JSON_HEX_APOS | JSON_HEX_QUOT | JSON_HEX_AMP; | ||
|
|
||
| if (PHP_VERSION_ID >= 50400 && $prettyPrint) { | ||
| $encodeOptions |= JSON_PRETTY_PRINT; |
There was a problem hiding this comment.
Why not check if JSON_PRETTY_PRINT is defined?
Now one must know the exact vesion of PHP that introduced this constant.
And what about hhvm - they may handle it a bit different (and may not, they for example have some problems with T_FINALLY).
Check if the feature you want is in the environment, not if the environment version is as you want.
add pretty print to Json ViewModel
|
I changed the support of pretty print in json_encode |
|
|
||
| $encodeOptions = JSON_HEX_TAG | JSON_HEX_APOS | JSON_HEX_QUOT | JSON_HEX_AMP; | ||
|
|
||
| if (defined(JSON_PRETTY_PRINT) && $prettyPrint) { |
There was a problem hiding this comment.
must be defined('JSON_PRETTY_PRINT')
There was a problem hiding this comment.
Wouldn't
if ($prettyPrint && defined('JSON_PRETTY_PRINT'))
be faster?
There was a problem hiding this comment.
You're nitpicker ;-)
But otherwise I wouldn't lern anything!
The line is fixed
Adapt Json\Json for better prettyPrint support
|
Merged to develop for release with 2.4. |
Adapt Json\Json for better prettyPrint support
Adapt Json\Json for better prettyPrint support
I adapted the Json::prettyPrint to generate the same output as json_encode
also I added a "prettyPrint" option to Json::encode to take advantage of the json_encode pretty print and align the functionality to json_encode
This is my first pull request ever, so please be kind :-)