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.

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

Dvorak: Ctrl+] may not work correctly over the PTY #4885

Closed
jkuebart opened this issue Mar 11, 2020 · 27 comments
Closed

Dvorak: Ctrl+] may not work correctly over the PTY #4885

jkuebart opened this issue Mar 11, 2020 · 27 comments
Assignees
Labels
Area-Input Related to input processing (key presses, mouse, etc.) Issue-Question For questions or discussion Needs-Tag-Fix Doesn't match tag requirements Product-Conpty For console issues specifically related to conpty Resolution-Answered Related to questions that have been answered
Milestone

Comments

@jkuebart
Copy link

jkuebart commented Mar 11, 2020

Environment

Windows build number: Microsoft Windows [Version 10.0.18363.592]
Windows Terminal version (if applicable): 0.9.433.0
WSL: Linux yacht 4.4.0-18362-Microsoft #476-Microsoft Fri Nov 01 16:53:00 PST 2019 x86_64 x86_64 x86_64 GNU/Linux
VIM - Vi IMproved 7.4 (2013 Aug 10, compiled Jun 07 2019 15:35:43)

Steps to reproduce

Start a Linux shell in Terminal using the default profile (»Ubuntu«). Open vi and type »:help help«. This opens a help window. Within the help window, move the cursor over one of the green words, such as »helplang« and press Ctrl-].

Expected behavior

The help window should update to show the help text for »helplang«. The Ctrl-] shortcut is used in vi to navigate to »tags« – these can be program code (created using ctags) or hyperlinks in the help system.

Actual behavior

The shortcut is ignored. Nothing happens. Unbinding ctrl+] does not help. There does not appear to be a way to navigate to tags in vi in Terminal.

If you followed the steps to reproduce you're going to need this.

@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Mar 11, 2020
@j4james
Copy link
Collaborator

j4james commented Mar 12, 2020

I suspect you're using a German keyboard (or at least something different from a typical US layout), and the Windows Terminal key handling just doesn't deal with that very well. You may find you can get the equivalent of Ctrl+] by using Ctrl++ (or whatever key is just to the left of your Return key). Not that I'm suggesting that's a real solution to the problem, but it may help you get by in the meantime.

I think there is probably an existing issue for this somewhere. Not necessarily the same key combo or language, but I'm guessing these bugs all have the same root cause.

@jkuebart
Copy link
Author

@j4james you're right, I use US Dvorak layout and when I switch it to US English the keys start working. However, I didn't find an alternate combination that works in Dvorak mode, and switching keyboard layouts each time is a little inconvenient… ;)

@DHowett-MSFT
Copy link
Contributor

Puzzlingly, I can't even get putty to work right with a German keyboard layout. I'm pressing LCtrl+AltGr+9 and just getting ] instead of ^]; this also seems to hold true for conhost, so it doesn't look like a pseudoconsole bug.

@DHowett-MSFT
Copy link
Contributor

(I also tried RCtrl, just to make sure I wasn't messing up AltGr)

@jkuebart
Copy link
Author

@DHowett-MSFT as far as I recall, it was never possible to exit telnet on Windows with a German keyboard layout. This is due to the problem that Ctrl + Alt is an alternative combination to AltGr, possibly for keyboards lacking the latter. According to rumours, Ctrl + + is supposed to work for telnet.

On the Dvorak layout, however, ] is an unshifted, unmodified key just like on the standard US layout, if in a different position. That's why I'm surprised that it also causes problems.

@DHowett-MSFT
Copy link
Contributor

So, here's something I just realized: the pseudoconsole does key-to-event translation based entirely on the main system keyboard (not the active one!). I'm not sure how we can do translation based on the active one.

Is dvorak your primary or secondary?

@DHowett-MSFT
Copy link
Contributor

(When I say I'm not sure how we can, I mean "how we can contribute that information to a pseudoconsole session, because it could be on a remote system for all we know)

@DHowett-MSFT DHowett-MSFT added Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something Product-Conpty For console issues specifically related to conpty Issue-Bug It either shouldn't be doing this or needs an investigation. Area-Input Related to input processing (key presses, mouse, etc.) labels Mar 24, 2020
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Mar 24, 2020
@jkuebart
Copy link
Author

jkuebart commented Mar 24, 2020

@DHowett-MSFT interesting question: it was in fact my secondary layout, so I removed the standard US layout and made Dvorak my only layout – the problem remains! Neither Ctrl+] nor Ctrl+= or Ctrl+- (keys to the left to enter) have the effect expected for Ctrl+]. As mentioned above, when I switch to standard US layout, I can use Ctrl+] normally.

As for your concern about input from a remote system: from my understanding of the ConPTY architecture, this really shouldn't matter. All that's transmitted to and from ConPTY, whether over the network or locally, is UTF-8 encoded »text/VT« and therefore keyboard layout agnostic.

It's the terminal app's job (on the left in the linked illustration) to convert INPUT_RECORD, KEY_EVENT_RECORD and whatnot to »text/VT« and send it to its PTY, and it should naturally use the user's currently active keyboard layout of the machine on which the terminal app is running (ignoring cases like RDP, for the moment).

If the other end of the PTY (locally or remotely, on the right in the linked illustration) is connected to a legacy ConDrv app, the ConHost on that side needs to translate »text/VT« back to INPUT_RECORD, KEY_EVENT_RECORD &c. In my opinion, it should reasonably assume that the legacy app is going to interpret those events based on the active keyboard layout on its machine. This is especially true since by design a legacy app has no knowledge that its input came through a PTY. But that issue is between ConHost and the legacy app, meaning that the terminal app doesn't need to concern itself with – if fact it can't, because the other side of its PTY could be connected to a non-Windows system for all it knows, where the concept of Windows keyboard layouts doesn't apply.

@ghost ghost added Needs-Attention The core contributors need to come back around and look at this ASAP. and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Mar 24, 2020
@DHowett-MSFT
Copy link
Contributor

Frg tbr, ,dayw C

@DHowett-MSFT
Copy link
Contributor

As you can likely tell, I've been installing different keyboard layouts. Okay, with that out of the way...

I bet, or at least hope quite a bit, that this is working for you with Terminal v0.10. I kept trying to reproduce the issue, and I just kept coming up blank.

We made some changes to ConPTY in v0.10 so that input sent into the pty gets passed through directly to the client application (as it should be!) without translation when the receiving application is expecting VT input.

@DHowett-MSFT
Copy link
Contributor

DHowett-MSFT commented Mar 27, 2020

You're right about our architecture, though: the onus of translating ^] back into a reasonable representation for the client application lies at the far end of the pty. There's a minor complication in conhost, in that up until WT 0.10 a key event took a trip:

User (Win32 input stack)
   | Key Event RBRACK+CTRL
   |
   v
Terminal --(^])--> Console host (pty in)
                     | Key Event RBRACK+CTRL
                     |
                     v
                     Console host (vt out) --(???)--> wsl -> vi
(0.10 is more direct)
User (Win32 input stack)
   | Key Event RBRACK+CTRL
   |
   v
Terminal --(^])------\
                     |
                     |
                     v
                     Console host (vt out) -> wsl -> vi

I can't get the issue to repro even in a case that does require back-translation, though, with either the old version of the PTY or the console host itself in VT input mode even with Dvorak as my primary. That's curious.

If it still reproduces for you, can you run showkey -a in WSL in Terminal and share the output when you mash ^]?

Thanks.

@DHowett-MSFT DHowett-MSFT added Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something Needs-Repro We can't figure out how to make this happen. Please help find a simplified repro. and removed Needs-Attention The core contributors need to come back around and look at this ASAP. labels Mar 27, 2020
@DHowett-MSFT DHowett-MSFT added this to the Terminal v1.x milestone Mar 27, 2020
@DHowett-MSFT DHowett-MSFT changed the title WSL: Ctrl-] does not work in vi Dvorak: Ctrl+] may not work correctly over the PTY Mar 27, 2020
@jkuebart
Copy link
Author

@DHowett-MSFT in the meantime, my Windows Terminal has updated itself to 0.10.781.0.

showkey -a is an interesting experiment. With a standard US layout, I get the following output for the sequence Ctrl+[, Ctrl+], Ctrl+Shift+6 (after setting keybindings for ctrl+shift+6 or equivalently ctrl+^ to unbound):

$ showkey -a

Press any keys - Ctrl-D will terminate this program

^[       27 0033 0x1b
^]       29 0035 0x1d
^^       30 0036 0x1e

However, I can't manage to get any output for Ctrl+- and Ctrl+=, even if I set their keybindings to unbound or null – they always control the zoom factor. So somehow these keys are treated specially.

With a US Dvorak layout, none of Ctrl+[, Ctrl+], Ctrl+Shift+6 produce any output from showkey -a. [ and ] are in the locations of - and = on the US keyboard, but they don't control the zoom factor either (the zoom factor is correctly adjusted by Ctrl+- and Ctrl++ in their Dvorak locations). All of these key combinations are simply dead.

Please let me know if there is any more testing I can do to help.

Note: while playing around with showkey, I noticed that both in US and Dvorak layouts Ctrl+5 and Ctrl+6 produce ^] and ^^, respectively – while this offers a workaround, it's not really a solution and I think unrelated to this issue.

@ghost ghost added Needs-Attention The core contributors need to come back around and look at this ASAP. and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Mar 27, 2020
@DHowett-MSFT
Copy link
Contributor

I'm willing to bet that this was fixed with #4192, but I'll leave it open and assigned to me, off the triage queue, until we can confirm (once 0.11 is out)

@DHowett-MSFT DHowett-MSFT removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Apr 8, 2020
@DHowett-MSFT DHowett-MSFT self-assigned this Apr 8, 2020
@jkuebart
Copy link
Author

jkuebart commented Apr 9, 2020

@DHowett-MSFT wow, that looks like a substantial refactoring – any timeline for 0.11?

@DHowett-MSFT
Copy link
Contributor

"Soon." If you're willing to test out a preflight build, would you mind e-mailing me (address is in my profile)? I'd love to see if this helps.

@DHowett-MSFT
Copy link
Contributor

(It'll be code-signed by Microsoft, no funny business here 😁)

@jkuebart
Copy link
Author

@DHowett-MSFT Hi Dustin, I ran a build and made it contained the changes from #4192.

Sadly, I must report that the behaviour I described above is unchanged – meaning I don't get any output from showkey -a for Ctrl+[, Ctrl+] and Ctrl+Shift+6 when using US Dvorak layout.

I guess now that I have the source code I could just put a breakpoint in and start debugging. Alas, today I'm too busy with other stuff. But if you have some specific question I'm happy to give it a shot!

PS: I was very impressed how easy it was to set up the build – kudos, most builds I've seen have been much more effort to get running ;-)

@DHowett-MSFT
Copy link
Contributor

Really sorry it didn't work out! I'm glad it was easy to build, however. I'd be very interested to see what's coming in on TermControl.cpp's _KeyDownHandler and _CharacterHandler...

If you have a debug build going, you can hold down both alt keys when clicking on a profile menu item to get a tap that will show, in red, any user input.

That'll help us determine, at least, if it's making it out of Terminal alright. :)

image

@jkuebart
Copy link
Author

Hi Dustin, no need to be sorry, it was worth a shot!

I'm currently at ea1bb2e. I activated the »raw« window and with Dvorak layout active, there is no red output (there is with US layout).

These are the results of breakpoints at the call to _TrySendKeyEvent() in _KeyDownHandler() and SendCharEvent() in _CharacterHandler() with US layout.

Keys vkey scanCode modifiers ch
Ctrl+[ db 1a 8 ^[
Ctrl+] dd 1b 8 ^]
Ctrl+Shift+6 '6' 7 18 ^^

The call to bindings.TryKeyChord() in _KeyDownHandler() returns false. The call to e.Handled() is called with false in _KeyDownHandler() and true in _CharacterHandler().

Here's the same for Dvorak layout:

Keys vkey scanCode modifiers ch
Ctrl+[ db c 8
Ctrl+] dd d 8
Ctrl+Shift+6 '6' 7 18

Most notably, there is no call to _CharacterHandler() at all.

Other than that, everything in _KeyDownHandler() appears to be the same, particularly bindings.TryKeyChord() returns false and e.Handled() is called with false at the very end. The scanCode is different where different physical keys are involved, as is to be expected.

I don't have time now to trace where the call to _CharacterHandler() is lost – returning from _KeyDownHandler() ends up in heavily templated code ;-) But I thought maybe these insights can already shed some light.

@DHowett-MSFT
Copy link
Contributor

It's really quite strange that ^[ and ^] are lost in the input stack between KeyDown and CharacterReceived... I'm still not sure what to do with this, but it's starting to look like it's not something Terminal can really help with. I'll see what I can find about Xaml input debugging 😄

@DHowett-MSFT
Copy link
Contributor

So, here's a question...

You've got a tool called Spy++, which came with Visual Studio. My copy is at c:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\common7\tools\spyxx_amd64.exe.

Can you launch it, and use the Find tool to find a Terminal window? Drag the crosshairs in the dialog over the edges of the terminal window until you find the CASCADIA_HOSTING_WINDOW_CLASS:

image

image

image

Click [OK], and it should open up that window in the tree view.
Right-click on the "InputSite" window:

image

and choose "Messages".

Then, go and press one of ^[ or ^] over in terminal. Here's what I get when I do that:

<000027> 00000000002107F8 P WM_KEYDOWN nVirtKey:VK_CONTROL cRepeat:1 ScanCode:1D fExtended:0 fAltDown:0 fRepeat:1 fUp:0
<000028> 00000000002107F8 P WM_KEYDOWN nVirtKey:VK_OEM_6 cRepeat:1 ScanCode:0D fExtended:0 fAltDown:0 fRepeat:0 fUp:0
<000029> 00000000002107F8 P WM_CHAR chCharCode:'29' (29) cRepeat:1 ScanCode:0D fExtended:0 fAltDown:0 fRepeat:0 fUp:0
<000030> 00000000002107F8 P WM_KEYUP nVirtKey:VK_OEM_6 cRepeat:1 ScanCode:0D fExtended:0 fAltDown:0 fRepeat:1 fUp:1
<000031> 00000000002107F8 P WM_KEYUP nVirtKey:VK_CONTROL cRepeat:1 ScanCode:1D fExtended:0 fAltDown:0 fRepeat:1 fUp:1

the WM_CHAR there in the middle is win32/win32k generating the thing that should trigger "CharacterHandler" down in Xaml land. This is the earliest possible place we could see this message getting generated.

@DHowett-MSFT DHowett-MSFT added Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something and removed Needs-Attention The core contributors need to come back around and look at this ASAP. labels Apr 22, 2020
@DHowett-MSFT
Copy link
Contributor

Oh: to get the log out of the messages window I had to go to the Messages menu and choose "save log file" ☹️ it is not intuitive and it does not support copy/paste.

@jkuebart
Copy link
Author

jkuebart commented Apr 25, 2020

@DHowett-MSFT Hi Dustin, here are the Sky++ key logs: I verified I get the same as you with a US layout. With Dvorak, it's the same keys (VM_OEM_6), but no WM_CHAR. Mean!

Ctrl+[

<000005> 00000000001902AC P WM_KEYDOWN nVirtKey:VK_CONTROL cRepeat:1 ScanCode:1D fExtended:0 fAltDown:0 fRepeat:0 fUp:0
<000006> 00000000001902AC P WM_KEYDOWN nVirtKey:VK_OEM_4 cRepeat:1 ScanCode:0C fExtended:0 fAltDown:0 fRepeat:0 fUp:0
<000007> 00000000001902AC P WM_KEYUP nVirtKey:VK_OEM_4 cRepeat:1 ScanCode:0C fExtended:0 fAltDown:0 fRepeat:1 fUp:1
<000008> 00000000001902AC P WM_KEYUP nVirtKey:VK_CONTROL cRepeat:1 ScanCode:1D fExtended:0 fAltDown:0 fRepeat:1 fUp:1

Ctrl+]

<000009> 00000000001902AC P WM_KEYDOWN nVirtKey:VK_CONTROL cRepeat:1 ScanCode:1D fExtended:0 fAltDown:0 fRepeat:0 fUp:0
<000010> 00000000001902AC P WM_KEYDOWN nVirtKey:VK_OEM_6 cRepeat:1 ScanCode:0D fExtended:0 fAltDown:0 fRepeat:0 fUp:0
<000011> 00000000001902AC P WM_KEYUP nVirtKey:VK_OEM_6 cRepeat:1 ScanCode:0D fExtended:0 fAltDown:0 fRepeat:1 fUp:1
<000012> 00000000001902AC P WM_KEYUP nVirtKey:VK_CONTROL cRepeat:1 ScanCode:1D fExtended:0 fAltDown:0 fRepeat:1 fUp:1

Ctrl+Shift+6

<000013> 00000000001902AC P WM_KEYDOWN nVirtKey:VK_CONTROL cRepeat:1 ScanCode:1D fExtended:0 fAltDown:0 fRepeat:0 fUp:0
<000014> 00000000001902AC P WM_KEYDOWN nVirtKey:VK_SHIFT cRepeat:1 ScanCode:2A fExtended:0 fAltDown:0 fRepeat:0 fUp:0
<000015> 00000000001902AC P WM_KEYDOWN nVirtKey:'6' cRepeat:1 ScanCode:07 fExtended:0 fAltDown:0 fRepeat:0 fUp:0
<000016> 00000000001902AC P WM_KEYUP nVirtKey:'6' cRepeat:1 ScanCode:07 fExtended:0 fAltDown:0 fRepeat:1 fUp:1
<000017> 00000000001902AC P WM_KEYUP nVirtKey:VK_SHIFT cRepeat:1 ScanCode:2A fExtended:0 fAltDown:0 fRepeat:1 fUp:1
<000018> 00000000001902AC P WM_KEYUP nVirtKey:VK_CONTROL cRepeat:1 ScanCode:1D fExtended:0 fAltDown:0 fRepeat:1 fUp:1

I have to add that in regular console windows, these key combinations do work, even though they also don't get a WM_CHAR (I checked).

Out of curiosity, I tried the keys without modifier and then they do generate WM_CHAR:
[

<000010> 00000000001902AC P WM_KEYDOWN nVirtKey:VK_OEM_4 cRepeat:1 ScanCode:0C fExtended:0 fAltDown:0 fRepeat:0 fUp:0
<000011> 00000000001902AC P WM_CHAR chCharCode:'91' (91) cRepeat:1 ScanCode:0C fExtended:0 fAltDown:0 fRepeat:0 fUp:0
<000012> 00000000001902AC P WM_KEYUP nVirtKey:VK_OEM_4 cRepeat:1 ScanCode:0C fExtended:0 fAltDown:0 fRepeat:1 fUp:1

]

<000013> 00000000001902AC P WM_KEYDOWN nVirtKey:VK_OEM_6 cRepeat:1 ScanCode:0D fExtended:0 fAltDown:0 fRepeat:0 fUp:0
<000014> 00000000001902AC P WM_CHAR chCharCode:'93' (93) cRepeat:1 ScanCode:0D fExtended:0 fAltDown:0 fRepeat:0 fUp:0
<000015> 00000000001902AC P WM_KEYUP nVirtKey:VK_OEM_6 cRepeat:1 ScanCode:0D fExtended:0 fAltDown:0 fRepeat:1 fUp:1

@ghost ghost added Needs-Attention The core contributors need to come back around and look at this ASAP. and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Apr 25, 2020
@jkuebart
Copy link
Author

@DHowett-MSFT Hi Dustin, the above investigation into WM_CHAR messages eventually lead to me figuring out what was wrong: I diligently compared my keyboard layout to the US International layout using a keyboard layout disassembler and found out that Terminal works fine with the latter because it contains explicit character codes for ^@, ^[, ^\, ^], ^^ and ^_.

As it turns out, the Windows keyboard system generates control character WM_CHAR messages automatically for ^A-^Z even when the keyboard layout contains no mappings. However, this doesn't happen for the six remaining ones listed above. That's the reason Terminal saw no WM_CHAR messages.

What made it confusing was that these key combinations do work in the normal Windows console. Since it also doesn't get any WM_CHAR messages (I checked), it must synthesize those control characters from the WM_KEYDOWN/WM_KEYUP messages by itself.

TL;DR
This issue can be closed. The keyboard layout was »broken« even though I never noticed it in over ten years of daily use. Whether the difference between console and Terminal warrants some special code is a different decision – likely somebody will run into a similar issue eventually, but with a bit of luck they might find this ticket and find out what's wrong ;-)

Thanks for your patience and help, if you have any more questions or debugging requests I'll be happy to respond.

@DHowett-MSFT
Copy link
Contributor

Wow! Thanks so much for digging in.

Which DVORAK layout have you been using? I was testing with the one that ships inbox with Windows ... 😉

@DHowett-MSFT DHowett-MSFT added Issue-Question For questions or discussion Resolution-Answered Related to questions that have been answered and removed Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Attention The core contributors need to come back around and look at this ASAP. Needs-Repro We can't figure out how to make this happen. Please help find a simplified repro. labels Apr 27, 2020
@ghost ghost added the Needs-Tag-Fix Doesn't match tag requirements label Apr 27, 2020
@jkuebart
Copy link
Author

Which DVORAK layout have you been using? I was testing with the one that ships inbox with Windows ... 😉

To be honest, it only dawned on me after sleeping over the missing MW_CHAR messages that I had created my own layout years ago using MSKLC – it contains non-ASCII characters in the AltGr positions similar to Microsoft's »US International« layout or the Dvorak layout shipped by some fruit-based company. Unfortunately, Microsoft's Dvorak is a little 1932 and really only usable for 7-bit ASCII ;-)

@DHowett-MSFT
Copy link
Contributor

Huh! That makes sense.

Thanks again -- this has been enlightening (and perhaps is worthy of a feature request to the input folks to improve the inbox Dvorak layout ... I'll follow up with that 😄)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Input Related to input processing (key presses, mouse, etc.) Issue-Question For questions or discussion Needs-Tag-Fix Doesn't match tag requirements Product-Conpty For console issues specifically related to conpty Resolution-Answered Related to questions that have been answered
Projects
None yet
Development

No branches or pull requests

3 participants