mirror of
https://we.phorge.it/source/arcanist.git
synced 2024-11-22 23:02:41 +01:00
Show text of moved files in Differential
Summary: "git diff -M -C" generates useful metadata (moves/copies) but (for a pure move) no diff text. Synthetically build the diff text after the fact so this information is available in Differential. This patch is kind of nasty but I couldn't see a cleaner way to do it. :/ This also needs some UI changes in Differential: we get a full-green new file right now, but it would be better to default-hide it with "This file was moved. Show More" or similar. Test Plan: Moved a file, ran "arc diff", got textual diff. Reviewers: aran, tuomaspelkonen, jungejason, btrahan, vrana Reviewed By: vrana CC: aran, epriestley, vrana Maniphest Tasks: T230 Differential Revision: https://secure.phabricator.com/D479
This commit is contained in:
parent
b32b868a61
commit
7070d0a065
4 changed files with 68 additions and 7 deletions
|
@ -493,6 +493,8 @@ final class ArcanistDiffParser {
|
||||||
$is_mercurial = $this->getIsMercurial();
|
$is_mercurial = $this->getIsMercurial();
|
||||||
$is_svn = (!$is_git && !$is_mercurial);
|
$is_svn = (!$is_git && !$is_mercurial);
|
||||||
|
|
||||||
|
$move_source = null;
|
||||||
|
|
||||||
$line = $this->getLine();
|
$line = $this->getLine();
|
||||||
if ($is_git) {
|
if ($is_git) {
|
||||||
do {
|
do {
|
||||||
|
@ -524,7 +526,21 @@ final class ArcanistDiffParser {
|
||||||
if ($line === null ||
|
if ($line === null ||
|
||||||
preg_match('/^(diff --git|commit) /', $line)) {
|
preg_match('/^(diff --git|commit) /', $line)) {
|
||||||
// In this case, there are ONLY file mode changes, or this is a
|
// In this case, there are ONLY file mode changes, or this is a
|
||||||
// pure move.
|
// pure move. If it's a move, flag these changesets so we can build
|
||||||
|
// synthetic changes later, enabling us to show file contents in
|
||||||
|
// Differential -- git only gives us a block like this:
|
||||||
|
//
|
||||||
|
// diff --git a/README b/READYOU
|
||||||
|
// similarity index 100%
|
||||||
|
// rename from README
|
||||||
|
// rename to READYOU
|
||||||
|
//
|
||||||
|
// ...i.e., there is no associated diff.
|
||||||
|
|
||||||
|
$change->setNeedsSyntheticGitHunks(true);
|
||||||
|
if ($move_source) {
|
||||||
|
$move_source->setNeedsSyntheticGitHunks(true);
|
||||||
|
}
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
break;
|
break;
|
||||||
|
@ -591,6 +607,9 @@ final class ArcanistDiffParser {
|
||||||
$old->setType(ArcanistDiffChangeType::TYPE_MOVE_AWAY);
|
$old->setType(ArcanistDiffChangeType::TYPE_MOVE_AWAY);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// We'll reference this above.
|
||||||
|
$move_source = $old;
|
||||||
|
|
||||||
$old->addAwayPath($change->getCurrentPath());
|
$old->addAwayPath($change->getCurrentPath());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -38,6 +38,8 @@ final class ArcanistDiffChange {
|
||||||
|
|
||||||
protected $hunks = array();
|
protected $hunks = array();
|
||||||
|
|
||||||
|
private $needsSyntheticGitHunks;
|
||||||
|
|
||||||
public function toDictionary() {
|
public function toDictionary() {
|
||||||
$hunks = array();
|
$hunks = array();
|
||||||
foreach ($this->hunks as $hunk) {
|
foreach ($this->hunks as $hunk) {
|
||||||
|
@ -239,4 +241,13 @@ final class ArcanistDiffChange {
|
||||||
return trim($match[1]);
|
return trim($match[1]);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function setNeedsSyntheticGitHunks($needs_synthetic_git_hunks) {
|
||||||
|
$this->needsSyntheticGitHunks = $needs_synthetic_git_hunks;
|
||||||
|
return $this;
|
||||||
|
}
|
||||||
|
|
||||||
|
public function getNeedsSyntheticGitHunks() {
|
||||||
|
return $this->needsSyntheticGitHunks;
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -210,16 +210,20 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
|
||||||
return $this->relativeCommit;
|
return $this->relativeCommit;
|
||||||
}
|
}
|
||||||
|
|
||||||
private function getDiffFullOptions() {
|
private function getDiffFullOptions($detect_moves_and_renames = true) {
|
||||||
$options = array(
|
$options = array(
|
||||||
self::getDiffBaseOptions(),
|
self::getDiffBaseOptions(),
|
||||||
'-M',
|
|
||||||
'-C',
|
|
||||||
'--no-color',
|
'--no-color',
|
||||||
'--src-prefix=a/',
|
'--src-prefix=a/',
|
||||||
'--dst-prefix=b/',
|
'--dst-prefix=b/',
|
||||||
'-U'.$this->getDiffLinesOfContext(),
|
'-U'.$this->getDiffLinesOfContext(),
|
||||||
);
|
);
|
||||||
|
|
||||||
|
if ($detect_moves_and_renames) {
|
||||||
|
$options[] = '-M';
|
||||||
|
$options[] = '-C';
|
||||||
|
}
|
||||||
|
|
||||||
return implode(' ', $options);
|
return implode(' ', $options);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -245,8 +249,14 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
|
||||||
return $stdout;
|
return $stdout;
|
||||||
}
|
}
|
||||||
|
|
||||||
public function getRawDiffText($path) {
|
/**
|
||||||
$options = $this->getDiffFullOptions();
|
* @param string Path to generate a diff for.
|
||||||
|
* @param bool If true, detect moves and renames. Otherwise, ignore
|
||||||
|
* moves/renames; this is useful because it prompts git to
|
||||||
|
* generate real diff text.
|
||||||
|
*/
|
||||||
|
public function getRawDiffText($path, $detect_moves_and_renames = true) {
|
||||||
|
$options = $this->getDiffFullOptions($detect_moves_and_renames);
|
||||||
list($stdout) = $this->execxLocal(
|
list($stdout) = $this->execxLocal(
|
||||||
"diff {$options} %s -- %s",
|
"diff {$options} %s -- %s",
|
||||||
$this->getRelativeCommit(),
|
$this->getRelativeCommit(),
|
||||||
|
|
|
@ -855,11 +855,32 @@ EOTEXT
|
||||||
}
|
}
|
||||||
|
|
||||||
foreach ($changes as $change) {
|
foreach ($changes as $change) {
|
||||||
|
$path = $change->getCurrentPath();
|
||||||
|
|
||||||
|
// Certain types of changes (moves and copies) don't contain change data
|
||||||
|
// when expressed in raw "git diff" form. Augment any such diffs with
|
||||||
|
// textual data.
|
||||||
|
if ($change->getNeedsSyntheticGitHunks()) {
|
||||||
|
$diff = $repository_api->getRawDiffText($path, $moves = false);
|
||||||
|
$parser = new ArcanistDiffParser();
|
||||||
|
$raw_changes = $parser->parseDiff($diff);
|
||||||
|
foreach ($raw_changes as $raw_change) {
|
||||||
|
if ($raw_change->getCurrentPath() == $path) {
|
||||||
|
$change->setFileType($raw_change->getFileType());
|
||||||
|
foreach ($raw_change->getHunks() as $hunk) {
|
||||||
|
$change->addHunk($hunk);
|
||||||
|
}
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
$change->setNeedsSyntheticGitHunks(false);
|
||||||
|
}
|
||||||
|
|
||||||
if ($change->getFileType() != ArcanistDiffChangeType::FILE_BINARY) {
|
if ($change->getFileType() != ArcanistDiffChangeType::FILE_BINARY) {
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
$path = $change->getCurrentPath();
|
|
||||||
$name = basename($path);
|
$name = basename($path);
|
||||||
|
|
||||||
$old_file = $repository_api->getOriginalFileData($path);
|
$old_file = $repository_api->getOriginalFileData($path);
|
||||||
|
|
Loading…
Reference in a new issue