Merge pull request #205 from atom/mb-ns-fix-display-marker-creation

Fix marker bugs

spec/display-marker-layer-spec.coffee

@@ -115,6 +115,18 @@ describe "DisplayMarkerLayer", ->
       textChanged: false
     }
 
+  it "does not create duplicate DisplayMarkers when it has onDidCreateMarker observers (regression)", ->
+    buffer = new TextBuffer(text: 'abc\ndef\nghi\nj\tk\tl\nmno')
+    displayLayer = buffer.addDisplayLayer(tabLength: 4)
+    markerLayer = displayLayer.addMarkerLayer()
+
+    emittedMarker = null
+    markerLayer.onDidCreateMarker (marker) ->
+      emittedMarker = marker
+
+    createdMarker = markerLayer.markBufferRange([[0, 1], [2, 3]])
+    expect(createdMarker).toBe(emittedMarker)
+
   it "emits events when markers are created and destroyed", ->
     buffer = new TextBuffer(text: 'hello world')
     displayLayer = buffer.addDisplayLayer(tabLength: 4)
@@ -334,3 +346,9 @@ describe "DisplayMarkerLayer", ->
       marker2 = markerLayer.markBufferRange([[8, 0], [8, 0]], class: 'a')
       displayLayer.foldBufferRange([[4, 0], [7, 0]])
       expect(markerLayer.findMarkers(class: 'a', intersectsScreenRange: [[5, 0], [10, 0]])).toEqual [marker2]
+
+    it "works when used from within a Marker.onDidDestroy callback (regression)", ->
+      displayMarker = markerLayer.markBufferRange([[0, 3], [0, 6]])
+      displayMarker.onDidDestroy ->
+        expect(markerLayer.findMarkers({containsBufferPosition: [0, 4]})).not.toContain(displayMarker)
+      displayMarker.destroy()

src/display-marker-layer.coffee

@@ -133,7 +133,7 @@ class DisplayMarkerLayer
   markScreenRange: (screenRange, options) ->
     screenRange = Range.fromObject(screenRange)
     bufferRange = @displayLayer.translateScreenRange(screenRange, options)
-    @createDisplayMarker(@bufferMarkerLayer.markRange(bufferRange, options))
+    @getMarker(@bufferMarkerLayer.markRange(bufferRange, options).id)
 
   # Public: Create a marker on this layer with its head at the given screen
   # position and no tail.
@@ -170,7 +170,7 @@ class DisplayMarkerLayer
   markScreenPosition: (screenPosition, options) ->
     screenPosition = Point.fromObject(screenPosition)
     bufferPosition = @displayLayer.translateScreenPosition(screenPosition, options)
-    @createDisplayMarker(@bufferMarkerLayer.markPosition(bufferPosition, options))
+    @getMarker(@bufferMarkerLayer.markPosition(bufferPosition, options).id)
 
   # Public: Create a marker with the given buffer range.
   #
@@ -203,7 +203,7 @@ class DisplayMarkerLayer
   # Returns a {DisplayMarker}.
   markBufferRange: (bufferRange, options) ->
     bufferRange = Range.fromObject(bufferRange)
-    @createDisplayMarker(@bufferMarkerLayer.markRange(bufferRange, options))
+    @getMarker(@bufferMarkerLayer.markRange(bufferRange, options).id)
 
   # Public: Create a marker on this layer with its head at the given buffer
   # position and no tail.
@@ -233,12 +233,7 @@ class DisplayMarkerLayer
   #
   # Returns a {DisplayMarker}.
   markBufferPosition: (bufferPosition, options) ->
-    @createDisplayMarker(@bufferMarkerLayer.markPosition(Point.fromObject(bufferPosition), options))
-
-  createDisplayMarker: (bufferMarker) ->
-    displayMarker = new DisplayMarker(this, bufferMarker)
-    @markersById[displayMarker.id] = displayMarker
-    displayMarker
+    @getMarker(@bufferMarkerLayer.markPosition(Point.fromObject(bufferPosition), options).id)
 
   ###
   Section: Querying

src/marker-layer.coffee

@@ -337,10 +337,10 @@ class MarkerLayer
   destroyMarker: (marker) ->
     if @markersById.hasOwnProperty(marker.id)
       delete @markersById[marker.id]
+      @index.delete(marker.id)
       @markersWithChangeListeners.delete(marker)
       @markersWithDestroyListeners.delete(marker)
       @displayMarkerLayers.forEach (displayMarkerLayer) -> displayMarkerLayer.destroyMarker(marker.id)
-      @index.delete(marker.id)
       @delegate.markersUpdated(this)
       @scheduleUpdateEvent()