Merge pull request #3058 from lkcampbell/fix-issue-2078

Fix issue 2078

Author: redmunds

src/command/KeyBindingManager.js

@@ -497,7 +497,7 @@ define(function (require, exports, module) {
 
     // TODO (issue #414): Replace this temporary fix with a more robust solution to handle focus and modality
     /**
-     * Enable or disable key bindings. Clients such as dialogs may wish to disable 
+     * Enable or disable key bindings. Clients such as dialogs may wish to disable
      * global key bindings temporarily.
      *
      * @param {string} A key-description string.
@@ -509,8 +509,8 @@ define(function (require, exports, module) {
 
     /**
      * Add one or more key bindings to a particular Command.
-     * 
-     * @param {!string} commandID
+     *
+     * @param {!string | Command} command - A command ID or command object
      * @param {?({key: string, displayKey: string} | Array.<{key: string, displayKey: string, platform: string}>)} keyBindings
      *     a single key binding or an array of keybindings. Example:
      *     "Shift-Cmd-F". Mac and Win key equivalents are automatically
@@ -522,14 +522,24 @@ define(function (require, exports, module) {
      * @return {{key: string, displayKey:String}|Array.<{key: string, displayKey:String}>}
      *     Returns record(s) for valid key binding(s)
      */
-    function addBinding(commandID, keyBindings, platform) {
-        if ((commandID === null) || (commandID === undefined) || !keyBindings) {
+    function addBinding(command, keyBindings, platform) {
+        var commandID           = "",
+            normalizedBindings  = [],
+            results;
+        
+        if (!command) {
+            console.error("addBinding(): missing required parameter: command");
             return;
         }
 
-        var normalizedBindings = [],
-            results;
-
+        if (!keyBindings) { return; }
+        
+        if (typeof (command) === "string") {
+            commandID = command;
+        } else {
+            commandID = command.getID();
+        }
+        
         if (Array.isArray(keyBindings)) {
             var keyBinding;
             results = [];
@@ -548,15 +558,29 @@ define(function (require, exports, module) {
 
         return results;
     }
-    
+
     /**
      * Retrieve key bindings currently associated with a command
      *
-     * @param {!string} command - A command ID
+     * @param {!string | Command} command - A command ID or command object
      * @return {!Array.<{{key: string, displayKey: string}}>} An array of associated key bindings.
      */
-    function getKeyBindings(commandID) {
-        var bindings = _commandMap[commandID];
+    function getKeyBindings(command) {
+        var bindings    = [],
+            commandID   = "";
+        
+        if (!command) {
+            console.error("getKeyBindings(): missing required parameter: command");
+            return [];
+        }
+        
+        if (typeof (command) === "string") {
+            commandID = command;
+        } else {
+            commandID = command.getID();
+        }
+        
+        bindings = _commandMap[commandID];
         return bindings || [];
     }
 
@@ -606,9 +630,9 @@ define(function (require, exports, module) {
     exports.getKeyBindings = getKeyBindings;
 
     /**
-     * Use windows-specific bindings if no other are found (e.g. Linux). 
+     * Use windows-specific bindings if no other are found (e.g. Linux).
      * Core Brackets modules that use key bindings should always define at
-     * least a generic keybinding that is applied for all platforms. This 
+     * least a generic keybinding that is applied for all platforms. This
      * setting effectively creates a compatibility mode for third party
      * extensions that define explicit key bindings for Windows and Mac, but
      * not Linux.

test/spec/KeyBindingManager-test.js

@@ -162,6 +162,30 @@ define(function (require, exports, module) {
                 expect(KeyBindingManager.getKeymap()).toEqual(expected);
             });
 
+            it("should allow the command argument to be a string or an object", function () {
+                var result = KeyBindingManager.addBinding("test.foo", "Ctrl-A"),
+                    keyTest = key("Ctrl-A");
+                
+                expect(result).toEqual(keyTest);
+                expect(KeyBindingManager.getKeyBindings("test.foo")).toEqual([keyTest]);
+                
+                var commandObj = CommandManager.register("Bar", "test.bar", function () { return; });
+                
+                result = KeyBindingManager.addBinding(commandObj, "Ctrl-B");
+                keyTest = key("Ctrl-B");
+                
+                expect(result).toEqual(keyTest);
+                expect(KeyBindingManager.getKeyBindings("test.bar")).toEqual([keyTest]);
+                
+                // only "test" platform bindings
+                var expected = keyMap([
+                    keyBinding("Ctrl-A", "test.foo"),
+                    keyBinding("Ctrl-B", "test.bar")
+                ]);
+                
+                expect(KeyBindingManager.getKeymap()).toEqual(expected);
+            });
+            
             it("should allow a generic key binding to be replaced", function () {
                 KeyBindingManager.addBinding("test.foo", "Ctrl-A");
                 KeyBindingManager.addBinding("test.bar", "Ctrl-A");
@@ -370,4 +394,4 @@ define(function (require, exports, module) {
         });
 
     });
-});
+});