User keybindings break if conditions on the default binding change

[akbyrd]

Issue Type: Bug

Before upgrading to 1.25 I had the following keybinding:

{
  "key": "ctrl+w",
  "command": "-workbench.action.closeWindow",
  "when": "!editorIsOpen"
}

This binding was generated by VS Code when I removed the default keybinding that I did not want.

After upgrading I find that Ctrl+W once again closes the window. It turns out the condition changed on the default binding, which breaks the disabled binding VS Code itself generated. So I manually updated it to match.

{
  "key": "ctrl+w",
  "command": "-workbench.action.closeWindow",
  "when": "!editorIsOpen && !multipleEditorGroups"
}

The issue here is that my keybindings should not break when upgrading VS Code. This is a workflow interruption and it makes me apprehensive about updating. I can imagine a number of solutions to this:

• Keybindings get a version number like tasks and can be auto-upgraded when changes occur.
• Keybindings that remove default bindings are stored differently such that they aren't so brittle.
• The "when" clause is left unset when VS Code generates a removed binding. This still appears to remove the binding and doesn't break as easily.

VS Code version: Code 1.25.0 (0f080e5, 2018-07-05T13:11:58.697Z)
OS version: Windows_NT x64 10.0.17134

System Info

Item	Value
CPUs	Intel(R) Core(TM) i7-7700HQ CPU @ 2.80GHz (8 x 2808)
GPU Status	2d_canvas: enabled 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: disabled_software video_decode: enabled video_encode: enabled vpx_decode: enabled webgl: enabled webgl2: enabled
Memory (System)	15.89GB (11.31GB free)
Process Argv	C:\Program Files\Microsoft VS Code\Code.exe
Screen Reader	no
VM	0%

Extensions (15)

Extension	Author (truncated)	Version
vscode-npm-script	eg2	0.3.5
tortoise-svn	fan	0.1.1
vsc-space-block-jumper	jmf	1.2.2
Lua	key	0.0.9
theme-monokai-pro-vscode	mon	1.1.8
python	ms-	2018.6.0
cpptools	ms-	0.17.6
csharp	ms-	1.15.2
Go	ms-	0.6.84
PowerShell	ms-	1.7.0
code-settings-sync	Sha	2.9.2
shader	sle	1.1.3
Align	ste	0.2.0
unity-debug	Uni	1.3.0
vscode-todo-highlight	way	0.5.12

(1 theme extensions excluded)

[akbyrd]

Also, by default, Ctrl+Down is bound to search.focus.nextInputBox which jumps focus to the results. Ctrl+Up is bound to search.action.focusSearchFromResults and only moves focus the first result is selected. That leads to a couple oddities:

• You can always get from the input box to the results with a single action, but to get back to the search box you have to navigate all the way back to the first result.
• The binding is asymmetric. Why is it search.focus.nextInputBox and search.action.focusSearchFromResults instead of search.focus.nextInputBox and search.focus.previousInputBox? It accomplishes exactly the same thing. In fact, I don't see a reason search.action.focusSearchFromResults should exist at all.
• Ctrl+Down is a 'jump focus' while Ctrl+Up is a 'move focus'. By move focus I mean move to an immediately adjacent piece of UI. This is a continuous move. By jump focus I mean move to an adjacent section of UI. This is a discontinuous move that may skip multiple pieces of UI.

If we accept the following goals:

• 'Move' focus changes and 'jump' focus changes should have separate bindings.
• You should be able to get back to the last focused spot using the opposite directional key. Up then Down should return to the same location. Ctrl+Up then Ctrl+Down should return to the same location.

Then it's clear we need to separate the current Ctrl+Up and Ctrl+Down into two separate sets Up/Down pairs: one for moving and one for jumping.

Up and Down are moves. Move smoothly between input boxes and results

{
  "key": "up",
  "command": "search.focus.previousInputBox",
  "when": "inputBoxFocus && searchViewletVisible"
},
{
  "key": "down",
  "command": "search.focus.nextInputBox",
  "when": "inputBoxFocus && searchViewletVisible"
},
//Ensure up on the first result moves to input boxes
{
  "key": "up",
  "command": "search.focus.previousInputBox",
  "when": "firstMatchFocus && searchViewletVisible"
},

Ctrl+Up and Ctrl+Down are jumps. Jump between sections (input boxes and results) without changing the selected result

{
  "key": "ctrl+up",
  "command": "search.focus.previousInputBox",
  "when": "searchViewletVisible"
},
{
  "key": "ctrl+down",
  "command": "search.focus.nextInputBox",
  "when": "searchViewletVisible"
},

and search.action.focusSearchFromResults deprecated.

EDIT
I'd also like to make a case against using Up/Down to navigate through search history. By and large the arrow keys are for movement. As a code editor, 99% of the time the arrow keys are moving a cursor (editors) or moving focus (explorer, bugs, extensions, menus, command palette, etc). Even in other search boxes like the keybindings editor and the new settings editor the arrow keys move out of the input box into the results. The current search panel behavior is an uncomfortable exception.

Ctrl+direction is often used for jumping focus in larger increments: jump cursor by word, jump between suggestions, jump between input boxes

Alt+direction seems to be a little more ad hoc. Sometimes it jumps, sometimes it navigates, sometimes it makes modifications. It generally seems to be the 'secondary action' modifier.

Therefore, it appears that Alt+Up/Down should be used for navigating through the search history, Up/Down should move to adjacent UI, and Ctrl+Up/Down should jump between regions.

[bpasero]

We should have documented this better in the release notes, thanks for clarifying this in case someone else runs into this issue. There is no intent to change this again but we should avoid these issues in the future if possible.

/cc @sandy081 I think #53730 (comment) is for you

[akbyrd]

@bpasero Should I open a separate issue for my second comment? Ctrl+Up and Ctrl+Down bindings in the search panel are...problematic.

[sandy081]

@akbyrd Thanks for your explanation about how navigation with up and down arrows with ctrl/alt modifiers.

Using up/down arrows for history navigation is the intuitive way (e.g., navigating commands in debug console) and also most users wanted #12645. Hence, we used up/down arrows consistently for navigating history across the product.

I am bit lost with overall description, So can you please file a separate issue for how you expect ctrl+up and ctrl+down keybindings should work in search view and what is missing in it?

Thanks