Improve description of moved chunks
- Replace due_to_move with move_details.
- Fetch line numbers and provide more info
about the move operation.
Change-Id: Ib2c4a6ac44deefc4e778cd18f17f45b290983213
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-side-by-side.ts b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-side-by-side.ts
index c203b33..efac2b1 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-side-by-side.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-side-by-side.ts
@@ -50,7 +50,7 @@
if (group.dueToRebase) {
sectionEl.classList.add('dueToRebase');
}
- if (group.dueToMove) {
+ if (group.moveDetails) {
sectionEl.classList.add('dueToMove');
sectionEl.appendChild(this._buildMoveControls(group));
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified.ts b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified.ts
index 9709e74..c4ea267 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified.ts
@@ -49,7 +49,7 @@
if (group.dueToRebase) {
sectionEl.classList.add('dueToRebase');
}
- if (group.dueToMove) {
+ if (group.moveDetails) {
sectionEl.classList.add('dueToMove');
sectionEl.appendChild(this._buildMoveControls(group));
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified_test.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified_test.js
index 07c6410..24d3635 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified_test.js
@@ -105,7 +105,7 @@
lines[0].text = 'def hello_world():';
lines[1].text = ' print "Hello World"';
const group = new GrDiffGroup(GrDiffGroupType.DELTA, lines);
- group.dueToMove = true;
+ group.moveDetails = {changed: false};
const sectionEl = diffBuilder.buildSectionElement(group);
@@ -128,7 +128,7 @@
lines[0].text = 'def hello_world():';
lines[1].text = ' print "Hello World"';
const group = new GrDiffGroup(GrDiffGroupType.DELTA, lines);
- group.dueToMove = true;
+ group.moveDetails = {changed: false};
const sectionEl = diffBuilder.buildSectionElement(group);
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 119bc15..f4aa660 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
@@ -892,6 +892,17 @@
}
}
+ _createMoveDescription(movedIn: boolean, group: GrDiffGroup) {
+ if (group.moveDetails?.range) {
+ const {changed, range} = group.moveDetails;
+ const moveLabel = 'Moved' + (changed ? ' and changed' : '');
+ const direction = movedIn ? 'from' : 'to';
+ const lineDetails = `lines ${range.start} - ${range.end}`;
+ return `${moveLabel} ${direction} ${lineDetails}`;
+ }
+ return movedIn ? 'Moved in' : 'Moved out';
+ }
+
_buildMoveControls(group: GrDiffGroup) {
const movedIn = group.adds.length > 0;
const {
@@ -901,16 +912,14 @@
} = this._getMoveControlsConfig();
let controlsClass;
- let descriptionText;
let descriptionIndex;
+ const descriptionText = this._createMoveDescription(movedIn, group);
if (movedIn) {
controlsClass = 'movedIn';
descriptionIndex = movedInIndex;
- descriptionText = 'Moved in';
} else {
controlsClass = 'movedOut';
descriptionIndex = movedOutIndex;
- descriptionText = 'Moved out';
}
const controls = document.createElement('tr');
const cells = [...Array(numberOfCells).keys()].map(() =>
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 f01de5f..718e11b 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
@@ -226,6 +226,7 @@
assert.equal(cursorElement.side, 'left');
});
+ // To be removed as soon due_to_move (deprecated) is removed
suite('moved chunks (dueToMove=true)', () => {
setup(done => {
const renderHandler = function() {
@@ -269,27 +270,62 @@
]};
});
- test('chunk skip functionality', () => {
- const chunks = diffElement.root.querySelectorAll(
- '.section.delta');
- const indexOfChunk = function(chunk) {
- return Array.prototype.indexOf.call(chunks, chunk);
+ test('renders moveControls with simple descriptions', () => {
+ const [movedIn, movedOut] = diffElement.root
+ .querySelectorAll('.dueToMove .moveControls');
+ assert.equal(movedIn.textContent, 'Moved in');
+ assert.equal(movedOut.textContent, 'Moved out');
+ });
+ });
+
+ suite('moved chunks (moveDetails)', () => {
+ setup(done => {
+ const renderHandler = function() {
+ diffElement.removeEventListener('render', renderHandler);
+ cursorElement.reInitCursor();
+ done();
};
+ diffElement.addEventListener('render', renderHandler);
+ diffElement.diff = {...diff, content: [
+ {
+ ab: [
+ 'Lorem ipsum dolor sit amet, suspendisse inceptos vehicula, ',
+ ],
+ },
+ {
+ b: [
+ 'Nullam neque, ligula ac, id blandit.',
+ 'Sagittis tincidunt torquent, tempor nunc amet.',
+ 'At rhoncus id.',
+ ],
+ move_details: {changed: false, range: {start: 4, end: 6}},
+ },
+ {
+ ab: [
+ 'Sem nascetur, erat ut, non in.',
+ ],
+ },
+ {
+ a: [
+ 'Nullam neque, ligula ac, id blandit.',
+ 'Sagittis tincidunt torquent, tempor nunc amet.',
+ 'At rhoncus id.',
+ ],
+ move_details: {changed: false, range: {start: 2, end: 4}},
+ },
+ {
+ ab: [
+ 'Arcu eget, rhoncus amet cursus, ipsum elementum.',
+ ],
+ },
+ ]};
+ });
- // We should be initialized to the first chunk (b)
- let currentIndex = indexOfChunk(cursorElement.diffRow.parentElement);
- assert.equal(currentIndex, 0);
- assert.equal(cursorElement.side, 'right');
-
- // Move to the next chunk.
- cursorElement.moveToNextChunk();
-
- // Since the next chunk only has content on the left side (a). we should have been
- // automatically moved over.
- const previousIndex = currentIndex;
- currentIndex = indexOfChunk(cursorElement.diffRow.parentElement);
- assert.equal(currentIndex, previousIndex + 1);
- assert.equal(cursorElement.side, 'left');
+ test('renders moveControls with simple descriptions', () => {
+ const [movedIn, movedOut] = diffElement.root
+ .querySelectorAll('.dueToMove .moveControls');
+ assert.equal(movedIn.textContent, 'Moved from lines 4 - 6');
+ assert.equal(movedOut.textContent, 'Moved to lines 2 - 4');
});
});
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 850e3ef..77b5499 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
@@ -366,7 +366,8 @@
const group = new GrDiffGroup(type, lines);
group.keyLocation = !!chunk.keyLocation;
group.dueToRebase = !!chunk.due_to_rebase;
- group.dueToMove = !!chunk.due_to_move;
+ group.moveDetails =
+ chunk.move_details || (chunk.due_to_move ? {changed: false} : undefined);
group.skip = chunk.skip;
group.ignoredWhitespaceOnly = !!chunk.common;
if (chunk.skip) {
@@ -700,6 +701,9 @@
if (chunk.due_to_move) {
subChunk.due_to_move = true;
}
+ if (chunk.move_details) {
+ subChunk.move_details = chunk.move_details;
+ }
return subChunk;
});
}
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 588b9d1..eb11588 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
@@ -237,8 +237,6 @@
dueToRebase = false;
- dueToMove = false;
-
/**
* True means all changes in this line are whitespace changes that should
* not be highlighted as changed as per the user settings.
@@ -269,6 +267,14 @@
right: {start: null, end: null},
};
+ moveDetails?: {
+ changed: boolean;
+ range?: {
+ start: number;
+ end: number;
+ };
+ };
+
/**
* Creates a new group with the same properties but different lines.
*
diff --git a/polygerrit-ui/app/types/diff.d.ts b/polygerrit-ui/app/types/diff.d.ts
index 417798a..b45b461 100644
--- a/polygerrit-ui/app/types/diff.d.ts
+++ b/polygerrit-ui/app/types/diff.d.ts
@@ -107,8 +107,25 @@
edit_b?: DiffIntralineInfo[];
/** Indicates whether this entry was introduced by a rebase. */
due_to_rebase?: boolean;
- /** Indicates whether this entry was introduced by a move. */
+ /** @deprecated Use move_details instead. */
due_to_move?: boolean;
+
+ /**
+ * Provides info about a move operation the chunk.
+ * It's presence indicates the current chunk exists due to a move.
+ */
+ move_details?: {
+ /** Indicates whether the content of the chunk changes while moving code */
+ changed: boolean;
+ /**
+ * Indicates the range (line numbers) on the other side of the comparison
+ * where the code related to the current chunk came from/went to.
+ */
+ range: {
+ start: number;
+ end: number;
+ };
+ };
/**
* Count of lines skipped on both sides when the file is too large to include
* all common lines.
