Add support to skip chunks
Change-Id: I4769eb25e9d990328e661f48230e2719d1515a0a
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 d1f52ae..91fd137 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
@@ -18,6 +18,7 @@
import {GrDiffLine, GrDiffLineType, LineNumber} from '../gr-diff/gr-diff-line';
import {
GrDiffGroup,
+ GrDiffGroupRange,
GrDiffGroupType,
hideInContextControl,
rangeBySide,
@@ -64,6 +65,10 @@
};
}
+export interface ContentLoadNeededEventDetail {
+ lineRange: GrDiffGroupRange;
+}
+
export abstract class GrDiffBuilder {
private readonly _diff: DiffInfo;
@@ -93,7 +98,7 @@
this._numLinesLeft = this._diff.content
? this._diff.content.reduce((sum, chunk) => {
const left = chunk.a || chunk.ab;
- return sum + (left ? left.length : 0);
+ return sum + (left?.length || chunk.skip || 0);
}, 0)
: 0;
this._prefs = prefs;
@@ -311,8 +316,13 @@
const td = this._createElement('td');
const showPartialLinks = numLines > PARTIAL_CONTEXT_AMOUNT;
-
- if (showPartialLinks && leftStart > 1) {
+ const firstGroupIsSkipped = !!contextGroups[0].skip;
+ const lastGroupIsSkipped = !!contextGroups[contextGroups.length - 1].skip;
+ const showAboveButton =
+ showPartialLinks && leftStart > 1 && !firstGroupIsSkipped;
+ const showBelowButton =
+ showPartialLinks && leftEnd < this._numLinesLeft && !lastGroupIsSkipped;
+ if (showAboveButton) {
td.appendChild(
this._createContextButton(
ContextButtonType.ABOVE,
@@ -322,7 +332,6 @@
)
);
}
-
td.appendChild(
this._createContextButton(
ContextButtonType.ALL,
@@ -331,8 +340,7 @@
numLines
)
);
-
- if (showPartialLinks && leftEnd < this._numLinesLeft) {
+ if (showBelowButton) {
td.appendChild(
this._createContextButton(
ContextButtonType.BELOW,
@@ -342,7 +350,6 @@
)
);
}
-
return td;
}
@@ -359,7 +366,9 @@
let text = '';
let groups: GrDiffGroup[] = []; // The groups that replace this one if tapped.
+ let requiresLoad = false;
if (type === GrDiffBuilder.ContextButtonType.ALL) {
+ requiresLoad = contextGroups.find(c => !!c.skip) !== undefined;
const icon = this._createElement('iron-icon', 'showContext');
icon.setAttribute('icon', 'gr-icons:unfold-more');
button.appendChild(icon);
@@ -368,6 +377,10 @@
if (numLines > 1) {
text += 's';
}
+ if (requiresLoad) {
+ // Expanding content would require load of more data
+ text += ' (too large)';
+ }
groups.push(...contextGroups);
} else if (type === GrDiffBuilder.ContextButtonType.ABOVE) {
text = `+${context} above`;
@@ -380,15 +393,36 @@
textSpan.textContent = text;
button.appendChild(textSpan);
- button.addEventListener('tap', e => {
- const event = e as ContextEvent;
- event.detail = {
- groups,
- section,
- numLines,
- };
- // Let it bubble up the DOM tree.
- });
+ if (requiresLoad) {
+ button.addEventListener('tap', e => {
+ e.stopPropagation();
+ const firstRange = groups[0].lineRange;
+ const lastRange = groups[groups.length - 1].lineRange;
+ const lineRange = {
+ left: {start: firstRange.left.start, end: lastRange.left.end},
+ right: {start: firstRange.right.start, end: lastRange.right.end},
+ };
+ button.dispatchEvent(
+ new CustomEvent<ContentLoadNeededEventDetail>('content-load-needed', {
+ detail: {
+ lineRange,
+ },
+ bubbles: true,
+ composed: true,
+ })
+ );
+ });
+ } else {
+ button.addEventListener('tap', e => {
+ const event = e as ContextEvent;
+ event.detail = {
+ groups,
+ section,
+ numLines,
+ };
+ // Let it bubble up the DOM tree.
+ });
+ }
return button;
}
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 39e12ca..08ea1a6 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
@@ -274,7 +274,7 @@
}
_isCollapsibleChunk(chunk: DiffContent) {
- return (chunk.ab || chunk.common) && !chunk.keyLocation;
+ return (chunk.ab || chunk.common || chunk.skip) && !chunk.keyLocation;
}
/**
@@ -326,7 +326,11 @@
}
_commonChunkLength(chunk: DiffContent) {
+ if (chunk.skip) {
+ return chunk.skip;
+ }
console.assert(!!chunk.ab || !!chunk.common);
+
console.assert(
!chunk.a || (!!chunk.b && chunk.a.length === chunk.b.length),
'common chunk needs same number of a and b lines: ',
@@ -354,13 +358,21 @@
offsetLeft: number,
offsetRight: number
): GrDiffGroup {
- const type = chunk.ab ? GrDiffGroupType.BOTH : GrDiffGroupType.DELTA;
+ const type =
+ chunk.ab || chunk.skip ? GrDiffGroupType.BOTH : GrDiffGroupType.DELTA;
const lines = this._linesFromChunk(chunk, offsetLeft, offsetRight);
const group = new GrDiffGroup(type, lines);
group.keyLocation = !!chunk.keyLocation;
group.dueToRebase = !!chunk.due_to_rebase;
group.dueToMove = !!chunk.due_to_move;
+ group.skip = chunk.skip;
group.ignoredWhitespaceOnly = !!chunk.common;
+ if (chunk.skip) {
+ group.lineRange = {
+ left: {start: offsetLeft, end: offsetLeft + chunk.skip - 1},
+ right: {start: offsetRight, end: offsetRight + chunk.skip - 1},
+ };
+ }
return group;
}
@@ -497,7 +509,7 @@
for (const chunk of chunks) {
// If it isn't a common chunk, append it as-is and update line numbers.
- if (!chunk.ab && !chunk.common) {
+ if (!chunk.ab && !chunk.skip && !chunk.common) {
if (chunk.a) {
leftLineNum += chunk.a.length;
}
@@ -522,7 +534,13 @@
leftLineNum += numLines;
rightLineNum += numLines;
- if (chunk.ab) {
+ if (chunk.skip) {
+ result.push({
+ ...chunk,
+ skip: chunk.skip,
+ keyLocation: false,
+ });
+ } else if (chunk.ab) {
result.push(
...this._splitAtChunkEnds(chunk.ab, chunkEnds).map(
({lines, keyLocation}) => {
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 3a074bb..eed5900 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
@@ -171,6 +171,56 @@
});
});
+ test('at the beginning with skip chunks', async () => {
+ element.context = 10;
+ const content = [
+ {ab: new Array(20)
+ .fill('all work and no play make jack a dull boy')},
+ {skip: 43900},
+ {ab: new Array(30)
+ .fill('some other content')},
+ {a: ['some other content']},
+ ];
+
+ await element.process(content);
+
+ const groups = element.groups;
+
+ // group[0] is the file group
+
+ const commonGroup = groups[1];
+
+ // Hidden context before
+ assert.equal(commonGroup.type, GrDiffGroupType.CONTEXT_CONTROL);
+ assert.instanceOf(commonGroup.contextGroups[0], GrDiffGroup);
+ assert.equal(commonGroup.contextGroups[0].lines.length, 20);
+ for (const l of commonGroup.contextGroups[0].lines) {
+ assert.equal(l.text, 'all work and no play make jack a dull boy');
+ }
+
+ // Skipped group
+ const skipGroup = commonGroup.contextGroups[1];
+ assert.equal(skipGroup.skip, 43900);
+ const expectedRange = {
+ left: {start: 21, end: 43920},
+ right: {start: 21, end: 43920},
+ };
+ assert.deepEqual(skipGroup.lineRange, expectedRange);
+
+ // Hidden context after
+ assert.equal(commonGroup.contextGroups[2].lines.length, 20);
+ for (const l of commonGroup.contextGroups[2].lines) {
+ assert.equal(l.text, 'some other content');
+ }
+
+ // Displayed lines
+ assert.equal(groups[2].type, GrDiffGroupType.BOTH);
+ assert.equal(groups[2].lines.length, 10);
+ for (const l of groups[2].lines) {
+ assert.equal(l.text, 'some other content');
+ }
+ });
+
test('at the beginning, smaller than context', () => {
element.context = 10;
const content = [
@@ -414,6 +464,55 @@
});
});
+ test('in the middle with skip chunks', async () => {
+ element.context = 10;
+ const content = [
+ {a: ['all work and no play make andybons a dull boy']},
+ {ab: new Array(20)
+ .fill('all work and no play make jill a dull girl')},
+ {skip: 60},
+ {ab: new Array(20)
+ .fill('all work and no play make jill a dull girl')},
+ {a: ['all work and no play make andybons a dull boy']},
+ ];
+
+ await element.process(content);
+
+ const groups = element.groups;
+
+ // group[0] is the file group
+ // group[1] is the chunk with a
+ // group[2] is the displayed part of ab before
+
+ const commonGroup = groups[3];
+
+ // Hidden context before
+ assert.equal(commonGroup.type, GrDiffGroupType.CONTEXT_CONTROL);
+ assert.instanceOf(commonGroup.contextGroups[0], GrDiffGroup);
+ assert.equal(commonGroup.contextGroups[0].lines.length, 10);
+ for (const l of commonGroup.contextGroups[0].lines) {
+ assert.equal(
+ l.text, 'all work and no play make jill a dull girl');
+ }
+
+ // Skipped group
+ const skipGroup = commonGroup.contextGroups[1];
+ assert.equal(skipGroup.skip, 60);
+ const expectedRange = {
+ left: {start: 22, end: 81},
+ right: {start: 21, end: 80},
+ };
+ assert.deepEqual(skipGroup.lineRange, expectedRange);
+
+ // Hidden context after
+ assert.equal(commonGroup.contextGroups[2].lines.length, 10);
+ for (const l of commonGroup.contextGroups[2].lines) {
+ assert.equal(
+ l.text, 'all work and no play make jill a dull girl');
+ }
+ // group[4] is the displayed part of the second ab
+ });
+
test('break up common diff chunks', () => {
element.keyLocations = {
left: {1: true},
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 9f5cdf3..588b9d1 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
@@ -38,7 +38,7 @@
end: number | null;
}
-interface GrDiffGroupRange {
+export interface GrDiffGroupRange {
left: Range;
right: Range;
}
@@ -89,7 +89,13 @@
[before, hidden] = _splitCommonGroups(hidden, hiddenStart);
}
if (hiddenEnd) {
- [hidden, after] = _splitCommonGroups(hidden, hiddenEnd - hiddenStart);
+ let beforeLength = 0;
+ if (before.length > 0) {
+ const beforeStart = before[0].lineRange.left.start || 0;
+ const beforeEnd = before[before.length - 1].lineRange.left.end || 0;
+ beforeLength = beforeEnd - beforeStart + 1;
+ }
+ [hidden, after] = _splitCommonGroups(hidden, hiddenEnd - beforeLength);
}
} else {
[hidden, after] = [[], hidden];
@@ -106,6 +112,67 @@
}
/**
+ * Splits a group in two, defined by leftSplit and rightSplit. Primarily to be
+ * used in function _splitCommonGroups
+ * Groups with some lines before and some lines after the split will be split
+ * into two groups, which will be put into the first and second list.
+ *
+ * @param group The group to be split in two
+ * @param leftSplit The line number relative to the split on the left side
+ * @param rightSplit The line number relative to the split on the right side
+ * @return two new groups, one before the split and another after it
+ */
+function _splitGroupInTwo(
+ group: GrDiffGroup,
+ leftSplit: number,
+ rightSplit: number
+) {
+ let beforeSplit: GrDiffGroup | undefined;
+ let afterSplit: GrDiffGroup | undefined;
+ // split line is in the middle of a group, we need to break the group
+ // in lines before and after the split.
+ if (group.skip) {
+ // Currently we assume skip chunks "refuse" to be split. Expanding this
+ // group will in the future mean load more data - and therefore we want to
+ // fire an event when user wants to do it.
+ const closerToStartThanEnd =
+ leftSplit - (group.lineRange.left.start || 0) <
+ (group.lineRange.right.end || 0) - leftSplit;
+ if (closerToStartThanEnd) {
+ afterSplit = group;
+ } else {
+ beforeSplit = group;
+ }
+ } else {
+ const before = [];
+ const after = [];
+ for (const line of group.lines) {
+ if (
+ (line.beforeNumber && line.beforeNumber < leftSplit) ||
+ (line.afterNumber && line.afterNumber < rightSplit)
+ ) {
+ before.push(line);
+ } else {
+ after.push(line);
+ }
+ }
+ if (before.length) {
+ beforeSplit =
+ before.length === group.lines.length
+ ? group
+ : group.cloneWithLines(before);
+ }
+ if (after.length) {
+ afterSplit =
+ after.length === group.lines.length
+ ? group
+ : group.cloneWithLines(after);
+ }
+ }
+ return {beforeSplit, afterSplit};
+}
+
+/**
* Splits a list of common groups into two lists of groups.
*
* Groups where all lines are before or all lines are after the split will be
@@ -129,47 +196,28 @@
const beforeGroups = [];
const afterGroups = [];
for (const group of groups) {
- if (
+ const isCompletelyBefore =
(group.lineRange.left.end || 0) < leftSplit ||
- (group.lineRange.right.end || 0) < rightSplit
- ) {
- beforeGroups.push(group);
- continue;
- }
- if (
+ (group.lineRange.right.end || 0) < rightSplit;
+ const isCompletelyAfter =
leftSplit <= (group.lineRange.left.start || 0) ||
- rightSplit <= (group.lineRange.right.start || 0)
- ) {
+ rightSplit <= (group.lineRange.right.start || 0);
+ if (isCompletelyBefore) {
+ beforeGroups.push(group);
+ } else if (isCompletelyAfter) {
afterGroups.push(group);
- continue;
- }
-
- const before = [];
- const after = [];
- for (const line of group.lines) {
- if (
- (line.beforeNumber && line.beforeNumber < leftSplit) ||
- (line.afterNumber && line.afterNumber < rightSplit)
- ) {
- before.push(line);
- } else {
- after.push(line);
+ } else {
+ const {beforeSplit, afterSplit} = _splitGroupInTwo(
+ group,
+ leftSplit,
+ rightSplit
+ );
+ if (beforeSplit) {
+ beforeGroups.push(beforeSplit);
}
- }
-
- if (before.length) {
- beforeGroups.push(
- before.length === group.lines.length
- ? group
- : group.cloneWithLines(before)
- );
- }
- if (after.length) {
- afterGroups.push(
- after.length === group.lines.length
- ? group
- : group.cloneWithLines(after)
- );
+ if (afterSplit) {
+ afterGroups.push(afterSplit);
+ }
}
}
return [beforeGroups, afterGroups];
@@ -213,6 +261,8 @@
contextGroups: GrDiffGroup[] = [];
+ skip?: number;
+
/** Both start and end line are inclusive. */
lineRange: GrDiffGroupRange = {
left: {start: null, end: null},
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-group_test.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-group_test.js
index 13fc4d1..3423834 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-group_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-group_test.js
@@ -177,6 +177,35 @@
assert.deepEqual(collapsedGroups[3].lines, groups[2].lines.slice(1));
});
+ suite('with skip chunks', () => {
+ setup(() => {
+ const skipGroup = new GrDiffGroup(GrDiffGroupType.BOTH);
+ skipGroup.skip = 60;
+ skipGroup.lineRange = {
+ left: {start: 8, end: 67},
+ right: {start: 10, end: 69},
+ };
+ groups = [
+ new GrDiffGroup(GrDiffGroupType.BOTH, [
+ new GrDiffLine(GrDiffLineType.BOTH, 5, 7),
+ new GrDiffLine(GrDiffLineType.BOTH, 6, 8),
+ new GrDiffLine(GrDiffLineType.BOTH, 7, 9),
+ ]),
+ skipGroup,
+ new GrDiffGroup(GrDiffGroupType.BOTH, [
+ new GrDiffLine(GrDiffLineType.BOTH, 68, 70),
+ new GrDiffLine(GrDiffLineType.BOTH, 69, 71),
+ new GrDiffLine(GrDiffLineType.BOTH, 70, 72),
+ ]),
+ ];
+ });
+
+ test('refuses to split skip group when closer to before', () => {
+ const collapsedGroups = hideInContextControl(groups, 4, 10);
+ assert.deepEqual(groups, collapsedGroups);
+ });
+ });
+
test('groups unchanged if the hidden range is empty', () => {
assert.deepEqual(
hideInContextControl(groups, 0, 0), groups);
diff --git a/polygerrit-ui/app/types/common.ts b/polygerrit-ui/app/types/common.ts
index 6b184b9..96cb02a 100644
--- a/polygerrit-ui/app/types/common.ts
+++ b/polygerrit-ui/app/types/common.ts
@@ -1254,7 +1254,7 @@
edit_b?: number[][];
due_to_rebase?: boolean;
due_to_move?: boolean;
- skip?: string;
+ skip?: number;
common?: string;
keyLocation?: boolean;
}
