Skip to content
This repository was archived by the owner on Dec 15, 2022. It is now read-only.

Fix bug expanding large-diff-gated patches with comments #2007

Merged
merged 4 commits into from
Mar 6, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 29 additions & 15 deletions lib/models/patch/multi-file-patch.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,21 +24,7 @@ export default class MultiFilePatch {
this.filePatchesByPath.set(filePatch.getPath(), filePatch);
this.filePatchesByMarker.set(filePatch.getMarker(), filePatch);

let diffRow = 1;
const index = new RBTree((a, b) => a.diffRow - b.diffRow);
this.diffRowOffsetIndices.set(filePatch.getPath(), {startBufferRow: filePatch.getStartRange().start.row, index});

for (let hunkIndex = 0; hunkIndex < filePatch.getHunks().length; hunkIndex++) {
const hunk = filePatch.getHunks()[hunkIndex];
this.hunksByMarker.set(hunk.getMarker(), hunk);

// Advance past the hunk body
diffRow += hunk.bufferRowCount();
index.insert({diffRow, offset: hunkIndex + 1});

// Advance past the next hunk header
diffRow++;
}
this.populateDiffRowOffsetIndices(filePatch);
}
}

Expand Down Expand Up @@ -210,6 +196,28 @@ export default class MultiFilePatch {
return Range.fromObject([[selectionRow, 0], [selectionRow, Infinity]]);
}

isDiffRowOffsetIndexEmpty(filePatchPath) {
const diffRowOffsetIndex = this.diffRowOffsetIndices.get(filePatchPath);
return diffRowOffsetIndex.index.size === 0;
}
populateDiffRowOffsetIndices(filePatch) {
let diffRow = 1;
const index = new RBTree((a, b) => a.diffRow - b.diffRow);
this.diffRowOffsetIndices.set(filePatch.getPath(), {startBufferRow: filePatch.getStartRange().start.row, index});

for (let hunkIndex = 0; hunkIndex < filePatch.getHunks().length; hunkIndex++) {
const hunk = filePatch.getHunks()[hunkIndex];
this.hunksByMarker.set(hunk.getMarker(), hunk);

// Advance past the hunk body
diffRow += hunk.bufferRowCount();
index.insert({diffRow, offset: hunkIndex + 1});

// Advance past the next hunk header
diffRow++;
}
}

adoptBuffer(nextPatchBuffer) {
nextPatchBuffer.clearAllLayers();

Expand Down Expand Up @@ -332,6 +340,12 @@ export default class MultiFilePatch {
for (const hunk of filePatch.getHunks()) {
this.hunksByMarker.set(hunk.getMarker(), hunk);
}

// if the patch was initially collapsed, we need to calculate
// the diffRowOffsetIndices to calculate comment position.
if (this.isDiffRowOffsetIndexEmpty(filePatch.getPath())) {
this.populateDiffRowOffsetIndices(filePatch);
}
}

getMarkersBefore(filePatchIndex) {
Expand Down
35 changes: 35 additions & 0 deletions test/models/patch/multi-file-patch.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -780,6 +780,41 @@ describe('MultiFilePatch', function() {
assert.strictEqual(multiFilePatch.getBufferRowForDiffPosition('2.txt', 13), 30);
assert.strictEqual(multiFilePatch.getBufferRowForDiffPosition('2.txt', 15), 32);
});

it('set the offset for diff-gated file patch upon expanding', function() {
const {multiFilePatch} = multiFilePatchBuilder()
.addFilePatch(fp => {
fp.setOldFile(f => f.path('1.txt'));
fp.addHunk(h => h.unchanged('0 (1)').added('1 (2)', '2 (3)').deleted('3 (4)').unchanged('4 (5)'));
fp.addHunk(h => h.unchanged('5 (7)').added('6 (8)', '7 (9)', '8 (10)').unchanged('9 (11)'));
fp.addHunk(h => h.unchanged('10 (13)').deleted('11 (14)').unchanged('12 (15)'));
fp.renderStatus(TOO_LARGE);
})
.build();
assert.isTrue(multiFilePatch.isDiffRowOffsetIndexEmpty('1.txt'));
const [fp] = multiFilePatch.getFilePatches();
multiFilePatch.expandFilePatch(fp);
assert.isFalse(multiFilePatch.isDiffRowOffsetIndexEmpty('1.txt'));
assert.strictEqual(multiFilePatch.getBufferRowForDiffPosition('1.txt', 11), 9);
});

it('does not reset the offset for normally collapsed file patch upon expanding', function() {
const {multiFilePatch} = multiFilePatchBuilder()
.addFilePatch(fp => {
fp.setOldFile(f => f.path('0.txt'));
fp.addHunk(h => h.oldRow(1).unchanged('0-0').deleted('0-1', '0-2').unchanged('0-3'));
})
.build();

const [fp] = multiFilePatch.getFilePatches();
const stub = sinon.stub(multiFilePatch, 'populateDiffRowOffsetIndices');

multiFilePatch.collapseFilePatch(fp);
assert.strictEqual(multiFilePatch.getBuffer().getText(), '');

multiFilePatch.expandFilePatch(fp);
assert.isFalse(stub.called);
});
});

describe('collapsing and expanding file patches', function() {
Expand Down