From 59c3dea77bcdd8d67cc030fb85621bb8e6e071d9 Mon Sep 17 00:00:00 2001
From: Kevin Sawicki <kevin@github.com>
Date: Tue, 3 Mar 2015 16:51:38 -0800
Subject: [PATCH] Show notification for invalid context menu selector

---
 .../menus/menu.json                           | 10 +++++
 .../package.json                              |  4 ++
 spec/package-manager-spec.coffee              | 11 +++++-
 src/package.coffee                            | 39 ++++++++++++-------
 4 files changed, 49 insertions(+), 15 deletions(-)
 create mode 100644 spec/fixtures/packages/package-with-invalid-context-menu/menus/menu.json
 create mode 100644 spec/fixtures/packages/package-with-invalid-context-menu/package.json

diff --git a/spec/fixtures/packages/package-with-invalid-context-menu/menus/menu.json b/spec/fixtures/packages/package-with-invalid-context-menu/menus/menu.json
new file mode 100644
index 000000000..67197f8fe
--- /dev/null
+++ b/spec/fixtures/packages/package-with-invalid-context-menu/menus/menu.json
@@ -0,0 +1,10 @@
+{
+  "context-menu": {
+    "<>": [
+      {
+        "label": "Hello",
+        "command:": "world"
+      }
+    ]
+  }
+}
diff --git a/spec/fixtures/packages/package-with-invalid-context-menu/package.json b/spec/fixtures/packages/package-with-invalid-context-menu/package.json
new file mode 100644
index 000000000..c742d422a
--- /dev/null
+++ b/spec/fixtures/packages/package-with-invalid-context-menu/package.json
@@ -0,0 +1,4 @@
+{
+  "name": "package-with-invalid-context-menu",
+  "version": "1.0.0"
+}
diff --git a/spec/package-manager-spec.coffee b/spec/package-manager-spec.coffee
index 5855006dd..a021f979a 100644
--- a/spec/package-manager-spec.coffee
+++ b/spec/package-manager-spec.coffee
@@ -2,7 +2,7 @@
 Package = require '../src/package'
 {Disposable} = require 'atom'
 
-fdescribe "PackageManager", ->
+describe "PackageManager", ->
   workspaceElement = null
 
   beforeEach ->
@@ -212,13 +212,20 @@ fdescribe "PackageManager", ->
           runs ->
             expect(mainModule.activate.callCount).toBe 1
 
-        it "logs a warning when the activation commands are invalid", ->
+        it "adds a notification when the activation commands are invalid", ->
           addErrorHandler = jasmine.createSpy()
           atom.notifications.onDidAddNotification(addErrorHandler)
           expect(-> atom.packages.activatePackage('package-with-invalid-activation-commands')).not.toThrow()
           expect(addErrorHandler.callCount).toBe 1
           expect(addErrorHandler.argsForCall[0][0].message).toContain("Failed to activate the package-with-invalid-activation-commands package")
 
+        it "adds a notification when the context menu is invalid", ->
+          addErrorHandler = jasmine.createSpy()
+          atom.notifications.onDidAddNotification(addErrorHandler)
+          expect(-> atom.packages.activatePackage('package-with-invalid-context-menu')).not.toThrow()
+          expect(addErrorHandler.callCount).toBe 1
+          expect(addErrorHandler.argsForCall[0][0].message).toContain("Failed to activate the package-with-invalid-context-menu package")
+
     describe "when the package has no main module", ->
       it "does not throw an exception", ->
         spyOn(console, "error")
diff --git a/src/package.coffee b/src/package.coffee
index ef4cc9c42..148e5d4e4 100644
--- a/src/package.coffee
+++ b/src/package.coffee
@@ -143,18 +143,14 @@ class Package
     unless @activationDeferred?
       @activationDeferred = Q.defer()
       @measure 'activateTime', =>
-        @activateResources()
-        if @hasActivationCommands()
-          try
+        try
+          @activateResources()
+          if @hasActivationCommands()
             @subscribeToActivationCommands()
-          catch error
-            if error.code is 'EBADSELECTOR'
-              metadataPath = path.join(@path, 'package.json')
-              error.message += " in #{metadataPath}"
-              error.stack += "\n  at #{metadataPath}:1:1"
-            @handleError("Failed to activate the #{@name} package", error)
-        else
-          @activateNow()
+          else
+            @activateNow()
+        catch error
+          @handleError("Failed to activate the #{@name} package", error)
 
     Q.all([@grammarsPromise, @settingsPromise, @activationDeferred.promise])
 
@@ -206,7 +202,16 @@ class Package
   activateResources: ->
     @activationDisposables = new CompositeDisposable
     @activationDisposables.add(atom.keymaps.add(keymapPath, map)) for [keymapPath, map] in @keymaps
-    @activationDisposables.add(atom.contextMenu.add(map['context-menu'])) for [menuPath, map] in @menus when map['context-menu']?
+
+    for [menuPath, map] in @menus when map['context-menu']?
+      try
+        @activationDisposables.add(atom.contextMenu.add(map['context-menu']))
+      catch error
+        if error.code is 'EBADSELECTOR'
+          error.message += " in #{menuPath}"
+          error.stack += "\n  at #{menuPath}:1:1"
+        throw error
+
     @activationDisposables.add(atom.menu.add(map['menu'])) for [menuPath, map] in @menus when map['menu']?
 
     unless @grammarsActivated
@@ -417,7 +422,15 @@ class Package
         do (selector, command) =>
           # Add dummy command so it appears in menu.
           # The real command will be registered on package activation
-          @activationCommandSubscriptions.add atom.commands.add selector, command, ->
+          try
+            @activationCommandSubscriptions.add atom.commands.add selector, command, ->
+          catch error
+            if error.code is 'EBADSELECTOR'
+              metadataPath = path.join(@path, 'package.json')
+              error.message += " in #{metadataPath}"
+              error.stack += "\n  at #{metadataPath}:1:1"
+            throw error
+
           @activationCommandSubscriptions.add atom.commands.onWillDispatch (event) =>
             return unless event.type is command
             currentTarget = event.target