Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Allow ArcanistDiffParser to parse `diff.suppress-blank-empty` Git diffs

Summary: See D3963. Instead, parse these diffs so they'll work with `--raw`, etc.

Test Plan:
Generated a failing diff, added it as a test case. Fixed issue. Ran test suite. Ran `arc` against it:

  $ git -c diff.suppress-blank-empty=true diff HEAD | arc diff --raw --only --conduit-uri=http://local.aphront.com:8080/
  Reading diff from stdin...
  Created a new Differential diff:
          Diff URI: http://local.aphront.com:8080/differential/diff/103/

  Included changes:
    M       things

Reviewers: vrana, jiiix, btrahan

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D3969
  • Loading branch information...
commit 2d3d7be09a48ec34f3eb00cb390526f6bd2f97cf 1 parent 515399c
@epriestley epriestley authored
View
29 src/parser/ArcanistDiffParser.php
@@ -840,12 +840,17 @@ protected function parseChangeset(ArcanistDiffChange $change) {
$add = 0;
$del = 0;
- $advance = false;
+ $hit_next_hunk = false;
while ((($line = $this->nextLine()) !== null)) {
- if (strlen($line)) {
+ if (strlen(rtrim($line, "\r\n"))) {
$char = $line[0];
} else {
- $char = '~';
+ // Normally, we do not encouter empty lines in diffs, because
+ // unchanged lines have an initial space. However, in Git, with
+ // the option `diff.suppress-blank-empty` set, unchanged blank lines
+ // emit as completely empty. If we encounter a completely empty line,
+ // treat it as a ' ' (i.e., unchanged empty line) line.
+ $char = ' ';
}
switch ($char) {
case '\\':
@@ -861,20 +866,19 @@ protected function parseChangeset(ArcanistDiffChange $change) {
$hunk->setIsMissingNewNewline(true);
}
if (!$new_len) {
- $advance = true;
break 2;
}
break;
case '+':
- if (!$new_len) {
- break 2;
- }
++$add;
--$new_len;
$real[] = $line;
break;
case '-':
if (!$old_len) {
+ // In this case, we've hit "---" from a new file. So don't
+ // advance the line cursor.
+ $hit_next_hunk = true;
break 2;
}
++$del;
@@ -889,17 +893,14 @@ protected function parseChangeset(ArcanistDiffChange $change) {
--$new_len;
$real[] = $line;
break;
- case "\r":
- case "\n":
- case '~':
- $advance = true;
- break 2;
default:
+ // We hit something, likely another hunk.
+ $hit_next_hunk = true;
break 2;
}
}
- if ($old_len != 0 || $new_len != 0) {
+ if ($old_len || $new_len) {
$this->didFailParse("Found the wrong number of hunk lines.");
}
@@ -939,7 +940,7 @@ protected function parseChangeset(ArcanistDiffChange $change) {
$change->addHunk($hunk);
}
- if ($advance) {
+ if (!$hit_next_hunk) {
$line = $this->nextNonemptyLine();
}
View
3  src/parser/__tests__/ArcanistDiffParserTestCase.php
@@ -536,6 +536,9 @@ private function parseDiff($diff_file) {
case 'more-newlines.svndiff':
$this->assertEqual(1, count($changes));
break;
+ case 'suppress-blank-empty.gitdiff':
+ $this->assertEqual(1, count($changes));
+ break;
default:
throw new Exception("No test block for diff file {$diff_file}.");
break;
View
11 src/parser/__tests__/diff/suppress-blank-empty.gitdiff
@@ -0,0 +1,11 @@
+diff --git a/things b/things
+index b6c8793..0c96a0f 100644
+--- a/things
++++ b/things
+@@ -1,5 +1,5 @@
+ apple
+
+-banana
++bananas
+
+ cherry
Please sign in to comment.
Something went wrong with that request. Please try again.