Pull out a BufferChangeOperation, which Buffer.change sends to UndoManager

spec/app/buffer-spec.coffee

@@ -220,6 +220,11 @@ describe 'Buffer', ->
         expect(event.oldText).toBe oldText
         expect(event.newText).toBe "foo\nbar"
 
+    it "allows a 'change' event handler to safely undo the change", ->
+      buffer.on 'change', -> buffer.undo()
+      buffer.change([0, 0], "hello")
+      expect(buffer.lineForRow(0)).toBe "var quicksort = function () {"
+
   describe ".setText(text)", ->
     it "changes the entire contents of the buffer and emits a change event", ->
       lastRow = buffer.getLastRow()

spec/app/undo-manager-spec.coffee

@@ -7,7 +7,7 @@ describe "UndoManager", ->
 
   beforeEach ->
     buffer = new Buffer(require.resolve('fixtures/sample.js'))
-    undoManager = new UndoManager(buffer)
+    undoManager = buffer.undoManager
 
   afterEach ->
     buffer.destroy()

src/app/buffer-change-operation.coffee

@@ -0,0 +1,53 @@
+Range = require 'range'
+
+module.exports =
+class BufferChangeOperation
+  buffer: null
+  oldRange: null
+  oldText: null
+  newRange: null
+  newText: null
+
+  constructor: ({@buffer, @oldRange, @newText}) ->
+
+  do: ->
+    @oldText = @buffer.getTextInRange(@oldRange)
+    @newRange = @calculateNewRange(@oldRange, @newText)
+    @changeBuffer
+      oldRange: @oldRange
+      newRange: @newRange
+      oldText: @oldText
+      newText: @newText
+
+  undo: ->
+    @changeBuffer
+      oldRange: @newRange
+      newRange: @oldRange
+      oldText: @newText
+      newText: @oldText
+
+  changeBuffer: ({ oldRange, newRange, newText, oldText }) ->
+    { prefix, suffix } = @buffer.prefixAndSuffixForRange(oldRange)
+
+    newTextLines = newText.split('\n')
+    if newTextLines.length == 1
+      newTextLines = [prefix + newText + suffix]
+    else
+      lastLineIndex = newTextLines.length - 1
+      newTextLines[0] = prefix + newTextLines[0]
+      newTextLines[lastLineIndex] += suffix
+
+    @buffer.replaceLines(oldRange.start.row, oldRange.end.row, newTextLines)
+    @buffer.trigger 'change', { oldRange, newRange, oldText, newText }
+    newRange
+
+  calculateNewRange: (oldRange, newText) ->
+    newRange = new Range(oldRange.start.copy(), oldRange.start.copy())
+    newTextLines = newText.split('\n')
+    if newTextLines.length == 1
+      newRange.end.column += newText.length
+    else
+      lastLineIndex = newTextLines.length - 1
+      newRange.end.row += lastLineIndex
+      newRange.end.column = newTextLines[lastLineIndex].length
+    newRange

src/app/buffer.coffee

@@ -5,6 +5,7 @@ Point = require 'point'
 Range = require 'range'
 EventEmitter = require 'event-emitter'
 UndoManager = require 'undo-manager'
+BufferChangeOperation = require 'buffer-change-operation'
 
 module.exports =
 class Buffer
@@ -130,29 +131,20 @@ class Buffer
 
   change: (oldRange, newText) ->
     oldRange = Range.fromObject(oldRange)
-    newRange = new Range(oldRange.start.copy(), oldRange.start.copy())
-    prefix = @lines[oldRange.start.row][0...oldRange.start.column]
-    suffix = @lines[oldRange.end.row][oldRange.end.column..]
-    oldText = @getTextInRange(oldRange)
-
-    newTextLines = newText.split('\n')
-
-    if newTextLines.length == 1
-      newRange.end.column += newText.length
-      newTextLines = [prefix + newText + suffix]
+    operation = new BufferChangeOperation({buffer: this, oldRange, newText})
+    if @undoManager
+      @undoManager.perform(operation)
     else
-      lastLineIndex = newTextLines.length - 1
-      newTextLines[0] = prefix + newTextLines[0]
-      newRange.end.row += lastLineIndex
-      newRange.end.column = newTextLines[lastLineIndex].length
-      newTextLines[lastLineIndex] += suffix
+      operation.do()
 
-    @lines[oldRange.start.row..oldRange.end.row] = newTextLines
+  prefixAndSuffixForRange: (range) ->
+    prefix: @lines[range.start.row][0...range.start.column]
+    suffix: @lines[range.end.row][range.end.column..]
+
+  replaceLines: (startRow, endRow, newLines) ->
+    @lines[startRow..endRow] = newLines
     @modified = true
 
-    @trigger 'change', { oldRange, newRange, oldText, newText }
-    newRange
-
   startUndoBatch: (selectedBufferRanges) ->
     @undoManager.startUndoBatch(selectedBufferRanges)
 

src/app/range.coffee

@@ -22,7 +22,7 @@ class Range
       @start = pointB
       @end = pointA
 
-  copy: (range) ->
+  copy: ->
     new Range(@start.copy(), @end.copy())
 
   isEqual: (other) ->

src/app/undo-manager.coffee

@@ -6,36 +6,33 @@ class UndoManager
   undoHistory: null
   redoHistory: null
   currentBatch: null
-  preserveHistory: false
   startBatchCallCount: null
 
   constructor: (@buffer) ->
     @startBatchCallCount = 0
     @undoHistory = []
     @redoHistory = []
-    @buffer.on 'change', (event) =>
-      unless @preserveHistory
-        op = new BufferChangeOperation(_.extend({ @buffer }, event))
-        if @currentBatch
-          @currentBatch.push(op)
-        else
-          @undoHistory.push([op])
-        @redoHistory = []
+
+  perform: (operation) ->
+    if @currentBatch
+      @currentBatch.push(operation)
+    else
+      @undoHistory.push([operation])
+    @redoHistory = []
+    operation.do()
 
   undo: ->
     if batch = @undoHistory.pop()
-      @preservingHistory =>
-        opsInReverse = new Array(batch...)
-        opsInReverse.reverse()
-        op.undo() for op in opsInReverse
-        @redoHistory.push batch
+      opsInReverse = new Array(batch...)
+      opsInReverse.reverse()
+      op.undo() for op in opsInReverse
+      @redoHistory.push batch
       batch.oldSelectionRanges
 
   redo: ->
     if batch = @redoHistory.pop()
-      @preservingHistory =>
-        op.do() for op in batch
-        @undoHistory.push(batch)
+      op.do() for op in batch
+      @undoHistory.push(batch)
       batch.newSelectionRanges
 
   startUndoBatch: (ranges) ->
@@ -50,17 +47,3 @@ class UndoManager
     @currentBatch.newSelectionRanges = ranges
     @undoHistory.push(@currentBatch) if @currentBatch.length > 0
     @currentBatch = null
-
-  preservingHistory: (fn) ->
-    @preserveHistory = true
-    fn()
-    @preserveHistory = false
-
-class BufferChangeOperation
-  constructor: ({@buffer, @oldRange, @newRange, @oldText, @newText}) ->
-
-  do: ->
-    @buffer.change @oldRange, @newText
-
-  undo: ->
-    @buffer.change @newRange, @oldText