From 1efdda25ee5cbaea2b7e2dd7afdbaf53063d4b68 Mon Sep 17 00:00:00 2001 From: Kevin Israel Date: Wed, 2 Apr 2014 21:51:09 -0400 Subject: [PATCH] FormatJson: Make it possible to change the indent string This is to allow consistency with MediaWiki PHP and JS files (e.g. when generating JSON i18n files), not because tabs are "better" than spaces for indenting code (both have advantages and disadvantages). Because PHP's json_encode() function hardcodes the indent string, using tabs has a performance cost (in post-processing the output) and is less suitable for web output; thus the API and ResourceLoader debug mode will continue to use four spaces. Adjusting the maintenance scripts and JSON files is left to separate change sets. Bug: 63444 Change-Id: Ic915c50b0acd2e236940b70d5dd48ea87954c9d5 --- RELEASE-NOTES-1.24 | 2 + includes/json/FormatJson.php | 39 +++++++++++----- .../phpunit/includes/json/FormatJsonTest.php | 44 +++++++++++++------ 3 files changed, 61 insertions(+), 24 deletions(-) diff --git a/RELEASE-NOTES-1.24 b/RELEASE-NOTES-1.24 index 0e7fbe680a..4e2b6a9e16 100644 --- a/RELEASE-NOTES-1.24 +++ b/RELEASE-NOTES-1.24 @@ -11,6 +11,8 @@ production. === Configuration changes in 1.24 === === New features in 1.24 === +* (bug 63444) Made it possible to change the indent string (default: 4 spaces) + used by FormatJson::encode(). === Bug fixes in 1.24 === * (bug 62258) A bug was fixed in File::getUnscaledThumb when a height diff --git a/includes/json/FormatJson.php b/includes/json/FormatJson.php index 83718c3103..97d98a4895 100644 --- a/includes/json/FormatJson.php +++ b/includes/json/FormatJson.php @@ -95,11 +95,17 @@ class FormatJson { * (cf. FormatJson::XMLMETA_OK). Use Xml::encodeJsVar() instead in such cases. * * @param mixed $value The value to encode. Can be any type except a resource. - * @param bool $pretty If true, add non-significant whitespace to improve readability. + * @param string|bool $pretty If a string, add non-significant whitespace to improve + * readability, using that string for indentation. If true, use the default indent + * string (four spaces). * @param int $escaping Bitfield consisting of _OK class constants * @return string|bool: String if successful; false upon failure */ public static function encode( $value, $pretty = false, $escaping = 0 ) { + if ( !is_string( $pretty ) ) { + $pretty = $pretty ? ' ' : false; + } + if ( defined( 'JSON_UNESCAPED_UNICODE' ) ) { return self::encode54( $value, $pretty, $escaping ); } @@ -125,7 +131,7 @@ class FormatJson { * JSON encoder wrapper for PHP >= 5.4, which supports useful encoding options. * * @param mixed $value - * @param bool $pretty + * @param string|bool $pretty * @param int $escaping * @return string|bool */ @@ -134,7 +140,7 @@ class FormatJson { // which is hardly useful when '<' and '>' are escaped (and inadequate), and such // escaping negatively impacts the human readability of URLs and similar strings. $options = JSON_UNESCAPED_SLASHES; - $options |= $pretty ? JSON_PRETTY_PRINT : 0; + $options |= $pretty !== false ? JSON_PRETTY_PRINT : 0; $options |= ( $escaping & self::UTF8_OK ) ? JSON_UNESCAPED_UNICODE : 0; $options |= ( $escaping & self::XMLMETA_OK ) ? 0 : ( JSON_HEX_TAG | JSON_HEX_AMP ); $json = json_encode( $value, $options ); @@ -142,10 +148,22 @@ class FormatJson { return false; } - if ( $pretty ) { + if ( $pretty !== false ) { // Remove whitespace inside empty arrays/objects; different JSON encoders // vary on this, and we want our output to be consistent across implementations. $json = preg_replace( self::WS_CLEANUP_REGEX, '', $json ); + if ( $pretty !== ' ' ) { + // Change the four-space indent to a tab indent + $json = str_replace( "\n ", "\n\t", $json ); + while ( strpos( $json, "\t " ) !== false ) { + $json = str_replace( "\t ", "\t\t", $json ); + } + + if ( $pretty !== "\t" ) { + // Change the tab indent to the provided indent + $json = str_replace( "\t", $pretty, $json ); + } + } } if ( $escaping & self::UTF8_OK ) { $json = str_replace( self::$badChars, self::$badCharsEscaped, $json ); @@ -159,7 +177,7 @@ class FormatJson { * Therefore, the missing options are implemented here purely in PHP code. * * @param mixed $value - * @param bool $pretty + * @param string|bool $pretty * @param int $escaping * @return string|bool */ @@ -188,8 +206,8 @@ class FormatJson { $json = str_replace( self::$badChars, self::$badCharsEscaped, $json ); } - if ( $pretty ) { - return self::prettyPrint( $json ); + if ( $pretty !== false ) { + return self::prettyPrint( $json, $pretty ); } return $json; @@ -200,9 +218,10 @@ class FormatJson { * Only needed for PHP < 5.4, which lacks the JSON_PRETTY_PRINT option. * * @param string $json + * @param string $indentString * @return string */ - private static function prettyPrint( $json ) { + private static function prettyPrint( $json, $indentString ) { $buf = ''; $indent = 0; $json = strtr( $json, array( '\\\\' => '\\\\', '\"' => "\x01" ) ); @@ -217,11 +236,11 @@ class FormatJson { ++$indent; // falls through case ',': - $buf .= $json[$i] . "\n" . str_repeat( ' ', $indent ); + $buf .= $json[$i] . "\n" . str_repeat( $indentString, $indent ); break; case ']': case '}': - $buf .= "\n" . str_repeat( ' ', --$indent ) . $json[$i]; + $buf .= "\n" . str_repeat( $indentString, --$indent ) . $json[$i]; break; case '"': $skip = strcspn( $json, '"', $i + 1 ) + 2; diff --git a/tests/phpunit/includes/json/FormatJsonTest.php b/tests/phpunit/includes/json/FormatJsonTest.php index 8359f0d2e7..307b3551e9 100644 --- a/tests/phpunit/includes/json/FormatJsonTest.php +++ b/tests/phpunit/includes/json/FormatJsonTest.php @@ -5,7 +5,22 @@ */ class FormatJsonTest extends MediaWikiTestCase { - public function testEncoderPrettyPrinting() { + public static function provideEncoderPrettyPrinting() { + return array( + // Four spaces + array( true, ' ' ), + array( ' ', ' ' ), + // Two spaces + array( ' ', ' ' ), + // One tab + array( "\t", "\t" ), + ); + } + + /** + * @dataProvider provideEncoderPrettyPrinting + */ + public function testEncoderPrettyPrinting( $pretty, $expectedIndent ) { $obj = array( 'emptyObject' => new stdClass, 'emptyArray' => array(), @@ -22,23 +37,24 @@ class FormatJsonTest extends MediaWikiTestCase { ), ); - // 4 space indent, no trailing whitespace, no trailing linefeed + // No trailing whitespace, no trailing linefeed $json = '{ - "emptyObject": {}, - "emptyArray": [], - "string": "foobar\\\\", - "filledArray": [ - [ - 123, - 456 - ], - "\"7\":[\"8\",{\"9\":\"10\"}]", - "{\n\t\"emptyObject\": {\n\t},\n\t\"emptyArray\": [ ]\n}" - ] + "emptyObject": {}, + "emptyArray": [], + "string": "foobar\\\\", + "filledArray": [ + [ + 123, + 456 + ], + "\"7\":[\"8\",{\"9\":\"10\"}]", + "{\n\t\"emptyObject\": {\n\t},\n\t\"emptyArray\": [ ]\n}" + ] }'; $json = str_replace( "\r", '', $json ); // Windows compat - $this->assertSame( $json, FormatJson::encode( $obj, true ) ); + $json = str_replace( "\t", $expectedIndent, $json ); + $this->assertSame( $json, FormatJson::encode( $obj, $pretty ) ); } public static function provideEncodeDefault() { -- 2.20.1