Skip to content
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.

Sign up for GitHub

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

Jump to bottom

'Notebook' commands should only show in command palette when you are in a notebook #95664

Closed
mjbvz opened this issue Apr 20, 2020 · 3 comments
Closed

'Notebook' commands should only show in command palette when you are in a notebook #95664

mjbvz opened this issue Apr 20, 2020 · 3 comments
Assignees
@rebornix @jrieken
Labels
notebook polish Cleanup and polish issue
Milestone
May 2020

Comments

@mjbvz
Copy link
Contributor

mjbvz commented Apr 20, 2020

Issue Type: Bug

Repro

  1. Open a js file
  2. In the command palette, search Notebook

Bug
A bunch of notebook specific commands are shown. Most start with Notebook: but there's also Insert code cell above and Insert markdown cell above

VS Code version: Code - Insiders 1.45.0-insider (a3083e5, 2020-04-17T08:08:39.508Z)
OS version: Darwin x64 19.4.0

System Info
Item Value
CPUs Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz (16 x 2400)
GPU Status 2d_canvas: enabled
flash_3d: enabled
flash_stage3d: enabled
flash_stage3d_baseline: enabled
gpu_compositing: enabled
metal: disabled_off
multiple_raster_threads: enabled_on
oop_rasterization: disabled_off
protected_video_decode: unavailable_off
rasterization: enabled
skia_renderer: disabled_off_ok
video_decode: enabled
viz_display_compositor: enabled_on
viz_hit_test_surface_layer: disabled_off_ok
webgl: enabled
webgl2: enabled
Load (avg) 1, 2, 2
Memory (System) 32.00GB (7.92GB free)
Process Argv -psn_0_14245269
Screen Reader no
VM 14%
@rebornix
Copy link
Member

@jrieken currently with the new action registration, we will always populate the command in command palette if f1 is set to true due to

vscode/src/vs/platform/actions/common/actions.ts

Lines 444 to 446 in 93958d9

if (f1) {
disposables.add(MenuRegistry.appendMenuItem(MenuId.CommandPalette, { command: command }));
}

To fix this, we can use precondition to control if one action should be populated to the command palette.

@rebornix rebornix added notebook polish Cleanup and polish issue labels Apr 29, 2020
@jrieken
Copy link
Member

jrieken commented Apr 30, 2020

Yes - precondition ftw. This also helps with unwanted keybindings

@jrieken jrieken added this to the May 2020 milestone Apr 30, 2020
@rebornix
Copy link
Member

@jrieken as discussed above, precondition will be used as menu when clause in the command palette, if the action is registered through registerAction2. I'm not sure if we have a debt item for registerAction but we may want to highlight that how precondition take effects.

@github-actions github-actions bot locked and limited conversation to collaborators Jun 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@rebornix rebornix

@jrieken jrieken

Labels
notebook polish Cleanup and polish issue
Projects
None yet
Milestone
May 2020
Development

No branches or pull requests
3 participants
@rebornix @jrieken @mjbvz