Add an "ignore all" whitespace mode

Summary:
We used to have "ignore all", but it became "ignore most". It does not ignore in-line whitespace changes or whitespace changes in language where whitespace is marked semantic.

Add an explicit "ignore all" option.

Test Plan: Used "ignore all" to view a change with heuristically-detected semantic whitespace, the change was ignored.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T970

Differential Revision: https://secure.phabricator.com/D1865
This commit is contained in:
epriestley
2012-03-12 17:06:36 -07:00
parent a38223023d
commit 09c1bd34f1
2 changed files with 20 additions and 3 deletions

View File

@@ -66,8 +66,12 @@ class DifferentialChangesetParser {
const WHITESPACE_SHOW_ALL = 'show-all'; const WHITESPACE_SHOW_ALL = 'show-all';
const WHITESPACE_IGNORE_TRAILING = 'ignore-trailing'; const WHITESPACE_IGNORE_TRAILING = 'ignore-trailing';
// TODO: This is now "Ignore Most" in the UI.
const WHITESPACE_IGNORE_ALL = 'ignore-all'; const WHITESPACE_IGNORE_ALL = 'ignore-all';
const WHITESPACE_IGNORE_FORCE = 'ignore-force';
/** /**
* Configure which Changeset comments added to the right side of the visible * Configure which Changeset comments added to the right side of the visible
* diff will be attached to. The ID must be the ID of a real Differential * diff will be attached to. The ID must be the ID of a real Differential
@@ -395,6 +399,12 @@ class DifferentialChangesetParser {
} }
$new[$k]['text'] = idx($new_text, $desc['line']); $new[$k]['text'] = idx($new_text, $desc['line']);
if ($this->whitespaceMode == self::WHITESPACE_IGNORE_FORCE) {
// Under forced ignore mode, ignore even internal whitespace
// changes.
continue;
}
// If there's a corresponding "old" text and the line is marked as // If there's a corresponding "old" text and the line is marked as
// unchanged, test if there are internal whitespace changes between // unchanged, test if there are internal whitespace changes between
// non-whitespace characters, e.g. spaces added to a string or spaces // non-whitespace characters, e.g. spaces added to a string or spaces
@@ -700,6 +710,7 @@ class DifferentialChangesetParser {
switch ($whitespace_mode) { switch ($whitespace_mode) {
case self::WHITESPACE_SHOW_ALL: case self::WHITESPACE_SHOW_ALL:
case self::WHITESPACE_IGNORE_TRAILING: case self::WHITESPACE_IGNORE_TRAILING:
case self::WHITESPACE_IGNORE_FORCE:
break; break;
default: default:
$whitespace_mode = self::WHITESPACE_IGNORE_ALL; $whitespace_mode = self::WHITESPACE_IGNORE_ALL;
@@ -715,10 +726,15 @@ 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); $ignore_all = (($whitespace_mode == self::WHITESPACE_IGNORE_ALL) ||
($whitespace_mode == self::WHITESPACE_IGNORE_FORCE));
if ($ignore_all && $changeset->getWhitespaceMatters()) { $force_ignore = ($whitespace_mode == self::WHITESPACE_IGNORE_FORCE);
$ignore_all = false;
if (!$force_ignore) {
if ($ignore_all && $changeset->getWhitespaceMatters()) {
$ignore_all = false;
}
} }
// The "ignore all whitespace" algorithm depends on rediffing the // The "ignore all whitespace" algorithm depends on rediffing the

View File

@@ -191,6 +191,7 @@ final class DifferentialRevisionUpdateHistoryView extends AphrontView {
)); ));
$options = array( $options = array(
DifferentialChangesetParser::WHITESPACE_IGNORE_FORCE => 'Ignore All',
DifferentialChangesetParser::WHITESPACE_IGNORE_ALL => 'Ignore Most', DifferentialChangesetParser::WHITESPACE_IGNORE_ALL => 'Ignore Most',
DifferentialChangesetParser::WHITESPACE_IGNORE_TRAILING => DifferentialChangesetParser::WHITESPACE_IGNORE_TRAILING =>
'Ignore Trailing', 'Ignore Trailing',