Store snippet tab stops in their own marker layer that maintains history

The previous commit changed the behavior of tab in the last tab stop to
jump to the end of the tab stop and eat the tab, whereas previously
hitting tab in the last tab stop just fell back to the default behavior
of tab.

Previously, when we undid a snippet expansion, we didn't destroy its
markers. Instead, we relied on the fact that hitting tab again would be
treated as if it were in the last tab stop, since all the markers got
scrunched together after the prefix following the undo. This would abort
handling of the command and we would move on to dispatch
`snippets:expand`.

Now that we don't abort the command handling in the last tab stop, we
need a better solution. Putting tab stop markers on their own layer and
maintaining history causes these markers to automatically be destroyed
when undoing. Now before working with any tab stop, we check that its
marker is not destroyed.

lib/snippet-expansion.coffee

@@ -22,14 +22,14 @@ class SnippetExpansion
 
     @editor.transact =>
       @ignoringBufferChanges =>
-        newRange = @editor.transact =>
-          @cursor.selection.insertText(body, autoIndent: false)
-        if @snippet.tabStopList.length > 0
-          @subscriptions.add @cursor.onDidChangePosition (event) => @cursorMoved(event)
-          @subscriptions.add @cursor.onDidDestroy => @cursorDestroyed()
-          @placeTabStopMarkers(startPosition, tabStops)
-          @snippets.addExpansion(@editor, this)
-          @editor.normalizeTabsInBufferRange(newRange)
+        @editor.transact =>
+          newRange = @cursor.selection.insertText(body, autoIndent: false)
+          if @snippet.tabStopList.length > 0
+            @subscriptions.add @cursor.onDidChangePosition (event) => @cursorMoved(event)
+            @subscriptions.add @cursor.onDidDestroy => @cursorDestroyed()
+            @placeTabStopMarkers(startPosition, tabStops)
+            @snippets.addExpansion(@editor, this)
+            @editor.normalizeTabsInBufferRange(newRange)
 
   # Set a flag on undo or redo so that we know not to re-apply transforms.
   # They're already accounted for in the history.
@@ -101,7 +101,7 @@ class SnippetExpansion
       for insertion in insertions
         {range} = insertion
         {start, end} = range
-        marker = @editor.markBufferRange([startPosition.traverse(start), startPosition.traverse(end)])
+        marker = @snippets.getMarkerLayer(@editor).markBufferRange([startPosition.traverse(start), startPosition.traverse(end)])
         markers.push({
           index: markers.length,
           marker: marker,
@@ -121,9 +121,9 @@ class SnippetExpansion
       else
         @goToNextTabStop()
     else
-      @goToEndOfLastTabStop()
+      succeeded = @goToEndOfLastTabStop()
       @destroy()
-      true
+      succeeded
 
   goToPreviousTabStop: ->
     @setTabStopIndex(@tabStopIndex - 1) if @tabStopIndex > 0
@@ -139,6 +139,7 @@ class SnippetExpansion
     @hasTransforms = false
     for item in items
       {marker, insertion} = item
+      continue if marker.isDestroyed()
       continue unless marker.isValid()
       if insertion.isTransformation()
         @hasTransforms = true
@@ -166,15 +167,18 @@ class SnippetExpansion
 
   goToEndOfLastTabStop: ->
     return unless @tabStopMarkers.length > 0
-    markers = @tabStopMarkers[@tabStopMarkers.length - 1]
-    return unless markers.length > 0
-    lastMarker = markers[markers.length - 1]
-    @editor.setCursorBufferPosition(lastMarker.getEndBufferPosition())
+    items = @tabStopMarkers[@tabStopMarkers.length - 1]
+    return unless items.length > 0
+    {marker: lastMarker} = items[items.length - 1]
+    if lastMarker.isDestroyed()
+      false
+    else
+      @editor.setCursorBufferPosition(lastMarker.getEndBufferPosition())
+      true
 
   destroy: ->
     @subscriptions.dispose()
-    for items in @tabStopMarkers
-      item.marker.destroy() for item in items
+    @snippets.getMarkerLayer(@editor).clear()
     @tabStopMarkers = []
     @snippets.stopObservingEditor(@editor)
     @snippets.clearExpansions(@editor)

lib/snippets.coffee

@@ -17,6 +17,7 @@ module.exports =
     @userSnippetsPath = null
     @snippetIdCounter = 0
     @parsedSnippetsById = new Map
+    @editorMarkerLayers = new WeakMap
     @scopedPropertyStore = new ScopedPropertyStore
     @subscriptions = new CompositeDisposable
     @subscriptions.add atom.workspace.addOpener (uri) =>
@@ -53,6 +54,7 @@ module.exports =
         snippets.availableSnippetsView.toggle(editor)
 
     @subscriptions.add atom.workspace.observeTextEditors (editor) =>
+      @createMarkerLayer(editor)
       @clearExpansions(editor)
 
   deactivate: ->
@@ -333,6 +335,12 @@ module.exports =
   getStore: (editor) ->
     EditorStore.findOrCreate(editor)
 
+  createMarkerLayer: (editor) ->
+    @editorMarkerLayers.set(editor, editor.addMarkerLayer({maintainHistory: true}))
+
+  getMarkerLayer: (editor) ->
+    @editorMarkerLayers.get(editor)
+
   getExpansions: (editor) ->
     @getStore(editor).getExpansions()
 

spec/snippets-spec.coffee

@@ -463,13 +463,9 @@ describe "Snippets extension", ->
           editor.setCursorScreenPosition([2, Infinity])
           editor.insertText ' t3'
           atom.commands.dispatch editorElement, 'snippets:expand'
-
-          markers = editor.getMarkers()
-          expect(markers.length).toBe 2
-          expect(markers[0].getBufferRange().start).toEqual row: 3, column: 12
-          expect(markers[0].getBufferRange().end).toEqual markers[0].getBufferRange().start
-          expect(markers[1].getBufferRange().start).toEqual row: 4, column: 4
-          expect(markers[1].getBufferRange().end).toEqual markers[1].getBufferRange().start
+          expect(editor.getCursorBufferPosition()).toEqual [3, 12]
+          atom.commands.dispatch editorElement, 'snippets:next-tab-stop'
+          expect(editor.getCursorBufferPosition()).toEqual [4, 4]
 
         it "indents the subsequent lines of the snippet based on the indent level before the snippet is inserted", ->
           editor.setCursorScreenPosition([2, Infinity])
@@ -488,34 +484,24 @@ describe "Snippets extension", ->
           editor.insertText 't4'
           atom.commands.dispatch editorElement, 'snippets:expand'
 
-          markers = editor.getMarkers()
-          expect(markers.length).toBe 2
-          expect(markers[0].getBufferRange().start).toEqual row: 3, column: 9
-          expect(markers[0].getBufferRange().end).toEqual row: 3, column: 10
-          expect(markers[1].getBufferRange().start).toEqual row: 4, column: 6
-          expect(markers[1].getBufferRange().end).toEqual row: 4, column: 13
-
+          expect(editor.getSelectedBufferRange()).toEqual [[3, 9], [3, 10]]
           atom.commands.dispatch editorElement, 'snippets:next-tab-stop'
+          expect(editor.getSelectedBufferRange()).toEqual [[4, 6], [4, 13]]
+
           editor.insertText 't4'
           atom.commands.dispatch editorElement, 'snippets:expand'
 
-          markers = editor.getMarkers()
-          expect(markers.length).toBe 4
-          expect(markers[2].getBufferRange().start).toEqual row: 4, column: 11
-          expect(markers[2].getBufferRange().end).toEqual row: 4, column: 12
-          expect(markers[3].getBufferRange().start).toEqual row: 5, column: 8
-          expect(markers[3].getBufferRange().end).toEqual row: 5, column: 15
+          expect(editor.getSelectedBufferRange()).toEqual [[4, 11], [4, 12]]
+          atom.commands.dispatch editorElement, 'snippets:next-tab-stop'
+          expect(editor.getSelectedBufferRange()).toEqual [[5, 8], [5, 15]]
 
           editor.setText('') # Clear editor
           editor.insertText 't4'
           atom.commands.dispatch editorElement, 'snippets:expand'
 
-          markers = editor.getMarkers()
-          expect(markers.length).toBe 6
-          expect(markers[4].getBufferRange().start).toEqual row: 0, column: 5
-          expect(markers[4].getBufferRange().end).toEqual row: 0, column: 6
-          expect(markers[5].getBufferRange().start).toEqual row: 1, column: 2
-          expect(markers[5].getBufferRange().end).toEqual row: 1, column: 9
+          expect(editor.getSelectedBufferRange()).toEqual [[0, 5], [0, 6]]
+          atom.commands.dispatch editorElement, 'snippets:next-tab-stop'
+          expect(editor.getSelectedBufferRange()).toEqual [[1, 2], [1, 9]]
 
       describe "when multiple snippets match the prefix", ->
         it "expands the snippet that is the longest match for the prefix", ->