-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Prioritize platform-specific keybindings over generic bindings #2372
Conversation
|
||
/** | ||
* Use windows-specific bindings if no other are found (e.g. linux) | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should add a note here that the purpose of this is to make extensions backward-compatible and brackets core code should always have keybindings explicitly defined.
Is there a way to verify this in the code and write something in the console log?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way to verify what? That we eventually get either (1) a generic key binding or (2) a key binding for the current platform? We could validate the default keyboard.json, but for clients calling addBinding
directly we can't.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added comment
Done with initial review. |
@redmunds ready to review again. I didn't add any validation of key bindings since it depends on usage. Clients may add key bindings at any time, in any order. It doesn't seem to make sense to do the validation work, at least in our current state. |
The validation I was asking about is whether a keybinding is for core Brackets code, or from an extension. |
There doesn't yet seem to be a reasonable way to determine if a shortcut is coming from an extension, so dropping that. |
Looks good. Merging. @joelrbrandt This fixes the key binding bug you showed in the architecture meeting |
Prioritize platform-specific keybindings over generic bindings
Fixes Joel's issue from this morning's standup