Don't store binding sets, instead store a keyBinding array

spec/keymap-spec.coffee

@@ -24,9 +24,9 @@ describe "Keymap", ->
     metaZHandler = null
 
     beforeEach ->
-      keymap.bindKeys '.command-mode', 'x': 'deleteChar'
-      keymap.bindKeys '.insert-mode', 'x': 'insertChar'
-      keymap.bindKeys '.command-mode', 'meta-z': 'metaZPressed'
+      keymap.bindKeys 'name', '.command-mode', 'x': 'deleteChar'
+      keymap.bindKeys 'name', '.insert-mode', 'x': 'insertChar'
+      keymap.bindKeys 'name', '.command-mode', 'meta-z': 'metaZPressed'
 
       deleteCharHandler = jasmine.createSpy('deleteCharHandler')
       insertCharHandler = jasmine.createSpy('insertCharHandler')
@@ -80,7 +80,7 @@ describe "Keymap", ->
 
       describe "when the event's target node descends from multiple nodes that match selectors with a binding", ->
         beforeEach ->
-          keymap.bindKeys '.child-node', 'x': 'foo'
+          keymap.bindKeys 'name', '.child-node', 'x': 'foo'
 
         it "only triggers bindings on selectors associated with the closest ancestor node", ->
           fooHandler = jasmine.createSpy 'fooHandler'
@@ -117,10 +117,10 @@ describe "Keymap", ->
       describe "when the event bubbles to a node that matches multiple selectors", ->
         describe "when the matching selectors differ in specificity", ->
           it "triggers the binding for the most specific selector", ->
-            keymap.bindKeys 'div .child-node', 'x': 'foo'
-            keymap.bindKeys '.command-mode .child-node !important', 'x': 'baz'
-            keymap.bindKeys '.command-mode .child-node', 'x': 'quux'
-            keymap.bindKeys '.child-node', 'x': 'bar'
+            keymap.bindKeys 'name', 'div .child-node', 'x': 'foo'
+            keymap.bindKeys 'name', '.command-mode .child-node !important', 'x': 'baz'
+            keymap.bindKeys 'name', '.command-mode .child-node', 'x': 'quux'
+            keymap.bindKeys 'name', '.child-node', 'x': 'bar'
 
             fooHandler = jasmine.createSpy 'fooHandler'
             barHandler = jasmine.createSpy 'barHandler'
@@ -138,8 +138,8 @@ describe "Keymap", ->
 
         describe "when the matching selectors have the same specificity", ->
           it "triggers the bindings for the most recently declared selector", ->
-            keymap.bindKeys '.child-node', 'x': 'foo', 'y': 'baz'
-            keymap.bindKeys '.child-node', 'x': 'bar'
+            keymap.bindKeys 'name', '.child-node', 'x': 'foo', 'y': 'baz'
+            keymap.bindKeys 'name', '.child-node', 'x': 'bar'
 
             fooHandler = jasmine.createSpy 'fooHandler'
             barHandler = jasmine.createSpy 'barHandler'
@@ -161,7 +161,7 @@ describe "Keymap", ->
         it "triggers the mapped event on the rootView", ->
           window.rootView = new RootView
           rootView.attachToDom()
-          keymap.bindKeys 'body', 'x': 'foo'
+          keymap.bindKeys 'name', 'body', 'x': 'foo'
           fooHandler = jasmine.createSpy("fooHandler")
           rootView.on 'foo', fooHandler
 
@@ -173,7 +173,7 @@ describe "Keymap", ->
 
       describe "when the event matches a 'native!' binding", ->
         it "returns true, allowing the browser's native key handling to process the event", ->
-          keymap.bindKeys '.grandchild-node', 'x': 'native!'
+          keymap.bindKeys 'name', '.grandchild-node', 'x': 'native!'
           nativeHandler = jasmine.createSpy("nativeHandler")
           fragment.on 'native!', nativeHandler
           expect(keymap.handleKeyEvent(keydownEvent('x', target: fragment.find('.grandchild-node')[0]))).toBe true
@@ -183,7 +183,7 @@ describe "Keymap", ->
       [quitHandler, closeOtherWindowsHandler] = []
 
       beforeEach ->
-        keymap.bindKeys "*",
+        keymap.bindKeys 'name', "*",
           'ctrl-x ctrl-c': 'quit'
           'ctrl-x 1': 'close-other-windows'
 
@@ -223,7 +223,7 @@ describe "Keymap", ->
 
       describe "when the event's target node descends from multiple nodes that match selectors with a partial binding match", ->
         it "allows any of the bindings to be triggered upon a second keystroke, favoring the most specific selector", ->
-          keymap.bindKeys ".grandchild-node", 'ctrl-x ctrl-c': 'more-specific-quit'
+          keymap.bindKeys 'name', ".grandchild-node", 'ctrl-x ctrl-c': 'more-specific-quit'
           grandchildNode = fragment.find('.grandchild-node')[0]
           moreSpecificQuitHandler = jasmine.createSpy('moreSpecificQuitHandler')
           fragment.on 'more-specific-quit', moreSpecificQuitHandler
@@ -247,17 +247,17 @@ describe "Keymap", ->
       describe "when there is a complete binding with a more specific selector", ->
         it "favors the more specific complete match", ->
 
-  describe ".bindKeys(selector, bindings)", ->
+  describe ".bindKeys(name, selector, bindings)", ->
     it "normalizes the key patterns in the hash to put the modifiers in alphabetical order", ->
       fooHandler = jasmine.createSpy('fooHandler')
       fragment.on 'foo', fooHandler
-      keymap.bindKeys '*', 'ctrl-alt-delete': 'foo'
+      keymap.bindKeys 'name', '*', 'ctrl-alt-delete': 'foo'
       result = keymap.handleKeyEvent(keydownEvent('delete', ctrlKey: true, altKey: true, target: fragment[0]))
       expect(result).toBe(false)
       expect(fooHandler).toHaveBeenCalled()
 
       fooHandler.reset()
-      keymap.bindKeys '*', 'ctrl-alt--': 'foo'
+      keymap.bindKeys 'name', '*', 'ctrl-alt--': 'foo'
       result = keymap.handleKeyEvent(keydownEvent('-', ctrlKey: true, altKey: true, target: fragment[0]))
       expect(result).toBe(false)
       expect(fooHandler).toHaveBeenCalled()
@@ -277,8 +277,6 @@ describe "Keymap", ->
 
       expect(keymap.bindingsMatchingElement($$ -> @div class: 'green')).toEqual []
       expect(keymap.bindingsMatchingElement($$ -> @div class: 'brown')).toEqual []
-      expect(keymap.bindingSetsByFirstKeystroke['ctrl-c']).toEqual []
-      expect(keymap.bindingSetsByFirstKeystroke['ctrl-h']).toEqual []
 
   describe ".keystrokeStringForEvent(event)", ->
     describe "when no modifiers are pressed", ->
@@ -305,8 +303,8 @@ describe "Keymap", ->
 
   describe ".bindingsMatchingElement(element)", ->
     it "returns the matching bindings for the element", ->
-      keymap.bindKeys '.command-mode', 'c': 'c'
-      keymap.bindKeys '.grandchild-node', 'g': 'g'
+      keymap.bindKeys 'name', '.command-mode', 'c': 'c'
+      keymap.bindKeys 'name', '.grandchild-node', 'g': 'g'
 
       bindings = keymap.bindingsMatchingElement(fragment.find('.grandchild-node'))
       expect(bindings).toHaveLength 2
@@ -315,9 +313,9 @@ describe "Keymap", ->
 
     describe "when multiple bindings match a keystroke", ->
       it "only returns bindings that match the most specific selector", ->
-        keymap.bindKeys '.command-mode', 'g': 'command-mode'
-        keymap.bindKeys '.command-mode .grandchild-node', 'g': 'command-and-grandchild-node'
-        keymap.bindKeys '.grandchild-node', 'g': 'grandchild-node'
+        keymap.bindKeys 'name', '.command-mode', 'g': 'command-mode'
+        keymap.bindKeys 'name', '.command-mode .grandchild-node', 'g': 'command-and-grandchild-node'
+        keymap.bindKeys 'name', '.grandchild-node', 'g': 'grandchild-node'
 
         bindings = keymap.bindingsMatchingElement(fragment.find('.grandchild-node'))
         expect(bindings).toHaveLength 3

spec/spec-helper.coffee

@@ -23,7 +23,7 @@ atom.themes.requireStylesheet '../static/jasmine'
 fixturePackagesPath = path.resolve(__dirname, './fixtures/packages')
 atom.packages.packageDirPaths.unshift(fixturePackagesPath)
 atom.keymap.loadBundledKeymaps()
-[bindingSetsToRestore, bindingSetsByFirstKeystrokeToRestore] = []
+keyBindingsToRestore = null
 
 $(window).on 'core:close', -> window.close()
 $(window).on 'unload', ->
@@ -67,8 +67,7 @@ beforeEach ->
   resolvePackagePath = _.bind(spy.originalValue, atom.packages)
 
   # used to reset keymap after each spec
-  bindingSetsToRestore = _.clone(atom.keymap.bindingSets)
-  bindingSetsByFirstKeystrokeToRestore = _.clone(atom.keymap.bindingSetsByFirstKeystroke)
+  keyBindingsToRestore = _.clone(atom.keymap.allBindings())
 
   # prevent specs from modifying Atom's menus
   spyOn(atom.menu, 'sendToBrowserProcess')
@@ -108,8 +107,7 @@ beforeEach ->
   addCustomMatchers(this)
 
 afterEach ->
-  atom.keymap.bindingSets = bindingSetsToRestore
-  atom.keymap.bindingSetsByFirstKeystroke = bindingSetsByFirstKeystrokeToRestore
+  atom.keymap.keyBindings = keyBindingsToRestore
   atom.deactivatePackages()
   atom.menu.template = []
 

src/binding-set.coffee

@@ -4,11 +4,12 @@ fs = require 'fs-plus'
 {specificity} = require 'clear-cut'
 PEG = require 'pegjs'
 
+nextBindingSetIndex = 0
+
 ### Internal ###
 
 module.exports =
 class BindingSet
-
   @parser: null
 
   selector: null
@@ -16,7 +17,8 @@ class BindingSet
   parser: null
   name: null
 
-  constructor: (selector, commandsByKeystroke, @index, @name) ->
+  constructor: (selector, commandsByKeystroke, @name) ->
+    @index = nextBindingSetIndex++
     keystrokePattern = fs.readFileSync(require.resolve('./keystroke-pattern.pegjs'), 'utf8')
     BindingSet.parser ?= PEG.buildParser(keystrokePattern)
     @specificity = specificity(selector)

src/keymap.coffee

@@ -25,14 +25,8 @@ module.exports =
 class Keymap
   Emitter.includeInto(this)
 
-  bindingSets: null
-  nextBindingSetIndex: 0
-  bindingSetsByFirstKeystroke: null
-  queuedKeystroke: null
-
   constructor: ({@resourcePath, @configDirPath})->
-    @bindingSets = []
-    @bindingSetsByFirstKeystroke = {}
+    @keyBindings = []
 
   loadBundledKeymaps: ->
     @loadDirectory(path.join(@resourcePath, 'keymaps'))
@@ -48,102 +42,81 @@ class Keymap
   load: (path) ->
     @add(path, CSON.readFileSync(path))
 
-  add: (args...) ->
-    name = args.shift() if args.length > 1
-    keymap = args.shift()
-    for selector, bindings of keymap
-      @bindKeys(name, selector, bindings)
+  add: (name, keyMappingsBySelector) ->
+    for selector, keyMappings of keyMappingsBySelector
+      @bindKeys(name, selector, keyMappings)
 
   remove: (name) ->
-    for bindingSet in @bindingSets.filter((bindingSet) -> bindingSet.name is name)
-      _.remove(@bindingSets, bindingSet)
-      for keystroke of bindingSet.commandsByKeystroke
-        firstKeystroke = keystroke.split(' ')[0]
-        _.remove(@bindingSetsByFirstKeystroke[firstKeystroke], bindingSet)
+    @keyBindings = @keyBindings.filter (keyBinding) -> keyBinding.name is name
 
-  bindKeys: (args...) ->
-    name = args.shift() if args.length > 2
-    [selector, bindings] = args
-    bindingSet = new BindingSet(selector, bindings, @nextBindingSetIndex++, name)
-    @bindingSets.unshift(bindingSet)
-    for keystroke of bindingSet.commandsByKeystroke
-      keystroke = keystroke.split(' ')[0] # only index by first keystroke
-      @bindingSetsByFirstKeystroke[keystroke] ?= []
-      @bindingSetsByFirstKeystroke[keystroke].push(bindingSet)
+  bindKeys: (name, selector, keyMappings) ->
+    bindingSet = new BindingSet(selector, keyMappings, name)
+    for keystroke, command of keyMappings
+      @keyBindings.push @buildBinding(bindingSet, command, keystroke)
 
-  unbindKeys: (selector, bindings) ->
-    bindingSet = _.detect @bindingSets, (bindingSet) ->
-      bindingSet.selector is selector and bindingSet.bindings is bindings
-
-    if bindingSet
-      _.remove(@bindingSets, bindingSet)
+  buildBinding: (bindingSet, command, keystroke) ->
+    keystroke = @normalizeKeystroke(keystroke)
+    selector = bindingSet.selector
+    specificity = bindingSet.specificity
+    index = bindingSet.index
+    source = bindingSet.name
+    {command, keystroke, selector, specificity, source, index}
 
   handleKeyEvent: (event) ->
     element = event.target
     element = rootView[0] if element == document.body
     keystroke = @keystrokeStringForEvent(event, @queuedKeystroke)
-    bindings = @bindingsForKeystrokeMatchingElement(keystroke, element)
+    keyBindings = @bindingsForKeystrokeMatchingElement(keystroke, element)
 
-    if bindings.length == 0 and @queuedKeystroke
+    if keyBindings.length == 0 and @queuedKeystroke
       @queuedKeystroke = null
       return false
     else
       @queuedKeystroke = null
 
-    for binding in bindings
-      partialMatch = binding.keystroke isnt keystroke
+    for keyBinding in keyBindings
+      partialMatch = keyBinding.keystroke isnt keystroke
       if partialMatch
         @queuedKeystroke = keystroke
         shouldBubble = false
       else
-        if binding.command is 'native!'
+        if keyBinding.command is 'native!'
           shouldBubble = true
-        else if @triggerCommandEvent(element, binding.command)
+        else if @triggerCommandEvent(element, keyBinding.command)
           shouldBubble = false
 
       break if shouldBubble?
 
     shouldBubble ? true
 
-  # Public: Returns an array of objects that represent every keybinding. Each
+  # Public: Returns an array of objects that represent every keyBinding. Each
   # object contains the following keys `source`, `selector`, `command`,
   # `keystroke`, `index`, `specificity`.
   allBindings: ->
-    bindings = []
-
-    for bindingSet in @bindingSets
-      for keystroke, command of bindingSet.getCommandsByKeystroke()
-        bindings.push @buildBinding(bindingSet, command, keystroke)
-
-    bindings
+    @keyBindings
 
   bindingsForKeystrokeMatchingElement: (keystroke, element) ->
-    bindings = @bindingsForKeystroke(keystroke)
-    @bindingsMatchingElement(element, bindings)
+    keyBindings = @bindingsForKeystroke(keystroke)
+    @bindingsMatchingElement(element, keyBindings)
 
   bindingsForKeystroke: (keystroke) ->
-    bindings = @allBindings().filter (binding) ->
+    keystroke = @normalizeKeystroke(keystroke)
+
+    keyBindings = @allBindings().filter (keyBinding) ->
       multiKeystroke = /\s/.test keystroke
       if multiKeystroke
-        keystroke == binding.keystroke
+        keystroke == keyBinding.keystroke
       else
-        keystroke.split(' ')[0] == binding.keystroke.split(' ')[0]
+        keystroke.split(' ')[0] == keyBinding.keystroke.split(' ')[0]
 
-  bindingsMatchingElement: (element, bindings=@allBindings()) ->
-    bindings = bindings.filter ({selector}) -> $(element).closest(selector).length > 0
-    bindings.sort (a, b) ->
+  bindingsMatchingElement: (element, keyBindings=@allBindings()) ->
+    keyBindings = keyBindings.filter ({selector}) -> $(element).closest(selector).length > 0
+    keyBindings.sort (a, b) ->
       if b.specificity == a.specificity
         b.index - a.index
       else
         b.specificity - a.specificity
 
-  buildBinding: (bindingSet, command, keystroke) ->
-    selector = bindingSet.selector
-    specificity = bindingSet.specificity
-    index = bindingSet.index
-    source = bindingSet.name
-    {command, keystroke, selector, specificity, source}
-
   triggerCommandEvent: (element, commandName) ->
     commandEvent = $.Event(commandName)
     commandEvent.abortKeyBinding = -> commandEvent.stopImmediatePropagation()
@@ -195,3 +168,11 @@ class Keymap
       when 32 then 'space'
       when 127 then 'delete'
       else String.fromCharCode(charCode)
+
+  normalizeKeystroke: (keystroke) ->
+    normalizedKeystroke = keystroke.split(/\s+/).map (keystroke) =>
+      keys = BindingSet.parser.parse(keystroke)
+      modifiers = keys[0...-1]
+      modifiers.sort()
+      [modifiers..., _.last(keys)].join('-')
+    normalizedKeystroke.join(' ')