Merge "RefUpdateUtil: Fix warning about missing case labels"
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 420a14f..aeaee9d 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
@@ -51,16 +51,6 @@
(function() {
'use strict';
- const Defs = {};
-
- /**
- * @typedef {{
- * number: number,
- * leftSide: {boolean}
- * }}
- */
- Defs.LineOfInterest;
-
const DiffViewMode = {
SIDE_BY_SIDE: 'SIDE_BY_SIDE',
UNIFIED: 'UNIFIED_DIFF',
@@ -115,10 +105,6 @@
parentIndex: Number,
path: String,
projectName: String,
- /**
- * @type {Defs.LineOfInterest|null}
- */
- lineOfInterest: Object,
_builder: Object,
_groups: Array,
@@ -161,7 +147,7 @@
this._layers = layers;
},
- render(comments, prefs) {
+ render(keyLocations, prefs) {
this.$.syntaxLayer.enabled = prefs.syntax_highlighting;
this._showTabs = !!prefs.show_tabs;
this._showTrailingWhitespace = !!prefs.show_whitespace_errors;
@@ -172,8 +158,7 @@
this._builder = this._getDiffBuilder(this.diff, prefs);
this.$.processor.context = prefs.context;
- this.$.processor.keyLocations = this._getKeyLocations(comments,
- this.lineOfInterest);
+ this.$.processor.keyLocations = keyLocations;
this._clearDiffContent();
this._builder.addColumns(this.diffElement, prefs.font_size);
@@ -332,33 +317,6 @@
this.diffElement.innerHTML = null;
},
- /**
- * @param {!Object} comments
- * @param {Defs.LineOfInterest|null} lineOfInterest
- */
- _getKeyLocations(comments, lineOfInterest) {
- const result = {
- left: {},
- right: {},
- };
- for (const side in comments) {
- if (side !== GrDiffBuilder.Side.LEFT &&
- side !== GrDiffBuilder.Side.RIGHT) {
- continue;
- }
- for (const c of comments[side]) {
- result[side][c.line || GrDiffLine.FILE] = true;
- }
- }
-
- if (lineOfInterest) {
- const side = lineOfInterest.leftSide ? 'left' : 'right';
- result[side][lineOfInterest.number] = true;
- }
-
- return result;
- },
-
_groupsChanged(changeRecord) {
if (!changeRecord) { return; }
for (const splice of changeRecord.indexSplices) {
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 fd74d55..c7a1140 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
@@ -396,33 +396,6 @@
`Fix in diff preferences`);
});
- test('_getKeyLocations', () => {
- assert.deepEqual(element._getKeyLocations({left: [], right: []}, null),
- {left: {}, right: {}});
- const comments = {
- left: [{line: 123}, {}],
- right: [{line: 456}],
- };
- assert.deepEqual(element._getKeyLocations(comments, null), {
- left: {FILE: true, 123: true},
- right: {456: true},
- });
-
- const lineOfInterest = {number: 789, leftSide: true};
- assert.deepEqual(
- element._getKeyLocations(comments, lineOfInterest), {
- left: {FILE: true, 123: true, 789: true},
- right: {456: true},
- });
-
- delete lineOfInterest.leftSide;
- assert.deepEqual(
- element._getKeyLocations(comments, lineOfInterest), {
- left: {FILE: true, 123: true},
- right: {456: true, 789: true},
- });
- });
-
suite('_isTotal', () => {
test('is total for add', () => {
const group = new GrDiffGroup(GrDiffGroup.Type.DELTA);
@@ -852,7 +825,7 @@
suite('rendering text, images and binary files', () => {
let processStub;
- let comments;
+ let keyLocations;
let prefs;
let content;
@@ -862,7 +835,7 @@
processStub = sandbox.stub(element.$.processor, 'process')
.returns(Promise.resolve());
sandbox.stub(element, '_anyLineTooLong').returns(true);
- comments = {left: [], right: [], meta: {patchRange: undefined}};
+ keyLocations = {left: {}, right: {}};
prefs = {
line_length: 10,
show_tabs: true,
@@ -883,7 +856,7 @@
test('text', () => {
element.diff = {content};
- return element.render(comments, prefs).then(() => {
+ return element.render(keyLocations, prefs).then(() => {
assert.isTrue(processStub.calledOnce);
assert.isFalse(processStub.lastCall.args[1]);
});
@@ -892,7 +865,7 @@
test('image', () => {
element.diff = {content, binary: true};
element.isImageDiff = true;
- return element.render(comments, prefs).then(() => {
+ return element.render(keyLocations, prefs).then(() => {
assert.isTrue(processStub.calledOnce);
assert.isTrue(processStub.lastCall.args[1]);
});
@@ -900,7 +873,7 @@
test('binary', () => {
element.diff = {content, binary: true};
- return element.render(comments, prefs).then(() => {
+ return element.render(keyLocations, prefs).then(() => {
assert.isTrue(processStub.calledOnce);
assert.isTrue(processStub.lastCall.args[1]);
});
@@ -910,7 +883,7 @@
suite('rendering', () => {
let content;
let outputEl;
- let comments;
+ let keyLocations;
setup(done => {
const prefs = {
@@ -938,7 +911,7 @@
});
element = fixture('basic');
outputEl = element.queryEffectiveChildren('#diffTable');
- comments = {left: [], right: [], meta: {patchRange: undefined}};
+ keyLocations = {left: {}, right: {}};
sandbox.stub(element, '_getDiffBuilder', () => {
const builder = new GrDiffBuilder({content}, [], prefs, outputEl);
sandbox.stub(builder, 'addColumns');
@@ -952,7 +925,7 @@
return builder;
});
element.diff = {content};
- element.render(comments, prefs).then(done);
+ element.render(keyLocations, prefs).then(done);
});
test('reporting', done => {
@@ -977,7 +950,7 @@
});
test('addColumns is called', done => {
- element.render(comments, {}).then(done);
+ element.render(keyLocations, {}).then(done);
assert.isTrue(element._builder.addColumns.called);
});
@@ -1001,7 +974,7 @@
test('render-start and render are fired', done => {
const dispatchEventStub = sandbox.stub(element, 'dispatchEvent');
- element.render(comments, {}).then(() => {
+ element.render(keyLocations, {}).then(() => {
const firedEventTypes = dispatchEventStub.getCalls()
.map(c => { return c.args[0].type; });
assert.include(firedEventTypes, 'render-start');
@@ -1029,7 +1002,7 @@
context: -1,
syntax_highlighting: true,
};
- element.render(comments, prefs);
+ element.render(keyLocations, prefs);
});
test('cancel', () => {
@@ -1046,7 +1019,7 @@
let builder;
let diff;
let prefs;
- let comments;
+ let keyLocations;
setup(done => {
element = fixture('mock-diff');
@@ -1058,9 +1031,9 @@
show_tabs: true,
tab_size: 4,
};
- comments = {left: [], right: [], meta: {patchRange: undefined}};
+ keyLocations = {left: {}, right: {}};
- element.render(comments, prefs).then(() => {
+ element.render(keyLocations, prefs).then(() => {
builder = element._builder;
done();
});
@@ -1170,7 +1143,7 @@
test('_getNextContentOnSide unified left', done => {
// Re-render as unified:
element.viewMode = 'UNIFIED_DIFF';
- element.render(comments, prefs).then(() => {
+ element.render(keyLocations, prefs).then(() => {
builder = element._builder;
const startElem = builder.getContentByLine(5, 'left',
@@ -1190,7 +1163,7 @@
test('_getNextContentOnSide unified right', done => {
// Re-render as unified:
element.viewMode = 'UNIFIED_DIFF';
- element.render(comments, prefs).then(() => {
+ element.render(keyLocations, prefs).then(() => {
builder = element._builder;
const startElem = builder.getContentByLine(5, 'right',
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 862db10..e587953 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
@@ -293,8 +293,7 @@
line-wrapping="[[lineWrapping]]"
is-image-diff="[[isImageDiff]]"
base-image="[[baseImage]]"
- revision-image="[[revisionImage]]"
- line-of-interest="[[lineOfInterest]]">
+ revision-image="[[revisionImage]]">
<slot></slot>
<table
id="diffTable"
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 f87e46f..d5f92f1 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
@@ -35,6 +35,18 @@
RIGHT: 'right',
};
+ const Defs = {};
+
+ /**
+ * Special line number which should not be collapsed into a shared region.
+ *
+ * @typedef {{
+ * number: number,
+ * leftSide: boolean
+ * }}
+ */
+ Defs.LineOfInterest;
+
const LARGE_DIFF_THRESHOLD_LINES = 10000;
const FULL_CONTEXT = -1;
const LIMITED_CONTEXT = 10;
@@ -121,13 +133,7 @@
observer: '_viewModeObserver',
},
- /**
- * Special line number which should not be collapsed into a shared region.
- * @type {{
- * number: number,
- * leftSide: {boolean}
- * }|null}
- */
+ /** @type ?Defs.LineOfInterest */
lineOfInterest: Object,
loading: {
@@ -639,7 +645,9 @@
}
this._showWarning = false;
- this.$.diffBuilder.render(this.comments, this._getBypassPrefs());
+ const keyLocations = this._getKeyLocations(this.comments,
+ this.lineOfInterest);
+ this.$.diffBuilder.render(keyLocations, this._getBypassPrefs());
},
_handleRenderContent() {
@@ -676,6 +684,39 @@
},
/**
+ * Returns the key locations based on the comments and line of interests,
+ * where lines should not be collapsed.
+ *
+ * @param {!Object} comments
+ * @param {Defs.LineOfInterest|null} lineOfInterest
+ *
+ * @return {{left: Object<(string|number), boolean>,
+ * right: Object<(string|number), boolean>}}
+ */
+ _getKeyLocations(comments, lineOfInterest) {
+ const result = {
+ left: {},
+ right: {},
+ };
+ for (const side in comments) {
+ if (side !== GrDiffBuilder.Side.LEFT &&
+ side !== GrDiffBuilder.Side.RIGHT) {
+ continue;
+ }
+ for (const c of comments[side]) {
+ result[side][c.line || GrDiffLine.FILE] = true;
+ }
+ }
+
+ if (lineOfInterest) {
+ const side = lineOfInterest.leftSide ? 'left' : 'right';
+ result[side][lineOfInterest.number] = true;
+ }
+
+ return result;
+ },
+
+ /**
* Get the preferences object including the safety bypass context (if any).
*/
_getBypassPrefs() {
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 62284ad..c6ef806 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
@@ -1127,6 +1127,33 @@
assert.equal(element._computeNewlineWarningClass('foo', false), shown);
});
});
+
+ test('_getKeyLocations', () => {
+ assert.deepEqual(element._getKeyLocations({left: [], right: []}, null),
+ {left: {}, right: {}});
+ const comments = {
+ left: [{line: 123}, {}],
+ right: [{line: 456}],
+ };
+ assert.deepEqual(element._getKeyLocations(comments, null), {
+ left: {FILE: true, 123: true},
+ right: {456: true},
+ });
+
+ const lineOfInterest = {number: 789, leftSide: true};
+ assert.deepEqual(
+ element._getKeyLocations(comments, lineOfInterest), {
+ left: {FILE: true, 123: true, 789: true},
+ right: {456: true},
+ });
+
+ delete lineOfInterest.leftSide;
+ assert.deepEqual(
+ element._getKeyLocations(comments, lineOfInterest), {
+ left: {FILE: true, 123: true},
+ right: {456: true, 789: true},
+ });
+ });
});
a11ySuite('basic');
