diff --git a/src/vs/editor/contrib/snippet/browser/snippetParser.ts b/src/vs/editor/contrib/snippet/browser/snippetParser.ts index ce4128189d8..779ed9263ef 100644 --- a/src/vs/editor/contrib/snippet/browser/snippetParser.ts +++ b/src/vs/editor/contrib/snippet/browser/snippetParser.ts @@ -132,7 +132,6 @@ export class Scanner { } export abstract class Marker { - _markerBrand: any; static toString(marker?: Marker[]): string { let result = ''; @@ -142,17 +141,30 @@ export abstract class Marker { return result; } - parent: Marker; + readonly _markerBrand: any; - private _children: Marker[] = []; + public parent: Marker; + protected _children: Marker[] = []; - set children(marker: Marker[]) { - this._children = []; - for (const m of marker) { - m.parent = this; - this._children.push(m); + appendChild(child: Marker): this { + if (child instanceof Text && this._children[this._children.length - 1] instanceof Text) { + // this and previous child are text -> merge them + (this._children[this._children.length - 1]).value += child.value; + } else { + // normal adoption of child + child.parent = this; + this._children.push(child); } - // Object.freeze(this._children); + return this; + } + + replace(child: Marker, others: Marker[]): void { + const { parent } = child; + const idx = parent.children.indexOf(child); + const newChildren = parent.children.slice(0); + newChildren.splice(idx, 1, ...others); + parent._children = newChildren; + others.forEach(node => node.parent = parent); } get children(): Marker[] { @@ -171,17 +183,17 @@ export abstract class Marker { } export class Text extends Marker { - constructor(public string: string) { + constructor(public value: string) { super(); } toString() { - return this.string; + return this.value; } len(): number { - return this.string.length; + return this.value.length; } clone(): Text { - return new Text(this.string); + return new Text(this.value); } } @@ -203,10 +215,10 @@ export class Placeholder extends Marker { } } - constructor(public index: number, children: Marker[] = []) { + constructor(public index: number) { super(); - this.children = children; } + get isFinalTabstop() { return this.index === 0; } @@ -214,34 +226,34 @@ export class Placeholder extends Marker { return Marker.toString(this.children); } clone(): Placeholder { - return new Placeholder(this.index, this.children.map(child => child.clone())); + let ret = new Placeholder(this.index); + ret._children = this.children.map(child => child.clone()); + return ret; } } export class Variable extends Marker { - resolvedValue: string; - - constructor(public name: string, children: Marker[] = []) { + constructor(public name: string) { super(); - this.children = children; } - get isDefined(): boolean { - return this.resolvedValue !== undefined; - } - len(): number { - if (this.isDefined) { - return this.resolvedValue.length; - } else { - return super.len(); + + resolve(resolver: { resolve(name: string): string }): boolean { + const value = resolver.resolve(this.name); + if (value !== undefined) { + this._children = [new Text(value)]; + return true; } + return false; } + toString() { - return this.isDefined ? this.resolvedValue : Marker.toString(this.children); + return Marker.toString(this.children); } + clone(): Variable { - const ret = new Variable(this.name, this.children.map(child => child.clone())); - ret.resolvedValue = this.resolvedValue; + const ret = new Variable(this.name); + ret._children = this.children.map(child => child.clone()); return ret; } } @@ -262,11 +274,6 @@ export class TextmateSnippet extends Marker { private _placeholders: Placeholder[]; - constructor(marker: Marker[]) { - super(); - this.children = marker; - } - get placeholders(): Placeholder[] { if (!this._placeholders) { // fill in placeholders @@ -327,10 +334,8 @@ export class TextmateSnippet extends Marker { resolveVariables(resolver: { resolve(name: string): string }): this { this.walk(candidate => { if (candidate instanceof Variable) { - candidate.resolvedValue = resolver.resolve(candidate.name); - if (candidate.isDefined) { - // remove default value from resolved variable - candidate.children = []; + if (candidate.resolve(resolver)) { + this._placeholders = undefined; } } return true; @@ -338,17 +343,20 @@ export class TextmateSnippet extends Marker { return this; } - replace(marker: Marker, others: Marker[]): void { - const { parent } = marker; - const idx = parent.children.indexOf(marker); - const newChildren = parent.children.slice(0); - newChildren.splice(idx, 1, ...others); - parent.children = newChildren; + appendChild(child: Marker) { this._placeholders = undefined; + return super.appendChild(child); + } + + replace(child: Marker, others: Marker[]): void { + this._placeholders = undefined; + return super.replace(child, others); } clone(): TextmateSnippet { - return new TextmateSnippet(this.children.map(child => child.clone())); + let ret = new TextmateSnippet(); + this._children = this.children.map(child => child.clone()); + return ret; } walk(visitor: (marker: Marker) => boolean): void { @@ -370,57 +378,38 @@ export class SnippetParser { } parse(value: string, insertFinalTabstop?: boolean, enforceFinalTabstop?: boolean): TextmateSnippet { - const marker: Marker[] = []; this._scanner.text(value); this._token = this._scanner.next(); - while (this._parse(marker)) { + + const snippet = new TextmateSnippet(); + while (this._parse(snippet)) { // nothing } - // * fill in default for empty placeHolders - // * compact sibling Text markers + // fill in values for placeholders. the first placeholder of an index + // that has a value defines the value for all placeholders with that index const placeholderDefaultValues = new Map(); const incompletePlaceholders: Placeholder[] = []; - - function walk(marker: Marker[]) { - - for (let i = 0; i < marker.length; i++) { - const thisMarker = marker[i]; - - // fill in default values for repeated placeholders - // like `${1:foo}and$1` becomes `${1:foo}and${1:foo}` - // and `$1and${1:foo}` becomes `${1:foo}and${1:foo}` - if (thisMarker instanceof Placeholder) { - // the first placeholder that has a value define the - // value for *all* placeholders of this index - if (thisMarker.isFinalTabstop) { - placeholderDefaultValues.set(0); - } else if (!placeholderDefaultValues.has(thisMarker.index) && thisMarker.children.length > 0) { - placeholderDefaultValues.set(thisMarker.index, thisMarker.children); - walk(thisMarker.children); - - } else { - incompletePlaceholders.push(thisMarker); - } - - - } else if (thisMarker instanceof Variable) { - walk(thisMarker.children); - - } else if (i > 0 && thisMarker instanceof Text && marker[i - 1] instanceof Text) { - (marker[i - 1]).string += (marker[i]).string; - marker.splice(i, 1); - i--; + snippet.walk(marker => { + if (marker instanceof Placeholder) { + if (marker.isFinalTabstop) { + placeholderDefaultValues.set(0); + } else if (!placeholderDefaultValues.has(marker.index) && marker.children.length > 0) { + placeholderDefaultValues.set(marker.index, marker.children); + } else { + incompletePlaceholders.push(marker); } } - } - walk(marker); - - // fill in defaults + return true; + }); for (const placeholder of incompletePlaceholders) { if (placeholderDefaultValues.has(placeholder.index)) { - placeholder.children = placeholderDefaultValues.get(placeholder.index).map(m => m.clone()); + const clone = new Placeholder(placeholder.index); + for (const child of placeholderDefaultValues.get(placeholder.index)) { + clone.appendChild(child.clone()); + } + snippet.replace(placeholder, [clone]); } } @@ -430,10 +419,10 @@ export class SnippetParser { ) { // the snippet uses placeholders but has no // final tabstop defined -> insert at the end - marker.push(new Placeholder(0, [])); + snippet.appendChild(new Placeholder(0)); } - return new TextmateSnippet(marker); + return snippet; } private _accept(type: TokenType): boolean; @@ -447,13 +436,13 @@ export class SnippetParser { return false; } - private _parse(marker: Marker[]): boolean { + private _parse(marker: Marker): boolean { return this._parseEscaped(marker) || this._parsePlaceholderOrVariable(marker) || this._parseAnything(marker); } - private _parseEscaped(marker: Marker[]): boolean { + private _parseEscaped(marker: Marker): boolean { let value: string; if (value = this._accept(TokenType.Backslash, true)) { // saw a backslash, append escaped token or that backslash @@ -462,13 +451,13 @@ export class SnippetParser { || this._accept(TokenType.Backslash, true) || value; - marker.push(new Text(value)); + marker.appendChild(new Text(value)); return true; } return false; } - private _parsePlaceholderOrVariable(marker: Marker[]): boolean { + private _parsePlaceholderOrVariable(marker: Marker): boolean { if (!this._accept(TokenType.Dollar)) { return false; @@ -479,7 +468,7 @@ export class SnippetParser { if (value) { // $foo -> variable, $1 -> tabstop - marker.push(/^\d+$/.test(value) ? new Placeholder(Number(value)) : new Variable(value)); + marker.appendChild(/^\d+$/.test(value) ? new Placeholder(Number(value)) : new Variable(value)); return true; } else if (this._accept(TokenType.CurlyOpen)) { @@ -489,45 +478,43 @@ export class SnippetParser { || this._accept(TokenType.Int, true); if (!value) { - marker.push(new Text('${')); + marker.appendChild(new Text('${')); return true; } let placeholderOrVariable = /^\d+$/.test(value) ? new Placeholder(Number(value)) : new Variable(value); - let children: Marker[] = []; while (true) { if (this._accept(TokenType.CurlyClose)) { - placeholderOrVariable.children = children; - marker.push(placeholderOrVariable); + marker.appendChild(placeholderOrVariable); return true; } - if (children.length === 0 && !this._accept(TokenType.Colon)) { - marker.push(new Text('${' + value)); + if (placeholderOrVariable.children.length === 0 && !this._accept(TokenType.Colon)) { + marker.appendChild(new Text('${' + value)); return true; } - if (this._parse(children)) { + if (this._parse(placeholderOrVariable)) { continue; } // fallback - marker.push(new Text('${' + value + ':')); - marker.push(...children); + marker.appendChild(new Text('${' + value + ':')); + placeholderOrVariable.children.forEach(marker.appendChild, marker); return true; } } else { - marker.push(new Text('$')); + marker.appendChild(new Text('$')); return true; } } - private _parseAnything(marker: Marker[]): boolean { + private _parseAnything(marker: Marker): boolean { if (this._token.type !== TokenType.EOF) { - marker.push(new Text(this._scanner.tokenText(this._token))); + marker.appendChild(new Text(this._scanner.tokenText(this._token))); this._accept(undefined); return true; } diff --git a/src/vs/editor/contrib/snippet/test/browser/snippetParser.test.ts b/src/vs/editor/contrib/snippet/test/browser/snippetParser.test.ts index 2b033fda205..3156cab7ea8 100644 --- a/src/vs/editor/contrib/snippet/test/browser/snippetParser.test.ts +++ b/src/vs/editor/contrib/snippet/test/browser/snippetParser.test.ts @@ -405,8 +405,8 @@ suite('SnippetParser', () => { test('Snippet order for placeholders, #28185', function () { - const _10 = new Placeholder(10, []); - const _2 = new Placeholder(2, []); + const _10 = new Placeholder(10); + const _2 = new Placeholder(2); assert.equal(Placeholder.compareByIndex(_10, _2), 1); }); diff --git a/src/vs/workbench/parts/emmet/electron-browser/editorAccessor.ts b/src/vs/workbench/parts/emmet/electron-browser/editorAccessor.ts index 453b58fa72a..8d141defe26 100644 --- a/src/vs/workbench/parts/emmet/electron-browser/editorAccessor.ts +++ b/src/vs/workbench/parts/emmet/electron-browser/editorAccessor.ts @@ -141,7 +141,7 @@ export class EditorAccessor implements emmet.Editor { // write back as string function toSnippetString(marker: Marker): string { if (marker instanceof Text) { - return SnippetParser.escape(marker.string); + return SnippetParser.escape(marker.value); } else if (marker instanceof Placeholder) { if (marker.children.length > 0) { diff --git a/src/vs/workbench/parts/snippets/electron-browser/TMSnippets.ts b/src/vs/workbench/parts/snippets/electron-browser/TMSnippets.ts index f70db475804..e9566b08c23 100644 --- a/src/vs/workbench/parts/snippets/electron-browser/TMSnippets.ts +++ b/src/vs/workbench/parts/snippets/electron-browser/TMSnippets.ts @@ -169,7 +169,7 @@ function _rewriteBogousVariables(snippet: ISnippet): boolean { function fixBogousVariables(marker: Marker): string { if (marker instanceof Text) { - return SnippetParser.escape(marker.string); + return SnippetParser.escape(marker.value); } else if (marker instanceof Placeholder) { if (marker.children.length > 0) {