Update markers before triggering 'changed' events

This prevents the editor from synchronously redrawing in specs with
cursors in invalid locations. Markers are always updated under the
hood before a change event is emitted from the Buffer or DisplayBuffer.
We still wait to trigger marker observers, but if their position gets
read, it is up to date.

spec/app/display-buffer-spec.coffee

@@ -732,11 +732,39 @@ describe "DisplayBuffer", ->
         displayBuffer.unfoldBufferRow(4)
         expect(observeHandler).not.toHaveBeenCalled()
 
+      it "updates the position of markers before emitting buffer change events, but does not notify their observers until the change event", ->
+        changeHandler = jasmine.createSpy("changeHandler").andCallFake ->
+          # calls change handler first
+          expect(observeHandler).not.toHaveBeenCalled()
+          # but still updates the markers
+          expect(displayBuffer.getMarkerScreenRange(marker)).toEqual [[5, 7], [5, 13]]
+          expect(displayBuffer.getMarkerHeadScreenPosition(marker)).toEqual [5, 13]
+          expect(displayBuffer.getMarkerTailScreenPosition(marker)).toEqual [5, 7]
+
+        displayBuffer.on 'changed', changeHandler
+
+        buffer.insert([8, 1], "...")
+
+        expect(changeHandler).toHaveBeenCalled()
+        expect(observeHandler).toHaveBeenCalled()
+
+      it "updates the position of markers before emitting change events that aren't caused by a buffer change", ->
+        changeHandler = jasmine.createSpy("changeHandler").andCallFake ->
+          # calls change handler first
+          expect(observeHandler).not.toHaveBeenCalled()
+          # but still updates the markers
+          expect(displayBuffer.getMarkerScreenRange(marker)).toEqual [[8, 4], [8, 10]]
+          expect(displayBuffer.getMarkerHeadScreenPosition(marker)).toEqual [8, 10]
+          expect(displayBuffer.getMarkerTailScreenPosition(marker)).toEqual [8, 4]
+        displayBuffer.on 'changed', changeHandler
+
+        displayBuffer.unfoldBufferRow(4)
+
+        expect(changeHandler).toHaveBeenCalled()
+        expect(observeHandler).toHaveBeenCalled()
+
     describe "marker destruction", ->
       it "allows markers to be destroyed", ->
         marker = displayBuffer.markScreenRange([[5, 4], [5, 10]])
         displayBuffer.destroyMarker(marker)
         expect(displayBuffer.getMarkerBufferRange(marker)).toBeUndefined()
-
-
-

src/app/buffer-change-operation.coffee

@@ -69,10 +69,14 @@ class BufferChangeOperation
     @buffer.cachedMemoryContents = null
     @buffer.conflict = false if @buffer.conflict and !@buffer.isModified()
 
+    @pauseMarkerObservation()
     event = { oldRange, newRange, oldText, newText }
+    @updateMarkers(event)
     @buffer.trigger 'changed', event
     @buffer.scheduleStoppedChangingEvent()
-    @updateMarkers(event)
+    @resumeMarkerObservation()
+    @buffer.trigger 'markers-updated'
+
     newRange
 
   calculateNewRange: (oldRange, newText) ->
@@ -89,9 +93,14 @@ class BufferChangeOperation
   invalidateMarkers: (oldRange) ->
     _.compact(@buffer.getMarkers().map (marker) -> marker.tryToInvalidate(oldRange))
 
+  pauseMarkerObservation: ->
+    marker.pauseEvents() for marker in @buffer.getMarkers()
+
+  resumeMarkerObservation: ->
+    marker.resumeEvents() for marker in @buffer.getMarkers()
+
   updateMarkers: (bufferChange) ->
     marker.handleBufferChange(bufferChange) for marker in @buffer.getMarkers()
-    @buffer.trigger 'markers-updated'
 
   restoreMarkers: (markersToRestore) ->
     for [id, previousRange] in markersToRestore

src/app/buffer-marker.coffee

@@ -1,18 +1,16 @@
 _ = require 'underscore'
 Point = require 'point'
 Range = require 'range'
+EventEmitter = require 'event-emitter'
 
 module.exports =
 class BufferMarker
   headPosition: null
   tailPosition: null
-  observers: null
   suppressObserverNotification: false
   stayValid: false
 
   constructor: ({@id, @buffer, range, @stayValid, noTail, reverse}) ->
-    @headPositionObservers = []
-    @observers = []
     @setRange(range, {noTail, reverse})
 
   setRange: (range, options={}) ->
@@ -115,11 +113,11 @@ class BufferMarker
     [newRow, newColumn]
 
   observe: (callback) ->
-    @observers.push(callback)
+    @on 'position-changed', callback
     cancel: => @unobserve(callback)
 
   unobserve: (callback) ->
-    _.remove(@observers, callback)
+    @off 'position-changed', callback
 
   containsPoint: (point) ->
     @getRange().containsPoint(point)
@@ -131,11 +129,7 @@ class BufferMarker
     newHeadPosition ?= @getHeadPosition()
     oldTailPosition ?= @getTailPosition()
     newTailPosition ?= @getTailPosition()
-    for observer in @getObservers()
-      observer({oldHeadPosition, newHeadPosition, oldTailPosition, newTailPosition, bufferChanged})
-
-  getObservers: ->
-    new Array(@observers...)
+    @trigger 'position-changed', {oldHeadPosition, newHeadPosition, oldTailPosition, newTailPosition, bufferChanged}
 
   consolidateObserverNotifications: (bufferChanged, fn) ->
     @suppressObserverNotification = true
@@ -150,3 +144,5 @@ class BufferMarker
   invalidate: (preserve) ->
     delete @buffer.validMarkers[@id]
     @buffer.invalidMarkers[@id] = this
+
+_.extend BufferMarker.prototype, EventEmitter

src/app/display-buffer-marker.coffee

@@ -1,9 +1,9 @@
 Range = require 'range'
 _ = require 'underscore'
+EventEmitter = require 'event-emitter'
 
 module.exports =
 class DisplayBufferMarker
-  observers: null
   bufferMarkerSubscription: null
   headScreenPosition: null
   tailScreenPosition: null
@@ -57,16 +57,15 @@ class DisplayBufferMarker
 
   observe: (callback) ->
     @observeBufferMarkerIfNeeded()
-    @observers.push(callback)
+    @on 'position-changed', callback
     cancel: => @unobserve(callback)
 
   unobserve: (callback) ->
-    _.remove(@observers, callback)
+    @off 'position-changed', callback
     @unobserveBufferMarkerIfNeeded()
 
   observeBufferMarkerIfNeeded: ->
-    return if @observers
-    @observers = []
+    return if @subscriptionCount()
     @getHeadScreenPosition() # memoize current value
     @getTailScreenPosition() # memoize current value
     @bufferMarkerSubscription =
@@ -80,8 +79,7 @@ class DisplayBufferMarker
     @displayBuffer.markers[@id] = this
 
   unobserveBufferMarkerIfNeeded: ->
-    return if @observers.length
-    @observers = null
+    return if @subscriptionCount()
     @bufferMarkerSubscription.cancel()
     delete @displayBuffer.markers[@id]
 
@@ -101,14 +99,12 @@ class DisplayBufferMarker
     oldTailBufferPosition ?= @getTailBufferPosition()
     newTailBufferPosition = @getTailBufferPosition()
 
-    for observer in @getObservers()
-      observer({
-        oldHeadScreenPosition, newHeadScreenPosition,
-        oldTailScreenPosition, newTailScreenPosition,
-        oldHeadBufferPosition, newHeadBufferPosition,
-        oldTailBufferPosition, newTailBufferPosition,
-        bufferChanged
-      })
+    @trigger 'position-changed', {
+      oldHeadScreenPosition, newHeadScreenPosition,
+      oldTailScreenPosition, newTailScreenPosition,
+      oldHeadBufferPosition, newHeadBufferPosition,
+      oldTailBufferPosition, newTailBufferPosition,
+      bufferChanged
+    }
 
-  getObservers: ->
-    new Array(@observers...)
+_.extend DisplayBufferMarker.prototype, EventEmitter

src/app/display-buffer.coffee

@@ -28,7 +28,8 @@ class DisplayBuffer
     @foldsById = {}
     @markers = {}
     @buildLineMap()
-    @tokenizedBuffer.on 'changed', (e) => @handleTokenizedBufferChange(e)
+    @tokenizedBuffer.on 'changed', @handleTokenizedBufferChange
+    @buffer.on 'markers-updated', @handleMarkersUpdated
 
   setVisible: (visible) -> @tokenizedBuffer.setVisible(visible)
 
@@ -37,8 +38,11 @@ class DisplayBuffer
     @lineMap.insertAtScreenRow 0, @buildLinesForBufferRows(0, @buffer.getLastRow())
 
   triggerChanged: (eventProperties, refreshMarkers=true) ->
-    @refreshMarkerScreenPositions() if refreshMarkers
+    if refreshMarkers
+      @pauseMarkerObservers()
+      @refreshMarkerScreenPositions()
     @trigger 'changed', eventProperties
+    @resumeMarkerObservers()
 
   setSoftWrapColumn: (@softWrapColumn) ->
     start = 0
@@ -131,7 +135,6 @@ class DisplayBuffer
       screenDelta = newScreenRange.end.row - oldScreenRange.end.row
       bufferDelta = 0
 
-      @refreshMarkerScreenPositions()
       @triggerChanged({ start, end, screenDelta, bufferDelta })
 
   destroyFoldsContainingBufferRow: (bufferRow) ->
@@ -216,7 +219,7 @@ class DisplayBuffer
     allFolds.push(folds...) for row, folds of @activeFolds
     fold.handleBufferChange(e) for fold in allFolds
 
-  handleTokenizedBufferChange: (tokenizedBufferChange) ->
+  handleTokenizedBufferChange: (tokenizedBufferChange) =>
     if bufferChange = tokenizedBufferChange.bufferChange
       @handleBufferChange(bufferChange)
       bufferDelta = bufferChange.newRange.end.row - bufferChange.oldRange.end.row
@@ -231,7 +234,17 @@ class DisplayBuffer
     @lineMap.replaceScreenRows(start, end, newScreenLines)
     screenDelta = @lastScreenRowForBufferRow(tokenizedBufferEnd + tokenizedBufferDelta) - end
 
-    @triggerChanged({ start, end, screenDelta, bufferDelta }, false)
+    changeEvent = { start, end, screenDelta, bufferDelta }
+    if bufferChange
+      @pauseMarkerObservers()
+      @pendingChangeEvent = changeEvent
+    else
+      @triggerChanged(changeEvent, false)
+
+  handleMarkersUpdated: =>
+    event = @pendingChangeEvent
+    @pendingChangeEvent = null
+    @triggerChanged(event, false)
 
   buildLineForBufferRow: (bufferRow) ->
     @buildLinesForBufferRows(bufferRow, bufferRow)
@@ -372,12 +385,19 @@ class DisplayBuffer
   observeMarker: (id, callback) ->
     @getMarker(id).observe(callback)
 
+  pauseMarkerObservers: ->
+    marker.pauseEvents() for marker in @getMarkers()
+
+  resumeMarkerObservers: ->
+    marker.resumeEvents() for marker in @getMarkers()
+
   refreshMarkerScreenPositions: ->
     for marker in @getMarkers()
       marker.notifyObservers(bufferChanged: false)
 
   destroy: ->
     @tokenizedBuffer.destroy()
+    @buffer.off 'markers-updated', @handleMarkersUpdated
 
   logLines: (start, end) ->
     @lineMap.logLines(start, end)