merge snippets, don't stack them, #27543

src/vs/editor/contrib/snippet/browser/snippetController2.ts

@@ -13,60 +13,6 @@ import { SnippetSession } from './snippetSession';
 import { EditorContextKeys } from 'vs/editor/common/editorContextKeys';
 import { KeyCode, KeyMod } from 'vs/base/common/keyCodes';
 
-class SnippetSessions {
-
-	private _stack: SnippetSession[] = [];
-
-	add(session: SnippetSession): number {
-		return this._stack.push(session);
-	}
-
-	clear(): void {
-		dispose(this._stack);
-		this._stack.length = 0;
-	}
-
-	get empty(): boolean {
-		return this._stack.length === 0;
-	}
-
-	get hasPlaceholder(): boolean {
-		return this._stack.some(s => s.hasPlaceholder);
-	}
-
-	get isAtFirstPlaceholder(): boolean {
-		return this._stack.every(s => s.isAtFirstPlaceholder);
-	}
-
-	get isAtFinalPlaceholder(): boolean {
-		return !this.empty && this._stack[0].isAtLastPlaceholder;
-	}
-
-	get isSelectionWithinPlaceholders(): boolean {
-		return this._stack.some(s => s.isSelectionWithinPlaceholders());
-	}
-
-	prev(): void {
-		for (let i = this._stack.length - 1; i >= 0; i--) {
-			const snippet = this._stack[i];
-			if (!snippet.isAtFirstPlaceholder) {
-				snippet.prev();
-				break;
-			}
-		}
-	}
-
-	next(): void {
-		for (let i = this._stack.length - 1; i >= 0; i--) {
-			const snippet = this._stack[i];
-			if (!snippet.isAtLastPlaceholder) {
-				snippet.next();
-				break;
-			}
-		}
-	}
-}
-
 @commonEditorContribution
 export class SnippetController2 {
 
@@ -82,7 +28,7 @@ export class SnippetController2 {
 	private readonly _hasNextTabstop: IContextKey<boolean>;
 	private readonly _hasPrevTabstop: IContextKey<boolean>;
 
-	private _sessions = new SnippetSessions();
+	private _session: SnippetSession;
 	private _snippetListener: IDisposable[] = [];
 	private _modelVersionId: number;
 
@@ -99,7 +45,7 @@ export class SnippetController2 {
 		this._inSnippet.reset();
 		this._hasPrevTabstop.reset();
 		this._hasNextTabstop.reset();
-		this._sessions.clear();
+		dispose(this._session);
 	}
 
 	getId(): string {
@@ -120,14 +66,12 @@ export class SnippetController2 {
 			this._editor.getModel().pushStackElement();
 		}
 
-		const snippet = new SnippetSession(this._editor, template, overwriteBefore, overwriteAfter);
-		const newLen = this._sessions.add(snippet);
-
-		if (newLen === 1) {
+		if (!this._session) {
 			this._modelVersionId = this._editor.getModel().getAlternativeVersionId();
-			snippet.insert(false);
+			this._session = new SnippetSession(this._editor, template, overwriteBefore, overwriteAfter);
+			this._session.insert();
 		} else {
-			snippet.insert(true);
+			this._session.merge(template, overwriteBefore, overwriteAfter);
 		}
 
 		if (undoStopAfter) {
@@ -142,7 +86,7 @@ export class SnippetController2 {
 	}
 
 	private _updateState(): void {
-		if (this._sessions.empty) {
+		if (!this._session) {
 			// canceled in the meanwhile
 			return;
 		}
@@ -153,19 +97,19 @@ export class SnippetController2 {
 			return this.cancel();
 		}
 
-		if (!this._sessions.hasPlaceholder) {
+		if (!this._session.hasPlaceholder) {
 			// don't listen for selection changes and don't
 			// update context keys when the snippet is plain text
 			return this.cancel();
 		}
 
-		if (this._sessions.isAtFinalPlaceholder || !this._sessions.isSelectionWithinPlaceholders) {
+		if (this._session.isAtLastPlaceholder || !this._session.isSelectionWithinPlaceholders()) {
 			return this.cancel();
 		}
 
 		this._inSnippet.set(true);
-		this._hasPrevTabstop.set(!this._sessions.isAtFirstPlaceholder);
-		this._hasNextTabstop.set(!this._sessions.isAtFinalPlaceholder);
+		this._hasPrevTabstop.set(!this._session.isAtFirstPlaceholder);
+		this._hasNextTabstop.set(!this._session.isAtLastPlaceholder);
 	}
 
 	finish(): void {
@@ -178,17 +122,19 @@ export class SnippetController2 {
 		this._inSnippet.reset();
 		this._hasPrevTabstop.reset();
 		this._hasNextTabstop.reset();
-		this._sessions.clear();
 		dispose(this._snippetListener);
+		dispose(this._session);
+		this._session = undefined;
+		this._modelVersionId = -1;
 	}
 
 	prev(): void {
-		this._sessions.prev();
+		this._session.prev();
 		this._updateState();
 	}
 
 	next(): void {
-		this._sessions.next();
+		this._session.next();
 		this._updateState();
 	}
 }

src/vs/editor/contrib/snippet/browser/snippetSession.ts

@@ -77,14 +77,14 @@ export class OneSnippet {
 		});
 	}
 
-	move(fwd: boolean): Selection[] {
+	move(fwd: boolean | undefined): Selection[] {
 
 		this._initDecorations();
 
-		if (fwd && this._placeholderGroupsIdx < this._placeholderGroups.length - 1) {
+		if (fwd === true && this._placeholderGroupsIdx < this._placeholderGroups.length - 1) {
 			this._placeholderGroupsIdx += 1;
 
-		} else if (!fwd && this._placeholderGroupsIdx > 0) {
+		} else if (fwd === false && this._placeholderGroupsIdx > 0) {
 			this._placeholderGroupsIdx -= 1;
 
 		} else {
@@ -153,6 +153,57 @@ export class OneSnippet {
 		});
 		return ret;
 	}
+
+	merge(others: OneSnippet[]): void {
+
+		const model = this._editor.getModel();
+
+		this._editor.changeDecorations(accessor => {
+
+			// For each active placeholder take one snippet and merge it
+			// in that the placeholder (can be many for `$1foo$1foo`). Because
+			// everything is sorted by editor selection we can simply remove
+			// elements from the beginning of the array
+			for (const placeholder of this._placeholderGroups[this._placeholderGroupsIdx]) {
+				const nested = others.shift();
+				console.assert(!nested._placeholderDecorations);
+
+				// Massage placeholder-indicies of the nested snippet to be
+				// sorted right after the insertion point. This ensures we move
+				// through the placeholders in the correct order
+				for (const nestedPlaceholder of nested._snippet.placeholders) {
+					if (nestedPlaceholder.isFinalTabstop) {
+						nestedPlaceholder.index = `${placeholder.index}.${nested._snippet.placeholders.length}`;
+					} else {
+						nestedPlaceholder.index = `${placeholder.index}.${nestedPlaceholder.index}`;
+					}
+				}
+				this._snippet.replace(placeholder, nested._snippet.children);
+
+				// Remove the placeholder at which position are inserting
+				// the snippet and also remove its decoration.
+				const id = this._placeholderDecorations.get(placeholder);
+				accessor.removeDecoration(id);
+				this._placeholderDecorations.delete(placeholder);
+
+				// For each *new* placeholder we create decoration to monitor
+				// how and if it grows/shrinks.
+				for (const placeholder of nested._snippet.placeholders) {
+					const placeholderOffset = nested._snippet.offset(placeholder);
+					const placeholderLen = nested._snippet.fullLen(placeholder);
+					const range = Range.fromPositions(
+						model.getPositionAt(nested._offset + placeholderOffset),
+						model.getPositionAt(nested._offset + placeholderOffset + placeholderLen)
+					);
+					const handle = accessor.addDecoration(range, OneSnippet._decor.inactive);
+					this._placeholderDecorations.set(placeholder, handle);
+				}
+			}
+
+			// Last, re-create the placeholder groups by sorting placeholders by their index.
+			this._placeholderGroups = groupBy(this._snippet.placeholders, Placeholder.compareByIndex);
+		});
+	}
 }
 
 export class SnippetSession {
@@ -192,41 +243,25 @@ export class SnippetSession {
 		return selection;
 	}
 
-	private readonly _editor: ICommonCodeEditor;
-	private readonly _template: string;
-	private readonly _overwriteBefore: number;
-	private readonly _overwriteAfter: number;
-	private _snippets: OneSnippet[] = [];
-
-	constructor(editor: ICommonCodeEditor, template: string, overwriteBefore: number = 0, overwriteAfter: number = 0) {
-		this._editor = editor;
-		this._template = template;
-		this._overwriteBefore = overwriteBefore;
-		this._overwriteAfter = overwriteAfter;
-	}
-
-	dispose(): void {
-		dispose(this._snippets);
-	}
+	static createEditsAndSnippets(editor: ICommonCodeEditor, template: string, overwriteBefore: number, overwriteAfter: number): { edits: IIdentifiedSingleEditOperation[], snippets: OneSnippet[] } {
 
-	insert(ignoreFinalTabstops: boolean = false): void {
-
-		const model = this._editor.getModel();
+		const model = editor.getModel();
 		const edits: IIdentifiedSingleEditOperation[] = [];
+		const snippets: OneSnippet[] = [];
 
 		let delta = 0;
 
 		// know what text the overwrite[Before|After] extensions
 		// of the primary curser have selected because only when
 		// secondary selections extend to the same text we can grow them
-		let firstBeforeText = model.getValueInRange(SnippetSession.adjustSelection(model, this._editor.getSelection(), this._overwriteBefore, 0));
-		let firstAfterText = model.getValueInRange(SnippetSession.adjustSelection(model, this._editor.getSelection(), 0, this._overwriteAfter));
+		let firstBeforeText = model.getValueInRange(SnippetSession.adjustSelection(model, editor.getSelection(), overwriteBefore, 0));
+		let firstAfterText = model.getValueInRange(SnippetSession.adjustSelection(model, editor.getSelection(), 0, overwriteAfter));
 
 		// sort selections by their start position but remeber
 		// the original index. that allows you to create correct
 		// offset-based selection logic without changing the
 		// primary selection
-		const indexedSelection = this._editor.getSelections()
+		const indexedSelection = editor.getSelections()
 			.map((selection, idx) => ({ selection, idx }))
 			.sort((a, b) => Range.compareRangesUsingStarts(a.selection, b.selection));
 
@@ -234,8 +269,8 @@ export class SnippetSession {
 
 			// extend selection with the `overwriteBefore` and `overwriteAfter` and then
 			// compare if this matches the extensions of the primary selection
-			let extensionBefore = SnippetSession.adjustSelection(model, selection, this._overwriteBefore, 0);
-			let extensionAfter = SnippetSession.adjustSelection(model, selection, 0, this._overwriteAfter);
+			let extensionBefore = SnippetSession.adjustSelection(model, selection, overwriteBefore, 0);
+			let extensionAfter = SnippetSession.adjustSelection(model, selection, 0, overwriteAfter);
 			if (firstBeforeText !== model.getValueInRange(extensionBefore)) {
 				extensionBefore = selection;
 			}
@@ -251,20 +286,10 @@ export class SnippetSession {
 			// adjust the template string to match the indentation and
 			// whitespace rules of this insert location (can be different for each cursor)
 			const start = snippetSelection.getStartPosition();
-			const adjustedTemplate = SnippetSession.adjustWhitespace(model, start, this._template);
+			const adjustedTemplate = SnippetSession.adjustWhitespace(model, start, template);
 
 			const snippet = SnippetParser.parse(adjustedTemplate).resolveVariables(new EditorSnippetVariableResolver(model, selection));
 
-			// rewrite final-tabstop to some other placeholder because this
-			// snippet sits inside another snippet
-			if (ignoreFinalTabstops) {
-				for (const placeholder of snippet.placeholders) {
-					if (placeholder.isFinalTabstop) {
-						placeholder.index = String(snippet.placeholders.length);
-					}
-				}
-			}
-
 			const offset = model.getOffsetAt(start) + delta;
 			delta += snippet.text.length - model.getValueLengthInRange(snippetSelection);
 
@@ -272,10 +297,36 @@ export class SnippetSession {
 			// that ensures the primiary cursor stays primary despite not being
 			// the one with lowest start position
 			edits[idx] = EditOperation.replaceMove(snippetSelection, snippet.text);
-			this._snippets[idx] = new OneSnippet(this._editor, snippet, offset);
+			snippets[idx] = new OneSnippet(editor, snippet, offset);
 		}
 
+		return { edits, snippets };
+	}
+
+	private readonly _editor: ICommonCodeEditor;
+	private readonly _template: string;
+	private readonly _overwriteBefore: number;
+	private readonly _overwriteAfter: number;
+	private _snippets: OneSnippet[] = [];
+
+	constructor(editor: ICommonCodeEditor, template: string, overwriteBefore: number = 0, overwriteAfter: number = 0) {
+		this._editor = editor;
+		this._template = template;
+		this._overwriteBefore = overwriteBefore;
+		this._overwriteAfter = overwriteAfter;
+	}
+
+	dispose(): void {
+		dispose(this._snippets);
+	}
+
+	insert(): void {
+
+		const model = this._editor.getModel();
+
 		// make insert edit and start with first selections
+		const { edits, snippets } = SnippetSession.createEditsAndSnippets(this._editor, this._template, this._overwriteBefore, this._overwriteAfter);
+		this._snippets = snippets;
 
 		this._editor.setSelections(model.pushEditOperations(this._editor.getSelections(), edits, undoEdits => {
 			if (this._snippets[0].hasPlaceholder) {
@@ -286,6 +337,24 @@ export class SnippetSession {
 		}));
 	}
 
+	merge(template: string, overwriteBefore: number = 0, overwriteAfter: number = 0): void {
+		const { edits, snippets } = SnippetSession.createEditsAndSnippets(this._editor, template, overwriteBefore, overwriteAfter);
+
+		this._editor.setSelections(this._editor.getModel().pushEditOperations(this._editor.getSelections(), edits, undoEdits => {
+
+			for (const snippet of this._snippets) {
+				snippet.merge(snippets);
+			}
+			console.assert(snippets.length === 0);
+
+			if (this._snippets[0].hasPlaceholder) {
+				return this._move(undefined);
+			} else {
+				return undoEdits.map(edit => Selection.fromPositions(edit.range.getEndPosition()));
+			}
+		}));
+	}
+
 	next(): void {
 		const newSelections = this._move(true);
 		this._editor.setSelections(newSelections);
@@ -296,7 +365,7 @@ export class SnippetSession {
 		this._editor.setSelections(newSelections);
 	}
 
-	private _move(fwd: boolean): Selection[] {
+	private _move(fwd: boolean | undefined): Selection[] {
 		const selections: Selection[] = [];
 		for (const snippet of this._snippets) {
 			const oneSelection = snippet.move(fwd);

src/vs/editor/contrib/snippet/test/browser/snippetController2.test.ts

@@ -175,34 +175,30 @@ suite('SnippetController2', function () {
 
 		ctrl.insert('farboo');
 		assertSelections(editor, new Selection(1, 7, 1, 7), new Selection(2, 11, 2, 11));
-		assertContextKeys(contextKeys, true, false, true);
-
-		ctrl.next();
-		assertSelections(editor, new Selection(1, 7, 1, 7), new Selection(2, 11, 2, 11));
 		assertContextKeys(contextKeys, false, false, false);
 	});
-	//
-	// 	test('Inconsistent tab stop behaviour with recursive snippets and tab / shift tab, #27543', function () {
-	// 		const ctrl = new SnippetController2(editor, contextKeys);
-	// 		ctrl.insert('1_calize(${1:nl}, \'${2:value}\')$0');
 
-	// 		assertContextKeys(contextKeys, true, false, true);
-	// 		assertSelections(editor, new Selection(1, 10, 1, 12), new Selection(2, 14, 2, 16));
+	test('Inconsistent tab stop behaviour with recursive snippets and tab / shift tab, #27543', function () {
+		const ctrl = new SnippetController2(editor, contextKeys);
+		ctrl.insert('1_calize(${1:nl}, \'${2:value}\')$0');
+
+		assertContextKeys(contextKeys, true, false, true);
+		assertSelections(editor, new Selection(1, 10, 1, 12), new Selection(2, 14, 2, 16));
 
-	// 		ctrl.insert('2_calize(${1:nl}, \'${2:value}\')$0');
+		ctrl.insert('2_calize(${1:nl}, \'${2:value}\')$0');
 
-	// 		assertSelections(editor, new Selection(1, 19, 1, 21), new Selection(2, 23, 2, 25));
+		assertSelections(editor, new Selection(1, 19, 1, 21), new Selection(2, 23, 2, 25));
 
-	// 		ctrl.next(); // inner `value`
-	// 		assertSelections(editor, new Selection(1, 24, 1, 29), new Selection(2, 28, 2, 33));
+		ctrl.next(); // inner `value`
+		assertSelections(editor, new Selection(1, 24, 1, 29), new Selection(2, 28, 2, 33));
 
-	// 		ctrl.next(); // inner `$0`
-	// 		assertSelections(editor, new Selection(1, 31, 1, 31), new Selection(2, 35, 2, 35));
+		ctrl.next(); // inner `$0`
+		assertSelections(editor, new Selection(1, 31, 1, 31), new Selection(2, 35, 2, 35));
 
-	// 		ctrl.next(); // outer `value`
-	// 		assertSelections(editor, new Selection(1, 34, 1, 39), new Selection(2, 38, 2, 43));
+		ctrl.next(); // outer `value`
+		assertSelections(editor, new Selection(1, 34, 1, 39), new Selection(2, 38, 2, 43));
 
-	// 		ctrl.prev(); // inner `$0`
-	// 		assertSelections(editor, new Selection(1, 31, 1, 31), new Selection(2, 35, 2, 35));
-	// 	});
+		ctrl.prev(); // inner `$0`
+		assertSelections(editor, new Selection(1, 31, 1, 31), new Selection(2, 35, 2, 35));
+	});
 });

src/vs/editor/contrib/snippet/test/browser/snippetParser.test.ts

@@ -387,17 +387,44 @@ suite('SnippetParser', () => {
 		assert.equal(placeholders.length, 3);
 	});
 
-	test('TextmateSnippet#replace', function () {
+	test('TextmateSnippet#replace 1/2', function () {
 		let snippet = SnippetParser.parse('aaa${1:bbb${2:ccc}}$0');
 
 		assert.equal(snippet.placeholders.length, 3);
 		const [, second] = snippet.placeholders;
 		assert.equal(second.index, '2');
 
+		const enclosing = snippet.enclosingPlaceholders(second);
+		assert.equal(enclosing.length, 1);
+		assert.equal(enclosing[0].index, '1');
+
 		let nested = SnippetParser.parse('ddd$1eee$0');
 		snippet.replace(second, nested.children);
 
 		assert.equal(snippet.text, 'aaabbbdddeee');
 		assert.equal(snippet.placeholders.length, 4);
+		assert.equal(snippet.placeholders[0].index, '1');
+		assert.equal(snippet.placeholders[1].index, '1');
+		assert.equal(snippet.placeholders[2].index, '0');
+		assert.equal(snippet.placeholders[3].index, '0');
+
+		const newEnclosing = snippet.enclosingPlaceholders(snippet.placeholders[1]);
+		assert.ok(newEnclosing[0] === snippet.placeholders[0]);
+		assert.equal(newEnclosing.length, 1);
+		assert.equal(newEnclosing[0].index, '1');
+	});
+
+	test('TextmateSnippet#replace 2/2', function () {
+		let snippet = SnippetParser.parse('aaa${1:bbb${2:ccc}}$0');
+
+		assert.equal(snippet.placeholders.length, 3);
+		const [, second] = snippet.placeholders;
+		assert.equal(second.index, '2');
+
+		let nested = SnippetParser.parse('dddeee$0');
+		snippet.replace(second, nested.children);
+
+		assert.equal(snippet.text, 'aaabbbdddeee');
+		assert.equal(snippet.placeholders.length, 3);
 	});
 });

src/vs/editor/contrib/snippet/test/browser/snippetSession.test.ts

@@ -403,5 +403,35 @@ suite('SnippetSession', function () {
 		assert.equal(model.getValue(), '@line=1function foo() {\n    @line=2console.log(a);\n}');
 		assertSelections(editor, new Selection(1, 8, 1, 8), new Selection(2, 12, 2, 12));
 	});
+
+	test('snippets, merge', function () {
+		editor.setSelection(new Selection(1, 1, 1, 1));
+		const session = new SnippetSession(editor, 'This ${1:is ${2:nested}}.$0');
+		session.insert();
+		session.next();
+		assertSelections(editor, new Selection(1, 9, 1, 15));
+
+		session.merge('really ${1:nested}$0');
+		assertSelections(editor, new Selection(1, 16, 1, 22));
+
+		session.next();
+		assertSelections(editor, new Selection(1, 22, 1, 22));
+		assert.equal(session.isAtLastPlaceholder, false);
+
+		session.next();
+		assert.equal(session.isAtLastPlaceholder, true);
+		assertSelections(editor, new Selection(1, 23, 1, 23));
+
+		session.prev();
+		editor.trigger('test', 'type', { text: 'AAA' });
+
+		// back to `really ${1:nested}`
+		session.prev();
+		assertSelections(editor, new Selection(1, 16, 1, 22));
+
+		// back to `${1:is ...}` which now grew
+		session.prev();
+		assertSelections(editor, new Selection(1, 6, 1, 25));
+	});
 });