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 8ca4f9d9ed..cc66e3bdff 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 @@ limitations under the License. // 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 @@ limitations under the License. this.dispatchEvent(new CustomEvent('render-content', {bubbles: true})); - if (this._anyLineTooLong()) { + if (this._diffTooLargeForSyntax()) { this.$.syntaxLayer.enabled = false; } @@ -473,10 +476,32 @@ limitations under the License. }, 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); + }, }); })(); 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 7f1450c184..4d7b8215be 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 @@ limitations under the License. }); }); + 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 6add75cd49..bddfc6d636 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 @@ limitations under the License.

Prevented render because "Whole file" is enabled and this diff is very - large (about [[_diffLength(diff)]] lines). + large (about [[_diffLength]] lines).

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 9f1b4123f9..3ae88065b9 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 0274faed69..faf529b464 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 @@ limitations under the License. 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 @@ limitations under the License. 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 @@ limitations under the License. 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 @@ limitations under the License. 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 @@ limitations under the License. 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);