Recalculate injections when registry changes

[Ingramz]

https://github.com/atom/language-php/pull/330 revealed that when a grammar is loaded, the injections include only grammars which have been loaded to registry prior to the grammar under question.

This pull request changes the grammar object to re-initialize the injections when the registry add/remove/update events are triggered.

[Ingramz]

cc @50Wliu

[Ingramz]

There needs to be some code which disposes the recalculating events if a grammar is removed from the registry, else they will outlive the grammar itself.

[50Wliu]

@Ingramz what do you think about the following diff? It includes some minor refactoring that makes the intent of grammarUpdated more explicit, and removes the need to dispose of subscriptions.

diff --git a/src/grammar.coffee b/src/grammar.coffee
index 6224ef0..284e07a 100644
--- a/src/grammar.coffee
+++ b/src/grammar.coffee
@@ -27,9 +27,6 @@ class Grammar
     @repository = null
     @initialRule = null

-    @rawPatterns = patterns
-    @rawRepository = repository
-
     if injectionSelector?
       @injectionSelector = new ScopeSelector(injectionSelector)
     else
@@ -43,11 +40,11 @@ class Grammar
     @fileTypes ?= []
     @includedGrammarScopes = []

-    if injections
-      recalculate = => @injections = new Injections(this, injections)
-      @registry.onDidAddGrammar recalculate
-      @registry.onDidUpdateGrammar recalculate
-      @registry.onDidRemoveGrammar recalculate
+    @rawPatterns = patterns
+    @rawRepository = repository
+    @rawInjections = injections
+
+    @updateRules()

   ###
   Section: Event Subscription
@@ -124,11 +121,10 @@ class Grammar
           openScopeTags.push(@registry.startIdForScope(contentScopeName)) if contentScopeName
     else
       openScopeTags = [] if compatibilityMode
-      initialRule = @getInitialRule()
-      {scopeName, contentScopeName} = initialRule
-      ruleStack = [{rule: initialRule, scopeName, contentScopeName}]
-      tags.push(@startIdForScope(initialRule.scopeName)) if scopeName
-      tags.push(@startIdForScope(initialRule.contentScopeName)) if contentScopeName
+      {scopeName, contentScopeName} = @initialRule
+      ruleStack = [{rule: @initialRule, scopeName, contentScopeName}]
+      tags.push(@startIdForScope(@initialRule.scopeName)) if scopeName
+      tags.push(@startIdForScope(@initialRule.contentScopeName)) if contentScopeName

     initialRuleStackLength = ruleStack.length
     position = 0
@@ -218,27 +214,28 @@ class Grammar
     @registration?.dispose()
     @registration = null

-  clearRules: ->
-    @initialRule = null
-    @repository = null
+  updateRules: ->
+    @initialRule = @createRule({@scopeName, patterns: @rawPatterns})
+    @repository = @createRepository()
+    @injections = new Injections(this, @rawInjections)
+
+  getInitialRule: -> @initialRule

-  getInitialRule: ->
-    @initialRule ?= @createRule({@scopeName, patterns: @rawPatterns})
+  getRepository: -> @repository

-  getRepository: ->
-    @repository ?= do =>
-      repository = {}
-      for name, data of @rawRepository
-        data = {patterns: [data], tempName: name} if data.begin? or data.match?
-        repository[name] = @createRule(data)
-      repository
+  createRepository: ->
+    repository = {}
+    for name, data of @rawRepository
+      data = {patterns: [data], tempName: name} if data.begin? or data.match?
+      repository[name] = @createRule(data)
+    repository

   addIncludedGrammarScope: (scope) ->
     @includedGrammarScopes.push(scope) unless _.include(@includedGrammarScopes, scope)

   grammarUpdated: (scopeName) ->
     return false unless _.include(@includedGrammarScopes, scopeName)
-    @clearRules()
+    @updateRules()
     @registry.grammarUpdated(@scopeName)
     @emit 'grammar-updated' if Grim.includeDeprecatedAPIs
     @emitter.emit 'did-update'

[50Wliu]

...also, shouldn't this line use @grammar instead? Most specs fail without that change. Coffeescript upgrade, perhaps?

(Answer: yes, it should. I had fixed it already in #79 before it was reverted in #86. Will create a new PR for that tomorrow.)

[50Wliu]

Discussed with @Ingramz on Slack and we're ok with this solution.