Hi
A few comments inline:
> diff --git a/format.c b/format.c
> index 6f988b9ac2cc..8630aabed5d7 100644
> --- a/format.c
> +++ b/format.c
> @@ -435,7 +435,8 @@ format_client(struct format_tree *ft, struct client *c)
> *strchr(tim, '\n') = '\0';
> format_add(ft, "client_activity_string", "%s", tim);
>
> - format_add(ft, "client_prefix", "%d", !!(c->flags & CLIENT_PREFIX));
> + format_add(ft, "client_prefix", strcmp(c->keytable->name, "root") ?
> "1": "0");
> + format_add(ft, "client_keytablename", "%s", c->keytable->name);
Let's call it client_key_table to fit the style of most of the others.
>
> if (c->tty.flags & TTY_UTF8)
> format_add(ft, "client_utf8", "%d", 1);
> diff --git a/key-bindings.c b/key-bindings.c
> index 58be0c6fe896..d391744a1bff 100644
> --- a/key-bindings.c
> +++ b/key-bindings.c
> @@ -24,61 +24,143 @@
>
> #include "tmux.h"
>
> +struct key_table *key_bindings_lookup_table_noref(const char *, int);
> +
> RB_GENERATE(key_bindings, key_binding, entry, key_bindings_cmp);
> +RB_GENERATE(key_tables, key_table, entry, key_table_cmp);
> +
> +struct key_tables key_tables;
>
> -struct key_bindings key_bindings;
> +int
> +key_table_cmp(struct key_table *e1, struct key_table *e2)
> +{
> + return strcmp(e1->name, e2->name);
> +}
>
> int
> key_bindings_cmp(struct key_binding *bd1, struct key_binding *bd2)
> {
> - int key1, key2;
> -
> - key1 = bd1->key & ~KEYC_PREFIX;
> - key2 = bd2->key & ~KEYC_PREFIX;
> - if (key1 != key2)
> - return (key1 - key2);
> -
> - if (bd1->key & KEYC_PREFIX && !(bd2->key & KEYC_PREFIX))
> - return (-1);
> - if (bd2->key & KEYC_PREFIX && !(bd1->key & KEYC_PREFIX))
> - return (1);
> - return (0);
> + return (bd1->key - bd2->key);
> }
>
> -struct key_binding *
> -key_bindings_lookup(int key)
> +struct key_table *
> +key_bindings_lookup_table_noref(const char *name, int create)
> {
> - struct key_binding bd;
> + struct key_table table_search;
> + struct key_table *table;
>
> - bd.key = key;
> - return (RB_FIND(key_bindings, &key_bindings, &bd));
> + table_search.name = name;
> + table = RB_FIND(key_tables, &key_tables, &table_search);
> + if (table)
> + return table;
> +
> + if (!create)
> + return NULL;
> +
> + table = xmalloc(sizeof *table);
> + table->name = table->name_for_free = xstrdup(name);
> + RB_INIT(&(table->key_bindings));
> + /* for key_tables */
> + table->references = 1;
> + RB_INSERT(key_tables, &key_tables, table);
> +
> + return table;
> +}
> +
> +struct key_table *
> +key_bindings_lookup_table(const char *name, int create)
I don't think it is intuitive for a lookup function to take a
reference. I would do without this function and just do lookup followed
by table->references++ in the caller.
Personally I would also split this into two functions, one for lookup
and one for create and do this in the caller if create is needed:
t = key_bindings_lookup_table("foo");
if (t == NULL)
t = key_bindings_create_table("foo");
But if you prefer a create flag that is probably okay, although in that
case I'd rather not call the function _lookup_table. Maybe _get_table
instead?
> +{
> + struct key_table *table;
> +
> + table = key_bindings_lookup_table_noref(name, create);
> + if (!table)
> + return NULL;
> +
> + table->references++;
> + return table;
> }
>
> void
> -key_bindings_add(int key, int can_repeat, struct cmd_list *cmdlist)
> +key_bindings_unreference_table(struct key_table *table)
I'd do s/unreference/unref/ here to make this a bit less of a mouthful.
> {
> struct key_binding *bd;
>
> - key_bindings_remove(key);
> + if (--table->references == 0) {
> + while (!RB_EMPTY(&(table->key_bindings))) {
> + bd = RB_ROOT(&(table->key_bindings));
> + RB_REMOVE(key_bindings, &(table->key_bindings), bd);
> + cmd_list_free(bd->cmdlist);
> + free(bd);
> + }
> + free(table->name_for_free);
> + free(table);
> + }
> +}
> +
> +void
> +key_bindings_add(const char *name, int key, int can_repeat, struct cmd_list
> *cmdlist)
> +{
> + struct key_table *table;
> + struct key_binding bd_search;
> + struct key_binding *bd;
> +
> + table = key_bindings_lookup_table_noref(name, 1);
> +
> + bd_search.key = key;
> + bd = RB_FIND(key_bindings, &(table->key_bindings), &bd_search);
> + if (bd != NULL) {
> + RB_REMOVE(key_bindings, &(table->key_bindings), bd);
> + cmd_list_free(bd->cmdlist);
> + free(bd);
> + }
>
> bd = xmalloc(sizeof *bd);
> bd->key = key;
> - RB_INSERT(key_bindings, &key_bindings, bd);
> + RB_INSERT(key_bindings, &(table->key_bindings), bd);
>
> bd->can_repeat = can_repeat;
> bd->cmdlist = cmdlist;
> }
>
> void
> -key_bindings_remove(int key)
> +key_bindings_remove(const char *name, int key)
> {
> - struct key_binding *bd;
> + struct key_table *table;
> + struct key_binding bd_search;
> + struct key_binding *bd;
>
> - if ((bd = key_bindings_lookup(key)) == NULL)
> + table = key_bindings_lookup_table_noref(name, 0);
> + if (!table)
> return;
> - RB_REMOVE(key_bindings, &key_bindings, bd);
> +
> + bd_search.key = key;
> + bd = RB_FIND(key_bindings, &(table->key_bindings), &bd_search);
> + if (!bd)
> + return;
> +
> + RB_REMOVE(key_bindings, &(table->key_bindings), bd);
> cmd_list_free(bd->cmdlist);
> free(bd);
> +
> + if (RB_EMPTY(&(table->key_bindings))) {
> + RB_REMOVE(key_tables, &key_tables, table);
> + /* This represents key_tables's reference */
I don't think we need these comments. If you feel it needs to be
documented that the tree holds a reference, put a comment once either
with the references member in the struct in tmux.h or somewhere around
where the tree is declared.
> + key_bindings_unreference_table(table);
> + }
> +}
> +
> +void
> +key_bindings_remove_table(const char *name)
> +{
> + struct key_table *table;
> +
> + table = key_bindings_lookup_table_noref(name, 0);
> + if (!table)
> + return;
> +
> + RB_REMOVE(key_tables, &key_tables, table);
> + /* This represents key_tables's reference */
> + key_bindings_unreference_table(table);
> }
>
> void
> @@ -167,7 +249,7 @@ key_bindings_init(void)
> struct cmd *cmd;
> struct cmd_list *cmdlist;
>
> - RB_INIT(&key_bindings);
> + RB_INIT(&key_tables);
>
> for (i = 0; i < nitems(table); i++) {
> cmdlist = xcalloc(1, sizeof *cmdlist);
> @@ -183,7 +265,7 @@ key_bindings_init(void)
> TAILQ_INSERT_HEAD(&cmdlist->list, cmd, qentry);
>
> key_bindings_add(
> - table[i].key | KEYC_PREFIX, table[i].can_repeat, cmdlist);
> + "prefix", table[i].key, table[i].can_repeat, cmdlist);
> }
> }
>
> diff --git a/server-fn.c b/server-fn.c
> index e0859f707034..6f3491693d76 100644
> --- a/server-fn.c
> +++ b/server-fn.c
> @@ -101,6 +101,13 @@ server_status_client(struct client *c)
> }
>
> void
> +server_keytable_client(struct client *c, const char *name)
> +{
> + key_bindings_unreference_table(c->keytable);
> + c->keytable = key_bindings_lookup_table(name, 1);
I'm not convinced we need this helper function for two lines.
> +}
> +
> +void
> server_redraw_session(struct session *s)
> {
> struct client *c;
> diff --git a/tmux.h b/tmux.h
> index f9d6087ab714..dcd2c8d25f44 100644
> --- a/tmux.h
> +++ b/tmux.h
> @@ -164,10 +164,9 @@ extern char **environ;
> #define KEYC_ESCAPE 0x2000
> #define KEYC_CTRL 0x4000
> #define KEYC_SHIFT 0x8000
> -#define KEYC_PREFIX 0x10000
>
> /* Mask to obtain key w/o modifiers. */
> -#define KEYC_MASK_MOD (KEYC_ESCAPE|KEYC_CTRL|KEYC_SHIFT|KEYC_PREFIX)
> +#define KEYC_MASK_MOD (KEYC_ESCAPE|KEYC_CTRL|KEYC_SHIFT)
> #define KEYC_MASK_KEY (~KEYC_MASK_MOD)
>
> /* Other key codes. */
> @@ -1298,7 +1297,7 @@ struct client {
> struct screen status;
>
> #define CLIENT_TERMINAL 0x1
> -#define CLIENT_PREFIX 0x2
> +/* replaced by keytablename: #define CLIENT_PREFIX 0x2 */
Just remove this and either renumber the rest or put a comment /* 0x2
unused */.
> #define CLIENT_EXIT 0x4
> #define CLIENT_REDRAW 0x8
> #define CLIENT_STATUS 0x10
> @@ -1317,6 +1316,7 @@ struct client {
> #define CLIENT_256COLOURS 0x20000
> #define CLIENT_IDENTIFIED 0x40000
> int flags;
> + struct key_table *keytable;
>
> struct event identify_timer;
>
> @@ -1444,6 +1444,22 @@ struct key_binding {
> RB_ENTRY(key_binding) entry;
> };
> RB_HEAD(key_bindings, key_binding);
> +struct key_table {
> + /*
> + * Search needs to be able to use const char * and free needs to be
> + * able to free the name we allocated when we put it in the tree for
> + * real. Search and keying is done by name, free is done with
> + * name_for_free.
> + */
> + const char *name;
> + char *name_for_free;
Don't do this, it's confusing. Just use a const char * and cast it to
void * when you pass it to free.
> + struct key_bindings key_bindings;
> +
> + u_int references;
> +
> + RB_ENTRY(key_table) entry;
> +};
> +RB_HEAD(key_tables, key_table);
>
> /*
> * Option table entries. The option table is the user-visible part of the
> @@ -1865,12 +1881,16 @@ int cmd_string_parse(const char *, struct cmd_list
> **, const char *,
> int client_main(int, char **, int);
>
> /* key-bindings.c */
> -extern struct key_bindings key_bindings;
> -int key_bindings_cmp(struct key_binding *, struct key_binding *);
> RB_PROTOTYPE(key_bindings, key_binding, entry, key_bindings_cmp);
> -struct key_binding *key_bindings_lookup(int);
> -void key_bindings_add(int, int, struct cmd_list *);
> -void key_bindings_remove(int);
> +RB_PROTOTYPE(key_tables, key_table, entry, key_table_cmp);
> +extern struct key_tables key_tables;
> +int key_table_cmp(struct key_table *, struct key_table *);
> +int key_bindings_cmp(struct key_binding *, struct key_binding *);
> +struct key_table *key_bindings_lookup_table(const char *, int);
> +void key_bindings_unreference_table(struct key_table *);
> +void key_bindings_add(const char *, int, int, struct cmd_list *);
> +void key_bindings_remove(const char *, int);
> +void key_bindings_remove_table(const char *);
> void key_bindings_init(void);
> void key_bindings_dispatch(struct key_binding *, struct client *);
>
> @@ -1906,6 +1926,7 @@ void server_write_session(struct session *, enum
> msgtype, const void *,
> size_t);
> void server_redraw_client(struct client *);
> void server_status_client(struct client *);
> +void server_keytable_client(struct client *, const char *);
> void server_redraw_session(struct session *);
> void server_redraw_session_group(struct session *);
> void server_status_session(struct session *);
> --
> 1.9.1
>
>
> ------------------------------------------------------------------------------
> "Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE
> Instantly run your Selenium tests across 300+ browser/OS combos.
> Get unparalleled scalability from the best Selenium testing platform available
> Simple to use. Nothing to install. Get started now for free."
> http://p.sf.net/sfu/SauceLabs
> _______________________________________________
> tmux-users mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/tmux-users
------------------------------------------------------------------------------
"Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE
Instantly run your Selenium tests across 300+ browser/OS combos.
Get unparalleled scalability from the best Selenium testing platform available
Simple to use. Nothing to install. Get started now for free."
http://p.sf.net/sfu/SauceLabs
_______________________________________________
tmux-users mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/tmux-users
