Refactor VT terminal input

[j4james]

The primary reason for this refactoring was to simplify the management
of VT input sequences that vary depending on modes, adding support for
the missing application keypad sequences, and preparing the way for
future extensions like S8C1T.

However, it also includes fixes for a number of keyboard related bugs,
including a variety of missing or incorrect mappings for the Ctrl and
Ctrl+Alt key combinations,

References and Relevant Issues

This PR also includes a fix for #10308, which was previously closed as a
duplicate of #10551. I don't think those bugs were related, though, and
although they're both supposed to be fixed in Windows 11, this PR fixes
the issue in Windows 10.

Detailed Description of the Pull Request / Additional comments

The way the input now works, there's a single keyboard map that takes a
virtual key code combined with Ctrl, Alt, and Shift modifier bits
as the lookup key, and the expected VT input sequence as the value. This
map is initially constructed at startup, and then regenerated whenever a
keyboard mode is changed.

This map takes care of the cursor keys, editing keys, function keys, and
keys like BkSp and Return which can be affected by mode changes. The
remaining "graphic" key combinations are determined manually at the time
of input.

The order of precedence looks like this:

1. If the virtual key is 0 or VK_PACKET, it's considered to be a
   synthesized keyboard event, and the UnicodeChar value is used
   exactly as given.

2. If it's a numeric keypad key, and Alt is pressed (but not Ctrl),
   then it's assumedly part of an Alt-Numpad composition, so the key
   press is ignored (the generated character will be transmitted when
   the Alt is released).

3. If the virtual key combined with modifier bits is found in the key
   map described above, then the matched escape sequence will be used
   used as the output.

4. If a UnicodeChar value has been provided, that will be used as the
   output, but possibly with additional Ctrl and Alt modifiers applied:

   a. If it's an AltGr key, and we've got either two Ctrl keys
   pressed or a left Ctrl key that is distinctly separate from a
   right Alt key, then we will try and convert the character into
   a C0 control code.

   b. If an Alt key is pressed (or in the case of an AltGr value,
   both Alt keys are pressed), then we will convert it into an
   Alt-key sequence by prefixing the character with an ESC.

5. If we don't have a UnicodeChar, we'll use the ToUnicodeEx API to
   check whether the current keyboard state reflects a dead key, and if
   so, return nothing.

6. Otherwise we'll make another ToUnicodeEx call but with any Ctrl
   and Alt modifiers removed from the state to determine the base key
   value. Once we have that, we can apply the modifiers ourself.

   a. If the Ctrl key is pressed, we'll try and convert the base value
   into a C0 control code. But if we can't do that, we'll try again
   with the virtual key code (if it's alphanumeric) as a fallback.

   b. If the Alt key is pressed, we'll convert the base value (or
   control code value) into an Alt-key sequence by prefixing it with
   an ESC.

For step 4-a, we determine whether the left Ctrl key is distinctly
separate from the right Alt key by recording the time that those keys
are pressed, and checking for a time gap greater than 50ms. This is
necessary to distinguish between the user pressing Ctrl+AltGr, or
just pressing AltGr alone, which triggers a fake Ctrl key press at
the same time.

Validation Steps Performed

I created a test script to automate key presses in the terminal window
for every relevant key, along with every Ctrl/Alt/Shift modifier, and
every relevant mode combination. I then compared the generated input
sequences with XTerm and a DEC VT240 terminal. The idea wasn't to match
either of them exactly, but to make sure the places where we differed
were intentional and reasonable.

This mostly dealt with the US keyboard layout. Comparing international
layouts wasn't really feasible because DEC, Linux, and Windows keyboard
assignments tend to be quite different. However, I've manually tested a
number of different layouts, and tried to make sure that they were all
working in a reasonable manner.

In terms of unit testing, I haven't done much more than patching the
ones that already existed to get them to pass. They're honestly not
great tests, because they aren't generating events in the form that
you'd expect for a genuine key press, and that can significantly affect
the results, but I can't think of an easy way to improve them.

PR Checklist

• Closes Add support for application keypad mode (DECKPAM) #16506
• Closes Alt arrow keys on numpad transmit extra characters in conhost #16508
• Closes TerminalInput::HandleKey should never return "unhandled" for a key event #16509
• Closes VT mouse sequences don't work when DECARM is disabled #16510
• Closes Ctrl+Keys that can't be encoded as VT should still fall through as the unmodified character #3483
• Closes Ctrl underscore sends RET in Emacs and terminal #11194
• Closes Pressing ctrl + : on a JIS-106 keyboard sends ^Z (was: editor (vim, emacs) will stop when push ctrl + :) #11700
• Closes ctrl-alt-\ is ignored in WSL #12555
• Closes Control key modifier chords interpreted incorrectly when using custom keyboard layout #13319
• Closes Alt doesn't work for most of the non-alphabetic ASCII control characters (Alt-Ctrl-[, Alt-Ctrl-\, Alt-Ctrl-], Alt-Ctrl-^) #15367
• Closes Keyboard shortcuts with crtl + alt gr (like crtl + \) on a German keyboard are not working #16173
• Tests added/passed

[j4james]

If you don't like the overall refactoring concept in this PR, it's worth noting that at least some of the bug fixes could easily be ported back to the current TerminalInput implementation, so it's not like it's all or nothing.

And on the flip side, if you like the refactoring, but don't like specific bug fixes, some of them can easily be dropped (I can imagine things like the #10308 fix, and the #16509 change might be controversial).

[zadjii-msft]

okay 11 bug fixes in January is setting the bar REAL high for 2024 😆 This might take us a bit to get to, but we should most certainly do this for 1.20

[lhecker]

By moving the surrogate pair check up in this function we could make it so that _makeCharOutput never returns an empty string.

[j4james]

I'm not sure what you meant here. Did you want to move the whole if til::is_leading_surrogate(ch)) _leadingSurrogate.emplace(ch) test out of the _makeCharOutput function? Because this isn't the only place that's being called. It's also used for the VK_PACKET and 0 virtual key events (although I'm not actually sure we should be delaying the surrogate pairs in that case).

[lhecker]

What I meant is that we could consider moving the leading-surrogate check right past the if (!keyEvent.bKeyDown) branch. It would then store any leading-surrogate into a member variable and could then early-return. That is:

class TerminalInput
{
    // no need for an optional
    wchar_t _leadingSurrogate = 0;
};

TerminalInput::OutputType TerminalInput::HandleKey(const INPUT_RECORD& event)
{
    // ...

    if (!keyEvent.bKeyDown)
    {
        // ...
    }

    // Unpaired surrogates are no good -> early return.
    if (til::is_leading_surrogate(unicodeChar))
    {
        _leadingSurrogate = unicodeChar;
        return MakeOutput({});
    }
    // Using a scope_exit ensures that a previous leading surrogate is forgotten
    // even if the KEY_EVENT that followed didn't end up calling _makeCharOutput.
    const auto leadingSurrogateReset = wil::scope_exit([&]() {
        _leadingSurrogate = 0;
    });

    // ...
}

TerminalInput::StringType TerminalInput::_makeCharOutput(const wchar_t ch)
{
    StringType str;

    if (_leadingSurrogate && til::is_trailing_surrogate(ch))
    {
        str.push_back(_leadingSurrogate);
    }

    str.push_back(ch);
    return str;
}

That way _makeCharOutput never returns an empty string which may allow you to simplify the if (unicodeChar != 0) branch.

However, there's one thing that made me wonder a bit now. The VK_PACKET handler doesn't deal with Alt key combinations. Why's that? Shouldn't it use the exact same code as the if (unicodeChar != 0) branch? Because if it did, you could abstract it away and then call that function directly when encountering a trailing surrogate key, since no character from a higher Unicode plane should ever hit any of the VK_* handlers, etc., anyways (I think?).

---

BTW: It may be worth introducing a _makeNoOutput() function or similar, since we now have 7 instances of MakeOutput({}), but it's conceptually the same thing every time.

[j4james]

OK. That makes sense. I've incorporated those changes now. It does simplify things. Thanks.

As for VK_PACKET, my understanding was that it serves as a kind of "pass through" mode, when you just want to send a raw unicode character as input. It will typically come from something like an emoji picker, or a SendInput call with KEYEVENTF_UNICODE.

It's not an actual keypress, so I don't think it's supposed to be affected by modifiers. If you wanted to simulate a VT Alt key with VK_PACKET you'd literally send through the sequence of unicode characters that make up that Alt key sequence.

[zadjii-msft]

okay this is pretty clever I'll give you that

[j4james]

I can't entirely take credit for this. I did some research to see how other people dealt with the issue, and the typical approach was to handle it at the time the Ctrl key message was read from the queue. The idea was that you would peek the next message looking for an Alt key, and compare the two MSG.time fields. If it was a fake Ctrl+Alt combo, they would have identical values, so you would know to ignore the Ctrl.

But that exact approach wouldn't have worked for us over conpty, so that's why I track the time manually and allow a bit of leeway in the time difference, but the general idea is the same.

[lhecker]

(I've taken the liberty to merge my suggestion as I didn't want to bother you with such a small nit. I hope you didn't mind this.)

[DHowett]

I wonder - will Alt+Numpad ever produce a leading/trailing surrogate pair?

[j4james]

If I remember correctly, Alt+Numpad is just for codepoints 0 to 255, and there's another variant (involving the plus key maybe?) which is supposed to handle Unicode characters. That's not currently supported.

[DHowett]

note to self: see if this differs from makeCharOutput

[DHowett]

it does; that inserts the leading s.p., this does not