From f5774972e999b54e25a5dde6a439cfe42dc97b4f Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Mon, 25 Mar 2013 10:57:58 -0600 Subject: [PATCH] Add `atom.deactivatePackage(id)` It serializes the package state to the atom.packageStates hash when the package is deactivated, which means we will be able to test package serialization independent of the overall window lifecycle by just deactivating and re-activating the package. --- spec/app/atom-spec.coffee | 150 ++++++++---------- .../packages/package-with-module/index.coffee | 4 +- spec/spec-helper.coffee | 2 +- src/app/atom-package.coffee | 24 ++- src/app/atom.coffee | 41 ++--- src/app/root-view.coffee | 4 +- src/app/text-mate-package.coffee | 3 + .../tree-view/spec/tree-view-spec.coffee | 1 + 8 files changed, 119 insertions(+), 110 deletions(-) diff --git a/spec/app/atom-spec.coffee b/spec/app/atom-spec.coffee index b83b47fc4..eec7d718c 100644 --- a/spec/app/atom-spec.coffee +++ b/spec/app/atom-spec.coffee @@ -6,116 +6,100 @@ describe "the `atom` global", -> beforeEach -> window.rootView = new RootView - describe "when a package is built and loaded", -> - [extension, stylesheetPath] = [] + describe "package lifecycle methods", -> + packageModule = null beforeEach -> - extension = require "package-with-module" - stylesheetPath = fs.resolveOnLoadPath("fixtures/packages/package-with-module/stylesheets/styles.css") + packageModule = require "package-with-module" afterEach -> - removeStylesheet(stylesheetPath) + atom.deactivatePackages() - it "requires and activates the package's main module if it exists", -> - spyOn(atom, 'activateAtomPackage').andCallThrough() - atom.activatePackage("package-with-module") - expect(atom.activateAtomPackage).toHaveBeenCalled() + describe ".activatePackage(id)", -> + stylesheetPath = null - it "logs warning instead of throwing an exception if a package fails to load", -> - config.set("core.disabledPackages", []) - spyOn(console, "warn") - expect(-> atom.activatePackage("package-that-throws-an-exception")).not.toThrow() - expect(console.warn).toHaveBeenCalled() + beforeEach -> + stylesheetPath = fs.resolveOnLoadPath("fixtures/packages/package-with-module/stylesheets/styles.css") - describe "keymap loading", -> - describe "when package.json does not contain a 'keymaps' manifest", -> - it "loads all the .cson/.json files in the keymaps directory", -> - element1 = $$ -> @div class: 'test-1' - element2 = $$ -> @div class: 'test-2' - element3 = $$ -> @div class: 'test-3' + afterEach -> + removeStylesheet(stylesheetPath) - expect(keymap.bindingsForElement(element1)['ctrl-z']).toBeUndefined() - expect(keymap.bindingsForElement(element2)['ctrl-z']).toBeUndefined() - expect(keymap.bindingsForElement(element3)['ctrl-z']).toBeUndefined() + it "requires and activates the package's main module if it exists", -> + spyOn(packageModule, 'activate').andCallThrough() + atom.activatePackage("package-with-module") + expect(packageModule.activate).toHaveBeenCalledWith({}) - atom.activatePackage("package-with-module") + it "passes the package its previously serialized state if it exists", -> + pack = atom.activatePackage("package-with-module") + expect(pack.mainModule.someNumber).not.toBe 77 + pack.mainModule.someNumber = 77 + atom.deactivatePackage("package-with-module") - expect(keymap.bindingsForElement(element1)['ctrl-z']).toBe "test-1" - expect(keymap.bindingsForElement(element2)['ctrl-z']).toBe "test-2" - expect(keymap.bindingsForElement(element3)['ctrl-z']).toBeUndefined() + pack.requireMainModule() # deactivating the package nukes its main module, so we require it again to spy on it + spyOn(pack.mainModule, 'activate').andCallThrough() - describe "when package.json contains a 'keymaps' manifest", -> - it "loads only the keymaps specified by the manifest, in the specified order", -> - element1 = $$ -> @div class: 'test-1' - element3 = $$ -> @div class: 'test-3' + atom.activatePackage("package-with-module") + expect(pack.mainModule.activate).toHaveBeenCalledWith({someNumber: 77}) - expect(keymap.bindingsForElement(element1)['ctrl-z']).toBeUndefined() + it "logs warning instead of throwing an exception if a package fails to load", -> + config.set("core.disabledPackages", []) + spyOn(console, "warn") + expect(-> atom.activatePackage("package-that-throws-an-exception")).not.toThrow() + expect(console.warn).toHaveBeenCalled() - atom.activatePackage("package-with-keymaps-manifest") + describe "keymap loading", -> + describe "when package.json does not contain a 'keymaps' manifest", -> + it "loads all the .cson/.json files in the keymaps directory", -> + element1 = $$ -> @div class: 'test-1' + element2 = $$ -> @div class: 'test-2' + element3 = $$ -> @div class: 'test-3' - expect(keymap.bindingsForElement(element1)['ctrl-z']).toBe 'keymap-1' - expect(keymap.bindingsForElement(element1)['ctrl-n']).toBe 'keymap-2' - expect(keymap.bindingsForElement(element3)['ctrl-y']).toBeUndefined() + expect(keymap.bindingsForElement(element1)['ctrl-z']).toBeUndefined() + expect(keymap.bindingsForElement(element2)['ctrl-z']).toBeUndefined() + expect(keymap.bindingsForElement(element3)['ctrl-z']).toBeUndefined() - it "loads stylesheets associated with the package", -> - stylesheetPath = fs.resolveOnLoadPath("fixtures/packages/package-with-module/stylesheets/styles.css") - expect(stylesheetElementForId(stylesheetPath).length).toBe 0 - atom.activatePackage("package-with-module") - expect(stylesheetElementForId(stylesheetPath).length).toBe 1 + atom.activatePackage("package-with-module") - describe "package lifecycle", -> - describe "activation", -> - it "calls activate on the package main with its previous state", -> - pack = atom.activatePackage('package-with-module') - spyOn(pack.mainModule, 'activate') + expect(keymap.bindingsForElement(element1)['ctrl-z']).toBe "test-1" + expect(keymap.bindingsForElement(element2)['ctrl-z']).toBe "test-2" + expect(keymap.bindingsForElement(element3)['ctrl-z']).toBeUndefined() - serializedState = rootView.serialize() - rootView.deactivate() + describe "when package.json contains a 'keymaps' manifest", -> + it "loads only the keymaps specified by the manifest, in the specified order", -> + element1 = $$ -> @div class: 'test-1' + element3 = $$ -> @div class: 'test-3' + expect(keymap.bindingsForElement(element1)['ctrl-z']).toBeUndefined() - RootView.deserialize(serializedState) - atom.activatePackage('package-with-module') + atom.activatePackage("package-with-keymaps-manifest") - expect(pack.mainModule.activate).toHaveBeenCalledWith(someNumber: 1) + expect(keymap.bindingsForElement(element1)['ctrl-z']).toBe 'keymap-1' + expect(keymap.bindingsForElement(element1)['ctrl-n']).toBe 'keymap-2' + expect(keymap.bindingsForElement(element3)['ctrl-y']).toBeUndefined() - describe "deactivation", -> - it "deactivates and removes the package module from the package module map", -> - pack = atom.activatePackage('package-with-module') - expect(atom.activatedAtomPackages.length).toBe 1 - spyOn(pack.mainModule, "deactivate").andCallThrough() - atom.deactivateAtomPackages() + it "loads stylesheets associated with the package", -> + stylesheetPath = fs.resolveOnLoadPath("fixtures/packages/package-with-module/stylesheets/styles.css") + expect(stylesheetElementForId(stylesheetPath).length).toBe 0 + atom.activatePackage("package-with-module") + expect(stylesheetElementForId(stylesheetPath).length).toBe 1 + + describe ".deactivatePackage(id)", -> + it "calls `deactivate` on the package's main module and deletes the package's module reference and require cache entry", -> + pack = atom.activatePackage("package-with-module") + expect(atom.getActivePackage("package-with-module")).toBe pack + spyOn(pack.mainModule, 'deactivate').andCallThrough() + + atom.deactivatePackage("package-with-module") expect(pack.mainModule.deactivate).toHaveBeenCalled() - expect(atom.activatedAtomPackages.length).toBe 0 - - describe "serialization", -> - it "uses previous serialization state on packages whose activation has been deferred", -> - atom.atomPackageStates['package-with-activation-events'] = {previousData: 'exists'} - unactivatedPackage = atom.activatePackage('package-with-activation-events') - activatedPackage = atom.activatePackage('package-with-module') - - expect(atom.serializeAtomPackages()).toEqual - 'package-with-module': - 'someNumber': 1 - 'package-with-activation-events': - 'previousData': 'exists' - - # ensure serialization occurs when the packageis activated - unactivatedPackage.deferActivation = false - unactivatedPackage.activate() - expect(atom.serializeAtomPackages()).toEqual - 'package-with-module': - 'someNumber': 1 - 'package-with-activation-events': - 'previousData': 'overwritten' + expect(atom.getActivePackage("package-with-module")).toBeUndefined() it "absorbs exceptions that are thrown by the package module's serialize methods", -> spyOn(console, 'error') atom.activatePackage('package-with-module', immediate: true) atom.activatePackage('package-with-serialize-error', immediate: true) - - packageStates = atom.serializeAtomPackages() - expect(packageStates['package-with-module']).toEqual someNumber: 1 - expect(packageStates['package-with-serialize-error']).toBeUndefined() + atom.deactivatePackages() + expect(atom.packageStates['package-with-module']).toEqual someNumber: 1 + expect(atom.packageStates['package-with-serialize-error']).toBeUndefined() expect(console.error).toHaveBeenCalled() describe ".getVersion(callback)", -> diff --git a/spec/fixtures/packages/package-with-module/index.coffee b/spec/fixtures/packages/package-with-module/index.coffee index 84d5c12a1..3872da074 100644 --- a/spec/fixtures/packages/package-with-module/index.coffee +++ b/spec/fixtures/packages/package-with-module/index.coffee @@ -4,8 +4,8 @@ module.exports = someNumber: 0 - activate: -> - @someNumber = 1 + activate: ({@someNumber}) -> + @someNumber ?= 1 deactivate: -> diff --git a/spec/spec-helper.coffee b/spec/spec-helper.coffee index ce33c0121..00afecf1a 100644 --- a/spec/spec-helper.coffee +++ b/spec/spec-helper.coffee @@ -36,7 +36,7 @@ beforeEach -> window.git = Git.open(window.project.getPath()) window.resetTimeouts() - atom.atomPackageStates = {} + atom.packageStates = {} atom.loadedPackages = [] spyOn(atom, 'saveWindowState') spyOn(atom, 'getSavedWindowState').andReturn(null) diff --git a/src/app/atom-package.coffee b/src/app/atom-package.coffee index cf241b6c5..61c9c3879 100644 --- a/src/app/atom-package.coffee +++ b/src/app/atom-package.coffee @@ -11,6 +11,8 @@ class AtomPackage extends Package keymaps: null stylesheets: null grammars: null + mainModulePath: null + resolvedMainModulePath: false mainModule: null deferActivation: false @@ -79,19 +81,33 @@ class AtomPackage extends Package try if @requireMainModule() config.setDefaults(@name, @mainModule.configDefaults) - atom.activateAtomPackage(this) + @mainModule.activate(atom.getPackageState(@name) ? {}) catch e console.warn "Failed to activate package named '#{@name}'", e.stack + serialize: -> + try + @mainModule?.serialize?() + catch e + console.error "Error serializing package '#{@name}'", e.stack + + deactivate: -> + @mainModule?.deactivate?() + requireMainModule: -> return @mainModule if @mainModule - mainPath = + mainModulePath = @getMainModulePath() + @mainModule = require(mainModulePath) if fs.isFile(mainModulePath) + + getMainModulePath: -> + return @mainModulePath if @resolvedMainModulePath + @resolvedMainModulePath = true + mainModulePath = if @metadata.main fs.join(@path, @metadata.main) else fs.join(@path, 'index') - mainPath = fs.resolveExtension(mainPath, ["", _.keys(require.extensions)...]) - @mainModule = require(mainPath) if fs.isFile(mainPath) + @mainModulePath = fs.resolveExtension(mainModulePath, ["", _.keys(require.extensions)...]) registerDeferredDeserializers: -> for deserializerName in @metadata.deferredDeserializers ? [] diff --git a/src/app/atom.coffee b/src/app/atom.coffee index 54d3b5991..809210d93 100644 --- a/src/app/atom.coffee +++ b/src/app/atom.coffee @@ -14,33 +14,23 @@ _.extend atom, pendingBrowserProcessCallbacks: {} loadedPackages: [] activePackages: [] - activatedAtomPackages: [] - atomPackageStates: {} + packageStates: {} presentingModal: false pendingModals: [[]] getPathToOpen: -> @getWindowState('pathToOpen') ? window.location.params.pathToOpen - activateAtomPackage: (pack) -> - @activatedAtomPackages.push(pack) - pack.mainModule.activate(@atomPackageStates[pack.name] ? {}) + getPackageState: (name) -> + @packageStates[name] - deactivateAtomPackages: -> - pack.mainModule.deactivate?() for pack in @activatedAtomPackages - @activatedAtomPackages = [] + setPackageState: (name, state) -> + @packageStates[name] = state serializeAtomPackages: -> - packageStates = {} - for pack in @loadedPackages - if pack in @activatedAtomPackages - try - packageStates[pack.name] = pack.mainModule.serialize?() - catch e - console.error("Exception serializing '#{pack.name}' package's module\n", e.stack) - else - packageStates[pack.name] = @atomPackageStates[pack.name] - packageStates + for pack in @getActivePackages() + @setPackageState(pack.name, state) if state = pack.serialize?() + @packageStates activatePackages: -> @activatePackage(pack.path) for pack in @getLoadedPackages() @@ -51,6 +41,21 @@ _.extend atom, pack.activate(options) pack + deactivatePackages: -> + @deactivatePackage(pack.path) for pack in @getActivePackages() + + deactivatePackage: (id) -> + if pack = @getActivePackage(id) + @setPackageState(pack.name, state) if state = pack.serialize?() + pack.deactivate() + _.remove(@activePackages, pack) + else + throw new Error("No active package for id '#{id}'") + + getActivePackage: (id) -> + if path = @resolvePackagePath(id) + _.detect @activePackages, (pack) -> pack.path is path + isPackageActive: (id) -> if path = @resolvePackagePath(id) _.detect @activePackages, (pack) -> pack.path is path diff --git a/src/app/root-view.coffee b/src/app/root-view.coffee index 5a9f35a3f..aa1f38dac 100644 --- a/src/app/root-view.coffee +++ b/src/app/root-view.coffee @@ -30,7 +30,7 @@ class RootView extends View @subview 'panes', panes ? new PaneContainer @deserialize: ({ panes, packages, projectPath }) -> - atom.atomPackageStates = packages ? {} + atom.packageStates = packages ? {} panes = deserialize(panes) if panes?.deserializer is 'PaneContainer' new RootView({panes}) @@ -95,7 +95,7 @@ class RootView extends View @focus() if onDom deactivate: -> - atom.deactivateAtomPackages() + atom.deactivatePackages() @remove() open: (path, options = {}) -> diff --git a/src/app/text-mate-package.coffee b/src/app/text-mate-package.coffee index e9519ba74..36b09cf03 100644 --- a/src/app/text-mate-package.coffee +++ b/src/app/text-mate-package.coffee @@ -36,6 +36,9 @@ class TextMatePackage extends Package for { selector, properties } in @scopedProperties syntax.addProperties(selector, properties) + deactivate: -> + # we should remove grammars and unregister properties, snippets, etc + legalGrammarExtensions: ['plist', 'tmLanguage', 'tmlanguage', 'cson', 'json'] loadGrammars: (done) -> diff --git a/src/packages/tree-view/spec/tree-view-spec.coffee b/src/packages/tree-view/spec/tree-view-spec.coffee index 20b7f4410..5d0cfe31d 100644 --- a/src/packages/tree-view/spec/tree-view-spec.coffee +++ b/src/packages/tree-view/spec/tree-view-spec.coffee @@ -76,6 +76,7 @@ describe "TreeView", -> rootView.deactivate() window.rootView = new RootView rootView.open('tree-view.js') + atom.packageStates = {} treeView = atom.activatePackage("tree-view").mainModule.createView() expect(treeView.hasParent()).toBeFalsy() expect(treeView.root).toExist()