Recreate destroyed markers when restoring marker snapshots

package.json

@@ -1,6 +1,6 @@
 {
   "name": "text-buffer",
-  "version": "6.6.1",
+  "version": "6.7.0-0",
   "description": "A container for large mutable strings with annotated regions",
   "main": "./lib/text-buffer",
   "scripts": {

spec/marker-spec.coffee

@@ -840,6 +840,8 @@ describe "Marker", ->
         }]
 
       it "correctly restores markers when the transaction is undone", ->
+        marker.destroy() for marker in allStrategies
+
         buffer.setText('')
 
         buffer.transact ->
@@ -849,31 +851,23 @@ describe "Marker", ->
           buffer.append('\n')
           buffer.append('bar')
 
-        marker1 = buffer.markRange([[0, 0], [0, 3]], invalidate: 'never', maintainHistory: true)
-        marker2 = buffer.markRange([[1, 0], [1, 3]], invalidate: 'never', maintainHistory: true)
-
-        marker1Ranges = []
-        marker2Ranges = []
-        buffer.onDidChange ->
-          marker1Ranges.push(marker1.getRange())
-          marker2Ranges.push(marker2.getRange())
+          buffer.markRange([[0, 0], [0, 3]], a: 'b', invalidate: 'never', maintainHistory: true)
+          buffer.markRange([[1, 0], [1, 3]], c: 'd', invalidate: 'never', maintainHistory: true)
 
         buffer.undo()
 
         expect(buffer.getText()).toBe 'foo'
-        expect(marker1Ranges).toEqual [[[0, 0], [0, 3]], [[0, 0], [0, 3]]]
-        expect(marker1.getRange()).toEqual([[0, 0], [0, 3]])
-        expect(marker2Ranges).toEqual [[[1, 0], [1, 0]], [[0, 3], [0, 3]]]
-        expect(marker2.getRange()).toEqual([[0, 3], [0, 3]])
+        markers = buffer.findMarkers({})
+        expect(markers).toEqual []
 
-        marker1Ranges = []
-        marker2Ranges = []
         buffer.redo()
 
-        expect(marker1Ranges).toEqual [[[0, 0], [1, 0]], [[0, 0], [1, 3]]]
-        expect(marker1.getRange()).toEqual([[0, 0], [0, 3]])
-        expect(marker2Ranges).toEqual [[[0, 3], [1, 0]], [[0, 3], [1, 3]]]
-        expect(marker2.getRange()).toEqual([[1, 0], [1, 3]])
+        expect(buffer.getText()).toBe 'foo\nbar'
+        markers = buffer.findMarkers({})
+        expect(markers[0].getProperties()).toEqual {a: 'b'}
+        expect(markers[1].getProperties()).toEqual {c: 'd'}
+        expect(markers[0].getRange()).toEqual [[0, 0], [0, 3]]
+        expect(markers[1].getRange()).toEqual [[1, 0], [1, 3]]
 
       it "only records marker patches for direct marker updates", ->
         buffer.setText("abcd")
@@ -969,13 +963,33 @@ describe "Marker", ->
       expect(marker.getStartPosition()).toEqual [0, 3]
       expect(marker.getEndPosition()).toEqual [0, 6]
 
-    it "does not reinsert a marker into ", ->
+    it "does not reinsert the marker if its range is later updated", ->
       marker = buffer.markRange([[0, 3], [0, 6]])
       marker.destroy()
       expect(buffer.findMarkers(intersectsRow: 0)).toEqual []
       marker.setRange([[0, 0], [0, 9]])
       expect(buffer.findMarkers(intersectsRow: 0)).toEqual []
 
+    it "recreates historied markers on undo", ->
+      marker1 = buffer.markRange([[0, 3], [0, 6]], a: 'b', maintainHistory: true)
+
+      createdMarkers = []
+      buffer.onDidCreateMarker (marker) ->
+        createdMarkers.push(marker)
+
+      buffer.append("...")
+      marker1.destroy()
+
+      marker2 = buffer.markRange([[0, 3], [0, 6]], c: 'd', maintainHistory: true)
+
+      buffer.undo()
+
+      expect(createdMarkers.length).toBe 2
+      expect(createdMarkers[1].getProperties()).toEqual {a: 'b'}
+      expect(createdMarkers[1].getRange()).toEqual [[0, 3], [0, 6]]
+
+      expect(marker2.isDestroyed()).toBe true
+
   describe "TextBuffer::findMarkers(properties)", ->
     [marker1, marker2, marker3, marker4] = []
 

spec/text-buffer-spec.coffee

@@ -575,7 +575,7 @@ describe "TextBuffer", ->
           buffer.append("three\n")
           buffer.append("four")
 
-        marker = buffer.markRange([[0, 1], [2, 3]], maintainHistory: true)
+        buffer.markRange([[0, 1], [2, 3]], a: 'b', maintainHistory: true)
         result = buffer.groupChangesSinceCheckpoint(checkpoint)
 
         expect(result).toBe true
@@ -597,7 +597,9 @@ describe "TextBuffer", ->
           four
         """
 
+        [marker] = buffer.getMarkers()
         expect(marker.getRange()).toEqual [[0, 1], [2, 3]]
+        expect(marker.getProperties()).toEqual {a: 'b'}
 
       it "skips any later checkpoints when grouping changes", ->
         buffer.append("one\n")

src/history.coffee

@@ -1,5 +1,3 @@
-_ = require 'underscore-plus'
-
 SerializationVersion = 2
 
 class Checkpoint
@@ -95,8 +93,7 @@ class History
     @undoStack.push(change)
     @clearRedoStack()
 
-  popUndoStack: (currentSnapshot) ->
-    snapshotAbove = null
+  popUndoStack: ->
     snapshotBelow = null
     spliceIndex = null
     withinGroup = false
@@ -116,7 +113,6 @@ class History
           if withinGroup
             throw new Error("Invalid undo stack state")
           else
-            snapshotAbove = entry.snapshot
             withinGroup = true
         when Checkpoint
           if entry.isBoundary
@@ -127,7 +123,6 @@ class History
             spliceIndex = i
 
     if spliceIndex?
-      _.defaults(snapshotAbove, currentSnapshot) if snapshotAbove?
       @redoStack.push(@undoStack.splice(spliceIndex).reverse()...)
       {
         snapshot: snapshotBelow
@@ -136,8 +131,7 @@ class History
     else
       false
 
-  popRedoStack: (currentSnapshot) ->
-    snapshotAbove = null
+  popRedoStack: ->
     snapshotBelow = null
     spliceIndex = null
     withinGroup = false
@@ -157,7 +151,6 @@ class History
           if withinGroup
             throw new Error("Invalid redo stack state")
           else
-            snapshotAbove = entry.snapshot
             withinGroup = true
         when Checkpoint
           if entry.isBoundary
@@ -171,7 +164,6 @@ class History
       spliceIndex--
 
     if spliceIndex?
-      _.defaults(snapshotAbove, currentSnapshot) if snapshotAbove?
       @undoStack.push(@redoStack.splice(spliceIndex).reverse()...)
       {
         snapshot: snapshotBelow

src/marker-store.coffee

@@ -122,14 +122,27 @@ class MarkerStore
     @index.splice(start, oldExtent, newExtent)
 
   restoreFromSnapshot: (snapshots) ->
-    markersUpdated = false
     return unless snapshots?
-    for id in Object.keys(@markersById)
+
+    createdIds = new Set
+    snapshotIds = Object.keys(snapshots)
+    existingMarkerIds = Object.keys(@markersById)
+
+    for id in snapshotIds
+      snapshot = snapshots[id]
       if marker = @markersById[id]
-        if snapshot = snapshots[id]
-          marker.update(marker.getRange(), snapshot, true)
+        marker.update(marker.getRange(), snapshot, true)
+      else
+        newMarker = @createMarker(snapshot.range, snapshot)
+        createdIds.add(newMarker.id)
+
+    for id in existingMarkerIds
+      if (marker = @markersById[id]) and (not snapshots[id]?)
+        if @historiedMarkers.has(id)
+          marker.destroy()
         else
           marker.emitChangeEvent(marker.getRange(), true, false)
+
     @delegate.markersUpdated()
     return
 

src/text-buffer.coffee

@@ -865,7 +865,7 @@ class TextBuffer
 
   # Public: Undo the last operation. If a transaction is in progress, aborts it.
   undo: ->
-    if pop = @history.popUndoStack(@markerStore.createSnapshot())
+    if pop = @history.popUndoStack()
       @applyChange(change, true) for change in pop.changes
       @markerStore.restoreFromSnapshot(pop.snapshot)
       true
@@ -874,7 +874,7 @@ class TextBuffer
 
   # Public: Redo the last operation
   redo: ->
-    if pop = @history.popRedoStack(@markerStore.createSnapshot())
+    if pop = @history.popRedoStack()
       @applyChange(change, true) for change in pop.changes
       @markerStore.restoreFromSnapshot(pop.snapshot)
       true