From 42eefb49a951595fde0aa5996ed2c0edce2bf750 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Thu, 5 Jul 2012 20:04:16 -0600 Subject: [PATCH] Add `UndoManager.prototype.transact` The `transact` method takes a function and batches all operations within that function as a single transaction to be undone and redone. The edit session now uses generic operations to restore selection state around transactions. The `undo` and `do` methods on operations are now optional. In addition, the undo manager now supports an optional `redo` method on an operation for code that should *only* be run on redo, and not when the operation is initially pushed. This is used by edit session to restore selection state after redo. --- spec/app/undo-manager-spec.coffee | 38 +++++++++++-------------------- src/app/buffer.coffee | 16 ++++++------- src/app/edit-session.coffee | 19 +++++++++------- src/app/undo-manager.coffee | 37 ++++++++++++++---------------- 4 files changed, 49 insertions(+), 61 deletions(-) diff --git a/spec/app/undo-manager-spec.coffee b/spec/app/undo-manager-spec.coffee index d6ccb1a3f..307412a74 100644 --- a/spec/app/undo-manager-spec.coffee +++ b/spec/app/undo-manager-spec.coffee @@ -63,49 +63,37 @@ describe "UndoManager", -> undoManager.redo() expect(buffer.getText()).toContain 'qsport' - describe "startUndoBatch() / endUndoBatch()", -> - it "causes changes in batch to be undone simultaneously and returns an array of ranges to select from undo and redo", -> + describe "transact(fn)", -> + it "causes changes in the transaction to be undone simultaneously", -> buffer.insert([0, 0], "foo") - ignoredRanges = [[[666, 666], [666, 666]], [[666, 666], [666, 666]]] - beforeRanges = [[[1, 2], [1, 2]], [[1, 9], [1, 9]]] - afterRanges =[[[1, 5], [1, 5]], [[1, 12], [1, 12]]] - - undoManager.startUndoBatch(beforeRanges) - undoManager.startUndoBatch(ignoredRanges) # calls can be nested - buffer.insert([1, 2], "111") - buffer.insert([1, 9], "222") - undoManager.endUndoBatch(ignoredRanges) # calls can be nested - undoManager.endUndoBatch(afterRanges) + undoManager.transact -> + undoManager.transact -> + buffer.insert([1, 2], "111") + buffer.insert([1, 9], "222") expect(buffer.lineForRow(1)).toBe ' 111var 222sort = function(items) {' - ranges = undoManager.undo() - expect(ranges).toBe beforeRanges + undoManager.undo() expect(buffer.lineForRow(1)).toBe ' var sort = function(items) {' expect(buffer.lineForRow(0)).toContain 'foo' - ranges = undoManager.undo() - expect(ranges).toBeUndefined() + undoManager.undo() expect(buffer.lineForRow(0)).not.toContain 'foo' - ranges = undoManager.redo() - expect(ranges).toBeUndefined() + undoManager.redo() expect(buffer.lineForRow(0)).toContain 'foo' - ranges = undoManager.redo() - expect(ranges).toBe afterRanges + undoManager.redo() expect(buffer.lineForRow(1)).toBe ' 111var 222sort = function(items) {' - ranges = undoManager.undo() - expect(ranges).toBe beforeRanges + undoManager.undo() expect(buffer.lineForRow(1)).toBe ' var sort = function(items) {' - it "does not store empty batches", -> + it "does not record empty transactions", -> buffer.insert([0,0], "foo") - undoManager.startUndoBatch() - undoManager.endUndoBatch() + undoManager.transact -> undoManager.undo() expect(buffer.lineForRow(0)).not.toContain("foo") diff --git a/src/app/buffer.coffee b/src/app/buffer.coffee index a42f72608..972ef5c34 100644 --- a/src/app/buffer.coffee +++ b/src/app/buffer.coffee @@ -132,10 +132,7 @@ class Buffer change: (oldRange, newText) -> oldRange = Range.fromObject(oldRange) operation = new BufferChangeOperation({buffer: this, oldRange, newText}) - if @undoManager - @undoManager.pushOperation(operation) - else - operation.do() + @pushOperation(operation) prefixAndSuffixForRange: (range) -> prefix: @lines[range.start.row][0...range.start.column] @@ -145,11 +142,14 @@ class Buffer @lines[startRow..endRow] = newLines @modified = true - startUndoBatch: (selectedBufferRanges) -> - @undoManager.startUndoBatch(selectedBufferRanges) + pushOperation: (operation) -> + if @undoManager + @undoManager.pushOperation(operation) + else + operation.do() - endUndoBatch: (selectedBufferRanges) -> - @undoManager.endUndoBatch(selectedBufferRanges) + transact: (fn) -> + @undoManager.transact(fn) undo: -> @undoManager.undo() diff --git a/src/app/edit-session.coffee b/src/app/edit-session.coffee index ea5073501..8428bd24f 100644 --- a/src/app/edit-session.coffee +++ b/src/app/edit-session.coffee @@ -177,12 +177,10 @@ class EditSession @insertText($native.readFromPasteboard()) undo: -> - if ranges = @buffer.undo() - @setSelectedBufferRanges(ranges) + @buffer.undo() redo: -> - if ranges = @buffer.redo() - @setSelectedBufferRanges(ranges) + @buffer.redo() foldSelection: -> selection.fold() for selection in @getSelections() @@ -234,10 +232,15 @@ class EditSession @tokenizedBuffer.toggleLineCommentsInRange(range) mutateSelectedText: (fn) -> - selections = @getSelections() - @buffer.startUndoBatch(@getSelectedBufferRanges()) - fn(selection) for selection in selections - @buffer.endUndoBatch(@getSelectedBufferRanges()) + @transact => fn(selection) for selection in @getSelections() + + transact: (fn) -> + @buffer.transact => + oldSelectedRanges = @getSelectedBufferRanges() + @buffer.pushOperation(undo: => @setSelectedBufferRanges(oldSelectedRanges)) + fn() + newSelectedRanges = @getSelectedBufferRanges() + @buffer.pushOperation(redo: => @setSelectedBufferRanges(newSelectedRanges)) getAnchors: -> new Array(@anchors...) diff --git a/src/app/undo-manager.coffee b/src/app/undo-manager.coffee index 47ae7b1d7..abfe16072 100644 --- a/src/app/undo-manager.coffee +++ b/src/app/undo-manager.coffee @@ -5,8 +5,7 @@ module.exports = class UndoManager undoHistory: null redoHistory: null - currentBatch: null - startBatchCallCount: null + currentTransaction: null constructor: (@buffer) -> @startBatchCallCount = 0 @@ -14,36 +13,34 @@ class UndoManager @redoHistory = [] pushOperation: (operation) -> - if @currentBatch - @currentBatch.push(operation) + if @currentTransaction + @currentTransaction.push(operation) else @undoHistory.push([operation]) @redoHistory = [] - operation.do() + operation.do?() + + transact: (fn) -> + if @currentTransaction + fn() + else + @currentTransaction = [] + fn() + @undoHistory.push(@currentTransaction) if @currentTransaction.length + @currentTransaction = null undo: -> if batch = @undoHistory.pop() opsInReverse = new Array(batch...) opsInReverse.reverse() - op.undo() for op in opsInReverse + op.undo?() for op in opsInReverse @redoHistory.push batch batch.oldSelectionRanges redo: -> if batch = @redoHistory.pop() - op.do() for op in batch + for op in batch + op.do?() + op.redo?() @undoHistory.push(batch) batch.newSelectionRanges - - startUndoBatch: (ranges) -> - @startBatchCallCount++ - return if @startBatchCallCount > 1 - @currentBatch = [] - @currentBatch.oldSelectionRanges = ranges - - endUndoBatch: (ranges) -> - @startBatchCallCount-- - return if @startBatchCallCount > 0 - @currentBatch.newSelectionRanges = ranges - @undoHistory.push(@currentBatch) if @currentBatch.length > 0 - @currentBatch = null