From 447dfd8bec6df7841df255edae73a8c43f854074 Mon Sep 17 00:00:00 2001
From: Nathan Sobo <nathan@github.com>
Date: Thu, 28 May 2015 22:58:29 +0200
Subject: [PATCH] =?UTF-8?q?If=20a=20keystroke=20is=20bound=20to=20?=
 =?UTF-8?q?=E2=80=98unset!=E2=80=99,=20omit=20it=20in=20the=20application?=
 =?UTF-8?q?=20menu?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Fixes atom/atom-keymap#79

This is more general than I would like. If the keystroke is unset in
any context, we err on the side of caution and don’t add it to the
application menu for any command. Since our application menu isn’t
context aware, this should be good enough for now and solve the 80%
case. Someday we should make the application menu update / gray out
options when the focused element changes.
---
 spec/menu-manager-spec.coffee | 30 ++++++++++++++++++++++++++++++
 src/menu-manager.coffee       | 12 +++++++++++-
 2 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/spec/menu-manager-spec.coffee b/spec/menu-manager-spec.coffee
index d92c921c4..f86b6e1a0 100644
--- a/spec/menu-manager-spec.coffee
+++ b/spec/menu-manager-spec.coffee
@@ -1,3 +1,4 @@
+path = require 'path'
 MenuManager = require '../src/menu-manager'
 
 describe "MenuManager", ->
@@ -46,3 +47,32 @@ describe "MenuManager", ->
       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"}]}
+
+  describe "::update()", ->
+    it "sends the current menu template and associated key bindings to the browser process", ->
+      spyOn(menu, 'sendToBrowserProcess')
+      menu.add [{label: "A", submenu: [{label: "B", command: "b"}]}]
+      atom.keymap.add 'test', 'atom-workspace': 'ctrl-b': 'b'
+      menu.update()
+
+      waits 1
+
+      runs -> expect(menu.sendToBrowserProcess.argsForCall[0][1]['b']).toEqual ['ctrl-b']
+
+    it "omits key bindings that are mapped to unset! in any context", ->
+      # it would be nice to be smarter about omitting, but that would require a much
+      # more dynamic interaction between the currently focused element and the menu
+      spyOn(menu, 'sendToBrowserProcess')
+      menu.add [{label: "A", submenu: [{label: "B", command: "b"}]}]
+      atom.keymap.add 'test', 'atom-workspace': 'ctrl-b': 'b'
+      atom.keymap.add 'test', 'atom-text-editor': 'ctrl-b': 'unset!'
+
+      waits 1
+
+      runs -> expect(menu.sendToBrowserProcess.argsForCall[0][1]['b']).toBeUndefined()
+
+  it "updates the application menu when a keymap is reloaded", ->
+    spyOn(menu, 'update')
+    keymapPath = path.join(__dirname, 'fixtures', 'packages', 'package-with-keymaps', 'keymaps', 'keymap-1.cson')
+    atom.keymaps.reloadKeymap(keymapPath)
+    expect(menu.update).toHaveBeenCalled()
diff --git a/src/menu-manager.coffee b/src/menu-manager.coffee
index b5c9a80e3..b138f2da2 100644
--- a/src/menu-manager.coffee
+++ b/src/menu-manager.coffee
@@ -63,6 +63,7 @@ class MenuManager
     @pendingUpdateOperation = null
     @template = []
     atom.keymaps.onDidLoadBundledKeymaps => @loadPlatformItems()
+    atom.keymaps.onDidReloadKeymap => @update()
     atom.packages.onDidActivateInitialPackages => @sortPackagesMenu()
 
   # Public: Adds the given items to the application menu.
@@ -139,10 +140,19 @@ class MenuManager
   update: ->
     clearImmediate(@pendingUpdateOperation) if @pendingUpdateOperation?
     @pendingUpdateOperation = setImmediate =>
-      keystrokesByCommand = {}
+      includedBindings = []
+      unsetKeystrokes = new Set
+
       for binding in atom.keymaps.getKeyBindings() when @includeSelector(binding.selector)
+        includedBindings.push(binding)
+        if binding.command is 'unset!'
+          unsetKeystrokes.add(binding.keystrokes)
+
+      keystrokesByCommand = {}
+      for binding in includedBindings when not unsetKeystrokes.has(binding.keystrokes)
         keystrokesByCommand[binding.command] ?= []
         keystrokesByCommand[binding.command].unshift binding.keystrokes
+
       @sendToBrowserProcess(@template, keystrokesByCommand)
 
   loadPlatformItems: ->