Skip to content

Commit 8d25202

Browse files
christianparpartchristianparpart
committed
Fix numpad keys not producing digits when NumLock is ON
On Wayland, the native modifier bit for NumLock (XCB_MOD_MASK_2 = 0x10) is unreliable, so makeModifiers() often fails to set Modifier::NumLock. This caused two problems: - Legacy protocol + DECKPAM: numpad keys sent escape sequences (e.g. CSI E) instead of digits - Kitty protocol: CSI u sequences omitted the NumLock modifier, so shells didn't know to insert a digit for numpad keys The fix infers NumLock state from Qt's key event resolution: Qt reports Key_0..9 with KeypadModifier only when NumLock is ON (otherwise it reports navigation keys like Insert, End, etc.). For operator/Enter keys, NumLock is inferred from non-empty event text. Additionally, add numpadAssociatedText() to emit the third CSI u parameter (associated text codepoint) when ReportAssociatedText is enabled in the Kitty protocol, allowing shells to resolve numpad keys to their character values. Fixes #1901 Signed-off-by: Christian Parpart <christian@parpart.family>
1 parent d9b06a9 commit 8d25202

File tree

5 files changed

+189
-14
lines changed

5 files changed

+189
-14
lines changed

‎.github/actions/spelling/allow/allow.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ copy'n'paste
3535
copyable
3636
crossfade
3737
dbus
38+
definitionally
3839
flamegraphs
3940
gha
4041
ilammy

‎metainfo.xml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,7 @@
128128
<li>Fixes Home/End key encoding in Kitty keyboard protocol (extended keyboard input generator)</li>
129129
<li>Fixes Kitty keyboard protocol: F1-F4 CSI u key codes, legacy F1-F4 modifier encoding, CSI u gating for ReportEventTypes and ReportAllKeys flags, lock modifier handling, and Press event type suppression</li>
130130
<li>Fixes Kitty keyboard protocol: proper CapsLock/NumLock modifier reporting, correct modifier encoding per spec, and Unicode case folding for non-ASCII characters</li>
131+
<li>Fixes numpad keys not producing digits when NumLock is ON, especially on Wayland where native modifier detection is unreliable (#1901)</li>
131132
<li>Fixes tab characters (HT) intermittently lost in terminal output due to bulk text parser skipping trailing C0 controls and tab cursor movement not filling intermediate cells with spaces (#1876)</li>
132133
<li>Replaces abrupt cell blink toggle with configurable smooth pulse animation via blink_style profile setting (classic/smooth/linger)</li>
133134
<li>Adds configurable crossfade transition between primary and alternate screens via screen_transition and screen_transition_duration profile settings</li>

‎src/contour/helper.cpp‎

Lines changed: 50 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -447,18 +447,54 @@ bool sendKeyEvent(QKeyEvent* event, vtbackend::KeyboardEventType eventType, Term
447447
if (event->modifiers().testFlag(Qt::KeypadModifier))
448448
{
449449
std::optional<Key> mappedKey = nullopt;
450+
auto inferredModifiers = modifiers;
450451
switch (key)
451452
{
452-
case Qt::Key_0: mappedKey = Key::Numpad_0; break;
453-
case Qt::Key_1: mappedKey = Key::Numpad_1; break;
454-
case Qt::Key_2: mappedKey = Key::Numpad_2; break;
455-
case Qt::Key_3: mappedKey = Key::Numpad_3; break;
456-
case Qt::Key_4: mappedKey = Key::Numpad_4; break;
457-
case Qt::Key_5: mappedKey = Key::Numpad_5; break;
458-
case Qt::Key_6: mappedKey = Key::Numpad_6; break;
459-
case Qt::Key_7: mappedKey = Key::Numpad_7; break;
460-
case Qt::Key_8: mappedKey = Key::Numpad_8; break;
461-
case Qt::Key_9: mappedKey = Key::Numpad_9; break;
453+
// Qt reports Key_0..9 with KeypadModifier only when NumLock is ON.
454+
// When NumLock is OFF, Qt reports navigation keys instead (Insert, End, etc.).
455+
// So the presence of these key codes definitionally implies NumLock is active.
456+
case Qt::Key_0:
457+
mappedKey = Key::Numpad_0;
458+
inferredModifiers |= Modifier::NumLock;
459+
break;
460+
case Qt::Key_1:
461+
mappedKey = Key::Numpad_1;
462+
inferredModifiers |= Modifier::NumLock;
463+
break;
464+
case Qt::Key_2:
465+
mappedKey = Key::Numpad_2;
466+
inferredModifiers |= Modifier::NumLock;
467+
break;
468+
case Qt::Key_3:
469+
mappedKey = Key::Numpad_3;
470+
inferredModifiers |= Modifier::NumLock;
471+
break;
472+
case Qt::Key_4:
473+
mappedKey = Key::Numpad_4;
474+
inferredModifiers |= Modifier::NumLock;
475+
break;
476+
case Qt::Key_5:
477+
mappedKey = Key::Numpad_5;
478+
inferredModifiers |= Modifier::NumLock;
479+
break;
480+
case Qt::Key_6:
481+
mappedKey = Key::Numpad_6;
482+
inferredModifiers |= Modifier::NumLock;
483+
break;
484+
case Qt::Key_7:
485+
mappedKey = Key::Numpad_7;
486+
inferredModifiers |= Modifier::NumLock;
487+
break;
488+
case Qt::Key_8:
489+
mappedKey = Key::Numpad_8;
490+
inferredModifiers |= Modifier::NumLock;
491+
break;
492+
case Qt::Key_9:
493+
mappedKey = Key::Numpad_9;
494+
inferredModifiers |= Modifier::NumLock;
495+
break;
496+
// Operator and Enter keys have the same Qt key code regardless of NumLock state,
497+
// but produce text when NumLock is active.
462498
case Qt::Key_Asterisk: mappedKey = Key::Numpad_Multiply; break;
463499
case Qt::Key_Plus: mappedKey = Key::Numpad_Add; break;
464500
case Qt::Key_Minus: mappedKey = Key::Numpad_Subtract; break;
@@ -469,7 +505,10 @@ bool sendKeyEvent(QKeyEvent* event, vtbackend::KeyboardEventType eventType, Term
469505
}
470506
if (mappedKey)
471507
{
472-
session.sendKeyEvent(*mappedKey, modifiers, eventType, now);
508+
// For operator/Enter keys, infer NumLock from non-empty text
509+
if (!event->text().isEmpty())
510+
inferredModifiers |= Modifier::NumLock;
511+
session.sendKeyEvent(*mappedKey, inferredModifiers, eventType, now);
473512
return true;
474513
}
475514
}

‎src/vtbackend/InputGenerator.cpp‎

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -551,6 +551,33 @@ constexpr pair<unsigned, char> mapKey(Key key) noexcept
551551
crispy::unreachable();
552552
}
553553

554+
/// Returns the associated text codepoint for a numpad key, or 0 if none.
555+
/// Per the Kitty keyboard protocol, associated text is the character the key would produce.
556+
constexpr char32_t numpadAssociatedText(Key key) noexcept
557+
{
558+
switch (key)
559+
{
560+
case Key::Numpad_0: return U'0';
561+
case Key::Numpad_1: return U'1';
562+
case Key::Numpad_2: return U'2';
563+
case Key::Numpad_3: return U'3';
564+
case Key::Numpad_4: return U'4';
565+
case Key::Numpad_5: return U'5';
566+
case Key::Numpad_6: return U'6';
567+
case Key::Numpad_7: return U'7';
568+
case Key::Numpad_8: return U'8';
569+
case Key::Numpad_9: return U'9';
570+
case Key::Numpad_Decimal: return U'.';
571+
case Key::Numpad_Divide: return U'/';
572+
case Key::Numpad_Multiply: return U'*';
573+
case Key::Numpad_Subtract: return U'-';
574+
case Key::Numpad_Add: return U'+';
575+
case Key::Numpad_Enter: return U'\r';
576+
case Key::Numpad_Equal: return U'=';
577+
default: return 0;
578+
}
579+
}
580+
554581
bool ExtendedKeyboardInputGenerator::generateKey(Key key, Modifiers modifiers, KeyboardEventType eventType)
555582
{
556583
if (!enabled(eventType))
@@ -577,12 +604,23 @@ bool ExtendedKeyboardInputGenerator::generateKey(Key key, Modifiers modifiers, K
577604

578605
auto const [code, function] = mapKey(key);
579606
auto const encodedModifiers = encodeModifiers(modifiers, eventType);
607+
608+
// Check if we should append associated text (third CSI u parameter)
609+
auto const associatedText =
610+
enabled(KeyboardEventFlag::ReportAssociatedText) ? numpadAssociatedText(key) : char32_t { 0 };
611+
580612
// Per Kitty spec: omit key number when code==1 and no modifiers/alternates/text.
581613
auto controlSequence = std::string("\033[");
582-
if (code != 1 || !encodedModifiers.empty())
614+
if (code != 1 || !encodedModifiers.empty() || associatedText)
583615
controlSequence += std::to_string(code);
584-
if (!encodedModifiers.empty())
585-
controlSequence += std::format(";{}", encodedModifiers);
616+
if (!encodedModifiers.empty() || associatedText)
617+
{
618+
// When associated text is present, the modifier field must be emitted
619+
// (use "1" as default when no modifiers, per Kitty spec encoding: 1 + 0 = 1)
620+
controlSequence += std::format(";{}", encodedModifiers.empty() ? "1" : encodedModifiers);
621+
}
622+
if (associatedText)
623+
controlSequence += std::format(";{}", static_cast<unsigned>(associatedText));
586624
controlSequence += function;
587625
append(controlSequence);
588626

‎src/vtbackend/InputGenerator_test.cpp‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -731,3 +731,99 @@ TEST_CASE("InputGenerator.Legacy.Ctrl_at", "[terminal,input]")
731731
}
732732

733733
// }}}
734+
735+
// {{{ Numpad + NumLock tests
736+
737+
TEST_CASE("InputGenerator.Numpad_with_NumLock_overrides_DECKPAM", "[terminal,input]")
738+
{
739+
// When NumLock is active, numpad digit keys must produce their literal characters
740+
// even in application keypad mode (DECKPAM), not escape sequences.
741+
auto input = InputGenerator {};
742+
input.setApplicationKeypadMode(true);
743+
REQUIRE(input.applicationKeypad());
744+
745+
auto constexpr NumLock = Modifiers { Modifier::NumLock };
746+
747+
struct Mapping
748+
{
749+
Key key;
750+
string_view expected;
751+
};
752+
753+
auto constexpr Mappings = std::array {
754+
Mapping { .key = Key::Numpad_0, .expected = "0"sv },
755+
Mapping { .key = Key::Numpad_1, .expected = "1"sv },
756+
Mapping { .key = Key::Numpad_2, .expected = "2"sv },
757+
Mapping { .key = Key::Numpad_3, .expected = "3"sv },
758+
Mapping { .key = Key::Numpad_4, .expected = "4"sv },
759+
Mapping { .key = Key::Numpad_5, .expected = "5"sv },
760+
Mapping { .key = Key::Numpad_6, .expected = "6"sv },
761+
Mapping { .key = Key::Numpad_7, .expected = "7"sv },
762+
Mapping { .key = Key::Numpad_8, .expected = "8"sv },
763+
Mapping { .key = Key::Numpad_9, .expected = "9"sv },
764+
Mapping { .key = Key::Numpad_Decimal, .expected = "."sv },
765+
Mapping { .key = Key::Numpad_Divide, .expected = "/"sv },
766+
Mapping { .key = Key::Numpad_Multiply, .expected = "*"sv },
767+
Mapping { .key = Key::Numpad_Add, .expected = "+"sv },
768+
Mapping { .key = Key::Numpad_Subtract, .expected = "-"sv },
769+
Mapping { .key = Key::Numpad_Enter, .expected = "\r"sv },
770+
Mapping { .key = Key::Numpad_Equal, .expected = "="sv },
771+
};
772+
773+
for (auto const& mapping: Mappings)
774+
{
775+
INFO(std::format("Testing {}+NumLock in DECKPAM => {}", mapping.key, escape(mapping.expected)));
776+
input.generate(mapping.key, NumLock, KeyboardEventType::Press);
777+
CHECK(escape(input.peek()) == escape(mapping.expected));
778+
input.consume(static_cast<int>(input.peek().size()));
779+
}
780+
}
781+
782+
TEST_CASE("InputGenerator.Numpad_without_NumLock_in_DECKPAM", "[terminal,input]")
783+
{
784+
// Without NumLock, numpad keys in DECKPAM must send their application keypad sequences.
785+
auto input = InputGenerator {};
786+
input.setApplicationKeypadMode(true);
787+
REQUIRE(input.applicationKeypad());
788+
789+
// KP_5 without NumLock in DECKPAM → CSI E
790+
input.generate(Key::Numpad_5, Modifiers {}, KeyboardEventType::Press);
791+
CHECK(escape(input.peek()) == escape("\033[E"sv));
792+
}
793+
794+
TEST_CASE("ExtendedKeyboardInputGenerator.CSIu.Numpad5_with_NumLock", "[terminal,input]")
795+
{
796+
// KP_5 + NumLock in Kitty disambiguate mode:
797+
// code=57404, modifier=1+128=129, final='u'
798+
auto input = ExtendedKeyboardInputGenerator {};
799+
input.enter(KeyboardEventFlag::DisambiguateEscapeCodes);
800+
801+
input.generateKey(Key::Numpad_5, Modifier::NumLock, KeyboardEventType::Press);
802+
REQUIRE(escape(input.take()) == escape("\033[57404;129u"sv));
803+
}
804+
805+
TEST_CASE("ExtendedKeyboardInputGenerator.CSIu.Numpad5_AssociatedText", "[terminal,input]")
806+
{
807+
// KP_5 + NumLock with ReportAssociatedText enabled:
808+
// code=57404, modifier=129, associated_text=53 ('5'), final='u'
809+
auto input = ExtendedKeyboardInputGenerator {};
810+
input.enter(KeyboardEventFlag::DisambiguateEscapeCodes);
811+
input.flags().enable(KeyboardEventFlag::ReportAssociatedText);
812+
813+
input.generateKey(Key::Numpad_5, Modifier::NumLock, KeyboardEventType::Press);
814+
REQUIRE(escape(input.take()) == escape("\033[57404;129;53u"sv));
815+
}
816+
817+
TEST_CASE("ExtendedKeyboardInputGenerator.CSIu.Numpad_AssociatedText_no_mods", "[terminal,input]")
818+
{
819+
// KP_5 without modifiers but with ReportAssociatedText:
820+
// code=57404, modifier=1 (default, needed for text parameter), associated_text=53, final='u'
821+
auto input = ExtendedKeyboardInputGenerator {};
822+
input.enter(KeyboardEventFlag::DisambiguateEscapeCodes);
823+
input.flags().enable(KeyboardEventFlag::ReportAssociatedText);
824+
825+
input.generateKey(Key::Numpad_5, Modifiers {}, KeyboardEventType::Press);
826+
REQUIRE(escape(input.take()) == escape("\033[57404;1;53u"sv));
827+
}
828+
829+
// }}}

0 commit comments

Comments
 (0)