Merge "Disable syntax computation on very long diffs"
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
index 8ca4f9d..cc66e3b 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
@@ -76,6 +76,9 @@
// syntax highlighting for the entire file.
const SYNTAX_MAX_LINE_LENGTH = 500;
+ // Disable syntax highlighting if the overall diff is too large.
+ const SYNTAX_MAX_DIFF_LENGTH = 20000;
+
const TRAILING_WHITESPACE_PATTERN = /\s+$/;
Polymer({
@@ -190,7 +193,7 @@
this.dispatchEvent(new CustomEvent('render-content',
{bubbles: true}));
- if (this._anyLineTooLong()) {
+ if (this._diffTooLargeForSyntax()) {
this.$.syntaxLayer.enabled = false;
}
@@ -473,10 +476,32 @@
}, false);
},
+ _diffTooLargeForSyntax() {
+ return this._anyLineTooLong() ||
+ this.getDiffLength() > SYNTAX_MAX_DIFF_LENGTH;
+ },
+
setBlame(blame) {
if (!this._builder || !blame) { return; }
this._builder.setBlame(blame);
},
+
+ /**
+ * Get the approximate length of the diff as the sum of the maximum
+ * length of the chunks.
+ * @return {number}
+ */
+ getDiffLength() {
+ return this.diff.content.reduce((sum, sec) => {
+ if (sec.hasOwnProperty('ab')) {
+ return sum + sec.ab.length;
+ } else {
+ return sum + Math.max(
+ sec.hasOwnProperty('a') ? sec.a.length : 0,
+ sec.hasOwnProperty('b') ? sec.b.length : 0);
+ }
+ }, 0);
+ },
});
})();
</script>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
index 7f1450c..4d7b821 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
@@ -1052,6 +1052,10 @@
});
});
+ test('getDiffLength', () => {
+ assert.equal(element.getDiffLength(diff), 52);
+ });
+
test('getContentByLine', () => {
let actual;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
index 6add75c..bddfc6d 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
@@ -310,7 +310,7 @@
<div id="sizeWarning" class$="[[_computeWarningClass(_showWarning)]]">
<p>
Prevented render because "Whole file" is enabled and this diff is very
- large (about [[_diffLength(diff)]] lines).
+ large (about [[_diffLength]] lines).
</p>
<gr-button on-tap="_handleLimitedBypass">
Render with limited context
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
index 9f1b412..3ae8806 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
@@ -195,6 +195,8 @@
return this._createCommentThreadGroup.bind(this);
},
},
+
+ _diffLength: Number,
},
behaviors: [
@@ -624,6 +626,7 @@
_diffChanged(newValue) {
if (newValue) {
+ this._diffLength = this.$.diffBuilder.getDiffLength();
this._renderDiffTable();
}
},
@@ -634,7 +637,7 @@
return;
}
if (this.prefs.context === -1 &&
- this._diffLength(this.diff) >= LARGE_DIFF_THRESHOLD_LINES &&
+ this._diffLength >= LARGE_DIFF_THRESHOLD_LINES &&
this._safetyBypass === null) {
this._showWarning = true;
this.dispatchEvent(new CustomEvent('render', {bubbles: true}));
@@ -684,25 +687,6 @@
return items.length === 0;
},
- /**
- * The number of lines in the diff. For delta chunks that are different
- * sizes on the left and the right, the longer side is used.
- * @param {!Object} diff
- * @return {number}
- */
- _diffLength(diff) {
- return diff.content.reduce((sum, sec) => {
- if (sec.hasOwnProperty('ab')) {
- return sum + sec.ab.length;
- } else {
- return sum + Math.max(
- sec.hasOwnProperty('a') ? sec.a.length : 0,
- sec.hasOwnProperty('b') ? sec.b.length : 0
- );
- }
- }, 0);
- },
-
_handleFullBypass() {
this._safetyBypass = FULL_CONTEXT;
this._renderDiffTable();
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
index 0274fae..faf529b 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
@@ -55,12 +55,6 @@
assert.isTrue(cancelStub.calledOnce);
});
- test('_diffLength', () => {
- element = fixture('basic');
- const mock = document.createElement('mock-diff-response');
- assert.equal(element._diffLength(mock.diffResponse), 52);
- });
-
test('line limit with line_wrapping', () => {
element = fixture('basic');
element.prefs = {line_wrapping: true, line_length: 80, tab_size: 2};
@@ -971,6 +965,7 @@
new CustomEvent('render', {bubbles: true}));
});
const mock = document.createElement('mock-diff-response');
+ sandbox.stub(element.$.diffBuilder, 'getDiffLength').returns(10000);
element.diff = mock.diffResponse;
element.comments = {left: [], right: []};
element.noRenderOnPrefsChange = true;
@@ -978,7 +973,6 @@
test('large render w/ context = 10', done => {
element.prefs = {context: 10};
- sandbox.stub(element, '_diffLength', () => 10000);
element.addEventListener('render', () => {
assert.isTrue(renderStub.called);
assert.isFalse(element._showWarning);
@@ -990,7 +984,6 @@
test('large render w/ whole file and bypass', done => {
element.prefs = {context: -1};
element._safetyBypass = 10;
- sandbox.stub(element, '_diffLength', () => 10000);
element.addEventListener('render', () => {
assert.isTrue(renderStub.called);
assert.isFalse(element._showWarning);
@@ -1001,7 +994,6 @@
test('large render w/ whole file and no bypass', done => {
element.prefs = {context: -1};
- sandbox.stub(element, '_diffLength', () => 10000);
element.addEventListener('render', () => {
assert.isFalse(renderStub.called);
assert.isTrue(element._showWarning);
