[styleguide] Fix error when styleguide is shown…

.eslintrc.js

@@ -14,6 +14,11 @@ module.exports = {
     ecmaVersion: "latest"
   },
   rules: {
+    "space-before-function-paren": ["error", {
+      anonymous: "always",
+      asyncArrow: "always",
+      named: "never"
+    }],
     "node/no-unpublished-require": [
       "error",
       {

packages/styleguide/lib/code-block.js

@@ -1,17 +1,57 @@
 const {TextEditor} = require('atom')
 
+const PROMISES_BY_SCOPE_NAME = new Map();
+
 module.exports =
 class CodeBlock {
-  constructor (props) {
+  constructor(props) {
     this.editor = new TextEditor({readonly: true, keyboardInputEnabled: false})
     this.element = document.createElement('div')
     this.element.appendChild(this.editor.getElement())
-    atom.grammars.assignLanguageMode(this.editor, props.grammarScopeName)
     this.update(props)
+    this.whenGrammarAdded(props.grammarScopeName)
+      .then(() => {
+        // We don't use the returned grammar here; instead we trigger logic
+        // that matches up the grammars present right now with the user's
+        // stated preferences for language mode (TextMate vs Tree-sitter).
+        //
+        // In other words: “Once any grammar for language X loads, wait another
+        // second, then pick the language X grammar that best fits our needs.”
+        atom.grammars.assignLanguageMode(this.editor, props.grammarScopeName)
+      })
   }
 
-  update ({cssClass, code}) {
+  update({cssClass, code}) {
     this.editor.setText(code)
     this.element.classList.add(cssClass)
   }
+
+  whenGrammarAdded(scopeName) {
+    // Lots of these will fire at once for the same scope name; we want them
+    // all to use the same promise.
+    if (PROMISES_BY_SCOPE_NAME.has(scopeName)) {
+      return PROMISES_BY_SCOPE_NAME.get(scopeName)
+    }
+
+    let grammar = atom.grammars.grammarForId(scopeName);
+    if (grammar) return Promise.resolve(grammar);
+
+    let promise = new Promise(resolve => {
+      let disposable = atom.grammars.onDidAddGrammar(grammar => {
+        if (grammar?.scopeName !== scopeName) return
+        disposable.dispose()
+
+        // If we resolve immediately, we might not get the right grammar for
+        // the user's preferred language mode setting. A short pause will allow
+        // all the grammars of a given language time to activate.
+        //
+        // This is how we balance assigning the grammar for the “wrong”
+        // language mode… versus waiting for another one that may never arrive.
+        setTimeout(resolve(grammar), 1000)
+      })
+    })
+
+    PROMISES_BY_SCOPE_NAME.set(scopeName, promise)
+    return promise
+  }
 }

packages/styleguide/spec/styleguide-spec.js

@@ -1,5 +1,9 @@
 const {it, fit, ffit, beforeEach, afterEach} = require('./async-spec-helpers') // eslint-disable-line no-unused-vars
 
+function wait(ms) {
+  return new Promise(resolve => setTimeout(resolve, ms))
+}
+
 describe('Style Guide', () => {
   beforeEach(async () => {
     await atom.packages.activatePackage('styleguide')
@@ -14,5 +18,18 @@ describe('Style Guide', () => {
     it('opens the style guide', () => {
       expect(styleGuideView.element.textContent).toContain('Styleguide')
     })
+
+    it('assigns a grammar to its editors even if present before the correct grammar is added', async () => {
+      jasmine.useRealClock();
+      await wait(100);
+      let editor = styleGuideView.element.querySelector('atom-text-editor')
+      let te = editor.getModel();
+      expect(te.getGrammar()?.scopeName).toBe('text.plain.null-grammar');
+
+      await atom.packages.activatePackage('language-html')
+      await wait(100);
+
+      expect(te.getGrammar()?.scopeName).toBe('text.html.basic');
+    })
   })
 })

spec/grammar-registry-spec.js

@@ -137,6 +137,22 @@ describe('GrammarRegistry', () => {
         grammarRegistry.grammarForId('source.js') instanceof SecondMate.Grammar
       ).toBe(true);
     });
+
+    it('never returns a stub object before a grammar has loaded', () => {
+      grammarRegistry.addInjectionPoint('source.js', {
+        type: 'some_node_type',
+        language() {
+          return 'some_language_name';
+        },
+        content(node) {
+          return node;
+        }
+      });
+
+      expect(
+        grammarRegistry.grammarForId('source.js')
+      ).toBe(undefined);
+    });
   });
 
   describe('.autoAssignLanguageMode(buffer)', () => {
@@ -878,23 +894,65 @@ describe('GrammarRegistry', () => {
       }
     };
 
+    let addCallbackFired;
+    let updateCallbackFired;
+    let addCallbackDisposable;
+    let updateCallbackDisposable;
+
     beforeEach(() => {
+      addCallbackFired = false;
+      updateCallbackFired = false;
       setConfigForLanguageMode('node-tree-sitter');
     });
 
+    afterEach(() => {
+      addCallbackDisposable?.dispose();
+      updateCallbackDisposable?.dispose();
+    });
+
     it('adds an injection point to the grammar with the given id', async () => {
       await atom.packages.activatePackage('language-javascript');
-      atom.grammars.addInjectionPoint('javascript', injectionPoint);
-      const grammar = atom.grammars.grammarForId('javascript');
-      expect(grammar.injectionPoints).toContain(injectionPoint);
+      atom.grammars.addInjectionPoint('source.js', injectionPoint);
+      const grammar = atom.grammars.grammarForId('source.js');
+      expect(
+        grammar.injectionPointsByType['some_node_type']
+      ).toContain(injectionPoint);
+    });
+
+    it('fires the onDidUpdateGrammar callback', async () => {
+      await atom.packages.activatePackage('language-javascript');
+      let callbackDisposable = atom.grammars.onDidUpdateGrammar((grammar) => {
+        if (grammar.scopeName === 'source.js') {
+          updateCallbackFired = true;
+        }
+      });
+      atom.grammars.addInjectionPoint('source.js', injectionPoint);
+      expect(updateCallbackFired).toBe(true);
     });
 
     describe('when called before a grammar with the given id is loaded', () => {
       it('adds the injection point once the grammar is loaded', async () => {
-        atom.grammars.addInjectionPoint('javascript', injectionPoint);
+        // Adding an injection point before a grammar loads should not trigger
+        // onDidUpdateGrammar at any point.
+        updateCallbackDisposable = atom.grammars.onDidUpdateGrammar((grammar) => {
+          if (!grammar.scopeName) {
+            updateCallbackFired = true;
+          }
+        });
+
+        // But onDidAddGrammar should be triggered when the grammar eventually
+        // loads.
+        addCallbackDisposable = atom.grammars.onDidAddGrammar((grammar) => {
+          if (grammar.scopeName === 'source.js') addCallbackFired = true;
+        });
+        atom.grammars.addInjectionPoint('source.js', injectionPoint);
         await atom.packages.activatePackage('language-javascript');
-        const grammar = atom.grammars.grammarForId('javascript');
-        expect(grammar.injectionPoints).toContain(injectionPoint);
+        const grammar = atom.grammars.grammarForId('source.js');
+        expect(
+          grammar.injectionPointsByType['some_node_type']
+        ).toContain(injectionPoint);
+        expect(updateCallbackFired).toBe(false);
+        expect(addCallbackFired).toBe(true);
       });
     });
   });

src/grammar-registry.js

@@ -2,7 +2,7 @@ const _ = require('underscore-plus');
 const Grim = require('grim');
 const CSON = require('season');
 const SecondMate = require('second-mate');
-const { Disposable, CompositeDisposable } = require('event-kit');
+const { Disposable, CompositeDisposable, Emitter } = require('event-kit');
 const TextMateLanguageMode = require('./text-mate-language-mode');
 const NodeTreeSitterLanguageMode = require('./tree-sitter-language-mode');
 const WASMTreeSitterLanguageMode = require('./wasm-tree-sitter-language-mode');
@@ -26,6 +26,7 @@ module.exports = class GrammarRegistry {
       maxTokensPerLine: 100,
       maxLineLength: 1000
     });
+    this.emitter = new Emitter();
     this.clear();
   }
 
@@ -137,7 +138,7 @@ module.exports = class GrammarRegistry {
     let grammar = null;
     if (languageId != null) {
       grammar = this.grammarForId(languageId);
-      if (!grammar) return false;
+      if (!grammar || !grammar.scopeName) return false;
       this.languageOverridesByBufferId.set(buffer.id, languageId);
     } else {
       this.languageOverridesByBufferId.set(buffer.id, null);
@@ -248,6 +249,9 @@ module.exports = class GrammarRegistry {
   }
 
   getLanguageParserForScope(scope) {
+    if (typeof scope === 'string') {
+      scope = new ScopeDescriptor({ scopes: [scope] })
+    }
     let useTreeSitterParsers = this.config.get('core.useTreeSitterParsers', { scope });
     let useExperimentalModernTreeSitter = this.config.get('core.useExperimentalModernTreeSitter', { scope });
 
@@ -401,14 +405,28 @@ module.exports = class GrammarRegistry {
       new ScopeDescriptor({ scopes: [languageId] })
     );
 
+    let getTreeSitterGrammar = (table, languageId) => {
+      let grammar = table[languageId];
+      if (grammar?.scopeName) {
+        return grammar;
+      }
+      return null;
+    };
+
     if (config === 'wasm-tree-sitter') {
       return (
-        this.wasmTreeSitterGrammarsById[languageId] ||
+        getTreeSitterGrammar(
+          this.wasmTreeSitterGrammarsById,
+          languageId
+        ) ||
         this.textmateRegistry.grammarForScopeName(languageId)
       );
     } else if (config === 'node-tree-sitter') {
       return (
-        this.treeSitterGrammarsById[languageId] ||
+        getTreeSitterGrammar(
+          this.treeSitterGrammarsById,
+          languageId
+        ) ||
         this.textmateRegistry.grammarForScopeName(languageId)
       );
     } else {
@@ -502,7 +520,12 @@ module.exports = class GrammarRegistry {
   //
   // Returns a {Disposable} on which `.dispose()` can be called to unsubscribe.
   onDidAddGrammar(callback) {
-    return this.textmateRegistry.onDidAddGrammar(callback);
+    let disposable = new CompositeDisposable();
+    disposable.add(
+      this.textmateRegistry.onDidAddGrammar(callback),
+      this.emitter.on('did-add-grammar', callback)
+    );
+    return disposable;
   }
 
   // Extended: Invoke the given callback when a grammar is updated due to a grammar
@@ -513,7 +536,12 @@ module.exports = class GrammarRegistry {
   //
   // Returns a {Disposable} on which `.dispose()` can be called to unsubscribe.
   onDidUpdateGrammar(callback) {
-    return this.textmateRegistry.onDidUpdateGrammar(callback);
+    let disposable = new CompositeDisposable();
+    disposable.add(
+      this.textmateRegistry.onDidUpdateGrammar(callback),
+      this.emitter.on('did-update-grammar', callback)
+    );
+    return disposable;
   }
 
   // Experimental: Specify a type of syntax node that may embed other languages.
@@ -557,10 +585,16 @@ module.exports = class GrammarRegistry {
       if (grammar) {
         if (grammar.addInjectionPoint) {
           grammar.addInjectionPoint(injectionPoint);
+
+          // This is a grammar that's already loaded — not just a stub. Editors
+          // that already use this grammar will want to know that we added an
+          // injection.
+          this.emitter.emit('did-update-grammar', grammar);
         } else {
           grammar.injectionPoints.push(injectionPoint);
         }
         grammarsToDispose.push(grammar);
+
       } else {
         table[grammarId] = { injectionPoints: [injectionPoint] }
       }
@@ -616,6 +650,7 @@ module.exports = class GrammarRegistry {
         }
       }
       this.grammarAddedOrUpdated(grammar);
+      this.emitter.emit('did-add-grammar', grammar);
       return new Disposable(() => this.removeGrammar(grammar));
     } else if (grammar instanceof TreeSitterGrammar) {
       const existingParams =
@@ -628,6 +663,7 @@ module.exports = class GrammarRegistry {
         }
       }
       this.grammarAddedOrUpdated(grammar);
+      this.emitter.emit('did-add-grammar', grammar);
       return new Disposable(() => this.removeGrammar(grammar));
     } else {
       return this.textmateRegistry.addGrammar(grammar);