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

Update 'addBinding' documentation in KeyBindingManager. #10225

Merged
merged 1 commit into from
Dec 20, 2014

Conversation

zaggino
Copy link
Contributor

@zaggino zaggino commented Dec 19, 2014

Related to issue brackets-userland/brackets-git#846
Btw what is the reason for this key replacement?

@@ -826,6 +826,7 @@ define(function (require, exports, module) {
* @param {?string} platform The target OS of the keyBindings either
* "mac", "win" or "linux". If undefined, all platforms not explicitly
* defined will use the key binding.
* NOTE: If platform is not specified, Ctrl will be replaced by Cmd for "mac" platform
Copy link
Contributor

Choose a reason for hiding this comment

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

@zaggino

Btw what is the reason for this key replacement?

It's to make specifying shortcuts simpler. Usually a command with a shortcut of Ctrl on Windows is Cmd on Mac.

Is this causing a problem, or you just weren't aware of it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wasn't aware of it and quite surprised me :)
I had an issue that user was trying to set Ctrl shortcuts in Git extension but it got binded to Cmd instead, so I had to look through the codes to realize I have to specify brackets.platform as a third argument for addBinding method.

Copy link
Contributor

Choose a reason for hiding this comment

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

I should have followed the link first :)

Shortcuts specified for keybindings are cross-platform, so Ctrl is translated to Cmd on Mac for reason stated here.

But, user key map is user-specific, which is more likely to be machine-specific, so we do not translate Ctrl is translated to Cmd on Mac in this case.

@redmunds
Copy link
Contributor

I'll merge this as soon as we tag the repo for Brackets 1.1.

@ingorichter
Copy link
Contributor

@redmunds this can be merged now. I have tagged the repo yesterday.

redmunds added a commit that referenced this pull request Dec 20, 2014
Update 'addBinding' documentation in KeyBindingManager.
@redmunds redmunds merged commit a84b2d7 into master Dec 20, 2014
@redmunds redmunds deleted the zaggino/doc-fix branch December 20, 2014 00:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants