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

@@ -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", ->

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('')

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()

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) ->

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 #

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: ->

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