During Alt+Numpad composition, stash keys in case we bail out (#17774)

We were erroneously eating Alt followed by VK_ADD. This change makes
sure we cache key presses and releases that happen once a numpad
composition is active so that we can send them when you release Alt.

Right now, we only send them when you release Alt after composing Alt
and VK_ADD (entering hex mode) and only if you haven't inserted an
actual hex numpad code. This does mean that `Alt VK_ADD 0 0 H I` will
result in an input of "+hi". That... seems like a small price to pay for
Alt VK_ADD working again.

Closes #17762

(cherry picked from commit e006f75f6c175542760ee4d338ee894b82acff5f)
Service-Card-Id: PVTI_lADOAF3p4s4AmhmszgSF50M
Service-Version: 1.21

src/cascadia/TerminalControl/TermControl.cpp

@@ -1492,11 +1492,24 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             auto encoding = s.encoding;
             wchar_t buf[4]{};
             size_t buf_len = 0;
+            bool handled = true;
 
             if (encoding == AltNumpadEncoding::Unicode)
             {
                 // UTF-32 -> UTF-16
-                if (s.accumulator <= 0xffff)
+                if (s.accumulator == 0)
+                {
+                    // If the user pressed Alt + VK_ADD, then released Alt, they probably didn't intend to insert a numpad character at all.
+                    // Send any accumulated key events instead.
+                    for (auto&& e : _altNumpadState.cachedKeyEvents)
+                    {
+                        handled = handled && _TrySendKeyEvent(e.vkey, e.scanCode, e.modifiers, e.keyDown);
+                    }
+                    // Send the alt keyup we are currently processing
+                    handled = handled && _TrySendKeyEvent(vkey, scanCode, modifiers, keyDown);
+                    // do not accumulate into the buffer
+                }
+                else if (s.accumulator <= 0xffff)
                 {
                     buf[buf_len++] = static_cast<uint16_t>(s.accumulator);
                 }
@@ -1540,7 +1553,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             }
 
             s = {};
-            return true;
+            return handled;
         }
         // As a continuation of the above, this handles the key-down case.
         if (modifiers.IsAltPressed())
@@ -1554,53 +1567,58 @@ namespace winrt::Microsoft::Terminal::Control::implementation
                 SCROLLLOCK_ON |
                 CAPSLOCK_ON;
 
-            if (keyDown && (modifiers.Value() & ~permittedModifiers) == 0)
+            if ((modifiers.Value() & ~permittedModifiers) == 0)
             {
                 auto& s = _altNumpadState;
 
-                if (vkey == VK_ADD)
+                if (keyDown)
                 {
-                    // Alt '+' <number> is used to input Unicode code points.
+                    if (vkey == VK_ADD)
-                    // Every time you press + it resets the entire state
-                    // in the original OS implementation as well.
-                    s.encoding = AltNumpadEncoding::Unicode;
-                    s.accumulator = 0;
-                    s.active = true;
-                }
-                else if (vkey == VK_NUMPAD0 && s.encoding == AltNumpadEncoding::OEM && s.accumulator == 0)
-                {
-                    // Alt '0' <number> is used to input ANSI code points.
-                    // Otherwise, they're OEM codepoints.
-                    s.encoding = AltNumpadEncoding::ANSI;
-                    s.active = true;
-                }
-                else
-                {
-                    // Otherwise, append the pressed key to the accumulator.
-                    const uint32_t base = s.encoding == AltNumpadEncoding::Unicode ? 16 : 10;
-                    uint32_t add = 0xffffff;
-
-                    if (vkey >= VK_NUMPAD0 && vkey <= VK_NUMPAD9)
                     {
-                        add = vkey - VK_NUMPAD0;
+                        // Alt '+' <number> is used to input Unicode code points.
-                    }
+                        // Every time you press + it resets the entire state
-                    else if (vkey >= 'A' && vkey <= 'F')
+                        // in the original OS implementation as well.
-                    {
+                        s.encoding = AltNumpadEncoding::Unicode;
-                        add = vkey - 'A' + 10;
+                        s.accumulator = 0;
-                    }
-
-                    // Pressing Alt + <not a number> should not activate the Alt+Numpad input, however.
-                    if (add < base)
-                    {
-                        s.accumulator = std::min(s.accumulator * base + add, 0x10FFFFu);
                         s.active = true;
                     }
+                    else if (vkey == VK_NUMPAD0 && s.encoding == AltNumpadEncoding::OEM && s.accumulator == 0)
+                    {
+                        // Alt '0' <number> is used to input ANSI code points.
+                        // Otherwise, they're OEM codepoints.
+                        s.encoding = AltNumpadEncoding::ANSI;
+                        s.active = true;
+                    }
+                    else
+                    {
+                        // Otherwise, append the pressed key to the accumulator.
+                        const uint32_t base = s.encoding == AltNumpadEncoding::Unicode ? 16 : 10;
+                        uint32_t add = 0xffffff;
+
+                        if (vkey >= VK_NUMPAD0 && vkey <= VK_NUMPAD9)
+                        {
+                            add = vkey - VK_NUMPAD0;
+                        }
+                        else if (vkey >= 'A' && vkey <= 'F')
+                        {
+                            add = vkey - 'A' + 10;
+                        }
+
+                        // Pressing Alt + <not a number> should not activate the Alt+Numpad input, however.
+                        if (add < base)
+                        {
+                            s.accumulator = std::min(s.accumulator * base + add, 0x10FFFFu);
+                            s.active = true;
+                        }
+                    }
                 }
 
                 // If someone pressed Alt + <not a number>, we'll skip the early
                 // return and send the Alt key combination as per usual.
                 if (s.active)
                 {
+                    // Cache it in case we have to emit it after alt is released
+                    _altNumpadState.cachedKeyEvents.emplace_back(vkey, scanCode, modifiers, keyDown);
                     return true;
                 }
 

src/cascadia/TerminalControl/TermControl.h

@@ -247,12 +247,20 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         };
         struct AltNumpadState
         {
+            struct CachedKey
+            {
+                WORD vkey;
+                WORD scanCode;
+                ::Microsoft::Terminal::Core::ControlKeyStates modifiers;
+                bool keyDown;
+            };
             AltNumpadEncoding encoding = AltNumpadEncoding::OEM;
             uint32_t accumulator = 0;
             // Checking for accumulator != 0 to see if we have an ongoing Alt+Numpad composition is insufficient.
             // The state can be active, while the accumulator is 0, if the user pressed Alt+Numpad0 which enabled
             // the OEM encoding mode (= active), and then pressed Numpad0 again (= accumulator is still 0).
             bool active = false;
+            til::small_vector<CachedKey, 4> cachedKeyEvents;
         };
         AltNumpadState _altNumpadState;