Add ignore-whitespace control
When a diff request includes an ignore-whitespace level, the same diff
content is returned, but a number of diff chunks are marked as "common"
with the understanding that those chunks only contain the kind of
whitespace that can be ignored. Thus, actually ignoring these chunks is
up to the frontend.
In this commit, support is added for editing the preferred whitespace
level, and the preferred level is included in diff requests. When a diff
is loaded with an ignore-whitespace level, gr-diff-host transforms the
marked deltas into shared chunks. In this way, when the transformed diff
is passed down into gr-diff, it can be rendered like normal and the
unwanted whitespace deltas do not appear.
To transform a diff with chunks to be ignored, the strategy is as
follows. If a marked delta chunk has revision (a right-side), then it's
converted to a shared chunk where both sides are made up of the revision
content. (If a marked delta chunk has no revision content, it's merely
omitted.) Finally, adjacent shared chunks that result from modifying
the marked chunks are merged together so that gr-diff will properly
position context-control barriers.
Feature: Issue 6198
Change-Id: I1e4cc1075edf34f5ce87ee6bfc2cf415a3b98d94
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
index 3e9e796..6f61fb9 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
@@ -28,6 +28,8 @@
UNIFIED: 'UNIFIED_DIFF',
};
+ const WHITESPACE_IGNORE_NONE = 'IGNORE_NONE';
+
/**
* @param {Object} diff
* @return {boolean}
@@ -105,7 +107,10 @@
type: Boolean,
reflectToAttribute: true,
},
- noRenderOnPrefsChange: Boolean,
+ noRenderOnPrefsChange: {
+ type: Boolean,
+ value: false,
+ },
comments: Object,
lineWrapping: {
type: Boolean,
@@ -167,12 +172,19 @@
type: Object,
value: null,
},
+
+ _loadedWhitespaceLevel: String,
},
listeners: {
'draft-interaction': '_handleDraftInteraction',
},
+ observers: [
+ '_whitespaceChanged(prefs.ignore_whitespace, _loadedWhitespaceLevel,' +
+ ' noRenderOnPrefsChange)',
+ ],
+
ready() {
if (this._canReload()) {
this.reload();
@@ -189,10 +201,15 @@
reload() {
this._loading = true;
this._errorMessage = null;
+ const whitespaceLevel = this._getIgnoreWhitespace();
const diffRequest = this._getDiff()
.then(diff => {
+ this._loadedWhitespaceLevel = whitespaceLevel;
this._reportDiff(diff);
+ if (this._getIgnoreWhitespace() !== WHITESPACE_IGNORE_NONE) {
+ return this._translateChunksToIgnore(diff);
+ }
return diff;
})
.catch(e => {
@@ -321,6 +338,7 @@
this.patchRange.basePatchNum,
this.patchRange.patchNum,
this.path,
+ this._getIgnoreWhitespace(),
reject)
.then(resolve);
});
@@ -430,5 +448,63 @@
_handleDraftInteraction() {
this.$.reporting.recordDraftInteraction();
},
+
+ /**
+ * Take a diff that was loaded with a ignore-whitespace other than
+ * IGNORE_NONE, and convert delta chunks labeled as common into shared
+ * chunks.
+ * @param {!Object} diff
+ * @returns {!Object}
+ */
+ _translateChunksToIgnore(diff) {
+ const newDiff = Object.assign({}, diff);
+ const mergedContent = [];
+
+ // Was the last chunk visited a shared chunk?
+ let lastWasShared = false;
+
+ for (const chunk of diff.content) {
+ if (lastWasShared && chunk.common && chunk.b) {
+ // The last chunk was shared and this chunk should be ignored, so
+ // add its revision content to the previous chunk.
+ mergedContent[mergedContent.length - 1].ab.push(...chunk.b);
+ } else if (chunk.common && !chunk.b) {
+ // If the chunk should be ignored, but it doesn't have revision
+ // content, then drop it and continue without updating lastWasShared.
+ continue;
+ } else if (lastWasShared && chunk.ab) {
+ // Both the last chunk and the current chunk are shared. Merge this
+ // chunk's shared content into the previous shared content.
+ mergedContent[mergedContent.length - 1].ab.push(...chunk.ab);
+ } else if (!lastWasShared && chunk.common && chunk.b) {
+ // If the previous chunk was not shared, but this one should be
+ // ignored, then add it as a shared chunk.
+ mergedContent.push({ab: chunk.b});
+ } else {
+ // Otherwise add the chunk as is.
+ mergedContent.push(chunk);
+ }
+
+ lastWasShared = !!mergedContent[mergedContent.length - 1].ab;
+ }
+
+ newDiff.content = mergedContent;
+ return newDiff;
+ },
+
+ _getIgnoreWhitespace() {
+ if (!this.prefs || !this.prefs.ignore_whitespace) {
+ return WHITESPACE_IGNORE_NONE;
+ }
+ return this.prefs.ignore_whitespace;
+ },
+
+ _whitespaceChanged(preferredWhitespaceLevel, loadedWhitespaceLevel,
+ noRenderOnPrefsChange) {
+ if (preferredWhitespaceLevel !== loadedWhitespaceLevel &&
+ !noRenderOnPrefsChange) {
+ this.reload();
+ }
+ },
});
})();
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
index a05d44f..f83253e 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
@@ -775,5 +775,89 @@
assert.isUndefined(reportStub.lastCall.args[1]);
});
});
+
+ suite('_translateChunksToIgnore', () => {
+ let content;
+
+ setup(() => {
+ content = [
+ {ab: ['one', 'two']},
+ {a: ['three'], b: ['different three']},
+ {b: ['four']},
+ {ab: ['five', 'six']},
+ {a: ['seven']},
+ {ab: ['eight', 'nine']},
+ ];
+ });
+
+ test('does nothing to unmarked diff', () => {
+ assert.deepEqual(element._translateChunksToIgnore({content}),
+ {content});
+ });
+
+ test('merges marked delta chunk', () => {
+ content[1].common = true;
+ assert.deepEqual(element._translateChunksToIgnore({content}), {
+ content: [
+ {ab: ['one', 'two', 'different three']},
+ {b: ['four']},
+ {ab: ['five', 'six']},
+ {a: ['seven']},
+ {ab: ['eight', 'nine']},
+ ],
+ });
+ });
+
+ test('merges marked addition chunk', () => {
+ content[2].common = true;
+ assert.deepEqual(element._translateChunksToIgnore({content}), {
+ content: [
+ {ab: ['one', 'two']},
+ {a: ['three'], b: ['different three']},
+ {ab: ['four', 'five', 'six']},
+ {a: ['seven']},
+ {ab: ['eight', 'nine']},
+ ],
+ });
+ });
+
+ test('merges multiple marked delta', () => {
+ content[1].common = true;
+ content[2].common = true;
+ assert.deepEqual(element._translateChunksToIgnore({content}), {
+ content: [
+ {ab: ['one', 'two', 'different three', 'four', 'five', 'six']},
+ {a: ['seven']},
+ {ab: ['eight', 'nine']},
+ ],
+ });
+ });
+
+ test('marked deletion chunks are omitted', () => {
+ content[4].common = true;
+ assert.deepEqual(element._translateChunksToIgnore({content}), {
+ content: [
+ {ab: ['one', 'two']},
+ {a: ['three'], b: ['different three']},
+ {b: ['four']},
+ {ab: ['five', 'six', 'eight', 'nine']},
+ ],
+ });
+ });
+
+ test('marked deltas can start shared chunks', () => {
+ content[0] = {a: ['one'], b: ['two'], common: true};
+ assert.deepEqual(element._translateChunksToIgnore({content}), {
+ content: [
+ {ab: ['two']},
+ {a: ['three'], b: ['different three']},
+ {b: ['four']},
+ {ab: ['five', 'six']},
+ {a: ['seven']},
+ {ab: ['eight', 'nine']},
+ ],
+ });
+ });
+ });
});
</script>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-preferences/gr-diff-preferences.html b/polygerrit-ui/app/elements/diff/gr-diff-preferences/gr-diff-preferences.html
index 375e598..78814d4 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-preferences/gr-diff-preferences.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-preferences/gr-diff-preferences.html
@@ -60,7 +60,7 @@
align-items: center;
display: flex;
padding: .35em 1.5em;
- width: 20em;
+ width: 25em;
}
.pref:hover {
background-color: var(--hover-background-color);
@@ -149,6 +149,15 @@
type="checkbox"
on-tap="_handleAutomaticReviewTap">
</div>
+ <div class="pref">
+ <label for="ignoreWhitespace">Ignore Whitespace</label>
+ <select id="ignoreWhitespace" on-change="_handleIgnoreWhitespaceChange">
+ <option value="IGNORE_NONE">None</option>
+ <option value="IGNORE_TRAILING">Trailing</option>
+ <option value="IGNORE_LEADING_AND_TRAILING">Leading & trailing</option>
+ <option value="IGNORE_ALL">All</option>
+ </select>
+ </div>
</div>
<div class="actions">
<gr-button id="cancelButton" link on-tap="_handleCancel">
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-preferences/gr-diff-preferences.js b/polygerrit-ui/app/elements/diff/gr-diff-preferences/gr-diff-preferences.js
index 47a3c2d..8fc90b9 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-preferences/gr-diff-preferences.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-preferences/gr-diff-preferences.js
@@ -67,6 +67,7 @@
this.$.lineWrappingInput.checked = prefs.line_wrapping;
this.$.syntaxHighlightInput.checked = prefs.syntax_highlighting;
this.$.automaticReviewInput.checked = !prefs.manual_review;
+ this.$.ignoreWhitespace.value = prefs.ignore_whitespace;
},
_localPrefsChanged(changeRecord) {
@@ -79,6 +80,11 @@
this.set('_newPrefs.context', parseInt(selectEl.value, 10));
},
+ _handleIgnoreWhitespaceChange(e) {
+ const selectEl = Polymer.dom(e).rootTarget;
+ this.set('_newPrefs.ignore_whitespace', selectEl.value);
+ },
+
_handleShowTabsTap(e) {
this.set('_newPrefs.show_tabs', Polymer.dom(e).rootTarget.checked);
},
diff --git a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.html b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.html
index 18c1734..029ce88 100644
--- a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.html
+++ b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.html
@@ -348,6 +348,20 @@
on-change="_handleDiffSyntaxHighlightingChanged">
</span>
</section>
+ <section>
+ <div class="pref">
+ <span class="title">Ignore Whitespace</span>
+ <span class="value">
+ <gr-select bind-value="{{_diffPrefs.ignore_whitespace}}">
+ <select>
+ <option value="IGNORE_NONE">None</option>
+ <option value="IGNORE_TRAILING">Trailing</option>
+ <option value="IGNORE_LEADING_AND_TRAILING">Leading & trailing</option>
+ <option value="IGNORE_ALL">All</option>
+ </select>
+ </gr-select>
+ </span>
+ </div>
<gr-button
id="saveDiffPrefs"
on-tap="_handleSaveDiffPreferences"
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
index af10b8371..c97c4c7 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
@@ -2134,13 +2134,16 @@
* index.
* @param {number|string} patchNum
* @param {string} path
+ * @param {string=} opt_whitespace the ignore-whitespace level for the diff
+ * algorithm.
* @param {function(?Response, string=)=} opt_errFn
*/
- getDiff(changeNum, basePatchNum, patchNum, path, opt_errFn) {
+ getDiff(changeNum, basePatchNum, patchNum, path, opt_whitespace,
+ opt_errFn) {
const params = {
context: 'ALL',
intraline: null,
- whitespace: 'IGNORE_NONE',
+ whitespace: opt_whitespace || 'IGNORE_NONE',
};
if (this.isMergeParent(basePatchNum)) {
params.parent = this.getParentIndex(basePatchNum);
