Page MenuHomePhabricator

Backlinks output format changed
Closed, ResolvedPublic

Assigned To
Authored By
Mr.Z-man
Feb 19 2009, 12:56 AM
Referenced Files
F5543: diff3
Nov 21 2014, 10:32 PM
F5544: diff4
Nov 21 2014, 10:32 PM
F5542: diff
Nov 21 2014, 10:32 PM

Description

After the last scap on Wikimedia, backlinks/embeddedin queries have started using a different JSON output format from the other list= queries, from an array to an object. The generator queries are also buggy, returning an empty array before the object with the pages - http://en.wikipedia.org/w/api.php?action=query&format=jsonfm&generator=embeddedin&geititle=Template:Fact


Version: 1.15.x
Severity: normal
URL: http://en.wikipedia.org/w/api.php?action=query&format=jsonfm&list=embeddedin&eititle=Template:Fact

Details

Reference
bz17563

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 10:32 PM
bzimport set Reference to bz17563.

Created attachment 5830
Patch against r47478 to fix the issue

This looks like a large patch, but it's mostly indentation. The actual changes are:

  • The whole block of code calling addValue is only called if $resultPageSet is null (i.e. non-generator mode). This results in all the indenting.
  • $this->resultArr is passed through array_values in the "try to add them all at once" case, so the output is as an array instead of an object.
  • In the "add elements one by one" case, similarly don't specify a key.
  • Slight rearrangement at the bottom of the function to eliminate a redundant conditional.

Attached:

ezyang wrote:

It's considered good form to separate whitespace/code formatting and actual code changes when constructing patches.

Created attachment 5831
The requested patch with b0rken indentation

Well, if you really want a patch that has b0rken indentation just so a followup can fix it...

Attached:

Created attachment 5832
The requested patch to fix the whitespace from the previous patch

And here's the followup to fix the b0rken indentation.

Attached:

(In reply to comment #1)

Created an attachment (id=5830) [details]
Patch against r47478 to fix the issue

Applied verbatim in r47514.

(In reply to comment #2)

It's considered good form to separate whitespace/code formatting and actual
code changes when constructing patches.

If the whitespace changes are completely unrelated, that's definitely true. This case, where an if() is added and the statements inside it are indented and changed is kind of a corner case. I decided to commit the whole thing in one go, but having a patch without whitespace changes certainly made review clearer.