rust-lang / rust-analyzer

A Rust compiler front-end for IDEs
https://rust-analyzer.github.io/
Apache License 2.0
14.21k stars 1.6k forks source link

[VSCode] - Change bindings for Rust Analyzer: Run #2733

Closed jasonwilliams closed 4 years ago

jasonwilliams commented 4 years ago

It's not usually good practise to overload common key bindings. ctrl + r is regularly used to change project (File: Open Recent).

Is it possible to change rust-analyzer.run to use a different key binding? Something less common or unused?

https://github.com/rust-analyzer/rust-analyzer/blob/e4d217074d1f2c922cf8c5a247ca05fa06b0b7ed/editors/code/package.json#L136-L140

matklad commented 4 years ago

Yeah, stumping over build-in bindings is pretty impolite.

What are the guidelines for extension keybindings? Is there a shortcut "namespace" which is considered ok to be occupied by extensions?

I am also fine with just removing this keybinding by default, but I think this is one of the best productivity booster, so having some default and nudging the user into trying it feels a right thing to do.

jasonwilliams commented 4 years ago

What are the guidelines for extension keybindings? Is there a shortcut "namespace" which is considered ok to be occupied by extensions?

Thats a good question, im not sure but can find out

p-avital commented 4 years ago

Maybe a good compromise would be overriding the default Run action (F5 on many platforms, I'm not sure if we can have different defaults for Macs on which I believe the default is different) ? Or using a variant on it, like Shift/Alt + F5 ?

jasonwilliams commented 4 years ago

What are the guidelines for extension keybindings? Is there a shortcut "namespace" which is considered ok to be occupied by extensions?

Ive looked around i don't think such a thing exists, it seems like most other keybindings are fine except for this one. @matklad are you happy for a PR to remove this keybinding with users adding it back when they need it?

Or switching it to the default run action (which is more safe to overload as that's what its there for) as @p-avital pointed out

matklad commented 4 years ago

yes. We also should update features.md, I think it mentions the default keybinding

On Fri, 28 Feb 2020 at 11:52, Jason Williams notifications@github.com wrote:

What are the guidelines for extension keybindings? Is there a shortcut "namespace" which is considered ok to be occupied by extensions?

Ive looked around i don't think such a thing exists, it seems like most other keybindings are fine except for this one. @matklad https://github.com/matklad are you happy for a PR to remove this keybinding with users adding it back when they need it?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/rust-analyzer/rust-analyzer/issues/2733?email_source=notifications&email_token=AANB3M45TNFGA5UUV76AXGDRFDUINA5CNFSM4KCNW7X2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOENIDXTY#issuecomment-592460751, or unsubscribe https://github.com/notifications/unsubscribe-auth/AANB3MYIAMAKRLXFT7EH4NTRFDUINANCNFSM4KCNW7XQ .