From a5415611959c63d3c872fa4a7c074a1d96c8f2ab Mon Sep 17 00:00:00 2001 From: "Dustin L. Howett" Date: Thu, 22 Aug 2024 14:32:30 -0500 Subject: [PATCH 1/2] During Alt+Numpad composition, stash keys in case we bail out 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 --- src/cascadia/TerminalControl/TermControl.cpp | 80 +++++++++++++------- src/cascadia/TerminalControl/TermControl.h | 1 + 2 files changed, 53 insertions(+), 28 deletions(-) diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 045762a3ac0..489e81ecf06 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -1543,7 +1543,20 @@ namespace winrt::Microsoft::Terminal::Control::implementation 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. + bool h = true; + for (auto&& e : _altNumpadState.cachedKeyEvents) + { + h = h && _TrySendKeyEvent(std::get<0>(e), std::get<1>(e), std::get<2>(e), std::get<3>(e)); + } + // Send the alt keyup we are currently processing + h = h && _TrySendKeyEvent(vkey, scanCode, modifiers, keyDown); + // do not accumulate into the buffer + } + else if (s.accumulator <= 0xffff) { buf[buf_len++] = static_cast(s.accumulator); } @@ -1601,46 +1614,55 @@ 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 '+' 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; + if (keyDown) + { + // Alt '+' 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' is used to input ANSI code points. - // Otherwise, they're OEM codepoints. - s.encoding = AltNumpadEncoding::ANSI; - s.active = true; + if (keyDown) + { + // Alt '0' 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) + if (keyDown) { - add = vkey - VK_NUMPAD0; - } - else if (vkey >= 'A' && vkey <= 'F') - { - add = vkey - 'A' + 10; - } + // Otherwise, append the pressed key to the accumulator. + const uint32_t base = s.encoding == AltNumpadEncoding::Unicode ? 16 : 10; + uint32_t add = 0xffffff; - // Pressing Alt + should not activate the Alt+Numpad input, however. - if (add < base) - { - s.accumulator = std::min(s.accumulator * base + add, 0x10FFFFu); - s.active = true; + if (vkey >= VK_NUMPAD0 && vkey <= VK_NUMPAD9) + { + add = vkey - VK_NUMPAD0; + } + else if (vkey >= 'A' && vkey <= 'F') + { + add = vkey - 'A' + 10; + } + + // Pressing Alt + 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 +1670,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..a1bceadd654 100644 --- a/src/cascadia/TerminalControl/TermControl.h +++ b/src/cascadia/TerminalControl/TermControl.h @@ -262,6 +262,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation // 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, 4> cachedKeyEvents; }; AltNumpadState _altNumpadState; From 35af1532ab86882469cf1852f9dccde5c50fa2f5 Mon Sep 17 00:00:00 2001 From: "Dustin L. Howett" Date: Thu, 22 Aug 2024 20:08:33 -0500 Subject: [PATCH 2/2] All PR feedback --- src/cascadia/TerminalControl/TermControl.cpp | 22 +++++++------------- src/cascadia/TerminalControl/TermControl.h | 9 +++++++- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 489e81ecf06..e259d9ea05d 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -1539,6 +1539,7 @@ 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) { @@ -1547,13 +1548,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation { // 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. - bool h = true; for (auto&& e : _altNumpadState.cachedKeyEvents) { - h = h && _TrySendKeyEvent(std::get<0>(e), std::get<1>(e), std::get<2>(e), std::get<3>(e)); + handled = handled && _TrySendKeyEvent(e.vkey, e.scanCode, e.modifiers, e.keyDown); } // Send the alt keyup we are currently processing - h = h && _TrySendKeyEvent(vkey, scanCode, modifiers, keyDown); + handled = handled && _TrySendKeyEvent(vkey, scanCode, modifiers, keyDown); // do not accumulate into the buffer } else if (s.accumulator <= 0xffff) @@ -1600,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()) @@ -1618,9 +1618,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation { auto& s = _altNumpadState; - if (vkey == VK_ADD) + if (keyDown) { - if (keyDown) + if (vkey == VK_ADD) { // Alt '+' is used to input Unicode code points. // Every time you press + it resets the entire state @@ -1629,20 +1629,14 @@ namespace winrt::Microsoft::Terminal::Control::implementation s.accumulator = 0; s.active = true; } - } - else if (vkey == VK_NUMPAD0 && s.encoding == AltNumpadEncoding::OEM && s.accumulator == 0) - { - if (keyDown) + else if (vkey == VK_NUMPAD0 && s.encoding == AltNumpadEncoding::OEM && s.accumulator == 0) { // Alt '0' is used to input ANSI code points. // Otherwise, they're OEM codepoints. s.encoding = AltNumpadEncoding::ANSI; s.active = true; } - } - else - { - if (keyDown) + else { // Otherwise, append the pressed key to the accumulator. const uint32_t base = s.encoding == AltNumpadEncoding::Unicode ? 16 : 10; diff --git a/src/cascadia/TerminalControl/TermControl.h b/src/cascadia/TerminalControl/TermControl.h index a1bceadd654..ab5b0a1ae78 100644 --- a/src/cascadia/TerminalControl/TermControl.h +++ b/src/cascadia/TerminalControl/TermControl.h @@ -256,13 +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, 4> cachedKeyEvents; + til::small_vector cachedKeyEvents; }; AltNumpadState _altNumpadState;