Select All keyboard shortcut does not work in integrated terminal find widget

[mjbvz]

Testing #29479

• VSCode Version: Code - Insiders 1.14.0-insider (63def6e, 2017-06-28T05:07:46.148Z)
• OS Version: Darwin x64 16.0.0
• Extensions:

Steps to Reproduce:

1. Open integrated terminal find widget and type in some text
2. Try using the standard system select all keyboard shortcut (cmda on mac)

Bug
The contents of the terminal are selected instead of the contents of the find widget input box. This is different than how the editor find widget works

[cleidigh]

@mjbvz
@Tyriar
I will pick off this one next ...

I'm assuming this is an OS X only issue - Windows works and I reproduced the issue on OS X I don't see any comments on Linux
I've been trying to find the demultiplexing point for keyboard events and context evaluation for some time
for several other issues with no luck, can either of you point me to where this happens? it sounds to me
that somehow on OS X the keyboard event is being evaluated in the terminal context instead of the find widget context

[Tyriar]

Yes this one's OSX only and it's related to the context keys (or lack of context keys) used in the keybinding, feel free to look for a different one as OSX is a hassle to setup for you.

[cleidigh]

@Tyriar
it will take me a bit longer on OS X, but the core of the issue is of interest to me because
it relates to the processing flow for keyboard / context keys that affects so many other issues.
can you tell me where in the event chain the context keys get looked at, I've had trouble finding this.

[Tyriar]

It's happening because of this default keybinding on macOS:

{ "key": "cmd+a",                 "command": "workbench.action.terminal.selectAll",
                                     "when": "terminalFocus" },

Which is defined here: https://github.com/Microsoft/vscode/blob/master/src/vs/workbench/parts/terminal/electron-browser/terminal.contribution.ts#L245

Currently the terminal has 2 context keys:

• terminalFindWidgetVisible: When the terminal is visible (regardless of terminal/find focus)
• terminalFocus: If the terminal or the find widget input is focused

So what's happening is that the keybinding system sees that the terminalFocus context is active so it performs the workbench.action.terminal.selectAll command even when the find widget is focused, instead of skipping the keybinding system and performing the default input behavior which is to select all.

I suggest looking into how this works in the editor, we want a context key setup as that since it's tried and tested.

[cleidigh]

@Tyriar
Thanks !
I'll do some more code research

[cleidigh]

@Tyriar
I have a working fix for the Mac SelectAll bug.
I did not do a PR yet, but posted to my repo so you can assess if this is what you want.
If so I will cleanup, remove dead code and do a PR.

master...cleidigh:terminal-mac-selectall/bug

Changes and issue summary:

• added find terminal widget context
• added Escape handling inside terminal widget keyboard handler
• did above because cannot have second binding for the two different contexts
• hide widget from terminal context done by normal key binding
• if we had ContextKeyExpr.or() function could do this with one key binding
  Is this function worthwhile to add?
• compiled and tested on both Windows and OS X

BTW I looked at your cleanup changes for the terminal Queue
I will try to keep account of this type of cleanup for the future
I see you don't like abbreviations like "Q", noted.
Hopefully you will have to do little to no cleanup for my subsequent PRs !

Cheers

[cleidigh]

@Tyriar
Oh well, beaten to the punch...
The OS X select all got fixed in #30016 - you can close this out.
I have to look for something new to do, if any of the help items are highest priority (and no one's working on them already ;-) let me know. I'm free and looking for work ! I'm also posting the problems I saw.

[Tyriar]

@cleidigh a bunch of team members actually jumped on terminal issues on Monday so a lot of the easier ones are taken 😛

Here's the backlog: https://github.com/Microsoft/vscode/issues?q=is%3Aopen+label%3Aintegrated-terminal+label%3A%22help+wanted%22

There's a bunch of issues in the upstream project too if you're interested in contributing there https://github.com/sourcelair/xterm.js/issues

[cleidigh]

@Tyriar
just wanted to reiterate I tested this on OS X and select all is fixed.
you can close this issue referencing PR #30016

[Tyriar]

Thanks @cleidigh