Merge pull request #216 from atom/ns-sync-marker-layer-update-events

Emit marker layer update events synchronously at end of next transaction
atom · Mar 2, 2017 · 4e36f19 · 4e36f19
2 parents 3b2c525 + 09ab899
commit 4e36f19
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 25 deletions.
diff --git a/spec/display-marker-layer-spec.coffee b/spec/display-marker-layer-spec.coffee
@@ -147,6 +147,7 @@ describe "DisplayMarkerLayer", ->
     buffer = new TextBuffer(text: 'hello world')
     displayLayer = buffer.addDisplayLayer(tabLength: 4)
     markerLayer = displayLayer.addMarkerLayer()
+    marker = null
 
     updateEventCount = 0
     markerLayer.onDidUpdate ->
@@ -160,7 +161,8 @@ describe "DisplayMarkerLayer", ->
       else if updateEventCount is 4
         done()
 
-    marker = markerLayer.markScreenRange([[0, 4], [1, 4]])
+    buffer.transact ->
+      marker = markerLayer.markScreenRange([[0, 4], [1, 4]])
 
   it "allows markers to be copied", ->
     buffer = new TextBuffer(text: '\ta\tbc\tdef\tg\n\th')

diff --git a/spec/marker-layer-spec.coffee b/spec/marker-layer-spec.coffee
@@ -176,24 +176,34 @@ describe "MarkerLayer", ->
       expect(layer2.findMarkers(containsPoint: [0, 4])).toEqual [layer2Marker]
 
   describe "::onDidUpdate", ->
-    it "notifies observers asynchronously when markers are created, updated, or destroyed", (done) ->
+    it "notifies observers at the end of the outermost transaction when markers are created, updated, or destroyed", ->
+      [marker1, marker2] = []
+
       updateCount = 0
       layer1.onDidUpdate ->
         updateCount++
         if updateCount is 1
-          marker1.setRange([[1, 2], [3, 4]])
-          marker2.setRange([[4, 5], [6, 7]])
+          buffer.transact ->
+            marker1.setRange([[1, 2], [3, 4]])
+            marker2.setRange([[4, 5], [6, 7]])
         else if updateCount is 2
-          buffer.insert([0, 1], "xxx")
-          buffer.insert([0, 1], "yyy")
+          buffer.transact ->
+            buffer.insert([0, 1], "xxx")
+            buffer.insert([0, 1], "yyy")
         else if updateCount is 3
           marker1.destroy()
           marker2.destroy()
-        else if updateCount is 4
-          done()
 
-      marker1 = layer1.markRange([[0, 2], [0, 4]])
-      marker2 = layer1.markRange([[0, 6], [0, 8]])
+      buffer.transact ->
+        buffer.transact ->
+          marker1 = layer1.markRange([[0, 2], [0, 4]])
+          marker2 = layer1.markRange([[0, 6], [0, 8]])
+
+      expect(updateCount).toBe(4)
+
+      # update events happen immediately when there is no parent transaction
+      layer1.markRange([[0, 2], [0, 4]])
+      expect(updateCount).toBe(5)
 
   describe "::clear()", ->
     it "destroys all of the layer's markers", (done) ->

diff --git a/src/marker-layer.coffee b/src/marker-layer.coffee
@@ -71,7 +71,7 @@ class MarkerLayer
     @markersById = {}
     @index = new MarkerIndex
     @displayMarkerLayers.forEach (layer) -> layer.didClearBufferMarkerLayer()
-    @scheduleUpdateEvent()
+    @delegate.markersUpdated(this)
 
   # Public: Determine whether this layer has been destroyed.
   isDestroyed: ->
@@ -272,7 +272,7 @@ class MarkerLayer
           marker.destroy()
         else
           marker.valid = false
-    @scheduleUpdateEvent()
+    @delegate.markersUpdated(this)
 
   restoreFromSnapshot: (snapshots) ->
     return unless snapshots?
@@ -305,7 +305,6 @@ class MarkerLayer
     @markersWithChangeListeners.forEach (marker) ->
       unless marker.isDestroyed() # event handlers could destroy markers
         marker.emitChangeEvent(snapshot?[marker.id]?.range, true, false)
-    @delegate.markersUpdated(this)
 
   serialize: ->
     ranges = @index.dump()
@@ -332,7 +331,6 @@ class MarkerLayer
 
   markerUpdated: ->
     @delegate.markersUpdated(this)
-    @scheduleUpdateEvent()
 
   destroyMarker: (marker) ->
     if @markersById.hasOwnProperty(marker.id)
@@ -342,7 +340,6 @@ class MarkerLayer
       @markersWithDestroyListeners.delete(marker)
       @displayMarkerLayers.forEach (displayMarkerLayer) -> displayMarkerLayer.destroyMarker(marker.id)
       @delegate.markersUpdated(this)
-      @scheduleUpdateEvent()
 
   hasMarker: (id) ->
     not @destroyed and @index.has(id)
@@ -374,7 +371,6 @@ class MarkerLayer
     marker = @addMarker(id, range, params)
     @delegate.markerCreated(this, marker)
     @delegate.markersUpdated(this)
-    @scheduleUpdateEvent()
     marker.trackDestruction = @trackDestructionInOnDidCreateMarkerCallbacks ? false
     @emitter.emit 'did-create-marker', marker if @emitCreateMarkerEvents
     marker.trackDestruction = false
@@ -390,13 +386,8 @@ class MarkerLayer
     @index.insert(id, range.start, range.end)
     @markersById[id] = new Marker(id, this, range, params)
 
-  scheduleUpdateEvent: ->
-    unless @didUpdateEventScheduled
-      @didUpdateEventScheduled = true
-      process.nextTick =>
-        return if @destroyed
-        @didUpdateEventScheduled = false
-        @emitter.emit 'did-update'
+  emitUpdateEvent: ->
+    @emitter.emit('did-update')
 
 filterSet = (set1, set2) ->
   if set1

diff --git a/src/text-buffer.coffee b/src/text-buffer.coffee
@@ -72,6 +72,7 @@ class TextBuffer
     @displayLayers = {}
     @markerLayers = params?.markerLayers ? {}
     @markerLayers[@defaultMarkerLayer.id] = @defaultMarkerLayer
+    @markerLayersWithPendingUpdateEvents = new Set()
     @nextMarkerId = params?.nextMarkerId ? 1
 
     @setEncoding(params?.encoding)
@@ -1501,6 +1502,14 @@ class TextBuffer
       @markerLayers[markerLayerId]?.restoreFromSnapshot(layerSnapshot)
 
   emitMarkerChangeEvents: (snapshot) ->
+    while @markerLayersWithPendingUpdateEvents.size > 0
+      updatedMarkerLayers = Array.from(@markerLayersWithPendingUpdateEvents)
+      @markerLayersWithPendingUpdateEvents.clear()
+      for markerLayer in updatedMarkerLayers
+        markerLayer.emitUpdateEvent()
+        if markerLayer is @defaultMarkerLayer
+          @emitter.emit 'did-update-markers'
+
     for markerLayerId, markerLayer of @markerLayers
       markerLayer.emitChangeEvents(snapshot?[markerLayerId])
 
@@ -1590,7 +1599,11 @@ class TextBuffer
       @emitter.emit 'did-create-marker', marker
 
   markersUpdated: (layer) ->
-    if layer is @defaultMarkerLayer
-      @emitter.emit 'did-update-markers'
+    if @transactCallDepth is 0
+      layer.emitUpdateEvent()
+      if layer is @defaultMarkerLayer
+        @emitter.emit 'did-update-markers'
+    else
+      @markerLayersWithPendingUpdateEvents.add(layer)
 
   getNextMarkerId: -> @nextMarkerId++