recaptime.dev / phorge
@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Differential whitespace mode IGNORE ALL now shows correct indentation

Summary:
Fixed buggy and incomplete logic for handling IGNORE ALL mode properly.
A subparser is used to parse the non-ws-ignoring changeset while a
ws-ignoring changeset is handed off to the original parser. At a later
step, the original parser queries the subparser for its lines of text
(which are formatted properly due to being in non-ws-ignoring mode) and
uses them to replace the text in the ws-ignoring diff.

Task ID: 549940

Test Plan:
-turn off caching temporarily (the cached view is still indented
improperly)
-visit http://phabricator.dev1943.facebook.com/D242591
-note aligned, but not completely highlighted, indentation right above the
comments complaining about
indentation issues

Reviewed By: aran
Reviewers: tuomaspelkonen, jungejason, aran
Commenters: epriestley
CC: aran, epriestley, grglr
Differential Revision: 174

grglr 3f3304fd 4a298125

+18 -17
+18 -17
src/applications/differential/parser/changeset/DifferentialChangesetParser.php
··· 141 141 142 142 $types = array(); 143 143 foreach ($lines as $line_index => $line) { 144 - $lines[$line_index] = $line; 145 144 if (isset($line[0])) { 146 145 $char = $line[0]; 147 146 if ($char == ' ') { ··· 308 307 309 308 $this->old = $old; 310 309 $this->new = $new; 310 + if ($this->subparser) { 311 311 312 - if ($this->subparser && false) { // TODO: This is bugged 312 + // Use this parser's side-by-side line information -- notably, the 313 + // change types -- but replace all the line text with the subparser's. 314 + // This lets us render whitespace-only changes without marking them as 315 + // different. 313 316 314 - // Use the subparser's side-by-side line information -- notably, the 315 - // change types -- but replace all the line text with ours. This lets us 316 - // render whitespace-only changes without marking them as different. 317 - 318 - $old = $this->subparser->old; 319 - $new = $this->subparser->new; 320 - 321 - $old_text = ipull($this->old, 'text', 'line'); 322 - $new_text = ipull($this->new, 'text', 'line'); 317 + $old = $this->old; 318 + $new = $this->new; 319 + $old_text = ipull($this->subparser->old, 'text', 'line'); 320 + $new_text = ipull($this->subparser->new, 'text', 'line'); 323 321 324 322 foreach ($old as $k => $desc) { 325 323 if (empty($desc)) { ··· 406 404 $this->tokenHighlight($this->newRender); 407 405 408 406 $unchanged = false; 409 - if ($this->subparser && false) { 407 + if ($this->subparser) { 410 408 $unchanged = $this->subparser->isUnchanged(); 411 409 $whitelines = $this->subparser->isWhitespaceOnly(); 412 410 } else if (!$changed) { ··· 671 669 EOSYNTHETIC; 672 670 } 673 671 672 + // subparser takes over the current non-whitespace-ignoring changeset 673 + $this->subparser = new DifferentialChangesetParser(); 674 + foreach ($changeset->getHunks() as $hunk) { 675 + $this->subparser->parseHunk($hunk); 676 + } 677 + 678 + // this parser takes new changeset; will use subparser's text later 674 679 $changes = id(new ArcanistDiffParser())->parseDiff($diff); 675 - 676 680 $diff = DifferentialDiff::newFromRawChanges($changes); 677 681 $changesets = $diff->getChangesets(); 678 682 $changeset = reset($changesets); 679 - 680 - $this->subparser = new DifferentialChangesetParser(); 681 - $this->subparser->setChangeset($changeset); 682 - $this->subparser->setWhitespaceMode(self::WHITESPACE_IGNORE_TRAILING); 683 + $this->setChangeset($changeset); 683 684 } 684 685 foreach ($changeset->getHunks() as $hunk) { 685 686 $this->parseHunk($hunk);