Fix kitty keyboard protocol parsing of ctrl-o

Commit 6e4510b (subshell: recognize CSI u encoding for ctrl-o, 2024-10-09)
made us parse sequences like \e[111;5u (meaning ctrl-o).  As reported in
fish-shell/fish-shell#10640 (comment),
the kitty keyboard protocol specifies various other sequences that also
mean ctrl-o. Specifically, when numlock is active.

Instead of matching raw byte values, let's add teach our ECMA-48 CSI parser to
decode parameter bytes and interpret them according to the kitty keyboard protocol.

Tested with fish version 4:

	1. Turn on numlock
	2. SHELL=/bin/fish src/mc
	3. ctrl-o
	4. Enter
	5. ctrl-o -- confirmed that this one is recognized now

Note that capslock still disables ctrl-o. We could change that easily
(and for consistency also allow ctrl-O).

While at it, add some unit tests (using a fish prompt as input data).

configure.ac

@@ -719,6 +719,7 @@ tests/lib/widget/Makefile
 tests/src/Makefile
 tests/src/filemanager/Makefile
 tests/src/editor/Makefile
+tests/src/lib/Makefile
 tests/src/editor/test-data.txt
 tests/src/vfs/Makefile
 tests/src/vfs/extfs/Makefile

lib/terminal.c

@@ -1,23 +1,116 @@
 #include <config.h>
 
+#include <assert.h>
+
 #include "lib/terminal.h"
 #include "lib/util.h"
 #include "lib/strutil.h"
 
 /* --------------------------------------------------------------------------------------------- */
+
 /**
- * Remove all control sequences from the argument string.  We define
- * "control sequence", in a sort of pidgin BNF, as follows:
+ * Parse a CSI command, starting from the third byte (i.e. the first
+ * parameter byte, if any).
+ *
+ * On success, *sptr will point to one-past the end of the sequence.
+ * On failure, *sptr will point to the first invalid byte.
+ *
+ * Here's the format in a sort of pidgin BNF:
  *
- * control-seq = Esc non-'['
- *             | Esc '[' (parameter-byte)* (intermediate-byte)* final-byte
+ * CSI-command = Esc '[' parameters (intermediate-byte)* final-byte
+ * parameters = [0-9;:]+
+ *            | [<=>?] (parameter-byte)* # private mode
  * parameter-byte = [\x30-\x3F]     # one of "0-9;:<=>?"
  * intermediate-byte = [\x20–\x2F]  # one of " !\"#$%&'()*+,-./"
  * final-byte = [\x40-\x7e]         # one of "@A–Z[\]^_`a–z{|}~"
+ */
+gboolean
+parse_csi (struct csi_command_t *out, const char **sptr, const char *end)
+{
+    gboolean ok = FALSE;
+    const char *s = *sptr;
+    if (s == end)
+        goto invalid_sequence;
+    char c = *s;
+
+#define NEXT_CHAR                                                                                  \
+    do                                                                                             \
+    {                                                                                              \
+        if (++s == end)                                                                            \
+            goto invalid_sequence;                                                                 \
+        c = *s;                                                                                    \
+    }                                                                                              \
+    while (0)
+
+    char private_mode = '\0';
+    assert (c != '\0');
+    if (strchr ("<=>?", c) != NULL)
+    {
+        private_mode = c;
+        NEXT_CHAR;
+    }
+
+    // parameter bytes
+    size_t param_count = 0;
+    if (private_mode)
+        while (c >= 0x30 && c <= 0x3F)
+            NEXT_CHAR;
+    else
+    {
+        if (out)
+            // N.B. Empty parameter strings are allowed. For our current use,
+            // defaulting to zero happens to work.
+            memset (out->params, 0, sizeof (out->params));
+        uint32_t tmp = 0;
+        size_t sub_index = 0;
+        while (c >= 0x30 && c <= 0x3F)
+        {
+            if (c >= '0' && c <= '9')
+            {
+                if (param_count == 0)
+                    param_count = 1;
+                if (tmp * 10 < tmp)
+                    goto invalid_sequence;
+                tmp *= 10;
+                tmp += c - '0';
+                if (out)
+                    out->params[param_count - 1][sub_index] = tmp;
+            }
+            else if (c == ':' && ++sub_index < ARRAY_LEN (out->params[0]))
+                tmp = 0;
+            else if (c == ';' && ++param_count <= ARRAY_LEN (out->params))
+                tmp = sub_index = 0;
+            else
+                goto invalid_sequence;
+            NEXT_CHAR;
+        }
+    }
+
+    while (c >= 0x20 && c <= 0x2F)  // intermediate bytes
+        NEXT_CHAR;
+#undef NEXT_CHAR
+    if (c < 0x40 || c > 0x7E)  // final byte
+        goto invalid_sequence;
+    ++s;
+    ok = TRUE;
+    if (out)
+    {
+        out->private_mode = private_mode;
+        out->param_count = param_count;
+    }
+invalid_sequence:
+    *sptr = s;
+    return ok;
+}
+
+/* ---------------------------------------------------------------------------------------------
+ */
+/**
+ * Remove all control sequences (CSI, OSC) from the argument string.
  *
- * The 256-color and true-color escape sequences should allow either ';' or ':' inside as separator,
- * actually, ':' is the more correct according to ECMA-48.
- * Some terminal emulators (e.g. xterm, gnome-terminal) support this.
+ * The 256-color and true-color escape sequences should allow either ';' or ':' inside as
+ * separator, actually, ':' is the more correct according to ECMA-48. Some terminal emulators
+ * (e.g. xterm, gnome-terminal) support this.
  *
  * Non-printable characters are also removed.
  */
@@ -39,11 +132,10 @@ strip_ctrl_codes (char *s)
             // '(' need to avoid strange 'B' letter in *Suse (if mc runs under root user)
             if (*(++r) == '[' || *r == '(')
             {
-                // strchr() matches trailing binary 0
-                while (*(++r) != '\0' && strchr ("0123456789;:<=>?", *r) != NULL)
-                    ;
-                while (*r != '\0' && (*r < 0x40 || *r > 0x7E))
-                    ++r;
+                ++r;
+                parse_csi (NULL, &r, r + strlen (r));
+                // We're already past the sequence, no need to increment.
+                continue;
             }
             else if (*r == ']')
             {
@@ -104,6 +196,7 @@ strip_ctrl_codes (char *s)
 }
 
 /* --------------------------------------------------------------------------------------------- */
+
 /**
  * Convert "\E" -> esc character and ^x to control-x key and ^^ to ^ key
  *
@@ -160,5 +253,3 @@ convert_controls (const char *p)
     *q = '\0';
     return valcopy;
 }
-
-/* --------------------------------------------------------------------------------------------- */

lib/terminal.h

@@ -10,6 +10,14 @@
 
 #include "lib/global.h"  // include <glib.h>
 
+struct csi_command_t
+{
+    char private_mode;
+    uint32_t params[16][4];
+    size_t param_count;
+};
+gboolean parse_csi (struct csi_command_t *out, const char **sptr, const char *end);
+
 char *strip_ctrl_codes (char *s);
 
 /* Replaces "\\E" and "\\e" with "\033". Replaces "^" + [a-z] with

lib/util.h

@@ -73,7 +73,9 @@
 #define _GL_CMP(n1, n2) (((n1) > (n2)) - ((n1) < (n2)))
 
 /* Difference or zero */
-#define DOZ(a, b) ((a) > (b) ? (a) - (b) : 0)
+#define DOZ(a, b)        ((a) > (b) ? (a) - (b) : 0)
+
+#define ARRAY_LEN(array) (sizeof (array) / sizeof ((array)[0]))
 
 /* flags for shell_execute */
 #define EXECUTE_INTERNAL (1 << 0)

src/subshell/common.c

@@ -118,7 +118,7 @@
 
 /* State of the subshell:
  * INACTIVE: the default state; awaiting a command
- * ACTIVE: remain in the shell until the user hits 'subshell_switch_key'
+ * ACTIVE: remain in the shell until the user hits the subshell switch key
  * RUNNING_COMMAND: return to MC when the current command finishes */
 enum subshell_state_enum subshell_state;
 
@@ -178,12 +178,6 @@ static char tcsh_fifo[128];
 
 static int subshell_pty_slave = -1;
 
-/* The key for switching back to MC from the subshell */
-static const char subshell_switch_key = XCTRL ('o') & 255;
-
-static const char subshell_switch_key_csi_u[] = "\x1b[111;5u";
-static const size_t subshell_switch_key_csi_u_len = sizeof (subshell_switch_key_csi_u) - 1;
-
 /* For reading/writing on the subshell's pty */
 static char pty_buffer[PTY_BUFFER_SIZE] = "\0";
 
@@ -783,6 +777,63 @@ set_prompt_string (void)
     setup_cmdline ();
 }
 
+static gboolean
+peek_subshell_switch_key (const char *buffer, size_t len)
+{
+    if (len == 0)
+        return FALSE;
+    if (buffer[0] == (XCTRL ('o') & 255))
+        return TRUE;
+
+    // Also check if ctrl-o is encoded as specified by the kitty keyboard protocol.
+    if (len == 1)
+        return FALSE;
+    if (buffer[0] != ESC_CHAR || buffer[1] != '[')  // CSI
+        return FALSE;
+    struct csi_command_t csi;
+    buffer += 2;
+    len -= 2;
+    if (!parse_csi (&csi, &buffer, buffer + len))
+        return FALSE;
+
+    const char csi_final_byte = buffer[-1];
+    if (csi.private_mode || csi_final_byte != 'u')
+        return FALSE;
+    if (csi.param_count != 2)
+        return FALSE;
+    uint32_t codepoint = csi.params[0][0];
+    const uint32_t shifted_key = csi.params[0][1];
+    if (csi.params[1][0] == 0)  // Bad modifier.
+        return FALSE;
+    uint32_t modifiers = csi.params[1][0] - 1;
+    const uint32_t event = csi.params[1][1];
+
+    enum
+    {
+        shift = 0x01,
+        ctrl = 0x04,
+        caps_lock = 0x40,
+        num_lock = 0x80,
+
+        evt_press = 1,
+        evt_repeat = 2
+    };
+
+    // Apply any shift modifier.
+    // Note that if both shift and caps set, there should be no shifted key.
+    if ((modifiers & (shift | caps_lock)) && shifted_key)
+    {
+        codepoint = shifted_key;
+        modifiers &= ~shift;
+    }
+
+    if (codepoint != 'o' || (modifiers & ~(caps_lock | num_lock)) != ctrl)
+        return FALSE;
+    if (event && event != evt_press && event != evt_repeat)
+        return FALSE;
+    return TRUE;
+}
+
 /* --------------------------------------------------------------------------------------------- */
 /** Feed the subshell our keyboard input until it says it's finished */
 
@@ -912,11 +963,7 @@ feed_subshell (int how, gboolean fail_on_error)
             }
 
             for (i = 0; i < bytes; ++i)
-                if (pty_buffer[i] == subshell_switch_key
-                    || (subshell_switch_key_csi_u_len <= (size_t) bytes - i
-                        && memcmp (&pty_buffer[i], subshell_switch_key_csi_u,
-                                   subshell_switch_key_csi_u_len)
-                            == 0))
+                if (peek_subshell_switch_key (pty_buffer + i, bytes - i))
                 {
                     write_all (mc_global.tty.subshell_pty, pty_buffer, i);
 

tests/src/lib/.gitignore

@@ -0,0 +1 @@
+terminal

tests/src/lib/Makefile.am

@@ -0,0 +1,23 @@
+PACKAGE_STRING = "/src/lib"
+
+AM_CPPFLAGS = \
+	-DTEST_SHARE_DIR=\"$(abs_srcdir)\" \
+	$(GLIB_CFLAGS) \
+	-I$(top_srcdir) \
+	@CHECK_CFLAGS@
+
+LIBS = @CHECK_LIBS@ \
+	$(top_builddir)/src/libinternal.la \
+	$(top_builddir)/lib/libmc.la
+
+if ENABLE_MCLIB
+LIBS += $(GLIB_LIBS)
+endif
+
+TESTS = \
+	terminal
+
+check_PROGRAMS = $(TESTS)
+
+edit_complete_word_cmd_SOURCES = \
+	terminal.c

tests/src/lib/terminal.c

@@ -0,0 +1,72 @@
+#include <string.h>
+#include <assert.h>
+
+#include <config.h>
+#include "lib/global.h"  // include <glib.h>
+#include "lib/terminal.h"
+#include "lib/strutil.h"
+
+#define TEST_SUITE_NAME "/src/lib/terminal"
+
+#include "tests/mctest.h"
+
+/* --------------------------------------------------------------------------------------------- */
+
+static void
+setup (void)
+{
+    str_init_strings (NULL);
+}
+
+static void
+teardown (void)
+{
+    str_uninit_strings ();
+}
+
+/* --------------------------------------------------------------------------------------------- */
+
+START_TEST (test_parse_csi)
+{
+    const char *s = &"\x1b[=5uRest"[2];
+    const char *end = s + strlen (s);
+    gboolean ok = parse_csi (NULL, &s, end);
+    ck_assert_msg (ok, "failed to parse");
+    ck_assert_str_eq (s, "Rest");
+}
+END_TEST
+
+/* --------------------------------------------------------------------------------------------- */
+
+START_TEST (test_strip_ctrl_codes)
+{
+    char *s = strdup (
+        "\033]0;~\a\033[30m\033(B\033[m\033]133;A;special_key=1\a$ "
+        "\033[K\033[?2004h\033[>4;1m\033[=5u\033=\033[?2004l\033[>4;0m\033[=0u\033>\033[?2004h\033["
+        ">4;1m\033[=5u\033=\033[?2004l\033[>4;0m\033[=0u\033>\033[?2004h\033[>4;1m\033[=5u\033=");
+    char *actual = strip_ctrl_codes (s);
+    const char *expected = "$ ";
+    ck_assert_str_eq (actual, expected);
+    free (s);
+}
+
+/* --------------------------------------------------------------------------------------------- */
+
+int
+main (void)
+{
+    TCase *tc_core;
+
+    tc_core = tcase_create ("Core");
+
+    tcase_add_checked_fixture (tc_core, setup, teardown);
+
+    // Add new tests here: ***************
+    tcase_add_test (tc_core, test_parse_csi);
+    tcase_add_test (tc_core, test_strip_ctrl_codes);
+    // ***********************************
+
+    return mctest_run_all (tc_core);
+}
+
+/* --------------------------------------------------------------------------------------------- */