From 587addfd9472293e60f1e7522b0b70f65703893f Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 3 Oct 2013 15:06:35 -0700
Subject: [PATCH] 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
---
 src/parser/ArcanistDiffParser.php             | 27 ++++++++++++++++---
 .../__tests__/ArcanistDiffParserTestCase.php  |  7 +++++
 src/parser/__tests__/diff/comment.svndiff     |  9 +++++++
 3 files changed, 39 insertions(+), 4 deletions(-)
 create mode 100644 src/parser/__tests__/diff/comment.svndiff

diff --git a/src/parser/ArcanistDiffParser.php b/src/parser/ArcanistDiffParser.php
index adaba998..b6e8cbcd 100644
--- a/src/parser/ArcanistDiffParser.php
+++ b/src/parser/ArcanistDiffParser.php
@@ -195,6 +195,14 @@ final class ArcanistDiffParser {
 
     $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 {
       $patterns = array(
         // This is a normal SVN text change, probably from "svn diff".
@@ -1092,12 +1100,23 @@ final class ArcanistDiffParser {
   }
 
   protected function isFirstNonEmptyLine() {
-    $count = count($this->text);
-    for ($i = 0; $i < $count; $i++) {
-      if (strlen(trim($this->text[$i])) != 0) {
-        return ($i == $this->line);
+    $len = count($this->text);
+    for ($ii = 0; $ii < $len; $ii++) {
+      $line = $this->text[$ii];
+
+      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.
     return false;
   }
diff --git a/src/parser/__tests__/ArcanistDiffParserTestCase.php b/src/parser/__tests__/ArcanistDiffParserTestCase.php
index cccd4435..32010f67 100644
--- a/src/parser/__tests__/ArcanistDiffParserTestCase.php
+++ b/src/parser/__tests__/ArcanistDiffParserTestCase.php
@@ -563,6 +563,13 @@ EOTEXT
           ArcanistDiffChangeType::TYPE_CHANGE,
           $change->getType());
         break;
+      case 'comment.svndiff':
+        $this->assertEqual(1, count($changes));
+        $change = array_shift($changes);
+        $this->assertEqual(
+          ArcanistDiffChangeType::TYPE_CHANGE,
+          $change->getType());
+        break;
       default:
         throw new Exception("No test block for diff file {$diff_file}.");
         break;
diff --git a/src/parser/__tests__/diff/comment.svndiff b/src/parser/__tests__/diff/comment.svndiff
new file mode 100644
index 00000000..f8c68724
--- /dev/null
+++ b/src/parser/__tests__/diff/comment.svndiff
@@ -0,0 +1,9 @@
+### Eclipse Workspace Patch 1.0
+#P Services
+Index: example
+===================================================================
+--- example	(revision 80)
++++ example	(working copy)
+@@ -1 +1 @@
+-a
++apple