Page MenuHomePhabricator

Wrong highlighting for manually uploaded diff
Closed, ResolvedPublic

Description

See https://secure.phabricator.com/differential/diff/37281/

The original diff:

diff --git a/god/logistics_api.php b/god/logistics_api.php
--- a/god/logistics_api.php
+++ b/god/logistics_api.php
@@ -52,14 +52,14 @@
                 foreach($stand->getCompartments() as $comp)
                 {
                     $compData[] = array(
-                        'id' => $comp->getCompartmentId(),
+                        'id' => $comp->getId(),
                         'name' => $comp->getName(),
                         'gridAccess' => $comp->getGridAccess(),
                         'y' => $comp->getYPosition()
                     );
                 }
                 $standData[] = array(
-                    'id' => $stand->getStandId(),
+                    'id' => $stand->getId(),
                     'name' => $stand->getName(),
                     'compartments' => $compData
                 );

At least right now this is rendered as:

pasted_file (280×468 px, 49 KB)

Revisions and Commits

Restricted Differential Revision

Event Timeline

epriestley added a revision: Restricted Differential Revision.Mar 13 2016, 2:07 PM

Thanks, that test case was very helpful.

This was an issue with snippet highlighting in XHPAST. Specifically, some set of conditions along the lines of these were required:

  • diff with missing context;
  • and leading whitespace;
  • PHP language; and
  • XHPAST built on the server.

D15469 should fix this issue.

I didn't bump the parser cache version because the fix was in a different library and the impact is so narrow, but to fix it retroactively you'll need to run bin/cache purge --purge-changeset after updating.

epriestley triaged this task as Normal priority.
epriestley added projects: Differential, XHPAST.
epriestley added a commit: Restricted Diffusion Commit.Mar 13 2016, 2:44 PM

I wouldn't have expected the issue to be in xhpast at all. Thanks for the quick resolution!

I didn't purge the cache here, but here's an updated version of the original diff without the rendering error:

https://secure.phabricator.com/differential/diff/37286/

Thanks for the report, and let us know if you run into anything else.