Repository: microsoft/terminal (language: cpp)

Issue:
CANARY: ALT+NUMPAD ADD is Broken
### Windows Terminal version

1.22.2334.0

### Windows build number

10.0.19045.4780

### Other Software

_No response_

### Steps to reproduce

Alt+NumpadAdd pressing combination is broken since Canary build terminal-1.22.2201.0
terminal-1.22.2191.0 - latest good
terminal-1.22.2201.0 - broken
...
terminal-1.22.2334.0 - broken

run utilities for getting codes DOWN/UP VK_CODES etc
press Alt key after press NumpadAdd and release NumpadAdd, press NumpadAdd and release NumpadAdd, press NumpadAdd and release NumpadAdd

### Expected Behavior

terminal-1.22.2191.0 - latest good:
21:42:56 KEY_EVENT_RECORD: Dn, 1, Vk="VK_ADD" [107/0x006B], Scan=0x004E uChar=[U='+' (0x002B): A='+' (0x2B)]
         Ctrl=0x00000002 (cAsac - ecns) (Windowed)
21:42:56 KEY_EVENT_RECORD: Dn, 1, Vk="VK_ADD" [107/0x006B], Scan=0x004E uChar=[U='+' (0x002B): A='+' (0x2B)]
         Ctrl=0x00000002 (cAsac - ecns) (Windowed)
21:42:57 KEY_EVENT_RECORD: Up, 1, Vk="VK_ADD" [107/0x006B], Scan=0x004E uChar=[U='+' (0x002B): A='+' (0x2B)]
         Ctrl=0x00000002 (cAsac - ecns) (Windowed)


### Actual Behavior

since terminal-1.22.2201.0  to latest canary build terminal-1.22.2334.0  is broken:

21:44:18 KEY_EVENT_RECORD: Up, 1, Vk="VK_ADD" [107/0x006B], Scan=0x004E uChar=[U='+' (0x002B): A='+' (0x2B)]
         Ctrl=0x00000002 (cAsac - ecns) (Windowed)
21:44:18 KEY_EVENT_RECORD: Up, 1, Vk="VK_ADD" [107/0x006B], Scan=0x004E uChar=[U='+' (0x002B): A='+' (0x2B)]
         Ctrl=0x00000002 (cAsac - ecns) (Windowed)
21:44:18 KEY_EVENT_RECORD: Up, 1, Vk="VK_ADD" [107/0x006B], Scan=0x004E uChar=[U='+' (0x002B): A='+' (0x2B)]
         Ctrl=0x00000002 (cAsac - ecns) (Windowed)


Discussion / hints:
with another combination I no have problem, only 100% with Alt+NumPadAdd or RAlt+NumpadAdd

if press and release only Alt/Ralt - no problem
if press and release only NumPadAdd - no problem

ps. What I found problem : my macros for FAR MANAGER  for key="AltAdd" is stop working...

Fix patch:
diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp
index 045762a3ac0..e259d9ea05d 100644
--- a/src/cascadia/TerminalControl/TermControl.cpp
+++ b/src/cascadia/TerminalControl/TermControl.cpp
@@ -1539,11 +1539,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);
                 }
@@ -1587,7 +1600,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())
@@ -1601,46 +1614,49 @@ 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)
-                {
-                    // Alt '+' <number> is used to input Unicode code points.
-                    // 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
+                if (keyDown)
                 {
-                    // 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)
+                    if (vkey == VK_ADD)
                     {
-                        add = vkey - VK_NUMPAD0;
+                        // Alt '+' <number> is used to input Unicode code points.
+                        // 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 >= 'A' && vkey <= 'F')
+                    else if (vkey == VK_NUMPAD0 && s.encoding == AltNumpadEncoding::OEM && s.accumulator == 0)
                     {
-                        add = vkey - 'A' + 10;
+                        // Alt '0' <number> is used to input ANSI code points.
+                        // Otherwise, they're OEM codepoints.
+                        s.encoding = AltNumpadEncoding::ANSI;
+                        s.active = true;
                     }
-
-                    // Pressing Alt + <not a number> should not activate the Alt+Numpad input, however.
-                    if (add < base)
+                    else
                     {
-                        s.accumulator = std::min(s.accumulator * base + add, 0x10FFFFu);
-                        s.active = true;
+                        // 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;
+                        }
                     }
                 }
 
@@ -1648,6 +1664,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
                 // 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;
                 }
 
diff --git a/src/cascadia/TerminalControl/TermControl.h b/src/cascadia/TerminalControl/TermControl.h
index f60a72ea792..ab5b0a1ae78 100644
--- a/src/cascadia/TerminalControl/TermControl.h
+++ b/src/cascadia/TerminalControl/TermControl.h
@@ -256,12 +256,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;
 


Test patch: