From 69300e0766e9cf70b2cafaf75b39e5b4a92a9aa2 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Wed, 1 May 2013 18:38:40 -0600 Subject: [PATCH] Make Buffer.transact restore marker ranges on undo/redo of transaction We no longer need to restore selection ranges before and after transactions now because selections are based on markers so they go along for the ride for free. This allows us to delegate directly to Buffer.transact from EditSession. --- spec/app/edit-session-spec.coffee | 10 -- spec/app/undo-manager-spec.coffee | 122 ++++++++----------------- src/app/buffer-change-operation.coffee | 28 +++--- src/app/buffer-marker.coffee | 33 +++---- src/app/edit-session.coffee | 21 +---- src/app/text-buffer.coffee | 19 +++- src/app/undo-manager.coffee | 6 -- 7 files changed, 90 insertions(+), 149 deletions(-) diff --git a/spec/app/edit-session-spec.coffee b/spec/app/edit-session-spec.coffee index 6801789a5..d06aeba7a 100644 --- a/spec/app/edit-session-spec.coffee +++ b/spec/app/edit-session-spec.coffee @@ -2061,16 +2061,6 @@ describe "EditSession", -> expect(editSession.isFoldedAtBufferRow(1)).toBeFalsy() expect(editSession.isFoldedAtBufferRow(2)).toBeTruthy() - it "restores selected ranges even when the change occurred in another edit session", -> - otherEditSession = project.buildEditSession(editSession.getPath()) - otherEditSession.setSelectedBufferRange([[2, 2], [3, 3]]) - otherEditSession.delete() - - editSession.undo() - - expect(editSession.getSelectedBufferRange()).toEqual [[2, 2], [3, 3]] - expect(otherEditSession.getSelectedBufferRange()).toEqual [[3, 3], [3, 3]] - describe ".transact([fn])", -> describe "when called without a function", -> it "restores the selection when the transaction is undone/redone", -> diff --git a/spec/app/undo-manager-spec.coffee b/spec/app/undo-manager-spec.coffee index e213751d4..0ac4a2ca2 100644 --- a/spec/app/undo-manager-spec.coffee +++ b/spec/app/undo-manager-spec.coffee @@ -64,103 +64,59 @@ describe "UndoManager", -> expect(buffer.getText()).toContain 'qsport' describe "transaction methods", -> - describe "transact([fn])", -> - describe "when called with a function", -> - it "causes changes performed within the function's dynamic extent to be undone simultaneously", -> - buffer.insert([0, 0], "foo") + describe "transact()", -> + beforeEach -> + buffer.setText('') - undoManager.transact -> - undoManager.transact -> - buffer.insert([1, 2], "111") - buffer.insert([1, 9], "222") + it "starts a transaction that can be committed later", -> + buffer.append('1') + undoManager.transact() + buffer.append('2') + buffer.append('3') + undoManager.commit() + buffer.append('4') - expect(buffer.lineForRow(1)).toBe ' 111var 222sort = function(items) {' + expect(buffer.getText()).toBe '1234' + undoManager.undo() + expect(buffer.getText()).toBe '123' + undoManager.undo() + expect(buffer.getText()).toBe '1' + undoManager.redo() + expect(buffer.getText()).toBe '123' - undoManager.undo() - expect(buffer.lineForRow(1)).toBe ' var sort = function(items) {' - expect(buffer.lineForRow(0)).toContain 'foo' + it "starts a transaction that can be aborted later", -> + buffer.append('1') + buffer.append('2') - undoManager.undo() + undoManager.transact() - expect(buffer.lineForRow(0)).not.toContain 'foo' + buffer.append('3') + buffer.append('4') + expect(buffer.getText()).toBe '1234' - undoManager.redo() - expect(buffer.lineForRow(0)).toContain 'foo' + undoManager.abort() + expect(buffer.getText()).toBe '12' - undoManager.redo() - expect(buffer.lineForRow(1)).toBe ' 111var 222sort = function(items) {' + undoManager.undo() + expect(buffer.getText()).toBe '1' - undoManager.undo() - expect(buffer.lineForRow(1)).toBe ' var sort = function(items) {' + undoManager.redo() + expect(buffer.getText()).toBe '12' - it "does not record empty transactions", -> - buffer.insert([0,0], "foo") - undoManager.transact -> - - undoManager.undo() - expect(buffer.lineForRow(0)).not.toContain("foo") - - it "undoes operations that occured prior to an exception when the transaction is undone", -> - buffer.setText("jumpstreet") - - expect(-> - undoManager.transact -> - buffer.insert([0,0], "3") - buffer.insert([0,0], "2") - throw new Error("problem") - buffer.insert([0,0], "2") - ).toThrow('problem') - - expect(buffer.lineForRow(0)).toBe "23jumpstreet" - undoManager.undo() - expect(buffer.lineForRow(0)).toBe "jumpstreet" - - describe "when called without a function", -> - beforeEach -> - buffer.setText('') - - it "returns a transaction object that can be committed later", -> - buffer.append('1') - undoManager.transact() - buffer.append('2') - buffer.append('3') - undoManager.commit() - buffer.append('4') - - expect(buffer.getText()).toBe '1234' - undoManager.undo() - expect(buffer.getText()).toBe '123' - undoManager.undo() - expect(buffer.getText()).toBe '1' - undoManager.redo() - expect(buffer.getText()).toBe '123' - - it "returns a transaction object that can be aborted later", -> - buffer.append('1') - buffer.append('2') - - undoManager.transact() - - buffer.append('3') - buffer.append('4') - expect(buffer.getText()).toBe '1234' - - undoManager.abort() - expect(buffer.getText()).toBe '12' - - undoManager.undo() - expect(buffer.getText()).toBe '1' - - undoManager.redo() - expect(buffer.getText()).toBe '12' - - undoManager.redo() - expect(buffer.getText()).toBe '12' + undoManager.redo() + expect(buffer.getText()).toBe '12' describe "commit", -> it "throws an exception if there is no current transaction", -> expect(-> buffer.commit()).toThrow() + it "does not record empty transactions", -> + buffer.insert([0,0], "foo") + undoManager.transact() + undoManager.commit() + undoManager.undo() + expect(buffer.lineForRow(0)).not.toContain("foo") + describe "abort", -> it "does not affect the undo stack when the current transaction is empty", -> buffer.setText('') diff --git a/src/app/buffer-change-operation.coffee b/src/app/buffer-change-operation.coffee index 556c69743..2de448992 100644 --- a/src/app/buffer-change-operation.coffee +++ b/src/app/buffer-change-operation.coffee @@ -21,14 +21,15 @@ class BufferChangeOperation do: -> @buffer.pauseEvents() @pauseMarkerObservation() - @oldText = @buffer.getTextInRange(@oldRange) - @newRange = @calculateNewRange(@oldRange, @newText) @markersToRestoreOnUndo = @invalidateMarkers(@oldRange) - newRange = @changeBuffer - oldRange: @oldRange - newRange: @newRange - oldText: @oldText - newText: @newText + if @oldRange? + @oldText = @buffer.getTextInRange(@oldRange) + @newRange = @calculateNewRange(@oldRange, @newText) + newRange = @changeBuffer + oldRange: @oldRange + newRange: @newRange + oldText: @oldText + newText: @newText @restoreMarkers(@markersToRestoreOnRedo) if @markersToRestoreOnRedo @buffer.resumeEvents() @resumeMarkerObservation() @@ -38,11 +39,12 @@ class BufferChangeOperation @buffer.pauseEvents() @pauseMarkerObservation() @markersToRestoreOnRedo = @invalidateMarkers(@newRange) - @changeBuffer - oldRange: @newRange - newRange: @oldRange - oldText: @newText - newText: @oldText + if @oldRange? + @changeBuffer + oldRange: @newRange + newRange: @oldRange + oldText: @newText + newText: @oldText @restoreMarkers(@markersToRestoreOnUndo) @buffer.resumeEvents() @resumeMarkerObservation() @@ -107,7 +109,7 @@ class BufferChangeOperation resumeMarkerObservation: -> marker.resumeEvents() for marker in @buffer.getMarkers(includeInvalid: true) - @buffer.trigger 'markers-updated' + @buffer.trigger 'markers-updated' if @oldRange? updateMarkers: (bufferChange) -> marker.handleBufferChange(bufferChange) for marker in @buffer.getMarkers() diff --git a/src/app/buffer-marker.coffee b/src/app/buffer-marker.coffee index 38a654b74..9677d5b15 100644 --- a/src/app/buffer-marker.coffee +++ b/src/app/buffer-marker.coffee @@ -179,23 +179,24 @@ class BufferMarker ### tryToInvalidate: (changedRange) -> - betweenStartAndEnd = @getRange().containsRange(changedRange, exclusive: false) - containsStart = changedRange.containsPoint(@getStartPosition(), exclusive: true) - containsEnd = changedRange.containsPoint(@getEndPosition(), exclusive: true) previousRange = @getRange() - switch @invalidationStrategy - when 'between' - @invalidate() if betweenStartAndEnd or containsStart or containsEnd - when 'contains' - @invalidate() if containsStart or containsEnd - when 'never' - if containsStart or containsEnd - if containsStart and containsEnd - @setRange([changedRange.end, changedRange.end]) - else if containsStart - @setRange([changedRange.end, @getEndPosition()]) - else - @setRange([@getStartPosition(), changedRange.start]) + if changedRange + betweenStartAndEnd = @getRange().containsRange(changedRange, exclusive: false) + containsStart = changedRange.containsPoint(@getStartPosition(), exclusive: true) + containsEnd = changedRange.containsPoint(@getEndPosition(), exclusive: true) + switch @invalidationStrategy + when 'between' + @invalidate() if betweenStartAndEnd or containsStart or containsEnd + when 'contains' + @invalidate() if containsStart or containsEnd + when 'never' + if containsStart or containsEnd + if containsStart and containsEnd + @setRange([changedRange.end, changedRange.end]) + else if containsStart + @setRange([changedRange.end, @getEndPosition()]) + else + @setRange([@getStartPosition(), changedRange.start]) [@id, previousRange] handleBufferChange: (bufferChange) -> diff --git a/src/app/edit-session.coffee b/src/app/edit-session.coffee index 6dac6f3cb..3a88e83fa 100644 --- a/src/app/edit-session.coffee +++ b/src/app/edit-session.coffee @@ -618,26 +618,11 @@ class EditSession # Internal # ### - transact: (fn) -> - isNewTransaction = @buffer.transact() - oldSelectedRanges = @getSelectedBufferRanges() - @pushOperation - undo: (editSession) -> - editSession?.setSelectedBufferRanges(oldSelectedRanges, preserveFolds: true) - if fn - result = fn() - @commit() if isNewTransaction - result + transact: (fn) -> @buffer.transact(fn) - commit: -> - newSelectedRanges = @getSelectedBufferRanges() - @pushOperation - redo: (editSession) -> - editSession?.setSelectedBufferRanges(newSelectedRanges, preserveFolds: true) - @buffer.commit() + commit: -> @buffer.commit() - abort: -> - @buffer.abort() + abort: -> @buffer.abort() ### # Public # diff --git a/src/app/text-buffer.coffee b/src/app/text-buffer.coffee index 36d2da6c8..634953083 100644 --- a/src/app/text-buffer.coffee +++ b/src/app/text-buffer.coffee @@ -376,17 +376,30 @@ class Buffer operation.do() # Internal: - transact: (fn) -> @undoManager.transact(fn) + transact: (fn) -> + if isNewTransaction = @undoManager.transact() + @pushOperation(new BufferChangeOperation(buffer: this)) # restores markers on undo + if fn + try + fn() + finally + @commit() if isNewTransaction + + commit: -> + @pushOperation(new BufferChangeOperation(buffer: this)) # restores markers on redo + @undoManager.commit() + + abort: -> @undoManager.abort() + # Public: Undos the last operation. # # editSession - The {EditSession} associated with the buffer. undo: (editSession) -> @undoManager.undo(editSession) + # Public: Redos the last operation. # # editSession - The {EditSession} associated with the buffer. redo: (editSession) -> @undoManager.redo(editSession) - commit: -> @undoManager.commit() - abort: -> @undoManager.abort() # Public: Saves the buffer. save: -> diff --git a/src/app/undo-manager.coffee b/src/app/undo-manager.coffee index 7a09e1556..e8462f675 100644 --- a/src/app/undo-manager.coffee +++ b/src/app/undo-manager.coffee @@ -31,11 +31,6 @@ class UndoManager transact: (fn) -> isNewTransaction = not @currentTransaction? @currentTransaction ?= [] - if fn - try - fn() - finally - @commit() if isNewTransaction isNewTransaction commit: -> @@ -61,7 +56,6 @@ class UndoManager opsInReverse = new Array(batch...) opsInReverse.reverse() op.undo?(editSession) for op in opsInReverse - @redoHistory.push batch batch.oldSelectionRanges catch e