Bring the changes from atom#281 into atom#288.

Author: savetheclocktower

lib/insertion.js

@@ -1,9 +1,10 @@
 const {transformWithSubstitution} = require('./util')
 
 class Insertion {
-  constructor ({range, substitution, choices=[], transformResolver}) {
+  constructor ({range, substitution, references, choices=[], transformResolver}) {
     this.range = range
     this.substitution = substitution
+    this.references = references
     if (substitution && substitution.replace === undefined) {
       substitution.replace = ''
     }

lib/snippet-expansion.js

@@ -12,7 +12,21 @@ module.exports = class SnippetExpansion {
     this.cursor = cursor
     this.snippets = snippets
     this.subscriptions = new CompositeDisposable
-    this.tabStopMarkers = []
+    this.insertionsByIndex = []
+    this.markersForInsertions = new Map()
+
+    // The index of the active tab stop. We don't use the tab stop's own
+    // numbering here; we renumber them consecutively starting at 0 in the order
+    // in which they should be visited. So `$1` will always be index `0` in the
+    // above list, and `$0` (if present) will always be the last index.
+    this.tabStopIndex = null
+
+    // If, say, tab stop 4's placeholder references tab stop 2, then tab stop
+    // 4's insertion goes into this map as a "related" insertion to tab stop 2.
+    // We need to keep track of this because tab stop 4's marker will need to be
+    // replaced while 2 is the active index.
+    this.relatedInsertionsByIndex = new Map()
+
     this.selections = [this.cursor.selection]
 
     const startPosition = this.cursor.selection.getBufferRange().start
@@ -29,8 +43,11 @@ module.exports = class SnippetExpansion {
 
     const tabStops = this.tabStopList.toArray()
     this.ignoringBufferChanges(() => {
+      // Insert the snippet body at the cursor.
       const newRange = this.cursor.selection.insertText(body, {autoIndent: false})
       if (this.tabStopList.length > 0) {
+        // Listen for cursor changes so we can decide whether to keep the
+        // snippet active or terminate it.
         this.subscriptions.add(this.cursor.onDidChangePosition(event => this.cursorMoved(event)))
         this.subscriptions.add(this.cursor.onDidDestroy(() => this.cursorDestroyed()))
         this.placeTabStopMarkers(tabStops)
@@ -49,9 +66,16 @@ module.exports = class SnippetExpansion {
 
   cursorMoved ({oldBufferPosition, newBufferPosition, textChanged}) {
     if (this.settingTabStop || (textChanged && !this.isUndoingOrRedoing)) { return }
-    const itemWithCursor = this.tabStopMarkers[this.tabStopIndex].find(item => item.marker.getBufferRange().containsPoint(newBufferPosition))
 
-    if (itemWithCursor && !itemWithCursor.insertion.isTransformation()) { return }
+    const insertionAtCursor = this.insertionsByIndex[this.tabStopIndex].find((insertion) => {
+      let marker = this.markersForInsertions.get(insertion)
+      return marker.getBufferRange().containsPoint(newBufferPosition)
+    })
+
+    if (insertionAtCursor && !insertionAtCursor.isTransformation()) {
+      // The cursor is still inside an insertion. Return so that the snippet doesn't get destroyed.
+      return
+    }
 
     // we get here if there is no item for the current index with the cursor
     if (this.isUndoingOrRedoing) {
@@ -95,31 +119,35 @@ module.exports = class SnippetExpansion {
   }
 
   applyAllTransformations () {
-    this.tabStopMarkers.forEach((item, index) => this.applyTransformations(index))
+    this.insertionsByIndex.forEach((_, index) => this.applyTransformations(index))
   }
 
   applyTransformations (tabStop) {
-    const items = [...this.tabStopMarkers[tabStop]]
-    if (items.length === 0) { return }
+    const insertions = [...this.insertionsByIndex[tabStop]]
+    if (insertions.length === 0) { return }
 
-    const primary = items.shift()
-    const primaryRange = primary.marker.getBufferRange()
+    const primaryInsertion = insertions.shift()
+    const primaryRange = this.markersForInsertions.get(primaryInsertion).getBufferRange()
     const inputText = this.editor.getTextInBufferRange(primaryRange)
 
     this.ignoringBufferChanges(() => {
-      for (const item of items) {
-        const {marker, insertion} = item
-        var range = marker.getBufferRange()
-
+      for (const insertion of insertions) {
         // Don't transform mirrored tab stops. They have their own cursors, so
         // mirroring happens automatically.
         if (!insertion.isTransformation()) { continue }
 
+        let marker = this.markersForInsertions.get(insertion)
+        let range = marker.getBufferRange()
+
         var outputText = insertion.transform(inputText)
 
         this.editor.setTextInBufferRange(range, outputText)
         // this.editor.buffer.groupLastChanges()
 
+        // Manually adjust the marker's range rather than rely on its internal
+        // heuristics. (We don't have to worry about whether it's been
+        // invalidated because setting its buffer range implicitly marks it as
+        // valid again.)
         const newRange = new Range(
           range.start,
           range.start.traverse(getEndpointOfText(outputText))
@@ -130,42 +158,125 @@ module.exports = class SnippetExpansion {
   }
 
   placeTabStopMarkers (tabStops) {
+    // Tab stops within a snippet refer to one another by their external index
+    // (1 for $1, 3 for $3, etc.). We respect the order of these tab stops, but
+    // we renumber them starting at 0 and using consecutive numbers.
+    //
+    // Luckily, we don't need to convert between the two numbering systems very
+    // often. But we do have to build a map from external index to our internal
+    // index. We do this in a separate loop so that the table is complete before
+    // we need to consult it in the following loop.
+    let indexTable = {}
+    Object.keys(tabStops).forEach((key, index) => {
+      let tabStop = tabStops[key]
+      indexTable[tabStop.index] = index
+    })
     const markerLayer = this.getMarkerLayer(this.editor)
 
+    let tabStopIndex = -1
     for (const tabStop of tabStops) {
+      tabStopIndex++
       const {insertions} = tabStop
-      const markers = []
-
       if (!tabStop.isValid()) { continue }
 
       for (const insertion of insertions) {
-        const marker = markerLayer.markBufferRange(insertion.range)
-        markers.push({
-          index: markers.length,
-          marker,
-          insertion
-        })
+        const {range: {start, end}} = insertion
+        let references = null
+        if (insertion.references) {
+          references = insertion.references.map(external => indexTable[external])
+        }
+        // Since this method is only called once at the beginning of a snippet
+        // expansion, we know that 0 is about to be the active tab stop.
+        let shouldBeInclusive = (tabStopIndex === 0) || (references && references.includes(0))
+
+        const marker = markerLayer.markBufferRange(insertion.range, {exclusive: !shouldBeInclusive})
+        this.markersForInsertions.set(insertion, marker)
+        if (references) {
+          let relatedInsertions = this.relatedInsertionsByIndex.get(tabStopIndex) || []
+          relatedInsertions.push(insertion)
+          this.relatedInsertionsByIndex.set(tabStopIndex, relatedInsertions)
+        }
       }
 
-      this.tabStopMarkers.push(markers)
+      // Since we have to replace markers in place when we change their
+      // exclusivity, we'll store them in a map keyed on the insertion itself.
+      this.insertionsByIndex[tabStopIndex] = insertions
     }
 
     this.setTabStopIndex(0)
     this.applyAllTransformations()
   }
 
+  // When two insertion markers are directly adjacent to one another, and the
+  // cursor is placed right at the border between them, the marker that should
+  // "claim" the newly-typed content will vary based on context.
+  //
+  // All else being equal, that content should get added to the marker (if any)
+  // whose tab stop is active (or the marker whose tab stop's placeholder
+  // references an active tab stop). The `exclusive` setting controls whether a
+  // marker grows to include content added at its edge.
+  //
+  // So we need to revisit the markers whenever the active tab stop changes,
+  // figure out which ones need to be touched, and replace them with markers
+  // that have the settings we need.
+  adjustTabStopMarkers (oldIndex, newIndex) {
+    // Take all the insertions belonging to the newly-active tab stop (and all
+    // insertions whose placeholders reference the newly-active tab stop) and
+    // change their markers to be inclusive.
+    let insertionsForNewIndex = [
+      ...this.insertionsByIndex[newIndex],
+      ...(this.relatedInsertionsByIndex.get(newIndex) || [])
+    ]
+
+    for (let insertion of insertionsForNewIndex) {
+      this.replaceMarkerForInsertion(insertion, {exclusive: false})
+    }
+
+    // Take all the insertions whose markers were made inclusive when they
+    // became active and restore their original marker settings.
+    let insertionsForOldIndex = [
+      ...this.insertionsByIndex[oldIndex],
+      ...(this.relatedInsertionsByIndex.get(oldIndex) || [])
+    ]
+
+    for (let insertion of insertionsForOldIndex) {
+      this.replaceMarkerForInsertion(insertion, {exclusive: true})
+    }
+  }
+
+  replaceMarkerForInsertion (insertion, settings) {
+    let marker = this.markersForInsertions.get(insertion)
+
+    // If the marker is invalid or destroyed, return it as-is. Other methods
+    // need to know if a marker has been invalidated or destroyed, and there's
+    // no case in which we'd need to change the settings on such a marker
+    // anyway.
+    if (!marker.isValid() || marker.isDestroyed()) {
+      return marker
+    }
+
+    // Otherwise, create a new marker with an identical range and the specified
+    // settings.
+    let range = marker.getBufferRange()
+    let replacement = this.getMarkerLayer(this.editor).markBufferRange(range, settings)
+
+    marker.destroy()
+    this.markersForInsertions.set(insertion, replacement)
+    return replacement
+  }
+
   goToNextTabStop () {
     const nextIndex = this.tabStopIndex + 1
 
     // if we have an endstop (implicit ends have already been added) it will be the last one
-    if (nextIndex === this.tabStopMarkers.length - 1 && this.tabStopList.hasEndStop) {
+    if (nextIndex === this.insertionsByIndex.length - 1 && this.tabStopList.hasEndStop) {
       const succeeded = this.setTabStopIndex(nextIndex)
       this.destroy()
       return {succeeded, isDestroyed: true}
     }
 
     // we are not at the end, and the next is not the endstop; just go to next stop
-    if (nextIndex < this.tabStopMarkers.length) {
+    if (nextIndex < this.insertionsByIndex.length) {
       const succeeded = this.setTabStopIndex(nextIndex)
       if (succeeded) { return {succeeded, isDestroyed: false} }
       return this.goToNextTabStop()
@@ -190,26 +301,38 @@ module.exports = class SnippetExpansion {
   }
 
   setTabStopIndex (tabStopIndex) {
+    let oldIndex = this.tabStopIndex
     this.tabStopIndex = tabStopIndex
+
+    // Set a flag before we move any selections so that our change handlers
+    // will know that the movements were initiated by us.
     this.settingTabStop = true
+
+    // Keep track of whether we replaced any selections or cursors.
     let markerSelected = false
 
-    const items = this.tabStopMarkers[this.tabStopIndex]
-    if (items.length === 0) { return false }
+    let insertions = this.insertionsByIndex[this.tabStopIndex]
+    if (insertions.length === 0) { return false }
 
     const ranges = []
     let hasTransforms = false
-    for (const item of items) {
-      const {marker, insertion} = item
+    // Go through the active tab stop's markers to figure out where to place
+    // cursors and/or selections.
+    for (const insertion of insertions) {
+      const marker = this.markersForInsertions.get(insertion)
       if (marker.isDestroyed() || !marker.isValid()) { continue }
       if (insertion.isTransformation()) {
+        // Set a flag for later, but skip transformation insertions because
+        // they don't get their own cursors.
         hasTransforms = true
         continue
       }
       ranges.push(marker.getBufferRange())
     }
 
     if (ranges.length > 0) {
+      // We have new selections to apply. Reuse existing selections if
+      // possible, and destroy the unused ones if we already have too many.
       for (const selection of this.selections.slice(ranges.length)) { selection.destroy() }
       this.selections = this.selections.slice(0, ranges.length)
       for (let i = 0; i < ranges.length; i++) {
@@ -223,20 +346,30 @@ module.exports = class SnippetExpansion {
           this.selections.push(newSelection)
         }
       }
+      // We placed at least one selection, so this tab stop was successfully
+      // set. Update our return value.
       markerSelected = true
     }
 
     this.settingTabStop = false
     // If this snippet has at least one transform, we need to observe changes
     // made to the editor so that we can update the transformed tab stops.
-    if (hasTransforms) { this.snippets.observeEditor(this.editor) }
+    if (hasTransforms) {
+      this.snippets.observeEditor(this.editor)
+    } else {
+      this.snippets.stopObservingEditor(this.editor)
+    }
+
+    if (oldIndex !== null) {
+      this.adjustTabStopMarkers(oldIndex, this.tabStopIndex)
+    }
 
     return markerSelected
   }
 
   destroy () {
     this.subscriptions.dispose()
-    this.tabStopMarkers = []
+    this.insertionsByIndex = []
   }
 
   getMarkerLayer () {

lib/snippet.js

@@ -3,6 +3,19 @@ const TabStopList = require('./tab-stop-list')
 const {transformWithSubstitution} = require('./util')
 const {VariableResolver, TransformResolver} = require('./resolvers')
 
+const tabStopsReferencedWithinTabStopContent = (segment) => {
+  let results = []
+  for (let item of segment) {
+    if (item.index) {
+      results.push(
+        item.index,
+        ...tabStopsReferencedWithinTabStopContent(item.content)
+      )
+    }
+  }
+  return new Set(results)
+}
+
 module.exports = class Snippet {
   constructor(params) {
     this.name = params.name
@@ -80,8 +93,13 @@ function stringifyTabstop (node, params, acc) {
   const index = node.index === 0 ? Infinity : node.index
   const start = new Point(acc.row, acc.column)
   stringifyContent(node.content, params, acc)
+  let referencedTabStops = tabStopsReferencedWithinTabStopContent(node.content)
   const range = new Range(start, [acc.row, acc.column])
-  acc.tabStopList.findOrCreate({index, snippet: this}).addInsertion({range, substitution: node.substitution})
+  acc.tabStopList.findOrCreate({index, snippet: this}).addInsertion({
+    range,
+    substitution: node.substitution,
+    references: [...referencedTabStops]
+  })
 }
 
 function stringifyChoice (node, params, acc) {

spec/fixtures/test-snippets.cson

@@ -143,3 +143,13 @@
   'has a transformed tab stop such that it is possible to move the cursor between the ordinary tab stop and its transformed version without an intermediate step':
     prefix: 't18'
     body: '// $1\n// ${1/./=/g}'
+  "has two tab stops adjacent to one another":
+    prefix: 't19'
+    body: """
+    ${2:bar}${3:baz}
+    """
+  "has several adjacent tab stops, one of which has a placeholder with a reference to another tab stop at its edge":
+    prefix: 't20'
+    body: """
+    ${1:foo}${2:bar}${3:baz $1}$4
+    """