1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-11-22 06:42:41 +01:00

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
This commit is contained in:
epriestley 2012-10-19 14:07:34 -07:00
parent 4fb2b48d06
commit f1bca1b7cb
3 changed files with 79 additions and 5 deletions

View file

@ -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);
}
}

View file

@ -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}'.");
}
}
}

View file

@ -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