Add new slot for ported threads without a range
Change-Id: I5f4ecb35d4ecee6f56db7ae7a3029ce88775c5c7
diff --git a/polygerrit-ui/app/api/diff.ts b/polygerrit-ui/app/api/diff.ts
index b5624c6..7e619c2 100644
--- a/polygerrit-ui/app/api/diff.ts
+++ b/polygerrit-ui/app/api/diff.ts
@@ -225,7 +225,7 @@
code_range: LineRange;
}
-export declare type LineNumber = number | 'FILE';
+export declare type LineNumber = number | 'FILE' | 'LOST';
/** The detail of the 'create-comment' event dispatched by gr-diff. */
export declare interface CreateCommentEventDetail {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element_test.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element_test.js
index 229ca76..0ae0e84 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element_test.js
@@ -866,7 +866,7 @@
});
test('getSectionsByLineRange one line', () => {
- const section = outputEl.querySelector('stub:nth-of-type(2)');
+ const section = outputEl.querySelector('stub:nth-of-type(3)');
const sections = element._builder.getSectionsByLineRange(1, 1, 'left');
assert.equal(sections.length, 1);
assert.strictEqual(sections[0], section);
@@ -874,8 +874,8 @@
test('getSectionsByLineRange over diff', () => {
const section = [
- outputEl.querySelector('stub:nth-of-type(2)'),
outputEl.querySelector('stub:nth-of-type(3)'),
+ outputEl.querySelector('stub:nth-of-type(4)'),
];
const sections = element._builder.getSectionsByLineRange(1, 2, 'left');
assert.equal(sections.length, 2);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.ts b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.ts
index 1d95057..da1b928 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.ts
@@ -541,7 +541,10 @@
td.classList.add('lineNum');
td.dataset['value'] = number.toString();
- if (this._prefs.show_file_comment_button === false && number === 'FILE') {
+ if (
+ (this._prefs.show_file_comment_button === false && number === 'FILE') ||
+ number === 'LOST'
+ ) {
return td;
}
@@ -589,7 +592,7 @@
}
td.classList.add(line.type);
- if (line.beforeNumber !== 'FILE') {
+ if (line.beforeNumber !== 'FILE' && line.beforeNumber !== 'LOST') {
const lineLimit = !this._prefs.line_wrapping
? this._prefs.line_length
: Infinity;
@@ -614,9 +617,8 @@
}
td.appendChild(contentText);
- } else {
- td.classList.add('file');
- }
+ } else if (line.beforeNumber === 'FILE') td.classList.add('file');
+ else if (line.beforeNumber === 'LOST') td.classList.add('lost');
return td;
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.js b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.js
index 4160d38..60b82da 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.js
@@ -606,7 +606,7 @@
test('_findRowByNumberAndFile', () => {
// Get the first ab row after the first chunk.
- const row = diffElement.root.querySelectorAll('tr')[8];
+ const row = diffElement.root.querySelectorAll('tr')[9];
// It should be line 8 on the right, but line 5 on the left.
assert.equal(cursorElement._findRowByNumberAndFile(8, 'right'), row);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.ts b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.ts
index fd5c19d..d5c7d8e 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.ts
@@ -357,7 +357,7 @@
const side = this.diffBuilder.getSideByLineEl(lineEl);
if (!side) return null;
const line = this.diffBuilder.getLineNumberByChild(lineEl);
- if (!line || line === FILE) return null;
+ if (!line || line === FILE || line === 'LOST') return null;
const contentTd = this.diffBuilder.getContentTdByLineEl(lineEl);
if (!contentTd) return null;
const contentText = contentTd.querySelector('.contentText');
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
index 89ee170..1b1e71c 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
@@ -840,7 +840,10 @@
_createThreadElement(thread: CommentThread) {
const threadEl = document.createElement('gr-comment-thread');
threadEl.className = 'comment-thread';
- threadEl.setAttribute('slot', `${thread.diffSide}-${thread.line}`);
+ threadEl.setAttribute(
+ 'slot',
+ `${thread.diffSide}-${thread.line || 'LOST'}`
+ );
threadEl.comments = thread.comments;
threadEl.diffSide = thread.diffSide;
threadEl.isOnParent = thread.commentSide === CommentSide.PARENT;
@@ -861,8 +864,9 @@
threadEl.patchNum = thread.patchNum;
threadEl.showPatchset = false;
threadEl.showPortedComment = !!thread.ported;
+ if (thread.rangeInfoLost) threadEl.lineNum = 'LOST';
// GrCommentThread does not understand 'FILE', but requires undefined.
- threadEl.lineNum = thread.line !== 'FILE' ? thread.line : undefined;
+ else threadEl.lineNum = thread.line !== 'FILE' ? thread.line : undefined;
threadEl.projectName = this.projectName;
threadEl.range = thread.range;
const threadDiscardListener = (e: Event) => {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.ts b/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.ts
index a0584b6..034081d 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.ts
@@ -22,6 +22,7 @@
GrDiffLineType,
FILE,
Highlights,
+ LineNumber,
} from '../gr-diff/gr-diff-line';
import {
GrDiffGroup,
@@ -150,7 +151,8 @@
this.cancel();
this.groups = [];
- this.push('groups', this._makeFileComments());
+ this.push('groups', this._makeGroup('LOST'));
+ this.push('groups', this._makeGroup(FILE));
// If it's a binary diff, we won't be rendering hunks of text differences
// so finish processing.
@@ -450,10 +452,10 @@
return line;
}
- _makeFileComments() {
+ _makeGroup(number: LineNumber) {
const line = new GrDiffLine(GrDiffLineType.BOTH);
- line.beforeNumber = FILE;
- line.afterNumber = FILE;
+ line.beforeNumber = number;
+ line.afterNumber = number;
return new GrDiffGroup(GrDiffGroupType.BOTH, [line]);
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor_test.js b/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor_test.js
index f5cbcc0..b8f7498 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor_test.js
@@ -73,7 +73,7 @@
return element.process(content).then(() => {
const groups = element.groups;
-
+ groups.shift(); // remove portedThreadsWithoutRangeGroup
assert.equal(groups.length, 4);
let group = groups[0];
@@ -133,6 +133,7 @@
return element.process(content).then(() => {
const groups = element.groups;
+ groups.shift(); // remove portedThreadsWithoutRangeGroup
assert.equal(groups[0].type, GrDiffGroupType.BOTH);
assert.equal(groups[0].lines.length, 1);
@@ -153,6 +154,7 @@
return element.process(content).then(() => {
const groups = element.groups;
+ groups.shift(); // remove portedThreadsWithoutRangeGroup
// group[0] is the file group
@@ -185,6 +187,7 @@
await element.process(content);
const groups = element.groups;
+ groups.shift(); // remove portedThreadsWithoutRangeGroup
// group[0] is the file group
@@ -231,6 +234,7 @@
return element.process(content).then(() => {
const groups = element.groups;
+ groups.shift(); // remove portedThreadsWithoutRangeGroup
// group[0] is the file group
@@ -252,6 +256,7 @@
return element.process(content).then(() => {
const groups = element.groups;
+ groups.shift(); // remove portedThreadsWithoutRangeGroup
// group[0] is the file group
// group[1] is the "a" group
@@ -283,6 +288,7 @@
return element.process(content).then(() => {
const groups = element.groups;
+ groups.shift(); // remove portedThreadsWithoutRangeGroup
// group[0] is the file group
// group[1] is the "a" group
@@ -324,6 +330,7 @@
return element.process(content).then(() => {
const groups = element.groups;
+ groups.shift(); // remove portedThreadsWithoutRangeGroup
// group[0] is the file group
// group[1] is the "a" group
@@ -411,6 +418,7 @@
return element.process(content).then(() => {
const groups = element.groups;
+ groups.shift(); // remove portedThreadsWithoutRangeGroup
// group[0] is the file group
// group[1] is the "a" group
@@ -450,6 +458,7 @@
return element.process(content).then(() => {
const groups = element.groups;
+ groups.shift(); // remove portedThreadsWithoutRangeGroup
// group[0] is the file group
// group[1] is the "a" group
@@ -479,6 +488,7 @@
await element.process(content);
const groups = element.groups;
+ groups.shift(); // remove portedThreadsWithoutRangeGroup
// group[0] is the file group
// group[1] is the chunk with a
@@ -744,12 +754,12 @@
element._isScrolling = true;
element.process(content);
// Just the files group - no more processing during scrolling.
- assert.equal(element.groups.length, 1);
+ assert.equal(element.groups.length, 2);
element._isScrolling = false;
element.process(content);
// More groups have been processed. How many does not matter here.
- assert.isAtLeast(element.groups.length, 2);
+ assert.isAtLeast(element.groups.length, 3);
});
test('image diffs', () => {
@@ -762,7 +772,7 @@
const content = _.times(200, _.constant(contentRow));
sinon.stub(element, 'async');
element.process(content, true);
- assert.equal(element.groups.length, 1);
+ assert.equal(element.groups.length, 2);
// Image diffs don't process content, just the 'FILE' line.
assert.equal(element.groups[0].lines.length, 1);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-group.ts b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-group.ts
index 3fd7775..ba6fe7e 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-group.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-group.ts
@@ -324,7 +324,12 @@
}
_updateRange(line: GrDiffLine) {
- if (line.beforeNumber === 'FILE' || line.afterNumber === 'FILE') {
+ if (
+ line.beforeNumber === 'FILE' ||
+ line.afterNumber === 'FILE' ||
+ line.beforeNumber === 'LOST' ||
+ line.afterNumber === 'LOST'
+ ) {
return;
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-utils.ts b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-utils.ts
index 1e5a8d3..5edd353 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-utils.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-utils.ts
@@ -48,12 +48,14 @@
const lineNumberStr = lineEl.getAttribute('data-value');
if (!lineNumberStr) return null;
if (lineNumberStr === FILE) return FILE;
+ if (lineNumberStr === 'LOST') return 'LOST';
const lineNumber = Number(lineNumberStr);
return Number.isInteger(lineNumber) ? lineNumber : null;
}
export function getLine(threadEl: HTMLElement): LineNumber {
const lineAtt = threadEl.getAttribute('line-num');
+ if (lineAtt === 'LOST') return lineAtt;
if (!lineAtt || lineAtt === 'FILE') return FILE;
const line = Number(lineAtt);
if (isNaN(line)) throw new Error(`cannot parse line number: ${lineAtt}`);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
index b85d948..9fb5e74 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
@@ -518,8 +518,9 @@
);
this.$.diffBuilder.showContext(e.detail.groups, e.detail.section);
} else if (
- el.classList.contains('lineNum') ||
- el.classList.contains('lineNumButton')
+ el.getAttribute('data-value') !== 'LOST' &&
+ (el.classList.contains('lineNum') ||
+ el.classList.contains('lineNumButton'))
) {
this.addDraftAtLine(el);
} else if (
@@ -817,6 +818,9 @@
}
const contentEl = this.$.diffBuilder.getContentTdByLineEl(lineEl);
if (!contentEl) continue;
+ if (lineNum === 'LOST' && !contentEl.hasChildNodes()) {
+ contentEl.appendChild(this._portedCommentsWithoutRangeMessage());
+ }
const threadGroupEl = this._getOrCreateThreadGroup(
contentEl,
commentSide
@@ -862,6 +866,17 @@
});
}
+ _portedCommentsWithoutRangeMessage() {
+ const div = document.createElement('div');
+ const icon = document.createElement('iron-icon');
+ icon.setAttribute('icon', 'gr-icons:info');
+ div.appendChild(icon);
+ const span = document.createElement('span');
+ span.innerText = 'Original comment position not found in this patchset';
+ div.appendChild(span);
+ return div;
+ }
+
_unobserveIncrementalNodes() {
if (this._incrementalNodeObserver) {
(dom(this) as PolymerDomWrapper).unobserveNodes(
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.ts b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.ts
index 291f842..b0f48ce 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.ts
@@ -427,6 +427,13 @@
.target-row td.blame {
background: var(--diff-selection-background-color);
}
+ td.lost div {
+ background-color: var(--blue-50);
+ padding: var(--spacing-s);
+ }
+ td.lost iron-icon {
+ margin-right: var(--spacing-s);
+ }
col.blame {
display: none;
}
diff --git a/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.ts b/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.ts
index 04bb3d2..da29b85 100644
--- a/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.ts
+++ b/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.ts
@@ -212,9 +212,8 @@
*/
annotate(el: HTMLElement, _: HTMLElement, line: GrDiffLine) {
if (!this.enabled) return;
- if (line.beforeNumber === FILE) return;
- if (line.afterNumber === FILE) return;
-
+ if (line.beforeNumber === FILE || line.afterNumber === FILE) return;
+ if (line.beforeNumber === 'LOST' || line.afterNumber === 'LOST') return;
// Determine the side.
let side;
if (
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
index c8b3be2..cb64001 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
@@ -277,6 +277,7 @@
(this.comments.length && this.comments[0].side === 'PARENT') ||
isDraft(this.comments[0])
) {
+ if (this.lineNum === 'LOST') throw new Error('invalid lineNum lost');
return GerritNav.getUrlForDiffById(
changeNum,
projectName,
@@ -503,7 +504,7 @@
__draftID: Math.random().toString(36),
__date: new Date(),
};
-
+ if (lineNum === 'LOST') throw new Error('invalid lineNum lost');
// For replies, always use same meta info as root.
if (this.comments && this.comments.length >= 1) {
const rootComment = this.comments[0];
