Merge pull request #4937 from atom/ns-clean-up-package-manager

Address issues with PackageManager

spec/text-editor-element-spec.coffee

@@ -107,7 +107,7 @@ describe "TextEditorElement", ->
 
       spyOn(atom.themes, 'isInitialLoadComplete').andCallFake ->
         initialThemeLoadComplete
-      spyOn(atom.themes, 'onDidReloadAll').andCallFake (fn) ->
+      spyOn(atom.themes, 'onDidChangeActiveThemes').andCallFake (fn) ->
         themeReloadCallback = fn
 
       atom.config.set("editor.useShadowDOM", false)

spec/theme-manager-spec.coffee

@@ -73,54 +73,54 @@ describe "ThemeManager", ->
 
   describe "when the core.themes config value changes", ->
     it "add/removes stylesheets to reflect the new config value", ->
-      themeManager.onDidReloadAll reloadHandler = jasmine.createSpy()
+      themeManager.onDidChangeActiveThemes didChangeActiveThemesHandler = jasmine.createSpy()
       spyOn(atom.styles, 'getUserStyleSheetPath').andCallFake -> null
 
       waitsForPromise ->
         themeManager.activateThemes()
 
       runs ->
-        reloadHandler.reset()
+        didChangeActiveThemesHandler.reset()
         atom.config.set('core.themes', [])
 
       waitsFor ->
-        reloadHandler.callCount == 1
+        didChangeActiveThemesHandler.callCount == 1
 
       runs ->
-        reloadHandler.reset()
+        didChangeActiveThemesHandler.reset()
         expect($('style.theme')).toHaveLength 0
         atom.config.set('core.themes', ['atom-dark-ui'])
 
       waitsFor ->
-        reloadHandler.callCount == 1
+        didChangeActiveThemesHandler.callCount == 1
 
       runs ->
-        reloadHandler.reset()
+        didChangeActiveThemesHandler.reset()
         expect($('style[priority=1]')).toHaveLength 2
         expect($('style[priority=1]:eq(0)').attr('source-path')).toMatch /atom-dark-ui/
         atom.config.set('core.themes', ['atom-light-ui', 'atom-dark-ui'])
 
       waitsFor ->
-        reloadHandler.callCount == 1
+        didChangeActiveThemesHandler.callCount == 1
 
       runs ->
-        reloadHandler.reset()
+        didChangeActiveThemesHandler.reset()
         expect($('style[priority=1]')).toHaveLength 2
         expect($('style[priority=1]:eq(0)').attr('source-path')).toMatch /atom-dark-ui/
         expect($('style[priority=1]:eq(1)').attr('source-path')).toMatch /atom-light-ui/
         atom.config.set('core.themes', [])
 
       waitsFor ->
-        reloadHandler.callCount == 1
+        didChangeActiveThemesHandler.callCount == 1
 
       runs ->
-        reloadHandler.reset()
+        didChangeActiveThemesHandler.reset()
         expect($('style[priority=1]')).toHaveLength 2
         # atom-dark-ui has an directory path, the syntax one doesn't
         atom.config.set('core.themes', ['theme-with-index-less', 'atom-dark-ui'])
 
       waitsFor ->
-        reloadHandler.callCount == 1
+        didChangeActiveThemesHandler.callCount == 1
 
       runs ->
         expect($('style[priority=1]')).toHaveLength 2
@@ -130,7 +130,7 @@ describe "ThemeManager", ->
 
     it 'adds theme-* classes to the workspace for each active theme', ->
       workspaceElement = atom.views.getView(atom.workspace)
-      themeManager.onDidReloadAll reloadHandler = jasmine.createSpy()
+      themeManager.onDidChangeActiveThemes didChangeActiveThemesHandler = jasmine.createSpy()
 
       waitsForPromise ->
         themeManager.activateThemes()
@@ -138,11 +138,11 @@ describe "ThemeManager", ->
       runs ->
         expect(workspaceElement).toHaveClass 'theme-atom-dark-ui'
 
-        themeManager.onDidReloadAll reloadHandler = jasmine.createSpy()
+        themeManager.onDidChangeActiveThemes didChangeActiveThemesHandler = jasmine.createSpy()
         atom.config.set('core.themes', ['theme-with-ui-variables', 'theme-with-syntax-variables'])
 
       waitsFor ->
-        reloadHandler.callCount > 0
+        didChangeActiveThemesHandler.callCount > 0
 
       runs ->
         # `theme-` twice as it prefixes the name with `theme-`
@@ -259,11 +259,11 @@ describe "ThemeManager", ->
         themeManager.activateThemes()
 
     it "loads the correct values from the theme's ui-variables file", ->
-      themeManager.onDidReloadAll reloadHandler = jasmine.createSpy()
+      themeManager.onDidChangeActiveThemes didChangeActiveThemesHandler = jasmine.createSpy()
       atom.config.set('core.themes', ['theme-with-ui-variables', 'theme-with-syntax-variables'])
 
       waitsFor ->
-        reloadHandler.callCount > 0
+        didChangeActiveThemesHandler.callCount > 0
 
       runs ->
         # an override loaded in the base css
@@ -276,11 +276,11 @@ describe "ThemeManager", ->
 
     describe "when there is a theme with incomplete variables", ->
       it "loads the correct values from the fallback ui-variables", ->
-        themeManager.onDidReloadAll reloadHandler = jasmine.createSpy()
+        themeManager.onDidChangeActiveThemes didChangeActiveThemesHandler = jasmine.createSpy()
         atom.config.set('core.themes', ['theme-with-incomplete-ui-variables', 'theme-with-syntax-variables'])
 
         waitsFor ->
-          reloadHandler.callCount > 0
+          didChangeActiveThemesHandler.callCount > 0
 
         runs ->
           # an override loaded in the base css

src/atom.coffee

@@ -280,7 +280,7 @@ class Atom extends Model
       deprecate "The atom.syntax global is deprecated. Use atom.grammars instead."
       @grammars
 
-    @subscribe @packages.onDidActivateAll => @watchThemes()
+    @subscribe @packages.onDidActivateInitialPackages => @watchThemes()
 
     Project = require './project'
     TextBuffer = require 'text-buffer'
@@ -721,7 +721,7 @@ class Atom extends Model
     @themes.load()
 
   watchThemes: ->
-    @themes.onDidReloadAll =>
+    @themes.onDidChangeActiveThemes =>
       # Only reload stylesheets from non-theme packages
       for pack in @packages.getActivePackages() when pack.getType() isnt 'theme'
         pack.reloadStylesheets?()

src/menu-manager.coffee

@@ -61,7 +61,7 @@ class MenuManager
     @pendingUpdateOperation = null
     @template = []
     atom.keymaps.onDidLoadBundledKeymaps => @loadPlatformItems()
-    atom.packages.onDidActivateAll => @sortPackagesMenu()
+    atom.packages.onDidActivateInitialPackages => @sortPackagesMenu()
 
   # Public: Adds the given items to the application menu.
   #

src/package-manager.coffee

@@ -5,7 +5,7 @@ EmitterMixin = require('emissary').Emitter
 {Emitter} = require 'event-kit'
 fs = require 'fs-plus'
 Q = require 'q'
-{deprecate} = require 'grim'
+Grim = require 'grim'
 
 Package = require './package'
 ThemePackage = require './theme-package'
@@ -53,25 +53,71 @@ class PackageManager
   # * `callback` {Function}
   #
   # Returns a {Disposable} on which `.dispose()` can be called to unsubscribe.
+  onDidLoadInitialPackages: (callback) ->
+    @emitter.on 'did-load-initial-packages', callback
+    @emitter.on 'did-load-all', callback # TODO: Remove once deprecated pre-1.0 APIs are gone
+
   onDidLoadAll: (callback) ->
-    @emitter.on 'did-load-all', callback
+    Grim.deprecate("Use `::onDidLoadInitialPackages` instead.")
+    @onDidLoadInitialPackages(callback)
 
   # Public: Invoke the given callback when all packages have been activated.
   #
   # * `callback` {Function}
   #
   # Returns a {Disposable} on which `.dispose()` can be called to unsubscribe.
+  onDidActivateInitialPackages: (callback) ->
+    @emitter.on 'did-activate-initial-packages', callback
+    @emitter.on 'did-activate-all', callback # TODO: Remove once deprecated pre-1.0 APIs are gone
+
   onDidActivateAll: (callback) ->
-    @emitter.on 'did-activate-all', callback
+    Grim.deprecate("Use `::onDidActivateInitialPackages` instead.")
+    @onDidActivateInitialPackages(callback)
+
+  # Public: Invoke the given callback when a package is activated.
+  #
+  # * `callback` A {Function} to be invoked when a package is activated.
+  #   * `package` The {Package} that was activated.
+  #
+  # Returns a {Disposable} on which `.dispose()` can be called to unsubscribe.
+  onDidActivatePackage: (callback) ->
+    @emitter.on 'did-activate-package', callback
+
+  # Public: Invoke the given callback when a package is deactivated.
+  #
+  # * `callback` A {Function} to be invoked when a package is deactivated.
+  #   * `package` The {Package} that was deactivated.
+  #
+  # Returns a {Disposable} on which `.dispose()` can be called to unsubscribe.
+  onDidDeactivatePackage: (callback) ->
+    @emitter.on 'did-deactivate-package', callback
+
+  # Public: Invoke the given callback when a package is loaded.
+  #
+  # * `callback` A {Function} to be invoked when a package is loaded.
+  #   * `package` The {Package} that was loaded.
+  #
+  # Returns a {Disposable} on which `.dispose()` can be called to unsubscribe.
+  onDidLoadPackage: (callback) ->
+    @emitter.on 'did-load-package', callback
+
+  # Public: Invoke the given callback when a package is unloaded.
+  #
+  # * `callback` A {Function} to be invoked when a package is unloaded.
+  #   * `package` The {Package} that was unloaded.
+  #
+  # Returns a {Disposable} on which `.dispose()` can be called to unsubscribe.
+  onDidUnloadPackage: (callback) ->
+    @emitter.on 'did-unload-package', callback
 
   on: (eventName) ->
     switch eventName
       when 'loaded'
-        deprecate 'Use PackageManager::onDidLoadAll instead'
+        Grim.deprecate 'Use PackageManager::onDidLoadInitialPackages instead'
       when 'activated'
-        deprecate 'Use PackageManager::onDidActivateAll instead'
+        Grim.deprecate 'Use PackageManager::onDidActivateInitialPackages instead'
       else
-        deprecate 'PackageManager::on is deprecated. Use event subscription methods instead.'
+        Grim.deprecate 'PackageManager::on is deprecated. Use event subscription methods instead.'
     EmitterMixin::on.apply(this, arguments)
 
   ###
@@ -278,7 +324,7 @@ class PackageManager
     packagePaths = _.uniq packagePaths, (packagePath) -> path.basename(packagePath)
     @loadPackage(packagePath) for packagePath in packagePaths
     @emit 'loaded'
-    @emitter.emit 'did-load-all'
+    @emitter.emit 'did-load-initial-packages'
 
   loadPackage: (nameOrPath) ->
     return pack if pack = @getLoadedPackage(nameOrPath)
@@ -295,6 +341,7 @@ class PackageManager
           pack = new Package(packagePath, metadata)
         pack.load()
         @loadedPackages[pack.name] = pack
+        @emitter.emit 'did-load-package', pack
         return pack
       catch error
         console.warn "Failed to load package.json '#{path.basename(packagePath)}'", error.stack ? error
@@ -312,16 +359,19 @@ class PackageManager
 
     if pack = @getLoadedPackage(name)
       delete @loadedPackages[pack.name]
+      @emitter.emit 'did-unload-package', pack
     else
       throw new Error("No loaded package for name '#{name}'")
 
   # Activate all the packages that should be activated.
   activate: ->
+    promises = []
     for [activator, types] in @packageActivators
       packages = @getLoadedPackagesForTypes(types)
-      activator.activatePackages(packages)
-    @emit 'activated'
-    @emitter.emit 'did-activate-all'
+      promises = promises.concat(activator.activatePackages(packages))
+    Q.all(promises).then =>
+      @emit 'activated'
+      @emitter.emit 'did-activate-initial-packages'
 
   # another type of package manager can handle other package types.
   # See ThemeManager
@@ -329,9 +379,13 @@ class PackageManager
     @packageActivators.push([activator, types])
 
   activatePackages: (packages) ->
+    promises = []
     atom.config.transact =>
-      @activatePackage(pack.name) for pack in packages
+      for pack in packages
+        promise = @activatePackage(pack.name)
+        promises.push(promise) unless pack.hasActivationCommands()
     @observeDisabledPackages()
+    promises
 
   # Activate a single package by name
   activatePackage: (name) ->
@@ -340,6 +394,7 @@ class PackageManager
     else if pack = @loadPackage(name)
       pack.activate().then =>
         @activePackages[pack.name] = pack
+        @emitter.emit 'did-activate-package', pack
         pack
     else
       Q.reject(new Error("Failed to load package '#{name}'"))
@@ -357,3 +412,4 @@ class PackageManager
       @setPackageState(pack.name, state) if state = pack.serialize?()
     pack.deactivate()
     delete @activePackages[pack.name]
+    @emitter.emit 'did-deactivate-package', pack

src/text-editor-component.coffee

@@ -185,7 +185,7 @@ TextEditorComponent = React.createClass
     @subscribe stylesElement.onDidUpdateStyleElement @onStylesheetsChanged
     @subscribe stylesElement.onDidRemoveStyleElement @onStylesheetsChanged
     unless atom.themes.isInitialLoadComplete()
-      @subscribe atom.themes.onDidReloadAll @onAllThemesLoaded
+      @subscribe atom.themes.onDidChangeActiveThemes @onAllThemesLoaded
     @subscribe scrollbarStyle.changes, @refreshScrollbars
 
     @domPollingIntervalId = setInterval(@pollDOM, @domPollingInterval)

src/theme-manager.coffee

@@ -61,8 +61,13 @@ class ThemeManager
   # updating the list of active themes have completed.
   #
   # * `callback` {Function}
+  onDidChangeActiveThemes: (callback) ->
+    @emitter.on 'did-change-active-themes', callback
+    @emitter.on 'did-reload-all', callback # TODO: Remove once deprecated pre-1.0 APIs are gone
+
   onDidReloadAll: (callback) ->
-    @emitter.on 'did-reload-all', callback
+    Grim.deprecate("Use `::onDidChangeActiveThemes` instead.")
+    @onDidChangeActiveThemes(callback)
 
   # Deprecated: Invoke `callback` when a stylesheet has been added to the dom.
   #
@@ -106,7 +111,7 @@ class ThemeManager
   on: (eventName) ->
     switch eventName
       when 'reloaded'
-        Grim.deprecate 'Use ThemeManager::onDidReloadAll instead'
+        Grim.deprecate 'Use ThemeManager::onDidChangeActiveThemes instead'
       when 'stylesheet-added'
         Grim.deprecate 'Use ThemeManager::onDidAddStylesheet instead'
       when 'stylesheet-removed'
@@ -132,9 +137,13 @@ class ThemeManager
   ###
 
   # Public: Get an array of all the loaded theme names.
-  getLoadedNames: ->
+  getLoadedThemeNames: ->
     theme.name for theme in @getLoadedThemes()
 
+  getLoadedNames: ->
+    Grim.deprecate("Use `::getLoadedThemeNames` instead.")
+    @getLoadedThemeNames()
+
   # Public: Get an array of all the loaded themes.
   getLoadedThemes: ->
     pack for pack in @packageManager.getLoadedPackages() when pack.isTheme()
@@ -144,9 +153,13 @@ class ThemeManager
   ###
 
   # Public: Get an array of all the active theme names.
-  getActiveNames: ->
+  getActiveThemeNames: ->
     theme.name for theme in @getActiveThemes()
 
+  getActiveNames: ->
+    Grim.deprecate("Use `::getActiveThemeNames` instead.")
+    @getActiveThemeNames()
+
   # Public: Get an array of all the active themes.
   getActiveThemes: ->
     pack for pack in @packageManager.getActivePackages() when pack.isTheme()
@@ -195,10 +208,11 @@ class ThemeManager
     # the first/top theme to override later themes in the stack.
     themeNames.reverse()
 
-  # Public: Set the list of enabled themes.
+  # Set the list of enabled themes.
   #
   # * `enabledThemeNames` An {Array} of {String} theme names.
   setEnabledThemes: (enabledThemeNames) ->
+    Grim.deprecate("Use `atom.config.set('core.themes', arrayOfThemeNames)` instead")
     atom.config.set('core.themes', enabledThemeNames)
 
   ###
@@ -322,7 +336,7 @@ class ThemeManager
         @reloadBaseStylesheets()
         @initialLoadComplete = true
         @emit 'reloaded'
-        @emitter.emit 'did-reload-all'
+        @emitter.emit 'did-change-active-themes'
         deferred.resolve()
 
     deferred.promise