microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
164.04k stars 29.21k forks source link

Key binding on view item command doesn't provide the view item #72442

Open eamodio opened 5 years ago

eamodio commented 5 years ago

Issue Type: Bug

This is easily reproducible using the tree-view-sample repo.

  1. Add a key binding for nodeDependencies.editEntry
  2. Open the package explorer view and click on one of the nodes
  3. 🐛 execute the key binding and notice the command fails (while if you click the pencil icon it will work fine)

When the command gets executed from the key binding it isn't getting the proper context. This breaks most key bindings for commands on view items.

/cc @jrieken

VS Code version: Code 1.33.1 (51b0b28134d51361cf996d2f0a1c698247aeabd8, 2019-04-11T08:27:14.102Z) OS version: Windows_NT x64 10.0.18362

System Info |Item|Value| |---|---| |CPUs|Intel(R) Core(TM) i7-6700K CPU @ 4.00GHz (8 x 4008)| |GPU Status|2d_canvas: enabled
checker_imaging: disabled_off
flash_3d: enabled
flash_stage3d: enabled
flash_stage3d_baseline: enabled
gpu_compositing: enabled
multiple_raster_threads: enabled_on
native_gpu_memory_buffers: disabled_software
rasterization: enabled
surface_synchronization: enabled_on
video_decode: enabled
webgl: enabled
webgl2: enabled| |Memory (System)|31.93GB (15.81GB free)| |Process Argv|| |Screen Reader|no| |VM|0%|
phmngocnghia commented 4 years ago

Any news on this? I Hope to see this on a near future mile-stone. This have been in On deck for too long...

katelynienaber commented 4 years ago

This is not working for me as well, when trying to set a keybinding for a command on a particular node.contextValue, ie:

      {
        "command": "custom.extension.command",
        "key": "ctrl+alt+z",
        "mac": "cmd+alt+z",
        "when": "sideBarFocus && focusedView == custom.view && viewItem =~ /nodeContextHere/"
      },

But this works if I remove && viewItem =~ /contextHere/

eamodio commented 3 years ago

🤣 I was about to file this issue again and found that I already had. This is very problematic for extensions wanting to allow key bindings tree items. And it is likely confusing to users -- because if they set a key binding for a command that is in a tree item's context menu, then the shortcut key shows up in the context menu, but it won't work -- because the command will get executed without any context.

eamodio commented 3 years ago

@alexr00 can you point to the likely place where changes would need to be made to make this happen?

Although, I this problem is bigger than just contributed tree view -- our built-in trees (explorer, scm, etc) all have the same issue

/cc @alexdima

alexr00 commented 3 years ago

I'm not sure where the changes would need to be made. Maybe @alexdima or @joaomoreno knows.

joaomoreno commented 3 years ago

When the command gets executed from the key binding it isn't getting the proper context.

What the the expected context? That somehow viewItem is the selected/focused tree item? If so, the custom tree view needs to create a scoped context key service in which it sets the viewItem context as the tree selection/focus changes. That way, the context will be set when the user invokes the keybinding and has DOM focus inside the custom tree view DOM element.

alexr00 commented 3 years ago

Thanks @joaomoreno!

eamodio commented 3 years ago

@joaomoreno is this something the list widget could do itself? Since this issue affects all our trees, whether custom or not. (Though I guess the actual context is different for each)

joaomoreno commented 3 years ago

Yes. We could potentially have a sort of IContextKeysProvider feature at the Workbench(List|Tree) level which would do this for you. Something like:

interface IContextKeysProvider<T> {
    getContextKeys(element: T) { key: string, value: string }[];
}

Or so.

eamodio commented 3 years ago

@joaomoreno who should own that? Should we turn this issue into that?

joaomoreno commented 3 years ago

I wouldn't mind a contribution! ;)