Send a KeyUp sequence only once a key has been released

[j4james]

When win32-input-mode is enabled, we generate an input sequence for both
key down and key up events. However, in the initial implementation the
key up sequence for most keypresses would be faked - they were generated
at the same time as the key down sequence, regardless of when the key
was actually released. After this PR, we'll only generate the key up
sequence once a key has actually been released.

References and Relevant Issues

The initial implementation of win32-input-mode was in PR #6309.
The spec for win32-input-mode was in PR #5887.

Validation Steps Performed

I've manually tested this with an app that polls ReadConsoleInput in a
loop and logs the results. With this PR applied, I can now see the key
up events as a key is released, rather than when it was first pressed.

When compared with conhost, though, there are some differences. When
typing a shifted key, e.g. Shift+A, WT generates key down and key up
events for both the Shift and the A, while conhost only generates
both events for the Shift - the A won't generate a key up event
unless you release the Shift before the A. That seems more like a
conhost flaw though.

Another case I tested was the Japanese Microsoft IME, which in conhost
will generate a key down event for the Japanese character being inserted
followed by a key up event for for Return. WT generates key up events
for the ASCII characters being typed in the IME, then both a key down
and key up event for the inserted Japanese character, and finally a key
up event for Return. Both of those seem weird, but they still appear
to work OK.

The current version of WT actually produces the most sensible behavior
for the IME - it just generates key up and key down events for the
inserted character. But that's only because it's dropping most of the
system generated key up events.

Closes #8440

[lhecker]

This seems right to me, but does the test case in #4192 still work? That was the initial reason to return false there...

[DHowett]

To add to Leonard's question, does this have any impact on experimental.input.forceVT?

This is less about this PR, but I did think of it: f an application requests Win32 input mode from conhost and forceVT is on in Terminal, does conhost synthesize weird events? Does VT input passthrough via conpty work if Terminal is in win32 input mode and the client app requests win32 as well?

[j4james]

This seems right to me, but does the test case in #4192 still work?

@lhecker If you mean a custom keyboard layout with dead keys mapping to themselves, I have definitely not tried that. I suspect it might still work, because the key down events should still be the same. However, the key up events would assumedly now be different, but I'm not sure that matters, given the conhost already sends key up events in a somewhat random manner.

I have tried your _SendPastedTextToConnection hack, and that seemed to work, but that doesn't prove anything regarding the effects of the key up events.

To add to Leonard's question, does this have any impact on experimental.input.forceVT?

@DHowett I've just tested now, and it doesn't appear to affect that in any way.

This is less about this PR, but I did think of it: f an application requests Win32 input mode from conhost and forceVT is on in Terminal, does conhost synthesize weird events?

Yes. Basic printable characters seems reasonable, except obviously you get the key up events immediately, because it doesn't know any better. Things like arrow keys and function keys are weird, since they are received as a bunch of keypresses representing the VT escape sequence, which isn't ideal. But that's no different from how it worked before.

Does VT input passthrough via conpty work if Terminal is in win32 input mode and the client app requests win32 as well?

I'm not sure what you're asking here. If you haven't set the forceVT option, and an app has requested win32-input-mode (i.e. ?9001), that does definitely work. And those apps will also now see the key up events when the key is actually released.

[j4james]

I've tested the custom AZERTY-NF keyboard now, and the AltGr+N,O test is still working as expected. In fact, looking at the events being generated, I think it's actually more correct than it was before.

In the tables below I've listed what is seen by ReadConsoleInput (columns on the left) and the VT win32-input-mode (second from right) when this key sequence is pressed.

In the current WT implementation, you don't see the dead N key at all, while the new implementation does at least see the up event (conhost generates both down and up, but the down event has a null char).

Also in the current WT implementation, both the down and up events for the generated õ character have the same char value (õ), where as conhost and the new implementation give an unaccented o for the up event, which I think is technically more correct.

Conhost

VK	SC	Char	D/U	Win32 Input Mode	Keys
17	29	(U+0000)	DOWN	^[[17;29;0;1;8;1_	AltGr
18	56	(U+0000)	DOWN	^[[18;56;0;1;265;1_	AltGr
78	49	(U+0000)	DOWN	^[[78;49;0;1;9;1_	N
78	49	(U+0303)	UP	^[[78;49;771;0;9;1_	N
17	29	(U+0000)	UP	^[[17;29;0;0;1;1_	AltGr
18	56	(U+0000)	UP	^[[18;56;0;0;256;1_	AltGr
79	24	(U+00F5)	DOWN	^[[79;24;245;1;0;1_	O
79	24	(U+006F)	UP	^[[79;24;111;0;0;1_	O

Windows Terminal (current)

VK	SC	Char	D/U	Win32 Input Mode	Keys
17	29	(U+0000)	DOWN	^[[17;29;0;1;8;1_	AltGr
18	56	(U+0000)	DOWN	^[[18;56;0;1;265;1_	AltGr
17	29	(U+0000)	UP	^[[17;29;0;0;1;1_	AltGr
18	56	(U+0000)	UP	^[[18;56;0;0;0;1_	AltGr
79	24	(U+00F5)	DOWN	^[[79;24;245;1;0;1_	O
79	24	(U+00F5)	UP	^[[79;24;245;0;0;1_	O

Windows Terminal (this PR)

VK	SC	Char	D/U	Win32 Input Mode	Keys
17	29	(U+0000)	DOWN	^[[17;29;0;1;8;1_	AltGr
18	56	(U+0000)	DOWN	^[[18;56;0;1;265;1_	AltGr
78	49	(U+0303)	UP	^[[78;49;771;0;9;1_	N
17	29	(U+0000)	UP	^[[17;29;0;0;1;1_	AltGr
18	56	(U+0000)	UP	^[[18;56;0;0;0;1_	AltGr
79	24	(U+00F5)	DOWN	^[[79;24;245;1;0;1_	O
79	24	(U+006F)	UP	^[[79;24;111;0;0;1_	O

[DHowett]

Okay, here's a thought! If you have snapOnInput enabled and you Alt+Tab to the Terminal, does it snap because releasing Tab counts as input? Or have we already accounted for this.

Sorry, I could be testing this myself. Your build artifacts now contain an unpackaged distribution... just a sec

[DHowett]

Okay, no, it doesn't.

[j4james]

If you have snapOnInput enabled and you Alt+Tab to the Terminal, does it snap because releasing Tab counts as input?

Tab is actually one of the keys which already triggers the key-up event separately from the key-down, and it is possible for the key-up to be received when you Alt+Tab to the Terminal if you get your timing right (or wrong, depending on how you see it). However, as you noted, that doesn't seem to be enough to make it snap.

[DHowett]

The write-up is longer than the change, for once! I love it!

Thanks for doing this. We tested this in a team bug bash and couldn't figure out how to explode it, so I'm gonna call it good for 1.18 Preview :)

Thanks again for all thee verification and testing it with the IME, too.