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/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 ; } 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/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() { diff --git a/lib/controllers/pr-reviews-controller.js b/lib/controllers/pr-reviews-controller.js index d10839eb69..f2a836fc32 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, + isPatchTooLargeOrCollapsed: PropTypes.func.isRequired, } constructor(props) { diff --git a/lib/models/patch/builder.js b/lib/models/patch/builder.js index e8e18982a7..396ab72601 100644 --- a/lib/models/patch/builder.js +++ b/lib/models/patch/builder.js @@ -1,31 +1,43 @@ -import {TextBuffer} from 'atom'; - +import PatchBuffer from './patch-buffer'; import Hunk from './hunk'; import File, {nullFile} from './file'; -import Patch 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'; -export function buildFilePatch(diffs) { - const layeredBuffer = initializeBuffer(); +export const DEFAULT_OPTIONS = { + // Number of lines after which we consider the diff "large" + // TODO: Set this based on performance measurements + largeDiffThreshold: 100, + + // Map of file path (relative to repository root) to Patch render status (EXPANDED, COLLAPSED, TOO_LARGE) + renderStatusOverrides: {}, +}; + +export function buildFilePatch(diffs, options) { + const opts = {...DEFAULT_OPTIONS, ...options}; + + const patchBuffer = new PatchBuffer(); let filePatch; if (diffs.length === 0) { filePatch = emptyDiffFilePatch(); } else if (diffs.length === 1) { - filePatch = singleDiffFilePatch(diffs[0], layeredBuffer); + filePatch = singleDiffFilePatch(diffs[0], patchBuffer, opts); } else if (diffs.length === 2) { - filePatch = dualDiffFilePatch(diffs[0], diffs[1], layeredBuffer); + filePatch = dualDiffFilePatch(diffs[0], diffs[1], patchBuffer, opts); } else { throw new Error(`Unexpected number of diffs: ${diffs.length}`); } - return new MultiFilePatch({filePatches: [filePatch], ...layeredBuffer}); + return new MultiFilePatch({patchBuffer, filePatches: [filePatch]}); } -export function buildMultiFilePatch(diffs) { - const layeredBuffer = initializeBuffer(); +export function buildMultiFilePatch(diffs, options) { + const opts = {...DEFAULT_OPTIONS, ...options}; + + const patchBuffer = new PatchBuffer(); const byPath = new Map(); const actions = []; @@ -40,7 +52,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, patchBuffer, opts); byPath.delete(thePath); } else { // The first half we've seen. @@ -48,40 +60,28 @@ export function buildMultiFilePatch(diffs) { index++; } } else { - actions[index] = () => singleDiffFilePatch(diff, layeredBuffer); + 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); + actions[originalIndex] = () => singleDiffFilePatch(unpairedDiff, patchBuffer, opts); } 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({filePatches, ...layeredBuffer}); + return new MultiFilePatch({patchBuffer, filePatches}); } function emptyDiffFilePatch() { return FilePatch.createNull(); } -function singleDiffFilePatch(diff, layeredBuffer) { +function singleDiffFilePatch(diff, patchBuffer, opts) { 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,12 +99,41 @@ function singleDiffFilePatch(diff, layeredBuffer) { const newFile = diff.newPath !== null || diff.newMode !== null ? new File({path: diff.newPath, mode: diff.newMode, symlink: newSymlink}) : nullFile; - const patch = new Patch({status: diff.status, hunks, marker: patchMarker, buffer: layeredBuffer.buffer}); - return new FilePatch(oldFile, newFile, patch); + 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 patchMarker = patchBuffer.markPosition( + Patch.layerName, + patchBuffer.getBuffer().getEndPosition(), + {invalidate: 'never', exclusive: false}, + ); + + return FilePatch.createHiddenFilePatch( + oldFile, newFile, patchMarker, TOO_LARGE, + () => { + const subPatchBuffer = new PatchBuffer(); + const [hunks, nextPatchMarker] = buildHunks(diff, subPatchBuffer); + const nextPatch = new Patch({status: diff.status, hunks, marker: nextPatchMarker}); + return {patch: nextPatch, patchBuffer: subPatchBuffer}; + }, + ); + } else { + const [hunks, patchMarker] = buildHunks(diff, patchBuffer); + const patch = new Patch({status: diff.status, hunks, marker: patchMarker}); + + return new FilePatch(oldFile, newFile, patch); + } } -function dualDiffFilePatch(diff1, diff2, layeredBuffer) { +function dualDiffFilePatch(diff1, diff2, patchBuffer, opts) { let modeChangeDiff, contentChangeDiff; if (diff1.oldMode === File.modes.SYMLINK || diff1.newMode === File.modes.SYMLINK) { modeChangeDiff = diff1; @@ -114,7 +143,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,7 +168,13 @@ 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}); - const patch = new Patch({status, hunks, marker: patchMarker, buffer: layeredBuffer.buffer}); + + if (isDiffLarge([diff1, diff2], opts)) { + // TODO: do something with large diff too + } + + const [hunks, patchMarker] = buildHunks(contentChangeDiff, patchBuffer); + const patch = new Patch({status, hunks, marker: patchMarker}); return new FilePatch(oldFile, newFile, patch); } @@ -152,103 +186,111 @@ 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; - }, {}); +function buildHunks(diff, patchBuffer) { + const inserter = patchBuffer.createInserterAtEnd() + .keepBefore(patchBuffer.findAllMarkers({endPosition: patchBuffer.getInsertionPoint()})); - return {buffer, layers}; -} - -function buildHunks(diff, {buffer, layers}) { - const layersByKind = new Map([ - [Unchanged, layers.unchanged], - [Addition, layers.addition], - [Deletion, layers.deletion], - [NoNewline, layers.noNewline], - ]); + let patchMarker = null; + let firstHunk = true; const hunks = []; - const patchStartRow = buffer.getLastRow(); - let bufferRow = patchStartRow; - let nextLineLength = 0; - - if (diff.hunks.length === 0) { - const patchMarker = layers.patch.markPosition( - [patchStartRow, 0], - {invalidate: 'never', exclusive: false}, - ); - - return [hunks, patchMarker]; + // 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'); } - for (const hunkData of diff.hunks) { - const bufferStartRow = bufferRow; - - const regions = []; - - let LastChangeKind = null; - let currentRangeStart = bufferRow; - let lastLineLength = 0; + inserter.markWhile(Patch.layerName, () => { + for (const rawHunk of diff.hunks) { + let firstRegion = true; + const regions = []; - const finishCurrentRange = () => { - if (currentRangeStart === bufferRow) { - return; - } - - regions.push( - new LastChangeKind( - layersByKind.get(LastChangeKind).markRange( - [[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; - buffer.append(bufferLine); - - const ChangeKind = CHANGEKIND[lineText[0]]; - if (ChangeKind === undefined) { - throw new Error(`Unknown diff status character: "${lineText[0]}"`); - } - - if (ChangeKind !== LastChangeKind) { - finishCurrentRange(); + // Separate hunks with an unmarked newline + if (firstHunk) { + firstHunk = false; + } else { + inserter.insert('\n'); } - LastChangeKind = ChangeKind; - bufferRow++; - lastLineLength = nextLineLength; + inserter.markWhile(Hunk.layerName, () => { + let currentRegionText = ''; + let CurrentRegionKind = null; + + function finishRegion() { + if (currentRegionText === '' || CurrentRegionKind === null) { + return; + } + + // Separate regions with an unmarked newline + if (firstRegion) { + firstRegion = false; + } else { + inserter.insert('\n'); + } + + inserter.insertMarked(currentRegionText, CurrentRegionKind.layerName, { + invalidate: 'never', + exclusive: false, + callback: (function(_regions, _CurrentRegionKind) { + return regionMarker => { _regions.push(new _CurrentRegionKind(regionMarker)); }; + })(regions, CurrentRegionKind), + }); + } + + 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); + + if (NextRegionKind === CurrentRegionKind) { + if (currentRegionText !== '') { + currentRegionText += '\n'; + } + currentRegionText += nextLine; + continue; + } else { + finishRegion(); + + CurrentRegionKind = NextRegionKind; + currentRegionText = nextLine; + } + } + finishRegion(); + }, { + invalidate: 'never', + exclusive: false, + 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), + }); } - finishCurrentRange(); - - hunks.push(new Hunk({ - oldStartRow: hunkData.oldStartLine, - newStartRow: hunkData.newStartLine, - oldRowCount: hunkData.oldLineCount, - newRowCount: hunkData.newLineCount, - sectionHeading: hunkData.heading, - marker: layers.hunk.markRange( - [[bufferStartRow, 0], [bufferRow - 1, nextLineLength]], - {invalidate: 'never', exclusive: false}, - ), - regions, - })); - } - - const patchMarker = layers.patch.markRange( - [[patchStartRow, 0], [bufferRow - 1, nextLineLength]], - {invalidate: 'never', exclusive: false}, - ); + }, { + invalidate: 'never', + exclusive: false, + callback: marker => { patchMarker = marker; }, + }); + inserter.apply(); return [hunks, patchMarker]; } + +function isDiffLarge(diffs, opts) { + const size = diffs.reduce((diffSizeCounter, diff) => { + return diffSizeCounter + diff.hunks.reduce((hunkSizeCounter, hunk) => { + return hunkSizeCounter + hunk.lines.length; + }, 0); + }, 0); + + return size > opts.largeDiffThreshold; +} diff --git a/lib/models/patch/file-patch.js b/lib/models/patch/file-patch.js index c40bd48dd3..9196e29c6b 100644 --- a/lib/models/patch/file-patch.js +++ b/lib/models/patch/file-patch.js @@ -1,5 +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 { @@ -7,16 +9,26 @@ export default class FilePatch { return new this(nullFile, nullFile, Patch.createNull()); } + static createHiddenFilePatch(oldFile, newFile, marker, renderStatus, showFn) { + return new this(oldFile, newFile, Patch.createHiddenPatch(marker, renderStatus, showFn)); + } + constructor(oldFile, newFile, patch) { this.oldFile = oldFile; this.newFile = newFile; this.patch = patch; + + this.emitter = new Emitter(); } isPresent() { return this.oldFile.isPresent() || this.newFile.isPresent() || this.patch.isPresent(); } + getRenderStatus() { + return this.patch.getRenderStatus(); + } + getOldFile() { return this.oldFile; } @@ -107,6 +119,60 @@ export default class FilePatch { return this.getPatch().getHunks(); } + updateMarkers(map) { + return this.patch.updateMarkers(map); + } + + triggerCollapseIn(patchBuffer) { + if (!this.patch.getRenderStatus().isVisible()) { + return false; + } + + const oldPatch = this.patch; + 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.didChangeRenderStatus(); + return true; + } + + triggerExpandIn(patchBuffer, {before, after}) { + if (this.patch.getRenderStatus().isVisible()) { + return false; + } + + 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) + .insert(atStart ? '' : '\n') + .insertPatchBuffer(subPatchBuffer, {callback: map => nextPatch.updateMarkers(map)}) + .apply(); + + this.patch.destroyMarkers(); + this.patch = nextPatch; + this.didChangeRenderStatus(); + return true; + } + + didChangeRenderStatus() { + return this.emitter.emit('change-render-status', this); + } + + onDidChangeRenderStatus(callback) { + return this.emitter.on('change-render-status', callback); + } + clone(opts = {}) { return new this.constructor( opts.oldFile !== undefined ? opts.oldFile : this.oldFile, @@ -115,6 +181,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') { @@ -200,6 +274,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 84cab39002..8bff265dd4 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, @@ -136,10 +138,50 @@ 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) { + this.marker = map.get(this.marker) || this.marker; + for (const region of this.regions) { + region.updateMarkers(map); + } + } + + 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'); } + + /* + * 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 e21ae6158f..b9f7f7572f 100644 --- a/lib/models/patch/multi-file-patch.js +++ b/lib/models/patch/multi-file-patch.js @@ -1,20 +1,20 @@ -import {TextBuffer, Range} from 'atom'; +import {Range} from 'atom'; import {RBTree} from 'bintrees'; -export default class MultiFilePatch { - constructor({buffer, layers, filePatches}) { - this.buffer = buffer || null; +import PatchBuffer from './patch-buffer'; +import {TOO_LARGE, COLLAPSED} from './patch'; - 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; +export default class MultiFilePatch { + static createNull() { + return new this({patchBuffer: new PatchBuffer(), filePatches: []}); + } - this.filePatches = filePatches || []; + constructor({patchBuffer, filePatches}) { + this.patchBuffer = patchBuffer; + 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 @@ -22,6 +22,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; @@ -44,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(), - }, + patchBuffer: opts.patchBuffer !== undefined ? opts.patchBuffer : this.getLayeredBuffer(), filePatches: opts.filePatches !== undefined ? opts.filePatches : this.getFilePatches(), }); } + getLayeredBuffer() { + return this.patchBuffer; + } + 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() { @@ -101,10 +98,10 @@ export default class MultiFilePatch { } getFilePatchAt(bufferRow) { - if (bufferRow < 0) { + if (bufferRow < 0 || bufferRow > this.patchBuffer.getBuffer().getLastRow()) { return undefined; } - const [marker] = this.patchLayer.findMarkers({intersectsRow: bufferRow}); + const [marker] = this.patchBuffer.findMarkers('patch', {intersectsRow: bufferRow}); return this.filePatchesByMarker.get(marker); } @@ -112,16 +109,16 @@ export default class MultiFilePatch { if (bufferRow < 0) { return undefined; } - const [marker] = this.hunkLayer.findMarkers({intersectsRow: bufferRow}); + const [marker] = this.patchBuffer.findMarkers('hunk', {intersectsRow: bufferRow}); return this.hunksByMarker.get(marker); } getStagePatchForLines(selectedLineSet) { - const nextLayeredBuffer = this.buildLayeredBuffer(); + const nextLayeredBuffer = new PatchBuffer(); const nextFilePatches = this.getFilePatchesContaining(selectedLineSet).map(fp => { return fp.buildStagePatchForLines(this.getBuffer(), nextLayeredBuffer, selectedLineSet); }); - return this.clone({...nextLayeredBuffer, filePatches: nextFilePatches}); + return this.clone({patchBuffer: nextLayeredBuffer, filePatches: nextFilePatches}); } getStagePatchForHunk(hunk) { @@ -129,11 +126,11 @@ export default class MultiFilePatch { } getUnstagePatchForLines(selectedLineSet) { - const nextLayeredBuffer = this.buildLayeredBuffer(); + const nextLayeredBuffer = new PatchBuffer(); const nextFilePatches = this.getFilePatchesContaining(selectedLineSet).map(fp => { return fp.buildUnstagePatchForLines(this.getBuffer(), nextLayeredBuffer, selectedLineSet); }); - return this.clone({...nextLayeredBuffer, filePatches: nextFilePatches}); + return this.clone({patchBuffer: nextLayeredBuffer, filePatches: nextFilePatches}); } getUnstagePatchForHunk(hunk) { @@ -216,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.patchBuffer = nextLayeredBuffer; } /* @@ -300,7 +262,7 @@ export default class MultiFilePatch { } anyPresent() { - return this.buffer !== null && this.filePatches.some(fp => fp.isPresent()); + return this.patchBuffer !== null && this.filePatches.some(fp => fp.isPresent()); } didAnyChangeExecutableMode() { @@ -339,6 +301,70 @@ export default class MultiFilePatch { return false; } + 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) { + const index = this.filePatches.indexOf(filePatch); + + this.filePatchesByMarker.delete(filePatch.getMarker()); + for (const hunk of filePatch.getHunks()) { + this.hunksByMarker.delete(hunk.getMarker()); + } + + 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 after = []; + let afterIndex = index + 1; + while (afterIndex < this.filePatches.length) { + const afterFilePatch = this.filePatches[afterIndex]; + after.push(...afterFilePatch.getStartingMarkers()); + + if (!afterFilePatch.getMarker().getRange().isEmpty()) { + break; + } + afterIndex++; + } + + 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 => { + const patch = this.filePatchesByPath.get(filePatchPath); + if (!patch) { + return null; + } + const renderStatus = patch.getRenderStatus(); + return renderStatus === TOO_LARGE || renderStatus === COLLAPSED; + } + getBufferRowForDiffPosition = (fileName, diffRow) => { const {startBufferRow, index} = this.diffRowOffsetIndices.get(fileName); const {offset} = index.lowerBound({diffRow}).data(); @@ -349,7 +375,21 @@ 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(''); + } + + /* + * Construct a string of diagnostic information useful for debugging. + */ + 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`; + for (const filePatch of this.filePatches) { + inspectString += filePatch.inspect({indent: 2}); + } + inspectString += ')\n'; + return inspectString; } isEqual(other) { diff --git a/lib/models/patch/patch-buffer.js b/lib/models/patch/patch-buffer.js new file mode 100644 index 0000000000..82a1c5d40f --- /dev/null +++ b/lib/models/patch/patch-buffer.js @@ -0,0 +1,248 @@ +import {TextBuffer, Range, Point} from 'atom'; +import {inspect} from 'util'; + +const LAYER_NAMES = ['unchanged', 'addition', 'deletion', 'nonewline', 'hunk', 'patch']; + +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; + }, {}); + } + + getBuffer() { + return this.buffer; + } + + getInsertionPoint() { + return this.buffer.getEndPosition(); + } + + getLayer(layerName) { + return this.layers[layerName]; + } + + findMarkers(layerName, ...args) { + 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); + } + + markRange(layerName, ...args) { + return this.layers[layerName].markRange(...args); + } + + clearAllLayers() { + for (const layerName of LAYER_NAMES) { + this.layers[layerName].clear(); + } + } + + createInserterAt(insertionPoint) { + return new Inserter(this, Point.fromObject(insertionPoint)); + } + + createInserterAtEnd() { + return this.createInserterAt(this.getInsertionPoint()); + } + + extractPatchBuffer(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; + }, {}); + 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 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(startOffset, endOffset), + oldMarker.getProperties(), + ); + markerMap.set(oldMarker, newMarker); + oldMarker.destroy(); + } + } + + this.buffer.setTextInRange(range, ''); + return {patchBuffer: subBuffer, markerMap}; + } + + inspect(opts = {}) { + const options = { + layerNames: LAYER_NAMES, + ...opts, + }; + + let inspectString = ''; + + const increasingMarkers = []; + 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}); + } + } + 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 { + constructor(patchBuffer, insertionPoint) { + const clipped = patchBuffer.getBuffer().clipPosition(insertionPoint); + + this.patchBuffer = patchBuffer; + this.startPoint = clipped.copy(); + this.insertionPoint = clipped.copy(); + this.markerBlueprints = []; + this.markerMapCallbacks = []; + + 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); + } + } + return this; + } + + keepAfter(markers) { + for (const marker of markers) { + if (marker.getRange().end.isEqual(this.startPoint)) { + this.markersAfter.add(marker); + } + } + return this; + } + + 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; + return this; + } + + insertMarked(text, layerName, markerOpts) { + return this.markWhile(layerName, () => this.insert(text), markerOpts); + } + + insertPatchBuffer(subPatchBuffer, opts) { + const baseOffset = this.insertionPoint.copy(); + this.insert(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}); + } + } + + if (opts.callback) { + this.markerMapCallbacks.push({markerMap: subMarkerMap, callback: opts.callback}); + } + + 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); + } + } + + for (const {markerMap, callback} of this.markerMapCallbacks) { + callback(markerMap); + } + + 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/lib/models/patch/patch.js b/lib/models/patch/patch.js index 0fdafa0011..93a6c66a87 100644 --- a/lib/models/patch/patch.js +++ b/lib/models/patch/patch.js @@ -3,11 +3,35 @@ import {TextBuffer, Range} from 'atom'; import Hunk from './hunk'; import {Unchanged, Addition, Deletion, NoNewline} from './region'; +export const EXPANDED = { + toString() { return 'RenderStatus(expanded)'; }, + + isVisible() { return true; }, +}; + +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(); } + static createHiddenPatch(marker, renderStatus, showFn) { + return new HiddenPatch(marker, renderStatus, showFn); + } + constructor({status, hunks, marker}) { this.status = status; this.hunks = hunks; @@ -45,8 +69,26 @@ export default class Patch { return this.marker.getRange().intersectsRow(row); } - reMarkOn(markable) { - this.marker = markable.markRange(this.getRange(), {invalidate: 'never', exclusive: false}); + reMarkOn(patchBuffer) { + this.marker = patchBuffer.markRange( + this.constructor.layerName, + this.getRange(), + {invalidate: 'never', exclusive: false}, + ); + } + + 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) { + hunk.updateMarkers(map); + } } getMaxLineNumberWidth() { @@ -62,6 +104,34 @@ 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) { const originalBaseOffset = this.getMarker().getRange().start.row; const builder = new BufferBuilder(originalBuffer, originalBaseOffset, nextLayeredBuffer); @@ -141,9 +211,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(); @@ -236,9 +308,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}); } @@ -262,9 +336,69 @@ 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; } + + getRenderStatus() { + return EXPANDED; + } +} + +class HiddenPatch extends Patch { + constructor(marker, renderStatus, showFn) { + super({status: null, hunks: [], marker}); + + this.renderStatus = renderStatus; + this.show = showFn; + } + + getInsertionPoint() { + return this.getRange().end; + } + + 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 { @@ -302,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() { @@ -313,7 +447,8 @@ class NullPatch { if ( opts.status === undefined && opts.hunks === undefined && - opts.marker === undefined + opts.marker === undefined && + opts.renderStatus === undefined ) { return this; } else { @@ -321,10 +456,19 @@ 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(), }); } } + getStartingMarkers() { + return []; + } + + getEndingMarkers() { + return []; + } + buildStagePatchForLines() { return this; } @@ -341,29 +485,41 @@ 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; } + + getRenderStatus() { + return EXPANDED; + } } 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 // TextBuffer. - this.offset = this.buffer.getLastRow() - originalBaseOffset; + this.offset = this.nextLayeredBuffer.getBuffer().getLastRow() - originalBaseOffset; this.hunkBufferText = ''; this.hunkRowCount = 0; @@ -406,15 +562,13 @@ class BufferBuilder { } latestHunkWasIncluded() { - this.buffer.append(this.hunkBufferText, {normalizeLineEndings: false}); + this.nextLayeredBuffer.buffer.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}), - ); + return new RegionKind(this.nextLayeredBuffer.layers[RegionKind.layerName]); }); - 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; @@ -437,18 +591,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..c4835d4e83 100644 --- a/lib/models/patch/region.js +++ b/lib/models/patch/region.js @@ -108,7 +108,19 @@ 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) { + this.marker = map.get(this.marker) || this.marker; + } + + destroyMarkers() { + this.marker.destroy(); } toStringIn(buffer) { @@ -117,6 +129,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; } @@ -125,6 +154,8 @@ class Region { export class Addition extends Region { static origin = '+'; + static layerName = 'addition'; + isAddition() { return true; } @@ -137,6 +168,8 @@ export class Addition extends Region { export class Deletion extends Region { static origin = '-'; + static layerName = 'deletion'; + isDeletion() { return true; } @@ -149,6 +182,8 @@ export class Deletion extends Region { export class Unchanged extends Region { static origin = ' '; + static layerName = 'unchanged'; + isUnchanged() { return true; } @@ -165,6 +200,8 @@ export class Unchanged extends Region { export class NoNewline extends Region { static origin = '\\'; + static layerName = 'nonewline'; + isNoNewline() { return true; } 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)); }); } diff --git a/lib/views/file-patch-header-view.js b/lib/views/file-patch-header-view.js index 1425221120..56ba16c466 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,9 +26,15 @@ 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, itemType: ItemTypePropType.isRequired, + + isCollapsed: PropTypes.bool.isRequired, + triggerExpand: PropTypes.func.isRequired, + triggerCollapse: PropTypes.func.isRequired, }; constructor(props) { @@ -42,12 +49,32 @@ export default class FilePatchHeaderView extends React.Component {
{this.renderTitle()} + {this.renderCollapseButton()} {this.renderButtonGroup()}
); } + togglePatchCollapse = () => { + if (this.props.isCollapsed) { + this.props.triggerExpand(); + } else { + this.props.triggerCollapse(); + } + } + + renderCollapseButton() { + const icon = this.props.isCollapsed ? 'chevron-up' : 'chevron-down'; + return ( + + ); + } + renderTitle() { if (this.props.itemType === ChangedFileItem) { const status = this.props.stagingStatus; diff --git a/lib/views/multi-file-patch-view.js b/lib/views/multi-file-patch-view.js index 9d0655b770..3fa7418f6a 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', @@ -319,6 +321,7 @@ export default class MultiFilePatchView extends React.Component { ); } else { @@ -347,17 +350,37 @@ 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().isVisible()} + triggerCollapse={() => this.props.multiFilePatch.collapseFilePatch(filePatch)} + triggerExpand={() => this.props.multiFilePatch.expandFilePatch(filePatch)} /> {this.renderSymlinkChangeMeta(filePatch)} {this.renderExecutableModeChangeMeta(filePatch)} + {filePatch.getPatch().getRenderStatus() === TOO_LARGE && this.renderDiffGate(filePatch)} + {this.renderHunkHeaders(filePatch)} ); } + renderDiffGate(filePatch) { + const showDiff = () => filePatch.triggerDelayedRender(); + return ( + + +

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

+ + + ); + } + renderExecutableModeChangeMeta(filePatch) { if (!filePatch.didChangeExecutableMode()) { return null; diff --git a/lib/views/pr-review-comments-view.js b/lib/views/pr-review-comments-view.js index 80e89af272..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, @@ -23,6 +24,7 @@ export default class PullRequestCommentsView extends React.Component { })), getBufferRowForDiffPosition: PropTypes.func.isRequired, switchToIssueish: PropTypes.func.isRequired, + isPatchTooLargeOrCollapsed: PropTypes.func.isRequired, } render() { @@ -32,6 +34,10 @@ export default class PullRequestCommentsView extends React.Component { return null; } + // 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); 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; diff --git a/test/builder/patch.js b/test/builder/patch.js index 18ff5067b5..f27429cb92 100644 --- a/test/builder/patch.js +++ b/test/builder/patch.js @@ -1,114 +1,79 @@ // Builders for classes related to MultiFilePatches. -import {TextBuffer} from 'atom'; -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'; - -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; - }, {}); - } +import {buildMultiFilePatch} from '../../lib/models/patch/builder'; +import File from '../../lib/models/patch/file'; - 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; - } - - markFrom(markerLayerName, startPosition) { - const endPosition = this.buffer.getEndPosition().translate([-1, Infinity]); - const layer = this.getLayer(markerLayerName); - return layer.markRange([startPosition, endPosition], {exclusive: true}); - } - - wrapReturn(object) { - return { - buffer: this.buffer, - layers: this.layers, - ...object, - }; - } -} +const UNSET = Symbol('unset'); class MultiFilePatchBuilder { - constructor(layeredBuffer = null) { - this.layeredBuffer = layeredBuffer; - - this.filePatches = []; + constructor() { + this.rawFilePatches = []; } addFilePatch(block = () => {}) { - const filePatch = new FilePatchBuilder(this.layeredBuffer); + const filePatch = new FilePatchBuilder(); block(filePatch); - this.filePatches.push(filePatch.build().filePatch); + this.rawFilePatches.push(filePatch.build().raw); return this; } - build() { - return this.layeredBuffer.wrapReturn({ - multiFilePatch: new MultiFilePatch({ - buffer: this.layeredBuffer.buffer, - layers: this.layeredBuffer.layers, - filePatches: this.filePatches, - }), - }); + build(opts = {}) { + const raw = this.rawFilePatches; + const multiFilePatch = buildMultiFilePatch(raw, opts); + return {raw, multiFilePatch}; } } class FilePatchBuilder { - constructor(layeredBuffer = null) { - this.layeredBuffer = layeredBuffer; + constructor() { + this._oldPath = 'file'; + this._oldMode = File.modes.NORMAL; + this._oldSymlink = null; - this.oldFile = new File({path: 'file', mode: File.modes.NORMAL}); - this.newFile = null; + this._newPath = UNSET; + this._newMode = UNSET; + this._newSymlink = null; - this.patchBuilder = new PatchBuilder(this.layeredBuffer); + 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.empty(); + 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.empty(); + this._newSymlink = rawFile.symlink; + } return this; } nullNewFile() { - this.newFile = nullFile; + this._newPath = null; + this._newMode = null; + this._newSymlink = null; return this; } @@ -127,16 +92,51 @@ class FilePatchBuilder { return this; } - build() { - const {patch} = this.patchBuilder.build(); + build(opts = {}) { + const {raw: rawPatch} = this.patchBuilder.build(); + + if (this._newPath === UNSET) { + this._newPath = this._oldPath; + } + + if (this._newMode === UNSET) { + this._newMode = this._oldMode; + } + + 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 hb = new HunkBuilder(); + if (this._oldSymlink !== null) { + hb.unchanged(this._oldSymlink); + } + if (this._oldSymlink !== null && this._newSymlink !== null) { + hb.unchanged('--'); + } + if (this._newSymlink !== null) { + hb.unchanged(this._newSymlink); + } - if (this.newFile === null) { - this.newFile = this.oldFile.clone(); + rawPatch.hunks = [hb.build().raw]; } - return this.layeredBuffer.wrapReturn({ - filePatch: new FilePatch(this.oldFile, this.newFile, patch), - }); + 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, + }; } } @@ -158,27 +158,23 @@ 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(layeredBuffer = null) { - this.layeredBuffer = layeredBuffer; - + constructor() { this._status = 'modified'; - this.hunks = []; - - this.patchStart = this.layeredBuffer.getInsertionPoint(); + this.rawHunks = []; this.drift = 0; this.explicitlyEmpty = false; } @@ -193,10 +189,10 @@ class PatchBuilder { } addHunk(block = () => {}) { - const builder = new HunkBuilder(this.layeredBuffer, 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; } @@ -207,7 +203,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')); @@ -218,17 +214,27 @@ class PatchBuilder { } } - const marker = this.layeredBuffer.markFrom('patch', this.patchStart); + const raw = { + status: this._status, + hunks: this.rawHunks, + }; + + 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 this.layeredBuffer.wrapReturn({ - patch: new Patch({status: this._status, hunks: this.hunks, marker}), - }); + return {raw, patch}; } } class HunkBuilder { - constructor(layeredBuffer = null, drift = 0) { - this.layeredBuffer = layeredBuffer; + constructor(drift = 0) { this.drift = drift; this.oldStartRow = 0; @@ -238,8 +244,7 @@ class HunkBuilder { this.sectionHeading = "don't care"; - this.hunkStartPoint = this.layeredBuffer.getInsertionPoint(); - this.regions = []; + this.lines = []; } oldRow(rowNumber) { @@ -248,36 +253,38 @@ class HunkBuilder { } unchanged(...lines) { - this.regions.push(new Unchanged(this.layeredBuffer.appendMarked('unchanged', lines))); + for (const line of lines) { + this.lines.push(` ${line}`); + } return this; } added(...lines) { - this.regions.push(new Addition(this.layeredBuffer.appendMarked('addition', lines))); + for (const line of lines) { + this.lines.push(`+${line}`); + } return this; } deleted(...lines) { - this.regions.push(new Deletion(this.layeredBuffer.appendMarked('deletion', lines))); + for (const line of lines) { + this.lines.push(`-${line}`); + } return this; } noNewline() { - this.regions.push(new NoNewline(this.layeredBuffer.appendMarked('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) { @@ -285,42 +292,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 = this.layeredBuffer.markFrom('hunk', this.hunkStartPoint); - - return this.layeredBuffer.wrapReturn({ - 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 LayeredBuffer()); + return new MultiFilePatchBuilder(); } export function filePatchBuilder() { - return new FilePatchBuilder(new LayeredBuffer()); + return new FilePatchBuilder(); } export function patchBuilder() { - return new PatchBuilder(new LayeredBuffer()); + return new PatchBuilder(); } export function hunkBuilder() { - return new HunkBuilder(new LayeredBuffer()); + return new HunkBuilder(); } 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, }; } } 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); + }); }); 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); + }); }); diff --git a/test/controllers/pr-reviews-controller.test.js b/test/controllers/pr-reviews-controller.test.js index 83cfa8eadd..fcd44eacc8 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: () => {}, + isPatchTooLargeOrCollapsed: () => {}, pullRequest: {reviews}, ...overrideProps, }; diff --git a/test/models/patch/builder.test.js b/test/models/patch/builder.test.js index 8e64488f05..9b0ca9f957 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() { @@ -78,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( @@ -112,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]]}, ], }, ); @@ -228,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( @@ -237,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]]}, ], }, ); @@ -275,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( @@ -284,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]]}, ], }, ); @@ -318,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, @@ -327,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]]}, ], }); }); @@ -382,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]]}, ], }); }); @@ -441,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]]}, ], }); }); @@ -499,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]]}, ], }); }); @@ -598,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'); @@ -636,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]]}, ], }, ); @@ -739,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]]}, ], }); }); @@ -793,13 +794,273 @@ 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]]); }); }); + describe('with a large diff', 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', + 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', range: [[1, 0], [1, 6]]}, + ], + }, + ); + }); + + it('re-parse a HiddenPatch 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(); + + mfp.expandFilePatch(fp); + + 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', range: [[3, 0], [3, 6]]}, + ], + }, + ); + }); + + 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', + 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', range: [[7, 0], [7, 6]]}, + ], + }, + ); + + mfp.expandFilePatch(fp1); + + 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', range: [[12, 0], [12, 6]]}, + ], + }, + ); + }); + + 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', + 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]]); + + mfp.expandFilePatch(fp1); + + 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(), [[3, 6], [3, 6]]); + }); + + 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', + 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,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', 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/); }); 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) { diff --git a/test/models/patch/multi-file-patch.test.js b/test/models/patch/multi-file-patch.test.js index cad02eaffa..ae2c472137 100644 --- a/test/models/patch/multi-file-patch.test.js +++ b/test/models/patch/multi-file-patch.test.js @@ -2,12 +2,14 @@ import dedent from 'dedent-js'; import {multiFilePatchBuilder, filePatchBuilder} from '../../builder/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); }); @@ -48,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()); }); @@ -268,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')); @@ -289,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')); @@ -305,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 @@ -333,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]], ]); @@ -704,6 +703,42 @@ describe('MultiFilePatch', function() { }); }); + describe('isPatchTooLargeOrCollapsed', function() { + let multiFilePatch; + const {largeDiffThreshold} = DEFAULT_OPTIONS; + 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() diff --git a/test/models/patch/patch-buffer.test.js b/test/models/patch/patch-buffer.test.js new file mode 100644 index 0000000000..95c65fe364 --- /dev/null +++ b/test/models/patch/patch-buffer.test.js @@ -0,0 +1,272 @@ +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(), [10, 0]); + }); + + 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); + }); + + it('extracts a subset of the buffer and layers as a new LayeredBuffer', function() { + 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 {patchBuffer: subPatchBuffer, markerMap} = patchBuffer.extractPatchBuffer([[4, 2], [7, 1]]); + + assert.strictEqual(patchBuffer.getBuffer().getText(), dedent` + 0000 + 0001 + 0002 + 0003 + 00007 + 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, 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 + 0005 + 0006 + 0 + `); + 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, 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() { + it('performs multiple modifications and only creates markers at the end', function() { + const inserter = patchBuffer.createInserterAtEnd(); + const cb0 = sinon.spy(); + const cb1 = sinon.spy(); + const cb2 = sinon.spy(); + + 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 + 0011 + 0012 + 0013 + 0014 + + `); + + 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); + + inserter.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)); + + 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() { + const inserter = patchBuffer.createInserterAt([4, 2]); + const callback = sinon.spy(); + + 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 + 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); + + inserter.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 inserter = patchBuffer.createInserterAt([4, 0]); + inserter.keepBefore([before0, before1]); + inserter.keepAfter([after0]); + + let marker = null; + const callback = m => { marker = m; }; + inserter.insertMarked('A\nB\nC\nD\nE\n', 'addition', {callback}); + + inserter.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]]); + }); + + it('appends another PatchBuffer at its insertion point', function() { + const subPatchBuffer = new PatchBuffer(); + subPatchBuffer.getBuffer().setText(dedent` + aaaa + bbbb + cc + `); + + 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, {callback: m => { markerMap = m; }}) + .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.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]]); + }); + }); +}); + +const TEXT = dedent` + 0000 + 0001 + 0002 + 0003 + 0004 + 0005 + 0006 + 0007 + 0008 + 0009 + +`; diff --git a/test/models/patch/patch.test.js b/test/models/patch/patch.test.js index b5989ab3fe..8307d832b7 100644 --- a/test/models/patch/patch.test.js +++ b/test/models/patch/patch.test.js @@ -1,6 +1,7 @@ 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'; @@ -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() { diff --git a/test/views/multi-file-patch-view.test.js b/test/views/multi-file-patch-view.test.js index d3436debf5..bf775699f5 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,18 @@ 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.setOldFile(f => f.path('f3').symlinkTo('unchanged')); + fp.setOldFile(f => f.path('f3')); tenLineHunk(fp); }) .addFilePatch(fp => { 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);