mirror of
https://we.phorge.it/source/phorge.git
synced 2024-12-22 13:30:55 +01:00
Deal more gracefully with unusual display changesets
Summary: These came up in dealing with the diff produced by T271. When a file is unmodified, don't try to use the "ignore all whitespace" algorithm on it. Also, detect "changed only by adding or removing trailing whitespace" vs "this file was not modified" correctly. Test Plan: Viewed the diff that came out of running 'arc diff' on my T271 mess. Reviewed By: tuomaspelkonen Reviewers: alex, jungejason, tuomaspelkonen, aran CC: aran, tuomaspelkonen Differential Revision: 548
This commit is contained in:
parent
eeb8d10f42
commit
6eed2ad2bb
1 changed files with 42 additions and 14 deletions
|
@ -341,6 +341,21 @@ class DifferentialChangesetParser {
|
||||||
switch ($this->whitespaceMode) {
|
switch ($this->whitespaceMode) {
|
||||||
case self::WHITESPACE_IGNORE_TRAILING:
|
case self::WHITESPACE_IGNORE_TRAILING:
|
||||||
if (rtrim($o_desc['text']) == rtrim($n_desc['text'])) {
|
if (rtrim($o_desc['text']) == rtrim($n_desc['text'])) {
|
||||||
|
if ($o_desc['type']) {
|
||||||
|
// If we're converting this into an unchanged line because of
|
||||||
|
// a trailing whitespace difference, mark it as a whitespace
|
||||||
|
// change so we can show "This file was modified only by
|
||||||
|
// adding or removing trailing whitespace." instead of
|
||||||
|
// "This file was not modified.".
|
||||||
|
$whitelines = true;
|
||||||
|
}
|
||||||
|
$similar = true;
|
||||||
|
}
|
||||||
|
break;
|
||||||
|
default:
|
||||||
|
// In this case, the lines are similar if there is no change type
|
||||||
|
// (that is, just trust the diff algorithm).
|
||||||
|
if (!$o_desc['type']) {
|
||||||
$similar = true;
|
$similar = true;
|
||||||
}
|
}
|
||||||
break;
|
break;
|
||||||
|
@ -349,7 +364,6 @@ class DifferentialChangesetParser {
|
||||||
$o_desc['type'] = null;
|
$o_desc['type'] = null;
|
||||||
$n_desc['type'] = null;
|
$n_desc['type'] = null;
|
||||||
$skip_intra[count($old)] = true;
|
$skip_intra[count($old)] = true;
|
||||||
$whitelines = true;
|
|
||||||
} else {
|
} else {
|
||||||
$changed = true;
|
$changed = true;
|
||||||
}
|
}
|
||||||
|
@ -744,24 +758,38 @@ class DifferentialChangesetParser {
|
||||||
$changeset->getFileType() == DifferentialChangeType::FILE_SYMLINK) {
|
$changeset->getFileType() == DifferentialChangeType::FILE_SYMLINK) {
|
||||||
if ($skip_cache || !$this->loadCache()) {
|
if ($skip_cache || !$this->loadCache()) {
|
||||||
|
|
||||||
|
$ignore_all = ($this->whitespaceMode == self::WHITESPACE_IGNORE_ALL);
|
||||||
|
|
||||||
// The "ignore all whitespace" algorithm depends on rediffing the
|
// The "ignore all whitespace" algorithm depends on rediffing the
|
||||||
// files, and we currently need complete representations of both
|
// files, and we currently need complete representations of both
|
||||||
// files to do anything reasonable. If we only have parts of the files,
|
// files to do anything reasonable. If we only have parts of the files,
|
||||||
// don't use the "ignore all" algorithm.
|
// don't use the "ignore all" algorithm.
|
||||||
$can_use_ignore_all = true;
|
if ($ignore_all) {
|
||||||
$hunks = $changeset->getHunks();
|
$hunks = $changeset->getHunks();
|
||||||
if (count($hunks) !== 1) {
|
if (count($hunks) !== 1) {
|
||||||
$can_use_ignore_all = false;
|
$ignore_all = false;
|
||||||
} else {
|
} else {
|
||||||
$first_hunk = reset($hunks);
|
$first_hunk = reset($hunks);
|
||||||
if ($first_hunk->getOldOffset() != 1 ||
|
if ($first_hunk->getOldOffset() != 1 ||
|
||||||
$first_hunk->getNewOffset() != 1) {
|
$first_hunk->getNewOffset() != 1) {
|
||||||
$can_use_ignore_all = false;
|
$ignore_all = false;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
if ($this->whitespaceMode == self::WHITESPACE_IGNORE_ALL &&
|
if ($ignore_all) {
|
||||||
$can_use_ignore_all) {
|
$old_file = $changeset->makeOldFile();
|
||||||
|
$new_file = $changeset->makeNewFile();
|
||||||
|
if ($old_file == $new_file) {
|
||||||
|
// If the old and new files are exactly identical, the synthetic
|
||||||
|
// diff below will give us nonsense and whitespace modes are
|
||||||
|
// irrelevant anyway. This occurs when you, e.g., copy a file onto
|
||||||
|
// itself in Subversion (see T271).
|
||||||
|
$ignore_all = false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if ($ignore_all) {
|
||||||
|
|
||||||
// Huge mess. Generate a "-bw" (ignore all whitespace changes) diff,
|
// Huge mess. Generate a "-bw" (ignore all whitespace changes) diff,
|
||||||
// parse it out, and then play a shell game with the parsed format
|
// parse it out, and then play a shell game with the parsed format
|
||||||
|
@ -773,8 +801,8 @@ class DifferentialChangesetParser {
|
||||||
|
|
||||||
$old_tmp = new TempFile();
|
$old_tmp = new TempFile();
|
||||||
$new_tmp = new TempFile();
|
$new_tmp = new TempFile();
|
||||||
Filesystem::writeFile($old_tmp, $changeset->makeOldFile());
|
Filesystem::writeFile($old_tmp, $old_file);
|
||||||
Filesystem::writeFile($new_tmp, $changeset->makeNewFile());
|
Filesystem::writeFile($new_tmp, $new_file);
|
||||||
list($err, $diff) = exec_manual(
|
list($err, $diff) = exec_manual(
|
||||||
'diff -bw -U65535 %s %s ',
|
'diff -bw -U65535 %s %s ',
|
||||||
$old_tmp,
|
$old_tmp,
|
||||||
|
|
Loading…
Reference in a new issue