diff --git a/spec/text-buffer-spec.coffee b/spec/text-buffer-spec.coffee index 7f125557f1..c7c35dfcea 100644 --- a/spec/text-buffer-spec.coffee +++ b/spec/text-buffer-spec.coffee @@ -138,7 +138,7 @@ describe "TextBuffer", -> buffer.setTextInRange([[0, 10], [0, 100]], "w", {undo: 'skip'}) expect(buffer.lineForRow(0)).toBe "yellow" - buffer.undo() + expect(buffer.undo()).toBe true expect(buffer.lineForRow(0)).toBe "hellow" describe "when the normalizeLineEndings argument is true (the default)", -> @@ -493,6 +493,44 @@ describe "TextBuffer", -> buffer.redo() expect(buffer.getText()).toBe "heyyyyy!!\nworms\r\nhow are you doing?" + it "allows undo/redo within transactions, but not beyond the start of the containing transaction", -> + buffer.setText("") + buffer.markPosition([0, 0]) + + buffer.append("a") + + buffer.transact -> + buffer.append("b") + buffer.transact -> buffer.append("c") + buffer.append("d") + + expect(buffer.undo()).toBe true + expect(buffer.getText()).toBe "abc" + + expect(buffer.undo()).toBe true + expect(buffer.getText()).toBe "ab" + + expect(buffer.undo()).toBe true + expect(buffer.getText()).toBe "a" + + expect(buffer.undo()).toBe false + expect(buffer.getText()).toBe "a" + + expect(buffer.redo()).toBe true + expect(buffer.getText()).toBe "ab" + + expect(buffer.redo()).toBe true + expect(buffer.getText()).toBe "abc" + + expect(buffer.redo()).toBe true + expect(buffer.getText()).toBe "abcd" + + expect(buffer.redo()).toBe false + expect(buffer.getText()).toBe "abcd" + + expect(buffer.undo()).toBe true + expect(buffer.getText()).toBe "a" + describe "checkpoints", -> beforeEach -> buffer = new TextBuffer @@ -525,6 +563,7 @@ describe "TextBuffer", -> buffer.append("three\n") buffer.append("four") + marker = buffer.markRange([[0, 1], [2, 3]]) result = buffer.groupChangesSinceCheckpoint(checkpoint) expect(result).toBe true @@ -546,6 +585,8 @@ describe "TextBuffer", -> four """ + expect(marker.getRange()).toEqual [[0, 1], [2, 3]] + it "skips any later checkpoints when grouping changes", -> buffer.append("one\n") checkpoint = buffer.createCheckpoint() @@ -633,6 +674,22 @@ describe "TextBuffer", -> expect(buffer.revertToCheckpoint(checkpoint)).toBe(false) expect(buffer.getText()).toBe("world") + it "does not allow changes based on checkpoints outside of the current transaction", -> + checkpoint = buffer.createCheckpoint() + + buffer.append("a") + + buffer.transact -> + expect(buffer.revertToCheckpoint(checkpoint)).toBe false + expect(buffer.getText()).toBe "a" + + buffer.append("b") + + expect(buffer.groupChangesSinceCheckpoint(checkpoint)).toBe false + + buffer.undo() + expect(buffer.getText()).toBe "a" + describe "::getTextInRange(range)", -> it "returns the text in a given range", -> buffer = new TextBuffer(text: "hello\nworld\r\nhow are you doing?") diff --git a/src/history.coffee b/src/history.coffee index 2ef2bf6d24..dbaf4bbba6 100644 --- a/src/history.coffee +++ b/src/history.coffee @@ -3,11 +3,19 @@ _ = require 'underscore-plus' SerializationVersion = 2 class Checkpoint - constructor: (@id, @snapshot) -> + constructor: (@id, @snapshot, @isBoundary) -> unless @snapshot? global.atom?.assert(false, "Checkpoint created without snapshot") @snapshot = {} +class GroupStart + constructor: (@snapshot) -> + +class GroupEnd + constructor: (@snapshot) -> + @timestamp = Date.now() + @groupingInterval = 0 + # Manages undo/redo for {TextBuffer} module.exports = class History @@ -21,75 +29,191 @@ class History @undoStack = [] @redoStack = [] - createCheckpoint: (snapshot) -> - checkpoint = new Checkpoint(@nextCheckpointId++, snapshot) + createCheckpoint: (snapshot, isBoundary) -> + checkpoint = new Checkpoint(@nextCheckpointId++, snapshot, isBoundary) @undoStack.push(checkpoint) checkpoint.id - groupChangesSinceCheckpoint: (checkpointId) -> - checkpointIndex = @getCheckpointIndex(checkpointId) - return false unless checkpointIndex? - for i in [(@undoStack.length - 2)...checkpointIndex] by -1 - @undoStack.splice(i, 1) if @undoStack[i] instanceof Checkpoint - true + groupChangesSinceCheckpoint: (checkpointId, endSnapshot, deleteCheckpoint=false) -> + withinGroup = false + checkpointIndex = null + startSnapshot = null + changesSinceCheckpoint = [] - applyCheckpointGroupingInterval: (checkpointId, now, groupingInterval) -> - return if groupingInterval is 0 + for entry, i in @undoStack by -1 + break if checkpointIndex? - checkpointIndex = @getCheckpointIndex(checkpointId) - return unless checkpointIndex? + switch entry.constructor + when GroupEnd + withinGroup = true + when GroupStart + if withinGroup + withinGroup = false + else + return false + when Checkpoint + if entry.id is checkpointId + checkpointIndex = i + startSnapshot = entry.snapshot + else if entry.isBoundary + return false + else + changesSinceCheckpoint.unshift(entry) - i = lastCheckpointIndex = checkpointIndex - while entry = @undoStack[--i] - if entry instanceof Checkpoint - if (entry.timestamp + Math.min(entry.groupingInterval, groupingInterval)) <= now - break - @undoStack.splice(lastCheckpointIndex, 1) - lastCheckpointIndex = i + if checkpointIndex? + if changesSinceCheckpoint.length > 0 + @undoStack.splice(checkpointIndex + 1) + @undoStack.push(new GroupStart(startSnapshot)) + @undoStack.push(changesSinceCheckpoint...) + @undoStack.push(new GroupEnd(endSnapshot)) + if deleteCheckpoint + @undoStack.splice(checkpointIndex, 1) + true + else + false - @undoStack[lastCheckpointIndex].timestamp = now - @undoStack[lastCheckpointIndex].groupingInterval = groupingInterval + applyGroupingInterval: (groupingInterval) -> + topEntry = @undoStack[@undoStack.length - 1] + if topEntry instanceof GroupEnd + topEntry.groupingInterval = groupingInterval + else + return - setCheckpointGroupingInterval: (checkpointId, timestamp, groupingInterval) -> - checkpointIndex = @getCheckpointIndex(checkpointId) - checkpoint = @undoStack[checkpointIndex] - checkpoint.timestamp = timestamp - checkpoint.groupingInterval = groupingInterval + return if groupingInterval is 0 + + for entry, i in @undoStack by -1 + if entry instanceof GroupStart + previousEntry = @undoStack[i - 1] + if previousEntry instanceof GroupEnd + if (topEntry.timestamp - previousEntry.timestamp < Math.min(previousEntry.groupingInterval, groupingInterval)) + @undoStack.splice(i - 1, 2) + return + + throw new Error("Didn't find matching group-start entry") pushChange: (change) -> @undoStack.push(change) @clearRedoStack() popUndoStack: (currentSnapshot) -> - if (checkpointIndex = @getBoundaryCheckpointIndex(@undoStack))? - pop = @popChanges(@undoStack, @redoStack, checkpointIndex) - _.defaults(pop.snapshotAbove, currentSnapshot) + snapshotAbove = null + snapshotBelow = null + spliceIndex = null + withinGroup = false + invertedChanges = [] + + for entry, i in @undoStack by -1 + break if spliceIndex? + + switch entry.constructor + when GroupStart + if withinGroup + snapshotBelow = entry.snapshot + spliceIndex = i + else + return false + when GroupEnd + if withinGroup + throw new Error("Invalid undo stack state") + else + snapshotAbove = entry.snapshot + withinGroup = true + when Checkpoint + if entry.isBoundary + return false + else + invertedChanges.push(@delegate.invertChange(entry)) + unless withinGroup + spliceIndex = i + + if spliceIndex? + _.defaults(snapshotAbove, currentSnapshot) if snapshotAbove? + @redoStack.push(@undoStack.splice(spliceIndex).reverse()...) { - snapshot: pop.snapshotBelow - changes: (@delegate.invertChange(change) for change in pop.changes) + snapshot: snapshotBelow + changes: invertedChanges } else false popRedoStack: (currentSnapshot) -> - if (checkpointIndex = @getBoundaryCheckpointIndex(@redoStack))? - checkpointIndex-- while @redoStack[checkpointIndex - 1] instanceof Checkpoint - checkpointIndex++ if @redoStack[checkpointIndex - 1]? - pop = @popChanges(@redoStack, @undoStack, checkpointIndex, true) - _.defaults(pop.snapshotAbove, currentSnapshot) + snapshotAbove = null + snapshotBelow = null + spliceIndex = null + withinGroup = false + changes = [] + + for entry, i in @redoStack by -1 + break if spliceIndex? + + switch entry.constructor + when GroupEnd + if withinGroup + snapshotBelow = entry.snapshot + spliceIndex = i + else + return false + when GroupStart + if withinGroup + throw new Error("Invalid redo stack state") + else + snapshotAbove = entry.snapshot + withinGroup = true + when Checkpoint + if entry.isBoundary + throw new Error("Invalid redo stack state") + else + changes.push(entry) + unless withinGroup + spliceIndex = i + + while @redoStack[spliceIndex - 1] instanceof Checkpoint + spliceIndex-- + + if spliceIndex? + _.defaults(snapshotAbove, currentSnapshot) if snapshotAbove? + @undoStack.push(@redoStack.splice(spliceIndex).reverse()...) { - snapshot: pop.snapshotBelow - changes: pop.changes + snapshot: snapshotBelow + changes: changes } else false truncateUndoStack: (checkpointId) -> - if (checkpointIndex = @getCheckpointIndex(checkpointId))? - pop = @popChanges(@undoStack, null, checkpointIndex) + snapshotBelow = null + spliceIndex = null + withinGroup = false + invertedChanges = [] + + for entry, i in @undoStack by -1 + break if spliceIndex? + + switch entry.constructor + when GroupStart + if withinGroup + withinGroup = false + else + return false + when GroupEnd + if withinGroup + throw new Error("Invalid undo stack state") + else + withinGroup = true + when Checkpoint + if entry.id is checkpointId + spliceIndex = i + snapshotBelow = entry.snapshot + else if entry.isBoundary + return false + else + invertedChanges.push(@delegate.invertChange(entry)) + + if spliceIndex? + @undoStack.splice(spliceIndex) { - snapshot: pop.snapshotBelow - changes: (@delegate.invertChange(change) for change in pop.changes) + snapshot: snapshotBelow + changes: invertedChanges } else false @@ -122,41 +246,31 @@ class History return i return null - getBoundaryCheckpointIndex: (stack) -> - hasSeenChanges = false - for entry, i in stack by -1 - if entry instanceof Checkpoint - return i if hasSeenChanges - else - hasSeenChanges = true - null - - popChanges: (fromStack, toStack, checkpointIndex, lookBack) -> - changes = [] - snapshotAbove = null - snapshotBelow = fromStack[checkpointIndex].snapshot - splicedEntries = fromStack.splice(checkpointIndex) - for entry in splicedEntries by -1 - toStack?.push(entry) - if entry instanceof Checkpoint - snapshotAbove = entry.snapshot if changes.length is 0 - else - changes.push(entry) - {changes, snapshotAbove, snapshotBelow} - serializeStack: (stack) -> for entry in stack - if entry instanceof Checkpoint - { - type: 'checkpoint' - id: entry.id - snapshot: @delegate.serializeSnapshot(entry.snapshot) - } - else - { - type: 'change' - content: @delegate.serializeChange(entry) - } + switch entry.constructor + when Checkpoint + { + type: 'checkpoint' + id: entry.id + snapshot: @delegate.serializeSnapshot(entry.snapshot) + isBoundary: entry.isBoundary + } + when GroupStart + { + type: 'group-start' + snapshot: @delegate.serializeSnapshot(entry.snapshot) + } + when GroupEnd + { + type: 'group-end' + snapshot: @delegate.serializeSnapshot(entry.snapshot) + } + else + { + type: 'change' + content: @delegate.serializeChange(entry) + } deserializeStack: (stack) -> for entry in stack @@ -165,6 +279,15 @@ class History new Checkpoint( entry.id @delegate.deserializeSnapshot(entry.snapshot) + entry.isBoundary + ) + when 'group-start' + new GroupStart( + @delegate.deserializeSnapshot(entry.snapshot) + ) + when 'group-end' + new GroupEnd( + @delegate.deserializeSnapshot(entry.snapshot) ) when 'change' @delegate.deserializeChange(entry.content) diff --git a/src/marker-store.coffee b/src/marker-store.coffee index 801fe865c3..5ec6ca1f9a 100644 --- a/src/marker-store.coffee +++ b/src/marker-store.coffee @@ -123,6 +123,7 @@ class MarkerStore restoreFromSnapshot: (snapshots) -> markersUpdated = false + return unless snapshots? for id in Object.keys(@markersById) if marker = @markersById[id] if snapshot = snapshots[id] diff --git a/src/text-buffer.coffee b/src/text-buffer.coffee index edf13360d4..f87f3e8a36 100644 --- a/src/text-buffer.coffee +++ b/src/text-buffer.coffee @@ -850,12 +850,18 @@ class TextBuffer if pop = @history.popUndoStack(@markerStore.createSnapshot()) @applyChange(change, true) for change in pop.changes @markerStore.restoreFromSnapshot(pop.snapshot) + true + else + false # Public: Redo the last operation redo: -> if pop = @history.popRedoStack(@markerStore.createSnapshot()) @applyChange(change, true) for change in pop.changes @markerStore.restoreFromSnapshot(pop.snapshot) + true + else + false # Public: Batch multiple operations as a single undo/redo step. # @@ -875,24 +881,21 @@ class TextBuffer fn = groupingInterval groupingInterval = 0 - checkpointBefore = @history.createCheckpoint(@markerStore.createSnapshot(false)) + checkpointBefore = @history.createCheckpoint(@markerStore.createSnapshot(false), true) try @transactCallDepth++ result = fn() catch exception - @revertToCheckpoint(checkpointBefore) + @revertToCheckpoint(checkpointBefore, true) throw exception unless exception instanceof TransactionAbortedError return finally @transactCallDepth-- - checkpointAfter = @history.createCheckpoint(@markerStore.createSnapshot(true)) - @history.groupChangesSinceCheckpoint(checkpointBefore) + @history.groupChangesSinceCheckpoint(checkpointBefore, @markerStore.createSnapshot(true), true) + @history.applyGroupingInterval(groupingInterval) - now = Date.now() - @history.setCheckpointGroupingInterval(checkpointAfter, now, groupingInterval) - @history.applyCheckpointGroupingInterval(checkpointBefore, now, groupingInterval) result abortTransaction: -> @@ -906,7 +909,7 @@ class TextBuffer # # Returns a checkpoint value. createCheckpoint: -> - @history.createCheckpoint(@markerStore.createSnapshot()) + @history.createCheckpoint(@markerStore.createSnapshot(), false) # Public: Revert the buffer to the state it was in when the given # checkpoint was created. @@ -935,7 +938,7 @@ class TextBuffer # # Returns a {Boolean} indicating whether the operation succeeded. groupChangesSinceCheckpoint: (checkpoint) -> - @history.groupChangesSinceCheckpoint(checkpoint) + @history.groupChangesSinceCheckpoint(checkpoint, @markerStore.createSnapshot(false), false) ### Section: Search And Replace