From 684a540c3b3b91d753bb11f1bd6cf5474b5cacdc Mon Sep 17 00:00:00 2001 From: Benjamin Gray Date: Mon, 13 Jul 2020 23:21:42 +1000 Subject: [PATCH 1/2] make snippet parsing and expansion lazy --- lib/snippet-expansion.js | 16 ++++++------ lib/snippet.js | 49 ++++++++++++++++++++++++++++-------- lib/snippets.js | 15 +++-------- spec/snippet-loading-spec.js | 39 ++++++++++++++++------------ spec/snippets-spec.js | 40 ----------------------------- 5 files changed, 73 insertions(+), 86 deletions(-) diff --git a/lib/snippet-expansion.js b/lib/snippet-expansion.js index 54a525bc..7a468f2a 100644 --- a/lib/snippet-expansion.js +++ b/lib/snippet-expansion.js @@ -5,7 +5,6 @@ module.exports = class SnippetExpansion { this.settingTabStop = false this.isIgnoringBufferChanges = false this.onUndoOrRedo = this.onUndoOrRedo.bind(this) - this.snippet = snippet this.editor = editor this.cursor = cursor this.snippets = snippets @@ -14,14 +13,17 @@ module.exports = class SnippetExpansion { this.selections = [this.cursor.selection] const startPosition = this.cursor.selection.getBufferRange().start - let {body, tabStopList} = this.snippet + const indent = this.editor.lineTextForBufferRow(startPosition.row).match(/^\s*/)[0] + const {bodyText, lineCount, tabStopList} = snippet.generateInstance({editor, cursor, indent, startPosition}); + + let body = bodyText; + this.hasEndStop = tabStopList.hasEndStop; + let tabStops = tabStopList.toArray() - let indent = this.editor.lineTextForBufferRow(startPosition.row).match(/^\s*/)[0] - if (this.snippet.lineCount > 1 && indent) { + if (lineCount > 1 && indent) { // Add proper leading indentation to the snippet body = body.replace(/\n/g, `\n${indent}`) - tabStops = tabStops.map(tabStop => tabStop.copyWithIndent(indent)) } @@ -29,7 +31,7 @@ module.exports = class SnippetExpansion { this.ignoringBufferChanges(() => { this.editor.transact(() => { const newRange = this.cursor.selection.insertText(body, {autoIndent: false}) - if (this.snippet.tabStopList.length > 0) { + if (tabStopList.length > 0) { this.subscriptions.add(this.cursor.onDidChangePosition(event => this.cursorMoved(event))) this.subscriptions.add(this.cursor.onDidDestroy(() => this.cursorDestroyed())) this.placeTabStopMarkers(startPosition, tabStops) @@ -152,7 +154,7 @@ module.exports = class SnippetExpansion { } else { // The user has tabbed past the last tab stop. If the last tab stop is a // $0, we shouldn't move the cursor any further. - if (this.snippet.tabStopList.hasEndStop) { + if (this.hasEndStop) { this.destroy() return false } else { diff --git a/lib/snippet.js b/lib/snippet.js index fcdfed90..085193b6 100644 --- a/lib/snippet.js +++ b/lib/snippet.js @@ -1,8 +1,20 @@ const {Range} = require('atom') const TabStopList = require('./tab-stop-list') +let bodyParser; +function getBodyParser () { + if (bodyParser == null) { + bodyParser = require('./snippet-body-parser') + } + return bodyParser +} + +/** + * A template for generating Snippet Expansions. Holds the parse tree of the snippet source (lazily), resolving it + * to a concrete insertion text + tab stops + transformations on demand, based on the provided context. + */ module.exports = class Snippet { - constructor({name, prefix, bodyText, description, descriptionMoreURL, rightLabelHTML, leftLabel, leftLabelHTML, bodyTree}) { + constructor({name, prefix, bodyText, description, descriptionMoreURL, rightLabelHTML, leftLabel, leftLabelHTML, bodyTree=null}) { this.name = name this.prefix = prefix this.bodyText = bodyText @@ -11,17 +23,30 @@ module.exports = class Snippet { this.rightLabelHTML = rightLabelHTML this.leftLabel = leftLabel this.leftLabelHTML = leftLabelHTML - this.tabStopList = new TabStopList(this) - this.body = this.extractTabStops(bodyTree) + this.bodyTree = bodyTree + this.instanceCache = null // cache for non-dynamic expansion } - extractTabStops (bodyTree) { + /** + * Takes this snippet "template" and returns insertion text + tab stops, where all variables have been evaluated + */ + generateInstance(_context={}) { + if (this.instanceCache) { + return this.instanceCache; + } + + if (!this.bodyTree) { + this.bodyTree = getBodyParser().parse(this.bodyText) + } + const bodyText = [] + const tabStopList = new TabStopList(this); let row = 0 let column = 0 + let dynamic = false // if this snippet has components that may depend on `context` (e.g., variables) // recursive helper function; mutates vars above - let extractTabStops = bodyTree => { + const extractTabStops = bodyTree => { for (const segment of bodyTree) { if (segment.index != null) { let {index, content, substitution} = segment @@ -29,14 +54,14 @@ module.exports = class Snippet { const start = [row, column] extractTabStops(content) const range = new Range(start, [row, column]) - const tabStop = this.tabStopList.findOrCreate({ + const tabStop = tabStopList.findOrCreate({ index, snippet: this }) tabStop.addInsertion({ range, substitution }) } else if (typeof segment === 'string') { bodyText.push(segment) - var segmentLines = segment.split('\n') + const segmentLines = segment.split('\n') column += segmentLines.shift().length let nextLine while ((nextLine = segmentLines.shift()) != null) { @@ -47,10 +72,12 @@ module.exports = class Snippet { } } - extractTabStops(bodyTree) - this.lineCount = row + 1 - this.insertions = this.tabStopList.getInsertions() + extractTabStops(this.bodyTree) - return bodyText.join('') + const result = {bodyText: bodyText.join(''), lineCount: row + 1, tabStopList} + if (!dynamic) { + this.instanceCache = result + } + return result } } diff --git a/lib/snippets.js b/lib/snippets.js index 8e67ec0b..ef414d90 100644 --- a/lib/snippets.js +++ b/lib/snippets.js @@ -416,9 +416,8 @@ module.exports = { getParsedSnippet (attributes) { let snippet = this.parsedSnippetsById.get(attributes.id) if (snippet == null) { - let {id, prefix, name, body, bodyTree, description, descriptionMoreURL, rightLabelHTML, leftLabel, leftLabelHTML} = attributes - if (bodyTree == null) { bodyTree = this.getBodyParser().parse(body) } - snippet = new Snippet({id, name, prefix, bodyTree, description, descriptionMoreURL, rightLabelHTML, leftLabel, leftLabelHTML, bodyText: body}) + let {id, prefix, name, body, description, descriptionMoreURL, rightLabelHTML, leftLabel, leftLabelHTML} = attributes + snippet = new Snippet({id, name, prefix, description, descriptionMoreURL, rightLabelHTML, leftLabel, leftLabelHTML, bodyText: body}) this.parsedSnippetsById.set(attributes.id, snippet) } return snippet @@ -432,13 +431,6 @@ module.exports = { } }, - getBodyParser () { - if (this.bodyParser == null) { - this.bodyParser = require('./snippet-body-parser') - } - return this.bodyParser - }, - // Get an {Object} with these keys: // * `snippetPrefix`: the possible snippet prefix text preceding the cursor // * `wordPrefix`: the word preceding the cursor @@ -623,8 +615,7 @@ module.exports = { if (editor == null) { editor = atom.workspace.getActiveTextEditor() } if (cursor == null) { cursor = editor.getLastCursor() } if (typeof snippet === 'string') { - const bodyTree = this.getBodyParser().parse(snippet) - snippet = new Snippet({name: '__anonymous', prefix: '', bodyTree, bodyText: snippet}) + snippet = new Snippet({name: '__anonymous', prefix: '', bodyText: snippet}) } return new SnippetExpansion(snippet, editor, cursor, this) }, diff --git a/spec/snippet-loading-spec.js b/spec/snippet-loading-spec.js index d6c5dd18..6d98affd 100644 --- a/spec/snippet-loading-spec.js +++ b/spec/snippet-loading-spec.js @@ -3,7 +3,7 @@ const fs = require('fs-plus'); const temp = require('temp').track(); describe("Snippet Loading", () => { - let configDirPath, snippetsService; + let configDirPath, snippetsService, defaultContext; beforeEach(() => { configDirPath = temp.mkdirSync('atom-config-dir-'); @@ -39,18 +39,20 @@ describe("Snippet Loading", () => { runs(() => { const jsonSnippet = snippetsService.snippetsForScopes(['.source.json'])['snip']; + let instance = jsonSnippet.generateInstance(); expect(jsonSnippet.name).toBe('Atom Snippet'); expect(jsonSnippet.prefix).toBe('snip'); - expect(jsonSnippet.body).toContain('"prefix":'); - expect(jsonSnippet.body).toContain('"body":'); - expect(jsonSnippet.tabStopList.length).toBeGreaterThan(0); + expect(instance.bodyText).toContain('"prefix":'); + expect(instance.bodyText).toContain('"body":'); + expect(instance.tabStopList.length).toBeGreaterThan(0); const csonSnippet = snippetsService.snippetsForScopes(['.source.coffee'])['snip']; + instance = csonSnippet.generateInstance(); expect(csonSnippet.name).toBe('Atom Snippet'); expect(csonSnippet.prefix).toBe('snip'); - expect(csonSnippet.body).toContain("'prefix':"); - expect(csonSnippet.body).toContain("'body':"); - expect(csonSnippet.tabStopList.length).toBeGreaterThan(0); + expect(instance.bodyText).toContain("'prefix':"); + expect(instance.bodyText).toContain("'body':"); + expect(instance.tabStopList.length).toBeGreaterThan(0); }); }); @@ -59,25 +61,30 @@ describe("Snippet Loading", () => { runs(() => { let snippet = snippetsService.snippetsForScopes(['.test'])['test']; + let instance = snippet.generateInstance(); expect(snippet.prefix).toBe('test'); - expect(snippet.body).toBe('testing 123'); + expect(instance.bodyText).toBe('testing 123'); snippet = snippetsService.snippetsForScopes(['.test'])['testd']; + instance = snippet.generateInstance(); expect(snippet.prefix).toBe('testd'); - expect(snippet.body).toBe('testing 456'); expect(snippet.description).toBe('a description'); expect(snippet.descriptionMoreURL).toBe('http://google.com'); + expect(instance.bodyText).toBe('testing 456'); snippet = snippetsService.snippetsForScopes(['.test'])['testlabelleft']; + instance = snippet.generateInstance(); expect(snippet.prefix).toBe('testlabelleft'); - expect(snippet.body).toBe('testing 456'); expect(snippet.leftLabel).toBe('a label'); + expect(instance.bodyText).toBe('testing 456'); + snippet = snippetsService.snippetsForScopes(['.test'])['testhtmllabels']; + instance = snippet.generateInstance(); expect(snippet.prefix).toBe('testhtmllabels'); - expect(snippet.body).toBe('testing 456'); expect(snippet.leftLabelHTML).toBe('Label'); expect(snippet.rightLabelHTML).toBe('Label'); + expect(instance.bodyText).toBe('testing 456'); }); }); @@ -106,7 +113,7 @@ describe("Snippet Loading", () => { runs(() => { const snippet = snippetsService.snippetsForScopes(['.source.js'])['log']; - expect(snippet.body).toBe("from-a-community-package"); + expect(snippet.generateInstance().bodyText).toBe("from-a-community-package"); }); }); }); @@ -148,7 +155,7 @@ describe("Snippet Loading", () => { runs(() => { expect(snippet.name).toBe('foo snippet'); expect(snippet.prefix).toBe("foo"); - expect(snippet.body).toBe("bar1"); + expect(snippet.generateInstance().bodyText).toBe("bar1"); }); }); @@ -168,7 +175,7 @@ describe("Snippet Loading", () => { waitsFor("snippets to be changed", () => { const snippet = snippetsService.snippetsForScopes(['.foo'])['foo']; - return snippet && snippet.body === 'bar2'; + return snippet && snippet.generateInstance().bodyText === 'bar2'; }); runs(() => { @@ -200,7 +207,7 @@ describe("Snippet Loading", () => { runs(() => { expect(snippet.name).toBe('foo snippet'); expect(snippet.prefix).toBe("foo"); - expect(snippet.body).toBe("bar1"); + expect(snippet.generateInstance().bodyText).toBe("bar1"); }); }); @@ -216,7 +223,7 @@ describe("Snippet Loading", () => { waitsFor("snippets to be changed", () => { const snippet = snippetsService.snippetsForScopes(['.foo'])['foo']; - return snippet && snippet.body === 'bar2'; + return snippet && snippet.generateInstance().bodyText === 'bar2'; }); runs(() => { diff --git a/spec/snippets-spec.js b/spec/snippets-spec.js index 68994478..6fcd4bb6 100644 --- a/spec/snippets-spec.js +++ b/spec/snippets-spec.js @@ -289,46 +289,6 @@ third tabstop $3\ }); }); - it("parses snippets once, reusing cached ones on subsequent queries", () => { - spyOn(Snippets, "getBodyParser").andCallThrough(); - - editor.insertText("t1"); - simulateTabKeyEvent(); - - expect(Snippets.getBodyParser).toHaveBeenCalled(); - expect(editor.lineTextForBufferRow(0)).toBe("this is a testvar quicksort = function () {"); - expect(editor.getCursorScreenPosition()).toEqual([0, 14]); - - Snippets.getBodyParser.reset(); - - editor.setText(""); - editor.insertText("t1"); - simulateTabKeyEvent(); - - expect(Snippets.getBodyParser).not.toHaveBeenCalled(); - expect(editor.lineTextForBufferRow(0)).toBe("this is a test"); - expect(editor.getCursorScreenPosition()).toEqual([0, 14]); - - Snippets.getBodyParser.reset(); - - Snippets.add(__filename, { - ".source.js": { - "invalidate previous snippet": { - prefix: "t1", - body: "new snippet" - } - } - }); - - editor.setText(""); - editor.insertText("t1"); - simulateTabKeyEvent(); - - expect(Snippets.getBodyParser).toHaveBeenCalled(); - expect(editor.lineTextForBufferRow(0)).toBe("new snippet"); - expect(editor.getCursorScreenPosition()).toEqual([0, 11]); - }); - describe("when the snippet body is invalid or missing", () => { it("does not register the snippet", () => { editor.setText(''); From 13c2fa58e76aae409abcb96dd2be7740dba5f066 Mon Sep 17 00:00:00 2001 From: Benjamin Gray Date: Tue, 14 Jul 2020 13:47:26 +1000 Subject: [PATCH 2/2] investigate loaded snippets --- spec/snippet-loading-spec.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/spec/snippet-loading-spec.js b/spec/snippet-loading-spec.js index 6d98affd..448b2c11 100644 --- a/spec/snippet-loading-spec.js +++ b/spec/snippet-loading-spec.js @@ -112,7 +112,11 @@ describe("Snippet Loading", () => { activateSnippetsPackage(); runs(() => { - const snippet = snippetsService.snippetsForScopes(['.source.js'])['log']; + expect(atom.packages.getLoadedPackages().length).toBe(2); + expect(atom.packages.isPackageLoaded("package-with-snippets")).toBe(true); + expect(atom.packages.isPackageLoaded("language-javascript")).toBe(true); + + const snippet = snippetsService.snippetsForScopes([".source.js"])["log"]; expect(snippet.generateInstance().bodyText).toBe("from-a-community-package"); }); });