Merge pull request #3655 from atom/ns-menu-disposables

Return disposables from MenuManager::add and add specs

spec/menu-manager-spec.coffee

@@ -0,0 +1,48 @@
+MenuManager = require '../src/menu-manager'
+
+describe "MenuManager", ->
+  menu = null
+
+  beforeEach ->
+    menu = new MenuManager(resourcePath: atom.getLoadSettings().resourcePath)
+
+  describe "::add(items)", ->
+    it "can add new menus that can be removed with the returned disposable", ->
+      disposable = menu.add [{label: "A", submenu: [{label: "B", command: "b"}]}]
+      expect(menu.template).toEqual [{label: "A", submenu: [{label: "B", command: "b"}]}]
+      disposable.dispose()
+      expect(menu.template).toEqual []
+
+    it "can add submenu items to existing menus that can be removed with the returned disposable", ->
+      disposable1 = menu.add [{label: "A", submenu: [{label: "B", command: "b"}]}]
+      disposable2 = menu.add [{label: "A", submenu: [{label: "C", submenu: [{label: "D", command: 'd'}]}]}]
+      disposable3 = menu.add [{label: "A", submenu: [{label: "C", submenu: [{label: "E", command: 'e'}]}]}]
+
+      expect(menu.template).toEqual [{
+        label: "A",
+        submenu: [
+          {label: "B", command: "b"},
+          {label: "C", submenu: [{label: 'D', command: 'd'}, {label: 'E', command: 'e'}]}
+        ]
+      }]
+
+      disposable3.dispose()
+      expect(menu.template).toEqual [{
+        label: "A",
+        submenu: [
+          {label: "B", command: "b"},
+          {label: "C", submenu: [{label: 'D', command: 'd'}]}
+        ]
+      }]
+
+      disposable2.dispose()
+      expect(menu.template).toEqual [{label: "A", submenu: [{label: "B", command: "b"}]}]
+
+      disposable1.dispose()
+      expect(menu.template).toEqual []
+
+    it "does not add duplicate labels to the same menu", ->
+      originalItemCount = menu.template.length
+      menu.add [{label: "A", submenu: [{label: "B", command: "b"}]}]
+      menu.add [{label: "A", submenu: [{label: "B", command: "b"}]}]
+      expect(menu.template[originalItemCount]).toEqual {label: "A", submenu: [{label: "B", command: "b"}]}

src/menu-manager.coffee

@@ -4,6 +4,7 @@ _ = require 'underscore-plus'
 ipc = require 'ipc'
 CSON = require 'season'
 fs = require 'fs-plus'
+{Disposable} = require 'event-kit'
 
 # Extended: Provides a registry for menu items that you'd like to appear in the
 # application menu.
@@ -34,10 +35,17 @@ class MenuManager
   #   * `submenu` An optional {Array} of sub menu items.
   #   * `command` An optional {String} command to trigger when the item is
   #     clicked.
+  #
+  # Returns a {Disposable} on which `.dispose()` can be called to remove the
+  # added menu items.
   add: (items) ->
     @merge(@template, item) for item in items
     @update()
-    undefined
+    new Disposable => @remove(items)
+
+  remove: (items) ->
+    @unmerge(@template, item) for item in items
+    @update()
 
   # Should the binding for the given selector be included in the menu
   # commands.
@@ -96,11 +104,29 @@ class MenuManager
   # appended to the bottom of existing menus where possible.
   merge: (menu, item) ->
     item = _.deepClone(item)
+    matchingItem = @findMatchingItem(menu, item)
 
-    if item.submenu? and match = _.find(menu, ({label, submenu}) => submenu? and label and @normalizeLabel(label) is @normalizeLabel(item.label))
-      @merge(match.submenu, i) for i in item.submenu
+    if matchingItem?
+      if item.submenu?
+        @merge(matchingItem.submenu, submenuItem) for submenuItem in item.submenu
     else
-      menu.push(item) unless _.find(menu, ({label}) => label and @normalizeLabel(label) is @normalizeLabel(item.label))
+      menu.push(item)
+
+  unmerge: (menu, item) ->
+    if matchingItem = @findMatchingItem(menu, item)
+      if item.submenu?
+        @unmerge(matchingItem.submenu, submenuItem) for submenuItem in item.submenu
+
+      unless matchingItem.submenu?.length > 0
+        menu.splice(menu.indexOf(matchingItem), 1)
+
+  # find an existing menu item matching the given item
+  findMatchingItem: (menu, {label, submenu}) ->
+    debugger unless menu?
+    for item in menu
+      if @normalizeLabel(item.label) is @normalizeLabel(label) and item.submenu? is submenu?
+        return item
+    null
 
   # OSX can't handle displaying accelerators for multiple keystrokes.
   # If they are sent across, it will stop processing accelerators for the rest