Ensure our contributed notebook keybindings respect jupyter.enableKeyboardShortcuts setting
#6293
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rchiodo
reviewed
Jun 15, 2021
| @@ -174,22 +174,22 @@ | |||
| }, | |||
| { | |||
| "key": "ctrl+enter", | |||
| "when": "editorTextFocus && inputFocus && notebookEditorFocused && notebookType == jupyter-notebook && config.jupyter.enableKeyboardShortcuts", | |||
| "when": "editorTextFocus && inputFocus && notebookEditorFocused && notebookType == jupyter-notebook && config.jupyter.enableKeyboardShortcuts || notebookCellListFocused && notebookCellType == 'code' && notebookType == jupyter-notebook && config.jupyter.enableKeyboardShortcuts", | |||
There was a problem hiding this comment.
What's the priority for an || statement? There's no parenthesis here so not sure if the or applies to just the two items next to it or the whole statement after?
There was a problem hiding this comment.
I don't believe parentheses are supported in when clauses. At least I tried the following and it didn't work:
"when": "((editorTextFocus && inputFocus && notebookEditorFocused) || notebookCellListFocused && notebookCellType == 'code')) && notebookType = jupyter-notebook && config.jupyter.enableKeyboardShortcuts"
The priority is not documented anywhere on this page: https://code.visualstudio.com/api/references/when-clause-contexts, but I would be very surprised if && didn't trump || in precedence.
There was a problem hiding this comment.
I believe this is the spot for parsing when expressions. Looks like VS Code splits on '||', then '&&', so unless I'm misreading and this code isn't the right spot, it seems like our when clauses should do what we expect https://github.com/microsoft/vscode/blob/5b73c885890e95af601df0f968194efeab1b70cd/src/vs/platform/contextkey/common/contextkey.ts#L128-L144
rchiodo
approved these changes
Jun 15, 2021
DavidKutu
approved these changes
Jun 15, 2021
IanMatthewHuff
approved these changes
Jun 15, 2021
Codecov Report
@@ Coverage Diff @@
## release-2021.06 #6293 +/- ##
=================================================
Coverage ? 72%
=================================================
Files ? 398
Lines ? 26909
Branches ? 3959
=================================================
Hits ? 19474
Misses ? 5809
Partials ? 1626 |
DavidKutu
pushed a commit
that referenced
this pull request
Jun 16, 2021
* release candidate (#6059) * release candidate * Memorial Day API changes (#6056) * update changelog Co-authored-by: Ian Huff <ianhu@microsoft.com> * Memorial Day API changes (#6056) * Send only error type in reason (#6067) * Fallback for sys.prefix not being returned by Python extension (#6053) * Fixes to toggling output (#6068) * Better telemetry when we don't find a matching kernel (#6060) * Better data when fail to find kernel connection (#6070) * change version and API port (#6085) * change version to 2021.6.x * match packagejson and changelog versions * 6/2 API Changes (#6089) Co-authored-by: Ian Huff <ianhu@microsoft.com> * Port #6025 to release (#6103) * Remove our Run Above/Below commands (#6026) * Update changelog * Fix pervasive test issue with editor properties (#6100) (#6106) * update version to 2021.6.99 to be able to filter (#6122) * update version to 2021.6.99 to be able to filter update changelog * update package lock and VSCode api * don * remove api changes in the code * Port Don's fixes (#6115) * Fix test failures resulting from VSCodes Notebook Start page (#6111) * Disable kernel auto startup in untrusted workspace (#6088) * Disable kernel auto startup in untrusted workspace * Fixes * Misc * oops * misc * Fixes to breaking tests (#6074) Co-authored-by: Don Jayamanne <don.jayamanne@outlook.com> * Breaking changes ports to release (2 commits) (#6140) * add component governance file (#6166) * add component governance file * remove npm components * Cherry pick changes from main branch into release (#6174) * final update (#6176) * final update * update changelog * add missing thanks to changelog (#6197) * Skip uploading vsix to azure blob store (#6204) * publish release * publish release * remove last 2 numbers from release (#6219) * publish release * Do not activate Python before opening nb (#6201) (#6230) * publish release * Port LiveKernelModel fix to release for point release (#6264) * Port keybinding updates to release (#6268) * Contribute shift+enter, ctrl+enter, L, shift+L (#6205) * Ctrl+Enter should put cell into command mode after executing (#6231) * Update CHANGELOG and remove news * port test fix (#6275) * Fix Restarting kernel... test (#6267) * wait for the restart command * add news file * update changelog * Fix native notebook interrupt toolbar (#6280) (#6282) * update changelog (#6284) * -update version -update changelog * leave version as it was * Respect jupyter.enableKeyboardShortcuts setting and enable ctrl+enter in command mode (#6293) * publish release * fix gulpfile * undo change Co-authored-by: Ian Huff <ianhu@microsoft.com> Co-authored-by: Don Jayamanne <don.jayamanne@yahoo.com> Co-authored-by: Don Jayamanne <don.jayamanne@outlook.com> Co-authored-by: Joyce Er <joyceerhuiling@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This also fixes the issue we found where ctrl+enter inserts a new cell below the current cell when in command mode. This was caused by a built-in keybinding:
Solution is to ensure our contributed ctrl+enter keybinding (for executing the current cell without advancing and leaving the cell in command mode) is active when the notebook cell list has focus, not just when a cell is in edit mode.
While looking at this I also found that the when clauses used for VS Code's ctrl+enter and shift+enter keybindings check the cell execution state and active kernel count. This didn't seem at all necessary to me so I removed those checks from the when clauses. Looking for feedback on whether that's a reasonable approach.
This also needs to be ported to main, but I'm creating this directly into the release branch first so as not to hold up the point release.