From f1bca1b7cbe880cae565413bb8fa94123e8556b9 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Fri, 19 Oct 2012 14:07:34 -0700
Subject: [PATCH] Make parser more flexible in the face of Git diff prefix
 flags

Summary: See T1675 for discussion. We currently strip `[abicwo12]/` from Git patches, but the user can provide arbitrary prefixes with `--src-prefix` and `--dst-prefix`, or strip prefixes entirely with `--no-prefix`. In these cases, trust they know what they're doing rather than rejecting the diff.

Test Plan: Added a bunch of tests. We have existing tests for `diff.mnemonicprefix` and normal prefixes.

Reviewers: btrahan, vrana

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1675

Differential Revision: https://secure.phabricator.com/D3744
---
 src/parser/ArcanistDiffParser.php             | 47 +++++++++++++++++--
 .../__tests__/ArcanistDiffParserTestCase.php  | 30 ++++++++++++
 .../__tests__/diff/custom-prefixes.gitdiff    |  7 +++
 3 files changed, 79 insertions(+), 5 deletions(-)
 create mode 100644 src/parser/__tests__/diff/custom-prefixes.gitdiff

diff --git a/src/parser/ArcanistDiffParser.php b/src/parser/ArcanistDiffParser.php
index cfcdd2b7..3116570d 100644
--- a/src/parser/ArcanistDiffParser.php
+++ b/src/parser/ArcanistDiffParser.php
@@ -221,9 +221,7 @@ final class ArcanistDiffParser {
         '(?P<type>commit) (?P<hash>[a-f0-9]+)(?: \(.*\))?',
         // This is a git diff, probably from "git show" or "git diff".
         // Note that the filenames may appear quoted.
-        '(?P<type>diff --git) '.
-          '(?P<old>"?[abicwo12]/.+"?) '.
-          '(?P<cur>"?[abicwo12]/.+"?)',
+        '(?P<type>diff --git) (?P<old>"?.+"?) (?P<cur>"?.+"?)',
         // This is a unified diff, probably from "diff -u" or synthetic diffing.
         '(?P<type>---) (?P<old>.+)\s+\d{4}-\d{2}-\d{2}.*',
         '(?P<binary>Binary) files '.
@@ -270,11 +268,11 @@ final class ArcanistDiffParser {
         if ($match['type'] == 'diff --git') {
           if (isset($match['old'])) {
             $match['old'] = $this->unescapeFilename($match['old']);
-            $match['old'] = substr($match['old'], 2);
+            $match['old'] = self::stripGitPathPrefix($match['old']);
           }
           if (isset($match['cur'])) {
             $match['cur'] = $this->unescapeFilename($match['cur']);
-            $match['cur'] = substr($match['cur'], 2);
+            $match['cur'] = self::stripGitPathPrefix($match['cur']);
           }
         }
       }
@@ -1185,4 +1183,43 @@ final class ArcanistDiffParser {
     return $changes;
   }
 
+  /**
+   * Strip prefixes off paths from `git diff`. By default git uses a/ and b/,
+   * but you can set `diff.mnemonicprefix` to get a different set of prefixes,
+   * or use `--no-prefix`, `--src-prefix` or `--dst-prefix` to set these to
+   * other arbitrary values.
+   *
+   * We strip the default and mnemonic prefixes, and trust the user knows what
+   * they're doing in the other cases.
+   *
+   * @param   string Path to strip.
+   * @return  string Stripped path.
+   */
+  public static function stripGitPathPrefix($path) {
+
+    static $regex;
+    if ($regex === null) {
+      $prefixes = array(
+        // These are the defaults.
+        'a/',
+        'b/',
+
+        // These show up when you set "diff.mnemonicprefix".
+        'i/',
+        'c/',
+        'w/',
+        'o/',
+        '1/',
+        '2/',
+      );
+
+      foreach ($prefixes as $key => $prefix) {
+        $prefixes[$key] = preg_quote($prefix, '@');
+      }
+      $regex = '@^('.implode('|', $prefixes).')@S';
+    }
+
+    return preg_replace($regex, '', $path);
+  }
+
 }
diff --git a/src/parser/__tests__/ArcanistDiffParserTestCase.php b/src/parser/__tests__/ArcanistDiffParserTestCase.php
index 3a6b785d..e59a2df5 100644
--- a/src/parser/__tests__/ArcanistDiffParserTestCase.php
+++ b/src/parser/__tests__/ArcanistDiffParserTestCase.php
@@ -542,9 +542,39 @@ EOTEXT
       case 'hg-patch.hgdiff':
         $this->assertEqual(1, count($changes));
         break;
+      case 'custom-prefixes.gitdiff':
+        $this->assertEqual(1, count($changes));
+        $change = head($changes);
+        $this->assertEqual(
+          'dst/file',
+          $change->getCurrentPath());
+        break;
       default:
         throw new Exception("No test block for diff file {$diff_file}.");
         break;
     }
   }
+
+  public function testGitPrefixStripping() {
+    static $tests = array(
+      'a/file.c'    => 'file.c',
+      'b/file.c'    => 'file.c',
+      'i/file.c'    => 'file.c',
+      'c/file.c'    => 'file.c',
+      'w/file.c'    => 'file.c',
+      'o/file.c'    => 'file.c',
+      '1/file.c'    => 'file.c',
+      '2/file.c'    => 'file.c',
+      'src/file.c'  => 'src/file.c',
+      'file.c'      => 'file.c',
+    );
+
+    foreach ($tests as $input => $expect) {
+      $this->assertEqual(
+        $expect,
+        ArcanistDiffParser::stripGitPathPrefix($input),
+        "Strip git prefix from '{$input}'.");
+    }
+  }
+
 }
diff --git a/src/parser/__tests__/diff/custom-prefixes.gitdiff b/src/parser/__tests__/diff/custom-prefixes.gitdiff
new file mode 100644
index 00000000..86cd84e7
--- /dev/null
+++ b/src/parser/__tests__/diff/custom-prefixes.gitdiff
@@ -0,0 +1,7 @@
+diff --git src/file dst/file
+new file mode 100644
+index 0000000..1269488
+--- /dev/null
++++ dst/file
+@@ -0,0 +1 @@
++data