diff --git a/lib/models/patch/multi-file-patch.js b/lib/models/patch/multi-file-patch.js index 8cf13824a4..e64570c7a6 100644 --- a/lib/models/patch/multi-file-patch.js +++ b/lib/models/patch/multi-file-patch.js @@ -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); } } @@ -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(); @@ -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) { diff --git a/test/models/patch/multi-file-patch.test.js b/test/models/patch/multi-file-patch.test.js index dc61a7dccd..a3515b7020 100644 --- a/test/models/patch/multi-file-patch.test.js +++ b/test/models/patch/multi-file-patch.test.js @@ -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() {