From d812cf0813333c8ac0447a589226b9b47cc4a7de Mon Sep 17 00:00:00 2001 From: Andrew Dupont Date: Mon, 17 Jun 2019 18:46:01 -0500 Subject: [PATCH] Bring the changes from #281 into #288. --- lib/insertion.js | 3 +- lib/snippet-expansion.js | 191 ++++++++++++++++++++++++++----- lib/snippet.js | 20 +++- spec/fixtures/test-snippets.cson | 10 ++ spec/snippets-spec.js | 38 ++++++ 5 files changed, 231 insertions(+), 31 deletions(-) diff --git a/lib/insertion.js b/lib/insertion.js index b80d2ea..0519d78 100644 --- a/lib/insertion.js +++ b/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 = '' } diff --git a/lib/snippet-expansion.js b/lib/snippet-expansion.js index b2b8af0..928b2ea 100644 --- a/lib/snippet-expansion.js +++ b/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,19 +301,29 @@ 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 } @@ -210,6 +331,8 @@ module.exports = class SnippetExpansion { } 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 () { diff --git a/lib/snippet.js b/lib/snippet.js index 2d471ad..8e55541 100644 --- a/lib/snippet.js +++ b/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) { diff --git a/spec/fixtures/test-snippets.cson b/spec/fixtures/test-snippets.cson index 72bfe31..2472914 100644 --- a/spec/fixtures/test-snippets.cson +++ b/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 + """ diff --git a/spec/snippets-spec.js b/spec/snippets-spec.js index 8d11825..5e01821 100644 --- a/spec/snippets-spec.js +++ b/spec/snippets-spec.js @@ -734,6 +734,44 @@ describe('Snippets extension', () => { }) }) + describe('when the snippet has two adjacent tab stops', () => { + it('ensures insertions are treated as part of the active tab stop', () => { + editor.setText('t19') + editor.setCursorScreenPosition([0, 3]) + simulateTabKeyEvent() + expect(editor.getText()).toBe('barbaz') + expect(editor.getSelectedBufferRange()).toEqual([[0, 0], [0, 3]]) + editor.insertText('w') + expect(editor.getText()).toBe('wbaz') + editor.insertText('at') + expect(editor.getText()).toBe('watbaz') + simulateTabKeyEvent() + expect(editor.getSelectedBufferRange()).toEqual([[0, 3], [0, 6]]) + editor.insertText('foo') + expect(editor.getText()).toBe('watfoo') + }) + }) + + describe('when the snippet has a placeholder with a tabstop mirror at its edge', () => { + it('allows the associated marker to include the inserted text', () => { + editor.setText('t20') + editor.setCursorScreenPosition([0, 3]) + simulateTabKeyEvent() + expect(editor.getText()).toBe('foobarbaz ') + expect(editor.getCursors().length).toBe(2) + let selections = editor.getSelections() + expect(selections[0].getBufferRange()).toEqual([[0, 0], [0, 3]]) + expect(selections[1].getBufferRange()).toEqual([[0, 10], [0, 10]]) + editor.insertText('nah') + expect(editor.getText()).toBe('nahbarbaz nah') + simulateTabKeyEvent() + editor.insertText('meh') + simulateTabKeyEvent() + editor.insertText('yea') + expect(editor.getText()).toBe('nahmehyea') + }) + }) + describe('when there are multiple cursors', () => { describe('when the cursors share a common snippet prefix', () => { it('expands the snippet for all cursors and allows simultaneous editing', () => {