-
Notifications
You must be signed in to change notification settings - Fork 99
Fix issues causing RStudio keybindings to be incorrectly shown in some contexts #7362
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
E2E Tests 🚀 |
src/vs/workbench/contrib/positronKeybindings/browser/positronKeybindings.contribution.ts
Outdated
Show resolved
Hide resolved
src/vs/workbench/contrib/positronKeybindings/browser/positronKeybindings.contribution.ts
Show resolved
Hide resolved
ContextKeyExpr.not('findInputFocussed'), | ||
ContextKeyExpr.not('replaceInputFocussed') | ||
), | ||
primary: KeyMod.CtrlCmd | KeyMod.Shift | KeyCode.Enter |
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.
same question here
positron/extensions/positron-rstudio-keymap/package.json
Lines 130 to 132 in 64988df
"mac": "ctrl+shift+enter", | |
"when": "config.rstudio.keymap.enable && editorTextFocus && editorLangId == quarto && !findInputFocussed && !replaceInputFocussed", | |
"command": "quarto.runCurrentCell" |
id: 'workbench.view.scm', | ||
weight: KeybindingWeight.WorkbenchContrib, | ||
primary: KeyMod.CtrlCmd | KeyMod.Alt | KeyCode.KeyM | ||
})); |
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.
WinCtrl here as well?
positron/extensions/positron-rstudio-keymap/package.json
Lines 199 to 204 in 64988df
"mac": "ctrl+alt+m", | |
"win": "ctrl+alt+m", | |
"linux": "ctrl+alt+m", | |
"key": "ctrl+alt+m", | |
"when": "config.rstudio.keymap.enable", | |
"command": "workbench.view.scm" |
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.
good call, updated!
…eybindings.contribution.ts Co-authored-by: Melissa Barca <[email protected]> Signed-off-by: Jonathan <[email protected]>
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.
LGTM!
Fixes an issue causing RStudio keybindings to show up in unwanted places. It turns out to be intractable to make extension keybindings work the way we want, so the fix is to delete the extension and move these keybindings into a new "Positron Keybindings" contrib, where we can avoid registering them entirely if they are not enabled.
A few notes:
CmdCtrl
resolves toCmd
on macOS andCtrl
elsewhere). In most cases I have tried to preserve the RStudio behavior by registering both, even though this pollutes the keymap more. (Confusingly enough the Ctrl key on macOS has the identifierWinCtrl
in VS Code.)Addresses #3993.
Release Notes
New Features
Bug Fixes
QA Notes
As noted in the setting, a restart is required to get these bindings to become active.