Make snippet parsing and expansion lazy

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,22 +13,25 @@ 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))
     }
 
     this.editor.transact(() => {
       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 {

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,32 +23,45 @@ 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
           if (index === 0) { index = Infinity; }
           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
   }
 }

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)
   },

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('<span style=\"color:red\">Label</span>');
       expect(snippet.rightLabelHTML).toBe('<span style=\"color:white\">Label</span>');
+      expect(instance.bodyText).toBe('testing 456');
     });
   });
 
@@ -105,8 +112,12 @@ describe("Snippet Loading", () => {
       activateSnippetsPackage();
 
       runs(() => {
-        const snippet = snippetsService.snippetsForScopes(['.source.js'])['log'];
-        expect(snippet.body).toBe("from-a-community-package");
+        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");
       });
     });
   });
@@ -148,7 +159,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 +179,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 +211,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 +227,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(() => {

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('');