From 2d3d7be09a48ec34f3eb00cb390526f6bd2f97cf Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 15 Nov 2012 15:47:35 -0800 Subject: [PATCH] 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 --- src/parser/ArcanistDiffParser.php | 29 ++++++++++--------- .../__tests__/ArcanistDiffParserTestCase.php | 3 ++ .../diff/suppress-blank-empty.gitdiff | 11 +++++++ 3 files changed, 29 insertions(+), 14 deletions(-) create mode 100644 src/parser/__tests__/diff/suppress-blank-empty.gitdiff diff --git a/src/parser/ArcanistDiffParser.php b/src/parser/ArcanistDiffParser.php index 38d74bd0..ad0e00e5 100644 --- a/src/parser/ArcanistDiffParser.php +++ b/src/parser/ArcanistDiffParser.php @@ -840,12 +840,17 @@ final class ArcanistDiffParser { $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 @@ final class ArcanistDiffParser { $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 @@ final class ArcanistDiffParser { --$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 @@ final class ArcanistDiffParser { $change->addHunk($hunk); } - if ($advance) { + if (!$hit_next_hunk) { $line = $this->nextNonemptyLine(); } diff --git a/src/parser/__tests__/ArcanistDiffParserTestCase.php b/src/parser/__tests__/ArcanistDiffParserTestCase.php index d1657d0c..d01bcf65 100644 --- a/src/parser/__tests__/ArcanistDiffParserTestCase.php +++ b/src/parser/__tests__/ArcanistDiffParserTestCase.php @@ -536,6 +536,9 @@ EOTEXT 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; diff --git a/src/parser/__tests__/diff/suppress-blank-empty.gitdiff b/src/parser/__tests__/diff/suppress-blank-empty.gitdiff new file mode 100644 index 00000000..673b8ab1 --- /dev/null +++ b/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