Page MenuHomePhabricator

ResourceLoader should pretty print JSON in mw.config/mw.user.options etc. in debug mode
Closed, ResolvedPublic

Description

Author: mdale

Description:
It would make it a lot easier to quickly see whats going on if JSON output of the resource loader was human readable formatted or at least put a new line after every comma in debug mode.

For example, its very difficult to quickly read what site variables you have available, in contrast the existing deployment page output has each site configuration variable on a new line and is easy to read.


Version: unspecified
Severity: enhancement
See Also:
https://bugzilla.wikimedia.org/show_bug.cgi?id=47431

Details

Reference
bz26818

Event Timeline

bzimport raised the priority of this task from to Low.Nov 21 2014, 11:16 PM
bzimport set Reference to bz26818.

We use json_encode, so it's minified from the get-go, but we could use something like this http://recursive-design.com/blog/2008/03/11/format-json-with-php/ when in debug mode.

One might also consider interactive debugging helpers -- these days devs pretty much always have either Firebug or the more basic debug consoles available in current Firefox, Chrome/Safari/WebKit, Opera, and IE 8+.

We could query the configuration info in a console at runtime -- or even trigger a visual display of what vars & modules have been set up. These would have the advantage that you could activate them at any time, without having to switch the page to debug mode to view source.

public static function encode( $value, $isHtml = false ) {

FormatJson::encode( $text, $true );

Is essentially what the API uses...

Seems a bit pointless re-inventing the wheel :)

(In reply to comment #1)

We use json_encode, so it's minified from the get-go, but we could use
something like this
http://recursive-design.com/blog/2008/03/11/format-json-with-php/ when in debug
mode.

We don't use json_encode directly (at least not anymore, but I don't think we ever did). We use FormatJson::encode which uses json_encode only if it is supported in the current php version and is not the "buggy" version. As Sam pointed out, FormatJson has a "pretty print" option which is also one of the conditions not to use json_encode.

Although, come to think of it, as of PHP 5.4.0 json_encode supports that too[1] (through JSON_PRETTY_PRINT), so we can fix FormatJson to reach out to the native method more often.

[1] http://php.net/json_encode

Per the current code[1], implementing this is not easy. First of all because it is a static function, so it needs to have access to the ResourceLoaderContext. Probably easiest to make it a public function instead of a static function, so that context is available.

And then it needs to not use Xml::encodeJsCall, because that doesn't have any of those options and doesn't use FormatJson.

[1] https://gerrit.wikimedia.org/r/gitweb?p=mediawiki/core.git;a=blob;f=includes/resourceloader/ResourceLoader.php;h=b3d7e75786e0c53b0be4982bba970aa62168c335;hb=HEAD#l994

gerrit 50140

Patch pretty prints only configuration variables, user options, and user tokens, covering the most problematic cases mentioned by the reporter. Further improvement might still be possible.

I would like to know whether gerrit 50140 (as well as the follow-ups also under the "combine-encoding" topic) will be part of MediaWiki 1.21, because if not, I would like to fix the release notes and doc comments now.

The change was merged between when REL1_21 was branched from master and when master's $wgVersion was bumped to 1.22alpha.

(In reply to comment #8)

The change was merged between when REL1_21 was branched from master and when
master's $wgVersion was bumped to 1.22alpha.

Setting tentative Backport_to_Stable? flag.

Related URL: https://gerrit.wikimedia.org/r/59265 (Gerrit Change I1987190f1ba5bf41738e7bd611209706c1f6bb5c)

(In reply to comment #9)

(In reply to comment #8)

The change was merged between when REL1_21 was branched from master and when
master's $wgVersion was bumped to 1.22alpha.

Setting tentative Backport_to_Stable? flag.

Most of the stack are enhancements, but one of them is a bug fix and one for improved stability. I'd be for backporting.

Backported in:

  • gerrit 59265
  • gerrit 59328
  • gerrit 59329

Yet reverted in gerrit 60080 *not* because of a bug in the code,
but rather because of a truly unforeseen licensing problem with the PHP JSON
extension (a use restriction in the license of Crockford's JSON_checker).

The aforementioned revert only applies to REL1_21, not to master.

See bug 47431 for details about the licensing problem and its (future)
resolution.

Changed Target Milestone from "1.21.0 release" to "1.22.0 release"
to reflect the aforementioned reversion.