1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-11-29 10:12:41 +01:00

Strip comments from all diffs before parsing them

Summary:
See <https://github.com/facebook/phabricator/issues/400>. Since all of `patch`, `git apply` and `hg export` either accept or emit header comments, parse them unconditionally.

This is a tiny bit messy because we already had a less-general parser for `hg export` diffs, which have a large header section.

This is less permissive than GNU `patch`, which allows comments anywhere. We could do that, but `git apply` won't read them and they seem pretty crazy.

Test Plan: Added and ran unit tests.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7207
This commit is contained in:
epriestley 2013-10-03 15:06:35 -07:00
parent 0ece525d6c
commit 587addfd94
3 changed files with 39 additions and 4 deletions

View file

@ -195,6 +195,14 @@ final class ArcanistDiffParser {
$this->didStartParse($diff); $this->didStartParse($diff);
// Strip off header comments. While `patch` allows comments anywhere in the
// file, `git apply` is more strict. We get these comments in `hg export`
// diffs, and Eclipse can also produce them.
$line = $this->getLineTrimmed();
while (preg_match('/^#/', $line)) {
$line = $this->nextLine();
}
do { do {
$patterns = array( $patterns = array(
// This is a normal SVN text change, probably from "svn diff". // This is a normal SVN text change, probably from "svn diff".
@ -1092,12 +1100,23 @@ final class ArcanistDiffParser {
} }
protected function isFirstNonEmptyLine() { protected function isFirstNonEmptyLine() {
$count = count($this->text); $len = count($this->text);
for ($i = 0; $i < $count; $i++) { for ($ii = 0; $ii < $len; $ii++) {
if (strlen(trim($this->text[$i])) != 0) { $line = $this->text[$ii];
return ($i == $this->line);
if (!strlen(trim($line))) {
// This line is empty, skip it.
continue;
} }
if (preg_match('/^#/', $line)) {
// This line is a comment, skip it.
continue;
} }
return ($ii == $this->line);
}
// Entire file is empty. // Entire file is empty.
return false; return false;
} }

View file

@ -563,6 +563,13 @@ EOTEXT
ArcanistDiffChangeType::TYPE_CHANGE, ArcanistDiffChangeType::TYPE_CHANGE,
$change->getType()); $change->getType());
break; break;
case 'comment.svndiff':
$this->assertEqual(1, count($changes));
$change = array_shift($changes);
$this->assertEqual(
ArcanistDiffChangeType::TYPE_CHANGE,
$change->getType());
break;
default: default:
throw new Exception("No test block for diff file {$diff_file}."); throw new Exception("No test block for diff file {$diff_file}.");
break; break;

View file

@ -0,0 +1,9 @@
### Eclipse Workspace Patch 1.0
#P Services
Index: example
===================================================================
--- example (revision 80)
+++ example (working copy)
@@ -1 +1 @@
-a
+apple