i9e: Avoid key binding macros.
authorAndre Noll <maan@tuebingen.mpg.de>
Sat, 3 Oct 2015 17:51:38 +0000 (17:51 +0000)
committerAndre Noll <maan@tuebingen.mpg.de>
Sun, 25 Oct 2015 15:13:43 +0000 (16:13 +0100)
The key binding code of interactive.c is quite an ugly hack which is
marked with a FIXME comment since forever. We bind each key sequence
given in the ->bound_keyseqs array of struct i9e_client_info to a
key code which is then mapped to a command handler provoded by the
application. The bindings started at key 64 with an arbitrary limit
of 32 possible mappings.

Besides being ugly, the problem with this mapping scheme is that
upper case keys are also in this range and are hence also mapped to
the specified commands.

This commit takes another approach. We now bind key sequences to a
function instead of a macro, passing ISFUNC as the first parameter to
libreadline's rl_generic_bind(). All key sequences are bound to the
same function, dispatch_key(), which calls the application-defined
key handler.

This is still not optimal because we need to look up the key sequence
again in dispatch_key(). But since this is not a fast path anyway,
it should be OK.

interactive.c

index 484f9550cae9a8e03e2cd76843bd03d7d25fb825..4de81d0d4a394df6d20f5b3a427ee768118e7981 100644 (file)
@@ -26,6 +26,7 @@ struct i9e_private {
        struct i9e_client_info *ici;
        FILE *stderr_stream;
        int num_columns;
+       int num_key_bindings;
        char empty_line[1000];
        struct task *task;
        struct btr_node *stdout_btrn;
@@ -417,15 +418,17 @@ static void update_winsize(void)
  * Defined key sequences are mapped to keys starting with this offset. I.e.
  * pressing the first defined key sequence yields the key number \p KEY_OFFSET.
  */
-#define KEY_OFFSET 64
-
-static int dispatch_key(__a_unused int count, int key)
+static int dispatch_key(__a_unused int count, __a_unused int key)
 {
-       int ret;
+       int i, ret;
 
-       assert(key >= KEY_OFFSET);
-       ret = i9ep->ici->key_handler(key - KEY_OFFSET);
-       return ret < 0? ret : 0;
+       for (i = i9ep->num_key_bindings - 1; i >= 0; i--) {
+               if (strcmp(rl_executing_keyseq, i9ep->ici->bound_keyseqs[i]))
+                       continue;
+               ret = i9ep->ici->key_handler(i);
+               return ret < 0? ret : 0;
+       }
+       assert(0);
 }
 
 /**
@@ -469,13 +472,11 @@ int i9e_open(struct i9e_client_info *ici, struct sched *s)
        if (ici->bound_keyseqs) {
                char *seq;
                int i;
-               /* FIXME: This is an arbitrary constant.  */
-               for (i = 0; i < 32 && (seq = ici->bound_keyseqs[i]); i++) {
-                       char buf[2] = {KEY_OFFSET + i, '\0'};
-                       /* readline needs an allocated buffer for the macro */
-                       rl_generic_bind(ISMACR, seq, para_strdup(buf), i9ep->bare_km);
-                       rl_bind_key_in_map(KEY_OFFSET + i, dispatch_key, i9ep->bare_km);
-               }
+               /* bind each key sequence to the our dispatcher */
+               for (i = 0; (seq = ici->bound_keyseqs[i]); i++)
+                       rl_generic_bind(ISFUNC, seq, (char *)dispatch_key,
+                               i9ep->bare_km);
+               i9ep->num_key_bindings = i;
        }
        if (ici->history_file)
                read_history(ici->history_file);