Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Fixed: #3032 KeyBindingManager no longer warns / prevents binding collisions #3081

Merged
merged 2 commits into from
Mar 11, 2013
Merged

Fixed: #3032 KeyBindingManager no longer warns / prevents binding collisions #3081

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bb5c613
Cleaned up the handling of an existing key binding
WebsiteDeveloper Mar 9, 2013
d964cf8
Changed check
WebsiteDeveloper Mar 11, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 6 additions & 8 deletions src/command/KeyBindingManager.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
*/


/*jslint vars: true, plusplus: true, devel: true, nomen: true, indent: 4, maxerr: 50, regexp: true, boss: true */
/*jslint vars: true, plusplus: true, devel: true, nomen: true, indent: 4, maxerr: 50, regexp: true */
/*global define, $, brackets, window */

/**
Expand Down Expand Up @@ -395,16 +395,14 @@ define(function (require, exports, module) {
return null;
}

// skip if the key is already assigned explicitly for this platform
// skip if the key is already assigned
if (existing) {
if (!existing.explicitPlatform) {
// remove existing generic bindings, then re-map this binding
// to the new command
if (!existing.explicitPlatform && explicitPlatform) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In #3032, what happens is that the extension tries to replace a cross-platform Ctrl-E/Cmd-E binding with another cross-platform Ctrl-E/Cmd-# binding. I think a more correct fix might be to check the new binding for an explicit platform and only override it in that case:

if (!existing.explicitPlatform && existingPlatform) {
  // remove the the generic binding to replace with this new platform-specific binding
  removeBinding(normalized)
} else { 
  console.error(...);
  return null;
}

// remove the the generic binding to replace with this new platform-specific binding
removeBinding(normalized);
} else {
// do not re-assign a platform-specific key binding
console.log("Cannot assign " + normalized + " to " + commandID +
". It is already assigned to " + _keyMap[normalized].commandID);
// do not re-assign a key binding
console.error("Cannot assign " + normalized + " to " + commandID + ". It is already assigned to " + _keyMap[normalized].commandID);
return null;
}
}
Expand Down