Skip to content

[RFC] Add json_encode indent parameter #7093

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.

Sign up for GitHub

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

Closed
wants to merge 24 commits into from
Closed

[RFC] Add json_encode indent parameter #7093

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
40747f4
Initial work for json_encode indentation option
tdgroot Jun 3, 2021
2bf91bd
Add string|int support for json_encode indent param
tdgroot Jun 3, 2021
98c208b
Fix formatting
tdgroot Jun 3, 2021
9d8a35a
Fix potential segfault
tdgroot Jun 3, 2021
727713d
Fix pointer initial value
tdgroot Jun 3, 2021
548d034
Update json.stub.php
tdgroot Jun 3, 2021
5566e76
Add error handling and use zend_string*
tdgroot Jun 4, 2021
d808cdc
Update ext/json/json.c
Jun 4, 2021
f2554ec
Update ext/json/json.c
Jun 4, 2021
f5b8fc7
Update ext/json/json.c
Jun 4, 2021
7725e9a
Early exit in php_json_pretty_print_indent
tdgroot Jun 4, 2021
c34482b
Move indent_str init to implicit function
tdgroot Jun 4, 2021
1005827
Cleanup json.c and add comments
tdgroot Jun 4, 2021
7345d24
Update ext/json/json.c
Jun 5, 2021
fada489
Merge branch 'master' into json_encode_indentation
tdgroot May 13, 2022
d416b4f
Simplify json_encode indent parameter
tdgroot May 13, 2022
13d138a
Add missing EOF to testfile
tdgroot May 13, 2022
f36942d
Remove unused global
tdgroot May 13, 2022
fcc1045
Update ext/json/json.c
tdgroot May 13, 2022
e7d7b4b
Update ext/json/tests/json_encode_pretty_print_indent_error.phpt
tdgroot May 13, 2022
eea6007
fixup! Update ext/json/tests/json_encode_pretty_print_indent_error.phpt
tdgroot May 13, 2022
574dc0c
Make the indent of internal json_encode more coherent with userspace …
tdgroot May 16, 2022
abaa117
Merge branch 'master' of github.com:php/php-src into json_encode_inde…
tdgroot Jul 4, 2022
b8b941d
Update json_arginfo.h
tdgroot Jul 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 34 additions & 3 deletions ext/json/json.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,13 +141,14 @@ static PHP_MINFO_FUNCTION(json)
}
/* }}} */

PHP_JSON_API int php_json_encode_ex(smart_str *buf, zval *val, int options, zend_long depth) /* {{{ */
PHP_JSON_API int php_json_encode_ex(smart_str *buf, zval *val, int options, zend_long depth, zend_string *indent_str) /* {{{ */
{
php_json_encoder encoder;
int return_code;

php_json_encode_init(&encoder);
encoder.max_depth = depth;
encoder.indent_str = indent_str;

return_code = php_json_encode_zval(buf, val, options, &encoder);
JSON_G(error_code) = encoder.error_code;
Expand All @@ -158,7 +159,13 @@ PHP_JSON_API int php_json_encode_ex(smart_str *buf, zval *val, int options, zend

PHP_JSON_API int php_json_encode(smart_str *buf, zval *val, int options) /* {{{ */
{
return php_json_encode_ex(buf, val, options, JSON_G(encode_max_depth));
zend_string *indent_str = zend_string_init(" ", strlen(" "), 0);

int result = php_json_encode_ex(buf, val, options, JSON_G(encode_max_depth), 0);

zend_string_release(indent_str);

return result;
}
/* }}} */

Expand Down Expand Up @@ -222,20 +229,44 @@ PHP_FUNCTION(json_encode)
zval *parameter;
php_json_encoder encoder;
smart_str buf = {0};
zend_string *indent_str = NULL;
zend_long options = 0;
zend_long depth = PHP_JSON_PARSER_DEFAULT_DEPTH;
zend_long indent = PHP_JSON_ENCODER_DEFAULT_INDENT;

ZEND_PARSE_PARAMETERS_START(1, 3)
ZEND_PARSE_PARAMETERS_START(1, 4)
Z_PARAM_ZVAL(parameter)
Z_PARAM_OPTIONAL
Z_PARAM_LONG(options)
Z_PARAM_LONG(depth)
Z_PARAM_STR_OR_LONG(indent_str, indent)
ZEND_PARSE_PARAMETERS_END();

if (indent_str == NULL) {
if (indent < 0) {
zend_argument_value_error(4, "must be either a string or a number greater than or equal to 0");
RETURN_THROWS();
}

if (indent > 0) {
// Generate indent_str
indent_str = zend_string_alloc(indent, 0);
for (zend_long i = 0; i < indent; i++)
ZSTR_VAL(indent_str)[i] = ' ';
}
}

php_json_encode_init(&encoder);
encoder.max_depth = (int)depth;
encoder.indent_str = indent_str;

php_json_encode_zval(&buf, parameter, (int)options, &encoder);

if (indent > 0 && indent_str) {
// If the indent_str was generated locally, we need to release it.
zend_string_release(indent_str);
}

if (!(options & PHP_JSON_THROW_ON_ERROR) || (options & PHP_JSON_PARTIAL_OUTPUT_ON_ERROR)) {
JSON_G(error_code) = encoder.error_code;
if (encoder.error_code != PHP_JSON_ERROR_NONE && !(options & PHP_JSON_PARTIAL_OUTPUT_ON_ERROR)) {
Expand Down
2 changes: 1 addition & 1 deletion ext/json/json.stub.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

/** @generate-class-entries */

function json_encode(mixed $value, int $flags = 0, int $depth = 512): string|false {}
function json_encode(mixed $value, int $flags = 0, int $depth = 512, string|int $indent = 4): string|false {}
Copy link
Contributor

@mvorisek mvorisek Jun 4, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think accepting string only here is enough, passing ' ' is easy as passing 4 but simple type is enough.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does make it more difficult to see that $indent=' ' specifies 4 spaces and that this indentation is actually consistent with some other call site that uses $indent=4.

Also, I prefer parameters having pure types rather than composite. Since we have named parameters now it might make sense to split into two parameters:

json_encode($value, indent: 4, indent_char: ' ');

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @dtakken about pure types rather than composite. As I was scanning the code I kept trying to figure out why the string|int types were being used instead of only int. Realizing finally if a string is passed as the argument the string literally is the indent value.
However, spiting this into two parameters does remove or complicate the feature of multiple characters:

json_encode($value, indent: 4, indent_chars: '🚀🌙🌠🌎');
// What to do if indent = 2 and indent_chars has more than 2 chars?
json_encode($value, indent: 2, indent_chars: '🚀🌙🌠'); // error? ignore the 3rd char?
// What to do if indent = 3 and indent_chars has less than 3 chars?
json_encode($value, indent: 3, indent_chars: '🚀🌙'); // repeat '🚀🌙🚀'? error? something else?


function json_decode(string $json, ?bool $associative = null, int $depth = 512, int $flags = 0): mixed {}

Expand Down
3 changes: 2 additions & 1 deletion ext/json/json_arginfo.h
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 6 additions & 1 deletion ext/json/json_encoder.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,14 @@ static inline void php_json_pretty_print_indent(smart_str *buf, int options, php
{
int i;

int indent_length = encoder->indent_str ? ZSTR_LEN(encoder->indent_str) : 0;
if (!indent_length) {
return;
}

if (options & PHP_JSON_PRETTY_PRINT) {
for (i = 0; i < encoder->depth; ++i) {
smart_str_appendl(buf, " ", 4);
smart_str_appendl(buf, ZSTR_VAL(encoder->indent_str), indent_length);
}
}
}
Expand Down
6 changes: 5 additions & 1 deletion ext/json/php_json.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,13 @@ typedef enum {
/* default depth */
#define PHP_JSON_PARSER_DEFAULT_DEPTH 512

/* default indent */
#define PHP_JSON_ENCODER_DEFAULT_INDENT 4

ZEND_BEGIN_MODULE_GLOBALS(json)
int encoder_depth;
int encode_max_depth;
int encoder_indent;
php_json_error_code error_code;
ZEND_END_MODULE_GLOBALS(json)

Expand All @@ -97,7 +101,7 @@ PHP_JSON_API ZEND_EXTERN_MODULE_GLOBALS(json)
ZEND_TSRMLS_CACHE_EXTERN()
#endif

PHP_JSON_API int php_json_encode_ex(smart_str *buf, zval *val, int options, zend_long depth);
PHP_JSON_API int php_json_encode_ex(smart_str *buf, zval *val, int options, zend_long depth, zend_string *indent_str);
PHP_JSON_API int php_json_encode(smart_str *buf, zval *val, int options);
PHP_JSON_API int php_json_decode_ex(zval *return_value, const char *str, size_t str_len, zend_long options, zend_long depth);

Expand Down
1 change: 1 addition & 0 deletions ext/json/php_json_encoder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ struct _php_json_encoder {
int depth;
int max_depth;
php_json_error_code error_code;
zend_string *indent_str;
};

static inline void php_json_encode_init(php_json_encoder *encoder)
Expand Down
23 changes: 23 additions & 0 deletions ext/json/tests/json_encode_pretty_print3.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
--TEST--
json_encode() with JSON_PRETTY_PRINT with 2 spaces string indentation
--FILE--
<?php

$data = [
'key' => 4,
'other_key' => [0, 1, 2, 3]
];

echo json_encode($data, JSON_PRETTY_PRINT, 512, 2);

?>
--EXPECT--
{
"key": 4,
"other_key": [
0,
1,
2,
3
]
}
23 changes: 23 additions & 0 deletions ext/json/tests/json_encode_pretty_print4.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
--TEST--
json_encode() with JSON_PRETTY_PRINT with custom 🚀 string indentation
--FILE--
<?php

$data = [
'key' => 4,
'other_key' => [0, 1, 2, 3]
];

echo json_encode($data, JSON_PRETTY_PRINT, 512, '🚀');

?>
--EXPECT--
{
🚀"key": 4,
🚀"other_key": [
🚀🚀0,
🚀🚀1,
🚀🚀2,
🚀🚀3
🚀]
}
25 changes: 25 additions & 0 deletions ext/json/tests/json_encode_pretty_print_indent_error.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
--TEST--
json_encode() with JSON_PRETTY_PRINT with negative indentation
--FILE--
<?php
echo "*** Testing json_encode() : error conditions ***\n";

echo "\n-- Testing json_encode() function (JSON_PRETTY_PRINT) with negative indentation --\n";

$data = [
'key' => 4,
'other_key' => [0, 1, 2, 3]
];

try {
echo json_encode($data, JSON_PRETTY_PRINT, 512, -2);
} catch (\ValueError $e) {
echo $e->getMessage() . \PHP_EOL;
}

?>
--EXPECT--
*** Testing json_encode() : error conditions ***

-- Testing json_encode() function (JSON_PRETTY_PRINT) with negative indentation --
json_encode(): Argument #4 ($indent) must be either a string or a number greater than or equal to 0