From a8bea5f424eb82f0bd8337c95ede19c8adc8a75e Mon Sep 17 00:00:00 2001 From: Vanessa Yuen Date: Mon, 14 Jan 2019 19:45:59 +0100 Subject: [PATCH 01/92] when diff is decided to be "too large", we skip hunk building and return a placeholder filepatch. --- lib/models/patch/builder.js | 32 +++++++++++++++++++++++++++++--- lib/models/patch/file-patch.js | 5 +++++ 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/lib/models/patch/builder.js b/lib/models/patch/builder.js index e8e18982a7..511d15c5a6 100644 --- a/lib/models/patch/builder.js +++ b/lib/models/patch/builder.js @@ -10,6 +10,8 @@ import MultiFilePatch from './multi-file-patch'; export function buildFilePatch(diffs) { const layeredBuffer = initializeBuffer(); + isDiffLarge(diffs); + let filePatch; if (diffs.length === 0) { filePatch = emptyDiffFilePatch(); @@ -80,8 +82,6 @@ function singleDiffFilePatch(diff, layeredBuffer) { const wasSymlink = diff.oldMode === File.modes.SYMLINK; const isSymlink = diff.newMode === File.modes.SYMLINK; - const [hunks, patchMarker] = buildHunks(diff, layeredBuffer); - let oldSymlink = null; let newSymlink = null; if (wasSymlink && !isSymlink) { @@ -99,6 +99,12 @@ function singleDiffFilePatch(diff, layeredBuffer) { const newFile = diff.newPath !== null || diff.newMode !== null ? new File({path: diff.newPath, mode: diff.newMode, symlink: newSymlink}) : nullFile; + + if (isDiffLarge([diff])) { + return FilePatch.createPlaceholder(oldFile, newFile); + } + + const [hunks, patchMarker] = buildHunks(diff, layeredBuffer); const patch = new Patch({status: diff.status, hunks, marker: patchMarker, buffer: layeredBuffer.buffer}); return new FilePatch(oldFile, newFile, patch); @@ -114,7 +120,6 @@ function dualDiffFilePatch(diff1, diff2, layeredBuffer) { contentChangeDiff = diff1; } - const [hunks, patchMarker] = buildHunks(contentChangeDiff, layeredBuffer); const filePath = contentChangeDiff.oldPath || contentChangeDiff.newPath; const symlink = modeChangeDiff.hunks[0].lines[0].slice(1); @@ -140,6 +145,12 @@ function dualDiffFilePatch(diff1, diff2, layeredBuffer) { const oldFile = new File({path: filePath, mode: oldMode, symlink: oldSymlink}); const newFile = new File({path: filePath, mode: newMode, symlink: newSymlink}); + + if (isDiffLarge([diff1, diff2])) { + return FilePatch.createPlaceholder(oldFile, newFile); + } + + const [hunks, patchMarker] = buildHunks(contentChangeDiff, layeredBuffer); const patch = new Patch({status, hunks, marker: patchMarker, buffer: layeredBuffer.buffer}); return new FilePatch(oldFile, newFile, patch); @@ -252,3 +263,18 @@ function buildHunks(diff, {buffer, layers}) { return [hunks, patchMarker]; } + +function isDiffLarge(diffs) { + const DIFF_BYTE_THRESHOLD = 32768; + + const size = diffs.reduce((diffSizeCounter, diff) => { + return diffSizeCounter + diff.hunks.reduce((hunkSizeCounter, hunk) => { + return hunkSizeCounter + hunk.lines.reduce((lineSizeCounter, line) => { + return lineSizeCounter + Buffer.byteLength(line, 'utf8'); + }, 0); + }, 0); + }, 0); + + console.log(diffs, `size: ${size}`); + return size > DIFF_BYTE_THRESHOLD; +} diff --git a/lib/models/patch/file-patch.js b/lib/models/patch/file-patch.js index c40bd48dd3..9d3c0c3ce9 100644 --- a/lib/models/patch/file-patch.js +++ b/lib/models/patch/file-patch.js @@ -7,6 +7,11 @@ export default class FilePatch { return new this(nullFile, nullFile, Patch.createNull()); } + // TODO: explain what this is for + static createPlaceholder(oldFile, newFile) { + return new this(oldFile, newFile, Patch.createNull()); + } + constructor(oldFile, newFile, patch) { this.oldFile = oldFile; this.newFile = newFile; From e2862ea0515ae40c361fb466b2401006b370d77a Mon Sep 17 00:00:00 2001 From: Vanessa Yuen Date: Mon, 14 Jan 2019 20:49:00 +0100 Subject: [PATCH 02/92] visually display the diff gate --- lib/models/patch/builder.js | 2 -- lib/models/patch/file-patch.js | 5 +++++ lib/views/multi-file-patch-view.js | 13 +++++++++++++ 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/lib/models/patch/builder.js b/lib/models/patch/builder.js index 511d15c5a6..b34ae9c3cd 100644 --- a/lib/models/patch/builder.js +++ b/lib/models/patch/builder.js @@ -10,8 +10,6 @@ import MultiFilePatch from './multi-file-patch'; export function buildFilePatch(diffs) { const layeredBuffer = initializeBuffer(); - isDiffLarge(diffs); - let filePatch; if (diffs.length === 0) { filePatch = emptyDiffFilePatch(); diff --git a/lib/models/patch/file-patch.js b/lib/models/patch/file-patch.js index 9d3c0c3ce9..b70fbfc3f2 100644 --- a/lib/models/patch/file-patch.js +++ b/lib/models/patch/file-patch.js @@ -22,6 +22,11 @@ export default class FilePatch { return this.oldFile.isPresent() || this.newFile.isPresent() || this.patch.isPresent(); } + isLarge() { + // TODO: fix this + return (this.oldFile.isPresent() || this.newFile.isPresent()) && !this.patch.isPresent(); + } + getOldFile() { return this.oldFile; } diff --git a/lib/views/multi-file-patch-view.js b/lib/views/multi-file-patch-view.js index 9d0655b770..a8c50824ec 100644 --- a/lib/views/multi-file-patch-view.js +++ b/lib/views/multi-file-patch-view.js @@ -353,11 +353,24 @@ export default class MultiFilePatchView extends React.Component { + {filePatch.isLarge() && this.renderDiffGate(filePatch)} + {this.renderHunkHeaders(filePatch)} ); } + renderDiffGate(filePatch) { + // TODO: startRange is incorrect here + return ( + + +

Diff is too large to be displayed.

+ + + ); + } + renderExecutableModeChangeMeta(filePatch) { if (!filePatch.didChangeExecutableMode()) { return null; From a9db48f084e1c4c65cebc7288f61ae90f1101aef Mon Sep 17 00:00:00 2001 From: Vanessa Yuen Date: Wed, 16 Jan 2019 14:20:30 +0100 Subject: [PATCH 03/92] experimenting wiht a DelayedPatch but idk --- lib/models/patch/builder.js | 14 ++++++++++---- lib/models/patch/file-patch.js | 5 ++--- lib/models/patch/patch.js | 22 ++++++++++++++++++++++ 3 files changed, 34 insertions(+), 7 deletions(-) diff --git a/lib/models/patch/builder.js b/lib/models/patch/builder.js index b34ae9c3cd..4a8679bb76 100644 --- a/lib/models/patch/builder.js +++ b/lib/models/patch/builder.js @@ -1,4 +1,4 @@ -import {TextBuffer} from 'atom'; +import {TextBuffer, Point} from 'atom'; import Hunk from './hunk'; import File, {nullFile} from './file'; @@ -99,7 +99,12 @@ function singleDiffFilePatch(diff, layeredBuffer) { : nullFile; if (isDiffLarge([diff])) { - return FilePatch.createPlaceholder(oldFile, newFile); + const delayed = FilePatch.createDelayedFilePatch( + oldFile, newFile, diff, + new Point(layeredBuffer.buffer.getLastRow(), 0), + ); + console.log(delayed.getPatch()); + return delayed; } const [hunks, patchMarker] = buildHunks(diff, layeredBuffer); @@ -145,7 +150,7 @@ function dualDiffFilePatch(diff1, diff2, layeredBuffer) { const newFile = new File({path: filePath, mode: newMode, symlink: newSymlink}); if (isDiffLarge([diff1, diff2])) { - return FilePatch.createPlaceholder(oldFile, newFile); + // TODO: do something with large diff too } const [hunks, patchMarker] = buildHunks(contentChangeDiff, layeredBuffer); @@ -263,7 +268,8 @@ function buildHunks(diff, {buffer, layers}) { } function isDiffLarge(diffs) { - const DIFF_BYTE_THRESHOLD = 32768; + // TODO: fixme + const DIFF_BYTE_THRESHOLD = 500; const size = diffs.reduce((diffSizeCounter, diff) => { return diffSizeCounter + diff.hunks.reduce((hunkSizeCounter, hunk) => { diff --git a/lib/models/patch/file-patch.js b/lib/models/patch/file-patch.js index b70fbfc3f2..4882c2760f 100644 --- a/lib/models/patch/file-patch.js +++ b/lib/models/patch/file-patch.js @@ -7,9 +7,8 @@ export default class FilePatch { return new this(nullFile, nullFile, Patch.createNull()); } - // TODO: explain what this is for - static createPlaceholder(oldFile, newFile) { - return new this(oldFile, newFile, Patch.createNull()); + static createDelayedFilePatch(oldFile, newFile, diff, insertPoint) { + return new this(oldFile, newFile, Patch.createDelayedPatch(diff, insertPoint)); } constructor(oldFile, newFile, patch) { diff --git a/lib/models/patch/patch.js b/lib/models/patch/patch.js index 0fdafa0011..57bd975dad 100644 --- a/lib/models/patch/patch.js +++ b/lib/models/patch/patch.js @@ -8,6 +8,10 @@ export default class Patch { return new NullPatch(); } + static createDelayedPatch(diff, insertPoint) { + return new DelayedPatch(diff, insertPoint); + } + constructor({status, hunks, marker}) { this.status = status; this.hunks = hunks; @@ -346,6 +350,24 @@ class NullPatch { } } +// TODO: it prob shouldn't extend NullPatch +class DelayedPatch extends NullPatch { + // should know: + // where to insert + // what the diff is + // reason it's delayed (???) + constructor(diff, insertPoint) { + super(); + this.insertPoint = insertPoint; + this.diff = diff; + } + + getStartRange() { + return new Range(this.insertPoint, this.insertPoint); + } + +} + class BufferBuilder { constructor(original, originalBaseOffset, nextLayeredBuffer) { this.originalBuffer = original; From f621d491fbbf67e53db0b29e7e6d643ec5343a30 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 16 Jan 2019 09:54:07 -0500 Subject: [PATCH 04/92] Delayed patch parsing Co-Authored-By: Vanessa Yuen --- lib/models/patch/builder.js | 63 +++++++++++++--------- lib/models/patch/file-patch.js | 13 +++-- lib/models/patch/patch.js | 38 +++++++++++--- test/models/patch/builder.test.js | 86 +++++++++++++++++++++++++++++++ 4 files changed, 164 insertions(+), 36 deletions(-) diff --git a/lib/models/patch/builder.js b/lib/models/patch/builder.js index 4a8679bb76..30adefe7f2 100644 --- a/lib/models/patch/builder.js +++ b/lib/models/patch/builder.js @@ -2,21 +2,29 @@ import {TextBuffer, Point} from 'atom'; import Hunk from './hunk'; import File, {nullFile} from './file'; -import Patch from './patch'; +import Patch, {TOO_LARGE} from './patch'; import {Unchanged, Addition, Deletion, NoNewline} from './region'; import FilePatch from './file-patch'; import MultiFilePatch from './multi-file-patch'; -export function buildFilePatch(diffs) { +// TODO: fixme +const DEFAULT_LARGE_DIFF_THRESHOLD = 500; + +export function buildFilePatch(diffs, options) { + const opts = { + largeDiffThreshold: DEFAULT_LARGE_DIFF_THRESHOLD, + ...options, + }; + const layeredBuffer = initializeBuffer(); let filePatch; if (diffs.length === 0) { filePatch = emptyDiffFilePatch(); } else if (diffs.length === 1) { - filePatch = singleDiffFilePatch(diffs[0], layeredBuffer); + filePatch = singleDiffFilePatch(diffs[0], layeredBuffer, opts); } else if (diffs.length === 2) { - filePatch = dualDiffFilePatch(diffs[0], diffs[1], layeredBuffer); + filePatch = dualDiffFilePatch(diffs[0], diffs[1], layeredBuffer, opts); } else { throw new Error(`Unexpected number of diffs: ${diffs.length}`); } @@ -24,7 +32,12 @@ export function buildFilePatch(diffs) { return new MultiFilePatch({filePatches: [filePatch], ...layeredBuffer}); } -export function buildMultiFilePatch(diffs) { +export function buildMultiFilePatch(diffs, options) { + const opts = { + largeDiffThreshold: DEFAULT_LARGE_DIFF_THRESHOLD, + ...options, + }; + const layeredBuffer = initializeBuffer(); const byPath = new Map(); const actions = []; @@ -40,7 +53,7 @@ export function buildMultiFilePatch(diffs) { if (otherHalf) { // The second half. Complete the paired diff, or fail if they have unexpected statuses or modes. const [otherDiff, otherIndex] = otherHalf; - actions[otherIndex] = () => dualDiffFilePatch(diff, otherDiff, layeredBuffer); + actions[otherIndex] = () => dualDiffFilePatch(diff, otherDiff, layeredBuffer, opts); byPath.delete(thePath); } else { // The first half we've seen. @@ -48,7 +61,7 @@ export function buildMultiFilePatch(diffs) { index++; } } else { - actions[index] = () => singleDiffFilePatch(diff, layeredBuffer); + actions[index] = () => singleDiffFilePatch(diff, layeredBuffer, opts); index++; } } @@ -76,7 +89,7 @@ function emptyDiffFilePatch() { return FilePatch.createNull(); } -function singleDiffFilePatch(diff, layeredBuffer) { +function singleDiffFilePatch(diff, layeredBuffer, opts) { const wasSymlink = diff.oldMode === File.modes.SYMLINK; const isSymlink = diff.newMode === File.modes.SYMLINK; @@ -98,22 +111,27 @@ function singleDiffFilePatch(diff, layeredBuffer) { ? new File({path: diff.newPath, mode: diff.newMode, symlink: newSymlink}) : nullFile; - if (isDiffLarge([diff])) { + if (isDiffLarge([diff], opts)) { + const insertPoint = new Point(layeredBuffer.buffer.getLastRow(), 0); const delayed = FilePatch.createDelayedFilePatch( - oldFile, newFile, diff, - new Point(layeredBuffer.buffer.getLastRow(), 0), + oldFile, newFile, insertPoint, + TOO_LARGE, + () => { + const [hunks, patchMarker] = buildHunks(diff, layeredBuffer, insertPoint.row); + return new Patch({status: diff.status, hunks, marker: patchMarker, buffer: layeredBuffer.buffer}); + }, ); console.log(delayed.getPatch()); return delayed; } - const [hunks, patchMarker] = buildHunks(diff, layeredBuffer); + const [hunks, patchMarker] = buildHunks(diff, layeredBuffer, layeredBuffer.buffer.getLastRow()); const patch = new Patch({status: diff.status, hunks, marker: patchMarker, buffer: layeredBuffer.buffer}); return new FilePatch(oldFile, newFile, patch); } -function dualDiffFilePatch(diff1, diff2, layeredBuffer) { +function dualDiffFilePatch(diff1, diff2, layeredBuffer, opts) { let modeChangeDiff, contentChangeDiff; if (diff1.oldMode === File.modes.SYMLINK || diff1.newMode === File.modes.SYMLINK) { modeChangeDiff = diff1; @@ -153,7 +171,7 @@ function dualDiffFilePatch(diff1, diff2, layeredBuffer) { // TODO: do something with large diff too } - const [hunks, patchMarker] = buildHunks(contentChangeDiff, layeredBuffer); + const [hunks, patchMarker] = buildHunks(contentChangeDiff, layeredBuffer, layeredBuffer.buffer.getLastRow()); const patch = new Patch({status, hunks, marker: patchMarker, buffer: layeredBuffer.buffer}); return new FilePatch(oldFile, newFile, patch); @@ -178,7 +196,7 @@ function initializeBuffer() { return {buffer, layers}; } -function buildHunks(diff, {buffer, layers}) { +function buildHunks(diff, {buffer, layers}, insertRow) { const layersByKind = new Map([ [Unchanged, layers.unchanged], [Addition, layers.addition], @@ -187,7 +205,7 @@ function buildHunks(diff, {buffer, layers}) { ]); const hunks = []; - const patchStartRow = buffer.getLastRow(); + const patchStartRow = insertRow; let bufferRow = patchStartRow; let nextLineLength = 0; @@ -228,7 +246,7 @@ function buildHunks(diff, {buffer, layers}) { for (const lineText of hunkData.lines) { const bufferLine = lineText.slice(1) + '\n'; nextLineLength = lineText.length - 1; - buffer.append(bufferLine); + buffer.insert([bufferRow, 0], bufferLine); const ChangeKind = CHANGEKIND[lineText[0]]; if (ChangeKind === undefined) { @@ -267,18 +285,13 @@ function buildHunks(diff, {buffer, layers}) { return [hunks, patchMarker]; } -function isDiffLarge(diffs) { - // TODO: fixme - const DIFF_BYTE_THRESHOLD = 500; - +function isDiffLarge(diffs, opts) { const size = diffs.reduce((diffSizeCounter, diff) => { return diffSizeCounter + diff.hunks.reduce((hunkSizeCounter, hunk) => { - return hunkSizeCounter + hunk.lines.reduce((lineSizeCounter, line) => { - return lineSizeCounter + Buffer.byteLength(line, 'utf8'); - }, 0); + return hunkSizeCounter + hunk.lines.length; }, 0); }, 0); console.log(diffs, `size: ${size}`); - return size > DIFF_BYTE_THRESHOLD; + return size > opts.largeDiffThreshold; } diff --git a/lib/models/patch/file-patch.js b/lib/models/patch/file-patch.js index 4882c2760f..868ce03753 100644 --- a/lib/models/patch/file-patch.js +++ b/lib/models/patch/file-patch.js @@ -7,8 +7,8 @@ export default class FilePatch { return new this(nullFile, nullFile, Patch.createNull()); } - static createDelayedFilePatch(oldFile, newFile, diff, insertPoint) { - return new this(oldFile, newFile, Patch.createDelayedPatch(diff, insertPoint)); + static createDelayedFilePatch(oldFile, newFile, position, renderStatus, parseFn) { + return new this(oldFile, newFile, Patch.createDelayedPatch(position, renderStatus, parseFn)); } constructor(oldFile, newFile, patch) { @@ -21,9 +21,8 @@ export default class FilePatch { return this.oldFile.isPresent() || this.newFile.isPresent() || this.patch.isPresent(); } - isLarge() { - // TODO: fix this - return (this.oldFile.isPresent() || this.newFile.isPresent()) && !this.patch.isPresent(); + getRenderStatus() { + return this.patch.getRenderStatus(); } getOldFile() { @@ -116,6 +115,10 @@ export default class FilePatch { return this.getPatch().getHunks(); } + triggerDelayedRender() { + this.patch = this.patch.parseFn(); + } + clone(opts = {}) { return new this.constructor( opts.oldFile !== undefined ? opts.oldFile : this.oldFile, diff --git a/lib/models/patch/patch.js b/lib/models/patch/patch.js index 57bd975dad..bfe2a69778 100644 --- a/lib/models/patch/patch.js +++ b/lib/models/patch/patch.js @@ -3,13 +3,19 @@ import {TextBuffer, Range} from 'atom'; import Hunk from './hunk'; import {Unchanged, Addition, Deletion, NoNewline} from './region'; +export const TOO_LARGE = Symbol('too large'); + +export const EXPANDED = Symbol('expanded'); + +export const COLLAPSED = Symbol('collapsed'); + export default class Patch { static createNull() { return new NullPatch(); } - static createDelayedPatch(diff, insertPoint) { - return new DelayedPatch(diff, insertPoint); + static createDelayedPatch(position, renderStatus, parseFn) { + return new DelayedPatch(position, renderStatus, parseFn); } constructor({status, hunks, marker}) { @@ -269,6 +275,14 @@ export default class Patch { isPresent() { return true; } + + getRenderStatus() { + return EXPANDED; + } + + parseFn() { + return this; + } } class NullPatch { @@ -348,6 +362,14 @@ class NullPatch { isPresent() { return false; } + + renderStatus() { + return EXPANDED; + } + + parseFn() { + return this; + } } // TODO: it prob shouldn't extend NullPatch @@ -356,16 +378,20 @@ class DelayedPatch extends NullPatch { // where to insert // what the diff is // reason it's delayed (???) - constructor(diff, insertPoint) { + constructor(position, renderStatus, parseFn) { super(); - this.insertPoint = insertPoint; - this.diff = diff; + this.position = position; + this.renderStatus = renderStatus; + this.parseFn = parseFn; } getStartRange() { - return new Range(this.insertPoint, this.insertPoint); + return new Range(this.position, this.position); } + getRenderStatus() { + return this.renderStatus; + } } class BufferBuilder { diff --git a/test/models/patch/builder.test.js b/test/models/patch/builder.test.js index 8e64488f05..9e759c2ebe 100644 --- a/test/models/patch/builder.test.js +++ b/test/models/patch/builder.test.js @@ -1,4 +1,5 @@ import {buildFilePatch, buildMultiFilePatch} from '../../../lib/models/patch'; +import {TOO_LARGE, EXPANDED} from '../../../lib/models/patch/patch'; import {assertInPatch, assertInFilePatch} from '../../helpers'; describe('buildFilePatch', function() { @@ -800,6 +801,91 @@ describe('buildFilePatch', function() { }); }); + describe('with a large diff', function() { + it('creates a DelayedPatch when the diff is "too large"', function() { + const mfp = buildMultiFilePatch([ + { + oldPath: 'first', oldMode: '100644', newPath: 'first', newMode: '100755', status: 'modified', + hunks: [ + { + oldStartLine: 1, oldLineCount: 3, newStartLine: 1, newLineCount: 3, + lines: [' line-0', '+line-1', '-line-2', ' line-3'], + }, + ], + }, + { + oldPath: 'second', oldMode: '100644', newPath: 'second', newMode: '100755', status: 'modified', + hunks: [ + { + oldStartLine: 1, oldLineCount: 1, newStartLine: 1, newLineCount: 2, + lines: [' line-4', '+line-5'], + }, + ], + }, + ], {largeDiffThreshold: 3}); + + assert.lengthOf(mfp.getFilePatches(), 2); + const [fp0, fp1] = mfp.getFilePatches(); + + assert.strictEqual(fp0.getRenderStatus(), TOO_LARGE); + assert.strictEqual(fp0.getOldPath(), 'first'); + assert.strictEqual(fp0.getNewPath(), 'first'); + assert.deepEqual(fp0.getStartRange().serialize(), [[0, 0], [0, 0]]); + assertInFilePatch(fp0).hunks(); + + assert.strictEqual(fp1.getRenderStatus(), EXPANDED); + assert.strictEqual(fp1.getOldPath(), 'second'); + assert.strictEqual(fp1.getNewPath(), 'second'); + assert.deepEqual(fp1.getMarker().getRange().serialize(), [[0, 0], [1, 6]]); + assertInFilePatch(fp1, mfp.getBuffer()).hunks( + { + startRow: 0, endRow: 1, header: '@@ -1,1 +1,2 @@', regions: [ + {kind: 'unchanged', string: ' line-4\n', range: [[0, 0], [0, 6]]}, + {kind: 'addition', string: '+line-5\n', range: [[1, 0], [1, 6]]}, + ], + }, + ); + }); + + it('re-parse a DelayedPatch as a Patch', function() { + const mfp = buildMultiFilePatch([ + { + oldPath: 'first', oldMode: '100644', newPath: 'first', newMode: '100644', status: 'modified', + hunks: [ + { + oldStartLine: 1, oldLineCount: 3, newStartLine: 1, newLineCount: 3, + lines: [' line-0', '+line-1', '-line-2', ' line-3'], + }, + ], + }, + ], {largeDiffThreshold: 3}); + + assert.lengthOf(mfp.getFilePatches(), 1); + const [fp] = mfp.getFilePatches(); + + assert.strictEqual(fp.getRenderStatus(), TOO_LARGE); + assert.strictEqual(fp.getOldPath(), 'first'); + assert.strictEqual(fp.getNewPath(), 'first'); + assert.deepEqual(fp.getStartRange().serialize(), [[0, 0], [0, 0]]); + assertInFilePatch(fp).hunks(); + + fp.triggerDelayedRender(); + + assert.strictEqual(fp.getRenderStatus(), EXPANDED); + assert.deepEqual(fp.getMarker().getRange().serialize(), [[0, 0], [3, 6]]); + assertInFilePatch(fp, mfp.getBuffer()).hunks( + { + startRow: 0, endRow: 3, header: '@@ -1,3 +1,3 @@', regions: [ + {kind: 'unchanged', string: ' line-0\n', range: [[0, 0], [0, 6]]}, + {kind: 'addition', string: '+line-1\n', range: [[1, 0], [1, 6]]}, + {kind: 'deletion', string: '-line-2\n', range: [[2, 0], [2, 6]]}, + {kind: 'unchanged', string: ' line-3\n', range: [[3, 0], [3, 6]]}, + ], + }, + ); + }); + }); + it('throws an error with an unexpected number of diffs', function() { assert.throws(() => buildFilePatch([1, 2, 3]), /Unexpected number of diffs: 3/); }); From a729493cdfbbb70faf728bb3ab1b1b32754807b2 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 16 Jan 2019 10:55:34 -0500 Subject: [PATCH 05/92] Correct pre-existing markers after building patch hunks --- lib/models/patch/builder.js | 11 ++++ test/models/patch/builder.test.js | 99 +++++++++++++++++++++++++++++++ 2 files changed, 110 insertions(+) diff --git a/lib/models/patch/builder.js b/lib/models/patch/builder.js index 30adefe7f2..9a52850237 100644 --- a/lib/models/patch/builder.js +++ b/lib/models/patch/builder.js @@ -205,6 +205,12 @@ function buildHunks(diff, {buffer, layers}, insertRow) { ]); const hunks = []; + const afterMarkers = []; + for (const layerName in layers) { + const layer = layers[layerName]; + afterMarkers.push(...layer.findMarkers({startPosition: [insertRow, 0]})); + } + const patchStartRow = insertRow; let bufferRow = patchStartRow; let nextLineLength = 0; @@ -282,6 +288,11 @@ function buildHunks(diff, {buffer, layers}, insertRow) { {invalidate: 'never', exclusive: false}, ); + // Correct any markers that used to start at the insertion point. + for (const marker of afterMarkers) { + marker.setTailPosition([bufferRow, 0]); + } + return [hunks, patchMarker]; } diff --git a/test/models/patch/builder.test.js b/test/models/patch/builder.test.js index 9e759c2ebe..550b17d506 100644 --- a/test/models/patch/builder.test.js +++ b/test/models/patch/builder.test.js @@ -884,6 +884,105 @@ describe('buildFilePatch', function() { }, ); }); + + it('does not interfere with markers from surrounding patches when expanded', function() { + const mfp = buildMultiFilePatch([ + { + oldPath: 'first', oldMode: '100644', newPath: 'first', newMode: '100644', status: 'modified', + hunks: [ + { + oldStartLine: 1, oldLineCount: 3, newStartLine: 1, newLineCount: 3, + lines: [' line-0', '+line-1', '-line-2', ' line-3'], + }, + ], + }, + { + oldPath: 'big', oldMode: '100644', newPath: 'big', newMode: '100644', status: 'modified', + hunks: [ + { + oldStartLine: 1, oldLineCount: 3, newStartLine: 1, newLineCount: 4, + lines: [' line-0', '+line-1', '+line-2', '-line-3', ' line-4'], + }, + ], + }, + { + oldPath: 'last', oldMode: '100644', newPath: 'last', newMode: '100644', status: 'modified', + hunks: [ + { + oldStartLine: 1, oldLineCount: 3, newStartLine: 1, newLineCount: 3, + lines: [' line-0', '+line-1', '-line-2', ' line-3'], + }, + ], + } + ], {largeDiffThreshold: 4}); + + assert.lengthOf(mfp.getFilePatches(), 3); + const [fp0, fp1, fp2] = mfp.getFilePatches(); + assert.strictEqual(fp0.getRenderStatus(), EXPANDED); + assertInFilePatch(fp0, mfp.getBuffer()).hunks( + { + startRow: 0, endRow: 3, header: '@@ -1,3 +1,3 @@', regions: [ + {kind: 'unchanged', string: ' line-0\n', range: [[0, 0], [0, 6]]}, + {kind: 'addition', string: '+line-1\n', range: [[1, 0], [1, 6]]}, + {kind: 'deletion', string: '-line-2\n', range: [[2, 0], [2, 6]]}, + {kind: 'unchanged', string: ' line-3\n', range: [[3, 0], [3, 6]]}, + ], + }, + ); + + assert.strictEqual(fp1.getRenderStatus(), TOO_LARGE); + assertInFilePatch(fp1, mfp.getBuffer()).hunks(); + + assert.strictEqual(fp2.getRenderStatus(), EXPANDED); + assertInFilePatch(fp2, mfp.getBuffer()).hunks( + { + startRow: 4, endRow: 7, header: '@@ -1,3 +1,3 @@', regions: [ + {kind: 'unchanged', string: ' line-0\n', range: [[4, 0], [4, 6]]}, + {kind: 'addition', string: '+line-1\n', range: [[5, 0], [5, 6]]}, + {kind: 'deletion', string: '-line-2\n', range: [[6, 0], [6, 6]]}, + {kind: 'unchanged', string: ' line-3\n', range: [[7, 0], [7, 6]]}, + ], + }, + ); + + fp1.triggerDelayedRender(); + + assert.strictEqual(fp0.getRenderStatus(), EXPANDED); + assertInFilePatch(fp0, mfp.getBuffer()).hunks( + { + startRow: 0, endRow: 3, header: '@@ -1,3 +1,3 @@', regions: [ + {kind: 'unchanged', string: ' line-0\n', range: [[0, 0], [0, 6]]}, + {kind: 'addition', string: '+line-1\n', range: [[1, 0], [1, 6]]}, + {kind: 'deletion', string: '-line-2\n', range: [[2, 0], [2, 6]]}, + {kind: 'unchanged', string: ' line-3\n', range: [[3, 0], [3, 6]]}, + ], + }, + ); + + assert.strictEqual(fp1.getRenderStatus(), EXPANDED); + assertInFilePatch(fp1, mfp.getBuffer()).hunks( + { + startRow: 4, endRow: 8, header: '@@ -1,3 +1,4 @@', regions: [ + {kind: 'unchanged', string: ' line-0\n', range: [[4, 0], [4, 6]]}, + {kind: 'addition', string: '+line-1\n+line-2\n', range: [[5, 0], [6, 6]]}, + {kind: 'deletion', string: '-line-3\n', range: [[7, 0], [7, 6]]}, + {kind: 'unchanged', string: ' line-4\n', range: [[8, 0], [8, 6]]}, + ], + }, + ); + + assert.strictEqual(fp2.getRenderStatus(), EXPANDED); + assertInFilePatch(fp2, mfp.getBuffer()).hunks( + { + startRow: 9, endRow: 12, header: '@@ -1,3 +1,3 @@', regions: [ + {kind: 'unchanged', string: ' line-0\n', range: [[9, 0], [9, 6]]}, + {kind: 'addition', string: '+line-1\n', range: [[10, 0], [10, 6]]}, + {kind: 'deletion', string: '-line-2\n', range: [[11, 0], [11, 6]]}, + {kind: 'unchanged', string: ' line-3\n', range: [[12, 0], [12, 6]]}, + ], + }, + ); + }); }); it('throws an error with an unexpected number of diffs', function() { From 68926e5d4ffe41548b5117605f8767d3a4888be1 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 16 Jan 2019 11:03:36 -0500 Subject: [PATCH 06/92] :shirt: --- test/models/patch/builder.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/models/patch/builder.test.js b/test/models/patch/builder.test.js index 550b17d506..5ee54e42d3 100644 --- a/test/models/patch/builder.test.js +++ b/test/models/patch/builder.test.js @@ -913,7 +913,7 @@ describe('buildFilePatch', function() { lines: [' line-0', '+line-1', '-line-2', ' line-3'], }, ], - } + }, ], {largeDiffThreshold: 4}); assert.lengthOf(mfp.getFilePatches(), 3); From 4686cfcda3224df682830ed69150e73dc742e0c0 Mon Sep 17 00:00:00 2001 From: Vanessa Yuen Date: Wed, 16 Jan 2019 18:14:10 +0100 Subject: [PATCH 07/92] get things to render properly first --- lib/models/patch/builder.js | 8 +++----- lib/views/multi-file-patch-view.js | 4 +++- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/models/patch/builder.js b/lib/models/patch/builder.js index 9a52850237..67c8140e31 100644 --- a/lib/models/patch/builder.js +++ b/lib/models/patch/builder.js @@ -8,7 +8,7 @@ import FilePatch from './file-patch'; import MultiFilePatch from './multi-file-patch'; // TODO: fixme -const DEFAULT_LARGE_DIFF_THRESHOLD = 500; +const DEFAULT_LARGE_DIFF_THRESHOLD = 10; export function buildFilePatch(diffs, options) { const opts = { @@ -68,7 +68,7 @@ export function buildMultiFilePatch(diffs, options) { // Populate unpaired diffs that looked like they could be part of a pair, but weren't. for (const [unpairedDiff, originalIndex] of byPath.values()) { - actions[originalIndex] = () => singleDiffFilePatch(unpairedDiff, layeredBuffer); + actions[originalIndex] = () => singleDiffFilePatch(unpairedDiff, layeredBuffer, opts); } const filePatches = actions.map(action => action()); @@ -121,7 +121,6 @@ function singleDiffFilePatch(diff, layeredBuffer, opts) { return new Patch({status: diff.status, hunks, marker: patchMarker, buffer: layeredBuffer.buffer}); }, ); - console.log(delayed.getPatch()); return delayed; } @@ -167,7 +166,7 @@ function dualDiffFilePatch(diff1, diff2, layeredBuffer, opts) { const oldFile = new File({path: filePath, mode: oldMode, symlink: oldSymlink}); const newFile = new File({path: filePath, mode: newMode, symlink: newSymlink}); - if (isDiffLarge([diff1, diff2])) { + if (isDiffLarge([diff1, diff2]), opts) { // TODO: do something with large diff too } @@ -303,6 +302,5 @@ function isDiffLarge(diffs, opts) { }, 0); }, 0); - console.log(diffs, `size: ${size}`); return size > opts.largeDiffThreshold; } diff --git a/lib/views/multi-file-patch-view.js b/lib/views/multi-file-patch-view.js index a8c50824ec..bf2b7daa3e 100644 --- a/lib/views/multi-file-patch-view.js +++ b/lib/views/multi-file-patch-view.js @@ -22,6 +22,8 @@ import CommitDetailItem from '../items/commit-detail-item'; import IssueishDetailItem from '../items/issueish-detail-item'; import File from '../models/patch/file'; +import {TOO_LARGE} from '../models/patch/patch'; + const executableText = { [File.modes.NORMAL]: 'non executable', [File.modes.EXECUTABLE]: 'executable', @@ -353,7 +355,7 @@ export default class MultiFilePatchView extends React.Component { - {filePatch.isLarge() && this.renderDiffGate(filePatch)} + {filePatch.getPatch().getRenderStatus() === TOO_LARGE && this.renderDiffGate(filePatch)} {this.renderHunkHeaders(filePatch)} From 0c3a2a6457690458e831c9da7f6629a242149a4f Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 16 Jan 2019 12:25:33 -0500 Subject: [PATCH 08/92] Accept renderStatusOverrides in ye MultiFilePatch builder --- lib/models/patch/builder.js | 50 ++++++++++++++++++------------- test/models/patch/builder.test.js | 32 ++++++++++++++++++++ 2 files changed, 61 insertions(+), 21 deletions(-) diff --git a/lib/models/patch/builder.js b/lib/models/patch/builder.js index 67c8140e31..6c75fc5b0d 100644 --- a/lib/models/patch/builder.js +++ b/lib/models/patch/builder.js @@ -2,19 +2,22 @@ import {TextBuffer, Point} from 'atom'; import Hunk from './hunk'; import File, {nullFile} from './file'; -import Patch, {TOO_LARGE} from './patch'; +import Patch, {TOO_LARGE, EXPANDED} from './patch'; import {Unchanged, Addition, Deletion, NoNewline} from './region'; import FilePatch from './file-patch'; import MultiFilePatch from './multi-file-patch'; -// TODO: fixme -const DEFAULT_LARGE_DIFF_THRESHOLD = 10; +const DEFAULT_OPTIONS = { + // Number of lines after which we consider the diff "large" + // TODO: Set this based on performance measurements + largeDiffThreshold: 10, + + // Map of file path (relative to repository root) to Patch render status (EXPANDED, COLLAPSED, TOO_LARGE) + renderStatusOverrides: {}, +}; export function buildFilePatch(diffs, options) { - const opts = { - largeDiffThreshold: DEFAULT_LARGE_DIFF_THRESHOLD, - ...options, - }; + const opts = {...DEFAULT_OPTIONS, ...options}; const layeredBuffer = initializeBuffer(); @@ -33,10 +36,7 @@ export function buildFilePatch(diffs, options) { } export function buildMultiFilePatch(diffs, options) { - const opts = { - largeDiffThreshold: DEFAULT_LARGE_DIFF_THRESHOLD, - ...options, - }; + const opts = {...DEFAULT_OPTIONS, ...options}; const layeredBuffer = initializeBuffer(); const byPath = new Map(); @@ -111,23 +111,31 @@ function singleDiffFilePatch(diff, layeredBuffer, opts) { ? new File({path: diff.newPath, mode: diff.newMode, symlink: newSymlink}) : nullFile; - if (isDiffLarge([diff], opts)) { + const renderStatusOverride = + (oldFile.isPresent() && opts.renderStatusOverrides[oldFile.getPath()]) || + (newFile.isPresent() && opts.renderStatusOverrides[newFile.getPath()]) || + undefined; + + const renderStatus = renderStatusOverride || + (isDiffLarge([diff], opts) && TOO_LARGE) || + EXPANDED; + + if (renderStatus === TOO_LARGE) { const insertPoint = new Point(layeredBuffer.buffer.getLastRow(), 0); - const delayed = FilePatch.createDelayedFilePatch( - oldFile, newFile, insertPoint, - TOO_LARGE, + + return FilePatch.createDelayedFilePatch( + oldFile, newFile, insertPoint, TOO_LARGE, () => { const [hunks, patchMarker] = buildHunks(diff, layeredBuffer, insertPoint.row); return new Patch({status: diff.status, hunks, marker: patchMarker, buffer: layeredBuffer.buffer}); }, ); - return delayed; - } - - const [hunks, patchMarker] = buildHunks(diff, layeredBuffer, layeredBuffer.buffer.getLastRow()); - const patch = new Patch({status: diff.status, hunks, marker: patchMarker, buffer: layeredBuffer.buffer}); + } else { + const [hunks, patchMarker] = buildHunks(diff, layeredBuffer, layeredBuffer.buffer.getLastRow()); + const patch = new Patch({status: diff.status, hunks, marker: patchMarker, buffer: layeredBuffer.buffer}); - return new FilePatch(oldFile, newFile, patch); + return new FilePatch(oldFile, newFile, patch); + } } function dualDiffFilePatch(diff1, diff2, layeredBuffer, opts) { diff --git a/test/models/patch/builder.test.js b/test/models/patch/builder.test.js index 5ee54e42d3..59951c2f63 100644 --- a/test/models/patch/builder.test.js +++ b/test/models/patch/builder.test.js @@ -983,6 +983,38 @@ describe('buildFilePatch', function() { }, ); }); + + it('does not create a DelayedPatch when the patch has been explicitly expanded', function() { + const mfp = buildMultiFilePatch([ + { + oldPath: 'big/file.txt', oldMode: '100644', newPath: 'big/file.txt', newMode: '100755', status: 'modified', + hunks: [ + { + oldStartLine: 1, oldLineCount: 3, newStartLine: 1, newLineCount: 3, + lines: [' line-0', '+line-1', '-line-2', ' line-3'], + }, + ], + }, + ], {largeDiffThreshold: 3, renderStatusOverrides: {'big/file.txt': EXPANDED}}); + + assert.lengthOf(mfp.getFilePatches(), 1); + const [fp] = mfp.getFilePatches(); + + assert.strictEqual(fp.getRenderStatus(), EXPANDED); + assert.strictEqual(fp.getOldPath(), 'big/file.txt'); + assert.strictEqual(fp.getNewPath(), 'big/file.txt'); + assert.deepEqual(fp.getMarker().getRange().serialize(), [[0, 0], [3, 6]]); + assertInFilePatch(fp, mfp.getBuffer()).hunks( + { + startRow: 0, endRow: 3, header: '@@ -1,1 +1,2 @@', regions: [ + {kind: 'unchanged', string: ' line-0\n', range: [[0, 0], [0, 6]]}, + {kind: 'addition', string: '+line-1\n', range: [[1, 0], [1, 6]]}, + {kind: 'deletion', string: '-line-2\n', range: [[2, 0], [2, 6]]}, + {kind: 'unchanged', string: ' line-3\n', range: [[3, 0], [3, 6]]}, + ], + }, + ); + }); }); it('throws an error with an unexpected number of diffs', function() { From 7db6fb04fb41d057581d3d58edf0eac7bff85884 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 16 Jan 2019 12:31:53 -0500 Subject: [PATCH 09/92] Pass builder options to getFilePatchForPath and getStagedChangesPatch --- lib/models/repository-states/present.js | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/lib/models/repository-states/present.js b/lib/models/repository-states/present.js index 830c18280e..4604efed66 100644 --- a/lib/models/repository-states/present.js +++ b/lib/models/repository-states/present.js @@ -698,16 +698,27 @@ export default class Present extends State { return {stagedFiles, unstagedFiles, mergeConflictFiles}; } - getFilePatchForPath(filePath, {staged} = {staged: false}) { - return this.cache.getOrSet(Keys.filePatch.oneWith(filePath, {staged}), async () => { - const diffs = await this.git().getDiffsForFilePath(filePath, {staged}); - return buildFilePatch(diffs); + getFilePatchForPath(filePath, options) { + const opts = { + staged: false, + builder: {}, + ...options, + }; + + return this.cache.getOrSet(Keys.filePatch.oneWith(filePath, {staged: opts.staged}), async () => { + const diffs = await this.git().getDiffsForFilePath(filePath, {staged: opts.staged}); + return buildFilePatch(diffs, opts.builder); }); } - getStagedChangesPatch() { + getStagedChangesPatch(options) { + const opts = { + builder: {}, + ...options, + }; + return this.cache.getOrSet(Keys.stagedChanges, () => { - return this.git().getStagedChangesPatch().then(buildMultiFilePatch); + return this.git().getStagedChangesPatch().then(diffs => buildMultiFilePatch(diffs, opts.builder)); }); } From 9de0eae26de12dd6aed388fac0a1ec2d986fbc86 Mon Sep 17 00:00:00 2001 From: Vanessa Yuen Date: Wed, 16 Jan 2019 18:17:36 +0100 Subject: [PATCH 10/92] some clean up --- lib/models/patch/patch.js | 4 ---- lib/views/multi-file-patch-view.js | 1 - 2 files changed, 5 deletions(-) diff --git a/lib/models/patch/patch.js b/lib/models/patch/patch.js index bfe2a69778..dd389a59c1 100644 --- a/lib/models/patch/patch.js +++ b/lib/models/patch/patch.js @@ -374,10 +374,6 @@ class NullPatch { // TODO: it prob shouldn't extend NullPatch class DelayedPatch extends NullPatch { - // should know: - // where to insert - // what the diff is - // reason it's delayed (???) constructor(position, renderStatus, parseFn) { super(); this.position = position; diff --git a/lib/views/multi-file-patch-view.js b/lib/views/multi-file-patch-view.js index bf2b7daa3e..d36af915bd 100644 --- a/lib/views/multi-file-patch-view.js +++ b/lib/views/multi-file-patch-view.js @@ -363,7 +363,6 @@ export default class MultiFilePatchView extends React.Component { } renderDiffGate(filePatch) { - // TODO: startRange is incorrect here return ( From e8f372ad3b6f5e143ea6d6a4420a8956d87a91ba Mon Sep 17 00:00:00 2001 From: Vanessa Yuen Date: Wed, 16 Jan 2019 19:02:56 +0100 Subject: [PATCH 11/92] click to show diff??? --- lib/views/multi-file-patch-view.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/views/multi-file-patch-view.js b/lib/views/multi-file-patch-view.js index d36af915bd..54956b1bcc 100644 --- a/lib/views/multi-file-patch-view.js +++ b/lib/views/multi-file-patch-view.js @@ -363,10 +363,14 @@ export default class MultiFilePatchView extends React.Component { } renderDiffGate(filePatch) { + const showDiff = () => filePatch.triggerDelayedRender(); return ( -

Diff is too large to be displayed.

+

+ Large diffs are collapsed by default for performance reasons. + Show diff anyway? +

); From dc04f498a99bb1fd9e55b2b70d6fad919e887e1d Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 16 Jan 2019 13:52:52 -0500 Subject: [PATCH 12/92] Add a renderStatus to Patch --- lib/models/patch/patch.js | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/lib/models/patch/patch.js b/lib/models/patch/patch.js index dd389a59c1..db4801bd45 100644 --- a/lib/models/patch/patch.js +++ b/lib/models/patch/patch.js @@ -18,10 +18,11 @@ export default class Patch { return new DelayedPatch(position, renderStatus, parseFn); } - constructor({status, hunks, marker}) { + constructor({status, hunks, marker, renderStatus}) { this.status = status; this.hunks = hunks; this.marker = marker; + this.renderStatus = renderStatus || EXPANDED; this.changedLineCount = this.getHunks().reduce((acc, hunk) => acc + hunk.changedLineCount(), 0); } @@ -69,9 +70,26 @@ export default class Patch { status: opts.status !== undefined ? opts.status : this.getStatus(), hunks: opts.hunks !== undefined ? opts.hunks : this.getHunks(), marker: opts.marker !== undefined ? opts.marker : this.getMarker(), + renderStatus: opts.renderStatus !== undefined ? opts.renderStatus : this.getRenderStatus(), }); } + collapsed() { + if (this.getRenderStatus() === COLLAPSED) { + return this; + } + + return this.clone({renderStatus: COLLAPSED}); + } + + expanded() { + if (this.getRenderStatus() === EXPANDED) { + return this; + } + + return this.clone({renderStatus: EXPANDED}); + } + buildStagePatchForLines(originalBuffer, nextLayeredBuffer, rowSet) { const originalBaseOffset = this.getMarker().getRange().start.row; const builder = new BufferBuilder(originalBuffer, originalBaseOffset, nextLayeredBuffer); @@ -277,7 +295,7 @@ export default class Patch { } getRenderStatus() { - return EXPANDED; + return this.renderStatus; } parseFn() { @@ -331,7 +349,8 @@ class NullPatch { if ( opts.status === undefined && opts.hunks === undefined && - opts.marker === undefined + opts.marker === undefined && + opts.renderStatus === undefined ) { return this; } else { @@ -339,6 +358,7 @@ class NullPatch { status: opts.status !== undefined ? opts.status : this.getStatus(), hunks: opts.hunks !== undefined ? opts.hunks : this.getHunks(), marker: opts.marker !== undefined ? opts.marker : this.getMarker(), + renderStatus: opts.renderStatus !== undefined ? opts.renderStatus : this.getRenderStatus(), }); } } @@ -363,7 +383,7 @@ class NullPatch { return false; } - renderStatus() { + getRenderStatus() { return EXPANDED; } From f0b8371ee74ab75b6f56a07c804543626873a323 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 16 Jan 2019 13:53:10 -0500 Subject: [PATCH 13/92] Emit events from a FilePatch when its render status changes --- lib/models/patch/file-patch.js | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/lib/models/patch/file-patch.js b/lib/models/patch/file-patch.js index 868ce03753..230928226a 100644 --- a/lib/models/patch/file-patch.js +++ b/lib/models/patch/file-patch.js @@ -1,3 +1,5 @@ +import {Emitter} from 'event-kit'; + import {nullFile} from './file'; import Patch from './patch'; import {toGitPathSep} from '../../helpers'; @@ -15,6 +17,8 @@ export default class FilePatch { this.oldFile = oldFile; this.newFile = newFile; this.patch = patch; + + this.emitter = new Emitter(); } isPresent() { @@ -116,7 +120,35 @@ export default class FilePatch { } triggerDelayedRender() { - this.patch = this.patch.parseFn(); + const nextPatch = this.patch.parseFn(); + if (nextPatch !== this.patch) { + this.patch = nextPatch; + this.didChangeRenderStatus(); + } + } + + triggerCollapse() { + const nextPatch = this.patch.collapsed(); + if (nextPatch !== this.patch) { + this.patch = nextPatch; + this.didChangeRenderStatus(); + } + } + + triggerExpand() { + const nextPatch = this.patch.expanded(); + if (nextPatch !== this.patch) { + this.patch = nextPatch; + this.didChangeRenderStatus(); + } + } + + didChangeRenderStatus() { + return this.emitter.emit('change-render-status', this); + } + + onDidChangeRenderStatus(callback) { + return this.emitter.on('change-render-status', callback); } clone(opts = {}) { From 9dad4f7942c0d27ee668e6946ec4b5605d8ba76f Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 16 Jan 2019 13:53:36 -0500 Subject: [PATCH 14/92] Unit tests for FilePatch render status events --- test/builder/patch.js | 13 ++++- test/models/patch/file-patch.test.js | 75 +++++++++++++++++++++++++++- 2 files changed, 85 insertions(+), 3 deletions(-) diff --git a/test/builder/patch.js b/test/builder/patch.js index 18ff5067b5..c2e0e830ab 100644 --- a/test/builder/patch.js +++ b/test/builder/patch.js @@ -122,6 +122,11 @@ class FilePatchBuilder { return this; } + renderStatus(...args) { + this.patchBuilder.renderStatus(...args); + return this; + } + empty() { this.patchBuilder.empty(); return this; @@ -175,6 +180,7 @@ class PatchBuilder { constructor(layeredBuffer = null) { this.layeredBuffer = layeredBuffer; + this._renderStatus = undefined; this._status = 'modified'; this.hunks = []; @@ -183,6 +189,11 @@ class PatchBuilder { this.explicitlyEmpty = false; } + renderStatus(status) { + this._renderStatus = status; + return this; + } + status(st) { if (['modified', 'added', 'deleted'].indexOf(st) === -1) { throw new Error(`Unrecognized status: ${st} (must be 'modified', 'added' or 'deleted')`); @@ -221,7 +232,7 @@ class PatchBuilder { const marker = this.layeredBuffer.markFrom('patch', this.patchStart); return this.layeredBuffer.wrapReturn({ - patch: new Patch({status: this._status, hunks: this.hunks, marker}), + patch: new Patch({status: this._status, hunks: this.hunks, marker, renderStatus: this._renderStatus}), }); } } diff --git a/test/models/patch/file-patch.test.js b/test/models/patch/file-patch.test.js index abc0b5551d..34bc44dbfb 100644 --- a/test/models/patch/file-patch.test.js +++ b/test/models/patch/file-patch.test.js @@ -1,11 +1,12 @@ -import {TextBuffer} from 'atom'; +import {TextBuffer, Point} from 'atom'; import FilePatch from '../../../lib/models/patch/file-patch'; import File, {nullFile} from '../../../lib/models/patch/file'; -import Patch from '../../../lib/models/patch/patch'; +import Patch, {TOO_LARGE, COLLAPSED, EXPANDED} from '../../../lib/models/patch/patch'; import Hunk from '../../../lib/models/patch/hunk'; import {Unchanged, Addition, Deletion, NoNewline} from '../../../lib/models/patch/region'; import {assertInFilePatch} from '../../helpers'; +import {filePatchBuilder, patchBuilder} from '../../builder/patch'; describe('FilePatch', function() { it('delegates methods to its files and patch', function() { @@ -669,6 +670,76 @@ describe('FilePatch', function() { assert.isFalse(nullFilePatch.buildUnstagePatchForLines(new Set([0])).isPresent()); assert.strictEqual(nullFilePatch.toStringIn(new TextBuffer()), ''); }); + + describe('render status changes', function() { + let sub; + + afterEach(function() { + sub && sub.dispose(); + }); + + it('announces a delayed render to subscribers', function() { + const {patch: expandedPatch} = patchBuilder().build(); + + const filePatch = FilePatch.createDelayedFilePatch( + new File({path: 'file-0.txt', mode: '100644'}), + new File({path: 'file-1.txt', mode: '100644'}), + new Point(0, 0), + TOO_LARGE, + () => expandedPatch, + ); + + const callback = sinon.spy(); + sub = filePatch.onDidChangeRenderStatus(callback); + + assert.strictEqual(TOO_LARGE, filePatch.getRenderStatus()); + assert.notStrictEqual(filePatch.getPatch(), expandedPatch); + filePatch.triggerDelayedRender(); + + assert.strictEqual(EXPANDED, filePatch.getRenderStatus()); + assert.strictEqual(filePatch.getPatch(), expandedPatch); + assert.isTrue(callback.calledWith(filePatch)); + }); + + it('announces the collapse of an expanded patch', function() { + const {filePatch} = filePatchBuilder().build(); + + const callback = sinon.spy(); + sub = filePatch.onDidChangeRenderStatus(callback); + + assert.strictEqual(EXPANDED, filePatch.getRenderStatus()); + filePatch.triggerCollapse(); + + assert.strictEqual(COLLAPSED, filePatch.getRenderStatus()); + assert.isTrue(callback.calledWith(filePatch)); + }); + + it('announces the expansion of a collapsed patch', function() { + const {filePatch} = filePatchBuilder().renderStatus(COLLAPSED).build(); + + const callback = sinon.spy(); + sub = filePatch.onDidChangeRenderStatus(callback); + + assert.strictEqual(COLLAPSED, filePatch.getRenderStatus()); + filePatch.triggerExpand(); + + assert.strictEqual(EXPANDED, filePatch.getRenderStatus()); + assert.isTrue(callback.calledWith(filePatch)); + }); + + it('does not announce non-changes', function() { + const {filePatch} = filePatchBuilder().build(); + + const callback = sinon.spy(); + sub = filePatch.onDidChangeRenderStatus(callback); + + filePatch.triggerDelayedRender(); + assert.isFalse(callback.called); + + filePatch.triggerExpand(); + assert.isFalse(callback.called); + }); + }); }); function buildLayers(buffer) { From cd5cc2899040950363f7f0018c363f75fa2d076e Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 16 Jan 2019 14:27:04 -0500 Subject: [PATCH 15/92] Remember changed renderStatus --- lib/containers/changed-file-container.js | 35 ++++++++++++++++++- .../containers/changed-file-container.test.js | 21 ++++++++++- 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/lib/containers/changed-file-container.js b/lib/containers/changed-file-container.js index fc9a83ee4c..221ac0c7fe 100644 --- a/lib/containers/changed-file-container.js +++ b/lib/containers/changed-file-container.js @@ -1,6 +1,7 @@ import React from 'react'; import PropTypes from 'prop-types'; import yubikiri from 'yubikiri'; +import {CompositeDisposable} from 'event-kit'; import {autobind} from '../helpers'; import ObserveModel from '../views/observe-model'; @@ -12,6 +13,7 @@ export default class ChangedFileContainer extends React.Component { repository: PropTypes.object.isRequired, stagingStatus: PropTypes.oneOf(['staged', 'unstaged']), relPath: PropTypes.string.isRequired, + largeDiffThreshold: PropTypes.number, workspace: PropTypes.object.isRequired, commands: PropTypes.object.isRequired, @@ -27,13 +29,26 @@ export default class ChangedFileContainer extends React.Component { constructor(props) { super(props); autobind(this, 'fetchData', 'renderWithData'); + + this.lastMultiFilePatch = null; + this.sub = new CompositeDisposable(); + + this.state = {renderStatusOverride: null}; } fetchData(repository) { const staged = this.props.stagingStatus === 'staged'; + const builder = {}; + if (this.state.renderStatusOverride !== null) { + builder.renderStatusOverrides = {[this.props.relPath]: this.state.renderStatusOverride}; + } + if (this.props.largeDiffThreshold !== undefined) { + builder.largeDiffThreshold = this.props.largeDiffThreshold; + } + return yubikiri({ - multiFilePatch: repository.getFilePatchForPath(this.props.relPath, {staged}), + multiFilePatch: repository.getFilePatchForPath(this.props.relPath, {staged, builder}), isPartiallyStaged: repository.isPartiallyStaged(this.props.relPath), hasUndoHistory: repository.hasDiscardHistory(this.props.relPath), }); @@ -48,6 +63,20 @@ export default class ChangedFileContainer extends React.Component { } renderWithData(data) { + const currentMultiFilePatch = data && data.multiFilePatch; + if (currentMultiFilePatch !== this.lastMultiFilePatch) { + this.sub.dispose(); + if (currentMultiFilePatch) { + // Keep this component's renderStatusOverride synchronized with the FilePatch we're rendering + this.sub = new CompositeDisposable( + ...currentMultiFilePatch.getFilePatches().map(fp => fp.onDidChangeRenderStatus(() => { + this.setState({renderStatusOverride: fp.getRenderStatus()}); + })), + ); + } + this.lastMultiFilePatch = currentMultiFilePatch; + } + if (this.props.repository.isLoading() || data === null) { return ; } @@ -59,4 +88,8 @@ export default class ChangedFileContainer extends React.Component { /> ); } + + componentWillUnmount() { + this.sub.dispose(); + } } diff --git a/test/containers/changed-file-container.test.js b/test/containers/changed-file-container.test.js index dd8fbf01f7..80573cbdad 100644 --- a/test/containers/changed-file-container.test.js +++ b/test/containers/changed-file-container.test.js @@ -5,6 +5,7 @@ import {mount} from 'enzyme'; import ChangedFileContainer from '../../lib/containers/changed-file-container'; import ChangedFileItem from '../../lib/items/changed-file-item'; +import {TOO_LARGE, EXPANDED} from '../../lib/models/patch/patch'; import {cloneRepository, buildRepository} from '../helpers'; describe('ChangedFileContainer', function() { @@ -17,7 +18,7 @@ describe('ChangedFileContainer', function() { repository = await buildRepository(workdirPath); // a.txt: unstaged changes - await fs.writeFile(path.join(workdirPath, 'a.txt'), 'changed\n'); + await fs.writeFile(path.join(workdirPath, 'a.txt'), '0\n1\n2\n3\n4\n5\n'); // b.txt: staged changes await fs.writeFile(path.join(workdirPath, 'b.txt'), 'changed\n'); @@ -100,4 +101,22 @@ describe('ChangedFileContainer', function() { const wrapper = mount(buildApp({relPath: 'a.txt', stagingStatus: 'unstaged', extra})); await assert.async.strictEqual(wrapper.update().find('MultiFilePatchView').prop('extra'), extra); }); + + it('remembers previously expanded large FilePatches', async function() { + const wrapper = mount(buildApp({relPath: 'a.txt', stagingStatus: 'unstaged', largeDiffThreshold: 2})); + + await assert.async.isTrue(wrapper.update().exists('ChangedFileController')); + const before = wrapper.find('ChangedFileController').prop('multiFilePatch'); + assert.strictEqual(before.getFilePatches()[0].getRenderStatus(), TOO_LARGE); + + before.getFilePatches()[0].triggerDelayedRender(); + assert.strictEqual(before.getFilePatches()[0].getRenderStatus(), EXPANDED); + + repository.refresh(); + await assert.async.notStrictEqual(wrapper.update().find('ChangedFileController').prop('multiFilePatch'), before); + + const after = wrapper.find('ChangedFileController').prop('multiFilePatch'); + assert.notStrictEqual(after, before); + assert.strictEqual(after.getFilePatches()[0].getRenderStatus(), EXPANDED); + }); }); From 7eab1197df074d1066698e543e5fa053075f8aad Mon Sep 17 00:00:00 2001 From: Tilde Ann Thurium Date: Fri, 21 Dec 2018 12:42:51 -0800 Subject: [PATCH 16/92] add collapse button to file patch header --- lib/views/file-patch-header-view.js | 15 +++++++++++++++ styles/file-patch-view.less | 11 +++++++++++ 2 files changed, 26 insertions(+) diff --git a/lib/views/file-patch-header-view.js b/lib/views/file-patch-header-view.js index 1425221120..a78b931750 100644 --- a/lib/views/file-patch-header-view.js +++ b/lib/views/file-patch-header-view.js @@ -4,6 +4,7 @@ import React, {Fragment} from 'react'; import PropTypes from 'prop-types'; import cx from 'classnames'; +import Octicon from '../atom/octicon'; import RefHolder from '../models/ref-holder'; import IssueishDetailItem from '../items/issueish-detail-item'; import ChangedFileItem from '../items/changed-file-item'; @@ -25,7 +26,10 @@ export default class FilePatchHeaderView extends React.Component { undoLastDiscard: PropTypes.func.isRequired, diveIntoMirrorPatch: PropTypes.func.isRequired, openFile: PropTypes.func.isRequired, + // should probably change 'toggleFile' to 'toggleFileStagingStatus' + // because the addition of another toggling function makes the old name confusing. toggleFile: PropTypes.func.isRequired, + toggleFileCollapse: PropTypes.func, itemType: ItemTypePropType.isRequired, }; @@ -42,12 +46,23 @@ export default class FilePatchHeaderView extends React.Component {
{this.renderTitle()} + {this.renderCollapseButton()} {this.renderButtonGroup()}
); } + renderCollapseButton() { + return ( + + ); + } + renderTitle() { if (this.props.itemType === ChangedFileItem) { const status = this.props.stagingStatus; diff --git a/styles/file-patch-view.less b/styles/file-patch-view.less index a51fd5d820..cc9af4ed6e 100644 --- a/styles/file-patch-view.less +++ b/styles/file-patch-view.less @@ -64,6 +64,17 @@ margin-right: @component-padding; } + &-collapseButton { + background: transparent; + border: none; + // dont' judge me. flexbox seemed like overkill, ok? + float: right; + } + + &-collapseButtonIcon { + position: absolute; + } + &-container { flex: 1; display: flex; From b2512645f062256ea04361935b41c77c94de12a2 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 16 Jan 2019 15:03:17 -0500 Subject: [PATCH 17/92] Use a Marker instead of a Point to track delayed patch location --- lib/models/patch/builder.js | 10 +++++++--- lib/models/patch/file-patch.js | 6 +++--- lib/models/patch/patch.js | 29 ++++++++--------------------- 3 files changed, 18 insertions(+), 27 deletions(-) diff --git a/lib/models/patch/builder.js b/lib/models/patch/builder.js index 6c75fc5b0d..75e1f4d11f 100644 --- a/lib/models/patch/builder.js +++ b/lib/models/patch/builder.js @@ -121,12 +121,16 @@ function singleDiffFilePatch(diff, layeredBuffer, opts) { EXPANDED; if (renderStatus === TOO_LARGE) { - const insertPoint = new Point(layeredBuffer.buffer.getLastRow(), 0); + const insertRow = layeredBuffer.buffer.getLastRow(); + const patchMarker = layeredBuffer.layers.patch.markPosition( + [insertRow, 0], + {invalidate: 'never', exclusive: false}, + ); return FilePatch.createDelayedFilePatch( - oldFile, newFile, insertPoint, TOO_LARGE, + oldFile, newFile, patchMarker, TOO_LARGE, () => { - const [hunks, patchMarker] = buildHunks(diff, layeredBuffer, insertPoint.row); + const [hunks] = buildHunks(diff, layeredBuffer, insertRow, patchMarker); return new Patch({status: diff.status, hunks, marker: patchMarker, buffer: layeredBuffer.buffer}); }, ); diff --git a/lib/models/patch/file-patch.js b/lib/models/patch/file-patch.js index 230928226a..362f630ae2 100644 --- a/lib/models/patch/file-patch.js +++ b/lib/models/patch/file-patch.js @@ -9,8 +9,8 @@ export default class FilePatch { return new this(nullFile, nullFile, Patch.createNull()); } - static createDelayedFilePatch(oldFile, newFile, position, renderStatus, parseFn) { - return new this(oldFile, newFile, Patch.createDelayedPatch(position, renderStatus, parseFn)); + static createDelayedFilePatch(oldFile, newFile, marker, renderStatus, parseFn) { + return new this(oldFile, newFile, Patch.createDelayedPatch(marker, renderStatus, parseFn)); } constructor(oldFile, newFile, patch) { @@ -120,7 +120,7 @@ export default class FilePatch { } triggerDelayedRender() { - const nextPatch = this.patch.parseFn(); + const nextPatch = this.patch.parseFn(this.patch); if (nextPatch !== this.patch) { this.patch = nextPatch; this.didChangeRenderStatus(); diff --git a/lib/models/patch/patch.js b/lib/models/patch/patch.js index db4801bd45..99741312ca 100644 --- a/lib/models/patch/patch.js +++ b/lib/models/patch/patch.js @@ -14,17 +14,22 @@ export default class Patch { return new NullPatch(); } - static createDelayedPatch(position, renderStatus, parseFn) { - return new DelayedPatch(position, renderStatus, parseFn); + static createDelayedPatch(marker, renderStatus, parseFn) { + return new this({status: null, hunks: [], marker, renderStatus, parseFn}); } - constructor({status, hunks, marker, renderStatus}) { + constructor({status, hunks, marker, renderStatus, parseFn}) { this.status = status; this.hunks = hunks; this.marker = marker; this.renderStatus = renderStatus || EXPANDED; this.changedLineCount = this.getHunks().reduce((acc, hunk) => acc + hunk.changedLineCount(), 0); + + if (parseFn) { + // Override the prototype's method + this.parseFn = parseFn; + } } getStatus() { @@ -392,24 +397,6 @@ class NullPatch { } } -// TODO: it prob shouldn't extend NullPatch -class DelayedPatch extends NullPatch { - constructor(position, renderStatus, parseFn) { - super(); - this.position = position; - this.renderStatus = renderStatus; - this.parseFn = parseFn; - } - - getStartRange() { - return new Range(this.position, this.position); - } - - getRenderStatus() { - return this.renderStatus; - } -} - class BufferBuilder { constructor(original, originalBaseOffset, nextLayeredBuffer) { this.originalBuffer = original; From 0bfacb6bbf77398e4ef71ccc73531f4644e1aa1a Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 16 Jan 2019 15:25:34 -0500 Subject: [PATCH 18/92] Track changed renderStatus in CommitPreviewContainer --- lib/containers/commit-preview-container.js | 39 ++++++++++++++++++- .../commit-preview-container.test.js | 30 ++++++++++++++ 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/lib/containers/commit-preview-container.js b/lib/containers/commit-preview-container.js index ef77f354fe..9b6cf49240 100644 --- a/lib/containers/commit-preview-container.js +++ b/lib/containers/commit-preview-container.js @@ -1,6 +1,7 @@ import React from 'react'; import PropTypes from 'prop-types'; import yubikiri from 'yubikiri'; +import {CompositeDisposable} from 'event-kit'; import ObserveModel from '../views/observe-model'; import LoadingView from '../views/loading-view'; @@ -9,11 +10,27 @@ import CommitPreviewController from '../controllers/commit-preview-controller'; export default class CommitPreviewContainer extends React.Component { static propTypes = { repository: PropTypes.object.isRequired, + largeDiffThreshold: PropTypes.number, + } + + constructor(props) { + super(props); + + this.lastMultiFilePatch = null; + this.sub = new CompositeDisposable(); + + this.state = {renderStatusOverrides: {}}; } fetchData = repository => { + const builder = {renderStatusOverrides: this.state.renderStatusOverrides}; + + if (this.props.largeDiffThreshold !== undefined) { + builder.largeDiffThreshold = this.props.largeDiffThreshold; + } + return yubikiri({ - multiFilePatch: repository.getStagedChangesPatch(), + multiFilePatch: repository.getStagedChangesPatch({builder}), }); } @@ -26,6 +43,26 @@ export default class CommitPreviewContainer extends React.Component { } renderResult = data => { + const currentMultiFilePatch = data && data.multiFilePatch; + if (currentMultiFilePatch !== this.lastMultiFilePatch) { + this.sub.dispose(); + if (currentMultiFilePatch) { + this.sub = new CompositeDisposable( + ...currentMultiFilePatch.getFilePatches().map(fp => fp.onDidChangeRenderStatus(() => { + this.setState(prevState => { + return { + renderStatusOverrides: { + ...prevState.renderStatusOverrides, + [fp.getPath()]: fp.getRenderStatus(), + }, + }; + }); + })), + ); + } + this.lastMultiFilePatch = currentMultiFilePatch; + } + if (this.props.repository.isLoading() || data === null) { return ; } diff --git a/test/containers/commit-preview-container.test.js b/test/containers/commit-preview-container.test.js index c11a5a9164..c24002d0ae 100644 --- a/test/containers/commit-preview-container.test.js +++ b/test/containers/commit-preview-container.test.js @@ -1,8 +1,11 @@ import React from 'react'; import {mount} from 'enzyme'; +import fs from 'fs-extra'; +import path from 'path'; import CommitPreviewContainer from '../../lib/containers/commit-preview-container'; import CommitPreviewItem from '../../lib/items/commit-preview-item'; +import {TOO_LARGE, EXPANDED} from '../../lib/models/patch/patch'; import {cloneRepository, buildRepository} from '../helpers'; describe('CommitPreviewContainer', function() { @@ -71,4 +74,31 @@ describe('CommitPreviewContainer', function() { await assert.async.isTrue(wrapper.update().find('CommitPreviewController').exists()); assert.strictEqual(wrapper.find('CommitPreviewController').prop('multiFilePatch'), patch); }); + + it('remembers previously expanded large patches', async function() { + const wd = repository.getWorkingDirectoryPath(); + await repository.getLoadPromise(); + + await fs.writeFile(path.join(wd, 'file-0.txt'), '0\n1\n2\n3\n4\n5\n', {encoding: 'utf8'}); + await fs.writeFile(path.join(wd, 'file-1.txt'), '0\n1\n2\n', {encoding: 'utf8'}); + await repository.stageFiles(['file-0.txt', 'file-1.txt']); + + repository.refresh(); + + const wrapper = mount(buildApp({largeDiffThreshold: 3})); + await assert.async.isTrue(wrapper.update().exists('CommitPreviewController')); + + const before = wrapper.find('CommitPreviewController').prop('multiFilePatch'); + assert.strictEqual(before.getFilePatches()[0].getRenderStatus(), TOO_LARGE); + assert.strictEqual(before.getFilePatches()[1].getRenderStatus(), EXPANDED); + + before.getFilePatches()[0].triggerDelayedRender(); + repository.refresh(); + + await assert.async.notStrictEqual(wrapper.update().find('CommitPreviewController').prop('multiFilePatch'), before); + const after = wrapper.find('CommitPreviewController').prop('multiFilePatch'); + + assert.strictEqual(after.getFilePatches()[0].getRenderStatus(), EXPANDED); + assert.strictEqual(after.getFilePatches()[1].getRenderStatus(), EXPANDED); + }); }); From 40cde30b343b0e8b490ba4ce4e42447f06f68ebc Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 16 Jan 2019 15:40:21 -0500 Subject: [PATCH 19/92] Ditto for CommitDetailContainer --- lib/containers/commit-detail-container.js | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/lib/containers/commit-detail-container.js b/lib/containers/commit-detail-container.js index c624d127f7..eedf3370be 100644 --- a/lib/containers/commit-detail-container.js +++ b/lib/containers/commit-detail-container.js @@ -1,6 +1,7 @@ import React from 'react'; import PropTypes from 'prop-types'; import yubikiri from 'yubikiri'; +import {CompositeDisposable} from 'event-kit'; import ObserveModel from '../views/observe-model'; import LoadingView from '../views/loading-view'; @@ -13,6 +14,13 @@ export default class CommitDetailContainer extends React.Component { itemType: PropTypes.func.isRequired, } + constructor(props) { + super(props); + + this.lastCommit = null; + this.sub = new CompositeDisposable(); + } + fetchData = repository => { return yubikiri({ commit: repository.getCommit(this.props.sha), @@ -31,6 +39,19 @@ export default class CommitDetailContainer extends React.Component { } renderResult = data => { + const currentCommit = data && data.commit; + if (currentCommit !== this.lastCommit) { + this.sub.dispose(); + if (currentCommit && currentCommit.isPresent()) { + this.sub = new CompositeDisposable( + ...currentCommit.getMultiFileDiff().getFilePatches().map(fp => fp.onDidChangeRenderStatus(() => { + this.forceUpdate(); + })), + ); + } + this.lastCommit = currentCommit; + } + if (this.props.repository.isLoading() || data === null || !data.commit.isPresent()) { return ; } From be2ab8f5deb6d9ca7d78965ab7bafda69ad51776 Mon Sep 17 00:00:00 2001 From: annthurium Date: Wed, 16 Jan 2019 13:50:05 -0800 Subject: [PATCH 20/92] fix bad parens in mfp builder --- lib/models/patch/builder.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/models/patch/builder.js b/lib/models/patch/builder.js index 75e1f4d11f..45bbcdc9c2 100644 --- a/lib/models/patch/builder.js +++ b/lib/models/patch/builder.js @@ -178,7 +178,7 @@ function dualDiffFilePatch(diff1, diff2, layeredBuffer, opts) { const oldFile = new File({path: filePath, mode: oldMode, symlink: oldSymlink}); const newFile = new File({path: filePath, mode: newMode, symlink: newSymlink}); - if (isDiffLarge([diff1, diff2]), opts) { + if (isDiffLarge([diff1, diff2], opts)) { // TODO: do something with large diff too } From e28522ededfa992f89fab4032f9963f0469eff19 Mon Sep 17 00:00:00 2001 From: annthurium Date: Wed, 16 Jan 2019 15:10:17 -0800 Subject: [PATCH 21/92] don't render pull request comments if patch exceeds large diff threshold --- lib/models/patch/multi-file-patch.js | 15 +++++++++++++++ lib/views/multi-file-patch-view.js | 1 + lib/views/pr-review-comments-view.js | 3 +++ 3 files changed, 19 insertions(+) diff --git a/lib/models/patch/multi-file-patch.js b/lib/models/patch/multi-file-patch.js index e21ae6158f..bf88e1fa32 100644 --- a/lib/models/patch/multi-file-patch.js +++ b/lib/models/patch/multi-file-patch.js @@ -1,6 +1,8 @@ import {TextBuffer, Range} from 'atom'; import {RBTree} from 'bintrees'; +import {TOO_LARGE} from './patch'; + export default class MultiFilePatch { constructor({buffer, layers, filePatches}) { this.buffer = buffer || null; @@ -339,6 +341,19 @@ export default class MultiFilePatch { return false; } + doesPatchExceedLargeDiffThreshold = filePatchPath => { + + const patches = this.getFilePatches(); + // can avoid iterating through all patches by adding a map + const patch = patches.filter(p => p.getPath() === filePatchPath)[0]; + // do I need to handle case where patch is null or something? + if (patch.getRenderStatus() === TOO_LARGE) { + return true; + } else { + return false; + } + } + getBufferRowForDiffPosition = (fileName, diffRow) => { const {startBufferRow, index} = this.diffRowOffsetIndices.get(fileName); const {offset} = index.lowerBound({diffRow}).data(); diff --git a/lib/views/multi-file-patch-view.js b/lib/views/multi-file-patch-view.js index 54956b1bcc..05bcd3635f 100644 --- a/lib/views/multi-file-patch-view.js +++ b/lib/views/multi-file-patch-view.js @@ -321,6 +321,7 @@ export default class MultiFilePatchView extends React.Component { ); } else { diff --git a/lib/views/pr-review-comments-view.js b/lib/views/pr-review-comments-view.js index 80e89af272..af5feaece9 100644 --- a/lib/views/pr-review-comments-view.js +++ b/lib/views/pr-review-comments-view.js @@ -33,6 +33,9 @@ export default class PullRequestCommentsView extends React.Component { } const nativePath = toNativePathSep(rootComment.path); + if (this.props.doesPatchExceedLargeDiffThreshold(rootComment.path)) { + return null; + } const row = this.props.getBufferRowForDiffPosition(nativePath, rootComment.position); const point = new Point(row, 0); const range = new Range(point, point); From b66c88c3c5a33be85a2c97aab143d540dfc0c814 Mon Sep 17 00:00:00 2001 From: annthurium Date: Wed, 16 Jan 2019 15:16:47 -0800 Subject: [PATCH 22/92] use a Map to avoid iterating through patches --- lib/models/patch/multi-file-patch.js | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/lib/models/patch/multi-file-patch.js b/lib/models/patch/multi-file-patch.js index bf88e1fa32..193b4cd803 100644 --- a/lib/models/patch/multi-file-patch.js +++ b/lib/models/patch/multi-file-patch.js @@ -17,6 +17,7 @@ export default class MultiFilePatch { this.filePatches = filePatches || []; this.filePatchesByMarker = new Map(); + this.filePatchesByPath = new Map(); this.hunksByMarker = new Map(); // Store a map of {diffRow, offset} for each FilePatch where offset is the number of Hunk headers within the current @@ -24,6 +25,7 @@ export default class MultiFilePatch { this.diffRowOffsetIndices = new Map(); for (const filePatch of this.filePatches) { + this.filePatchesByPath.set(filePatch.getPath(), filePatch); this.filePatchesByMarker.set(filePatch.getMarker(), filePatch); let diffRow = 1; @@ -342,16 +344,9 @@ export default class MultiFilePatch { } doesPatchExceedLargeDiffThreshold = filePatchPath => { - - const patches = this.getFilePatches(); - // can avoid iterating through all patches by adding a map - const patch = patches.filter(p => p.getPath() === filePatchPath)[0]; + const patch = this.filePatchesByPath.get(filePatchPath); + return patch.getRenderStatus() === TOO_LARGE; // do I need to handle case where patch is null or something? - if (patch.getRenderStatus() === TOO_LARGE) { - return true; - } else { - return false; - } } getBufferRowForDiffPosition = (fileName, diffRow) => { From 98fbfc910e76cccc6359558c5370bfe28566686d Mon Sep 17 00:00:00 2001 From: annthurium Date: Wed, 16 Jan 2019 15:22:07 -0800 Subject: [PATCH 23/92] deal with null patches ...not sure how we would get into this situation, but it seems like an important case to cover. --- lib/models/patch/multi-file-patch.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/models/patch/multi-file-patch.js b/lib/models/patch/multi-file-patch.js index 193b4cd803..be4f516e1b 100644 --- a/lib/models/patch/multi-file-patch.js +++ b/lib/models/patch/multi-file-patch.js @@ -345,8 +345,10 @@ export default class MultiFilePatch { doesPatchExceedLargeDiffThreshold = filePatchPath => { const patch = this.filePatchesByPath.get(filePatchPath); + if (!patch) { + return null; + } return patch.getRenderStatus() === TOO_LARGE; - // do I need to handle case where patch is null or something? } getBufferRowForDiffPosition = (fileName, diffRow) => { From ccf2a99fc04e917f0a112f8a56fc96e12f2c00c5 Mon Sep 17 00:00:00 2001 From: annthurium Date: Wed, 16 Jan 2019 15:26:08 -0800 Subject: [PATCH 24/92] :shirt: --- lib/models/patch/builder.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/models/patch/builder.js b/lib/models/patch/builder.js index 45bbcdc9c2..29fc3f6156 100644 --- a/lib/models/patch/builder.js +++ b/lib/models/patch/builder.js @@ -1,4 +1,4 @@ -import {TextBuffer, Point} from 'atom'; +import {TextBuffer} from 'atom'; import Hunk from './hunk'; import File, {nullFile} from './file'; From 26ee209b4a37d03d1f56a57d136751fbb56dc2da Mon Sep 17 00:00:00 2001 From: annthurium Date: Wed, 16 Jan 2019 15:27:36 -0800 Subject: [PATCH 25/92] danananana PROP TYPES --- lib/controllers/pr-reviews-controller.js | 1 + lib/views/pr-review-comments-view.js | 1 + test/controllers/pr-reviews-controller.test.js | 1 + 3 files changed, 3 insertions(+) diff --git a/lib/controllers/pr-reviews-controller.js b/lib/controllers/pr-reviews-controller.js index d10839eb69..9c72522333 100644 --- a/lib/controllers/pr-reviews-controller.js +++ b/lib/controllers/pr-reviews-controller.js @@ -19,6 +19,7 @@ export default class PullRequestReviewsController extends React.Component { ), }), getBufferRowForDiffPosition: PropTypes.func.isRequired, + doesPatchExceedLargeDiffThreshold: PropTypes.func.isRequired, } constructor(props) { diff --git a/lib/views/pr-review-comments-view.js b/lib/views/pr-review-comments-view.js index af5feaece9..9268728284 100644 --- a/lib/views/pr-review-comments-view.js +++ b/lib/views/pr-review-comments-view.js @@ -23,6 +23,7 @@ export default class PullRequestCommentsView extends React.Component { })), getBufferRowForDiffPosition: PropTypes.func.isRequired, switchToIssueish: PropTypes.func.isRequired, + doesPatchExceedLargeDiffThreshold: PropTypes.func.isRequired, } render() { diff --git a/test/controllers/pr-reviews-controller.test.js b/test/controllers/pr-reviews-controller.test.js index 83cfa8eadd..b6080d5b3c 100644 --- a/test/controllers/pr-reviews-controller.test.js +++ b/test/controllers/pr-reviews-controller.test.js @@ -43,6 +43,7 @@ describe('PullRequestReviewsController', function() { switchToIssueish: () => {}, getBufferRowForDiffPosition: () => {}, + doesPatchExceedLargeDiffThreshold: () => {}, pullRequest: {reviews}, ...overrideProps, }; From 010cd56374443aeb7b95199af9c4e04b45a09e5c Mon Sep 17 00:00:00 2001 From: annthurium Date: Wed, 16 Jan 2019 16:31:34 -0800 Subject: [PATCH 26/92] also check for COLLAPSED patch state --- lib/controllers/pr-reviews-controller.js | 2 +- lib/models/patch/multi-file-patch.js | 7 ++++--- lib/views/multi-file-patch-view.js | 2 +- lib/views/pr-review-comments-view.js | 7 ++++--- test/controllers/pr-reviews-controller.test.js | 2 +- 5 files changed, 11 insertions(+), 9 deletions(-) diff --git a/lib/controllers/pr-reviews-controller.js b/lib/controllers/pr-reviews-controller.js index 9c72522333..f2a836fc32 100644 --- a/lib/controllers/pr-reviews-controller.js +++ b/lib/controllers/pr-reviews-controller.js @@ -19,7 +19,7 @@ export default class PullRequestReviewsController extends React.Component { ), }), getBufferRowForDiffPosition: PropTypes.func.isRequired, - doesPatchExceedLargeDiffThreshold: PropTypes.func.isRequired, + isPatchTooLargeOrCollapsed: PropTypes.func.isRequired, } constructor(props) { diff --git a/lib/models/patch/multi-file-patch.js b/lib/models/patch/multi-file-patch.js index be4f516e1b..0490c5fd12 100644 --- a/lib/models/patch/multi-file-patch.js +++ b/lib/models/patch/multi-file-patch.js @@ -1,7 +1,7 @@ import {TextBuffer, Range} from 'atom'; import {RBTree} from 'bintrees'; -import {TOO_LARGE} from './patch'; +import {TOO_LARGE, COLLAPSED} from './patch'; export default class MultiFilePatch { constructor({buffer, layers, filePatches}) { @@ -343,12 +343,13 @@ export default class MultiFilePatch { return false; } - doesPatchExceedLargeDiffThreshold = filePatchPath => { + isPatchTooLargeOrCollapsed = filePatchPath => { const patch = this.filePatchesByPath.get(filePatchPath); if (!patch) { return null; } - return patch.getRenderStatus() === TOO_LARGE; + const renderStatus = patch.getRenderStatus(); + return renderStatus === TOO_LARGE || renderStatus === COLLAPSED; } getBufferRowForDiffPosition = (fileName, diffRow) => { diff --git a/lib/views/multi-file-patch-view.js b/lib/views/multi-file-patch-view.js index 05bcd3635f..5554ac910c 100644 --- a/lib/views/multi-file-patch-view.js +++ b/lib/views/multi-file-patch-view.js @@ -321,7 +321,7 @@ export default class MultiFilePatchView extends React.Component { ); } else { diff --git a/lib/views/pr-review-comments-view.js b/lib/views/pr-review-comments-view.js index 9268728284..ca597f066e 100644 --- a/lib/views/pr-review-comments-view.js +++ b/lib/views/pr-review-comments-view.js @@ -23,7 +23,7 @@ export default class PullRequestCommentsView extends React.Component { })), getBufferRowForDiffPosition: PropTypes.func.isRequired, switchToIssueish: PropTypes.func.isRequired, - doesPatchExceedLargeDiffThreshold: PropTypes.func.isRequired, + isPatchTooLargeOrCollapsed: PropTypes.func.isRequired, } render() { @@ -33,10 +33,11 @@ export default class PullRequestCommentsView extends React.Component { return null; } - const nativePath = toNativePathSep(rootComment.path); - if (this.props.doesPatchExceedLargeDiffThreshold(rootComment.path)) { + // if file patch is collapsed or too large, do not render the comments + if (this.props.isPatchTooLargeOrCollapsed(rootComment.path)) { return null; } + const nativePath = toNativePathSep(rootComment.path); const row = this.props.getBufferRowForDiffPosition(nativePath, rootComment.position); const point = new Point(row, 0); const range = new Range(point, point); diff --git a/test/controllers/pr-reviews-controller.test.js b/test/controllers/pr-reviews-controller.test.js index b6080d5b3c..fcd44eacc8 100644 --- a/test/controllers/pr-reviews-controller.test.js +++ b/test/controllers/pr-reviews-controller.test.js @@ -43,7 +43,7 @@ describe('PullRequestReviewsController', function() { switchToIssueish: () => {}, getBufferRowForDiffPosition: () => {}, - doesPatchExceedLargeDiffThreshold: () => {}, + isPatchTooLargeOrCollapsed: () => {}, pullRequest: {reviews}, ...overrideProps, }; From cd2b7f982c119abf53066e67621404915f086b36 Mon Sep 17 00:00:00 2001 From: annthurium Date: Wed, 16 Jan 2019 16:56:09 -0800 Subject: [PATCH 27/92] honk if you like test helpers --- lib/views/pr-review-comments-view.js | 1 + test/views/pr-comments-view.test.js | 21 +++++++++++++++++++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/lib/views/pr-review-comments-view.js b/lib/views/pr-review-comments-view.js index ca597f066e..f31a9fe36b 100644 --- a/lib/views/pr-review-comments-view.js +++ b/lib/views/pr-review-comments-view.js @@ -11,6 +11,7 @@ import GithubDotcomMarkdown from './github-dotcom-markdown'; import Timeago from './timeago'; export default class PullRequestCommentsView extends React.Component { + // do we even need the relay props here? does not look like they are used static propTypes = { relay: PropTypes.shape({ hasMore: PropTypes.func.isRequired, diff --git a/test/views/pr-comments-view.test.js b/test/views/pr-comments-view.test.js index 15e2562626..7e32b755da 100644 --- a/test/views/pr-comments-view.test.js +++ b/test/views/pr-comments-view.test.js @@ -6,6 +6,23 @@ import {pullRequestBuilder} from '../builder/pr'; import PullRequestCommentsView, {PullRequestCommentView} from '../../lib/views/pr-review-comments-view'; describe('PullRequestCommentsView', function() { + function buildApp(multiFilePatch, pullRequest, overrideProps) { + const relay = { + hasMore: () => {}, + loadMore: () => {}, + isLoading: () => {}, + }; + return shallow( + {}} + {...overrideProps} + />, + ); + } it('adjusts the position for comments after hunk headers', function() { const {multiFilePatch} = multiFilePatchBuilder() .addFilePatch(fp => { @@ -31,7 +48,7 @@ describe('PullRequestCommentsView', function() { }) .build(); - const wrapper = shallow(); + const wrapper = buildApp(multiFilePatch, pr, {}); assert.deepEqual(wrapper.find('Marker').at(0).prop('bufferRange').serialize(), [[1, 0], [1, 0]]); assert.deepEqual(wrapper.find('Marker').at(1).prop('bufferRange').serialize(), [[12, 0], [12, 0]]); @@ -57,7 +74,7 @@ describe('PullRequestCommentsView', function() { }) .build(); - const wrapper = shallow(); + const wrapper = buildApp(multiFilePatch, pr, {}); const comments = wrapper.find('PullRequestCommentView'); assert.lengthOf(comments, 1); From 8324cb3080cd684a11bb1a7b23626c1380f408ab Mon Sep 17 00:00:00 2001 From: annthurium Date: Wed, 16 Jan 2019 17:00:15 -0800 Subject: [PATCH 28/92] clean up pr comments builder unnecessary console output during tests is a bummer. --- test/builder/pr.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/test/builder/pr.js b/test/builder/pr.js index 3616872b77..52c72a95dc 100644 --- a/test/builder/pr.js +++ b/test/builder/pr.js @@ -6,9 +6,10 @@ class CommentBuilder { this._authorLogin = 'someone'; this._authorAvatarUrl = 'https://avatars3.githubusercontent.com/u/17565?s=32&v=4'; this._url = 'https://github.com/atom/github/pull/1829/files#r242224689'; - this._createdAt = 0; + this._createdAt = '2018-12-27T17:51:17Z'; this._body = 'Lorem ipsum dolor sit amet, te urbanitas appellantur est.'; this._replyTo = null; + this._isMinimized = false; } id(i) { @@ -16,6 +17,11 @@ class CommentBuilder { return this; } + minmized(m) { + this._isMinimized = m; + return this; + } + path(p) { this._path = p; return this; @@ -69,6 +75,7 @@ class CommentBuilder { createdAt: this._createdAt, url: this._url, replyTo: this._replyTo, + isMinimized: this._isMinimized, }; } } From ee6c9c22f2204df05cdb988ed64d99d79649a5fa Mon Sep 17 00:00:00 2001 From: annthurium Date: Wed, 16 Jan 2019 17:10:30 -0800 Subject: [PATCH 29/92] [wip] add tests for `isPatchTooLargeOrCollapsed` --- test/models/patch/multi-file-patch.test.js | 38 ++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/test/models/patch/multi-file-patch.test.js b/test/models/patch/multi-file-patch.test.js index cad02eaffa..61fa236f7a 100644 --- a/test/models/patch/multi-file-patch.test.js +++ b/test/models/patch/multi-file-patch.test.js @@ -2,9 +2,12 @@ import dedent from 'dedent-js'; import {multiFilePatchBuilder, filePatchBuilder} from '../../builder/patch'; +import {TOO_LARGE} from '../../../lib/models/patch/patch'; import MultiFilePatch from '../../../lib/models/patch/multi-file-patch'; + import {assertInFilePatch} from '../../helpers'; + describe('MultiFilePatch', function() { it('creates an empty patch when constructed with no arguments', function() { const empty = new MultiFilePatch({}); @@ -704,6 +707,41 @@ describe('MultiFilePatch', function() { }); }); + describe('isPatchTooLargeOrCollapsed', function() { + let multiFilePatch; + beforeEach(function() { + multiFilePatch = multiFilePatchBuilder() + .addFilePatch(fp => { + fp.setOldFile(f => f.path('file-0')); + // do we even give a shit about the hunks here? + fp.addHunk(h => h.unchanged('0').added('1').deleted('2', '3').unchanged('4')); + fp.addHunk(h => h.unchanged('5').added('6').unchanged('7')); + }) + .addFilePatch(fp => { + fp.setOldFile(f => f.path('file-1')); + fp.addHunk(h => h.unchanged('8').deleted('9', '10').unchanged('11')); + }) + .build() + .multiFilePatch; + }); + it('returns true if patch exceeds large diff threshold', function() { + // todo: what is the best way to use the patch builder to build at patch that + // exceeds the large diff threshold? + }); + + it('returns true if patch is collapsed', function() { + }); + + it('returns false if patch does not exceed large diff threshold and is not collapsed', function() { + assert.isFalse(multiFilePatch.isPatchTooLargeOrCollapsed('file-1')); + }); + + it('returns null if patch does not exist', function() { + assert.isNull(multiFilePatch.isPatchTooLargeOrCollapsed('invalid-file-path')); + }); + }); + + describe('diff position translation', function() { it('offsets rows in the first hunk by the first hunk header', function() { const {multiFilePatch} = multiFilePatchBuilder() From 07336769f5acd64b799c1cb05da33906253cd1fa Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 17 Jan 2019 08:35:54 -0500 Subject: [PATCH 30/92] Test case for Patch Marker movement --- test/models/patch/builder.test.js | 44 +++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/test/models/patch/builder.test.js b/test/models/patch/builder.test.js index 59951c2f63..d917d8af36 100644 --- a/test/models/patch/builder.test.js +++ b/test/models/patch/builder.test.js @@ -984,6 +984,50 @@ describe('buildFilePatch', function() { ); }); + it('preserves patch markers when a delayed patch is expanded', function() { + const mfp = buildMultiFilePatch([ + { + oldPath: 'first', oldMode: '100644', newPath: 'first', newMode: '100644', status: 'modified', + hunks: [ + { + oldStartLine: 1, oldLineCount: 3, newStartLine: 1, newLineCount: 3, + lines: [' line-0', '+line-1', '-line-2', ' line-3'], + }, + ], + }, + { + oldPath: 'second', oldMode: '100644', newPath: 'second', newMode: '100644', status: 'modified', + hunks: [ + { + oldStartLine: 1, oldLineCount: 3, newStartLine: 1, newLineCount: 3, + lines: [' line-0', '+line-1', '-line-2', ' line-3'], + }, + ], + }, + { + oldPath: 'third', oldMode: '100644', newPath: 'third', newMode: '100644', status: 'modified', + hunks: [ + { + oldStartLine: 1, oldLineCount: 3, newStartLine: 1, newLineCount: 3, + lines: [' line-0', '+line-1', '-line-2', ' line-3'], + }, + ], + }, + ], {largeDiffThreshold: 3}); + + assert.lengthOf(mfp.getFilePatches(), 3); + const [fp0, fp1, fp2] = mfp.getFilePatches(); + assert.deepEqual(fp0.getMarker().getRange().serialize(), [[0, 0], [0, 0]]); + assert.deepEqual(fp1.getMarker().getRange().serialize(), [[0, 0], [0, 0]]); + assert.deepEqual(fp2.getMarker().getRange().serialize(), [[0, 0], [0, 0]]); + + fp1.triggerDelayedRender(); + + assert.deepEqual(fp0.getMarker().getRange().serialize(), [[0, 0], [0, 0]]); + assert.deepEqual(fp1.getMarker().getRange().serialize(), [[0, 0], [3, 6]]); + assert.deepEqual(fp2.getMarker().getRange().serialize(), [[4, 0], [4, 0]]); + }); + it('does not create a DelayedPatch when the patch has been explicitly expanded', function() { const mfp = buildMultiFilePatch([ { From 1237458dec35cc4ee510bf3933a65374c9e981bf Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 17 Jan 2019 10:23:21 -0500 Subject: [PATCH 31/92] Extract LayeredBuffer class and use it consistently in Patch logic --- lib/models/patch/builder.js | 84 ++++++++------------ lib/models/patch/hunk.js | 2 + lib/models/patch/layered-buffer.js | 43 ++++++++++ lib/models/patch/multi-file-patch.js | 112 +++++++++------------------ lib/models/patch/patch.js | 76 +++++++++--------- lib/models/patch/region.js | 8 ++ test/builder/patch.js | 85 ++++++++------------ test/models/patch/builder.test.js | 4 +- 8 files changed, 193 insertions(+), 221 deletions(-) create mode 100644 lib/models/patch/layered-buffer.js diff --git a/lib/models/patch/builder.js b/lib/models/patch/builder.js index 29fc3f6156..6a308b2f7d 100644 --- a/lib/models/patch/builder.js +++ b/lib/models/patch/builder.js @@ -1,5 +1,4 @@ -import {TextBuffer} from 'atom'; - +import LayeredBuffer from './layered-buffer'; import Hunk from './hunk'; import File, {nullFile} from './file'; import Patch, {TOO_LARGE, EXPANDED} from './patch'; @@ -10,7 +9,7 @@ import MultiFilePatch from './multi-file-patch'; const DEFAULT_OPTIONS = { // Number of lines after which we consider the diff "large" // TODO: Set this based on performance measurements - largeDiffThreshold: 10, + largeDiffThreshold: 100, // Map of file path (relative to repository root) to Patch render status (EXPANDED, COLLAPSED, TOO_LARGE) renderStatusOverrides: {}, @@ -19,7 +18,7 @@ const DEFAULT_OPTIONS = { export function buildFilePatch(diffs, options) { const opts = {...DEFAULT_OPTIONS, ...options}; - const layeredBuffer = initializeBuffer(); + const layeredBuffer = new LayeredBuffer(); let filePatch; if (diffs.length === 0) { @@ -32,13 +31,13 @@ export function buildFilePatch(diffs, options) { throw new Error(`Unexpected number of diffs: ${diffs.length}`); } - return new MultiFilePatch({filePatches: [filePatch], ...layeredBuffer}); + return new MultiFilePatch({layeredBuffer, filePatches: [filePatch]}); } export function buildMultiFilePatch(diffs, options) { const opts = {...DEFAULT_OPTIONS, ...options}; - const layeredBuffer = initializeBuffer(); + const layeredBuffer = new LayeredBuffer(); const byPath = new Map(); const actions = []; @@ -82,7 +81,7 @@ export function buildMultiFilePatch(diffs, options) { } }); - return new MultiFilePatch({filePatches, ...layeredBuffer}); + return new MultiFilePatch({layeredBuffer, filePatches}); } function emptyDiffFilePatch() { @@ -121,8 +120,9 @@ function singleDiffFilePatch(diff, layeredBuffer, opts) { EXPANDED; if (renderStatus === TOO_LARGE) { - const insertRow = layeredBuffer.buffer.getLastRow(); - const patchMarker = layeredBuffer.layers.patch.markPosition( + const insertRow = layeredBuffer.getBuffer().getLastRow(); + const patchMarker = layeredBuffer.markPosition( + Patch.layerName, [insertRow, 0], {invalidate: 'never', exclusive: false}, ); @@ -131,12 +131,12 @@ function singleDiffFilePatch(diff, layeredBuffer, opts) { oldFile, newFile, patchMarker, TOO_LARGE, () => { const [hunks] = buildHunks(diff, layeredBuffer, insertRow, patchMarker); - return new Patch({status: diff.status, hunks, marker: patchMarker, buffer: layeredBuffer.buffer}); + return new Patch({status: diff.status, hunks, marker: patchMarker}); }, ); } else { - const [hunks, patchMarker] = buildHunks(diff, layeredBuffer, layeredBuffer.buffer.getLastRow()); - const patch = new Patch({status: diff.status, hunks, marker: patchMarker, buffer: layeredBuffer.buffer}); + const [hunks, patchMarker] = buildHunks(diff, layeredBuffer, layeredBuffer.getBuffer().getLastRow()); + const patch = new Patch({status: diff.status, hunks, marker: patchMarker}); return new FilePatch(oldFile, newFile, patch); } @@ -182,8 +182,8 @@ function dualDiffFilePatch(diff1, diff2, layeredBuffer, opts) { // TODO: do something with large diff too } - const [hunks, patchMarker] = buildHunks(contentChangeDiff, layeredBuffer, layeredBuffer.buffer.getLastRow()); - const patch = new Patch({status, hunks, marker: patchMarker, buffer: layeredBuffer.buffer}); + const [hunks, patchMarker] = buildHunks(contentChangeDiff, layeredBuffer, layeredBuffer.getBuffer().getLastRow()); + const patch = new Patch({status, hunks, marker: patchMarker}); return new FilePatch(oldFile, newFile, patch); } @@ -195,41 +195,18 @@ const CHANGEKIND = { '\\': NoNewline, }; -function initializeBuffer() { - const buffer = new TextBuffer(); - buffer.retain(); - - const layers = ['patch', 'hunk', 'unchanged', 'addition', 'deletion', 'noNewline'].reduce((obj, key) => { - obj[key] = buffer.addMarkerLayer(); - return obj; - }, {}); - - return {buffer, layers}; -} - -function buildHunks(diff, {buffer, layers}, insertRow) { - const layersByKind = new Map([ - [Unchanged, layers.unchanged], - [Addition, layers.addition], - [Deletion, layers.deletion], - [NoNewline, layers.noNewline], - ]); +function buildHunks(diff, layeredBuffer, insertRow, existingMarker = null) { const hunks = []; - const afterMarkers = []; - for (const layerName in layers) { - const layer = layers[layerName]; - afterMarkers.push(...layer.findMarkers({startPosition: [insertRow, 0]})); - } - const patchStartRow = insertRow; let bufferRow = patchStartRow; let nextLineLength = 0; if (diff.hunks.length === 0) { - const patchMarker = layers.patch.markPosition( + const patchMarker = layeredBuffer.markPosition( + Patch.layerName, [patchStartRow, 0], - {invalidate: 'never', exclusive: false}, + {invalidate: 'never', exclusive: true}, ); return [hunks, patchMarker]; @@ -251,7 +228,8 @@ function buildHunks(diff, {buffer, layers}, insertRow) { regions.push( new LastChangeKind( - layersByKind.get(LastChangeKind).markRange( + layeredBuffer.markRange( + LastChangeKind.layerName, [[currentRangeStart, 0], [bufferRow - 1, lastLineLength]], {invalidate: 'never', exclusive: false}, ), @@ -263,7 +241,7 @@ function buildHunks(diff, {buffer, layers}, insertRow) { for (const lineText of hunkData.lines) { const bufferLine = lineText.slice(1) + '\n'; nextLineLength = lineText.length - 1; - buffer.insert([bufferRow, 0], bufferLine); + layeredBuffer.getBuffer().insert([bufferRow, 0], bufferLine); const ChangeKind = CHANGEKIND[lineText[0]]; if (ChangeKind === undefined) { @@ -286,7 +264,8 @@ function buildHunks(diff, {buffer, layers}, insertRow) { oldRowCount: hunkData.oldLineCount, newRowCount: hunkData.newLineCount, sectionHeading: hunkData.heading, - marker: layers.hunk.markRange( + marker: layeredBuffer.markRange( + Hunk.layerName, [[bufferStartRow, 0], [bufferRow - 1, nextLineLength]], {invalidate: 'never', exclusive: false}, ), @@ -294,14 +273,15 @@ function buildHunks(diff, {buffer, layers}, insertRow) { })); } - const patchMarker = layers.patch.markRange( - [[patchStartRow, 0], [bufferRow - 1, nextLineLength]], - {invalidate: 'never', exclusive: false}, - ); - - // Correct any markers that used to start at the insertion point. - for (const marker of afterMarkers) { - marker.setTailPosition([bufferRow, 0]); + let patchMarker = existingMarker; + if (patchMarker) { + patchMarker.setRange([[patchStartRow, 0], [bufferRow - 1, nextLineLength]], {exclusive: false}); + } else { + patchMarker = layeredBuffer.markRange( + Patch.layerName, + [[patchStartRow, 0], [bufferRow - 1, nextLineLength]], + {invalidate: 'never', exclusive: false}, + ); } return [hunks, patchMarker]; diff --git a/lib/models/patch/hunk.js b/lib/models/patch/hunk.js index 84cab39002..074f86f7f3 100644 --- a/lib/models/patch/hunk.js +++ b/lib/models/patch/hunk.js @@ -1,4 +1,6 @@ export default class Hunk { + static layerName = 'hunk'; + constructor({ oldStartRow, newStartRow, diff --git a/lib/models/patch/layered-buffer.js b/lib/models/patch/layered-buffer.js new file mode 100644 index 0000000000..2265ae7899 --- /dev/null +++ b/lib/models/patch/layered-buffer.js @@ -0,0 +1,43 @@ +import {TextBuffer} from 'atom'; + +const LAYER_NAMES = ['unchanged', 'addition', 'deletion', 'nonewline', 'hunk', 'patch']; + +export default class LayeredBuffer { + constructor() { + this.buffer = new TextBuffer(); + this.layers = LAYER_NAMES.reduce((map, layerName) => { + map[layerName] = this.buffer.addMarkerLayer(); + return map; + }, {}); + } + + getBuffer() { + return this.buffer; + } + + getInsertionPoint() { + return this.buffer.getEndPosition(); + } + + getLayer(layerName) { + return this.layers[layerName]; + } + + findMarkers(layerName, ...args) { + return this.layers[layerName].findMarkers(...args); + } + + markPosition(layerName, ...args) { + return this.layers[layerName].markPosition(...args); + } + + markRange(layerName, ...args) { + return this.layers[layerName].markRange(...args); + } + + clearAllLayers() { + for (const layerName of LAYER_NAMES) { + this.layers[layerName].clear(); + } + } +} diff --git a/lib/models/patch/multi-file-patch.js b/lib/models/patch/multi-file-patch.js index 0490c5fd12..3e5a85c602 100644 --- a/lib/models/patch/multi-file-patch.js +++ b/lib/models/patch/multi-file-patch.js @@ -1,20 +1,17 @@ -import {TextBuffer, Range} from 'atom'; +import {Range} from 'atom'; import {RBTree} from 'bintrees'; +import LayeredBuffer from './layered-buffer'; import {TOO_LARGE, COLLAPSED} from './patch'; export default class MultiFilePatch { - constructor({buffer, layers, filePatches}) { - this.buffer = buffer || null; - - this.patchLayer = layers && layers.patch; - this.hunkLayer = layers && layers.hunk; - this.unchangedLayer = layers && layers.unchanged; - this.additionLayer = layers && layers.addition; - this.deletionLayer = layers && layers.deletion; - this.noNewlineLayer = layers && layers.noNewline; + static createNull() { + return new this({layeredBuffer: new LayeredBuffer(), filePatches: []}); + } - this.filePatches = filePatches || []; + constructor({layeredBuffer, filePatches}) { + this.layeredBuffer = layeredBuffer; + this.filePatches = filePatches; this.filePatchesByMarker = new Map(); this.filePatchesByPath = new Map(); @@ -48,45 +45,41 @@ export default class MultiFilePatch { clone(opts = {}) { return new this.constructor({ - buffer: opts.buffer !== undefined ? opts.buffer : this.getBuffer(), - layers: opts.layers !== undefined ? opts.layers : { - patch: this.getPatchLayer(), - hunk: this.getHunkLayer(), - unchanged: this.getUnchangedLayer(), - addition: this.getAdditionLayer(), - deletion: this.getDeletionLayer(), - noNewline: this.getNoNewlineLayer(), - }, + layeredBuffer: opts.layeredBuffer !== undefined ? opts.layeredBuffer : this.getLayeredBuffer(), filePatches: opts.filePatches !== undefined ? opts.filePatches : this.getFilePatches(), }); } + getLayeredBuffer() { + return this.layeredBuffer; + } + getBuffer() { - return this.buffer; + return this.getLayeredBuffer().getBuffer(); } getPatchLayer() { - return this.patchLayer; + return this.getLayeredBuffer().getLayer('patch'); } getHunkLayer() { - return this.hunkLayer; + return this.getLayeredBuffer().getLayer('hunk'); } getUnchangedLayer() { - return this.unchangedLayer; + return this.getLayeredBuffer().getLayer('unchanged'); } getAdditionLayer() { - return this.additionLayer; + return this.getLayeredBuffer().getLayer('addition'); } getDeletionLayer() { - return this.deletionLayer; + return this.getLayeredBuffer().getLayer('deletion'); } getNoNewlineLayer() { - return this.noNewlineLayer; + return this.getLayeredBuffer().getLayer('nonewline'); } getFilePatches() { @@ -108,7 +101,7 @@ export default class MultiFilePatch { if (bufferRow < 0) { return undefined; } - const [marker] = this.patchLayer.findMarkers({intersectsRow: bufferRow}); + const [marker] = this.layeredBuffer.findMarkers('patch', {intersectsRow: bufferRow}); return this.filePatchesByMarker.get(marker); } @@ -116,16 +109,16 @@ export default class MultiFilePatch { if (bufferRow < 0) { return undefined; } - const [marker] = this.hunkLayer.findMarkers({intersectsRow: bufferRow}); + const [marker] = this.layeredBuffer.findMarkers('hunk', {intersectsRow: bufferRow}); return this.hunksByMarker.get(marker); } getStagePatchForLines(selectedLineSet) { - const nextLayeredBuffer = this.buildLayeredBuffer(); + const nextLayeredBuffer = new LayeredBuffer(); const nextFilePatches = this.getFilePatchesContaining(selectedLineSet).map(fp => { return fp.buildStagePatchForLines(this.getBuffer(), nextLayeredBuffer, selectedLineSet); }); - return this.clone({...nextLayeredBuffer, filePatches: nextFilePatches}); + return this.clone({layeredBuffer: nextLayeredBuffer, filePatches: nextFilePatches}); } getStagePatchForHunk(hunk) { @@ -133,11 +126,11 @@ export default class MultiFilePatch { } getUnstagePatchForLines(selectedLineSet) { - const nextLayeredBuffer = this.buildLayeredBuffer(); + const nextLayeredBuffer = new LayeredBuffer(); const nextFilePatches = this.getFilePatchesContaining(selectedLineSet).map(fp => { return fp.buildUnstagePatchForLines(this.getBuffer(), nextLayeredBuffer, selectedLineSet); }); - return this.clone({...nextLayeredBuffer, filePatches: nextFilePatches}); + return this.clone({layeredBuffer: nextLayeredBuffer, filePatches: nextFilePatches}); } getUnstagePatchForHunk(hunk) { @@ -220,64 +213,29 @@ export default class MultiFilePatch { } adoptBufferFrom(lastMultiFilePatch) { - lastMultiFilePatch.getPatchLayer().clear(); - lastMultiFilePatch.getHunkLayer().clear(); - lastMultiFilePatch.getUnchangedLayer().clear(); - lastMultiFilePatch.getAdditionLayer().clear(); - lastMultiFilePatch.getDeletionLayer().clear(); - lastMultiFilePatch.getNoNewlineLayer().clear(); + const nextLayeredBuffer = lastMultiFilePatch.getLayeredBuffer(); + nextLayeredBuffer.clearAllLayers(); this.filePatchesByMarker.clear(); this.hunksByMarker.clear(); - const nextBuffer = lastMultiFilePatch.getBuffer(); - nextBuffer.setText(this.getBuffer().getText()); + nextLayeredBuffer.getBuffer().setText(this.getBuffer().getText()); for (const filePatch of this.getFilePatches()) { - filePatch.getPatch().reMarkOn(lastMultiFilePatch.getPatchLayer()); + filePatch.getPatch().reMarkOn(nextLayeredBuffer); this.filePatchesByMarker.set(filePatch.getMarker(), filePatch); for (const hunk of filePatch.getHunks()) { - hunk.reMarkOn(lastMultiFilePatch.getHunkLayer()); + hunk.reMarkOn(nextLayeredBuffer); this.hunksByMarker.set(hunk.getMarker(), hunk); for (const region of hunk.getRegions()) { - const target = region.when({ - unchanged: () => lastMultiFilePatch.getUnchangedLayer(), - addition: () => lastMultiFilePatch.getAdditionLayer(), - deletion: () => lastMultiFilePatch.getDeletionLayer(), - nonewline: () => lastMultiFilePatch.getNoNewlineLayer(), - }); - region.reMarkOn(target); + region.reMarkOn(nextLayeredBuffer); } } } - this.patchLayer = lastMultiFilePatch.getPatchLayer(); - this.hunkLayer = lastMultiFilePatch.getHunkLayer(); - this.unchangedLayer = lastMultiFilePatch.getUnchangedLayer(); - this.additionLayer = lastMultiFilePatch.getAdditionLayer(); - this.deletionLayer = lastMultiFilePatch.getDeletionLayer(); - this.noNewlineLayer = lastMultiFilePatch.getNoNewlineLayer(); - - this.buffer = nextBuffer; - } - - buildLayeredBuffer() { - const buffer = new TextBuffer(); - buffer.retain(); - - return { - buffer, - layers: { - patch: buffer.addMarkerLayer(), - hunk: buffer.addMarkerLayer(), - unchanged: buffer.addMarkerLayer(), - addition: buffer.addMarkerLayer(), - deletion: buffer.addMarkerLayer(), - noNewline: buffer.addMarkerLayer(), - }, - }; + this.layeredBuffer = nextLayeredBuffer; } /* @@ -304,7 +262,7 @@ export default class MultiFilePatch { } anyPresent() { - return this.buffer !== null && this.filePatches.some(fp => fp.isPresent()); + return this.layeredBuffer !== null && this.filePatches.some(fp => fp.isPresent()); } didAnyChangeExecutableMode() { @@ -362,7 +320,7 @@ export default class MultiFilePatch { * Construct an apply-able patch String. */ toString() { - return this.filePatches.map(fp => fp.toStringIn(this.buffer)).join(''); + return this.filePatches.map(fp => fp.toStringIn(this.getBuffer())).join(''); } isEqual(other) { diff --git a/lib/models/patch/patch.js b/lib/models/patch/patch.js index 99741312ca..b5fc8209e5 100644 --- a/lib/models/patch/patch.js +++ b/lib/models/patch/patch.js @@ -3,13 +3,27 @@ import {TextBuffer, Range} from 'atom'; import Hunk from './hunk'; import {Unchanged, Addition, Deletion, NoNewline} from './region'; -export const TOO_LARGE = Symbol('too large'); +export const EXPANDED = { + toString() { return 'RenderStatus(expanded)'; }, -export const EXPANDED = Symbol('expanded'); + isVisible() { return true; }, +}; -export const COLLAPSED = Symbol('collapsed'); +export const COLLAPSED = { + toString() { return 'RenderStatus(collapsed)'; }, + + isVisible() { return false; }, +}; + +export const TOO_LARGE = { + toString() { return 'RenderStatus(too-large)'; }, + + isVisible() { return false; }, +}; export default class Patch { + static layerName = 'patch'; + static createNull() { return new NullPatch(); } @@ -61,8 +75,12 @@ export default class Patch { return this.marker.getRange().intersectsRow(row); } - reMarkOn(markable) { - this.marker = markable.markRange(this.getRange(), {invalidate: 'never', exclusive: false}); + reMarkOn(layeredBuffer) { + this.marker = layeredBuffer.markRange( + this.constructor.layerName, + this.getRange(), + {invalidate: 'never', exclusive: false}, + ); } getMaxLineNumberWidth() { @@ -174,9 +192,11 @@ export default class Patch { } } - const buffer = builder.getBuffer(); - const layers = builder.getLayers(); - const marker = layers.patch.markRange([[0, 0], [buffer.getLastRow() - 1, Infinity]]); + const marker = nextLayeredBuffer.markRange( + this.constructor.layerName, + [[0, 0], [nextLayeredBuffer.getBuffer().getLastRow() - 1, Infinity]], + {invalidate: 'never', exclusive: false}, + ); const wholeFile = rowSet.size === this.changedLineCount; const status = this.getStatus() === 'deleted' && !wholeFile ? 'modified' : this.getStatus(); @@ -269,9 +289,11 @@ export default class Patch { status = 'added'; } - const buffer = builder.getBuffer(); - const layers = builder.getLayers(); - const marker = layers.patch.markRange([[0, 0], [buffer.getLastRow(), Infinity]]); + const marker = nextLayeredBuffer.markRange( + this.constructor.layerName, + [[0, 0], [nextLayeredBuffer.getBuffer().getLastRow(), Infinity]], + {invalidate: 'never', exclusive: false}, + ); return this.clone({hunks, status, marker}); } @@ -400,16 +422,7 @@ class NullPatch { class BufferBuilder { constructor(original, originalBaseOffset, nextLayeredBuffer) { this.originalBuffer = original; - - this.buffer = nextLayeredBuffer.buffer; - this.layers = new Map([ - [Unchanged, nextLayeredBuffer.layers.unchanged], - [Addition, nextLayeredBuffer.layers.addition], - [Deletion, nextLayeredBuffer.layers.deletion], - [NoNewline, nextLayeredBuffer.layers.noNewline], - ['hunk', nextLayeredBuffer.layers.hunk], - ['patch', nextLayeredBuffer.layers.patch], - ]); + this.nextLayeredBuffer = nextLayeredBuffer; // The ranges provided to builder methods are expected to be valid within the original buffer. Account for // the position of the Patch within its original TextBuffer, and any existing content already on the next @@ -457,15 +470,15 @@ class BufferBuilder { } latestHunkWasIncluded() { - this.buffer.append(this.hunkBufferText, {normalizeLineEndings: false}); + this.nextLayeredBuffer.getBuffer().append(this.hunkBufferText, {normalizeLineEndings: false}); const regions = this.hunkRegions.map(({RegionKind, range}) => { return new RegionKind( - this.layers.get(RegionKind).markRange(range, {invalidate: 'never', exclusive: false}), + this.nextLayeredBuffer.getLayer(RegionKind.layerName, range, {invalidate: 'never', exclusive: false}), ); }); - const marker = this.layers.get('hunk').markRange(this.hunkRange, {invalidate: 'never', exclusive: false}); + const marker = this.nextLayeredBuffer.markRange('hunk', this.hunkRange, {invalidate: 'never', exclusive: false}); this.hunkBufferText = ''; this.hunkRowCount = 0; @@ -488,18 +501,7 @@ class BufferBuilder { return {regions: [], marker: null}; } - getBuffer() { - return this.buffer; - } - - getLayers() { - return { - patch: this.layers.get('patch'), - hunk: this.layers.get('hunk'), - unchanged: this.layers.get(Unchanged), - addition: this.layers.get(Addition), - deletion: this.layers.get(Deletion), - noNewline: this.layers.get(NoNewline), - }; + getLayeredBuffer() { + return this.nextLayeredBuffer; } } diff --git a/lib/models/patch/region.js b/lib/models/patch/region.js index 3afaef844d..d246b911bc 100644 --- a/lib/models/patch/region.js +++ b/lib/models/patch/region.js @@ -125,6 +125,8 @@ class Region { export class Addition extends Region { static origin = '+'; + static layerName = 'addition'; + isAddition() { return true; } @@ -137,6 +139,8 @@ export class Addition extends Region { export class Deletion extends Region { static origin = '-'; + static layerName = 'deletion'; + isDeletion() { return true; } @@ -149,6 +153,8 @@ export class Deletion extends Region { export class Unchanged extends Region { static origin = ' '; + static layerName = 'unchanged'; + isUnchanged() { return true; } @@ -165,6 +171,8 @@ export class Unchanged extends Region { export class NoNewline extends Region { static origin = '\\'; + static layerName = 'nonewline'; + isNoNewline() { return true; } diff --git a/test/builder/patch.js b/test/builder/patch.js index c2e0e830ab..6da24e5f29 100644 --- a/test/builder/patch.js +++ b/test/builder/patch.js @@ -1,6 +1,6 @@ // Builders for classes related to MultiFilePatches. -import {TextBuffer} from 'atom'; +import LayeredBuffer from '../../lib/models/patch/layered-buffer'; import MultiFilePatch from '../../lib/models/patch/multi-file-patch'; import FilePatch from '../../lib/models/patch/file-patch'; import File, {nullFile} from '../../lib/models/patch/file'; @@ -8,49 +8,29 @@ import Patch from '../../lib/models/patch/patch'; import Hunk from '../../lib/models/patch/hunk'; import {Unchanged, Addition, Deletion, NoNewline} from '../../lib/models/patch/region'; -class LayeredBuffer { - constructor() { - this.buffer = new TextBuffer(); - this.layers = ['patch', 'hunk', 'unchanged', 'addition', 'deletion', 'noNewline'].reduce((layers, name) => { - layers[name] = this.buffer.addMarkerLayer(); - return layers; - }, {}); - } - - getInsertionPoint() { - return this.buffer.getEndPosition(); - } - - getLayer(markerLayerName) { - const layer = this.layers[markerLayerName]; - if (!layer) { - throw new Error(`invalid marker layer name: ${markerLayerName}`); - } - return layer; - } - - appendMarked(markerLayerName, lines) { - const startPosition = this.buffer.getEndPosition(); - const layer = this.getLayer(markerLayerName); - this.buffer.append(lines.join('\n')); - const marker = layer.markRange([startPosition, this.buffer.getEndPosition()], {exclusive: true}); - this.buffer.append('\n'); - return marker; - } +function appendMarked(layeredBuffer, layerName, lines) { + const startPosition = layeredBuffer.getInsertionPoint(); + layeredBuffer.getBuffer().append(lines.join('\n')); + const marker = layeredBuffer.markRange( + layerName, + [startPosition, layeredBuffer.getInsertionPoint()], + {invalidate: 'never', exclusive: false}, + ); + layeredBuffer.getBuffer().append('\n'); + return marker; +} - markFrom(markerLayerName, startPosition) { - const endPosition = this.buffer.getEndPosition().translate([-1, Infinity]); - const layer = this.getLayer(markerLayerName); - return layer.markRange([startPosition, endPosition], {exclusive: true}); - } +function markFrom(layeredBuffer, layerName, startPosition) { + const endPosition = layeredBuffer.getInsertionPoint().translate([-1, Infinity]); + return layeredBuffer.markRange( + layerName, + [startPosition, endPosition], + {invalidate: 'never', exclusive: false}, + ); +} - wrapReturn(object) { - return { - buffer: this.buffer, - layers: this.layers, - ...object, - }; - } +function wrapReturn(layeredBuffer, object) { + return {buffer: layeredBuffer.getBuffer(), layers: layeredBuffer.getLayers(), ...object}; } class MultiFilePatchBuilder { @@ -68,10 +48,9 @@ class MultiFilePatchBuilder { } build() { - return this.layeredBuffer.wrapReturn({ + return wrapReturn(this.layeredBuffer, { multiFilePatch: new MultiFilePatch({ - buffer: this.layeredBuffer.buffer, - layers: this.layeredBuffer.layers, + layeredBuffer: this.layeredBuffer, filePatches: this.filePatches, }), }); @@ -229,9 +208,9 @@ class PatchBuilder { } } - const marker = this.layeredBuffer.markFrom('patch', this.patchStart); + const marker = markFrom(this.layeredBuffer, 'patch', this.patchStart); - return this.layeredBuffer.wrapReturn({ + return wrapReturn(this.layeredBuffer, { patch: new Patch({status: this._status, hunks: this.hunks, marker, renderStatus: this._renderStatus}), }); } @@ -259,22 +238,22 @@ class HunkBuilder { } unchanged(...lines) { - this.regions.push(new Unchanged(this.layeredBuffer.appendMarked('unchanged', lines))); + this.regions.push(new Unchanged(appendMarked(this.layeredBuffer, 'unchanged', lines))); return this; } added(...lines) { - this.regions.push(new Addition(this.layeredBuffer.appendMarked('addition', lines))); + this.regions.push(new Addition(appendMarked(this.layeredBuffer, 'addition', lines))); return this; } deleted(...lines) { - this.regions.push(new Deletion(this.layeredBuffer.appendMarked('deletion', lines))); + this.regions.push(new Deletion(appendMarked(this.layeredBuffer, 'deletion', lines))); return this; } noNewline() { - this.regions.push(new NoNewline(this.layeredBuffer.appendMarked('noNewline', [' No newline at end of file']))); + this.regions.push(new NoNewline(appendMarked(this.layeredBuffer, 'nonewline', [' No newline at end of file']))); return this; } @@ -303,9 +282,9 @@ class HunkBuilder { }), 0); } - const marker = this.layeredBuffer.markFrom('hunk', this.hunkStartPoint); + const marker = markFrom(this.layeredBuffer, 'hunk', this.hunkStartPoint); - return this.layeredBuffer.wrapReturn({ + return wrapReturn(this.layeredBuffer, { hunk: new Hunk({ oldStartRow: this.oldStartRow, oldRowCount: this.oldRowCount, diff --git a/test/models/patch/builder.test.js b/test/models/patch/builder.test.js index d917d8af36..75949de05e 100644 --- a/test/models/patch/builder.test.js +++ b/test/models/patch/builder.test.js @@ -885,7 +885,7 @@ describe('buildFilePatch', function() { ); }); - it('does not interfere with markers from surrounding patches when expanded', function() { + it('does not interfere with markers from surrounding visible patches when expanded', function() { const mfp = buildMultiFilePatch([ { oldPath: 'first', oldMode: '100644', newPath: 'first', newMode: '100644', status: 'modified', @@ -984,7 +984,7 @@ describe('buildFilePatch', function() { ); }); - it('preserves patch markers when a delayed patch is expanded', function() { + it('does not interfere with markers from surrounding non-visible patches when expanded', function() { const mfp = buildMultiFilePatch([ { oldPath: 'first', oldMode: '100644', newPath: 'first', newMode: '100644', status: 'modified', From f963e7510a14f6bf7015ad9e7694f852efa23246 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 17 Jan 2019 14:01:46 -0500 Subject: [PATCH 32/92] Rename LayeredBuffer to PatchBuffer --- lib/models/patch/builder.js | 46 +++++------ lib/models/patch/multi-file-patch.js | 28 +++---- .../{layered-buffer.js => patch-buffer.js} | 2 +- lib/models/patch/patch.js | 4 +- test/builder/patch.js | 80 +++++++++---------- 5 files changed, 80 insertions(+), 80 deletions(-) rename lib/models/patch/{layered-buffer.js => patch-buffer.js} (96%) diff --git a/lib/models/patch/builder.js b/lib/models/patch/builder.js index 6a308b2f7d..ab8c81109c 100644 --- a/lib/models/patch/builder.js +++ b/lib/models/patch/builder.js @@ -1,4 +1,4 @@ -import LayeredBuffer from './layered-buffer'; +import PatchBuffer from './patch-buffer'; import Hunk from './hunk'; import File, {nullFile} from './file'; import Patch, {TOO_LARGE, EXPANDED} from './patch'; @@ -18,26 +18,26 @@ const DEFAULT_OPTIONS = { export function buildFilePatch(diffs, options) { const opts = {...DEFAULT_OPTIONS, ...options}; - const layeredBuffer = new LayeredBuffer(); + const patchBuffer = new PatchBuffer(); let filePatch; if (diffs.length === 0) { filePatch = emptyDiffFilePatch(); } else if (diffs.length === 1) { - filePatch = singleDiffFilePatch(diffs[0], layeredBuffer, opts); + filePatch = singleDiffFilePatch(diffs[0], patchBuffer, opts); } else if (diffs.length === 2) { - filePatch = dualDiffFilePatch(diffs[0], diffs[1], layeredBuffer, opts); + filePatch = dualDiffFilePatch(diffs[0], diffs[1], patchBuffer, opts); } else { throw new Error(`Unexpected number of diffs: ${diffs.length}`); } - return new MultiFilePatch({layeredBuffer, filePatches: [filePatch]}); + return new MultiFilePatch({patchBuffer, filePatches: [filePatch]}); } export function buildMultiFilePatch(diffs, options) { const opts = {...DEFAULT_OPTIONS, ...options}; - const layeredBuffer = new LayeredBuffer(); + const patchBuffer = new PatchBuffer(); const byPath = new Map(); const actions = []; @@ -52,7 +52,7 @@ export function buildMultiFilePatch(diffs, options) { if (otherHalf) { // The second half. Complete the paired diff, or fail if they have unexpected statuses or modes. const [otherDiff, otherIndex] = otherHalf; - actions[otherIndex] = () => dualDiffFilePatch(diff, otherDiff, layeredBuffer, opts); + actions[otherIndex] = () => dualDiffFilePatch(diff, otherDiff, patchBuffer, opts); byPath.delete(thePath); } else { // The first half we've seen. @@ -60,14 +60,14 @@ export function buildMultiFilePatch(diffs, options) { index++; } } else { - actions[index] = () => singleDiffFilePatch(diff, layeredBuffer, opts); + actions[index] = () => singleDiffFilePatch(diff, patchBuffer, opts); index++; } } // Populate unpaired diffs that looked like they could be part of a pair, but weren't. for (const [unpairedDiff, originalIndex] of byPath.values()) { - actions[originalIndex] = () => singleDiffFilePatch(unpairedDiff, layeredBuffer, opts); + actions[originalIndex] = () => singleDiffFilePatch(unpairedDiff, patchBuffer, opts); } const filePatches = actions.map(action => action()); @@ -81,14 +81,14 @@ export function buildMultiFilePatch(diffs, options) { } }); - return new MultiFilePatch({layeredBuffer, filePatches}); + return new MultiFilePatch({patchBuffer, filePatches}); } function emptyDiffFilePatch() { return FilePatch.createNull(); } -function singleDiffFilePatch(diff, layeredBuffer, opts) { +function singleDiffFilePatch(diff, patchBuffer, opts) { const wasSymlink = diff.oldMode === File.modes.SYMLINK; const isSymlink = diff.newMode === File.modes.SYMLINK; @@ -120,8 +120,8 @@ function singleDiffFilePatch(diff, layeredBuffer, opts) { EXPANDED; if (renderStatus === TOO_LARGE) { - const insertRow = layeredBuffer.getBuffer().getLastRow(); - const patchMarker = layeredBuffer.markPosition( + const insertRow = patchBuffer.getBuffer().getLastRow(); + const patchMarker = patchBuffer.markPosition( Patch.layerName, [insertRow, 0], {invalidate: 'never', exclusive: false}, @@ -130,19 +130,19 @@ function singleDiffFilePatch(diff, layeredBuffer, opts) { return FilePatch.createDelayedFilePatch( oldFile, newFile, patchMarker, TOO_LARGE, () => { - const [hunks] = buildHunks(diff, layeredBuffer, insertRow, patchMarker); + const [hunks] = buildHunks(diff, patchBuffer, insertRow, patchMarker); return new Patch({status: diff.status, hunks, marker: patchMarker}); }, ); } else { - const [hunks, patchMarker] = buildHunks(diff, layeredBuffer, layeredBuffer.getBuffer().getLastRow()); + const [hunks, patchMarker] = buildHunks(diff, patchBuffer, patchBuffer.getBuffer().getLastRow()); const patch = new Patch({status: diff.status, hunks, marker: patchMarker}); return new FilePatch(oldFile, newFile, patch); } } -function dualDiffFilePatch(diff1, diff2, layeredBuffer, opts) { +function dualDiffFilePatch(diff1, diff2, patchBuffer, opts) { let modeChangeDiff, contentChangeDiff; if (diff1.oldMode === File.modes.SYMLINK || diff1.newMode === File.modes.SYMLINK) { modeChangeDiff = diff1; @@ -182,7 +182,7 @@ function dualDiffFilePatch(diff1, diff2, layeredBuffer, opts) { // TODO: do something with large diff too } - const [hunks, patchMarker] = buildHunks(contentChangeDiff, layeredBuffer, layeredBuffer.getBuffer().getLastRow()); + const [hunks, patchMarker] = buildHunks(contentChangeDiff, patchBuffer, patchBuffer.getBuffer().getLastRow()); const patch = new Patch({status, hunks, marker: patchMarker}); return new FilePatch(oldFile, newFile, patch); @@ -195,7 +195,7 @@ const CHANGEKIND = { '\\': NoNewline, }; -function buildHunks(diff, layeredBuffer, insertRow, existingMarker = null) { +function buildHunks(diff, patchBuffer, insertRow, existingMarker = null) { const hunks = []; const patchStartRow = insertRow; @@ -203,7 +203,7 @@ function buildHunks(diff, layeredBuffer, insertRow, existingMarker = null) { let nextLineLength = 0; if (diff.hunks.length === 0) { - const patchMarker = layeredBuffer.markPosition( + const patchMarker = patchBuffer.markPosition( Patch.layerName, [patchStartRow, 0], {invalidate: 'never', exclusive: true}, @@ -228,7 +228,7 @@ function buildHunks(diff, layeredBuffer, insertRow, existingMarker = null) { regions.push( new LastChangeKind( - layeredBuffer.markRange( + patchBuffer.markRange( LastChangeKind.layerName, [[currentRangeStart, 0], [bufferRow - 1, lastLineLength]], {invalidate: 'never', exclusive: false}, @@ -241,7 +241,7 @@ function buildHunks(diff, layeredBuffer, insertRow, existingMarker = null) { for (const lineText of hunkData.lines) { const bufferLine = lineText.slice(1) + '\n'; nextLineLength = lineText.length - 1; - layeredBuffer.getBuffer().insert([bufferRow, 0], bufferLine); + patchBuffer.getBuffer().insert([bufferRow, 0], bufferLine); const ChangeKind = CHANGEKIND[lineText[0]]; if (ChangeKind === undefined) { @@ -264,7 +264,7 @@ function buildHunks(diff, layeredBuffer, insertRow, existingMarker = null) { oldRowCount: hunkData.oldLineCount, newRowCount: hunkData.newLineCount, sectionHeading: hunkData.heading, - marker: layeredBuffer.markRange( + marker: patchBuffer.markRange( Hunk.layerName, [[bufferStartRow, 0], [bufferRow - 1, nextLineLength]], {invalidate: 'never', exclusive: false}, @@ -277,7 +277,7 @@ function buildHunks(diff, layeredBuffer, insertRow, existingMarker = null) { if (patchMarker) { patchMarker.setRange([[patchStartRow, 0], [bufferRow - 1, nextLineLength]], {exclusive: false}); } else { - patchMarker = layeredBuffer.markRange( + patchMarker = patchBuffer.markRange( Patch.layerName, [[patchStartRow, 0], [bufferRow - 1, nextLineLength]], {invalidate: 'never', exclusive: false}, diff --git a/lib/models/patch/multi-file-patch.js b/lib/models/patch/multi-file-patch.js index 3e5a85c602..7d0a9ae8d4 100644 --- a/lib/models/patch/multi-file-patch.js +++ b/lib/models/patch/multi-file-patch.js @@ -1,16 +1,16 @@ import {Range} from 'atom'; import {RBTree} from 'bintrees'; -import LayeredBuffer from './layered-buffer'; +import PatchBuffer from './patch-buffer'; import {TOO_LARGE, COLLAPSED} from './patch'; export default class MultiFilePatch { static createNull() { - return new this({layeredBuffer: new LayeredBuffer(), filePatches: []}); + return new this({patchBuffer: new PatchBuffer(), filePatches: []}); } - constructor({layeredBuffer, filePatches}) { - this.layeredBuffer = layeredBuffer; + constructor({patchBuffer, filePatches}) { + this.patchBuffer = patchBuffer; this.filePatches = filePatches; this.filePatchesByMarker = new Map(); @@ -45,13 +45,13 @@ export default class MultiFilePatch { clone(opts = {}) { return new this.constructor({ - layeredBuffer: opts.layeredBuffer !== undefined ? opts.layeredBuffer : this.getLayeredBuffer(), + patchBuffer: opts.patchBuffer !== undefined ? opts.patchBuffer : this.getLayeredBuffer(), filePatches: opts.filePatches !== undefined ? opts.filePatches : this.getFilePatches(), }); } getLayeredBuffer() { - return this.layeredBuffer; + return this.patchBuffer; } getBuffer() { @@ -101,7 +101,7 @@ export default class MultiFilePatch { if (bufferRow < 0) { return undefined; } - const [marker] = this.layeredBuffer.findMarkers('patch', {intersectsRow: bufferRow}); + const [marker] = this.patchBuffer.findMarkers('patch', {intersectsRow: bufferRow}); return this.filePatchesByMarker.get(marker); } @@ -109,16 +109,16 @@ export default class MultiFilePatch { if (bufferRow < 0) { return undefined; } - const [marker] = this.layeredBuffer.findMarkers('hunk', {intersectsRow: bufferRow}); + const [marker] = this.patchBuffer.findMarkers('hunk', {intersectsRow: bufferRow}); return this.hunksByMarker.get(marker); } getStagePatchForLines(selectedLineSet) { - const nextLayeredBuffer = new LayeredBuffer(); + const nextLayeredBuffer = new PatchBuffer(); const nextFilePatches = this.getFilePatchesContaining(selectedLineSet).map(fp => { return fp.buildStagePatchForLines(this.getBuffer(), nextLayeredBuffer, selectedLineSet); }); - return this.clone({layeredBuffer: nextLayeredBuffer, filePatches: nextFilePatches}); + return this.clone({patchBuffer: nextLayeredBuffer, filePatches: nextFilePatches}); } getStagePatchForHunk(hunk) { @@ -126,11 +126,11 @@ export default class MultiFilePatch { } getUnstagePatchForLines(selectedLineSet) { - const nextLayeredBuffer = new LayeredBuffer(); + const nextLayeredBuffer = new PatchBuffer(); const nextFilePatches = this.getFilePatchesContaining(selectedLineSet).map(fp => { return fp.buildUnstagePatchForLines(this.getBuffer(), nextLayeredBuffer, selectedLineSet); }); - return this.clone({layeredBuffer: nextLayeredBuffer, filePatches: nextFilePatches}); + return this.clone({patchBuffer: nextLayeredBuffer, filePatches: nextFilePatches}); } getUnstagePatchForHunk(hunk) { @@ -235,7 +235,7 @@ export default class MultiFilePatch { } } - this.layeredBuffer = nextLayeredBuffer; + this.patchBuffer = nextLayeredBuffer; } /* @@ -262,7 +262,7 @@ export default class MultiFilePatch { } anyPresent() { - return this.layeredBuffer !== null && this.filePatches.some(fp => fp.isPresent()); + return this.patchBuffer !== null && this.filePatches.some(fp => fp.isPresent()); } didAnyChangeExecutableMode() { diff --git a/lib/models/patch/layered-buffer.js b/lib/models/patch/patch-buffer.js similarity index 96% rename from lib/models/patch/layered-buffer.js rename to lib/models/patch/patch-buffer.js index 2265ae7899..a6f1d0666f 100644 --- a/lib/models/patch/layered-buffer.js +++ b/lib/models/patch/patch-buffer.js @@ -2,7 +2,7 @@ import {TextBuffer} from 'atom'; const LAYER_NAMES = ['unchanged', 'addition', 'deletion', 'nonewline', 'hunk', 'patch']; -export default class LayeredBuffer { +export default class PatchBuffer { constructor() { this.buffer = new TextBuffer(); this.layers = LAYER_NAMES.reduce((map, layerName) => { diff --git a/lib/models/patch/patch.js b/lib/models/patch/patch.js index b5fc8209e5..129e23cfd3 100644 --- a/lib/models/patch/patch.js +++ b/lib/models/patch/patch.js @@ -75,8 +75,8 @@ export default class Patch { return this.marker.getRange().intersectsRow(row); } - reMarkOn(layeredBuffer) { - this.marker = layeredBuffer.markRange( + reMarkOn(patchBuffer) { + this.marker = patchBuffer.markRange( this.constructor.layerName, this.getRange(), {invalidate: 'never', exclusive: false}, diff --git a/test/builder/patch.js b/test/builder/patch.js index 6da24e5f29..cd8f45a79f 100644 --- a/test/builder/patch.js +++ b/test/builder/patch.js @@ -1,6 +1,6 @@ // Builders for classes related to MultiFilePatches. -import LayeredBuffer from '../../lib/models/patch/layered-buffer'; +import PatchBuffer from '../../lib/models/patch/patch-buffer'; import MultiFilePatch from '../../lib/models/patch/multi-file-patch'; import FilePatch from '../../lib/models/patch/file-patch'; import File, {nullFile} from '../../lib/models/patch/file'; @@ -8,49 +8,49 @@ import Patch from '../../lib/models/patch/patch'; import Hunk from '../../lib/models/patch/hunk'; import {Unchanged, Addition, Deletion, NoNewline} from '../../lib/models/patch/region'; -function appendMarked(layeredBuffer, layerName, lines) { - const startPosition = layeredBuffer.getInsertionPoint(); - layeredBuffer.getBuffer().append(lines.join('\n')); - const marker = layeredBuffer.markRange( +function appendMarked(patchBuffer, layerName, lines) { + const startPosition = patchBuffer.getInsertionPoint(); + patchBuffer.getBuffer().append(lines.join('\n')); + const marker = patchBuffer.markRange( layerName, - [startPosition, layeredBuffer.getInsertionPoint()], + [startPosition, patchBuffer.getInsertionPoint()], {invalidate: 'never', exclusive: false}, ); - layeredBuffer.getBuffer().append('\n'); + patchBuffer.getBuffer().append('\n'); return marker; } -function markFrom(layeredBuffer, layerName, startPosition) { - const endPosition = layeredBuffer.getInsertionPoint().translate([-1, Infinity]); - return layeredBuffer.markRange( +function markFrom(patchBuffer, layerName, startPosition) { + const endPosition = patchBuffer.getInsertionPoint().translate([-1, Infinity]); + return patchBuffer.markRange( layerName, [startPosition, endPosition], {invalidate: 'never', exclusive: false}, ); } -function wrapReturn(layeredBuffer, object) { - return {buffer: layeredBuffer.getBuffer(), layers: layeredBuffer.getLayers(), ...object}; +function wrapReturn(patchBuffer, object) { + return {buffer: patchBuffer.getBuffer(), layers: patchBuffer.getLayers(), ...object}; } class MultiFilePatchBuilder { - constructor(layeredBuffer = null) { - this.layeredBuffer = layeredBuffer; + constructor(patchBuffer = null) { + this.patchBuffer = patchBuffer; this.filePatches = []; } addFilePatch(block = () => {}) { - const filePatch = new FilePatchBuilder(this.layeredBuffer); + const filePatch = new FilePatchBuilder(this.patchBuffer); block(filePatch); this.filePatches.push(filePatch.build().filePatch); return this; } build() { - return wrapReturn(this.layeredBuffer, { + return wrapReturn(this.patchBuffer, { multiFilePatch: new MultiFilePatch({ - layeredBuffer: this.layeredBuffer, + patchBuffer: this.patchBuffer, filePatches: this.filePatches, }), }); @@ -58,13 +58,13 @@ class MultiFilePatchBuilder { } class FilePatchBuilder { - constructor(layeredBuffer = null) { - this.layeredBuffer = layeredBuffer; + constructor(patchBuffer = null) { + this.patchBuffer = patchBuffer; this.oldFile = new File({path: 'file', mode: File.modes.NORMAL}); this.newFile = null; - this.patchBuilder = new PatchBuilder(this.layeredBuffer); + this.patchBuilder = new PatchBuilder(this.patchBuffer); } setOldFile(block) { @@ -118,7 +118,7 @@ class FilePatchBuilder { this.newFile = this.oldFile.clone(); } - return this.layeredBuffer.wrapReturn({ + return this.patchBuffer.wrapReturn({ filePatch: new FilePatch(this.oldFile, this.newFile, patch), }); } @@ -156,14 +156,14 @@ class FileBuilder { } class PatchBuilder { - constructor(layeredBuffer = null) { - this.layeredBuffer = layeredBuffer; + constructor(patchBuffer = null) { + this.patchBuffer = patchBuffer; this._renderStatus = undefined; this._status = 'modified'; this.hunks = []; - this.patchStart = this.layeredBuffer.getInsertionPoint(); + this.patchStart = this.patchBuffer.getInsertionPoint(); this.drift = 0; this.explicitlyEmpty = false; } @@ -183,7 +183,7 @@ class PatchBuilder { } addHunk(block = () => {}) { - const builder = new HunkBuilder(this.layeredBuffer, this.drift); + const builder = new HunkBuilder(this.patchBuffer, this.drift); block(builder); const {hunk, drift} = builder.build(); this.hunks.push(hunk); @@ -208,17 +208,17 @@ class PatchBuilder { } } - const marker = markFrom(this.layeredBuffer, 'patch', this.patchStart); + const marker = markFrom(this.patchBuffer, 'patch', this.patchStart); - return wrapReturn(this.layeredBuffer, { + return wrapReturn(this.patchBuffer, { patch: new Patch({status: this._status, hunks: this.hunks, marker, renderStatus: this._renderStatus}), }); } } class HunkBuilder { - constructor(layeredBuffer = null, drift = 0) { - this.layeredBuffer = layeredBuffer; + constructor(patchBuffer = null, drift = 0) { + this.patchBuffer = patchBuffer; this.drift = drift; this.oldStartRow = 0; @@ -228,7 +228,7 @@ class HunkBuilder { this.sectionHeading = "don't care"; - this.hunkStartPoint = this.layeredBuffer.getInsertionPoint(); + this.hunkStartPoint = this.patchBuffer.getInsertionPoint(); this.regions = []; } @@ -238,22 +238,22 @@ class HunkBuilder { } unchanged(...lines) { - this.regions.push(new Unchanged(appendMarked(this.layeredBuffer, 'unchanged', lines))); + this.regions.push(new Unchanged(appendMarked(this.patchBuffer, 'unchanged', lines))); return this; } added(...lines) { - this.regions.push(new Addition(appendMarked(this.layeredBuffer, 'addition', lines))); + this.regions.push(new Addition(appendMarked(this.patchBuffer, 'addition', lines))); return this; } deleted(...lines) { - this.regions.push(new Deletion(appendMarked(this.layeredBuffer, 'deletion', lines))); + this.regions.push(new Deletion(appendMarked(this.patchBuffer, 'deletion', lines))); return this; } noNewline() { - this.regions.push(new NoNewline(appendMarked(this.layeredBuffer, 'nonewline', [' No newline at end of file']))); + this.regions.push(new NoNewline(appendMarked(this.patchBuffer, 'nonewline', [' No newline at end of file']))); return this; } @@ -282,9 +282,9 @@ class HunkBuilder { }), 0); } - const marker = markFrom(this.layeredBuffer, 'hunk', this.hunkStartPoint); + const marker = markFrom(this.patchBuffer, 'hunk', this.hunkStartPoint); - return wrapReturn(this.layeredBuffer, { + return wrapReturn(this.patchBuffer, { hunk: new Hunk({ oldStartRow: this.oldStartRow, oldRowCount: this.oldRowCount, @@ -300,17 +300,17 @@ class HunkBuilder { } export function multiFilePatchBuilder() { - return new MultiFilePatchBuilder(new LayeredBuffer()); + return new MultiFilePatchBuilder(new PatchBuffer()); } export function filePatchBuilder() { - return new FilePatchBuilder(new LayeredBuffer()); + return new FilePatchBuilder(new PatchBuffer()); } export function patchBuilder() { - return new PatchBuilder(new LayeredBuffer()); + return new PatchBuilder(new PatchBuffer()); } export function hunkBuilder() { - return new HunkBuilder(new LayeredBuffer()); + return new HunkBuilder(new PatchBuffer()); } From 6df1380f609d9784c9348f1cb29ec36fc565f786 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 17 Jan 2019 14:15:07 -0500 Subject: [PATCH 33/92] Begin an "atomic" (with respect to markers) Modification API --- lib/models/patch/patch-buffer.js | 37 ++++++++++- test/models/patch/patch-buffer.test.js | 86 ++++++++++++++++++++++++++ 2 files changed, 122 insertions(+), 1 deletion(-) create mode 100644 test/models/patch/patch-buffer.test.js diff --git a/lib/models/patch/patch-buffer.js b/lib/models/patch/patch-buffer.js index a6f1d0666f..1ff655824f 100644 --- a/lib/models/patch/patch-buffer.js +++ b/lib/models/patch/patch-buffer.js @@ -1,4 +1,4 @@ -import {TextBuffer} from 'atom'; +import {TextBuffer, Range} from 'atom'; const LAYER_NAMES = ['unchanged', 'addition', 'deletion', 'nonewline', 'hunk', 'patch']; @@ -40,4 +40,39 @@ export default class PatchBuffer { this.layers[layerName].clear(); } } + + createModifier() { + return new Modification(this); + } +} + +class Modification { + constructor(patchBuffer) { + this.patchBuffer = patchBuffer; + this.markerBlueprints = []; + } + + append(text) { + this.patchBuffer.getBuffer().append(text); + } + + appendMarked(text, layerName, markerOpts) { + const start = this.patchBuffer.getBuffer().getEndPosition(); + this.append(text); + const end = this.patchBuffer.getBuffer().getEndPosition(); + this.markerBlueprints.push({layerName, range: new Range(start, end), markerOpts}); + return this; + } + + apply() { + for (const {layerName, range, markerOpts} of this.markerBlueprints) { + const callback = markerOpts.callback; + delete markerOpts.callback; + + const marker = this.patchBuffer.markRange(layerName, range, markerOpts); + if (callback) { + callback(marker); + } + } + } } diff --git a/test/models/patch/patch-buffer.test.js b/test/models/patch/patch-buffer.test.js new file mode 100644 index 0000000000..135f32ba30 --- /dev/null +++ b/test/models/patch/patch-buffer.test.js @@ -0,0 +1,86 @@ +import dedent from 'dedent-js'; + +import PatchBuffer from '../../../lib/models/patch/patch-buffer'; + +describe('PatchBuffer', function() { + let patchBuffer; + + beforeEach(function() { + patchBuffer = new PatchBuffer(); + patchBuffer.getBuffer().setText(TEXT); + }); + + it('has simple accessors', function() { + assert.strictEqual(patchBuffer.getBuffer().getText(), TEXT); + assert.deepEqual(patchBuffer.getInsertionPoint().serialize(), [9, 4]); + }); + + it('creates and finds markers on specified layers', function() { + const patchMarker = patchBuffer.markRange('patch', [[1, 0], [2, 4]]); + const hunkMarker = patchBuffer.markRange('hunk', [[2, 0], [3, 4]]); + + assert.deepEqual(patchBuffer.findMarkers('patch', {}), [patchMarker]); + assert.deepEqual(patchBuffer.findMarkers('hunk', {}), [hunkMarker]); + }); + + it('clears markers from all layers at once', function() { + patchBuffer.markRange('patch', [[0, 0], [0, 4]]); + patchBuffer.markPosition('hunk', [0, 1]); + + patchBuffer.clearAllLayers(); + + assert.lengthOf(patchBuffer.findMarkers('patch', {}), 0); + assert.lengthOf(patchBuffer.findMarkers('hunk', {}), 0); + }); + + describe('deferred-marking modifications', function() { + it('performs multiple modifications and only creates markers at the end', function() { + const modifier = patchBuffer.createModifier(); + const cb0 = sinon.spy(); + const cb1 = sinon.spy(); + + modifier.append('0010\n'); + modifier.appendMarked('0011\n', 'patch', {invalidate: 'never', callback: cb0}); + modifier.append('0012\n'); + modifier.appendMarked('0013\n0014\n', 'hunk', {invalidate: 'surround', callback: cb1}); + + assert.strictEqual(patchBuffer.getBuffer().getText(), dedent` + ${TEXT}0010 + 0011 + 0012 + 0013 + 0014 + + `); + + assert.isFalse(cb0.called); + assert.isFalse(cb1.called); + assert.lengthOf(patchBuffer.findMarkers('patch', {}), 0); + assert.lengthOf(patchBuffer.findMarkers('hunk', {}), 0); + + modifier.apply(); + + assert.lengthOf(patchBuffer.findMarkers('patch', {}), 1); + const [marker0] = patchBuffer.findMarkers('patch', {}); + assert.isTrue(cb0.calledWith(marker0)); + + assert.lengthOf(patchBuffer.findMarkers('hunk', {}), 1); + const [marker1] = patchBuffer.findMarkers('hunk', {}); + assert.isTrue(cb1.calledWith(marker1)); + }); + }); +}); + +const TEXT = dedent` + 0000 + 0001 + 0002 + 0003 + 0004 + 0005 + 0006 + 0007 + 0008 + 0009 + +`; From 3b340999f60a0d7f2973d84cbbab2bc5fe73ab36 Mon Sep 17 00:00:00 2001 From: annthurium Date: Thu, 17 Jan 2019 12:06:06 -0800 Subject: [PATCH 34/92] import LARGE_DIFF_THRESHOLD --- lib/models/patch/builder.js | 2 +- test/models/patch/multi-file-patch.test.js | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/models/patch/builder.js b/lib/models/patch/builder.js index ab8c81109c..af0814e94e 100644 --- a/lib/models/patch/builder.js +++ b/lib/models/patch/builder.js @@ -6,7 +6,7 @@ import {Unchanged, Addition, Deletion, NoNewline} from './region'; import FilePatch from './file-patch'; import MultiFilePatch from './multi-file-patch'; -const DEFAULT_OPTIONS = { +export const DEFAULT_OPTIONS = { // Number of lines after which we consider the diff "large" // TODO: Set this based on performance measurements largeDiffThreshold: 100, diff --git a/test/models/patch/multi-file-patch.test.js b/test/models/patch/multi-file-patch.test.js index 61fa236f7a..b5e665da38 100644 --- a/test/models/patch/multi-file-patch.test.js +++ b/test/models/patch/multi-file-patch.test.js @@ -3,6 +3,7 @@ import dedent from 'dedent-js'; import {multiFilePatchBuilder, filePatchBuilder} from '../../builder/patch'; import {TOO_LARGE} from '../../../lib/models/patch/patch'; +import {DEFAULT_OPTIONS} from '../../../lib/models/patch/builder.js'; import MultiFilePatch from '../../../lib/models/patch/multi-file-patch'; import {assertInFilePatch} from '../../helpers'; @@ -709,6 +710,7 @@ describe('MultiFilePatch', function() { describe('isPatchTooLargeOrCollapsed', function() { let multiFilePatch; + const {largeDiffThreshold} = DEFAULT_OPTIONS; beforeEach(function() { multiFilePatch = multiFilePatchBuilder() .addFilePatch(fp => { From 49b406d5000e2e63ce5f4bca30b6e2bbdb68eb1a Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 17 Jan 2019 14:59:28 -0500 Subject: [PATCH 35/92] Preserve Markers that begin or end at a Modification's insertion point --- lib/models/patch/patch-buffer.js | 57 ++++++++++++++++++++--- test/models/patch/patch-buffer.test.js | 63 +++++++++++++++++++++++++- 2 files changed, 111 insertions(+), 9 deletions(-) diff --git a/lib/models/patch/patch-buffer.js b/lib/models/patch/patch-buffer.js index 1ff655824f..aec6b2493d 100644 --- a/lib/models/patch/patch-buffer.js +++ b/lib/models/patch/patch-buffer.js @@ -1,4 +1,4 @@ -import {TextBuffer, Range} from 'atom'; +import {TextBuffer, Range, Point} from 'atom'; const LAYER_NAMES = ['unchanged', 'addition', 'deletion', 'nonewline', 'hunk', 'patch']; @@ -41,25 +41,52 @@ export default class PatchBuffer { } } - createModifier() { - return new Modification(this); + createModifierAt(insertionPoint) { + return new Modification(this, Point.fromObject(insertionPoint)); + } + + createModifierAtEnd() { + return this.createModifierAt(this.getInsertionPoint()); } } class Modification { - constructor(patchBuffer) { + constructor(patchBuffer, insertionPoint) { this.patchBuffer = patchBuffer; + this.startPoint = insertionPoint.copy(); + this.insertionPoint = insertionPoint; this.markerBlueprints = []; + + this.markersBefore = new Set(); + this.markersAfter = new Set(); + } + + keepBefore(markers) { + for (const marker of markers) { + if (marker.getRange().end.isEqual(this.startPoint)) { + this.markersBefore.add(marker); + } + } + } + + keepAfter(markers) { + for (const marker of markers) { + if (marker.getRange().end.isEqual(this.startPoint)) { + this.markersAfter.add(marker); + } + } } append(text) { - this.patchBuffer.getBuffer().append(text); + const insertedRange = this.patchBuffer.getBuffer().insert(this.insertionPoint, text); + this.insertionPoint = insertedRange.end; + return this; } appendMarked(text, layerName, markerOpts) { - const start = this.patchBuffer.getBuffer().getEndPosition(); + const start = this.insertionPoint.copy(); this.append(text); - const end = this.patchBuffer.getBuffer().getEndPosition(); + const end = this.insertionPoint.copy(); this.markerBlueprints.push({layerName, range: new Range(start, end), markerOpts}); return this; } @@ -74,5 +101,21 @@ class Modification { callback(marker); } } + + for (const beforeMarker of this.markersBefore) { + if (!beforeMarker.isReversed()) { + beforeMarker.setHeadPosition(this.startPoint); + } else { + beforeMarker.setTailPosition(this.startPoint); + } + } + + for (const afterMarker of this.markersAfter) { + if (!afterMarker.isReversed()) { + afterMarker.setTailPosition(this.insertionPoint); + } else { + afterMarker.setHeadPosition(this.insertionPoint); + } + } } } diff --git a/test/models/patch/patch-buffer.test.js b/test/models/patch/patch-buffer.test.js index 135f32ba30..102ed2d409 100644 --- a/test/models/patch/patch-buffer.test.js +++ b/test/models/patch/patch-buffer.test.js @@ -12,7 +12,7 @@ describe('PatchBuffer', function() { it('has simple accessors', function() { assert.strictEqual(patchBuffer.getBuffer().getText(), TEXT); - assert.deepEqual(patchBuffer.getInsertionPoint().serialize(), [9, 4]); + assert.deepEqual(patchBuffer.getInsertionPoint().serialize(), [10, 0]); }); it('creates and finds markers on specified layers', function() { @@ -35,7 +35,7 @@ describe('PatchBuffer', function() { describe('deferred-marking modifications', function() { it('performs multiple modifications and only creates markers at the end', function() { - const modifier = patchBuffer.createModifier(); + const modifier = patchBuffer.createModifierAtEnd(); const cb0 = sinon.spy(); const cb1 = sinon.spy(); @@ -68,6 +68,65 @@ describe('PatchBuffer', function() { const [marker1] = patchBuffer.findMarkers('hunk', {}); assert.isTrue(cb1.calledWith(marker1)); }); + + it('inserts into the middle of an existing buffer', function() { + const modifier = patchBuffer.createModifierAt([4, 2]); + const callback = sinon.spy(); + + modifier.append('aa\nbbbb\n'); + modifier.appendMarked('-patch-\n-patch-\n', 'patch', {callback}); + modifier.appendMarked('-hunk-\ndd', 'hunk', {}); + + assert.strictEqual(patchBuffer.getBuffer().getText(), dedent` + 0000 + 0001 + 0002 + 0003 + 00aa + bbbb + -patch- + -patch- + -hunk- + dd04 + 0005 + 0006 + 0007 + 0008 + 0009 + + `); + + assert.lengthOf(patchBuffer.findMarkers('patch', {}), 0); + assert.lengthOf(patchBuffer.findMarkers('hunk', {}), 0); + assert.isFalse(callback.called); + + modifier.apply(); + + assert.lengthOf(patchBuffer.findMarkers('patch', {}), 1); + const [marker] = patchBuffer.findMarkers('patch', {}); + assert.isTrue(callback.calledWith(marker)); + }); + + it('preserves markers that should be before or after the modification region', function() { + const before0 = patchBuffer.markRange('patch', [[1, 0], [4, 0]]); + const before1 = patchBuffer.markPosition('hunk', [4, 0]); + const after0 = patchBuffer.markPosition('patch', [4, 0]); + + const modifier = patchBuffer.createModifierAt([4, 0]); + modifier.keepBefore([before0, before1]); + modifier.keepAfter([after0]); + + let marker = null; + const callback = m => { marker = m; }; + modifier.appendMarked('A\nB\nC\nD\nE\n', 'addition', {callback}); + + modifier.apply(); + + assert.deepEqual(before0.getRange().serialize(), [[1, 0], [4, 0]]); + assert.deepEqual(before1.getRange().serialize(), [[4, 0], [4, 0]]); + assert.deepEqual(marker.getRange().serialize(), [[4, 0], [9, 0]]); + assert.deepEqual(after0.getRange().serialize(), [[9, 0], [9, 0]]); + }); }); }); From a2d453c41b9742f385dc1a4e03d7c0ad6b4169ca Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 17 Jan 2019 16:12:11 -0500 Subject: [PATCH 36/92] PatchBuffer::extract() to slice a Range out and preserve its markers --- lib/models/patch/patch-buffer.js | 27 ++++++++++++++++ test/models/patch/patch-buffer.test.js | 45 ++++++++++++++++++++++++++ 2 files changed, 72 insertions(+) diff --git a/lib/models/patch/patch-buffer.js b/lib/models/patch/patch-buffer.js index aec6b2493d..d827c97974 100644 --- a/lib/models/patch/patch-buffer.js +++ b/lib/models/patch/patch-buffer.js @@ -48,6 +48,33 @@ export default class PatchBuffer { createModifierAtEnd() { return this.createModifierAt(this.getInsertionPoint()); } + + extract(rangeLike) { + const range = Range.fromObject(rangeLike); + const movedMarkersByLayer = LAYER_NAMES.reduce((map, layerName) => { + map[layerName] = this.layers[layerName].findMarkers({containedInRange: range}); + return map; + }, {}); + const markerMap = new Map(); + + const subBuffer = new PatchBuffer(); + subBuffer.getBuffer().setText(this.buffer.getTextInRange(range)); + + for (const layerName of LAYER_NAMES) { + for (const oldMarker of movedMarkersByLayer[layerName]) { + const newMarker = subBuffer.markRange( + layerName, + oldMarker.getRange().translate(range.start.negate()), + oldMarker.getProperties(), + ); + markerMap.set(oldMarker, newMarker); + oldMarker.destroy(); + } + } + + this.buffer.setTextInRange(range, ''); + return subBuffer; + } } class Modification { diff --git a/test/models/patch/patch-buffer.test.js b/test/models/patch/patch-buffer.test.js index 102ed2d409..c9759fc197 100644 --- a/test/models/patch/patch-buffer.test.js +++ b/test/models/patch/patch-buffer.test.js @@ -33,6 +33,51 @@ describe('PatchBuffer', function() { assert.lengthOf(patchBuffer.findMarkers('hunk', {}), 0); }); + it('extracts a subset of the buffer and layers as a new LayeredBuffer', function() { + patchBuffer.markRange('patch', [[1, 0], [3, 0]]); // before + patchBuffer.markRange('hunk', [[2, 0], [4, 0]]); // before, ending at the extraction point + patchBuffer.markRange('hunk', [[4, 0], [5, 0]]); // within + patchBuffer.markRange('patch', [[6, 0], [7, 0]]); // within + patchBuffer.markRange('hunk', [[7, 0], [9, 0]]); // after, starting at the extraction point + patchBuffer.markRange('patch', [[8, 0], [10, 0]]); // after + + const subPatchBuffer = patchBuffer.extract([[4, 0], [7, 0]]); + + assert.strictEqual(patchBuffer.getBuffer().getText(), dedent` + 0000 + 0001 + 0002 + 0003 + 0007 + 0008 + 0009 + + `); + assert.deepEqual( + patchBuffer.findMarkers('patch', {}).map(m => m.getRange().serialize()), + [[[1, 0], [3, 0]], [[5, 0], [7, 0]]], + ); + assert.deepEqual( + patchBuffer.findMarkers('hunk', {}).map(m => m.getRange().serialize()), + [[[2, 0], [4, 0]], [[4, 0], [6, 0]]], + ); + + assert.strictEqual(subPatchBuffer.getBuffer().getText(), dedent` + 0004 + 0005 + 0006 + + `); + assert.deepEqual( + subPatchBuffer.findMarkers('hunk', {}).map(m => m.getRange().serialize()), + [[[0, 0], [1, 0]]], + ); + assert.deepEqual( + subPatchBuffer.findMarkers('patch', {}).map(m => m.getRange().serialize()), + [[[2, 0], [3, 0]]], + ); + }); + describe('deferred-marking modifications', function() { it('performs multiple modifications and only creates markers at the end', function() { const modifier = patchBuffer.createModifierAtEnd(); From 34e104af8af8e56bc644cf6eb5d5ac47c4999e7b Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 17 Jan 2019 16:29:31 -0500 Subject: [PATCH 37/92] Unit test for inserting one PatchBuffer into another --- test/models/patch/patch-buffer.test.js | 55 ++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/test/models/patch/patch-buffer.test.js b/test/models/patch/patch-buffer.test.js index c9759fc197..5c9f697031 100644 --- a/test/models/patch/patch-buffer.test.js +++ b/test/models/patch/patch-buffer.test.js @@ -172,6 +172,61 @@ describe('PatchBuffer', function() { assert.deepEqual(marker.getRange().serialize(), [[4, 0], [9, 0]]); assert.deepEqual(after0.getRange().serialize(), [[9, 0], [9, 0]]); }); + + it('appends another PatchBuffer at its insertion point', function() { + const subPatchBuffer = new PatchBuffer(); + subPatchBuffer.getBuffer().setText(dedent` + aaaa + bbbb + cc + `); + + subPatchBuffer.markPosition('patch', [0, 0]); + subPatchBuffer.markRange('hunk', [[0, 0], [1, 4]]); + subPatchBuffer.markRange('addition', [[1, 2], [2, 2]]); + + const mBefore = patchBuffer.markRange('deletion', [[0, 0], [2, 0]]); + const mAfter = patchBuffer.markRange('deletion', [[7, 0], [7, 4]]); + + patchBuffer + .createModifierAt([3, 2]) + .appendPatchBuffer(subPatchBuffer) + .apply(); + + assert.strictEqual(patchBuffer.getBuffer().getText(), dedent` + 0000 + 0001 + 0002 + 00aaaa + bbbb + cc03 + 0004 + 0005 + 0006 + 0007 + 0008 + 0009 + + `); + + assert.deepEqual(mBefore.getRange().serialize(), [[0, 0], [2, 0]]); + assert.deepEqual(mAfter.getRange().serialize(), [[9, 0], [9, 4]]); + + assert.deepEqual( + patchBuffer.findMarkers('patch', {}).map(m => m.getRange().serialize()), + [[[3, 2], [3, 2]]], + ); + + assert.deepEqual( + patchBuffer.findMarkers('hunk', {}).map(m => m.getRange().serialize()), + [[[3, 2], [4, 4]]], + ); + + assert.deepEqual( + patchBuffer.findMarkers('addition', {}).map(m => m.getRange().serialize()), + [[[4, 2], [5, 2]]], + ); + }); }); }); From dfc2fee53c5e4a10337a748297b6f9a96227050a Mon Sep 17 00:00:00 2001 From: annthurium Date: Thu, 17 Jan 2019 16:08:03 -0800 Subject: [PATCH 38/92] actually trigger expanding and collapsing from `FilePatchHeaderView` --- lib/models/patch/file-patch.js | 4 ++-- lib/views/file-patch-header-view.js | 15 +++++++++++++-- lib/views/multi-file-patch-view.js | 6 +++++- 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/lib/models/patch/file-patch.js b/lib/models/patch/file-patch.js index 362f630ae2..7c76d6a0e9 100644 --- a/lib/models/patch/file-patch.js +++ b/lib/models/patch/file-patch.js @@ -127,7 +127,7 @@ export default class FilePatch { } } - triggerCollapse() { + triggerCollapse = () => { const nextPatch = this.patch.collapsed(); if (nextPatch !== this.patch) { this.patch = nextPatch; @@ -135,7 +135,7 @@ export default class FilePatch { } } - triggerExpand() { + triggerExpand = () => { const nextPatch = this.patch.expanded(); if (nextPatch !== this.patch) { this.patch = nextPatch; diff --git a/lib/views/file-patch-header-view.js b/lib/views/file-patch-header-view.js index a78b931750..ae10db1fde 100644 --- a/lib/views/file-patch-header-view.js +++ b/lib/views/file-patch-header-view.js @@ -29,9 +29,12 @@ export default class FilePatchHeaderView extends React.Component { // should probably change 'toggleFile' to 'toggleFileStagingStatus' // because the addition of another toggling function makes the old name confusing. toggleFile: PropTypes.func.isRequired, - toggleFileCollapse: PropTypes.func, itemType: ItemTypePropType.isRequired, + + isCollapsed: PropTypes.bool.isRequired, + triggerExpand: PropTypes.func.isRequired, + triggerCollapse: PropTypes.func.isRequired, }; constructor(props) { @@ -53,11 +56,19 @@ export default class FilePatchHeaderView extends React.Component { ); } + togglePatchCollapse = () => { + if (this.props.isCollapsed) { + this.props.triggerExpand(); + } else { + this.props.triggerCollapse(); + } + } + renderCollapseButton() { return ( ); diff --git a/lib/views/multi-file-patch-view.js b/lib/views/multi-file-patch-view.js index 5554ac910c..5bb93e7c7b 100644 --- a/lib/views/multi-file-patch-view.js +++ b/lib/views/multi-file-patch-view.js @@ -22,7 +22,7 @@ import CommitDetailItem from '../items/commit-detail-item'; import IssueishDetailItem from '../items/issueish-detail-item'; import File from '../models/patch/file'; -import {TOO_LARGE} from '../models/patch/patch'; +import {TOO_LARGE, COLLAPSED} from '../models/patch/patch'; const executableText = { [File.modes.NORMAL]: 'non executable', @@ -350,6 +350,10 @@ export default class MultiFilePatchView extends React.Component { diveIntoMirrorPatch={() => this.props.diveIntoMirrorPatch(filePatch)} openFile={() => this.didOpenFile({selectedFilePatch: filePatch})} toggleFile={() => this.props.toggleFile(filePatch)} + + isCollapsed={filePatch.getRenderStatus === TOO_LARGE || filePatch.getRenderStatus === COLLAPSED} + triggerCollapse={filePatch.triggerCollapse} + triggerExpand={filePatch.triggerExpand} /> {this.renderSymlinkChangeMeta(filePatch)} {this.renderExecutableModeChangeMeta(filePatch)} From ebdec242ceaf3860f1ebae2c27387e5763f24c11 Mon Sep 17 00:00:00 2001 From: annthurium Date: Thu, 17 Jan 2019 17:04:27 -0800 Subject: [PATCH 39/92] change the chevron icon direction depending on collapsed prop --- lib/views/file-patch-header-view.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/views/file-patch-header-view.js b/lib/views/file-patch-header-view.js index ae10db1fde..56ba16c466 100644 --- a/lib/views/file-patch-header-view.js +++ b/lib/views/file-patch-header-view.js @@ -65,11 +65,12 @@ export default class FilePatchHeaderView extends React.Component { } renderCollapseButton() { + const icon = this.props.isCollapsed ? 'chevron-up' : 'chevron-down'; return ( ); } From 01b82f0e23cd22fb4afb6c8571a2f789b3a8fd58 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Fri, 18 Jan 2019 13:19:32 -0500 Subject: [PATCH 40/92] Implement appendPatchBuffer() --- lib/models/patch/patch-buffer.js | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/lib/models/patch/patch-buffer.js b/lib/models/patch/patch-buffer.js index d827c97974..7ec517a7a8 100644 --- a/lib/models/patch/patch-buffer.js +++ b/lib/models/patch/patch-buffer.js @@ -118,6 +118,28 @@ class Modification { return this; } + appendPatchBuffer(subPatchBuffer) { + const baseOffset = this.insertionPoint.copy(); + this.append(subPatchBuffer.getBuffer().getText()); + + const subMarkerMap = new Map(); + for (const layerName of LAYER_NAMES) { + for (const oldMarker of subPatchBuffer.findMarkers(layerName, {})) { + const startOffset = oldMarker.getRange().start.row === 0 ? baseOffset : [baseOffset.row, 0]; + const endOffset = oldMarker.getRange().end.row === 0 ? baseOffset : [baseOffset.row, 0]; + + const range = oldMarker.getRange().translate(startOffset, endOffset); + const markerOpts = { + ...oldMarker.getProperties(), + callback: newMarker => { subMarkerMap.set(oldMarker, newMarker); }, + }; + this.markerBlueprints.push({layerName, range, markerOpts}); + } + } + + return this; + } + apply() { for (const {layerName, range, markerOpts} of this.markerBlueprints) { const callback = markerOpts.callback; From 2d7c2f465733717e66c7fca9e7d62267f5a35876 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Fri, 18 Jan 2019 13:50:14 -0500 Subject: [PATCH 41/92] Translate Markers when extracting and appending PatchBuffers midline --- lib/models/patch/patch-buffer.js | 5 ++++- test/models/patch/patch-buffer.test.js | 20 ++++++++++---------- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/lib/models/patch/patch-buffer.js b/lib/models/patch/patch-buffer.js index 7ec517a7a8..d1ed781035 100644 --- a/lib/models/patch/patch-buffer.js +++ b/lib/models/patch/patch-buffer.js @@ -51,6 +51,7 @@ export default class PatchBuffer { extract(rangeLike) { const range = Range.fromObject(rangeLike); + const baseOffset = range.start.negate(); const movedMarkersByLayer = LAYER_NAMES.reduce((map, layerName) => { map[layerName] = this.layers[layerName].findMarkers({containedInRange: range}); return map; @@ -62,9 +63,11 @@ export default class PatchBuffer { for (const layerName of LAYER_NAMES) { for (const oldMarker of movedMarkersByLayer[layerName]) { + const startOffset = oldMarker.getRange().start.row === range.start.row ? baseOffset : [baseOffset.row, 0]; + const endOffset = oldMarker.getRange().end.row === range.start.row ? baseOffset : [baseOffset.row, 0]; const newMarker = subBuffer.markRange( layerName, - oldMarker.getRange().translate(range.start.negate()), + oldMarker.getRange().translate(startOffset, endOffset), oldMarker.getProperties(), ); markerMap.set(oldMarker, newMarker); diff --git a/test/models/patch/patch-buffer.test.js b/test/models/patch/patch-buffer.test.js index 5c9f697031..5256c1c6c6 100644 --- a/test/models/patch/patch-buffer.test.js +++ b/test/models/patch/patch-buffer.test.js @@ -35,20 +35,20 @@ describe('PatchBuffer', function() { it('extracts a subset of the buffer and layers as a new LayeredBuffer', function() { patchBuffer.markRange('patch', [[1, 0], [3, 0]]); // before - patchBuffer.markRange('hunk', [[2, 0], [4, 0]]); // before, ending at the extraction point - patchBuffer.markRange('hunk', [[4, 0], [5, 0]]); // within - patchBuffer.markRange('patch', [[6, 0], [7, 0]]); // within - patchBuffer.markRange('hunk', [[7, 0], [9, 0]]); // after, starting at the extraction point + patchBuffer.markRange('hunk', [[2, 0], [4, 2]]); // before, ending at the extraction point + patchBuffer.markRange('hunk', [[4, 2], [5, 0]]); // within + patchBuffer.markRange('patch', [[6, 0], [7, 1]]); // within + patchBuffer.markRange('hunk', [[7, 1], [9, 0]]); // after, starting at the extraction point patchBuffer.markRange('patch', [[8, 0], [10, 0]]); // after - const subPatchBuffer = patchBuffer.extract([[4, 0], [7, 0]]); + const subPatchBuffer = patchBuffer.extract([[4, 2], [7, 1]]); assert.strictEqual(patchBuffer.getBuffer().getText(), dedent` 0000 0001 0002 0003 - 0007 + 00007 0008 0009 @@ -59,14 +59,14 @@ describe('PatchBuffer', function() { ); assert.deepEqual( patchBuffer.findMarkers('hunk', {}).map(m => m.getRange().serialize()), - [[[2, 0], [4, 0]], [[4, 0], [6, 0]]], + [[[2, 0], [4, 2]], [[4, 2], [6, 0]]], ); assert.strictEqual(subPatchBuffer.getBuffer().getText(), dedent` - 0004 + 04 0005 0006 - + 0 `); assert.deepEqual( subPatchBuffer.findMarkers('hunk', {}).map(m => m.getRange().serialize()), @@ -74,7 +74,7 @@ describe('PatchBuffer', function() { ); assert.deepEqual( subPatchBuffer.findMarkers('patch', {}).map(m => m.getRange().serialize()), - [[[2, 0], [3, 0]]], + [[[2, 0], [3, 1]]], ); }); From 03e03f342d58df1c664dab704e5ccd7d1026a7e6 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Fri, 18 Jan 2019 13:55:26 -0500 Subject: [PATCH 42/92] Renamings --- lib/models/patch/patch-buffer.js | 12 ++++----- test/models/patch/patch-buffer.test.js | 36 +++++++++++++------------- 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/lib/models/patch/patch-buffer.js b/lib/models/patch/patch-buffer.js index d1ed781035..4177d33ef0 100644 --- a/lib/models/patch/patch-buffer.js +++ b/lib/models/patch/patch-buffer.js @@ -41,15 +41,15 @@ export default class PatchBuffer { } } - createModifierAt(insertionPoint) { - return new Modification(this, Point.fromObject(insertionPoint)); + createInserterAt(insertionPoint) { + return new Inserter(this, Point.fromObject(insertionPoint)); } - createModifierAtEnd() { - return this.createModifierAt(this.getInsertionPoint()); + createInserterAtEnd() { + return this.createInserterAt(this.getInsertionPoint()); } - extract(rangeLike) { + extractPatchBuffer(rangeLike) { const range = Range.fromObject(rangeLike); const baseOffset = range.start.negate(); const movedMarkersByLayer = LAYER_NAMES.reduce((map, layerName) => { @@ -80,7 +80,7 @@ export default class PatchBuffer { } } -class Modification { +class Inserter { constructor(patchBuffer, insertionPoint) { this.patchBuffer = patchBuffer; this.startPoint = insertionPoint.copy(); diff --git a/test/models/patch/patch-buffer.test.js b/test/models/patch/patch-buffer.test.js index 5256c1c6c6..3efc44a53c 100644 --- a/test/models/patch/patch-buffer.test.js +++ b/test/models/patch/patch-buffer.test.js @@ -41,7 +41,7 @@ describe('PatchBuffer', function() { patchBuffer.markRange('hunk', [[7, 1], [9, 0]]); // after, starting at the extraction point patchBuffer.markRange('patch', [[8, 0], [10, 0]]); // after - const subPatchBuffer = patchBuffer.extract([[4, 2], [7, 1]]); + const subPatchBuffer = patchBuffer.extractPatchBuffer([[4, 2], [7, 1]]); assert.strictEqual(patchBuffer.getBuffer().getText(), dedent` 0000 @@ -80,14 +80,14 @@ describe('PatchBuffer', function() { describe('deferred-marking modifications', function() { it('performs multiple modifications and only creates markers at the end', function() { - const modifier = patchBuffer.createModifierAtEnd(); + const inserter = patchBuffer.createInserterAtEnd(); const cb0 = sinon.spy(); const cb1 = sinon.spy(); - modifier.append('0010\n'); - modifier.appendMarked('0011\n', 'patch', {invalidate: 'never', callback: cb0}); - modifier.append('0012\n'); - modifier.appendMarked('0013\n0014\n', 'hunk', {invalidate: 'surround', callback: cb1}); + inserter.append('0010\n'); + inserter.appendMarked('0011\n', 'patch', {invalidate: 'never', callback: cb0}); + inserter.append('0012\n'); + inserter.appendMarked('0013\n0014\n', 'hunk', {invalidate: 'surround', callback: cb1}); assert.strictEqual(patchBuffer.getBuffer().getText(), dedent` ${TEXT}0010 @@ -103,7 +103,7 @@ describe('PatchBuffer', function() { assert.lengthOf(patchBuffer.findMarkers('patch', {}), 0); assert.lengthOf(patchBuffer.findMarkers('hunk', {}), 0); - modifier.apply(); + inserter.apply(); assert.lengthOf(patchBuffer.findMarkers('patch', {}), 1); const [marker0] = patchBuffer.findMarkers('patch', {}); @@ -115,12 +115,12 @@ describe('PatchBuffer', function() { }); it('inserts into the middle of an existing buffer', function() { - const modifier = patchBuffer.createModifierAt([4, 2]); + const inserter = patchBuffer.createInserterAt([4, 2]); const callback = sinon.spy(); - modifier.append('aa\nbbbb\n'); - modifier.appendMarked('-patch-\n-patch-\n', 'patch', {callback}); - modifier.appendMarked('-hunk-\ndd', 'hunk', {}); + inserter.append('aa\nbbbb\n'); + inserter.appendMarked('-patch-\n-patch-\n', 'patch', {callback}); + inserter.appendMarked('-hunk-\ndd', 'hunk', {}); assert.strictEqual(patchBuffer.getBuffer().getText(), dedent` 0000 @@ -145,7 +145,7 @@ describe('PatchBuffer', function() { assert.lengthOf(patchBuffer.findMarkers('hunk', {}), 0); assert.isFalse(callback.called); - modifier.apply(); + inserter.apply(); assert.lengthOf(patchBuffer.findMarkers('patch', {}), 1); const [marker] = patchBuffer.findMarkers('patch', {}); @@ -157,15 +157,15 @@ describe('PatchBuffer', function() { const before1 = patchBuffer.markPosition('hunk', [4, 0]); const after0 = patchBuffer.markPosition('patch', [4, 0]); - const modifier = patchBuffer.createModifierAt([4, 0]); - modifier.keepBefore([before0, before1]); - modifier.keepAfter([after0]); + const inserter = patchBuffer.createInserterAt([4, 0]); + inserter.keepBefore([before0, before1]); + inserter.keepAfter([after0]); let marker = null; const callback = m => { marker = m; }; - modifier.appendMarked('A\nB\nC\nD\nE\n', 'addition', {callback}); + inserter.appendMarked('A\nB\nC\nD\nE\n', 'addition', {callback}); - modifier.apply(); + inserter.apply(); assert.deepEqual(before0.getRange().serialize(), [[1, 0], [4, 0]]); assert.deepEqual(before1.getRange().serialize(), [[4, 0], [4, 0]]); @@ -189,7 +189,7 @@ describe('PatchBuffer', function() { const mAfter = patchBuffer.markRange('deletion', [[7, 0], [7, 4]]); patchBuffer - .createModifierAt([3, 2]) + .createInserterAt([3, 2]) .appendPatchBuffer(subPatchBuffer) .apply(); From 400d1b821d94dc49fb844cbd9dd7c4bf9a9e49b2 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Fri, 18 Jan 2019 13:57:30 -0500 Subject: [PATCH 43/92] Rename Inserter methods as "insert..." instead of "append..." --- lib/models/patch/patch-buffer.js | 10 +++++----- test/models/patch/patch-buffer.test.js | 18 +++++++++--------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/lib/models/patch/patch-buffer.js b/lib/models/patch/patch-buffer.js index 4177d33ef0..4852742825 100644 --- a/lib/models/patch/patch-buffer.js +++ b/lib/models/patch/patch-buffer.js @@ -107,23 +107,23 @@ class Inserter { } } - append(text) { + insert(text) { const insertedRange = this.patchBuffer.getBuffer().insert(this.insertionPoint, text); this.insertionPoint = insertedRange.end; return this; } - appendMarked(text, layerName, markerOpts) { + insertMarked(text, layerName, markerOpts) { const start = this.insertionPoint.copy(); - this.append(text); + this.insert(text); const end = this.insertionPoint.copy(); this.markerBlueprints.push({layerName, range: new Range(start, end), markerOpts}); return this; } - appendPatchBuffer(subPatchBuffer) { + insertPatchBuffer(subPatchBuffer) { const baseOffset = this.insertionPoint.copy(); - this.append(subPatchBuffer.getBuffer().getText()); + this.insert(subPatchBuffer.getBuffer().getText()); const subMarkerMap = new Map(); for (const layerName of LAYER_NAMES) { diff --git a/test/models/patch/patch-buffer.test.js b/test/models/patch/patch-buffer.test.js index 3efc44a53c..570143f87d 100644 --- a/test/models/patch/patch-buffer.test.js +++ b/test/models/patch/patch-buffer.test.js @@ -84,10 +84,10 @@ describe('PatchBuffer', function() { const cb0 = sinon.spy(); const cb1 = sinon.spy(); - inserter.append('0010\n'); - inserter.appendMarked('0011\n', 'patch', {invalidate: 'never', callback: cb0}); - inserter.append('0012\n'); - inserter.appendMarked('0013\n0014\n', 'hunk', {invalidate: 'surround', callback: cb1}); + inserter.insert('0010\n'); + inserter.insertMarked('0011\n', 'patch', {invalidate: 'never', callback: cb0}); + inserter.insert('0012\n'); + inserter.insertMarked('0013\n0014\n', 'hunk', {invalidate: 'surround', callback: cb1}); assert.strictEqual(patchBuffer.getBuffer().getText(), dedent` ${TEXT}0010 @@ -118,9 +118,9 @@ describe('PatchBuffer', function() { const inserter = patchBuffer.createInserterAt([4, 2]); const callback = sinon.spy(); - inserter.append('aa\nbbbb\n'); - inserter.appendMarked('-patch-\n-patch-\n', 'patch', {callback}); - inserter.appendMarked('-hunk-\ndd', 'hunk', {}); + inserter.insert('aa\nbbbb\n'); + inserter.insertMarked('-patch-\n-patch-\n', 'patch', {callback}); + inserter.insertMarked('-hunk-\ndd', 'hunk', {}); assert.strictEqual(patchBuffer.getBuffer().getText(), dedent` 0000 @@ -163,7 +163,7 @@ describe('PatchBuffer', function() { let marker = null; const callback = m => { marker = m; }; - inserter.appendMarked('A\nB\nC\nD\nE\n', 'addition', {callback}); + inserter.insertMarked('A\nB\nC\nD\nE\n', 'addition', {callback}); inserter.apply(); @@ -190,7 +190,7 @@ describe('PatchBuffer', function() { patchBuffer .createInserterAt([3, 2]) - .appendPatchBuffer(subPatchBuffer) + .insertPatchBuffer(subPatchBuffer) .apply(); assert.strictEqual(patchBuffer.getBuffer().getText(), dedent` From 37f9ae6cb1f991f5119b7c4767b06ce1750e0783 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Fri, 18 Jan 2019 14:13:53 -0500 Subject: [PATCH 44/92] Produce Maps of equivalent Markers after extract/insert PatchBuffers --- lib/models/patch/patch-buffer.js | 8 +++-- test/models/patch/patch-buffer.test.js | 44 ++++++++++++++++++-------- 2 files changed, 37 insertions(+), 15 deletions(-) diff --git a/lib/models/patch/patch-buffer.js b/lib/models/patch/patch-buffer.js index 4852742825..19c7979c7d 100644 --- a/lib/models/patch/patch-buffer.js +++ b/lib/models/patch/patch-buffer.js @@ -76,7 +76,7 @@ export default class PatchBuffer { } this.buffer.setTextInRange(range, ''); - return subBuffer; + return {patchBuffer: subBuffer, markerMap}; } } @@ -121,7 +121,7 @@ class Inserter { return this; } - insertPatchBuffer(subPatchBuffer) { + insertPatchBuffer(subPatchBuffer, opts) { const baseOffset = this.insertionPoint.copy(); this.insert(subPatchBuffer.getBuffer().getText()); @@ -140,6 +140,10 @@ class Inserter { } } + if (opts.callback) { + opts.callback(subMarkerMap); + } + return this; } diff --git a/test/models/patch/patch-buffer.test.js b/test/models/patch/patch-buffer.test.js index 570143f87d..75a7de3b2e 100644 --- a/test/models/patch/patch-buffer.test.js +++ b/test/models/patch/patch-buffer.test.js @@ -34,14 +34,14 @@ describe('PatchBuffer', function() { }); it('extracts a subset of the buffer and layers as a new LayeredBuffer', function() { - patchBuffer.markRange('patch', [[1, 0], [3, 0]]); // before - patchBuffer.markRange('hunk', [[2, 0], [4, 2]]); // before, ending at the extraction point - patchBuffer.markRange('hunk', [[4, 2], [5, 0]]); // within - patchBuffer.markRange('patch', [[6, 0], [7, 1]]); // within - patchBuffer.markRange('hunk', [[7, 1], [9, 0]]); // after, starting at the extraction point - patchBuffer.markRange('patch', [[8, 0], [10, 0]]); // after + const m0 = patchBuffer.markRange('patch', [[1, 0], [3, 0]]); // before + const m1 = patchBuffer.markRange('hunk', [[2, 0], [4, 2]]); // before, ending at the extraction point + const m2 = patchBuffer.markRange('hunk', [[4, 2], [5, 0]]); // within + const m3 = patchBuffer.markRange('patch', [[6, 0], [7, 1]]); // within + const m4 = patchBuffer.markRange('hunk', [[7, 1], [9, 0]]); // after, starting at the extraction point + const m5 = patchBuffer.markRange('patch', [[8, 0], [10, 0]]); // after - const subPatchBuffer = patchBuffer.extractPatchBuffer([[4, 2], [7, 1]]); + const {patchBuffer: subPatchBuffer, markerMap} = patchBuffer.extractPatchBuffer([[4, 2], [7, 1]]); assert.strictEqual(patchBuffer.getBuffer().getText(), dedent` 0000 @@ -61,6 +61,17 @@ describe('PatchBuffer', function() { patchBuffer.findMarkers('hunk', {}).map(m => m.getRange().serialize()), [[[2, 0], [4, 2]], [[4, 2], [6, 0]]], ); + assert.deepEqual(m0.getRange().serialize(), [[1, 0], [3, 0]]); + assert.deepEqual(m1.getRange().serialize(), [[2, 0], [4, 2]]); + assert.isTrue(m2.isDestroyed()); + assert.isTrue(m3.isDestroyed()); + assert.deepEqual(m4.getRange().serialize(), [[4, 2], [6, 0]]); + assert.deepEqual(m5.getRange().serialize(), [[5, 0], [7, 0]]); + + assert.isFalse(markerMap.has(m0)); + assert.isFalse(markerMap.has(m1)); + assert.isFalse(markerMap.has(m4)); + assert.isFalse(markerMap.has(m5)); assert.strictEqual(subPatchBuffer.getBuffer().getText(), dedent` 04 @@ -76,6 +87,8 @@ describe('PatchBuffer', function() { subPatchBuffer.findMarkers('patch', {}).map(m => m.getRange().serialize()), [[[2, 0], [3, 1]]], ); + assert.deepEqual(markerMap.get(m2).getRange().serialize(), [[0, 0], [1, 0]]); + assert.deepEqual(markerMap.get(m3).getRange().serialize(), [[2, 0], [3, 1]]); }); describe('deferred-marking modifications', function() { @@ -181,16 +194,17 @@ describe('PatchBuffer', function() { cc `); - subPatchBuffer.markPosition('patch', [0, 0]); - subPatchBuffer.markRange('hunk', [[0, 0], [1, 4]]); - subPatchBuffer.markRange('addition', [[1, 2], [2, 2]]); + const m0 = subPatchBuffer.markPosition('patch', [0, 0]); + const m1 = subPatchBuffer.markRange('hunk', [[0, 0], [1, 4]]); + const m2 = subPatchBuffer.markRange('addition', [[1, 2], [2, 2]]); const mBefore = patchBuffer.markRange('deletion', [[0, 0], [2, 0]]); const mAfter = patchBuffer.markRange('deletion', [[7, 0], [7, 4]]); + let markerMap; patchBuffer .createInserterAt([3, 2]) - .insertPatchBuffer(subPatchBuffer) + .insertPatchBuffer(subPatchBuffer, {callback: m => { markerMap = m; }}) .apply(); assert.strictEqual(patchBuffer.getBuffer().getText(), dedent` @@ -211,21 +225,25 @@ describe('PatchBuffer', function() { assert.deepEqual(mBefore.getRange().serialize(), [[0, 0], [2, 0]]); assert.deepEqual(mAfter.getRange().serialize(), [[9, 0], [9, 4]]); + assert.isFalse(markerMap.has(mBefore)); + assert.isFalse(markerMap.has(mAfter)); assert.deepEqual( patchBuffer.findMarkers('patch', {}).map(m => m.getRange().serialize()), [[[3, 2], [3, 2]]], ); - assert.deepEqual( patchBuffer.findMarkers('hunk', {}).map(m => m.getRange().serialize()), [[[3, 2], [4, 4]]], ); - assert.deepEqual( patchBuffer.findMarkers('addition', {}).map(m => m.getRange().serialize()), [[[4, 2], [5, 2]]], ); + + assert.deepEqual(markerMap.get(m0).getRange().serialize(), [[3, 2], [3, 2]]); + assert.deepEqual(markerMap.get(m1).getRange().serialize(), [[3, 2], [4, 4]]); + assert.deepEqual(markerMap.get(m2).getRange().serialize(), [[4, 2], [5, 2]]); }); }); }); From 498c7200566a5b5f013bb428b829d8819774a1bd Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Fri, 18 Jan 2019 15:57:09 -0500 Subject: [PATCH 45/92] Inserter::markWhile() to mark all changes made in a block --- lib/models/patch/patch-buffer.js | 14 +++++++++----- test/models/patch/patch-buffer.test.js | 17 +++++++++++++---- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/lib/models/patch/patch-buffer.js b/lib/models/patch/patch-buffer.js index 19c7979c7d..18801b14a1 100644 --- a/lib/models/patch/patch-buffer.js +++ b/lib/models/patch/patch-buffer.js @@ -107,6 +107,14 @@ class Inserter { } } + markWhile(layerName, block, markerOpts) { + const start = this.insertionPoint.copy(); + block(); + const end = this.insertionPoint.copy(); + this.markerBlueprints.push({layerName, range: new Range(start, end), markerOpts}); + return this; + } + insert(text) { const insertedRange = this.patchBuffer.getBuffer().insert(this.insertionPoint, text); this.insertionPoint = insertedRange.end; @@ -114,11 +122,7 @@ class Inserter { } insertMarked(text, layerName, markerOpts) { - const start = this.insertionPoint.copy(); - this.insert(text); - const end = this.insertionPoint.copy(); - this.markerBlueprints.push({layerName, range: new Range(start, end), markerOpts}); - return this; + return this.markWhile(layerName, () => this.insert(text), markerOpts); } insertPatchBuffer(subPatchBuffer, opts) { diff --git a/test/models/patch/patch-buffer.test.js b/test/models/patch/patch-buffer.test.js index 75a7de3b2e..95c65fe364 100644 --- a/test/models/patch/patch-buffer.test.js +++ b/test/models/patch/patch-buffer.test.js @@ -96,11 +96,14 @@ describe('PatchBuffer', function() { const inserter = patchBuffer.createInserterAtEnd(); const cb0 = sinon.spy(); const cb1 = sinon.spy(); + const cb2 = sinon.spy(); - inserter.insert('0010\n'); - inserter.insertMarked('0011\n', 'patch', {invalidate: 'never', callback: cb0}); - inserter.insert('0012\n'); - inserter.insertMarked('0013\n0014\n', 'hunk', {invalidate: 'surround', callback: cb1}); + inserter.markWhile('addition', () => { + inserter.insert('0010\n'); + inserter.insertMarked('0011\n', 'patch', {invalidate: 'never', callback: cb0}); + inserter.insert('0012\n'); + inserter.insertMarked('0013\n0014\n', 'hunk', {invalidate: 'surround', callback: cb1}); + }, {invalidate: 'never', callback: cb2}); assert.strictEqual(patchBuffer.getBuffer().getText(), dedent` ${TEXT}0010 @@ -113,6 +116,8 @@ describe('PatchBuffer', function() { assert.isFalse(cb0.called); assert.isFalse(cb1.called); + assert.isFalse(cb2.called); + assert.lengthOf(patchBuffer.findMarkers('addition', {}), 0); assert.lengthOf(patchBuffer.findMarkers('patch', {}), 0); assert.lengthOf(patchBuffer.findMarkers('hunk', {}), 0); @@ -125,6 +130,10 @@ describe('PatchBuffer', function() { assert.lengthOf(patchBuffer.findMarkers('hunk', {}), 1); const [marker1] = patchBuffer.findMarkers('hunk', {}); assert.isTrue(cb1.calledWith(marker1)); + + assert.lengthOf(patchBuffer.findMarkers('addition', {}), 1); + const [marker2] = patchBuffer.findMarkers('addition', {}); + assert.isTrue(cb2.calledWith(marker2)); }); it('inserts into the middle of an existing buffer', function() { From 5c8181e44c6f7eac977254b64bd9701c8906046f Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Fri, 18 Jan 2019 16:45:16 -0500 Subject: [PATCH 46/92] Rewrite buildHunks() to use an Inserter --- lib/models/patch/builder.js | 149 +++++++++++++++--------------------- 1 file changed, 63 insertions(+), 86 deletions(-) diff --git a/lib/models/patch/builder.js b/lib/models/patch/builder.js index af0814e94e..a2de6603b3 100644 --- a/lib/models/patch/builder.js +++ b/lib/models/patch/builder.js @@ -195,94 +195,71 @@ const CHANGEKIND = { '\\': NoNewline, }; -function buildHunks(diff, patchBuffer, insertRow, existingMarker = null) { - const hunks = []; - - const patchStartRow = insertRow; - let bufferRow = patchStartRow; - let nextLineLength = 0; - - if (diff.hunks.length === 0) { - const patchMarker = patchBuffer.markPosition( - Patch.layerName, - [patchStartRow, 0], - {invalidate: 'never', exclusive: true}, - ); - - return [hunks, patchMarker]; - } - - for (const hunkData of diff.hunks) { - const bufferStartRow = bufferRow; - - const regions = []; - - let LastChangeKind = null; - let currentRangeStart = bufferRow; - let lastLineLength = 0; - - const finishCurrentRange = () => { - if (currentRangeStart === bufferRow) { - return; - } +function buildHunks(diff, patchBuffer, insertRow) { + const inserter = patchBuffer.createInserterAt([insertRow, 0]); - regions.push( - new LastChangeKind( - patchBuffer.markRange( - LastChangeKind.layerName, - [[currentRangeStart, 0], [bufferRow - 1, lastLineLength]], - {invalidate: 'never', exclusive: false}, - ), - ), - ); - currentRangeStart = bufferRow; - }; - - for (const lineText of hunkData.lines) { - const bufferLine = lineText.slice(1) + '\n'; - nextLineLength = lineText.length - 1; - patchBuffer.getBuffer().insert([bufferRow, 0], bufferLine); - - const ChangeKind = CHANGEKIND[lineText[0]]; - if (ChangeKind === undefined) { - throw new Error(`Unknown diff status character: "${lineText[0]}"`); - } - - if (ChangeKind !== LastChangeKind) { - finishCurrentRange(); - } - - LastChangeKind = ChangeKind; - bufferRow++; - lastLineLength = nextLineLength; + let patchMarker = null; + const hunks = []; + inserter.markWhile(Patch.layerName, () => { + for (const rawHunk of diff.hunks) { + const regions = []; + + inserter.markWhile(Hunk.layerName, () => { + let currentRegionText = ''; + let CurrentRegionKind = null; + + function finishRegion() { + if (currentRegionText === '' || CurrentRegionKind === null) { + return; + } + + inserter.insertMarked(currentRegionText, CurrentRegionKind.layerName, { + invalidate: 'never', + exclusive: false, + callback: regionMarker => { regions.push(new CurrentRegionKind(regionMarker)); }, + }); + } + + for (const rawLine of rawHunk.lines) { + const NextRegionKind = CHANGEKIND[rawLine[0]]; + if (NextRegionKind === undefined) { + throw new Error(`Unknown diff status character: "${rawLine[0]}"`); + } + const nextLine = rawLine.slice(1) + '\n'; + + if (NextRegionKind === CurrentRegionKind) { + currentRegionText += nextLine; + continue; + } else { + finishRegion(); + + CurrentRegionKind = NextRegionKind; + currentRegionText = nextLine; + } + } + finishRegion(); + }, { + invalidate: 'never', + exclusive: false, + callback: hunkMarker => { + hunks.push(new Hunk({ + oldStartRow: rawHunk.oldStartLine, + newStartRow: rawHunk.newStartLine, + oldRowCount: rawHunk.oldLineCount, + newRowCount: rawHunk.newLineCount, + sectionHeading: rawHunk.heading, + marker: hunkMarker, + regions, + })); + }, + }); } - finishCurrentRange(); - - hunks.push(new Hunk({ - oldStartRow: hunkData.oldStartLine, - newStartRow: hunkData.newStartLine, - oldRowCount: hunkData.oldLineCount, - newRowCount: hunkData.newLineCount, - sectionHeading: hunkData.heading, - marker: patchBuffer.markRange( - Hunk.layerName, - [[bufferStartRow, 0], [bufferRow - 1, nextLineLength]], - {invalidate: 'never', exclusive: false}, - ), - regions, - })); - } - - let patchMarker = existingMarker; - if (patchMarker) { - patchMarker.setRange([[patchStartRow, 0], [bufferRow - 1, nextLineLength]], {exclusive: false}); - } else { - patchMarker = patchBuffer.markRange( - Patch.layerName, - [[patchStartRow, 0], [bufferRow - 1, nextLineLength]], - {invalidate: 'never', exclusive: false}, - ); - } + }, { + invalidate: 'never', + exclusive: false, + callback: marker => { patchMarker = marker; }, + }); + inserter.apply(); return [hunks, patchMarker]; } From 1a74e83b02712812ce1cd776d7ffa99860765d74 Mon Sep 17 00:00:00 2001 From: annthurium Date: Tue, 22 Jan 2019 11:45:55 -0800 Subject: [PATCH 47/92] [wip] make patch model use `nextLayeredBuffer` not sure why getBuffer() sometimes does not work, but nextLayeredBuffer.buffer does... --- lib/models/patch/patch.js | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/lib/models/patch/patch.js b/lib/models/patch/patch.js index 129e23cfd3..d5c5982ccd 100644 --- a/lib/models/patch/patch.js +++ b/lib/models/patch/patch.js @@ -192,9 +192,9 @@ export default class Patch { } } - const marker = nextLayeredBuffer.markRange( + const marker = nextLayeredBuffer.buffer.markRange( this.constructor.layerName, - [[0, 0], [nextLayeredBuffer.getBuffer().getLastRow() - 1, Infinity]], + [[0, 0], [nextLayeredBuffer.buffer.getLastRow() - 1, Infinity]], {invalidate: 'never', exclusive: false}, ); @@ -427,7 +427,7 @@ class BufferBuilder { // The ranges provided to builder methods are expected to be valid within the original buffer. Account for // the position of the Patch within its original TextBuffer, and any existing content already on the next // TextBuffer. - this.offset = this.buffer.getLastRow() - originalBaseOffset; + this.offset = this.nextLayeredBuffer.buffer.getLastRow() - originalBaseOffset; this.hunkBufferText = ''; this.hunkRowCount = 0; @@ -470,15 +470,13 @@ class BufferBuilder { } latestHunkWasIncluded() { - this.nextLayeredBuffer.getBuffer().append(this.hunkBufferText, {normalizeLineEndings: false}); + this.nextLayeredBuffer.buffer.append(this.hunkBufferText, {normalizeLineEndings: false}); const regions = this.hunkRegions.map(({RegionKind, range}) => { - return new RegionKind( - this.nextLayeredBuffer.getLayer(RegionKind.layerName, range, {invalidate: 'never', exclusive: false}), - ); + return new RegionKind(this.nextLayeredBuffer.layers[RegionKind.layerName]); }); - const marker = this.nextLayeredBuffer.markRange('hunk', this.hunkRange, {invalidate: 'never', exclusive: false}); + const marker = this.nextLayeredBuffer.buffer.markRange('hunk', this.hunkRange, {invalidate: 'never', exclusive: false}); this.hunkBufferText = ''; this.hunkRowCount = 0; From 510c88766daaaa511f7e7e9f688b3ebfdbecc1ff Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 22 Jan 2019 15:25:02 -0500 Subject: [PATCH 48/92] PatchBuffer::inspect() to debug marker placement --- lib/models/patch/patch-buffer.js | 44 ++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/lib/models/patch/patch-buffer.js b/lib/models/patch/patch-buffer.js index 18801b14a1..e5dca106e7 100644 --- a/lib/models/patch/patch-buffer.js +++ b/lib/models/patch/patch-buffer.js @@ -1,4 +1,5 @@ import {TextBuffer, Range, Point} from 'atom'; +import {inspect} from 'util'; const LAYER_NAMES = ['unchanged', 'addition', 'deletion', 'nonewline', 'hunk', 'patch']; @@ -78,6 +79,49 @@ export default class PatchBuffer { this.buffer.setTextInRange(range, ''); return {patchBuffer: subBuffer, markerMap}; } + + inspect() { + let inspectString = ''; + + const increasingMarkers = []; + for (const layerName of LAYER_NAMES) { + for (const marker of this.findMarkers(layerName, {})) { + increasingMarkers.push({layerName, point: marker.getRange().start, start: true, id: marker.id}); + increasingMarkers.push({layerName, point: marker.getRange().end, end: true, id: marker.id}); + } + } + increasingMarkers.sort((a, b) => { + const cmp = a.point.compare(b.point); + if (cmp !== 0) { + return cmp; + } else if (a.start && b.start) { + return 0; + } else if (a.start && !b.start) { + return -1; + } else if (!a.start && b.start) { + return 1; + } else { + return 0; + } + }); + + let inspectPoint = Point.fromObject([0, 0]); + for (const marker of increasingMarkers) { + if (!marker.point.isEqual(inspectPoint)) { + inspectString += inspect(this.buffer.getTextInRange([inspectPoint, marker.point])) + '\n'; + } + + if (marker.start) { + inspectString += ` start ${marker.layerName}@${marker.id}\n`; + } else if (marker.end) { + inspectString += ` end ${marker.layerName}@${marker.id}\n`; + } + + inspectPoint = marker.point; + } + + return inspectString; + } } class Inserter { From 18ff065dfbe6935499a8e531ff67065149078812 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 22 Jan 2019 15:34:10 -0500 Subject: [PATCH 49/92] PatchBuffer::findAllMarkers() to query markers across all layers --- lib/models/patch/patch-buffer.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/models/patch/patch-buffer.js b/lib/models/patch/patch-buffer.js index e5dca106e7..834b3e785a 100644 --- a/lib/models/patch/patch-buffer.js +++ b/lib/models/patch/patch-buffer.js @@ -28,6 +28,13 @@ export default class PatchBuffer { return this.layers[layerName].findMarkers(...args); } + findAllMarkers(...args) { + return LAYER_NAMES.reduce((arr, layerName) => { + arr.push(...this.findMarkers(layerName, ...args)); + return arr; + }, []); + } + markPosition(layerName, ...args) { return this.layers[layerName].markPosition(...args); } From 5283be0ba884d4b628133b32d058c6c4fcbc2ef3 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 22 Jan 2019 15:34:24 -0500 Subject: [PATCH 50/92] Clip insertion point to valid buffer positions --- lib/models/patch/patch-buffer.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/models/patch/patch-buffer.js b/lib/models/patch/patch-buffer.js index 834b3e785a..2ce96fe416 100644 --- a/lib/models/patch/patch-buffer.js +++ b/lib/models/patch/patch-buffer.js @@ -133,9 +133,11 @@ export default class PatchBuffer { class Inserter { constructor(patchBuffer, insertionPoint) { + const clipped = patchBuffer.getBuffer().clipPosition(insertionPoint); + this.patchBuffer = patchBuffer; - this.startPoint = insertionPoint.copy(); - this.insertionPoint = insertionPoint; + this.startPoint = clipped.copy(); + this.insertionPoint = clipped.copy(); this.markerBlueprints = []; this.markersBefore = new Set(); From c56becb6b1f56cc263a182fa45d441e04b7e37e3 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 22 Jan 2019 15:34:30 -0500 Subject: [PATCH 51/92] Fluent APIs --- lib/models/patch/patch-buffer.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/models/patch/patch-buffer.js b/lib/models/patch/patch-buffer.js index 2ce96fe416..936e9a7735 100644 --- a/lib/models/patch/patch-buffer.js +++ b/lib/models/patch/patch-buffer.js @@ -150,6 +150,7 @@ class Inserter { this.markersBefore.add(marker); } } + return this; } keepAfter(markers) { @@ -158,6 +159,7 @@ class Inserter { this.markersAfter.add(marker); } } + return this; } markWhile(layerName, block, markerOpts) { From d6e5947b5cf7e38f0556e7cee374806e279cd833 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 22 Jan 2019 15:34:53 -0500 Subject: [PATCH 52/92] No need to manually touch up patch/hunk markers --- lib/models/patch/builder.js | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/lib/models/patch/builder.js b/lib/models/patch/builder.js index a2de6603b3..bcf95bd6fb 100644 --- a/lib/models/patch/builder.js +++ b/lib/models/patch/builder.js @@ -71,16 +71,6 @@ export function buildMultiFilePatch(diffs, options) { } const filePatches = actions.map(action => action()); - - // Fix markers for patches with no hunks. - // Head position was moved everytime lines were appended. - filePatches.forEach(filePatch => { - if (filePatch.getHunks().length === 0) { - const marker = filePatch.getMarker(); - marker.setHeadPosition(marker.getTailPosition()); - } - }); - return new MultiFilePatch({patchBuffer, filePatches}); } From 4c39a4bb83e5f535a36bb69800702596bfc9c9c2 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 22 Jan 2019 15:36:12 -0500 Subject: [PATCH 53/92] Insert newlines *between* consecutive patches, hunks, regions --- lib/models/patch/builder.js | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/lib/models/patch/builder.js b/lib/models/patch/builder.js index bcf95bd6fb..4caf56db52 100644 --- a/lib/models/patch/builder.js +++ b/lib/models/patch/builder.js @@ -189,11 +189,28 @@ function buildHunks(diff, patchBuffer, insertRow) { const inserter = patchBuffer.createInserterAt([insertRow, 0]); let patchMarker = null; + let firstHunk = true; const hunks = []; + + // Separate multiple patches on the same buffer with an unmarked newline + if (patchBuffer.getBuffer().getLength() > 0) { + console.log('inserting newline to separate Patches'); + inserter.insert('\n'); + } + inserter.markWhile(Patch.layerName, () => { for (const rawHunk of diff.hunks) { + let firstRegion = true; const regions = []; + // Separate hunks with an unmarked newline + if (firstHunk) { + firstHunk = false; + } else { + console.log('inserting newline to separate Hunks'); + inserter.insert('\n'); + } + inserter.markWhile(Hunk.layerName, () => { let currentRegionText = ''; let CurrentRegionKind = null; @@ -203,6 +220,14 @@ function buildHunks(diff, patchBuffer, insertRow) { return; } + // Separate regions with an unmarked newline + if (firstRegion) { + firstRegion = false; + } else { + console.log('inserting newline to separate Regions'); + inserter.insert('\n'); + } + inserter.insertMarked(currentRegionText, CurrentRegionKind.layerName, { invalidate: 'never', exclusive: false, @@ -215,9 +240,12 @@ function buildHunks(diff, patchBuffer, insertRow) { if (NextRegionKind === undefined) { throw new Error(`Unknown diff status character: "${rawLine[0]}"`); } - const nextLine = rawLine.slice(1) + '\n'; + const nextLine = rawLine.slice(1); if (NextRegionKind === CurrentRegionKind) { + if (currentRegionText !== '') { + currentRegionText += '\n'; + } currentRegionText += nextLine; continue; } else { From 62951be35ecd5f24a47dda299ad14ec887c17f8b Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 22 Jan 2019 15:36:41 -0500 Subject: [PATCH 54/92] Need IIFEs for callbacks created within loops --- lib/models/patch/builder.js | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/lib/models/patch/builder.js b/lib/models/patch/builder.js index 4caf56db52..5a57832296 100644 --- a/lib/models/patch/builder.js +++ b/lib/models/patch/builder.js @@ -231,7 +231,9 @@ function buildHunks(diff, patchBuffer, insertRow) { inserter.insertMarked(currentRegionText, CurrentRegionKind.layerName, { invalidate: 'never', exclusive: false, - callback: regionMarker => { regions.push(new CurrentRegionKind(regionMarker)); }, + callback: (function(_regions, _CurrentRegionKind) { + return regionMarker => { _regions.push(new _CurrentRegionKind(regionMarker)); }; + })(regions, CurrentRegionKind), }); } @@ -259,17 +261,19 @@ function buildHunks(diff, patchBuffer, insertRow) { }, { invalidate: 'never', exclusive: false, - callback: hunkMarker => { - hunks.push(new Hunk({ - oldStartRow: rawHunk.oldStartLine, - newStartRow: rawHunk.newStartLine, - oldRowCount: rawHunk.oldLineCount, - newRowCount: rawHunk.newLineCount, - sectionHeading: rawHunk.heading, - marker: hunkMarker, - regions, - })); - }, + callback: (function(_hunks, _rawHunk, _regions) { + return hunkMarker => { + _hunks.push(new Hunk({ + oldStartRow: _rawHunk.oldStartLine, + newStartRow: _rawHunk.newStartLine, + oldRowCount: _rawHunk.oldLineCount, + newRowCount: _rawHunk.newLineCount, + sectionHeading: _rawHunk.heading, + marker: hunkMarker, + regions: _regions, + })); + }; + })(hunks, rawHunk, regions), }); } }, { From 0187f4c2cec163430e34fea5a7b6a88a43ff8ac1 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 22 Jan 2019 15:37:04 -0500 Subject: [PATCH 55/92] buildHunks() always inserts at the end --- lib/models/patch/builder.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/models/patch/builder.js b/lib/models/patch/builder.js index 5a57832296..555f54a173 100644 --- a/lib/models/patch/builder.js +++ b/lib/models/patch/builder.js @@ -120,12 +120,12 @@ function singleDiffFilePatch(diff, patchBuffer, opts) { return FilePatch.createDelayedFilePatch( oldFile, newFile, patchMarker, TOO_LARGE, () => { - const [hunks] = buildHunks(diff, patchBuffer, insertRow, patchMarker); + const [hunks] = buildHunks(diff, patchBuffer); return new Patch({status: diff.status, hunks, marker: patchMarker}); }, ); } else { - const [hunks, patchMarker] = buildHunks(diff, patchBuffer, patchBuffer.getBuffer().getLastRow()); + const [hunks, patchMarker] = buildHunks(diff, patchBuffer); const patch = new Patch({status: diff.status, hunks, marker: patchMarker}); return new FilePatch(oldFile, newFile, patch); @@ -172,7 +172,7 @@ function dualDiffFilePatch(diff1, diff2, patchBuffer, opts) { // TODO: do something with large diff too } - const [hunks, patchMarker] = buildHunks(contentChangeDiff, patchBuffer, patchBuffer.getBuffer().getLastRow()); + const [hunks, patchMarker] = buildHunks(contentChangeDiff, patchBuffer); const patch = new Patch({status, hunks, marker: patchMarker}); return new FilePatch(oldFile, newFile, patch); @@ -185,8 +185,9 @@ const CHANGEKIND = { '\\': NoNewline, }; -function buildHunks(diff, patchBuffer, insertRow) { - const inserter = patchBuffer.createInserterAt([insertRow, 0]); +function buildHunks(diff, patchBuffer) { + const inserter = patchBuffer.createInserterAtEnd() + .keepBefore(patchBuffer.findAllMarkers({endPosition: patchBuffer.getInsertionPoint()})); let patchMarker = null; let firstHunk = true; From 07f3c88a7d4de3873c6b61980801c4552eb35e4c Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 22 Jan 2019 15:37:52 -0500 Subject: [PATCH 56/92] :fire: trailing newline --- test/models/patch/builder.test.js | 32 +++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/test/models/patch/builder.test.js b/test/models/patch/builder.test.js index 75949de05e..a8cd12b2db 100644 --- a/test/models/patch/builder.test.js +++ b/test/models/patch/builder.test.js @@ -79,7 +79,7 @@ describe('buildFilePatch', function() { const bufferText = 'line-0\nline-1\nline-2\nline-3\nline-4\nline-5\nline-6\nline-7\nline-8\nline-9\nline-10\n' + - 'line-11\nline-12\nline-13\nline-14\nline-15\nline-16\nline-17\nline-18\n'; + 'line-11\nline-12\nline-13\nline-14\nline-15\nline-16\nline-17\nline-18'; assert.strictEqual(buffer.getText(), bufferText); assertInPatch(p, buffer).hunks( @@ -113,7 +113,7 @@ describe('buildFilePatch', function() { {kind: 'unchanged', string: ' line-13\n', range: [[13, 0], [13, 7]]}, {kind: 'deletion', string: '-line-14\n-line-15\n', range: [[14, 0], [15, 7]]}, {kind: 'addition', string: '+line-16\n+line-17\n', range: [[16, 0], [17, 7]]}, - {kind: 'unchanged', string: ' line-18\n', range: [[18, 0], [18, 7]]}, + {kind: 'unchanged', string: ' line-18', range: [[18, 0], [18, 7]]}, ], }, ); @@ -229,7 +229,7 @@ describe('buildFilePatch', function() { assert.isFalse(p.getNewFile().isPresent()); assert.strictEqual(p.getPatch().getStatus(), 'deleted'); - const bufferText = 'line-0\nline-1\nline-2\nline-3\n\n'; + const bufferText = 'line-0\nline-1\nline-2\nline-3\n'; assert.strictEqual(buffer.getText(), bufferText); assertInPatch(p, buffer).hunks( @@ -238,7 +238,7 @@ describe('buildFilePatch', function() { endRow: 4, header: '@@ -1,5 +0,0 @@', regions: [ - {kind: 'deletion', string: '-line-0\n-line-1\n-line-2\n-line-3\n-\n', range: [[0, 0], [4, 0]]}, + {kind: 'deletion', string: '-line-0\n-line-1\n-line-2\n-line-3\n-', range: [[0, 0], [4, 0]]}, ], }, ); @@ -276,7 +276,7 @@ describe('buildFilePatch', function() { assert.strictEqual(p.getNewMode(), '100755'); assert.strictEqual(p.getPatch().getStatus(), 'added'); - const bufferText = 'line-0\nline-1\nline-2\n'; + const bufferText = 'line-0\nline-1\nline-2'; assert.strictEqual(buffer.getText(), bufferText); assertInPatch(p, buffer).hunks( @@ -285,7 +285,7 @@ describe('buildFilePatch', function() { endRow: 2, header: '@@ -0,0 +1,3 @@', regions: [ - {kind: 'addition', string: '+line-0\n+line-1\n+line-2\n', range: [[0, 0], [2, 6]]}, + {kind: 'addition', string: '+line-0\n+line-1\n+line-2', range: [[0, 0], [2, 6]]}, ], }, ); @@ -319,7 +319,7 @@ describe('buildFilePatch', function() { assert.lengthOf(multiFilePatch.getFilePatches(), 1); const [p] = multiFilePatch.getFilePatches(); const buffer = multiFilePatch.getBuffer(); - assert.strictEqual(buffer.getText(), 'line-0\nline-1\n No newline at end of file\n'); + assert.strictEqual(buffer.getText(), 'line-0\nline-1\n No newline at end of file'); assertInPatch(p, buffer).hunks({ startRow: 0, @@ -328,7 +328,7 @@ describe('buildFilePatch', function() { regions: [ {kind: 'addition', string: '+line-0\n', range: [[0, 0], [0, 6]]}, {kind: 'deletion', string: '-line-1\n', range: [[1, 0], [1, 6]]}, - {kind: 'nonewline', string: '\\ No newline at end of file\n', range: [[2, 0], [2, 26]]}, + {kind: 'nonewline', string: '\\ No newline at end of file', range: [[2, 0], [2, 26]]}, ], }); }); @@ -383,13 +383,13 @@ describe('buildFilePatch', function() { assert.strictEqual(p.getNewSymlink(), 'the-destination'); assert.strictEqual(p.getStatus(), 'deleted'); - assert.strictEqual(buffer.getText(), 'line-0\nline-1\n'); + assert.strictEqual(buffer.getText(), 'line-0\nline-1'); assertInPatch(p, buffer).hunks({ startRow: 0, endRow: 1, header: '@@ -0,0 +0,2 @@', regions: [ - {kind: 'addition', string: '+line-0\n+line-1\n', range: [[0, 0], [1, 6]]}, + {kind: 'addition', string: '+line-0\n+line-1', range: [[0, 0], [1, 6]]}, ], }); }); @@ -442,13 +442,13 @@ describe('buildFilePatch', function() { assert.isNull(p.getNewSymlink()); assert.strictEqual(p.getStatus(), 'added'); - assert.strictEqual(buffer.getText(), 'line-0\nline-1\n'); + assert.strictEqual(buffer.getText(), 'line-0\nline-1'); assertInPatch(p, buffer).hunks({ startRow: 0, endRow: 1, header: '@@ -0,2 +0,0 @@', regions: [ - {kind: 'deletion', string: '-line-0\n-line-1\n', range: [[0, 0], [1, 6]]}, + {kind: 'deletion', string: '-line-0\n-line-1', range: [[0, 0], [1, 6]]}, ], }); }); @@ -500,13 +500,13 @@ describe('buildFilePatch', function() { assert.strictEqual(p.getNewSymlink(), 'the-destination'); assert.strictEqual(p.getStatus(), 'deleted'); - assert.strictEqual(buffer.getText(), 'line-0\nline-1\n'); + assert.strictEqual(buffer.getText(), 'line-0\nline-1'); assertInPatch(p, buffer).hunks({ startRow: 0, endRow: 1, header: '@@ -0,0 +0,2 @@', regions: [ - {kind: 'addition', string: '+line-0\n+line-1\n', range: [[0, 0], [1, 6]]}, + {kind: 'addition', string: '+line-0\n+line-1', range: [[0, 0], [1, 6]]}, ], }); }); @@ -599,7 +599,7 @@ describe('buildFilePatch', function() { mp.getBuffer().getText(), 'line-0\nline-1\nline-2\nline-3\nline-4\nline-5\nline-6\n' + 'line-5\nline-6\nline-7\nline-8\n' + - 'line-0\nline-1\nline-2\n', + 'line-0\nline-1\nline-2', ); assert.strictEqual(mp.getFilePatches()[0].getOldPath(), 'first'); @@ -637,7 +637,7 @@ describe('buildFilePatch', function() { assertInFilePatch(mp.getFilePatches()[2], buffer).hunks( { startRow: 11, endRow: 13, header: '@@ -1,0 +1,3 @@', regions: [ - {kind: 'addition', string: '+line-0\n+line-1\n+line-2\n', range: [[11, 0], [13, 6]]}, + {kind: 'addition', string: '+line-0\n+line-1\n+line-2', range: [[11, 0], [13, 6]]}, ], }, ); From f4ea1a2e6155274bcbe4c1cbb4ac759a475896bd Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 22 Jan 2019 15:41:03 -0500 Subject: [PATCH 57/92] Remove console.logs :eyes: --- lib/models/patch/builder.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/models/patch/builder.js b/lib/models/patch/builder.js index 555f54a173..b993d74214 100644 --- a/lib/models/patch/builder.js +++ b/lib/models/patch/builder.js @@ -195,7 +195,6 @@ function buildHunks(diff, patchBuffer) { // Separate multiple patches on the same buffer with an unmarked newline if (patchBuffer.getBuffer().getLength() > 0) { - console.log('inserting newline to separate Patches'); inserter.insert('\n'); } @@ -208,7 +207,6 @@ function buildHunks(diff, patchBuffer) { if (firstHunk) { firstHunk = false; } else { - console.log('inserting newline to separate Hunks'); inserter.insert('\n'); } @@ -225,7 +223,6 @@ function buildHunks(diff, patchBuffer) { if (firstRegion) { firstRegion = false; } else { - console.log('inserting newline to separate Regions'); inserter.insert('\n'); } From 57c03e16bb5681373ab92acb89c5b7fdd1bb02c8 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 22 Jan 2019 15:50:29 -0500 Subject: [PATCH 58/92] Mark zero-length patches --- lib/models/patch/builder.js | 4 ++-- test/models/patch/builder.test.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/models/patch/builder.js b/lib/models/patch/builder.js index b993d74214..df38c7807e 100644 --- a/lib/models/patch/builder.js +++ b/lib/models/patch/builder.js @@ -193,8 +193,8 @@ function buildHunks(diff, patchBuffer) { let firstHunk = true; const hunks = []; - // Separate multiple patches on the same buffer with an unmarked newline - if (patchBuffer.getBuffer().getLength() > 0) { + // Separate multiple non-empty patches on the same buffer with an unmarked newline + if (patchBuffer.getBuffer().getLength() > 0 && diff.hunks.length > 0) { inserter.insert('\n'); } diff --git a/test/models/patch/builder.test.js b/test/models/patch/builder.test.js index a8cd12b2db..3555a3fc48 100644 --- a/test/models/patch/builder.test.js +++ b/test/models/patch/builder.test.js @@ -740,7 +740,7 @@ describe('buildFilePatch', function() { assert.strictEqual(fp3.getNewPath(), 'third'); assertInFilePatch(fp3, buffer).hunks({ startRow: 7, endRow: 9, header: '@@ -1,3 +1,0 @@', regions: [ - {kind: 'deletion', string: '-line-0\n-line-1\n-line-2\n', range: [[7, 0], [9, 6]]}, + {kind: 'deletion', string: '-line-0\n-line-1\n-line-2', range: [[7, 0], [9, 6]]}, ], }); }); @@ -794,7 +794,7 @@ describe('buildFilePatch', function() { assert.strictEqual(mp.getFilePatches()[1].getOldPath(), 'second'); assert.deepEqual(mp.getFilePatches()[1].getHunks(), []); - assert.deepEqual(mp.getFilePatches()[1].getMarker().getRange().serialize(), [[7, 0], [7, 0]]); + assert.deepEqual(mp.getFilePatches()[1].getMarker().getRange().serialize(), [[6, 6], [6, 6]]); assert.strictEqual(mp.getFilePatches()[2].getOldPath(), 'third'); assert.deepEqual(mp.getFilePatches()[2].getMarker().getRange().serialize(), [[7, 0], [10, 6]]); From 869ec0ebacc76148cd610392dcc349a8b0bdb927 Mon Sep 17 00:00:00 2001 From: annthurium Date: Tue, 22 Jan 2019 16:09:50 -0800 Subject: [PATCH 59/92] [wip] make Patch model tests use a PatchBuffer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit sometimes `Region.marker` is a `Marker` (as it should be) and sometimes it’s a `MarkerLayer` (wtf). I think Patch.clone() is the problem but needs more investigation. Pushing what I've got so others can take a look tomorrow. --- lib/models/patch/patch.js | 8 ++++---- test/models/patch/patch.test.js | 11 ++++------- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/lib/models/patch/patch.js b/lib/models/patch/patch.js index d5c5982ccd..ede2804b97 100644 --- a/lib/models/patch/patch.js +++ b/lib/models/patch/patch.js @@ -192,9 +192,9 @@ export default class Patch { } } - const marker = nextLayeredBuffer.buffer.markRange( + const marker = nextLayeredBuffer.markRange( this.constructor.layerName, - [[0, 0], [nextLayeredBuffer.buffer.getLastRow() - 1, Infinity]], + [[0, 0], [nextLayeredBuffer.getBuffer().getLastRow() - 1, Infinity]], {invalidate: 'never', exclusive: false}, ); @@ -427,7 +427,7 @@ class BufferBuilder { // The ranges provided to builder methods are expected to be valid within the original buffer. Account for // the position of the Patch within its original TextBuffer, and any existing content already on the next // TextBuffer. - this.offset = this.nextLayeredBuffer.buffer.getLastRow() - originalBaseOffset; + this.offset = this.nextLayeredBuffer.getBuffer().getLastRow() - originalBaseOffset; this.hunkBufferText = ''; this.hunkRowCount = 0; @@ -476,7 +476,7 @@ class BufferBuilder { return new RegionKind(this.nextLayeredBuffer.layers[RegionKind.layerName]); }); - const marker = this.nextLayeredBuffer.buffer.markRange('hunk', this.hunkRange, {invalidate: 'never', exclusive: false}); + const marker = this.nextLayeredBuffer.markRange('hunk', this.hunkRange, {invalidate: 'never', exclusive: false}); this.hunkBufferText = ''; this.hunkRowCount = 0; diff --git a/test/models/patch/patch.test.js b/test/models/patch/patch.test.js index b5989ab3fe..a543deec8f 100644 --- a/test/models/patch/patch.test.js +++ b/test/models/patch/patch.test.js @@ -1,11 +1,12 @@ import {TextBuffer} from 'atom'; import Patch from '../../../lib/models/patch/patch'; +import PatchBuffer from '../../../lib/models/patch/patch-buffer'; import Hunk from '../../../lib/models/patch/hunk'; import {Unchanged, Addition, Deletion, NoNewline} from '../../../lib/models/patch/region'; import {assertInPatch} from '../../helpers'; -describe('Patch', function() { +describe.only('Patch', function() { it('has some standard accessors', function() { const buffer = new TextBuffer({text: 'bufferText'}); const layers = buildLayers(buffer); @@ -165,9 +166,7 @@ describe('Patch', function() { let stageLayeredBuffer; beforeEach(function() { - const stageBuffer = new TextBuffer(); - const stageLayers = buildLayers(stageBuffer); - stageLayeredBuffer = {buffer: stageBuffer, layers: stageLayers}; + stageLayeredBuffer = new PatchBuffer(); }); it('creates a patch that applies selected lines from only the first hunk', function() { @@ -372,9 +371,7 @@ describe('Patch', function() { let unstageLayeredBuffer; beforeEach(function() { - const unstageBuffer = new TextBuffer(); - const unstageLayers = buildLayers(unstageBuffer); - unstageLayeredBuffer = {buffer: unstageBuffer, layers: unstageLayers}; + unstageLayeredBuffer = new PatchBuffer(); }); it('creates a patch that updates the index to unapply selected lines from a single hunk', function() { From d1fcf707d55e97b2f498270bb19b99372d009661 Mon Sep 17 00:00:00 2001 From: annthurium Date: Tue, 22 Jan 2019 16:11:40 -0800 Subject: [PATCH 60/92] :fire: .only --- test/models/patch/patch.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/models/patch/patch.test.js b/test/models/patch/patch.test.js index a543deec8f..8307d832b7 100644 --- a/test/models/patch/patch.test.js +++ b/test/models/patch/patch.test.js @@ -6,7 +6,7 @@ import Hunk from '../../../lib/models/patch/hunk'; import {Unchanged, Addition, Deletion, NoNewline} from '../../../lib/models/patch/region'; import {assertInPatch} from '../../helpers'; -describe.only('Patch', function() { +describe('Patch', function() { it('has some standard accessors', function() { const buffer = new TextBuffer({text: 'bufferText'}); const layers = buildLayers(buffer); From d13d1849d82d2bfa097adaf929b172b25e6c4688 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 22 Jan 2019 21:42:23 -0500 Subject: [PATCH 61/92] DelayedPatch :point_right: HiddenPatch --- test/models/patch/builder.test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/models/patch/builder.test.js b/test/models/patch/builder.test.js index 3555a3fc48..1f279c5c32 100644 --- a/test/models/patch/builder.test.js +++ b/test/models/patch/builder.test.js @@ -802,7 +802,7 @@ describe('buildFilePatch', function() { }); describe('with a large diff', function() { - it('creates a DelayedPatch when the diff is "too large"', function() { + it('creates a HiddenPatch when the diff is "too large"', function() { const mfp = buildMultiFilePatch([ { oldPath: 'first', oldMode: '100644', newPath: 'first', newMode: '100755', status: 'modified', @@ -847,7 +847,7 @@ describe('buildFilePatch', function() { ); }); - it('re-parse a DelayedPatch as a Patch', function() { + it('re-parse a HiddenPatch as a Patch', function() { const mfp = buildMultiFilePatch([ { oldPath: 'first', oldMode: '100644', newPath: 'first', newMode: '100644', status: 'modified', @@ -1028,7 +1028,7 @@ describe('buildFilePatch', function() { assert.deepEqual(fp2.getMarker().getRange().serialize(), [[4, 0], [4, 0]]); }); - it('does not create a DelayedPatch when the patch has been explicitly expanded', function() { + it('does not create a HiddenPatch when the patch has been explicitly expanded', function() { const mfp = buildMultiFilePatch([ { oldPath: 'big/file.txt', oldMode: '100644', newPath: 'big/file.txt', newMode: '100755', status: 'modified', From d25db25239f98660849b63332818f1fbbacac2af Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 22 Jan 2019 21:42:43 -0500 Subject: [PATCH 62/92] More trailing newlines --- test/models/patch/builder.test.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/models/patch/builder.test.js b/test/models/patch/builder.test.js index 1f279c5c32..d47ccdff1e 100644 --- a/test/models/patch/builder.test.js +++ b/test/models/patch/builder.test.js @@ -841,7 +841,7 @@ describe('buildFilePatch', function() { { startRow: 0, endRow: 1, header: '@@ -1,1 +1,2 @@', regions: [ {kind: 'unchanged', string: ' line-4\n', range: [[0, 0], [0, 6]]}, - {kind: 'addition', string: '+line-5\n', range: [[1, 0], [1, 6]]}, + {kind: 'addition', string: '+line-5', range: [[1, 0], [1, 6]]}, ], }, ); @@ -879,7 +879,7 @@ describe('buildFilePatch', function() { {kind: 'unchanged', string: ' line-0\n', range: [[0, 0], [0, 6]]}, {kind: 'addition', string: '+line-1\n', range: [[1, 0], [1, 6]]}, {kind: 'deletion', string: '-line-2\n', range: [[2, 0], [2, 6]]}, - {kind: 'unchanged', string: ' line-3\n', range: [[3, 0], [3, 6]]}, + {kind: 'unchanged', string: ' line-3', range: [[3, 0], [3, 6]]}, ], }, ); @@ -940,7 +940,7 @@ describe('buildFilePatch', function() { {kind: 'unchanged', string: ' line-0\n', range: [[4, 0], [4, 6]]}, {kind: 'addition', string: '+line-1\n', range: [[5, 0], [5, 6]]}, {kind: 'deletion', string: '-line-2\n', range: [[6, 0], [6, 6]]}, - {kind: 'unchanged', string: ' line-3\n', range: [[7, 0], [7, 6]]}, + {kind: 'unchanged', string: ' line-3', range: [[7, 0], [7, 6]]}, ], }, ); @@ -978,7 +978,7 @@ describe('buildFilePatch', function() { {kind: 'unchanged', string: ' line-0\n', range: [[9, 0], [9, 6]]}, {kind: 'addition', string: '+line-1\n', range: [[10, 0], [10, 6]]}, {kind: 'deletion', string: '-line-2\n', range: [[11, 0], [11, 6]]}, - {kind: 'unchanged', string: ' line-3\n', range: [[12, 0], [12, 6]]}, + {kind: 'unchanged', string: ' line-3', range: [[12, 0], [12, 6]]}, ], }, ); @@ -1054,7 +1054,7 @@ describe('buildFilePatch', function() { {kind: 'unchanged', string: ' line-0\n', range: [[0, 0], [0, 6]]}, {kind: 'addition', string: '+line-1\n', range: [[1, 0], [1, 6]]}, {kind: 'deletion', string: '-line-2\n', range: [[2, 0], [2, 6]]}, - {kind: 'unchanged', string: ' line-3\n', range: [[3, 0], [3, 6]]}, + {kind: 'unchanged', string: ' line-3', range: [[3, 0], [3, 6]]}, ], }, ); From 92a113087c05a53ec3194be57aa1f4ddbe3c83fa Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 22 Jan 2019 21:44:08 -0500 Subject: [PATCH 63/92] updateMarkers methods to consume marker Maps --- lib/models/patch/file-patch.js | 8 ++------ lib/models/patch/hunk.js | 7 +++++++ lib/models/patch/patch.js | 7 +++++++ lib/models/patch/region.js | 4 ++++ 4 files changed, 20 insertions(+), 6 deletions(-) diff --git a/lib/models/patch/file-patch.js b/lib/models/patch/file-patch.js index 7c76d6a0e9..677e8a4e3e 100644 --- a/lib/models/patch/file-patch.js +++ b/lib/models/patch/file-patch.js @@ -119,12 +119,8 @@ export default class FilePatch { return this.getPatch().getHunks(); } - triggerDelayedRender() { - const nextPatch = this.patch.parseFn(this.patch); - if (nextPatch !== this.patch) { - this.patch = nextPatch; - this.didChangeRenderStatus(); - } + updateMarkers(map) { + return this.patch.updateMarkers(map); } triggerCollapse = () => { diff --git a/lib/models/patch/hunk.js b/lib/models/patch/hunk.js index 074f86f7f3..6922203bb9 100644 --- a/lib/models/patch/hunk.js +++ b/lib/models/patch/hunk.js @@ -141,6 +141,13 @@ export default class Hunk { this.marker = markable.markRange(this.getRange(), {invalidate: 'never', exclusive: false}); } + updateMarkers(map) { + this.marker = map.get(this.marker) || this.marker; + for (const region of this.regions) { + region.updateMarkers(map); + } + } + toStringIn(buffer) { return this.getRegions().reduce((str, region) => str + region.toStringIn(buffer), this.getHeader() + '\n'); } diff --git a/lib/models/patch/patch.js b/lib/models/patch/patch.js index ede2804b97..3bc53902cc 100644 --- a/lib/models/patch/patch.js +++ b/lib/models/patch/patch.js @@ -83,6 +83,13 @@ export default class Patch { ); } + updateMarkers(map) { + this.marker = map.get(this.marker) || this.marker; + for (const hunk of this.hunks) { + hunk.updateMarkers(map); + } + } + getMaxLineNumberWidth() { const lastHunk = this.hunks[this.hunks.length - 1]; return lastHunk ? lastHunk.getMaxLineNumberWidth() : 0; diff --git a/lib/models/patch/region.js b/lib/models/patch/region.js index d246b911bc..ea30b4b024 100644 --- a/lib/models/patch/region.js +++ b/lib/models/patch/region.js @@ -111,6 +111,10 @@ class Region { this.marker = markable.markRange(this.getRange(), {invalidate: 'never', exclusive: false}); } + updateMarkers(map) { + this.marker = map.get(this.marker) || this.marker; + } + toStringIn(buffer) { const raw = buffer.getTextInRange(this.getRange()); return this.constructor.origin + raw.replace(/\r?\n/g, '$&' + this.constructor.origin) + From 70ea3024605130ae4849d805ba2fe91d72020f2f Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 22 Jan 2019 21:45:08 -0500 Subject: [PATCH 64/92] Implement HiddenPatch --- lib/models/patch/file-patch.js | 4 +-- lib/models/patch/patch.js | 48 ++++++++++++++-------------------- 2 files changed, 21 insertions(+), 31 deletions(-) diff --git a/lib/models/patch/file-patch.js b/lib/models/patch/file-patch.js index 677e8a4e3e..923f0b039d 100644 --- a/lib/models/patch/file-patch.js +++ b/lib/models/patch/file-patch.js @@ -9,8 +9,8 @@ export default class FilePatch { return new this(nullFile, nullFile, Patch.createNull()); } - static createDelayedFilePatch(oldFile, newFile, marker, renderStatus, parseFn) { - return new this(oldFile, newFile, Patch.createDelayedPatch(marker, renderStatus, parseFn)); + static createHiddenFilePatch(oldFile, newFile, marker, renderStatus, showFn) { + return new this(oldFile, newFile, Patch.createHiddenPatch(marker, renderStatus, showFn)); } constructor(oldFile, newFile, patch) { diff --git a/lib/models/patch/patch.js b/lib/models/patch/patch.js index 3bc53902cc..e734b130b5 100644 --- a/lib/models/patch/patch.js +++ b/lib/models/patch/patch.js @@ -28,22 +28,16 @@ export default class Patch { return new NullPatch(); } - static createDelayedPatch(marker, renderStatus, parseFn) { - return new this({status: null, hunks: [], marker, renderStatus, parseFn}); + static createHiddenPatch(marker, renderStatus, showFn) { + return new HiddenPatch(marker, renderStatus, showFn); } - constructor({status, hunks, marker, renderStatus, parseFn}) { + constructor({status, hunks, marker}) { this.status = status; this.hunks = hunks; this.marker = marker; - this.renderStatus = renderStatus || EXPANDED; this.changedLineCount = this.getHunks().reduce((acc, hunk) => acc + hunk.changedLineCount(), 0); - - if (parseFn) { - // Override the prototype's method - this.parseFn = parseFn; - } } getStatus() { @@ -100,24 +94,11 @@ export default class Patch { status: opts.status !== undefined ? opts.status : this.getStatus(), hunks: opts.hunks !== undefined ? opts.hunks : this.getHunks(), marker: opts.marker !== undefined ? opts.marker : this.getMarker(), - renderStatus: opts.renderStatus !== undefined ? opts.renderStatus : this.getRenderStatus(), }); } - collapsed() { - if (this.getRenderStatus() === COLLAPSED) { - return this; } - - return this.clone({renderStatus: COLLAPSED}); - } - - expanded() { - if (this.getRenderStatus() === EXPANDED) { - return this; } - - return this.clone({renderStatus: EXPANDED}); } buildStagePatchForLines(originalBuffer, nextLayeredBuffer, rowSet) { @@ -329,11 +310,24 @@ export default class Patch { } getRenderStatus() { - return this.renderStatus; + return EXPANDED; + } +} + +class HiddenPatch extends Patch { + constructor(marker, renderStatus, showFn) { + super({status: null, hunks: [], marker}); + + this.renderStatus = renderStatus; + this.show = showFn; } - parseFn() { - return this; + getInsertionPoint() { + return this.getRange().end; + } + + getRenderStatus() { + return this.renderStatus; } } @@ -420,10 +414,6 @@ class NullPatch { getRenderStatus() { return EXPANDED; } - - parseFn() { - return this; - } } class BufferBuilder { From 24515a1033cd037c349deaf0aafdeb00bce96941 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 22 Jan 2019 21:45:30 -0500 Subject: [PATCH 65/92] Get starting and ending Marker sets --- lib/models/patch/patch.js | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/lib/models/patch/patch.js b/lib/models/patch/patch.js index e734b130b5..3a2e1e1367 100644 --- a/lib/models/patch/patch.js +++ b/lib/models/patch/patch.js @@ -97,8 +97,32 @@ export default class Patch { }); } + /* Return the set of Markers owned by this Patch that butt up against the patch's beginning. */ + getStartingMarkers() { + const markers = [this.marker]; + if (this.hunks.length > 0) { + const firstHunk = this.hunks[0]; + markers.push(firstHunk.getMarker()); + if (firstHunk.getRegions().length > 0) { + const firstRegion = firstHunk.getRegions()[0]; + markers.push(firstRegion.getMarker()); + } } + return markers; + } + + /* Return the set of Markers owned by this Patch that end at the patch's end position. */ + getEndingMarkers() { + const markers = [this.marker]; + if (this.hunks.length > 0) { + const lastHunk = this.hunks[this.hunks.length - 1]; + markers.push(lastHunk.getMarker()); + if (lastHunk.getRegions().length > 0) { + const lastRegion = lastHunk.getRegions()[lastHunk.getRegions().length - 1]; + markers.push(lastRegion.getMarker()); + } } + return markers; } buildStagePatchForLines(originalBuffer, nextLayeredBuffer, rowSet) { From f27f5ab8509b05e70aaccf374c5256ea3a756e9c Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 22 Jan 2019 21:45:56 -0500 Subject: [PATCH 66/92] Create a HiddenFilePatch for diff sets that are too large --- lib/models/patch/builder.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/models/patch/builder.js b/lib/models/patch/builder.js index df38c7807e..04164d89d1 100644 --- a/lib/models/patch/builder.js +++ b/lib/models/patch/builder.js @@ -110,18 +110,19 @@ function singleDiffFilePatch(diff, patchBuffer, opts) { EXPANDED; if (renderStatus === TOO_LARGE) { - const insertRow = patchBuffer.getBuffer().getLastRow(); const patchMarker = patchBuffer.markPosition( Patch.layerName, - [insertRow, 0], + patchBuffer.getBuffer().getEndPosition(), {invalidate: 'never', exclusive: false}, ); - return FilePatch.createDelayedFilePatch( + return FilePatch.createHiddenFilePatch( oldFile, newFile, patchMarker, TOO_LARGE, () => { - const [hunks] = buildHunks(diff, patchBuffer); - return new Patch({status: diff.status, hunks, marker: patchMarker}); + const subPatchBuffer = new PatchBuffer(); + const [hunks, nextPatchMarker] = buildHunks(diff, patchBuffer); + const nextPatch = new Patch({status: diff.status, hunks, marker: nextPatchMarker}); + return {patch: nextPatch, patchBuffer: subPatchBuffer}; }, ); } else { From 36ac87cd0af4d9e54f71902aca67a3c4874a1608 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 22 Jan 2019 21:46:28 -0500 Subject: [PATCH 67/92] Move collapsing and expanding to MFP --- lib/models/patch/file-patch.js | 53 ++++++++++++++++++++++------ lib/models/patch/multi-file-patch.js | 18 +++++++++- 2 files changed, 59 insertions(+), 12 deletions(-) diff --git a/lib/models/patch/file-patch.js b/lib/models/patch/file-patch.js index 923f0b039d..aec7896cb9 100644 --- a/lib/models/patch/file-patch.js +++ b/lib/models/patch/file-patch.js @@ -1,7 +1,7 @@ import {Emitter} from 'event-kit'; import {nullFile} from './file'; -import Patch from './patch'; +import Patch, {COLLAPSED} from './patch'; import {toGitPathSep} from '../../helpers'; export default class FilePatch { @@ -123,20 +123,43 @@ export default class FilePatch { return this.patch.updateMarkers(map); } - triggerCollapse = () => { - const nextPatch = this.patch.collapsed(); - if (nextPatch !== this.patch) { - this.patch = nextPatch; - this.didChangeRenderStatus(); + triggerCollapseIn(patchBuffer) { + if (!this.patch.getRenderStatus().isVisible()) { + return false; } + + const {patchBuffer: subPatchBuffer, markerMap} = patchBuffer.extractPatchBuffer(this.patch.getRange()); + + const oldPatch = this.patch; + this.patch = Patch.createHiddenPatch(oldPatch.getMarker(), COLLAPSED, () => { + const [marker] = subPatchBuffer.findMarkers(Patch.layerName, {}); + const newPatch = oldPatch.clone({marker}); + + return {patch: newPatch, patchBuffer: subPatchBuffer}; + }); + this.updateMarkers(markerMap); + + this.didChangeRenderStatus(); + return true; } - triggerExpand = () => { - const nextPatch = this.patch.expanded(); - if (nextPatch !== this.patch) { - this.patch = nextPatch; - this.didChangeRenderStatus(); + triggerExpandIn(patchBuffer, {before, after}) { + if (this.patch.getRenderStatus().isVisible()) { + return false; } + + const {patch: nextPatch, patchBuffer: subPatchBuffer} = this.patch.show(); + + patchBuffer + .createInserterAt(this.patch.getInsertionPoint()) + .keepBefore(before) + .keepAfter(after) + .insertPatchBuffer(subPatchBuffer, {callback: map => this.updateMarkers(map)}) + .apply(); + + this.patch = nextPatch; + this.didChangeRenderStatus(); + return true; } didChangeRenderStatus() { @@ -155,6 +178,14 @@ export default class FilePatch { ); } + getStartingMarkers() { + return this.patch.getStartingMarkers(); + } + + getEndingMarkers() { + return this.patch.getEndingMarkers(); + } + buildStagePatchForLines(originalBuffer, nextLayeredBuffer, selectedLineSet) { let newFile = this.getNewFile(); if (this.getStatus() === 'deleted') { diff --git a/lib/models/patch/multi-file-patch.js b/lib/models/patch/multi-file-patch.js index 7d0a9ae8d4..9c0f738bfe 100644 --- a/lib/models/patch/multi-file-patch.js +++ b/lib/models/patch/multi-file-patch.js @@ -98,7 +98,7 @@ export default class MultiFilePatch { } getFilePatchAt(bufferRow) { - if (bufferRow < 0) { + if (bufferRow < 0 || bufferRow > this.patchBuffer.getBuffer().getLastRow()) { return undefined; } const [marker] = this.patchBuffer.findMarkers('patch', {intersectsRow: bufferRow}); @@ -301,6 +301,22 @@ export default class MultiFilePatch { return false; } + collapseFilePatch(filePatch) { + filePatch.triggerCollapseIn(this.patchBuffer); + } + + expandFilePatch(filePatch) { + const range = filePatch.getMarker().getRange(); + + const beforeFilePatch = this.getFilePatchAt(range.start.row - 1); + const before = beforeFilePatch ? beforeFilePatch.getEndingMarkers() : []; + + const afterFilePatch = this.getFilePatchAt(range.end.row + 1); + const after = afterFilePatch ? afterFilePatch.getStartingMarkers() : []; + + filePatch.triggerExpandIn(this.patchBuffer, {before, after}); + } + isPatchTooLargeOrCollapsed = filePatchPath => { const patch = this.filePatchesByPath.get(filePatchPath); if (!patch) { From 4bba2f41570bf1e2e277c84a350238b7cc5173b2 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 22 Jan 2019 21:46:42 -0500 Subject: [PATCH 68/92] Use getRenderStatus() methods --- lib/views/multi-file-patch-view.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/views/multi-file-patch-view.js b/lib/views/multi-file-patch-view.js index 5bb93e7c7b..bab2e28737 100644 --- a/lib/views/multi-file-patch-view.js +++ b/lib/views/multi-file-patch-view.js @@ -22,7 +22,7 @@ import CommitDetailItem from '../items/commit-detail-item'; import IssueishDetailItem from '../items/issueish-detail-item'; import File from '../models/patch/file'; -import {TOO_LARGE, COLLAPSED} from '../models/patch/patch'; +import {TOO_LARGE} from '../models/patch/patch'; const executableText = { [File.modes.NORMAL]: 'non executable', @@ -351,7 +351,7 @@ export default class MultiFilePatchView extends React.Component { openFile={() => this.didOpenFile({selectedFilePatch: filePatch})} toggleFile={() => this.props.toggleFile(filePatch)} - isCollapsed={filePatch.getRenderStatus === TOO_LARGE || filePatch.getRenderStatus === COLLAPSED} + isCollapsed={!filePatch.getRenderStatus().isVisible()} triggerCollapse={filePatch.triggerCollapse} triggerExpand={filePatch.triggerExpand} /> From 5ca5f1207fc17d47b5901639670be9a8651fdb57 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 22 Jan 2019 21:47:00 -0500 Subject: [PATCH 69/92] Use MFP-level expand and collapse methods --- test/models/patch/builder.test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/models/patch/builder.test.js b/test/models/patch/builder.test.js index d47ccdff1e..1f300a7854 100644 --- a/test/models/patch/builder.test.js +++ b/test/models/patch/builder.test.js @@ -869,7 +869,7 @@ describe('buildFilePatch', function() { assert.deepEqual(fp.getStartRange().serialize(), [[0, 0], [0, 0]]); assertInFilePatch(fp).hunks(); - fp.triggerDelayedRender(); + mfp.expandFilePatch(fp); assert.strictEqual(fp.getRenderStatus(), EXPANDED); assert.deepEqual(fp.getMarker().getRange().serialize(), [[0, 0], [3, 6]]); @@ -945,7 +945,7 @@ describe('buildFilePatch', function() { }, ); - fp1.triggerDelayedRender(); + mfp.expandFilePatch(fp1); assert.strictEqual(fp0.getRenderStatus(), EXPANDED); assertInFilePatch(fp0, mfp.getBuffer()).hunks( @@ -1021,7 +1021,7 @@ describe('buildFilePatch', function() { assert.deepEqual(fp1.getMarker().getRange().serialize(), [[0, 0], [0, 0]]); assert.deepEqual(fp2.getMarker().getRange().serialize(), [[0, 0], [0, 0]]); - fp1.triggerDelayedRender(); + mfp.expandFilePatch(fp1); assert.deepEqual(fp0.getMarker().getRange().serialize(), [[0, 0], [0, 0]]); assert.deepEqual(fp1.getMarker().getRange().serialize(), [[0, 0], [3, 6]]); From fae0a50bfa9f38e547fed08636303f28b5a23ed9 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 23 Jan 2019 12:00:48 -0500 Subject: [PATCH 70/92] Show specific marker layers in PatchBuffer::inspect() --- lib/models/patch/patch-buffer.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/models/patch/patch-buffer.js b/lib/models/patch/patch-buffer.js index 936e9a7735..8858fd4f21 100644 --- a/lib/models/patch/patch-buffer.js +++ b/lib/models/patch/patch-buffer.js @@ -87,11 +87,16 @@ export default class PatchBuffer { return {patchBuffer: subBuffer, markerMap}; } - inspect() { + inspect(opts = {}) { + const options = { + layerNames: LAYER_NAMES, + ...opts, + }; + let inspectString = ''; const increasingMarkers = []; - for (const layerName of LAYER_NAMES) { + for (const layerName of options.layerNames) { for (const marker of this.findMarkers(layerName, {})) { increasingMarkers.push({layerName, point: marker.getRange().start, start: true, id: marker.id}); increasingMarkers.push({layerName, point: marker.getRange().end, end: true, id: marker.id}); From 219a4093d8b547d0c1563a4c4f7d25345f8951e1 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 23 Jan 2019 13:07:05 -0500 Subject: [PATCH 71/92] Implement inspect() methods on patch model classes --- lib/models/patch/file-patch.js | 28 ++++++++++++++ lib/models/patch/hunk.js | 22 +++++++++++ lib/models/patch/multi-file-patch.js | 14 +++++++ lib/models/patch/patch.js | 56 ++++++++++++++++++++++++++++ lib/models/patch/region.js | 17 +++++++++ 5 files changed, 137 insertions(+) diff --git a/lib/models/patch/file-patch.js b/lib/models/patch/file-patch.js index aec7896cb9..d2d57a3c51 100644 --- a/lib/models/patch/file-patch.js +++ b/lib/models/patch/file-patch.js @@ -271,6 +271,34 @@ export default class FilePatch { } } + /* + * Construct a String containing diagnostic information about the internal state of this FilePatch. + */ + inspect(opts = {}) { + const options = { + indent: 0, + ...opts, + }; + + let indentation = ''; + for (let i = 0; i < options.indent; i++) { + indentation += ' '; + } + + let inspectString = `${indentation}(FilePatch `; + if (this.getOldPath() !== this.getNewPath()) { + inspectString += `oldPath=${this.getOldPath()} newPath=${this.getNewPath()}`; + } else { + inspectString += `path=${this.getPath()}`; + } + inspectString += '\n'; + + inspectString += this.patch.inspect({indent: options.indent + 2}); + + inspectString += `${indentation})\n`; + return inspectString; + } + getHeaderString() { const fromPath = this.getOldPath() || this.getNewPath(); const toPath = this.getNewPath() || this.getOldPath(); diff --git a/lib/models/patch/hunk.js b/lib/models/patch/hunk.js index 6922203bb9..c9302e6aa5 100644 --- a/lib/models/patch/hunk.js +++ b/lib/models/patch/hunk.js @@ -151,4 +151,26 @@ export default class Hunk { toStringIn(buffer) { return this.getRegions().reduce((str, region) => str + region.toStringIn(buffer), this.getHeader() + '\n'); } + + /* + * Construct a String containing internal diagnostic information. + */ + inspect(opts = {}) { + const options = { + indent: 0, + ...opts, + }; + + let indentation = ''; + for (let i = 0; i < options.indent; i++) { + indentation += ' '; + } + + let inspectString = `${indentation}(Hunk marker=${this.marker.id}\n`; + for (const region of this.regions) { + inspectString += region.inspect({indent: options.indent + 2}); + } + inspectString += `${indentation})\n`; + return inspectString; + } } diff --git a/lib/models/patch/multi-file-patch.js b/lib/models/patch/multi-file-patch.js index 9c0f738bfe..611a4639df 100644 --- a/lib/models/patch/multi-file-patch.js +++ b/lib/models/patch/multi-file-patch.js @@ -339,6 +339,20 @@ export default class MultiFilePatch { return this.filePatches.map(fp => fp.toStringIn(this.getBuffer())).join(''); } + /* + * Construct a string of diagnostic information useful for debugging. + */ + inspect(opts = {}) { + let inspectString = '(MultiFilePatch'; + inspectString += ` filePatchesByMarker=(${Array.from(this.filePatchesByMarker.keys(), m => m.id).join(', ')})`; + inspectString += ` hunksByMarker=(${Array.from(this.hunksByMarker.keys(), m => m.id).join(', ')})\n`; + for (const filePatch of this.filePatches) { + inspectString += filePatch.inspect({indent: 2}); + } + inspectString += ')\n'; + return inspectString; + } + isEqual(other) { return this.toString() === other.toString(); } diff --git a/lib/models/patch/patch.js b/lib/models/patch/patch.js index 3a2e1e1367..0e9c681de1 100644 --- a/lib/models/patch/patch.js +++ b/lib/models/patch/patch.js @@ -329,6 +329,28 @@ export default class Patch { return this.getHunks().reduce((str, hunk) => str + hunk.toStringIn(buffer), ''); } + /* + * Construct a String containing internal diagnostic information. + */ + inspect(opts = {}) { + const options = { + indent: 0, + ...opts, + }; + + let indentation = ''; + for (let i = 0; i < options.indent; i++) { + indentation += ' '; + } + + let inspectString = `${indentation}(Patch marker=${this.marker.id}\n`; + for (const hunk of this.hunks) { + inspectString += hunk.inspect({indent: options.indent + 2}); + } + inspectString += `${indentation})\n`; + return inspectString; + } + isPresent() { return true; } @@ -353,6 +375,23 @@ class HiddenPatch extends Patch { getRenderStatus() { return this.renderStatus; } + + /* + * Construct a String containing internal diagnostic information. + */ + inspect(opts = {}) { + const options = { + indent: 0, + ...opts, + }; + + let indentation = ''; + for (let i = 0; i < options.indent; i++) { + indentation += ' '; + } + + return `${indentation}(HiddenPatch marker=${this.marker.id})\n`; + } } class NullPatch { @@ -431,6 +470,23 @@ class NullPatch { return ''; } + /* + * Construct a String containing internal diagnostic information. + */ + inspect(opts = {}) { + const options = { + indent: 0, + ...opts, + }; + + let indentation = ''; + for (let i = 0; i < options.indent; i++) { + indentation += ' '; + } + + return `${indentation}(NullPatch)\n`; + } + isPresent() { return false; } diff --git a/lib/models/patch/region.js b/lib/models/patch/region.js index ea30b4b024..43bf03519a 100644 --- a/lib/models/patch/region.js +++ b/lib/models/patch/region.js @@ -121,6 +121,23 @@ class Region { buffer.lineEndingForRow(this.getRange().end.row); } + /* + * Construct a String containing internal diagnostic information. + */ + inspect(opts = {}) { + const options = { + indent: 0, + ...opts, + }; + + let indentation = ''; + for (let i = 0; i < options.indent; i++) { + indentation += ' '; + } + + return `${indentation}(${this.constructor.name} marker=${this.marker.id})\n`; + } + isChange() { return true; } From 9767a871066704701b98e5a0a4acd0905e906b3b Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 23 Jan 2019 13:31:42 -0500 Subject: [PATCH 72/92] destroyMarkers() calls to... destroy... the markers --- lib/models/patch/hunk.js | 7 +++++++ lib/models/patch/patch.js | 7 +++++++ lib/models/patch/region.js | 4 ++++ 3 files changed, 18 insertions(+) diff --git a/lib/models/patch/hunk.js b/lib/models/patch/hunk.js index c9302e6aa5..c8b69aef57 100644 --- a/lib/models/patch/hunk.js +++ b/lib/models/patch/hunk.js @@ -148,6 +148,13 @@ export default class Hunk { } } + destroyMarkers() { + this.marker.destroy(); + for (const region of this.regions) { + region.destroyMarkers(); + } + } + toStringIn(buffer) { return this.getRegions().reduce((str, region) => str + region.toStringIn(buffer), this.getHeader() + '\n'); } diff --git a/lib/models/patch/patch.js b/lib/models/patch/patch.js index 0e9c681de1..b111ddd314 100644 --- a/lib/models/patch/patch.js +++ b/lib/models/patch/patch.js @@ -77,6 +77,13 @@ export default class Patch { ); } + destroyMarkers() { + this.marker.destroy(); + for (const hunk of this.hunks) { + hunk.destroyMarkers(); + } + } + updateMarkers(map) { this.marker = map.get(this.marker) || this.marker; for (const hunk of this.hunks) { diff --git a/lib/models/patch/region.js b/lib/models/patch/region.js index 43bf03519a..4d0a14e200 100644 --- a/lib/models/patch/region.js +++ b/lib/models/patch/region.js @@ -115,6 +115,10 @@ class Region { this.marker = map.get(this.marker) || this.marker; } + destroyMarkers() { + this.marker.destroy(); + } + toStringIn(buffer) { const raw = buffer.getTextInRange(this.getRange()); return this.constructor.origin + raw.replace(/\r?\n/g, '$&' + this.constructor.origin) + From b1d7ff5b70fcfdf4180f894c0d7ce3537d258774 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 23 Jan 2019 13:32:08 -0500 Subject: [PATCH 73/92] Build hunk data for expanding too-large patches on the correct buffer --- lib/models/patch/builder.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/models/patch/builder.js b/lib/models/patch/builder.js index 04164d89d1..396ab72601 100644 --- a/lib/models/patch/builder.js +++ b/lib/models/patch/builder.js @@ -120,7 +120,7 @@ function singleDiffFilePatch(diff, patchBuffer, opts) { oldFile, newFile, patchMarker, TOO_LARGE, () => { const subPatchBuffer = new PatchBuffer(); - const [hunks, nextPatchMarker] = buildHunks(diff, patchBuffer); + const [hunks, nextPatchMarker] = buildHunks(diff, subPatchBuffer); const nextPatch = new Patch({status: diff.status, hunks, marker: nextPatchMarker}); return {patch: nextPatch, patchBuffer: subPatchBuffer}; }, From fc3d89c602ba73411ab3e416f47dbba697391b49 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 23 Jan 2019 13:32:37 -0500 Subject: [PATCH 74/92] Fix up Marker-keyed maps in MFP after expand or collapse --- lib/models/patch/multi-file-patch.js | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/lib/models/patch/multi-file-patch.js b/lib/models/patch/multi-file-patch.js index 611a4639df..8d1dfe3b4d 100644 --- a/lib/models/patch/multi-file-patch.js +++ b/lib/models/patch/multi-file-patch.js @@ -302,10 +302,26 @@ export default class MultiFilePatch { } collapseFilePatch(filePatch) { + this.filePatchesByMarker.delete(filePatch.getMarker()); + for (const hunk of filePatch.getHunks()) { + this.hunksByMarker.delete(hunk.getMarker()); + } + filePatch.triggerCollapseIn(this.patchBuffer); + + this.filePatchesByMarker.set(filePatch.getMarker(), filePatch); + // This hunk collection should be empty, but let's iterate anyway just in case filePatch was already collapsed + for (const hunk of filePatch.getHunks()) { + this.hunksByMarker.set(hunk.getMarker(), hunk); + } } expandFilePatch(filePatch) { + this.filePatchesByMarker.delete(filePatch.getMarker()); + for (const hunk of filePatch.getHunks()) { + this.hunksByMarker.delete(hunk.getMarker()); + } + const range = filePatch.getMarker().getRange(); const beforeFilePatch = this.getFilePatchAt(range.start.row - 1); @@ -315,6 +331,11 @@ export default class MultiFilePatch { const after = afterFilePatch ? afterFilePatch.getStartingMarkers() : []; filePatch.triggerExpandIn(this.patchBuffer, {before, after}); + + this.filePatchesByMarker.set(filePatch.getMarker(), filePatch); + for (const hunk of filePatch.getHunks()) { + this.hunksByMarker.set(hunk.getMarker(), hunk); + } } isPatchTooLargeOrCollapsed = filePatchPath => { From e1a1a5cd07363212874cf336b3a47381b0eba78a Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 23 Jan 2019 13:33:01 -0500 Subject: [PATCH 75/92] NullPatch implementations of adjacent-marker methods --- lib/models/patch/patch.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/models/patch/patch.js b/lib/models/patch/patch.js index b111ddd314..5880e67da2 100644 --- a/lib/models/patch/patch.js +++ b/lib/models/patch/patch.js @@ -461,6 +461,14 @@ class NullPatch { } } + getStartingMarkers() { + return []; + } + + getEndingMarkers() { + return []; + } + buildStagePatchForLines() { return this; } From 37c53cae1b4758684b53172fd3beee8591c22f90 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 23 Jan 2019 13:33:34 -0500 Subject: [PATCH 76/92] Defer insertPatchBuffer()'s callback until the marker map is populated --- lib/models/patch/patch-buffer.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/models/patch/patch-buffer.js b/lib/models/patch/patch-buffer.js index 8858fd4f21..761eec0df7 100644 --- a/lib/models/patch/patch-buffer.js +++ b/lib/models/patch/patch-buffer.js @@ -144,6 +144,7 @@ class Inserter { this.startPoint = clipped.copy(); this.insertionPoint = clipped.copy(); this.markerBlueprints = []; + this.markerMapCallbacks = []; this.markersBefore = new Set(); this.markersAfter = new Set(); @@ -205,7 +206,7 @@ class Inserter { } if (opts.callback) { - opts.callback(subMarkerMap); + this.markerMapCallbacks.push({markerMap: subMarkerMap, callback: opts.callback}); } return this; @@ -222,6 +223,10 @@ class Inserter { } } + for (const {markerMap, callback} of this.markerMapCallbacks) { + callback(markerMap); + } + for (const beforeMarker of this.markersBefore) { if (!beforeMarker.isReversed()) { beforeMarker.setHeadPosition(this.startPoint); From 9d174a3f5b16020a6336a5a4d53ab2d76c77c0e4 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 23 Jan 2019 13:34:45 -0500 Subject: [PATCH 77/92] More collapse and expand work --- lib/models/patch/file-patch.js | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/lib/models/patch/file-patch.js b/lib/models/patch/file-patch.js index d2d57a3c51..9196e29c6b 100644 --- a/lib/models/patch/file-patch.js +++ b/lib/models/patch/file-patch.js @@ -128,16 +128,16 @@ export default class FilePatch { return false; } - const {patchBuffer: subPatchBuffer, markerMap} = patchBuffer.extractPatchBuffer(this.patch.getRange()); - const oldPatch = this.patch; - this.patch = Patch.createHiddenPatch(oldPatch.getMarker(), COLLAPSED, () => { - const [marker] = subPatchBuffer.findMarkers(Patch.layerName, {}); - const newPatch = oldPatch.clone({marker}); - - return {patch: newPatch, patchBuffer: subPatchBuffer}; + const position = oldPatch.getRange().start.copy(); + const {patchBuffer: subPatchBuffer, markerMap} = patchBuffer.extractPatchBuffer(oldPatch.getRange()); + oldPatch.destroyMarkers(); + oldPatch.updateMarkers(markerMap); + + const patchMarker = patchBuffer.markPosition(Patch.layerName, position, {invalidate: 'never', exclude: true}); + this.patch = Patch.createHiddenPatch(patchMarker, COLLAPSED, () => { + return {patch: oldPatch, patchBuffer: subPatchBuffer}; }); - this.updateMarkers(markerMap); this.didChangeRenderStatus(); return true; @@ -149,14 +149,17 @@ export default class FilePatch { } const {patch: nextPatch, patchBuffer: subPatchBuffer} = this.patch.show(); + const atStart = this.patch.getInsertionPoint().isEqual([0, 0]); patchBuffer .createInserterAt(this.patch.getInsertionPoint()) .keepBefore(before) .keepAfter(after) - .insertPatchBuffer(subPatchBuffer, {callback: map => this.updateMarkers(map)}) + .insert(atStart ? '' : '\n') + .insertPatchBuffer(subPatchBuffer, {callback: map => nextPatch.updateMarkers(map)}) .apply(); + this.patch.destroyMarkers(); this.patch = nextPatch; this.didChangeRenderStatus(); return true; From a0ae01ca9f617d7e206a95bde4468966fc935e9f Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 23 Jan 2019 14:03:44 -0500 Subject: [PATCH 78/92] Find adjacent FilePatches by array index instead of markers --- lib/models/patch/multi-file-patch.js | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/lib/models/patch/multi-file-patch.js b/lib/models/patch/multi-file-patch.js index 8d1dfe3b4d..f47f673c8e 100644 --- a/lib/models/patch/multi-file-patch.js +++ b/lib/models/patch/multi-file-patch.js @@ -317,18 +317,36 @@ export default class MultiFilePatch { } expandFilePatch(filePatch) { + const index = this.filePatches.indexOf(filePatch); + this.filePatchesByMarker.delete(filePatch.getMarker()); for (const hunk of filePatch.getHunks()) { this.hunksByMarker.delete(hunk.getMarker()); } - const range = filePatch.getMarker().getRange(); + const before = []; + let beforeIndex = index - 1; + while (beforeIndex >= 0) { + const beforeFilePatch = this.filePatches[beforeIndex]; + before.push(...beforeFilePatch.getEndingMarkers()); + + if (!beforeFilePatch.getMarker().getRange().isEmpty()) { + break; + } + beforeIndex--; + } - const beforeFilePatch = this.getFilePatchAt(range.start.row - 1); - const before = beforeFilePatch ? beforeFilePatch.getEndingMarkers() : []; + const after = []; + let afterIndex = index + 1; + while (afterIndex < this.filePatches.length) { + const afterFilePatch = this.filePatches[afterIndex]; + after.push(...afterFilePatch.getStartingMarkers()); - const afterFilePatch = this.getFilePatchAt(range.end.row + 1); - const after = afterFilePatch ? afterFilePatch.getStartingMarkers() : []; + if (!afterFilePatch.getMarker().getRange().isEmpty()) { + break; + } + afterIndex++; + } filePatch.triggerExpandIn(this.patchBuffer, {before, after}); From cba6f0578a14e3bd9675837873b7671a3987dfca Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 23 Jan 2019 14:03:50 -0500 Subject: [PATCH 79/92] Remove unused argument --- lib/models/patch/multi-file-patch.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/models/patch/multi-file-patch.js b/lib/models/patch/multi-file-patch.js index f47f673c8e..b9f7f7572f 100644 --- a/lib/models/patch/multi-file-patch.js +++ b/lib/models/patch/multi-file-patch.js @@ -381,7 +381,7 @@ export default class MultiFilePatch { /* * Construct a string of diagnostic information useful for debugging. */ - inspect(opts = {}) { + inspect() { let inspectString = '(MultiFilePatch'; inspectString += ` filePatchesByMarker=(${Array.from(this.filePatchesByMarker.keys(), m => m.id).join(', ')})`; inspectString += ` hunksByMarker=(${Array.from(this.hunksByMarker.keys(), m => m.id).join(', ')})\n`; From 715f10a3823238ab4de29749dfe0c9e72c6b1953 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 23 Jan 2019 14:04:02 -0500 Subject: [PATCH 80/92] Touch up expected marker range --- test/models/patch/builder.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/models/patch/builder.test.js b/test/models/patch/builder.test.js index 1f300a7854..956a348ec8 100644 --- a/test/models/patch/builder.test.js +++ b/test/models/patch/builder.test.js @@ -1025,7 +1025,7 @@ describe('buildFilePatch', function() { assert.deepEqual(fp0.getMarker().getRange().serialize(), [[0, 0], [0, 0]]); assert.deepEqual(fp1.getMarker().getRange().serialize(), [[0, 0], [3, 6]]); - assert.deepEqual(fp2.getMarker().getRange().serialize(), [[4, 0], [4, 0]]); + assert.deepEqual(fp2.getMarker().getRange().serialize(), [[3, 6], [3, 6]]); }); it('does not create a HiddenPatch when the patch has been explicitly expanded', function() { From cff0d2929d8fab24f620a74cf10e06e3a3be6aaa Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 23 Jan 2019 14:05:32 -0500 Subject: [PATCH 81/92] Correct expected Hunk header --- test/models/patch/builder.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/models/patch/builder.test.js b/test/models/patch/builder.test.js index 956a348ec8..9b0ca9f957 100644 --- a/test/models/patch/builder.test.js +++ b/test/models/patch/builder.test.js @@ -1050,7 +1050,7 @@ describe('buildFilePatch', function() { assert.deepEqual(fp.getMarker().getRange().serialize(), [[0, 0], [3, 6]]); assertInFilePatch(fp, mfp.getBuffer()).hunks( { - startRow: 0, endRow: 3, header: '@@ -1,1 +1,2 @@', regions: [ + startRow: 0, endRow: 3, header: '@@ -1,3 +1,3 @@', regions: [ {kind: 'unchanged', string: ' line-0\n', range: [[0, 0], [0, 6]]}, {kind: 'addition', string: '+line-1\n', range: [[1, 0], [1, 6]]}, {kind: 'deletion', string: '-line-2\n', range: [[2, 0], [2, 6]]}, From 2d7c81a74a6e1c9b82b3e473edd7fdf8ab1839e9 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 23 Jan 2019 15:49:50 -0500 Subject: [PATCH 82/92] Build what-the-diff output instead of just model objects --- test/builder/patch.js | 269 ++++++++++++++++++++++-------------------- 1 file changed, 143 insertions(+), 126 deletions(-) diff --git a/test/builder/patch.js b/test/builder/patch.js index cd8f45a79f..bc02cf6afd 100644 --- a/test/builder/patch.js +++ b/test/builder/patch.js @@ -1,93 +1,74 @@ // Builders for classes related to MultiFilePatches. -import PatchBuffer from '../../lib/models/patch/patch-buffer'; -import MultiFilePatch from '../../lib/models/patch/multi-file-patch'; -import FilePatch from '../../lib/models/patch/file-patch'; -import File, {nullFile} from '../../lib/models/patch/file'; -import Patch from '../../lib/models/patch/patch'; -import Hunk from '../../lib/models/patch/hunk'; -import {Unchanged, Addition, Deletion, NoNewline} from '../../lib/models/patch/region'; - -function appendMarked(patchBuffer, layerName, lines) { - const startPosition = patchBuffer.getInsertionPoint(); - patchBuffer.getBuffer().append(lines.join('\n')); - const marker = patchBuffer.markRange( - layerName, - [startPosition, patchBuffer.getInsertionPoint()], - {invalidate: 'never', exclusive: false}, - ); - patchBuffer.getBuffer().append('\n'); - return marker; -} - -function markFrom(patchBuffer, layerName, startPosition) { - const endPosition = patchBuffer.getInsertionPoint().translate([-1, Infinity]); - return patchBuffer.markRange( - layerName, - [startPosition, endPosition], - {invalidate: 'never', exclusive: false}, - ); -} - -function wrapReturn(patchBuffer, object) { - return {buffer: patchBuffer.getBuffer(), layers: patchBuffer.getLayers(), ...object}; -} +import {buildMultiFilePatch} from '../../lib/models/patch/builder'; +import File from '../../lib/models/patch/file'; class MultiFilePatchBuilder { - constructor(patchBuffer = null) { - this.patchBuffer = patchBuffer; - - this.filePatches = []; + constructor() { + this.rawFilePatches = []; } addFilePatch(block = () => {}) { - const filePatch = new FilePatchBuilder(this.patchBuffer); + const filePatch = new FilePatchBuilder(); block(filePatch); - this.filePatches.push(filePatch.build().filePatch); + this.rawFilePatches.push(filePatch.build().raw); return this; } - build() { - return wrapReturn(this.patchBuffer, { - multiFilePatch: new MultiFilePatch({ - patchBuffer: this.patchBuffer, - filePatches: this.filePatches, - }), - }); + build(opts = {}) { + const raw = this.rawFilePatches; + const multiFilePatch = buildMultiFilePatch(raw, opts); + return {raw, multiFilePatch}; } } class FilePatchBuilder { - constructor(patchBuffer = null) { - this.patchBuffer = patchBuffer; - - this.oldFile = new File({path: 'file', mode: File.modes.NORMAL}); - this.newFile = null; + constructor() { + this._oldPath = 'file'; + this._oldMode = File.modes.NORMAL; + this._oldSymlink = null; + this._newPath = null; + this._newMode = null; + this._newSymlink = null; - this.patchBuilder = new PatchBuilder(this.patchBuffer); + this.patchBuilder = new PatchBuilder(); } setOldFile(block) { const file = new FileBuilder(); block(file); - this.oldFile = file.build().file; + const rawFile = file.build(); + this._oldPath = rawFile.path; + this._oldMode = rawFile.mode; + if (rawFile.symlink) { + this._oldSymlink = rawFile.symlink; + } return this; } nullOldFile() { - this.oldFile = nullFile; + this._oldPath = null; + this._oldMode = null; + this._oldSymlink = null; return this; } setNewFile(block) { const file = new FileBuilder(); block(file); - this.newFile = file.build().file; + const rawFile = file.build(); + this._newPath = rawFile.path; + this._newMode = rawFile.mode; + if (rawFile.symlink) { + this._newSymlink = rawFile.symlink; + } return this; } nullNewFile() { - this.newFile = nullFile; + this._newPath = null; + this._newMode = null; + this._newSymlink = null; return this; } @@ -101,26 +82,54 @@ class FilePatchBuilder { return this; } - renderStatus(...args) { - this.patchBuilder.renderStatus(...args); - return this; - } - empty() { this.patchBuilder.empty(); return this; } - build() { - const {patch} = this.patchBuilder.build(); + build(opts = {}) { + const {raw: rawPatch} = this.patchBuilder.build(); + + if (this._newPath === null) { + this._newPath = this._oldPath; + } - if (this.newFile === null) { - this.newFile = this.oldFile.clone(); + if (this._newMode === null) { + this._newMode = this._oldMode; } - return this.patchBuffer.wrapReturn({ - filePatch: new FilePatch(this.oldFile, this.newFile, patch), - }); + if (this._oldSymlink !== null || this._newSymlink !== null) { + if (rawPatch.hunks.length > 0) { + throw new Error('Cannot have both a symlink target and hunk content'); + } + + const lines = []; + if (this._oldSymlink !== null) { + lines.push(` ${this._oldSymlink}`); + } + if (this._oldSymlink !== null && this._newSymlink !== null) { + lines.push(' --'); + } + if (this._newSymlink !== null) { + lines.push(` ${this._newSymlink}`); + } + } + + const raw = { + oldPath: this._oldPath, + oldMode: this._oldMode, + newPath: this._newPath, + newMode: this._newMode, + ...rawPatch, + }; + + const mfp = buildMultiFilePatch([raw], opts); + const [filePatch] = mfp.getFilePatches(); + + return { + raw, + filePatch, + }; } } @@ -142,37 +151,27 @@ class FileBuilder { } executable() { - return this.mode('100755'); + return this.mode(File.modes.EXECUTABLE); } symlinkTo(destinationPath) { this._symlink = destinationPath; - return this.mode('120000'); + return this.mode(File.modes.SYMLINK); } build() { - return {file: new File({path: this._path, mode: this._mode, symlink: this._symlink})}; + return {path: this._path, mode: this._mode, symlink: this._symlink}; } } class PatchBuilder { - constructor(patchBuffer = null) { - this.patchBuffer = patchBuffer; - - this._renderStatus = undefined; + constructor() { this._status = 'modified'; - this.hunks = []; - - this.patchStart = this.patchBuffer.getInsertionPoint(); + this.rawHunks = []; this.drift = 0; this.explicitlyEmpty = false; } - renderStatus(status) { - this._renderStatus = status; - return this; - } - status(st) { if (['modified', 'added', 'deleted'].indexOf(st) === -1) { throw new Error(`Unrecognized status: ${st} (must be 'modified', 'added' or 'deleted')`); @@ -183,10 +182,10 @@ class PatchBuilder { } addHunk(block = () => {}) { - const builder = new HunkBuilder(this.patchBuffer, this.drift); + const builder = new HunkBuilder(this.drift); block(builder); - const {hunk, drift} = builder.build(); - this.hunks.push(hunk); + const {raw, drift} = builder.build(); + this.rawHunks.push(raw); this.drift = drift; return this; } @@ -197,7 +196,7 @@ class PatchBuilder { } build() { - if (this.hunks.length === 0 && !this.explicitlyEmpty) { + if (this.rawHunks.length === 0 && !this.explicitlyEmpty) { if (this._status === 'modified') { this.addHunk(hunk => hunk.oldRow(1).unchanged('0000').added('0001').deleted('0002').unchanged('0003')); this.addHunk(hunk => hunk.oldRow(10).unchanged('0004').added('0005').deleted('0006').unchanged('0007')); @@ -208,17 +207,27 @@ class PatchBuilder { } } - const marker = markFrom(this.patchBuffer, 'patch', this.patchStart); + const raw = { + status: this._status, + hunks: this.rawHunks, + }; - return wrapReturn(this.patchBuffer, { - patch: new Patch({status: this._status, hunks: this.hunks, marker, renderStatus: this._renderStatus}), - }); + const mfp = buildMultiFilePatch([{ + oldPath: 'file', + oldMode: File.modes.NORMAL, + newPath: 'file', + newMode: File.modes.NORMAL, + ...raw, + }]); + const [filePatch] = mfp.getFilePatches(); + const patch = filePatch.getPatch(); + + return {raw, patch}; } } class HunkBuilder { - constructor(patchBuffer = null, drift = 0) { - this.patchBuffer = patchBuffer; + constructor(drift = 0) { this.drift = drift; this.oldStartRow = 0; @@ -228,8 +237,7 @@ class HunkBuilder { this.sectionHeading = "don't care"; - this.hunkStartPoint = this.patchBuffer.getInsertionPoint(); - this.regions = []; + this.lines = []; } oldRow(rowNumber) { @@ -238,36 +246,38 @@ class HunkBuilder { } unchanged(...lines) { - this.regions.push(new Unchanged(appendMarked(this.patchBuffer, 'unchanged', lines))); + for (const line of lines) { + this.lines.push(` ${line}`); + } return this; } added(...lines) { - this.regions.push(new Addition(appendMarked(this.patchBuffer, 'addition', lines))); + for (const line of lines) { + this.lines.push(`+${line}`); + } return this; } deleted(...lines) { - this.regions.push(new Deletion(appendMarked(this.patchBuffer, 'deletion', lines))); + for (const line of lines) { + this.lines.push(`-${line}`); + } return this; } noNewline() { - this.regions.push(new NoNewline(appendMarked(this.patchBuffer, 'nonewline', [' No newline at end of file']))); + this.lines.push('\\ No newline at end of file'); return this; } build() { - if (this.regions.length === 0) { + if (this.lines.length === 0) { this.unchanged('0000').added('0001').deleted('0002').unchanged('0003'); } if (this.oldRowCount === null) { - this.oldRowCount = this.regions.reduce((count, region) => region.when({ - unchanged: () => count + region.bufferRowCount(), - deletion: () => count + region.bufferRowCount(), - default: () => count, - }), 0); + this.oldRowCount = this.lines.filter(line => /^[ -]/.test(line)).length; } if (this.newStartRow === null) { @@ -275,42 +285,49 @@ class HunkBuilder { } if (this.newRowCount === null) { - this.newRowCount = this.regions.reduce((count, region) => region.when({ - unchanged: () => count + region.bufferRowCount(), - addition: () => count + region.bufferRowCount(), - default: () => count, - }), 0); + this.newRowCount = this.lines.filter(line => /^[ +]/.test(line)).length; } - const marker = markFrom(this.patchBuffer, 'hunk', this.hunkStartPoint); - - return wrapReturn(this.patchBuffer, { - hunk: new Hunk({ - oldStartRow: this.oldStartRow, - oldRowCount: this.oldRowCount, - newStartRow: this.newStartRow, - newRowCount: this.newRowCount, - sectionHeading: this.sectionHeading, - marker, - regions: this.regions, - }), + const raw = { + oldStartLine: this.oldStartRow, + oldLineCount: this.oldRowCount, + newStartLine: this.newStartRow, + newLineCount: this.newRowCount, + heading: this.sectionHeading, + lines: this.lines, + }; + + const mfp = buildMultiFilePatch([{ + oldPath: 'file', + oldMode: File.modes.NORMAL, + newPath: 'file', + newMode: File.modes.NORMAL, + status: 'modified', + hunks: [raw], + }]); + const [fp] = mfp.getFilePatches(); + const [hunk] = fp.getHunks(); + + return { + raw, + hunk, drift: this.drift + this.newRowCount - this.oldRowCount, - }); + }; } } export function multiFilePatchBuilder() { - return new MultiFilePatchBuilder(new PatchBuffer()); + return new MultiFilePatchBuilder(); } export function filePatchBuilder() { - return new FilePatchBuilder(new PatchBuffer()); + return new FilePatchBuilder(); } export function patchBuilder() { - return new PatchBuilder(new PatchBuffer()); + return new PatchBuilder(); } export function hunkBuilder() { - return new HunkBuilder(new PatchBuffer()); + return new HunkBuilder(); } From 537fd0efccaa5ca32d4a47e09a79f18ea8c86925 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 23 Jan 2019 16:19:25 -0500 Subject: [PATCH 83/92] Fix building symlink related FilePatches --- test/builder/patch.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/test/builder/patch.js b/test/builder/patch.js index bc02cf6afd..dbe3f22d27 100644 --- a/test/builder/patch.js +++ b/test/builder/patch.js @@ -41,6 +41,7 @@ class FilePatchBuilder { this._oldPath = rawFile.path; this._oldMode = rawFile.mode; if (rawFile.symlink) { + this.empty(); this._oldSymlink = rawFile.symlink; } return this; @@ -60,6 +61,7 @@ class FilePatchBuilder { this._newPath = rawFile.path; this._newMode = rawFile.mode; if (rawFile.symlink) { + this.empty(); this._newSymlink = rawFile.symlink; } return this; @@ -103,16 +105,18 @@ class FilePatchBuilder { throw new Error('Cannot have both a symlink target and hunk content'); } - const lines = []; + const hb = new HunkBuilder(); if (this._oldSymlink !== null) { - lines.push(` ${this._oldSymlink}`); + hb.unchanged(this._oldSymlink); } if (this._oldSymlink !== null && this._newSymlink !== null) { - lines.push(' --'); + hb.unchanged('--'); } if (this._newSymlink !== null) { - lines.push(` ${this._newSymlink}`); + hb.unchanged(this._newSymlink); } + + rawPatch.hunks = [hb.build().raw]; } const raw = { From 6685330b042a644b5061cf35ee895321b31a12e4 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 23 Jan 2019 16:19:46 -0500 Subject: [PATCH 84/92] ::reMarkOn() accepts a PatchBuffer --- lib/models/patch/hunk.js | 6 +++++- lib/models/patch/region.js | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/lib/models/patch/hunk.js b/lib/models/patch/hunk.js index c8b69aef57..8bff265dd4 100644 --- a/lib/models/patch/hunk.js +++ b/lib/models/patch/hunk.js @@ -138,7 +138,11 @@ export default class Hunk { } reMarkOn(markable) { - this.marker = markable.markRange(this.getRange(), {invalidate: 'never', exclusive: false}); + this.marker = markable.markRange( + this.constructor.layerName, + this.getRange(), + {invalidate: 'never', exclusive: false}, + ); } updateMarkers(map) { diff --git a/lib/models/patch/region.js b/lib/models/patch/region.js index 4d0a14e200..c4835d4e83 100644 --- a/lib/models/patch/region.js +++ b/lib/models/patch/region.js @@ -108,7 +108,11 @@ class Region { } reMarkOn(markable) { - this.marker = markable.markRange(this.getRange(), {invalidate: 'never', exclusive: false}); + this.marker = markable.markRange( + this.constructor.layerName, + this.getRange(), + {invalidate: 'never', exclusive: false}, + ); } updateMarkers(map) { From 2cd40d3527624151f4f13c980a4404f65c7e3ce1 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 23 Jan 2019 16:20:52 -0500 Subject: [PATCH 85/92] Adapt MultiFilePatch tests to model changes --- test/models/patch/multi-file-patch.test.js | 65 ++++++++++------------ 1 file changed, 30 insertions(+), 35 deletions(-) diff --git a/test/models/patch/multi-file-patch.test.js b/test/models/patch/multi-file-patch.test.js index b5e665da38..ae2c472137 100644 --- a/test/models/patch/multi-file-patch.test.js +++ b/test/models/patch/multi-file-patch.test.js @@ -2,16 +2,14 @@ import dedent from 'dedent-js'; import {multiFilePatchBuilder, filePatchBuilder} from '../../builder/patch'; -import {TOO_LARGE} from '../../../lib/models/patch/patch'; import {DEFAULT_OPTIONS} from '../../../lib/models/patch/builder.js'; import MultiFilePatch from '../../../lib/models/patch/multi-file-patch'; import {assertInFilePatch} from '../../helpers'; - describe('MultiFilePatch', function() { - it('creates an empty patch when constructed with no arguments', function() { - const empty = new MultiFilePatch({}); + it('creates an empty patch', function() { + const empty = MultiFilePatch.createNull(); assert.isFalse(empty.anyPresent()); assert.lengthOf(empty.getFilePatches(), 0); }); @@ -52,17 +50,17 @@ describe('MultiFilePatch', function() { assert.strictEqual(dup.getFilePatches(), original.getFilePatches()); }); - it('creates a copy with a new buffer and layer set', function() { - const {buffer, layers} = multiFilePatchBuilder().build(); - const dup = original.clone({buffer, layers}); - - assert.strictEqual(dup.getBuffer(), buffer); - assert.strictEqual(dup.getPatchLayer(), layers.patch); - assert.strictEqual(dup.getHunkLayer(), layers.hunk); - assert.strictEqual(dup.getUnchangedLayer(), layers.unchanged); - assert.strictEqual(dup.getAdditionLayer(), layers.addition); - assert.strictEqual(dup.getDeletionLayer(), layers.deletion); - assert.strictEqual(dup.getNoNewlineLayer(), layers.noNewline); + it('creates a copy with a new PatchBuffer', function() { + const {multiFilePatch} = multiFilePatchBuilder().build(); + const dup = original.clone({patchBuffer: multiFilePatch.getLayeredBuffer()}); + + assert.strictEqual(dup.getBuffer(), multiFilePatch.getBuffer()); + assert.strictEqual(dup.getPatchLayer(), multiFilePatch.getPatchLayer()); + assert.strictEqual(dup.getHunkLayer(), multiFilePatch.getHunkLayer()); + assert.strictEqual(dup.getUnchangedLayer(), multiFilePatch.getUnchangedLayer()); + assert.strictEqual(dup.getAdditionLayer(), multiFilePatch.getAdditionLayer()); + assert.strictEqual(dup.getDeletionLayer(), multiFilePatch.getDeletionLayer()); + assert.strictEqual(dup.getNoNewlineLayer(), multiFilePatch.getNoNewlineLayer()); assert.strictEqual(dup.getFilePatches(), original.getFilePatches()); }); @@ -272,12 +270,11 @@ describe('MultiFilePatch', function() { +1;1;1 -1;1;2 1;1;3 - `); }); it('adopts a buffer from a previous patch', function() { - const {multiFilePatch: lastMultiPatch, buffer: lastBuffer, layers: lastLayers} = multiFilePatchBuilder() + const {multiFilePatch: lastMultiPatch} = multiFilePatchBuilder() .addFilePatch(fp => { fp.setOldFile(f => f.path('A0.txt')); fp.addHunk(h => h.unchanged('a0').added('a1').deleted('a2').unchanged('a3')); @@ -293,7 +290,7 @@ describe('MultiFilePatch', function() { }) .build(); - const {multiFilePatch: nextMultiPatch, buffer: nextBuffer, layers: nextLayers} = multiFilePatchBuilder() + const {multiFilePatch: nextMultiPatch} = multiFilePatchBuilder() .addFilePatch(fp => { fp.setOldFile(f => f.path('B0.txt')); fp.addHunk(h => h.unchanged('b0', 'b1').added('b2').unchanged('b3', 'b4')); @@ -309,20 +306,19 @@ describe('MultiFilePatch', function() { }) .build(); - assert.notStrictEqual(nextBuffer, lastBuffer); - assert.notStrictEqual(nextLayers, lastLayers); + assert.notStrictEqual(nextMultiPatch.getBuffer(), lastMultiPatch.getBuffer()); nextMultiPatch.adoptBufferFrom(lastMultiPatch); - assert.strictEqual(nextMultiPatch.getBuffer(), lastBuffer); - assert.strictEqual(nextMultiPatch.getPatchLayer(), lastLayers.patch); - assert.strictEqual(nextMultiPatch.getHunkLayer(), lastLayers.hunk); - assert.strictEqual(nextMultiPatch.getUnchangedLayer(), lastLayers.unchanged); - assert.strictEqual(nextMultiPatch.getAdditionLayer(), lastLayers.addition); - assert.strictEqual(nextMultiPatch.getDeletionLayer(), lastLayers.deletion); - assert.strictEqual(nextMultiPatch.getNoNewlineLayer(), lastLayers.noNewline); + assert.strictEqual(nextMultiPatch.getBuffer(), lastMultiPatch.getBuffer()); + assert.strictEqual(nextMultiPatch.getPatchLayer(), lastMultiPatch.getPatchLayer()); + assert.strictEqual(nextMultiPatch.getHunkLayer(), lastMultiPatch.getHunkLayer()); + assert.strictEqual(nextMultiPatch.getUnchangedLayer(), lastMultiPatch.getUnchangedLayer()); + assert.strictEqual(nextMultiPatch.getAdditionLayer(), lastMultiPatch.getAdditionLayer()); + assert.strictEqual(nextMultiPatch.getDeletionLayer(), lastMultiPatch.getDeletionLayer()); + assert.strictEqual(nextMultiPatch.getNoNewlineLayer(), lastMultiPatch.getNoNewlineLayer()); - assert.deepEqual(lastBuffer.getText(), dedent` + assert.deepEqual(nextMultiPatch.getBuffer().getText(), dedent` b0 b1 b2 @@ -337,29 +333,28 @@ describe('MultiFilePatch', function() { b11 b12 No newline at end of file - `); const assertMarkedLayerRanges = (layer, ranges) => { assert.deepEqual(layer.getMarkers().map(m => m.getRange().serialize()), ranges); }; - assertMarkedLayerRanges(lastLayers.patch, [ + assertMarkedLayerRanges(nextMultiPatch.getPatchLayer(), [ [[0, 0], [4, 2]], [[5, 0], [7, 2]], [[8, 0], [13, 26]], ]); - assertMarkedLayerRanges(lastLayers.hunk, [ + assertMarkedLayerRanges(nextMultiPatch.getHunkLayer(), [ [[0, 0], [4, 2]], [[5, 0], [7, 2]], [[8, 0], [11, 3]], [[12, 0], [13, 26]], ]); - assertMarkedLayerRanges(lastLayers.unchanged, [ + assertMarkedLayerRanges(nextMultiPatch.getUnchangedLayer(), [ [[0, 0], [1, 2]], [[3, 0], [4, 2]], [[5, 0], [6, 2]], [[8, 0], [9, 2]], [[11, 0], [11, 3]], ]); - assertMarkedLayerRanges(lastLayers.addition, [ + assertMarkedLayerRanges(nextMultiPatch.getAdditionLayer(), [ [[2, 0], [2, 2]], [[7, 0], [7, 2]], ]); - assertMarkedLayerRanges(lastLayers.deletion, [ + assertMarkedLayerRanges(nextMultiPatch.getDeletionLayer(), [ [[10, 0], [10, 3]], [[12, 0], [12, 3]], ]); - assertMarkedLayerRanges(lastLayers.noNewline, [ + assertMarkedLayerRanges(nextMultiPatch.getNoNewlineLayer(), [ [[13, 0], [13, 26]], ]); From 5af6a872e0a106ac997bc806f1c597a176395fc9 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 24 Jan 2019 09:03:44 -0500 Subject: [PATCH 86/92] Distinguish between unset and null files --- test/builder/patch.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/test/builder/patch.js b/test/builder/patch.js index dbe3f22d27..f27429cb92 100644 --- a/test/builder/patch.js +++ b/test/builder/patch.js @@ -3,6 +3,8 @@ import {buildMultiFilePatch} from '../../lib/models/patch/builder'; import File from '../../lib/models/patch/file'; +const UNSET = Symbol('unset'); + class MultiFilePatchBuilder { constructor() { this.rawFilePatches = []; @@ -27,8 +29,9 @@ class FilePatchBuilder { this._oldPath = 'file'; this._oldMode = File.modes.NORMAL; this._oldSymlink = null; - this._newPath = null; - this._newMode = null; + + this._newPath = UNSET; + this._newMode = UNSET; this._newSymlink = null; this.patchBuilder = new PatchBuilder(); @@ -92,11 +95,11 @@ class FilePatchBuilder { build(opts = {}) { const {raw: rawPatch} = this.patchBuilder.build(); - if (this._newPath === null) { + if (this._newPath === UNSET) { this._newPath = this._oldPath; } - if (this._newMode === null) { + if (this._newMode === UNSET) { this._newMode = this._oldMode; } From 8bab5ed828debc4ee41d73c31f1d9babd0700f28 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 24 Jan 2019 09:04:34 -0500 Subject: [PATCH 87/92] Set file patch status on symlink pairs --- test/views/multi-file-patch-view.test.js | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/test/views/multi-file-patch-view.test.js b/test/views/multi-file-patch-view.test.js index d3436debf5..f56be37719 100644 --- a/test/views/multi-file-patch-view.test.js +++ b/test/views/multi-file-patch-view.test.js @@ -1034,7 +1034,13 @@ describe('MultiFilePatchView', function() { const {multiFilePatch} = multiFilePatchBuilder() .addFilePatch(fp => { + fp.status('deleted'); fp.setOldFile(f => f.path('f0').symlinkTo('elsewhere')); + fp.nullNewFile(); + }) + .addFilePatch(fp => { + fp.status('added'); + fp.nullOldFile(); fp.setNewFile(f => f.path('f0')); tenLineHunk(fp); }) @@ -1043,12 +1049,25 @@ describe('MultiFilePatchView', function() { tenLineHunk(fp); }) .addFilePatch(fp => { - fp.setNewFile(f => f.path('f2')); + fp.status('deleted'); fp.setOldFile(f => f.path('f2').symlinkTo('somewhere')); + fp.nullNewFile(); + }) + .addFilePatch(fp => { + fp.status('added'); + fp.nullOldFile(); + fp.setNewFile(f => f.path('f2')); tenLineHunk(fp); }) .addFilePatch(fp => { + fp.status('deleted'); fp.setOldFile(f => f.path('f3').symlinkTo('unchanged')); + fp.nullNewFile(); + }) + .addFilePatch(fp => { + fp.status('added'); + fp.nullOldFile(); + fp.setNewFile(f => f.path('f3')); tenLineHunk(fp); }) .addFilePatch(fp => { From 97154cbeb7ef8c69b14ccdcd356f7f1dca5dba7f Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 24 Jan 2019 09:12:31 -0500 Subject: [PATCH 88/92] fp3 isn't supposed to be a symlink change? --- test/views/multi-file-patch-view.test.js | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/test/views/multi-file-patch-view.test.js b/test/views/multi-file-patch-view.test.js index f56be37719..bf775699f5 100644 --- a/test/views/multi-file-patch-view.test.js +++ b/test/views/multi-file-patch-view.test.js @@ -1060,14 +1060,7 @@ describe('MultiFilePatchView', function() { tenLineHunk(fp); }) .addFilePatch(fp => { - fp.status('deleted'); - fp.setOldFile(f => f.path('f3').symlinkTo('unchanged')); - fp.nullNewFile(); - }) - .addFilePatch(fp => { - fp.status('added'); - fp.nullOldFile(); - fp.setNewFile(f => f.path('f3')); + fp.setOldFile(f => f.path('f3')); tenLineHunk(fp); }) .addFilePatch(fp => { From 5b35a4f5da3d2473e85c683b62925e3eaf52e3fb Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 24 Jan 2019 09:12:42 -0500 Subject: [PATCH 89/92] Use MFP-level collapse and expand functions --- lib/views/multi-file-patch-view.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/views/multi-file-patch-view.js b/lib/views/multi-file-patch-view.js index bab2e28737..3fa7418f6a 100644 --- a/lib/views/multi-file-patch-view.js +++ b/lib/views/multi-file-patch-view.js @@ -352,8 +352,8 @@ export default class MultiFilePatchView extends React.Component { toggleFile={() => this.props.toggleFile(filePatch)} isCollapsed={!filePatch.getRenderStatus().isVisible()} - triggerCollapse={filePatch.triggerCollapse} - triggerExpand={filePatch.triggerExpand} + triggerCollapse={() => this.props.multiFilePatch.collapseFilePatch(filePatch)} + triggerExpand={() => this.props.multiFilePatch.expandFilePatch(filePatch)} /> {this.renderSymlinkChangeMeta(filePatch)} {this.renderExecutableModeChangeMeta(filePatch)} From ecb49ead4937e509e5fbeb0eba503e8d59c79532 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 24 Jan 2019 09:31:13 -0500 Subject: [PATCH 90/92] Retain the TextBuffer within a PatchBuffer --- lib/models/patch/patch-buffer.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/models/patch/patch-buffer.js b/lib/models/patch/patch-buffer.js index 761eec0df7..82a1c5d40f 100644 --- a/lib/models/patch/patch-buffer.js +++ b/lib/models/patch/patch-buffer.js @@ -6,6 +6,8 @@ const LAYER_NAMES = ['unchanged', 'addition', 'deletion', 'nonewline', 'hunk', ' export default class PatchBuffer { constructor() { this.buffer = new TextBuffer(); + this.buffer.retain(); + this.layers = LAYER_NAMES.reduce((map, layerName) => { map[layerName] = this.buffer.addMarkerLayer(); return map; From e6a74b011fdd9f61788eb8f30664f41adec94c8e Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 24 Jan 2019 09:31:23 -0500 Subject: [PATCH 91/92] NullPatch::reMarkOn() accepts a PatchBuffer --- lib/models/patch/patch.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/models/patch/patch.js b/lib/models/patch/patch.js index 5880e67da2..93a6c66a87 100644 --- a/lib/models/patch/patch.js +++ b/lib/models/patch/patch.js @@ -436,7 +436,7 @@ class NullPatch { } reMarkOn(markable) { - this.marker = markable.markRange(this.getRange(), {invalidate: 'never', exclusive: false}); + this.marker = markable.markRange(Patch.layerName, this.getRange(), {invalidate: 'never', exclusive: false}); } getMaxLineNumberWidth() { From aa419303625b256d48be20b8265fbb0496b44490 Mon Sep 17 00:00:00 2001 From: annthurium Date: Thu, 24 Jan 2019 15:58:43 -0800 Subject: [PATCH 92/92] nits nits nits --- lib/controllers/commit-detail-controller.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/controllers/commit-detail-controller.js b/lib/controllers/commit-detail-controller.js index 3f6014b9c7..980ff188d7 100644 --- a/lib/controllers/commit-detail-controller.js +++ b/lib/controllers/commit-detail-controller.js @@ -16,7 +16,7 @@ export default class CommitDetailController extends React.Component { this.state = { messageCollapsible: this.props.commit.isBodyLong(), messageOpen: !this.props.commit.isBodyLong(), - }; + } } render() {