Kitty Keyboard Protocol support #4769

horkykuba · 2025-09-27T18:42:01Z

Kitty Keyboard Protocol support

Proposed changes

I've added support for Kitty Keyboard Protocol.

I've done some basic testing it WezTerm and kitty terminals, and it appears to work. However, I haven't tested it thoroughly. In particular, please review the integrity of the get_key_code() function, as I'm not entirely sure I understand all of its functionality in detail.

Resolves: #4762

zyv · 2025-09-27T18:53:34Z

Hi @egmontkob and @krobelus, any chance you could leave a review? Thank you!

zyv · 2025-09-27T18:55:48Z

Supersedes #4768.

zyv · 2025-09-27T19:08:34Z

My question would be: do we really need the kitty_keyboard_protocol toggle?

egmontkob · 2025-09-27T19:11:58Z

As I've already noted in #4663 (and it hasn't changed since), I'm absolutely not familiar with Kitty's keyboard protocol, so I'm sorry but I can't help here.

zyv · 2025-09-27T19:15:13Z

As I've already noted in #4663 (and it hasn't changed since), I'm absolutely not familiar with Kitty's keyboard protocol, so I'm sorry but I can't help here.

The hope dies last - I think of you when I think of terminals :) Sorry for the noise.

zyv · 2025-09-27T19:17:08Z

@horkykuba could you please fix the formatting (make indent) and ncurses build? Thanks.

horkykuba · 2025-09-27T20:32:59Z

@horkykuba could you please fix the formatting (make indent) and ncurses build? Thanks.

Done.

zyv · 2025-09-28T07:09:19Z

@horkykuba I've fixed everything for the CI to pass.

Could you please add some tests for get_key_code that show how it works with and without changes you made? You can find an example of how to write such tests here:

https://github.com/MidnightCommander/mc/blob/master/tests/lib/terminal.c

horkykuba · 2025-09-28T15:35:24Z

@horkykuba I've fixed everything for the CI to pass.

Thanks, it was already past Midnight when I was making the last edits :)

I've also modified the get_key_code() function to handle multiple consecutive sequences, could you run tests again pls?

Could you please add some tests for get_key_code that show how it works with and without changes you made? You can find an example of how to write such tests here:

Ok, but it will take me a while. Should I implement console input by mocking it using MC_MOCKABLE, or rather by using something like freopen()?

ossilator

i did mostly only a formal review, without checking the actual protocol impl. before i do the latter, please add a comment with a high-level overview of what that code is supposed to do (basically a tldr version of the protocol spec, with a link to the actual spec), and short comments that outline the particular cases.

i'd prefer it if you clean up the series (that is, squash most commits to achieve atomicity), though zyv might disagree ...

lib/tty/tty-ncurses.c

lib/tty/tty-slang.c

lib/tty/tty.c

lib/tty/key.c

zyv · 2025-09-29T08:18:34Z

i'd prefer it if you clean up the series (that is, squash most commits to achieve atomicity), though zyv might disagree ...

I have no objections. I have added my commits on top just to show what I did to fix formatting, linters, and CI.

Probably it would be still good to address your comments in a separate commit though, just to make it easier to evaluate that they are indeed addressed before we move further.

doc/man/es/mc.1.in

zyv · 2025-09-29T10:03:39Z

Ok, but it will take me a while. Should I implement console input by mocking it using MC_MOCKABLE, or rather by using something like freopen()?

Good question! I think this function is a scary, hairy, hardly testable monster. However, making it even more complicated is not helpful.

Perhaps you could extract the parsing code into smaller functions that work on buffers. Then these functions could be easily tested, and you wouldn't need to mock things like tty_lowlevel_getch, etc.

There is no hurry. I would really like to have something like this included, but please understand that I also don't want to introduce a lot of potential breakage, which I can't deal with. And the worst part is that I don't have time.

So even if it takes you a long time to come up with a tested and reviewable solution, it's absolutely worth it, because then it can be included.

horkykuba · 2025-09-29T12:54:06Z

i did mostly only a formal review, without checking the actual protocol impl. before i do the latter, please add a comment with a high-level overview of what that code is supposed to do (basically a tldr version of the protocol spec, with a link to the actual spec), and short comments that outline the particular cases.

@ossilator It simply waits if a sequence is found in the keys tree and if not, it tries to get all remaining pending chars from stdin and parse them as a parametrized CSI sequence. If that succeeds, it converts the input to a char array and checks if it matches the Kitty Keyboard Protocol sequence:

CSI parameters [u~ABCDEFHPQS]

If it matches, it retrieves the modifier bits, checks for non-alphabet keys using a predefined table, and then corrects shifted keys as described in previous comment. Next, it checks if it is a non-Basic Latin Unicode character. If so, it converts it to UTF-8 and inserts the remaining bytes into the pending queue. Finally, it masks the resulting value with 0x1F if the CTRL modifier is set.

Throughout the process, it carefully handles possible following sequences, leaving them in the pending queue.

Examples:

zyv · 2025-09-29T14:16:58Z

i did mostly only a formal review, without checking the actual protocol impl. before i do the latter, please add a comment with a high-level overview of what that code is supposed to do (basically a tldr version of the protocol spec, with a link to the actual spec), and short comments that outline the particular cases.

@ossilator It simply waits if a sequence is found in the keys tree and if not, it tries to get all remaining pending chars from stdin and parse them as a parametrized CSI sequence. If that succeeds, it converts the input to a char array and checks if it matches the Kitty Keyboard Protocol sequence:

CSI parameters [u~ABCDEFHPQS]

If it matches, it retrieves the modifier bits, checks for non-alphabet keys using a predefined table, and then corrects shifted keys as described in previous comment. Next, it checks if it is a non-Basic Latin Unicode character. If so, it converts it to UTF-8 and inserts the remaining bytes into the pending queue. Finally, it masks the resulting value with 0x1F if the CTRL modifier is set.

Throughout the process, it carefully handles possible following sequences, leaving them in the pending queue.

Examples:

\E[2~ or \E[2;1~ -> KEY_IC \E[1;2A -> KEY_M_SHIFT | KEY_UP \E[13;6u -> KEY_M_CTRL | KEY_M_SHIFT | KEY_ENTER \E[111;5u -> KEY_M_CTRL | 'o' \E[44:63;132u -> KEY_M_ALT | '?' (on Czech keyboard, with Num Lock on)

This is actually a very nice explanation! I'd love to see it as a comment in the code.

horkykuba · 2025-10-23T17:11:18Z

Thanks, I've fixed library dependency issue which prevented to build on some systems, split mocking code to two commits, and updated commit messages.

zyv · 2025-10-23T17:44:50Z

Thanks, I've fixed library dependency issue which prevented to build on some systems, split mocking code to two commits, and updated commit messages.

Hmmm, this didn't seem to help. If you have linking problems, then maybe #4806 would fix that. Very unfortunately, I didn't have time yet to follow up on the removal of the dynamic library. For now, maybe you can replace just one LIB with LDADD...

horkykuba · 2025-10-23T18:24:12Z

Hmmm, this didn't seem to help. If you have linking problems, then maybe #4806 would fix that. Very unfortunately, I didn't have time yet to follow up on the removal of the dynamic library. For now, maybe you can replace just one LIB with LDADD...

I can't reproduce it. It succeeds on my system in both Ubuntu and Fedora. Is it possible to get the test-suite.log somehow?

And if the test-suite.log is the same as you posted previously, is it possible to get the core dump of tests/lib/tty?

zyv · 2025-10-23T19:45:48Z

I can't reproduce it. It succeeds on my system in both Ubuntu and Fedora. Is it possible to get the test-suite.log somehow?

And if the test-suite.log is the same as you posted previously, is it possible to get the core dump of tests/lib/tty?

Yes, it's the same. I don't think there is an easy way to get to the core dump. I have to try to reproduce it locally.

FAIL: tty
=========

Running suite(s): /lib
Running suite /lib
The TERM environment variable is unset!
../../../tests/lib/tty.c:96:P:Core:test_tty_check_term_unset:0: Passed
../../../tests/lib/tty.c:114:P:Core:test_tty_check_term_non_xterm:0: Passed
../../../tests/lib/tty.c:132:P:Core:test_tty_check_term_xterm_like:0: Passed
TERM environment variable needs set.
../../../tests/lib/tty.c:138:E:Core:test_tty_get_key_code:0: (after this point) Early exit with return value 1
75%: Checks: 4, Failures: 0, Errors: 1
../../../tests/lib/tty.c:138:E:Core:test_tty_get_key_code:0: (after this point) Early exit with return value 1
Results for all suites run:
75%: Checks: 4, Failures: 0, Errors: 1
FAIL tty (exit status: 1)

horkykuba · 2025-10-23T21:49:30Z

Maybe it is because TERM variable is not set in CI worker, I've tried to hardcode it in mock

zyv · 2025-10-24T06:58:41Z

Maybe it is because TERM variable is not set in CI worker, I've tried to hardcode it in mock

Wow, bingo, CI (mostly) runs headless (except for Solaris), so TERM was indeed unset! With your latest changes, it builds and passes the tests on all systems.

lib/tty/key.c

mc-worker · 2025-10-24T16:49:20Z

lib/tty/key.c

+
+            for (i = 0; pending_keys[i] != '\0'; i++)
+                seq_char_buffer[i] = pending_keys[i];
+            seq_char_buffer[i] = '\0';


Memmove() or memcpy() instead of loop?

that would be more of a strcpy() thing.

but i wonder whether this is robust against malformed (unterminated) sequences?
(hmm, i think i already asked this ...)

These functions can't be used as this is conversion from 32bit to 8bit array.

This is robust because pending_keys is filled by push_char(), which checks for buffer overflow, and seq_char_buffer has the same size.

lib/tty/tty-ncurses.c

mc-worker · 2025-10-24T16:54:34Z

lib/tty/tty.c

+
+/** Enable or disable Kitty Keyboard Protocol */
+void
+tty_kitty (gboolean set)


This function is used only in lib/tty. Should it be declared/defined in tty-internal.[hc]?

This function is used in src/execute.c also.

lib/tty/key.c

ossilator

so now i made some effort to actually understand what this code does.
i didn't check the wider context this is embedded into.
but with these disclaimers in place, i'd say it looks reasonable.

lib/terminal.c

lib/tty/key.c

ossilator · 2025-10-24T18:00:19Z

lib/tty/key.c

+        || (csi.final_byte >= 'A' && csi.final_byte <= 'F') || (csi.final_byte == 'H')
+        || (csi.final_byte >= 'P' && csi.final_byte <= 'S'))
+    {
+        // Private mode is used for reporting protocol presence and mode from terminal


fwiw, as i had to look it up: "private mode" is a kinda misleading naming for what it does. it's basically a private namespace for vendor-specific features.

Actually, it is "private mode" of parameter string format. ISO/IEC 6429, section 5.4.1, specifies that the parameter string must follow precise rules if it does not begin with [<=>?], but may have an arbitrary format if it does. The standard doesn't split the final byte to two namespaces (private vs. non-private parameter string). The term "private mode" is also used in the current mc source code in the description of CSI format in lib/terminal.c.

i know. it's still a stupid name. not your fault. you could still extend the comment for clarity if you want to, but whatever.

lib/tty/key.c

ossilator · 2025-10-24T18:17:30Z

lib/tty/key.c

+
+            for (i = 0; pending_keys[i] != '\0'; i++)
+                seq_char_buffer[i] = pending_keys[i];
+            seq_char_buffer[i] = '\0';


that would be more of a strcpy() thing.

but i wonder whether this is robust against malformed (unterminated) sequences?
(hmm, i think i already asked this ...)

lib/tty/key.c

mc-worker

I think, changes after changes is not a good practice. Since this PR is not merged, all these fixes should be moved to commits where appropriate changed were introduced.

lib/tty/key.c

horkykuba · 2025-10-26T20:32:21Z

Squashed commits and done changes suggested by @aborodin

ossilator

just nitpicks.

lib/tty/key.c

ossilator · 2025-10-27T10:25:34Z

lib/tty/key.c

+        || (csi.final_byte >= 'A' && csi.final_byte <= 'F') || (csi.final_byte == 'H')
+        || (csi.final_byte >= 'P' && csi.final_byte <= 'S'))
+    {
+        // Private mode is used for reporting protocol presence and mode from terminal


i know. it's still a stupid name. not your fault. you could still extend the comment for clarity if you want to, but whatever.

Signed-off-by: Jakub Horký <jakub.git@horky.net>

Make functions tty_lowlevel_getch() and getch_with_timeout() mockable by defining them as weak. Fake key input can be passed by mock_input(). Signed-off-by: Jakub Horký <jakub.git@horky.net>

The protocol is described here: https://sw.kovidgoyal.net/kitty/keyboard-protocol/ The old functionality that "eats" all characters following an unknown escape sequence has been removed, as it causes severe side effects (such as not responding to any keys following an unknown sequence for some time). This is superseded by parsing CSI and SS3 sequences, which are the de facto standard for sending any function keys. Signed-off-by: Jakub Horký <jakub.git@horky.net>

horkykuba · 2025-10-27T18:17:10Z

Made changes suggested by @ossilator and removed key sequence definitions duplicate to kitty in misc/mc.lib

github-actions bot added the needs triage Needs triage by maintainers label Sep 27, 2025

github-actions bot added this to the Future Releases milestone Sep 27, 2025

github-actions bot added the prio: medium Has the potential to affect progress label Sep 27, 2025

This was referenced Sep 27, 2025

savannah: ctrl-\ hotlist key won't work under tty<i> #21

Open

Ctrl+Enter doesn't copy selected file name to subshell in terminal emulator --without-x #2150

Open

horkykuba force-pushed the kitty branch from 5f6b0fc to 21869f1 Compare September 27, 2025 19:03

zyv added area: tty Interaction with the terminal, screen libraries and removed needs triage Needs triage by maintainers labels Sep 27, 2025

zyv requested a review from aborodin September 27, 2025 19:08

zyv requested a review from ossilator September 27, 2025 19:18

horkykuba force-pushed the kitty branch 2 times, most recently from 5c71db3 to eda9b61 Compare September 28, 2025 19:04

ossilator suggested changes Sep 29, 2025

View reviewed changes

krobelus reviewed Sep 29, 2025

View reviewed changes

doc/man/es/mc.1.in Show resolved Hide resolved

horkykuba force-pushed the kitty branch from 6468333 to e4161fb Compare September 29, 2025 09:50

horkykuba requested a review from ossilator September 29, 2025 10:45

horkykuba force-pushed the kitty branch from 4a91bb6 to e4161fb Compare September 29, 2025 13:44

horkykuba force-pushed the kitty branch 2 times, most recently from faf4583 to ce57fcf Compare October 23, 2025 17:23

horkykuba force-pushed the kitty branch from ce57fcf to fbf1ba3 Compare October 23, 2025 21:45

horkykuba force-pushed the kitty branch from fbf1ba3 to 561e044 Compare October 23, 2025 21:50

horkykuba requested a review from ossilator October 24, 2025 06:37

mc-worker requested review from mc-worker and removed request for aborodin and krobelus October 24, 2025 16:28

mc-worker reviewed Oct 24, 2025

View reviewed changes

ossilator reviewed Oct 24, 2025

View reviewed changes

mc-worker reviewed Oct 25, 2025

View reviewed changes

lib/tty/key.c Outdated Show resolved Hide resolved

lib/tty/key.c Outdated Show resolved Hide resolved

horkykuba force-pushed the kitty branch 2 times, most recently from b9dbba9 to 54cde8a Compare October 26, 2025 20:30

horkykuba requested review from krobelus, mc-worker and ossilator October 27, 2025 00:35

ossilator reviewed Oct 27, 2025

View reviewed changes

Jakub Horký added 3 commits October 27, 2025 19:13

lib/tty/key.c: Fix typo in comment

db05396

Signed-off-by: Jakub Horký <jakub.git@horky.net>

tests/lib/tty.c: Add key mocking support

2b2e1ef

Make functions tty_lowlevel_getch() and getch_with_timeout() mockable by defining them as weak. Fake key input can be passed by mock_input(). Signed-off-by: Jakub Horký <jakub.git@horky.net>

horkykuba force-pushed the kitty branch from 54cde8a to dd51953 Compare October 27, 2025 18:14

horkykuba requested a review from ossilator October 27, 2025 18:21

Uh oh!

Kitty Keyboard Protocol support #4769

Are you sure you want to change the base?

Kitty Keyboard Protocol support #4769

Uh oh!

Conversation

horkykuba commented Sep 27, 2025

Proposed changes

Uh oh!

zyv commented Sep 27, 2025

Uh oh!

zyv commented Sep 27, 2025

Uh oh!

zyv commented Sep 27, 2025

Uh oh!

egmontkob commented Sep 27, 2025

Uh oh!

zyv commented Sep 27, 2025

Uh oh!

zyv commented Sep 27, 2025

Uh oh!

horkykuba commented Sep 27, 2025

Uh oh!

zyv commented Sep 28, 2025

Uh oh!

horkykuba commented Sep 28, 2025

Uh oh!

ossilator left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zyv commented Sep 29, 2025

Uh oh!

Uh oh!

zyv commented Sep 29, 2025

Uh oh!

horkykuba commented Sep 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zyv commented Sep 29, 2025

Uh oh!

horkykuba commented Oct 23, 2025

Uh oh!

zyv commented Oct 23, 2025

Uh oh!

horkykuba commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zyv commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

horkykuba commented Oct 23, 2025

Uh oh!

zyv commented Oct 24, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ossilator left a comment

horkykuba commented Sep 29, 2025 •

edited

Loading

horkykuba commented Oct 23, 2025 •

edited

Loading

zyv commented Oct 23, 2025 •

edited

Loading