From fc2bacbb7891228b0005d8d6139a0129570a65ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sat, 4 Dec 2021 21:53:48 +0100 Subject: [PATCH 01/14] config: remove struct key_combo, use config_key_binding instead --- config.c | 325 +++++++++++++++++++++++++------------------------------ 1 file changed, 146 insertions(+), 179 deletions(-) diff --git a/config.c b/config.c index 92a0fb53..6e9d3b77 100644 --- a/config.c +++ b/config.c @@ -1443,32 +1443,27 @@ parse_section_csd(struct context *ctx) return true; } -/* Struct that holds temporary key/mouse binding parsed data */ -struct key_combo { - char *text; /* Raw text, e.g. "Control+Shift+V" */ - struct config_key_modifiers modifiers; - union { - xkb_keysym_t sym; /* Key converted to an XKB symbol, e.g. XKB_KEY_V */ - struct { - int button; - int count; - } m; - }; -}; - -struct key_combo_list { - size_t count; - struct key_combo *combos; -}; +static void +binding_pipe_free(struct config_binding_pipe *pipe) +{ + if (pipe->master_copy) + free_argv(&pipe->argv); +} static void NOINLINE -free_key_combo_list(struct key_combo_list *key_combos) +key_binding_list_free(struct config_key_binding_list *bindings) { - for (size_t i = 0; i < key_combos->count; i++) - free(key_combos->combos[i].text); - free(key_combos->combos); - key_combos->count = 0; - key_combos->combos = NULL; + for (size_t i = 0; i < bindings->count; i++) + binding_pipe_free(&bindings->arr[i].pipe); + free(bindings->arr); +} + +static void +mouse_binding_list_free(struct config_mouse_binding_list *bindings) +{ + for (size_t i = 0; i < bindings->count; i++) + binding_pipe_free(&bindings->arr[i].pipe); + free(bindings->arr); } static bool @@ -1511,10 +1506,11 @@ out: } static bool -value_to_key_combos(struct context *ctx, struct key_combo_list *key_combos) +value_to_key_combos(struct context *ctx, + struct config_key_binding_list *key_combos) { xassert(key_combos != NULL); - xassert(key_combos->count == 0 && key_combos->combos == NULL); + xassert(key_combos->count == 0 && key_combos->arr == NULL); size_t size = 0; /* Size of ‘combos’ array in the key-combo list */ @@ -1545,13 +1541,12 @@ value_to_key_combos(struct context *ctx, struct key_combo_list *key_combos) if (key_combos->count + 1 > size) { size += 4; - key_combos->combos = xrealloc( - key_combos->combos, size * sizeof(key_combos->combos[0])); + key_combos->arr = xrealloc( + key_combos->arr, size * sizeof(key_combos->arr[0])); } xassert(key_combos->count + 1 <= size); - key_combos->combos[key_combos->count++] = (struct key_combo){ - .text = xstrdup(combo), + key_combos->arr[key_combos->count++] = (struct config_key_binding){ .modifiers = modifiers, .sym = sym, }; @@ -1561,7 +1556,7 @@ value_to_key_combos(struct context *ctx, struct key_combo_list *key_combos) return true; err: - free_key_combo_list(key_combos); + key_binding_list_free(key_combos); free(copy); return false; } @@ -1594,11 +1589,45 @@ argv_compare(const struct argv *argv1, const struct argv *argv2) return 1; } +static bool +modifiers_equal(const struct config_key_modifiers *mods1, + const struct config_key_modifiers *mods2) +{ + bool shift = mods1->shift == mods2->shift; + bool alt = mods1->alt == mods2->alt; + bool ctrl = mods1->ctrl == mods2->ctrl; + bool meta = mods1->meta == mods2->meta; + return shift && alt && ctrl && meta; +} + +static bool +modifiers_disjoint(const struct config_key_modifiers *mods1, + const struct config_key_modifiers *mods2) +{ + bool shift = mods1->shift && mods2->shift; + bool alt = mods1->alt && mods2->alt; + bool ctrl = mods1->ctrl && mods2->ctrl; + bool meta = mods1->meta && mods2->meta; + return !(shift || alt || ctrl || meta); +} + +static char * +modifiers_to_str(const struct config_key_modifiers *mods) +{ + char *ret = xasprintf("%s%s%s%s", + mods->ctrl ? "Control+" : "", + mods->alt ? "Alt+": "", + mods->meta ? "Meta+": "", + mods->shift ? "Shift+": ""); + ret[strlen(ret) - 1] = '\0'; + return ret; +} + static bool has_key_binding_collisions(struct context *ctx, int action, const char *const action_map[], const struct config_key_binding_list *bindings, - const struct key_combo_list *key_combos, + const struct config_key_binding_list *key_combos, const struct argv *pipe_argv) { for (size_t j = 0; j < bindings->count; j++) { @@ -1613,25 +1642,29 @@ has_key_binding_collisions(struct context *ctx, } for (size_t i = 0; i < key_combos->count; i++) { - const struct key_combo *combo2 = &key_combos->combos[i]; + const struct config_key_binding *combo2 = &key_combos->arr[i]; const struct config_key_modifiers *mods1 = &combo1->modifiers; const struct config_key_modifiers *mods2 = &combo2->modifiers; - bool shift = mods1->shift == mods2->shift; - bool alt = mods1->alt == mods2->alt; - bool ctrl = mods1->ctrl == mods2->ctrl; - bool meta = mods1->meta == mods2->meta; - bool sym = combo1->sym == combo2->sym; + bool mods_equal = modifiers_equal(mods1, mods2); + bool sym_equal = combo1->sym == combo2->sym; - if (shift && alt && ctrl && meta && sym) { + if (mods_equal && sym_equal) { bool has_pipe = combo1->pipe.argv.args != NULL; - LOG_CONTEXTUAL_ERR("%s already mapped to '%s%s%s%s'", - combo2->text, + + char *modifier_names = modifiers_to_str(mods2); + char sym_name[64]; + xkb_keysym_get_name(combo2->sym, sym_name, sizeof(sym_name)); + + LOG_CONTEXTUAL_ERR("%s+%s already mapped to '%s%s%s%s'", + modifier_names, sym_name, action_map[combo1->action], has_pipe ? " [" : "", has_pipe ? combo1->pipe.argv.args[0] : "", has_pipe ? "]" : ""); + + free(modifier_names); return true; } } @@ -1757,13 +1790,12 @@ parse_key_binding_section(struct context *ctx, return true; } - struct key_combo_list key_combos = {0}; + struct config_key_binding_list key_combos = {0}; if (!value_to_key_combos(ctx, &key_combos) || - has_key_binding_collisions( - ctx, action, action_map, bindings, &key_combos, &pipe_argv)) + has_key_binding_collisions(ctx, action, action_map, bindings, &key_combos, &pipe_argv)) { free_argv(&pipe_argv); - free_key_combo_list(&key_combos); + key_binding_list_free(&key_combos); return false; } @@ -1777,23 +1809,17 @@ parse_key_binding_section(struct context *ctx, bool first = true; for (size_t i = 0; i < key_combos.count; i++) { - const struct key_combo *combo = &key_combos.combos[i]; - struct config_key_binding binding = { - .action = action, - .modifiers = combo->modifiers, - .sym = combo->sym, - .pipe = { - .argv = pipe_argv, - .master_copy = first, - }, - }; + struct config_key_binding *binding = &bindings->arr[ofs + i]; + + *binding = key_combos.arr[i]; + binding->action = action; + binding->pipe.argv = pipe_argv; + binding->pipe.master_copy = first; - /* TODO: we could re-use free:d slots */ - bindings->arr[ofs + i] = binding; first = false; } - free_key_combo_list(&key_combos); + key_binding_list_free(&key_combos); return true; } @@ -1982,10 +2008,11 @@ mouse_event_code_get_name(int code) } static bool -value_to_mouse_combos(struct context *ctx, struct key_combo_list *key_combos) +value_to_mouse_combos(struct context *ctx, + struct config_mouse_binding_list *mouse_combos) { - xassert(key_combos != NULL); - xassert(key_combos->count == 0 && key_combos->combos == NULL); + xassert(mouse_combos != NULL); + xassert(mouse_combos->count == 0 && mouse_combos->arr == NULL); size_t size = 0; /* Size of the ‘combos’ array in key_combos */ @@ -2042,93 +2069,55 @@ value_to_mouse_combos(struct context *ctx, struct key_combo_list *key_combos) goto err; } - struct key_combo new = { - .text = xstrdup(combo), + struct config_mouse_binding new = { .modifiers = modifiers, - .m = { - .button = button, - .count = count, - }, + .button = button, + .count = count, }; - if (key_combos->count + 1 > size) { + if (mouse_combos->count + 1 > size) { size += 4; - key_combos->combos = xrealloc( - key_combos->combos, size * sizeof(key_combos->combos[0])); + mouse_combos->arr = xrealloc( + mouse_combos->arr, size * sizeof(mouse_combos->arr[0])); } - xassert(key_combos->count + 1 <= size); - key_combos->combos[key_combos->count++] = new; + xassert(mouse_combos->count + 1 <= size); + mouse_combos->arr[mouse_combos->count++] = new; } free(copy); return true; err: - free_key_combo_list(key_combos); + mouse_binding_list_free(mouse_combos); free(copy); return false; } -static bool -modifiers_equal(const struct config_key_modifiers *mods1, - const struct config_key_modifiers *mods2) -{ - bool shift = mods1->shift == mods2->shift; - bool alt = mods1->alt == mods2->alt; - bool ctrl = mods1->ctrl == mods2->ctrl; - bool meta = mods1->meta == mods2->meta; - return shift && alt && ctrl && meta; -} - -static bool -modifiers_disjoint(const struct config_key_modifiers *mods1, - const struct config_key_modifiers *mods2) -{ - bool shift = mods1->shift && mods2->shift; - bool alt = mods1->alt && mods2->alt; - bool ctrl = mods1->ctrl && mods2->ctrl; - bool meta = mods1->meta && mods2->meta; - return !(shift || alt || ctrl || meta); -} - static char * -modifiers_to_str(const struct config_key_modifiers *mods) -{ - char *ret = xasprintf("%s%s%s%s", - mods->ctrl ? "Control+" : "", - mods->alt ? "Alt+": "", - mods->meta ? "Meta+": "", - mods->shift ? "Shift+": ""); - ret[strlen(ret) - 1] = '\0'; - return ret; -} - -static char * -mouse_combo_to_str(const struct key_combo *combo) +mouse_combo_to_str(const struct config_mouse_binding *combo) { char *combo_modifiers_str = modifiers_to_str(&combo->modifiers); - const char *combo_button_str = mouse_event_code_get_name(combo->m.button); + const char *combo_button_str = mouse_event_code_get_name(combo->button); xassert(combo_button_str != NULL); char *ret; - if (combo->m.count == 1) + if (combo->count == 1) ret = xasprintf("%s+%s", combo_modifiers_str, combo_button_str); else ret = xasprintf("%s+%s-%d", combo_modifiers_str, combo_button_str, - combo->m.count); + combo->count); free (combo_modifiers_str); return ret; } static bool -selection_override_interferes_with_mouse_binding(struct context *ctx, - int action, - const struct key_combo_list *key_combos, - bool blame_modifiers) +selection_override_interferes_with_mouse_binding( + struct context *ctx, int action, + const struct config_mouse_binding_list *mouse_combos, bool blame_modifiers) { struct config *conf = ctx->conf; @@ -2137,8 +2126,8 @@ selection_override_interferes_with_mouse_binding(struct context *ctx, const struct config_key_modifiers *override_mods = &conf->mouse.selection_override_modifiers; - for (size_t i = 0; i < key_combos->count; i++) { - const struct key_combo *combo = &key_combos->combos[i]; + for (size_t i = 0; i < mouse_combos->count; i++) { + const struct config_mouse_binding *combo = &mouse_combos->arr[i]; if (!modifiers_disjoint(&combo->modifiers, override_mods)) { char *modifiers_str = modifiers_to_str(override_mods); @@ -2163,8 +2152,9 @@ selection_override_interferes_with_mouse_binding(struct context *ctx, } static bool -has_mouse_binding_collisions(struct context *ctx, - const struct key_combo_list *key_combos) +has_mouse_binding_collisions( + struct context *ctx, + const struct config_mouse_binding_list *mouse_combos) { struct config *conf = ctx->conf; @@ -2173,23 +2163,33 @@ has_mouse_binding_collisions(struct context *ctx, if (combo1->action == BIND_ACTION_NONE) continue; - for (size_t i = 0; i < key_combos->count; i++) { - const struct key_combo *combo2 = &key_combos->combos[i]; + for (size_t i = 0; i < mouse_combos->count; i++) { + const struct config_mouse_binding *combo2 = &mouse_combos->arr[i]; const struct config_key_modifiers *mods1 = &combo1->modifiers; const struct config_key_modifiers *mods2 = &combo2->modifiers; - bool button = combo1->button == combo2->m.button; - bool count = combo1->count == combo2->m.count; + bool button = combo1->button == combo2->button; + bool count = combo1->count == combo2->count; if (modifiers_equal(mods1, mods2) && button && count) { bool has_pipe = combo1->pipe.argv.args != NULL; - LOG_CONTEXTUAL_ERR("%s already mapped to '%s%s%s%s'", - combo2->text, + + char *modifier_names = modifiers_to_str(mods2); + const char *btn_name = mouse_event_code_get_name(combo2->button); + + char count_string[16] = {0}; + if (combo2->count > 1) + sprintf(count_string, "-%d", combo2->count); + + LOG_CONTEXTUAL_ERR("%s+%s%s already mapped to '%s%s%s%s'", + modifier_names, btn_name, count_string, binding_action_map[combo1->action], has_pipe ? " [" : "", has_pipe ? combo1->pipe.argv.args[0] : "", has_pipe ? "]" : ""); + + free(modifier_names); return true; } } @@ -2215,21 +2215,15 @@ parse_section_mouse_bindings(struct context *ctx) /* Ensure no existing bindings use these modifiers */ for (size_t i = 0; i < conf->bindings.mouse.count; i++) { - const struct config_mouse_binding *binding = &conf->bindings.mouse.arr[i]; - struct key_combo combo = { - .modifiers = binding->modifiers, - .m = { - .button = binding->button, - .count = binding->count, - }, - }; - - struct key_combo_list key_combos = { + struct config_mouse_binding *binding = &conf->bindings.mouse.arr[i]; + struct config_mouse_binding_list mouse_combos = { .count = 1, - .combos = &combo, + .arr = binding, }; - if (selection_override_interferes_with_mouse_binding(ctx, binding->action, &key_combos, true)) { + if (selection_override_interferes_with_mouse_binding( + ctx, binding->action, &mouse_combos, true)) + { return false; } } @@ -2269,13 +2263,14 @@ parse_section_mouse_bindings(struct context *ctx) return true; } - struct key_combo_list key_combos = {0}; - if (!value_to_mouse_combos(ctx, &key_combos) || - has_mouse_binding_collisions(ctx, &key_combos) || - selection_override_interferes_with_mouse_binding(ctx, action, &key_combos, false)) + struct config_mouse_binding_list mouse_combos = {0}; + if (!value_to_mouse_combos(ctx, &mouse_combos) || + has_mouse_binding_collisions(ctx, &mouse_combos) || + selection_override_interferes_with_mouse_binding( + ctx, action, &mouse_combos, false)) { free_argv(&pipe_argv); - free_key_combo_list(&key_combos); + mouse_binding_list_free(&mouse_combos); return false; } @@ -2295,30 +2290,25 @@ parse_section_mouse_bindings(struct context *ctx) /* Emit mouse bindings */ size_t ofs = conf->bindings.mouse.count; - conf->bindings.mouse.count += key_combos.count; + conf->bindings.mouse.count += mouse_combos.count; conf->bindings.mouse.arr = xrealloc( conf->bindings.mouse.arr, conf->bindings.mouse.count * sizeof(conf->bindings.mouse.arr[0])); bool first = true; - for (size_t i = 0; i < key_combos.count; i++) { - const struct key_combo *combo = &key_combos.combos[i]; - struct config_mouse_binding binding = { - .action = action, - .modifiers = combo->modifiers, - .button = combo->m.button, - .count = combo->m.count, - .pipe = { - .argv = pipe_argv, - .master_copy = first, - }, - }; + for (size_t i = 0; i < mouse_combos.count; i++) { + struct config_mouse_binding *binding = + &conf->bindings.mouse.arr[ofs + i]; + + *binding = mouse_combos.arr[i]; + binding->action = action; + binding->pipe.argv = pipe_argv; + binding->pipe.master_copy = first; - conf->bindings.mouse.arr[ofs + i] = binding; first = false; } - free_key_combo_list(&key_combos); + mouse_binding_list_free(&mouse_combos); return true; } @@ -3118,13 +3108,6 @@ config_override_apply(struct config *conf, config_override_t *overrides, bool er return true; } -static void -binding_pipe_free(struct config_binding_pipe *pipe) -{ - if (pipe->master_copy) - free_argv(&pipe->argv); -} - static void binding_pipe_clone(struct config_binding_pipe *dst, const struct config_binding_pipe *src) @@ -3133,14 +3116,6 @@ binding_pipe_clone(struct config_binding_pipe *dst, clone_argv(&dst->argv, &src->argv); } -static void NOINLINE -key_binding_list_free(struct config_key_binding_list *bindings) -{ - for (size_t i = 0; i < bindings->count; i++) - binding_pipe_free(&bindings->arr[i].pipe); - free(bindings->arr); -} - static void NOINLINE key_binding_list_clone(struct config_key_binding_list *dst, const struct config_key_binding_list *src) @@ -3169,14 +3144,6 @@ key_binding_list_clone(struct config_key_binding_list *dst, } } -static void -mouse_binding_list_free(struct config_mouse_binding_list *bindings) -{ - for (size_t i = 0; i < bindings->count; i++) - binding_pipe_free(&bindings->arr[i].pipe); - free(bindings->arr); -} - static void NOINLINE mouse_binding_list_clone(struct config_mouse_binding_list *dst, const struct config_mouse_binding_list *src) From 197c1c5ced20f5783f2359a10d7096f0e1a438d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 5 Dec 2021 15:19:22 +0100 Subject: [PATCH 02/14] config: do key binding collision detection after loading the conf MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This allows the user to write, to swap two key bindings: [key-bindings] show-urls-launch=Control+Shift+r search-start=Control+Shift+u instead of: [key-bindings] search-start=none show-urls-launch=Control+Shift+r search-start=Control+Shift+u This should simplify the configuration for people who replace a lot of the default key bindings. This also simplifies the parsing somewhat, since we no longer has to parse a key-binding into a temporary list, ensure it doesn’t have any collisions, and then copy it into the actual key binding list. _While_ parsing a key-binding, we still need a temporary array (at least for the time being), but that temporary array is no longer visible outside the parsing function that allocates it. --- config.c | 457 ++++++++++++++++++++++++++++++------------------------- config.h | 4 + 2 files changed, 255 insertions(+), 206 deletions(-) diff --git a/config.c b/config.c index 6e9d3b77..e82385a7 100644 --- a/config.c +++ b/config.c @@ -119,8 +119,40 @@ static const char *const binding_action_map[] = { [BIND_ACTION_SELECT_ROW] = "select-row", }; +static const char *const search_binding_action_map[] = { + [BIND_ACTION_SEARCH_NONE] = NULL, + [BIND_ACTION_SEARCH_CANCEL] = "cancel", + [BIND_ACTION_SEARCH_COMMIT] = "commit", + [BIND_ACTION_SEARCH_FIND_PREV] = "find-prev", + [BIND_ACTION_SEARCH_FIND_NEXT] = "find-next", + [BIND_ACTION_SEARCH_EDIT_LEFT] = "cursor-left", + [BIND_ACTION_SEARCH_EDIT_LEFT_WORD] = "cursor-left-word", + [BIND_ACTION_SEARCH_EDIT_RIGHT] = "cursor-right", + [BIND_ACTION_SEARCH_EDIT_RIGHT_WORD] = "cursor-right-word", + [BIND_ACTION_SEARCH_EDIT_HOME] = "cursor-home", + [BIND_ACTION_SEARCH_EDIT_END] = "cursor-end", + [BIND_ACTION_SEARCH_DELETE_PREV] = "delete-prev", + [BIND_ACTION_SEARCH_DELETE_PREV_WORD] = "delete-prev-word", + [BIND_ACTION_SEARCH_DELETE_NEXT] = "delete-next", + [BIND_ACTION_SEARCH_DELETE_NEXT_WORD] = "delete-next-word", + [BIND_ACTION_SEARCH_EXTEND_WORD] = "extend-to-word-boundary", + [BIND_ACTION_SEARCH_EXTEND_WORD_WS] = "extend-to-next-whitespace", + [BIND_ACTION_SEARCH_CLIPBOARD_PASTE] = "clipboard-paste", + [BIND_ACTION_SEARCH_PRIMARY_PASTE] = "primary-paste", +}; + +static const char *const url_binding_action_map[] = { + [BIND_ACTION_URL_NONE] = NULL, + [BIND_ACTION_URL_CANCEL] = "cancel", + [BIND_ACTION_URL_TOGGLE_URL_ON_JUMP_LABEL] = "toggle-url-visible", +}; + static_assert(ALEN(binding_action_map) == BIND_ACTION_COUNT, "binding action map size mismatch"); +static_assert(ALEN(search_binding_action_map) == BIND_ACTION_SEARCH_COUNT, + "search binding action map size mismatch"); +static_assert(ALEN(url_binding_action_map) == BIND_ACTION_URL_COUNT, + "URL binding action map size mismatch"); struct context { struct config *conf; @@ -1444,26 +1476,48 @@ parse_section_csd(struct context *ctx) } static void -binding_pipe_free(struct config_binding_pipe *pipe) +free_binding_pipe(struct config_binding_pipe *pipe) { if (pipe->master_copy) free_argv(&pipe->argv); } -static void NOINLINE -key_binding_list_free(struct config_key_binding_list *bindings) +static void +free_key_binding(struct config_key_binding *binding) { - for (size_t i = 0; i < bindings->count; i++) - binding_pipe_free(&bindings->arr[i].pipe); + free_binding_pipe(&binding->pipe); +} + +static void NOINLINE +free_key_binding_list(struct config_key_binding_list *bindings) +{ + struct config_key_binding *binding = &bindings->arr[0]; + + for (size_t i = 0; i < bindings->count; i++, binding++) + free_key_binding(binding); free(bindings->arr); + + bindings->arr = NULL; + bindings->count = 0; +} + +static void +free_mouse_binding(struct config_mouse_binding *binding) +{ + free_binding_pipe(&binding->pipe); } static void mouse_binding_list_free(struct config_mouse_binding_list *bindings) { - for (size_t i = 0; i < bindings->count; i++) - binding_pipe_free(&bindings->arr[i].pipe); + struct config_mouse_binding *binding = &bindings->arr[0]; + + for (size_t i = 0; i < bindings->count; i++, binding++) + free_mouse_binding(binding); free(bindings->arr); + + bindings->arr = NULL; + bindings->count = 0; } static bool @@ -1505,63 +1559,7 @@ out: return ret; } -static bool -value_to_key_combos(struct context *ctx, - struct config_key_binding_list *key_combos) -{ - xassert(key_combos != NULL); - xassert(key_combos->count == 0 && key_combos->arr == NULL); - - size_t size = 0; /* Size of ‘combos’ array in the key-combo list */ - - char *copy = xstrdup(ctx->value); - - for (char *tok_ctx = NULL, *combo = strtok_r(copy, " ", &tok_ctx); - combo != NULL; - combo = strtok_r(NULL, " ", &tok_ctx)) - { - struct config_key_modifiers modifiers = {0}; - char *key = strrchr(combo, '+'); - - if (key == NULL) { - /* No modifiers */ - key = combo; - } else { - if (!parse_modifiers(ctx, combo, key - combo, &modifiers)) - goto err; - key++; /* Skip past the '+' */ - } - - /* Translate key name to symbol */ - xkb_keysym_t sym = xkb_keysym_from_name(key, 0); - if (sym == XKB_KEY_NoSymbol) { - LOG_CONTEXTUAL_ERR("not a valid XKB key name: %s", key); - goto err; - } - - if (key_combos->count + 1 > size) { - size += 4; - key_combos->arr = xrealloc( - key_combos->arr, size * sizeof(key_combos->arr[0])); - } - - xassert(key_combos->count + 1 <= size); - key_combos->arr[key_combos->count++] = (struct config_key_binding){ - .modifiers = modifiers, - .sym = sym, - }; - } - - free(copy); - return true; - -err: - key_binding_list_free(key_combos); - free(copy); - return false; -} - -static int +static int NOINLINE argv_compare(const struct argv *argv1, const struct argv *argv2) { if (argv1->args == NULL && argv2->args == NULL) @@ -1589,6 +1587,113 @@ argv_compare(const struct argv *argv1, const struct argv *argv2) return 1; } +static void NOINLINE +remove_action_from_key_bindings_list(struct config_key_binding_list *bindings, + int action, const struct argv *pipe_argv) +{ + size_t remove_first_idx = 0; + size_t remove_count = 0; + + for (size_t i = 0; i < bindings->count; i++) { + struct config_key_binding *binding = &bindings->arr[i]; + + if (binding->action != action) + continue; + + if (argv_compare(&binding->pipe.argv, pipe_argv) == 0) { + if (remove_count++ == 0) + remove_first_idx = i; + + xassert(remove_first_idx + remove_count - 1 == i); + free_key_binding(binding); + } + } + + if (remove_count == 0) + return; + + size_t move_count = bindings->count - (remove_first_idx + remove_count); + + memmove( + &bindings->arr[remove_first_idx], + &bindings->arr[remove_first_idx + remove_count], + move_count * sizeof(bindings->arr[0])); + bindings->count -= remove_count; +} + +static bool NOINLINE +value_to_key_combos(struct context *ctx, int action, struct argv *argv, + struct config_key_binding_list *bindings) +{ + if (strcasecmp(ctx->value, "none") == 0) { + remove_action_from_key_bindings_list(bindings, action, argv); + return true; + } + + /* Count number of combinations */ + size_t combo_count = 1; + for (const char *p = strchr(ctx->value, ' '); + p != NULL; + p = strchr(p + 1, ' ')) + { + combo_count++; + } + + struct config_key_binding new_combos[combo_count]; + + char *copy = xstrdup(ctx->value); + size_t idx = 0; + + for (char *tok_ctx = NULL, *combo = strtok_r(copy, " ", &tok_ctx); + combo != NULL; + combo = strtok_r(NULL, " ", &tok_ctx), + idx++) + { + struct config_key_binding *new_combo = &new_combos[idx]; + new_combo->action = action; + new_combo->pipe.master_copy = idx == 0; + new_combo->pipe.argv = *argv; + new_combo->path = ctx->path; + new_combo->lineno = ctx->lineno; + + char *key = strrchr(combo, '+'); + + if (key == NULL) { + /* No modifiers */ + key = combo; + } else { + if (!parse_modifiers(ctx, combo, key - combo, &new_combo->modifiers)) + goto err; + key++; /* Skip past the '+' */ + } + + /* Translate key name to symbol */ + new_combo->sym = xkb_keysym_from_name(key, 0); + if (new_combo->sym == XKB_KEY_NoSymbol) { + LOG_CONTEXTUAL_ERR("not a valid XKB key name: %s", key); + goto err; + } + } + + remove_action_from_key_bindings_list(bindings, action, argv); + + bindings->arr = xrealloc( + bindings->arr, + (bindings->count + combo_count) * sizeof(bindings->arr[0])); + + memcpy(&bindings->arr[bindings->count], + new_combos, + combo_count * sizeof(bindings->arr[0])); + bindings->count += combo_count; + + free(copy); + return true; + +err: + free(copy); + return false; +} + static bool modifiers_equal(const struct config_key_modifiers *mods1, const struct config_key_modifiers *mods2) @@ -1624,53 +1729,78 @@ modifiers_to_str(const struct config_key_modifiers *mods) } static bool -has_key_binding_collisions(struct context *ctx, - int action, const char *const action_map[], - const struct config_key_binding_list *bindings, - const struct config_key_binding_list *key_combos, - const struct argv *pipe_argv) +resolve_key_binding_collisions(struct config *conf, const char *section_name, + const char *const action_map[], + struct config_key_binding_list *bindings) { - for (size_t j = 0; j < bindings->count; j++) { - const struct config_key_binding *combo1 = &bindings->arr[j]; + bool ret = true; - if (combo1->action == BIND_ACTION_NONE) - continue; + for (size_t i = 1; i < bindings->count; i++) { + struct config_key_binding *binding1 = &bindings->arr[i]; + xassert(binding1->action != BIND_ACTION_NONE); - if (combo1->action == action) { - if (argv_compare(&combo1->pipe.argv, pipe_argv) == 0) - continue; - } + for (ssize_t j = i - 1; j >= 0; j--) { + const struct config_key_binding *binding2 = &bindings->arr[j]; + xassert(binding2->action != BIND_ACTION_NONE); - for (size_t i = 0; i < key_combos->count; i++) { - const struct config_key_binding *combo2 = &key_combos->arr[i]; + if (binding2->action == binding1->action) { + if (argv_compare(&binding1->pipe.argv, &binding2->pipe.argv)) + continue; + } - const struct config_key_modifiers *mods1 = &combo1->modifiers; - const struct config_key_modifiers *mods2 = &combo2->modifiers; + const struct config_key_modifiers *mods1 = &binding1->modifiers; + const struct config_key_modifiers *mods2 = &binding2->modifiers; bool mods_equal = modifiers_equal(mods1, mods2); - bool sym_equal = combo1->sym == combo2->sym; + bool sym_equal = binding1->sym == binding2->sym; - if (mods_equal && sym_equal) { - bool has_pipe = combo1->pipe.argv.args != NULL; + if (!mods_equal || !sym_equal) + continue; - char *modifier_names = modifiers_to_str(mods2); - char sym_name[64]; - xkb_keysym_get_name(combo2->sym, sym_name, sizeof(sym_name)); + bool has_pipe = binding2->pipe.argv.args != NULL; - LOG_CONTEXTUAL_ERR("%s+%s already mapped to '%s%s%s%s'", - modifier_names, sym_name, - action_map[combo1->action], - has_pipe ? " [" : "", - has_pipe ? combo1->pipe.argv.args[0] : "", - has_pipe ? "]" : ""); + char *modifier_names = modifiers_to_str(mods2); + char sym_name[64]; + xkb_keysym_get_name(binding2->sym, sym_name, sizeof(sym_name)); - free(modifier_names); - return true; + LOG_AND_NOTIFY_ERR( + "%s:%d: [%s].%s: %s+%s already mapped to '%s%s%s%s'", + binding1->path, binding1->lineno, section_name, + action_map[binding1->action], + modifier_names, sym_name, + action_map[binding2->action], + has_pipe ? " [" : "", + has_pipe ? binding2->pipe.argv.args[0] : "", + has_pipe ? "]" : ""); + + free(modifier_names); + ret = false; + + if (binding1->pipe.master_copy && i + 1 < bindings->count) { + struct config_key_binding *next = &bindings->arr[i + 1]; + + if (next->action == binding1->action && + argv_compare(&binding1->pipe.argv, &next->pipe.argv) == 0) + { + /* Transfer ownership to next binding */ + next->pipe.master_copy = true; + binding1->pipe.master_copy = false; + } } + + free_key_binding(binding1); + + /* Remove the most recent binding */ + size_t move_count = bindings->count - (i + 1); + memmove(&bindings->arr[i], &bindings->arr[i + 1], + move_count * sizeof(bindings->arr[0])); + bindings->count--; + i--; + break; } } - return false; + return ret; } /* @@ -1728,42 +1858,6 @@ pipe_argv_from_value(struct context *ctx, struct argv *argv) return remove_len; } -static void NOINLINE -remove_action_from_key_bindings_list(struct config_key_binding_list *bindings, - int action, const struct argv *pipe_argv) -{ - size_t remove_first_idx = 0; - size_t remove_count = 0; - - for (size_t i = 0; i < bindings->count; i++) { - struct config_key_binding *binding = &bindings->arr[i]; - - if (binding->action != action) - continue; - - if (argv_compare(&binding->pipe.argv, pipe_argv) == 0) { - if (remove_count++ == 0) - remove_first_idx = i; - - xassert(remove_first_idx + remove_count - 1 == i); - - if (binding->pipe.master_copy) - free_argv(&binding->pipe.argv); - } - } - - if (remove_count == 0) - return; - - size_t move_count = bindings->count - (remove_first_idx + remove_count); - - memmove( - &bindings->arr[remove_first_idx], - &bindings->arr[remove_first_idx + remove_count], - move_count * sizeof(bindings->arr[0])); - bindings->count -= remove_count; -} - static bool NOINLINE parse_key_binding_section(struct context *ctx, int action_count, @@ -1783,43 +1877,11 @@ parse_key_binding_section(struct context *ctx, if (strcmp(ctx->key, action_map[action]) != 0) continue; - /* Unset binding */ - if (strcasecmp(ctx->value, "none") == 0) { - remove_action_from_key_bindings_list(bindings, action, &pipe_argv); + if (!value_to_key_combos(ctx, action, &pipe_argv, bindings)) { free_argv(&pipe_argv); - return true; - } - - struct config_key_binding_list key_combos = {0}; - if (!value_to_key_combos(ctx, &key_combos) || - has_key_binding_collisions(ctx, action, action_map, bindings, &key_combos, &pipe_argv)) - { - free_argv(&pipe_argv); - key_binding_list_free(&key_combos); return false; } - remove_action_from_key_bindings_list(bindings, action, &pipe_argv); - - /* Emit key bindings */ - size_t ofs = bindings->count; - bindings->count += key_combos.count; - bindings->arr = xrealloc( - bindings->arr, bindings->count * sizeof(bindings->arr[0])); - - bool first = true; - for (size_t i = 0; i < key_combos.count; i++) { - struct config_key_binding *binding = &bindings->arr[ofs + i]; - - *binding = key_combos.arr[i]; - binding->action = action; - binding->pipe.argv = pipe_argv; - binding->pipe.master_copy = first; - - first = false; - } - - key_binding_list_free(&key_combos); return true; } @@ -1933,31 +1995,6 @@ parse_section_key_bindings(struct context *ctx) static bool parse_section_search_bindings(struct context *ctx) { - static const char *const search_binding_action_map[] = { - [BIND_ACTION_SEARCH_NONE] = NULL, - [BIND_ACTION_SEARCH_CANCEL] = "cancel", - [BIND_ACTION_SEARCH_COMMIT] = "commit", - [BIND_ACTION_SEARCH_FIND_PREV] = "find-prev", - [BIND_ACTION_SEARCH_FIND_NEXT] = "find-next", - [BIND_ACTION_SEARCH_EDIT_LEFT] = "cursor-left", - [BIND_ACTION_SEARCH_EDIT_LEFT_WORD] = "cursor-left-word", - [BIND_ACTION_SEARCH_EDIT_RIGHT] = "cursor-right", - [BIND_ACTION_SEARCH_EDIT_RIGHT_WORD] = "cursor-right-word", - [BIND_ACTION_SEARCH_EDIT_HOME] = "cursor-home", - [BIND_ACTION_SEARCH_EDIT_END] = "cursor-end", - [BIND_ACTION_SEARCH_DELETE_PREV] = "delete-prev", - [BIND_ACTION_SEARCH_DELETE_PREV_WORD] = "delete-prev-word", - [BIND_ACTION_SEARCH_DELETE_NEXT] = "delete-next", - [BIND_ACTION_SEARCH_DELETE_NEXT_WORD] = "delete-next-word", - [BIND_ACTION_SEARCH_EXTEND_WORD] = "extend-to-word-boundary", - [BIND_ACTION_SEARCH_EXTEND_WORD_WS] = "extend-to-next-whitespace", - [BIND_ACTION_SEARCH_CLIPBOARD_PASTE] = "clipboard-paste", - [BIND_ACTION_SEARCH_PRIMARY_PASTE] = "primary-paste", - }; - - static_assert(ALEN(search_binding_action_map) == BIND_ACTION_SEARCH_COUNT, - "search binding action map size mismatch"); - return parse_key_binding_section( ctx, BIND_ACTION_SEARCH_COUNT, search_binding_action_map, @@ -1967,15 +2004,6 @@ parse_section_search_bindings(struct context *ctx) static bool parse_section_url_bindings(struct context *ctx) { - static const char *const url_binding_action_map[] = { - [BIND_ACTION_URL_NONE] = NULL, - [BIND_ACTION_URL_CANCEL] = "cancel", - [BIND_ACTION_URL_TOGGLE_URL_ON_JUMP_LABEL] = "toggle-url-visible", - }; - - static_assert(ALEN(url_binding_action_map) == BIND_ACTION_URL_COUNT, - "URL binding action map size mismatch"); - return parse_key_binding_section( ctx, BIND_ACTION_URL_COUNT, url_binding_action_map, @@ -2709,8 +2737,8 @@ static void add_default_key_bindings(struct config *conf) { static const struct config_key_binding bindings[] = { - {BIND_ACTION_SCROLLBACK_UP_PAGE, m_shift, XKB_KEY_Page_Up}, - {BIND_ACTION_SCROLLBACK_DOWN_PAGE, m_shift, XKB_KEY_Page_Down}, + {BIND_ACTION_SCROLLBACK_UP_PAGE, m_shift, XKB_KEY_Prior}, + {BIND_ACTION_SCROLLBACK_DOWN_PAGE, m_shift, XKB_KEY_Next}, {BIND_ACTION_CLIPBOARD_COPY, m_ctrl_shift, XKB_KEY_c}, {BIND_ACTION_CLIPBOARD_PASTE, m_ctrl_shift, XKB_KEY_v}, {BIND_ACTION_PRIMARY_PASTE, m_shift, XKB_KEY_Insert}, @@ -3027,7 +3055,22 @@ config_load(struct config *conf, const char *conf_path, } ret = parse_config_file(f, conf, conf_file.path, errors_are_fatal) && - config_override_apply(conf, overrides, errors_are_fatal); + config_override_apply(conf, overrides, errors_are_fatal); + + if (ret && + (!resolve_key_binding_collisions( + conf, section_info[SECTION_KEY_BINDINGS].name, + binding_action_map, &conf->bindings.key) || + !resolve_key_binding_collisions( + conf, section_info[SECTION_SEARCH_BINDINGS].name, + search_binding_action_map, &conf->bindings.search) || + !resolve_key_binding_collisions( + conf, section_info[SECTION_URL_BINDINGS].name, + url_binding_action_map, &conf->bindings.url))) + { + ret = !errors_are_fatal; + } + fclose(f); conf->colors.use_custom.selection = @@ -3067,7 +3110,8 @@ out: } bool -config_override_apply(struct config *conf, config_override_t *overrides, bool errors_are_fatal) +config_override_apply(struct config *conf, config_override_t *overrides, + bool errors_are_fatal) { struct context context = { .conf = conf, @@ -3105,6 +3149,7 @@ config_override_apply(struct config *conf, config_override_t *overrides, bool er continue; } } + return true; } @@ -3260,9 +3305,9 @@ config_free(struct config conf) free(conf.url.protocols); free(conf.url.uri_characters); - key_binding_list_free(&conf.bindings.key); - key_binding_list_free(&conf.bindings.search); - key_binding_list_free(&conf.bindings.url); + free_key_binding_list(&conf.bindings.key); + free_key_binding_list(&conf.bindings.search); + free_key_binding_list(&conf.bindings.url); mouse_binding_list_free(&conf.bindings.mouse); user_notifications_free(&conf.notifications); diff --git a/config.h b/config.h index 2ae3a2d5..ee968748 100644 --- a/config.h +++ b/config.h @@ -45,6 +45,10 @@ struct config_key_binding { struct config_key_modifiers modifiers; xkb_keysym_t sym; struct config_binding_pipe pipe; + + /* For error messages in collision handling */ + const char *path; + int lineno; }; DEFINE_LIST(struct config_key_binding); From e67639a6822c906b65b25850cd6c00738c2db648 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 5 Dec 2021 15:25:26 +0100 Subject: [PATCH 03/14] main: ignore SIGPIPE We want to handle SIGPIPEs without crashing... One way to trigger this was to use e.g. pipe-visible=[cat /foo/bar] Control+Shift+q That is, pipe output to something that did not consume it. This led to a SIGPIPE when we tried to write the terminal contents to the pipe, and crashed the whole foot instance. --- main.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/main.c b/main.c index 4f2dc7b6..b5a610e2 100644 --- a/main.c +++ b/main.c @@ -556,8 +556,11 @@ main(int argc, char *const *argv) goto out; } - if (sigaction(SIGHUP, &(struct sigaction){.sa_handler = SIG_IGN}, NULL) < 0) { - LOG_ERRNO("failed to ignore SIGHUP"); + const struct sigaction sig_ign = {.sa_handler = SIG_IGN}; + if (sigaction(SIGHUP, &sig_ign, NULL) < 0 || + sigaction(SIGPIPE, &sig_ign, NULL) < 0) + { + LOG_ERRNO("failed to ignore SIGHUP+SIGPIPE"); goto out; } From bb4b4ae43d64e9d51e01cfd5c983684ec1bfe381 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 5 Dec 2021 15:51:52 +0100 Subject: [PATCH 04/14] config: value_to_key_combos(): reset modifiers in binding --- config.c | 1 + 1 file changed, 1 insertion(+) diff --git a/config.c b/config.c index e82385a7..e1d9e0fc 100644 --- a/config.c +++ b/config.c @@ -1661,6 +1661,7 @@ value_to_key_combos(struct context *ctx, int action, struct argv *argv, if (key == NULL) { /* No modifiers */ key = combo; + new_combo->modifiers = (struct config_key_modifiers){0}; } else { if (!parse_modifiers(ctx, combo, key - combo, &new_combo->modifiers)) goto err; From 4c50c44cf768238486f80fc5daa70d47688902a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 5 Dec 2021 16:30:01 +0100 Subject: [PATCH 05/14] config: do mouse binding collision detection after loading the conf --- config.c | 456 ++++++++++++++++++++++++++++--------------------------- config.h | 6 +- input.c | 2 +- 3 files changed, 238 insertions(+), 226 deletions(-) diff --git a/config.c b/config.c index e1d9e0fc..75e5407f 100644 --- a/config.c +++ b/config.c @@ -1507,8 +1507,8 @@ free_mouse_binding(struct config_mouse_binding *binding) free_binding_pipe(&binding->pipe); } -static void -mouse_binding_list_free(struct config_mouse_binding_list *bindings) +static void NOINLINE +free_mouse_binding_list(struct config_mouse_binding_list *bindings) { struct config_mouse_binding *binding = &bindings->arr[0]; @@ -1545,7 +1545,7 @@ parse_modifiers(struct context *ctx, const char *text, size_t len, else if (strcmp(key, XKB_MOD_NAME_ALT) == 0) modifiers->alt = true; else if (strcmp(key, XKB_MOD_NAME_LOGO) == 0) - modifiers->meta = true; + modifiers->super = true; else { LOG_CONTEXTUAL_ERR("not a valid modifier name: %s", key); goto out; @@ -1702,8 +1702,8 @@ modifiers_equal(const struct config_key_modifiers *mods1, bool shift = mods1->shift == mods2->shift; bool alt = mods1->alt == mods2->alt; bool ctrl = mods1->ctrl == mods2->ctrl; - bool meta = mods1->meta == mods2->meta; - return shift && alt && ctrl && meta; + bool super = mods1->super == mods2->super; + return shift && alt && ctrl && super; } static bool @@ -1713,19 +1713,19 @@ modifiers_disjoint(const struct config_key_modifiers *mods1, bool shift = mods1->shift && mods2->shift; bool alt = mods1->alt && mods2->alt; bool ctrl = mods1->ctrl && mods2->ctrl; - bool meta = mods1->meta && mods2->meta; - return !(shift || alt || ctrl || meta); + bool super = mods1->super && mods2->super; + return !(shift || alt || ctrl || super); } static char * modifiers_to_str(const struct config_key_modifiers *mods) { - char *ret = xasprintf("%s%s%s%s", - mods->ctrl ? "Control+" : "", - mods->alt ? "Alt+": "", - mods->meta ? "Meta+": "", - mods->shift ? "Shift+": ""); - ret[strlen(ret) - 1] = '\0'; + char *ret = xasprintf( + "%s%s%s%s", + mods->ctrl ? XKB_MOD_NAME_CTRL "+" : "", + mods->alt ? XKB_MOD_NAME_ALT "+": "", + mods->super ? XKB_MOD_NAME_LOGO "+": "", + mods->shift ? XKB_MOD_NAME_SHIFT "+": ""); return ret; } @@ -1765,7 +1765,7 @@ resolve_key_binding_collisions(struct config *conf, const char *section_name, xkb_keysym_get_name(binding2->sym, sym_name, sizeof(sym_name)); LOG_AND_NOTIFY_ERR( - "%s:%d: [%s].%s: %s+%s already mapped to '%s%s%s%s'", + "%s:%d: [%s].%s: %s%s already mapped to '%s%s%s%s'", binding1->path, binding1->lineno, section_name, action_map[binding1->action], modifier_names, sym_name, @@ -2025,8 +2025,18 @@ static const struct { {"BTN_TASK", BTN_TASK}, }; +static int +mouse_button_name_to_code(const char *name) +{ + for (size_t i = 0; i < ALEN(button_map); i++) { + if (strcmp(button_map[i].name, name) == 0) + return button_map[i].code; + } + return -1; +} + static const char* -mouse_event_code_get_name(int code) +mouse_button_code_to_name(int code) { for (size_t i = 0; i < ALEN(button_map); i++) { if (code == button_map[i].code) @@ -2036,35 +2046,90 @@ mouse_event_code_get_name(int code) return NULL; } -static bool -value_to_mouse_combos(struct context *ctx, - struct config_mouse_binding_list *mouse_combos) +static void NOINLINE +remove_action_from_mouse_bindings_list( + struct config_mouse_binding_list *bindings, + int action, const struct argv *pipe_argv) { - xassert(mouse_combos != NULL); - xassert(mouse_combos->count == 0 && mouse_combos->arr == NULL); + size_t remove_first_idx = 0; + size_t remove_count = 0; - size_t size = 0; /* Size of the ‘combos’ array in key_combos */ + for (size_t i = 0; i < bindings->count; i++) { + struct config_mouse_binding *binding = &bindings->arr[i]; + + if (binding->action != action) + continue; + + if (argv_compare(&binding->pipe.argv, pipe_argv) == 0) { + if (remove_count++ == 0) + remove_first_idx = i; + + xassert(remove_first_idx + remove_count - 1 == i); + free_mouse_binding(binding); + } + } + + if (remove_count == 0) + return; + + size_t move_count = bindings->count - (remove_first_idx + remove_count); + + memmove( + &bindings->arr[remove_first_idx], + &bindings->arr[remove_first_idx + remove_count], + move_count * sizeof(bindings->arr[0])); + bindings->count -= remove_count; +} + +static bool +value_to_mouse_combos(struct context *ctx, int action, struct argv *argv, + struct config_mouse_binding_list *bindings) +{ + if (strcasecmp(ctx->value, "none") == 0) { + remove_action_from_mouse_bindings_list(bindings, action, argv); + return true; + } + + /* Count number of combinations */ + size_t combo_count = 1; + for (const char *p = strchr(ctx->value, ' '); + p != NULL; + p = strchr(p + 1, ' ')) + { + combo_count++; + } + + struct config_mouse_binding new_combos[combo_count]; char *copy = xstrdup(ctx->value); + size_t idx = 0; for (char *tok_ctx = NULL, *combo = strtok_r(copy, " ", &tok_ctx); combo != NULL; - combo = strtok_r(NULL, " ", &tok_ctx)) + combo = strtok_r(NULL, " ", &tok_ctx), + idx++) { - struct config_key_modifiers modifiers = {0}; + struct config_mouse_binding *new_combo = &new_combos[idx]; + new_combo->action = action; + new_combo->pipe.master_copy = idx == 0; + new_combo->pipe.argv = *argv; + new_combo->path = ctx->path; + new_combo->lineno = ctx->lineno; + char *key = strrchr(combo, '+'); if (key == NULL) { /* No modifiers */ key = combo; + new_combo->modifiers = (struct config_key_modifiers){0}; } else { *key = '\0'; - if (!parse_modifiers(ctx, combo, key - combo, &modifiers)) + if (!parse_modifiers(ctx, combo, key - combo, &new_combo->modifiers)) goto err; key++; /* Skip past the '+' */ } - size_t count = 1; + new_combo->count = 1; { char *_count = strrchr(key, '-'); if (_count != NULL) { @@ -2081,153 +2146,162 @@ value_to_mouse_combos(struct context *ctx, LOG_CONTEXTUAL_ERR("invalid click count: %s", _count); goto err; } - count = value; + + new_combo->count = value; } } - int button = 0; - for (size_t i = 0; i < ALEN(button_map); i++) { - if (strcmp(key, button_map[i].name) == 0) { - button = button_map[i].code; - break; - } - } - - if (button == 0) { + new_combo->button = mouse_button_name_to_code(key); + if (new_combo->button < 0) { LOG_CONTEXTUAL_ERR("invalid mouse button name: %s", key); goto err; } - - struct config_mouse_binding new = { - .modifiers = modifiers, - .button = button, - .count = count, - }; - - if (mouse_combos->count + 1 > size) { - size += 4; - mouse_combos->arr = xrealloc( - mouse_combos->arr, size * sizeof(mouse_combos->arr[0])); - } - - xassert(mouse_combos->count + 1 <= size); - mouse_combos->arr[mouse_combos->count++] = new; } + remove_action_from_mouse_bindings_list(bindings, action, argv); + + bindings->arr = xrealloc( + bindings->arr, + (bindings->count + combo_count) * sizeof(bindings->arr[0])); + + memcpy(&bindings->arr[bindings->count], + new_combos, + combo_count * sizeof(bindings->arr[0])); + bindings->count += combo_count; + free(copy); return true; err: - mouse_binding_list_free(mouse_combos); free(copy); return false; } -static char * -mouse_combo_to_str(const struct config_mouse_binding *combo) -{ - char *combo_modifiers_str = modifiers_to_str(&combo->modifiers); - const char *combo_button_str = mouse_event_code_get_name(combo->button); - xassert(combo_button_str != NULL); - - char *ret; - if (combo->count == 1) - ret = xasprintf("%s+%s", combo_modifiers_str, combo_button_str); - else - ret = xasprintf("%s+%s-%d", - combo_modifiers_str, - combo_button_str, - combo->count); - - free (combo_modifiers_str); - return ret; -} - static bool -selection_override_interferes_with_mouse_binding( - struct context *ctx, int action, - const struct config_mouse_binding_list *mouse_combos, bool blame_modifiers) +resolve_mouse_binding_collisions(struct config *conf, const char *section_name, + const char *const action_map[], + struct config_mouse_binding_list *bindings) { - struct config *conf = ctx->conf; + bool ret = true; - if (action == BIND_ACTION_NONE) - return false; + for (size_t i = 1; i < bindings->count; i++) { + enum {COLLISION_NONE, + COLLISION_OVERRIDE, + COLLISION_BINDING} collision_type = COLLISION_NONE; + const struct config_mouse_binding *collision_binding = NULL; - const struct config_key_modifiers *override_mods = - &conf->mouse.selection_override_modifiers; - for (size_t i = 0; i < mouse_combos->count; i++) { - const struct config_mouse_binding *combo = &mouse_combos->arr[i]; + struct config_mouse_binding *binding1 = &bindings->arr[i]; + xassert(binding1->action != BIND_ACTION_NONE); - if (!modifiers_disjoint(&combo->modifiers, override_mods)) { - char *modifiers_str = modifiers_to_str(override_mods); - char *combo_str = mouse_combo_to_str(combo); - if (blame_modifiers) { - LOG_CONTEXTUAL_ERR( - "modifiers conflict with existing binding %s=%s", - binding_action_map[action], - combo_str); - } else { - LOG_CONTEXTUAL_ERR( - "binding conflicts with selection override modifiers (%s)", - modifiers_str); + const struct config_key_modifiers *mods1 = &binding1->modifiers; + + /* Does our modifiers collide with the selection override mods? */ + if (!modifiers_disjoint(mods1, &conf->mouse.selection_override_modifiers)) + collision_type = COLLISION_OVERRIDE; + + /* Does our binding collide with another binding? */ + for (ssize_t j = i - 1; + collision_type == COLLISION_NONE && j >= 0; + j--) + { + const struct config_mouse_binding *binding2 = &bindings->arr[j]; + xassert(binding2->action != BIND_ACTION_NONE); + + if (binding2->action == binding1->action) { + if (argv_compare(&binding1->pipe.argv, &binding2->pipe.argv)) + continue; } - free (modifiers_str); - free (combo_str); - return false; + + const struct config_key_modifiers *mods2 = &binding2->modifiers; + + bool mods_equal = modifiers_equal(mods1, mods2); + bool button_equal = binding1->button == binding2->button; + bool count_equal = binding1->count == binding2->count; + + if (!mods_equal || !button_equal || !count_equal) + continue; + + collision_binding = binding2; + collision_type = COLLISION_BINDING; + break; } + + if (collision_type != COLLISION_NONE) { + char *modifier_names = modifiers_to_str(mods1); + const char *button_name = mouse_button_code_to_name(binding1->button); + + char click_count[16]; + if (binding1->count > 1) + sprintf(click_count, "-%d", binding1->count); + else + click_count[0] = '\0'; + + switch (collision_type) { + case COLLISION_NONE: + break; + + case COLLISION_BINDING: { + bool has_pipe = collision_binding->pipe.argv.args != NULL; + LOG_AND_NOTIFY_ERR( + "%s:%d: [%s].%s: %s%s%s already mapped to '%s%s%s%s'", + binding1->path, binding1->lineno, section_name, + action_map[binding1->action], + modifier_names, button_name, click_count, + action_map[collision_binding->action], + has_pipe ? " [" : "", + has_pipe ? collision_binding->pipe.argv.args[0] : "", + has_pipe ? "]" : ""); + break; + } + + case COLLISION_OVERRIDE: { + char *override_names = modifiers_to_str( + &conf->mouse.selection_override_modifiers); + + if (override_names[0] != '\0') + override_names[strlen(override_names) - 1] = '\0'; + + LOG_AND_NOTIFY_ERR( + "%s:%d: [%s].%s: %s%s%s: " + "modifiers conflict with 'selection-override-modifiers=%s'", + binding1->path, binding1->lineno, section_name, + action_map[binding1->action], + modifier_names, button_name, click_count, + override_names); + free(override_names); + break; + } + } + free(modifier_names); + + if (binding1->pipe.master_copy && i + 1 < bindings->count) { + struct config_mouse_binding *next = &bindings->arr[i + 1]; + + if (next->action == binding1->action && + argv_compare(&binding1->pipe.argv, &next->pipe.argv) == 0) + { + /* Transfer ownership to next binding */ + next->pipe.master_copy = true; + binding1->pipe.master_copy = false; + } + } + + free_mouse_binding(binding1); + + /* Remove the most recent binding */ + size_t move_count = bindings->count - (i + 1); + memmove(&bindings->arr[i], &bindings->arr[i + 1], + move_count * sizeof(bindings->arr[0])); + bindings->count--; + + i--; + } + } - return false; + return ret; } -static bool -has_mouse_binding_collisions( - struct context *ctx, - const struct config_mouse_binding_list *mouse_combos) -{ - struct config *conf = ctx->conf; - - for (size_t j = 0; j < conf->bindings.mouse.count; j++) { - const struct config_mouse_binding *combo1 = &conf->bindings.mouse.arr[j]; - if (combo1->action == BIND_ACTION_NONE) - continue; - - for (size_t i = 0; i < mouse_combos->count; i++) { - const struct config_mouse_binding *combo2 = &mouse_combos->arr[i]; - - const struct config_key_modifiers *mods1 = &combo1->modifiers; - const struct config_key_modifiers *mods2 = &combo2->modifiers; - - bool button = combo1->button == combo2->button; - bool count = combo1->count == combo2->count; - - if (modifiers_equal(mods1, mods2) && button && count) { - bool has_pipe = combo1->pipe.argv.args != NULL; - - char *modifier_names = modifiers_to_str(mods2); - const char *btn_name = mouse_event_code_get_name(combo2->button); - - char count_string[16] = {0}; - if (combo2->count > 1) - sprintf(count_string, "-%d", combo2->count); - - LOG_CONTEXTUAL_ERR("%s+%s%s already mapped to '%s%s%s%s'", - modifier_names, btn_name, count_string, - binding_action_map[combo1->action], - has_pipe ? " [" : "", - has_pipe ? combo1->pipe.argv.args[0] : "", - has_pipe ? "]" : ""); - - free(modifier_names); - return true; - } - } - } - - return false; -} - - static bool parse_section_mouse_bindings(struct context *ctx) { @@ -2235,28 +2309,14 @@ parse_section_mouse_bindings(struct context *ctx) const char *key = ctx->key; const char *value = ctx->value; - if (strcmp(ctx->key, "selection-override-modifiers") == 0) { - if (!parse_modifiers(ctx, ctx->value, strlen(ctx->value), - &conf->mouse.selection_override_modifiers)) { + if (strcmp(key, "selection-override-modifiers") == 0) { + if (!parse_modifiers( + ctx, ctx->value, strlen(value), + &conf->mouse.selection_override_modifiers)) + { LOG_CONTEXTUAL_ERR("%s: invalid modifiers '%s'", key, ctx->value); return false; } - - /* Ensure no existing bindings use these modifiers */ - for (size_t i = 0; i < conf->bindings.mouse.count; i++) { - struct config_mouse_binding *binding = &conf->bindings.mouse.arr[i]; - struct config_mouse_binding_list mouse_combos = { - .count = 1, - .arr = binding, - }; - - if (selection_override_interferes_with_mouse_binding( - ctx, binding->action, &mouse_combos, true)) - { - return false; - } - } - return true; } @@ -2276,68 +2336,13 @@ parse_section_mouse_bindings(struct context *ctx) if (strcmp(key, binding_action_map[action]) != 0) continue; - /* Unset binding */ - if (strcasecmp(value, "none") == 0) { - for (size_t i = 0; i < conf->bindings.mouse.count; i++) { - struct config_mouse_binding *binding = - &conf->bindings.mouse.arr[i]; - - if (binding->action == action) { - if (binding->pipe.master_copy) - free_argv(&binding->pipe.argv); - binding->action = BIND_ACTION_NONE; - } - } - free_argv(&pipe_argv); - return true; - } - - struct config_mouse_binding_list mouse_combos = {0}; - if (!value_to_mouse_combos(ctx, &mouse_combos) || - has_mouse_binding_collisions(ctx, &mouse_combos) || - selection_override_interferes_with_mouse_binding( - ctx, action, &mouse_combos, false)) + if (!value_to_mouse_combos( + ctx, action, &pipe_argv, &conf->bindings.mouse)) { free_argv(&pipe_argv); - mouse_binding_list_free(&mouse_combos); return false; } - /* Remove existing bindings for this action */ - for (size_t i = 0; i < conf->bindings.mouse.count; i++) { - struct config_mouse_binding *binding = &conf->bindings.mouse.arr[i]; - - if (binding->action != action) - continue; - - if (argv_compare(&binding->pipe.argv, &pipe_argv) == 0) { - if (binding->pipe.master_copy) - free_argv(&binding->pipe.argv); - binding->action = BIND_ACTION_NONE; - } - } - - /* Emit mouse bindings */ - size_t ofs = conf->bindings.mouse.count; - conf->bindings.mouse.count += mouse_combos.count; - conf->bindings.mouse.arr = xrealloc( - conf->bindings.mouse.arr, - conf->bindings.mouse.count * sizeof(conf->bindings.mouse.arr[0])); - - bool first = true; - for (size_t i = 0; i < mouse_combos.count; i++) { - struct config_mouse_binding *binding = - &conf->bindings.mouse.arr[ofs + i]; - - *binding = mouse_combos.arr[i]; - binding->action = action; - binding->pipe.argv = pipe_argv; - binding->pipe.master_copy = first; - - first = false; - } - - mouse_binding_list_free(&mouse_combos); return true; } @@ -2939,7 +2944,7 @@ config_load(struct config *conf, const char *conf_path, .shift = true, .alt = false, .ctrl = false, - .meta = false, + .super = false, }, }, .csd = { @@ -3067,7 +3072,10 @@ config_load(struct config *conf, const char *conf_path, search_binding_action_map, &conf->bindings.search) || !resolve_key_binding_collisions( conf, section_info[SECTION_URL_BINDINGS].name, - url_binding_action_map, &conf->bindings.url))) + url_binding_action_map, &conf->bindings.url) || + !resolve_mouse_binding_collisions( + conf, section_info[SECTION_MOUSE_BINDINGS].name, + binding_action_map, &conf->bindings.mouse))) { ret = !errors_are_fatal; } @@ -3309,7 +3317,7 @@ config_free(struct config conf) free_key_binding_list(&conf.bindings.key); free_key_binding_list(&conf.bindings.search); free_key_binding_list(&conf.bindings.url); - mouse_binding_list_free(&conf.bindings.mouse); + free_mouse_binding_list(&conf.bindings.mouse); user_notifications_free(&conf.notifications); } diff --git a/config.h b/config.h index ee968748..153b9c27 100644 --- a/config.h +++ b/config.h @@ -28,7 +28,7 @@ struct config_key_modifiers { bool shift; bool alt; bool ctrl; - bool meta; + bool super; }; struct argv { @@ -58,6 +58,10 @@ struct config_mouse_binding { int button; int count; struct config_binding_pipe pipe; + + /* For error messages in collision handling */ + const char *path; + int lineno; }; DEFINE_LIST(struct config_mouse_binding); diff --git a/input.c b/input.c index 41349d9a..0bd4b347 100644 --- a/input.c +++ b/input.c @@ -369,7 +369,7 @@ conf_modifiers_to_mask(const struct seat *seat, if (seat->kbd.mod_alt != XKB_MOD_INVALID) mods |= modifiers->alt << seat->kbd.mod_alt; if (seat->kbd.mod_super != XKB_MOD_INVALID) - mods |= modifiers->meta << seat->kbd.mod_super; + mods |= modifiers->super << seat->kbd.mod_super; return mods; } From f6a591b80a820ae5ab6886eb9ba43c90986f302b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Mon, 6 Dec 2021 21:04:38 +0100 Subject: [PATCH 06/14] config: unify key- and mouse bindings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With this patch, key- and mouse-bindings structs (the non-layout specific ones) are unified into a single struct. The logic that parses, and manages, the key- and mouse binding lists are almost identical. The *only* difference between a key- and a mouse binding is that key bindings have an XKB symbol, and mouse bindings a button and click-count. The new, unified, struct uses a union around these, and all functions that need to know which members to use/operate on now takes a ‘type’ parameter. --- config.c | 608 +++++++++++++++++++------------------------------------ config.h | 23 ++- input.c | 22 +- 3 files changed, 242 insertions(+), 411 deletions(-) diff --git a/config.c b/config.c index 75e5407f..cee4454c 100644 --- a/config.c +++ b/config.c @@ -1501,26 +1501,7 @@ free_key_binding_list(struct config_key_binding_list *bindings) bindings->count = 0; } -static void -free_mouse_binding(struct config_mouse_binding *binding) -{ - free_binding_pipe(&binding->pipe); -} - -static void NOINLINE -free_mouse_binding_list(struct config_mouse_binding_list *bindings) -{ - struct config_mouse_binding *binding = &bindings->arr[0]; - - for (size_t i = 0; i < bindings->count; i++, binding++) - free_mouse_binding(binding); - free(bindings->arr); - - bindings->arr = NULL; - bindings->count = 0; -} - -static bool +static bool NOINLINE parse_modifiers(struct context *ctx, const char *text, size_t len, struct config_key_modifiers *modifiers) { @@ -1588,8 +1569,8 @@ argv_compare(const struct argv *argv1, const struct argv *argv2) } static void NOINLINE -remove_action_from_key_bindings_list(struct config_key_binding_list *bindings, - int action, const struct argv *pipe_argv) +remove_from_key_bindings_list(struct config_key_binding_list *bindings, + int action, const struct argv *pipe_argv) { size_t remove_first_idx = 0; size_t remove_count = 0; @@ -1621,12 +1602,48 @@ remove_action_from_key_bindings_list(struct config_key_binding_list *bindings, bindings->count -= remove_count; } +static const struct { + const char *name; + int code; +} button_map[] = { + {"BTN_LEFT", BTN_LEFT}, + {"BTN_RIGHT", BTN_RIGHT}, + {"BTN_MIDDLE", BTN_MIDDLE}, + {"BTN_SIDE", BTN_SIDE}, + {"BTN_EXTRA", BTN_EXTRA}, + {"BTN_FORWARD", BTN_FORWARD}, + {"BTN_BACK", BTN_BACK}, + {"BTN_TASK", BTN_TASK}, +}; + +static int +mouse_button_name_to_code(const char *name) +{ + for (size_t i = 0; i < ALEN(button_map); i++) { + if (strcmp(button_map[i].name, name) == 0) + return button_map[i].code; + } + return -1; +} + +static const char* +mouse_button_code_to_name(int code) +{ + for (size_t i = 0; i < ALEN(button_map); i++) { + if (code == button_map[i].code) + return button_map[i].name; + } + + return NULL; +} + static bool NOINLINE value_to_key_combos(struct context *ctx, int action, struct argv *argv, - struct config_key_binding_list *bindings) + struct config_key_binding_list *bindings, + enum config_key_binding_type type) { if (strcasecmp(ctx->value, "none") == 0) { - remove_action_from_key_bindings_list(bindings, action, argv); + remove_from_key_bindings_list(bindings, action, argv); return true; } @@ -1663,20 +1680,57 @@ value_to_key_combos(struct context *ctx, int action, struct argv *argv, key = combo; new_combo->modifiers = (struct config_key_modifiers){0}; } else { + *key = '\0'; if (!parse_modifiers(ctx, combo, key - combo, &new_combo->modifiers)) goto err; key++; /* Skip past the '+' */ } - /* Translate key name to symbol */ - new_combo->sym = xkb_keysym_from_name(key, 0); - if (new_combo->sym == XKB_KEY_NoSymbol) { - LOG_CONTEXTUAL_ERR("not a valid XKB key name: %s", key); - goto err; + switch (type) { + case KEY_BINDING: + /* Translate key name to symbol */ + new_combo->k.sym = xkb_keysym_from_name(key, 0); + if (new_combo->k.sym == XKB_KEY_NoSymbol) { + LOG_CONTEXTUAL_ERR("not a valid XKB key name: %s", key); + goto err; + } + break; + + case MOUSE_BINDING: { + new_combo->m.count = 1; + + char *_count = strrchr(key, '-'); + if (_count != NULL) { + *_count = '\0'; + _count++; + + errno = 0; + char *end; + unsigned long value = strtoul(_count, &end, 10); + if (_count[0] == '\0' || *end != '\0' || errno != 0) { + if (errno != 0) + LOG_CONTEXTUAL_ERRNO("invalid click count: %s", _count); + else + LOG_CONTEXTUAL_ERR("invalid click count: %s", _count); + goto err; + } + + new_combo->m.count = value; + } + + new_combo->m.button = mouse_button_name_to_code(key); + if (new_combo->m.button < 0) { + LOG_CONTEXTUAL_ERR("invalid mouse button name: %s", key); + goto err; + } + + break; } + } + } - remove_action_from_key_bindings_list(bindings, action, argv); + remove_from_key_bindings_list(bindings, action, argv); bindings->arr = xrealloc( bindings->arr, @@ -1729,81 +1783,6 @@ modifiers_to_str(const struct config_key_modifiers *mods) return ret; } -static bool -resolve_key_binding_collisions(struct config *conf, const char *section_name, - const char *const action_map[], - struct config_key_binding_list *bindings) -{ - bool ret = true; - - for (size_t i = 1; i < bindings->count; i++) { - struct config_key_binding *binding1 = &bindings->arr[i]; - xassert(binding1->action != BIND_ACTION_NONE); - - for (ssize_t j = i - 1; j >= 0; j--) { - const struct config_key_binding *binding2 = &bindings->arr[j]; - xassert(binding2->action != BIND_ACTION_NONE); - - if (binding2->action == binding1->action) { - if (argv_compare(&binding1->pipe.argv, &binding2->pipe.argv)) - continue; - } - - const struct config_key_modifiers *mods1 = &binding1->modifiers; - const struct config_key_modifiers *mods2 = &binding2->modifiers; - - bool mods_equal = modifiers_equal(mods1, mods2); - bool sym_equal = binding1->sym == binding2->sym; - - if (!mods_equal || !sym_equal) - continue; - - bool has_pipe = binding2->pipe.argv.args != NULL; - - char *modifier_names = modifiers_to_str(mods2); - char sym_name[64]; - xkb_keysym_get_name(binding2->sym, sym_name, sizeof(sym_name)); - - LOG_AND_NOTIFY_ERR( - "%s:%d: [%s].%s: %s%s already mapped to '%s%s%s%s'", - binding1->path, binding1->lineno, section_name, - action_map[binding1->action], - modifier_names, sym_name, - action_map[binding2->action], - has_pipe ? " [" : "", - has_pipe ? binding2->pipe.argv.args[0] : "", - has_pipe ? "]" : ""); - - free(modifier_names); - ret = false; - - if (binding1->pipe.master_copy && i + 1 < bindings->count) { - struct config_key_binding *next = &bindings->arr[i + 1]; - - if (next->action == binding1->action && - argv_compare(&binding1->pipe.argv, &next->pipe.argv) == 0) - { - /* Transfer ownership to next binding */ - next->pipe.master_copy = true; - binding1->pipe.master_copy = false; - } - } - - free_key_binding(binding1); - - /* Remove the most recent binding */ - size_t move_count = bindings->count - (i + 1); - memmove(&bindings->arr[i], &bindings->arr[i + 1], - move_count * sizeof(bindings->arr[0])); - bindings->count--; - i--; - break; - } - } - - return ret; -} - /* * Parses a key binding value on the form * "[cmd-to-exec arg1 arg2] Mods+Key" @@ -1825,7 +1804,7 @@ resolve_key_binding_collisions(struct config *conf, const char *section_name, * - cmd: allocated string containing "cmd arg1 arg2...". Caller frees. * - argv: allocated array containing {"cmd", "arg1", "arg2", NULL}. Caller frees. */ -static ssize_t +static ssize_t NOINLINE pipe_argv_from_value(struct context *ctx, struct argv *argv) { argv->args = NULL; @@ -1878,7 +1857,9 @@ parse_key_binding_section(struct context *ctx, if (strcmp(ctx->key, action_map[action]) != 0) continue; - if (!value_to_key_combos(ctx, action, &pipe_argv, bindings)) { + if (!value_to_key_combos( + ctx, action, &pipe_argv, bindings, KEY_BINDING)) + { free_argv(&pipe_argv); return false; } @@ -1925,7 +1906,7 @@ UNITTEST xassert(parse_key_binding_section(&ctx, ALEN(map), map, &bindings)); xassert(bindings.count == 1); xassert(bindings.arr[0].action == TEST_ACTION_FOO); - xassert(bindings.arr[0].sym == XKB_KEY_Escape); + xassert(bindings.arr[0].k.sym == XKB_KEY_Escape); /* * ADD bar=Control+g Control+Shift+x @@ -1938,10 +1919,10 @@ UNITTEST xassert(bindings.count == 3); xassert(bindings.arr[0].action == TEST_ACTION_FOO); xassert(bindings.arr[1].action == TEST_ACTION_BAR); - xassert(bindings.arr[1].sym == XKB_KEY_g); + xassert(bindings.arr[1].k.sym == XKB_KEY_g); xassert(bindings.arr[1].modifiers.ctrl); xassert(bindings.arr[2].action == TEST_ACTION_BAR); - xassert(bindings.arr[2].sym == XKB_KEY_x); + xassert(bindings.arr[2].k.sym == XKB_KEY_x); xassert(bindings.arr[2].modifiers.ctrl && bindings.arr[2].modifiers.shift); /* @@ -1957,10 +1938,10 @@ UNITTEST xassert(bindings.arr[0].action == TEST_ACTION_BAR); xassert(bindings.arr[1].action == TEST_ACTION_BAR); xassert(bindings.arr[2].action == TEST_ACTION_FOO); - xassert(bindings.arr[2].sym == XKB_KEY_v); + xassert(bindings.arr[2].k.sym == XKB_KEY_v); xassert(bindings.arr[2].modifiers.alt); xassert(bindings.arr[3].action == TEST_ACTION_FOO); - xassert(bindings.arr[3].sym == XKB_KEY_q); + xassert(bindings.arr[3].k.sym == XKB_KEY_q); xassert(bindings.arr[3].modifiers.shift); /* @@ -2011,176 +1992,11 @@ parse_section_url_bindings(struct context *ctx) &ctx->conf->bindings.url); } -static const struct { - const char *name; - int code; -} button_map[] = { - {"BTN_LEFT", BTN_LEFT}, - {"BTN_RIGHT", BTN_RIGHT}, - {"BTN_MIDDLE", BTN_MIDDLE}, - {"BTN_SIDE", BTN_SIDE}, - {"BTN_EXTRA", BTN_EXTRA}, - {"BTN_FORWARD", BTN_FORWARD}, - {"BTN_BACK", BTN_BACK}, - {"BTN_TASK", BTN_TASK}, -}; - -static int -mouse_button_name_to_code(const char *name) -{ - for (size_t i = 0; i < ALEN(button_map); i++) { - if (strcmp(button_map[i].name, name) == 0) - return button_map[i].code; - } - return -1; -} - -static const char* -mouse_button_code_to_name(int code) -{ - for (size_t i = 0; i < ALEN(button_map); i++) { - if (code == button_map[i].code) - return button_map[i].name; - } - - return NULL; -} - -static void NOINLINE -remove_action_from_mouse_bindings_list( - struct config_mouse_binding_list *bindings, - int action, const struct argv *pipe_argv) -{ - size_t remove_first_idx = 0; - size_t remove_count = 0; - - for (size_t i = 0; i < bindings->count; i++) { - struct config_mouse_binding *binding = &bindings->arr[i]; - - if (binding->action != action) - continue; - - if (argv_compare(&binding->pipe.argv, pipe_argv) == 0) { - if (remove_count++ == 0) - remove_first_idx = i; - - xassert(remove_first_idx + remove_count - 1 == i); - free_mouse_binding(binding); - } - } - - if (remove_count == 0) - return; - - size_t move_count = bindings->count - (remove_first_idx + remove_count); - - memmove( - &bindings->arr[remove_first_idx], - &bindings->arr[remove_first_idx + remove_count], - move_count * sizeof(bindings->arr[0])); - bindings->count -= remove_count; -} - -static bool -value_to_mouse_combos(struct context *ctx, int action, struct argv *argv, - struct config_mouse_binding_list *bindings) -{ - if (strcasecmp(ctx->value, "none") == 0) { - remove_action_from_mouse_bindings_list(bindings, action, argv); - return true; - } - - /* Count number of combinations */ - size_t combo_count = 1; - for (const char *p = strchr(ctx->value, ' '); - p != NULL; - p = strchr(p + 1, ' ')) - { - combo_count++; - } - - struct config_mouse_binding new_combos[combo_count]; - - char *copy = xstrdup(ctx->value); - size_t idx = 0; - - for (char *tok_ctx = NULL, *combo = strtok_r(copy, " ", &tok_ctx); - combo != NULL; - combo = strtok_r(NULL, " ", &tok_ctx), - idx++) - { - struct config_mouse_binding *new_combo = &new_combos[idx]; - new_combo->action = action; - new_combo->pipe.master_copy = idx == 0; - new_combo->pipe.argv = *argv; - new_combo->path = ctx->path; - new_combo->lineno = ctx->lineno; - - char *key = strrchr(combo, '+'); - - if (key == NULL) { - /* No modifiers */ - key = combo; - new_combo->modifiers = (struct config_key_modifiers){0}; - } else { - *key = '\0'; - if (!parse_modifiers(ctx, combo, key - combo, &new_combo->modifiers)) - goto err; - key++; /* Skip past the '+' */ - } - - new_combo->count = 1; - { - char *_count = strrchr(key, '-'); - if (_count != NULL) { - *_count = '\0'; - _count++; - - errno = 0; - char *end; - unsigned long value = strtoul(_count, &end, 10); - if (_count[0] == '\0' || *end != '\0' || errno != 0) { - if (errno != 0) - LOG_CONTEXTUAL_ERRNO("invalid click count: %s", _count); - else - LOG_CONTEXTUAL_ERR("invalid click count: %s", _count); - goto err; - } - - new_combo->count = value; - } - } - - new_combo->button = mouse_button_name_to_code(key); - if (new_combo->button < 0) { - LOG_CONTEXTUAL_ERR("invalid mouse button name: %s", key); - goto err; - } - } - - remove_action_from_mouse_bindings_list(bindings, action, argv); - - bindings->arr = xrealloc( - bindings->arr, - (bindings->count + combo_count) * sizeof(bindings->arr[0])); - - memcpy(&bindings->arr[bindings->count], - new_combos, - combo_count * sizeof(bindings->arr[0])); - bindings->count += combo_count; - - free(copy); - return true; - -err: - free(copy); - return false; -} - -static bool -resolve_mouse_binding_collisions(struct config *conf, const char *section_name, - const char *const action_map[], - struct config_mouse_binding_list *bindings) +static bool NOINLINE +resolve_key_binding_collisions(struct config *conf, const char *section_name, + const char *const action_map[], + struct config_key_binding_list *bindings, + enum config_key_binding_type type) { bool ret = true; @@ -2188,23 +2004,27 @@ resolve_mouse_binding_collisions(struct config *conf, const char *section_name, enum {COLLISION_NONE, COLLISION_OVERRIDE, COLLISION_BINDING} collision_type = COLLISION_NONE; - const struct config_mouse_binding *collision_binding = NULL; + const struct config_key_binding *collision_binding = NULL; - struct config_mouse_binding *binding1 = &bindings->arr[i]; + struct config_key_binding *binding1 = &bindings->arr[i]; xassert(binding1->action != BIND_ACTION_NONE); const struct config_key_modifiers *mods1 = &binding1->modifiers; /* Does our modifiers collide with the selection override mods? */ - if (!modifiers_disjoint(mods1, &conf->mouse.selection_override_modifiers)) + if (type == MOUSE_BINDING && + !modifiers_disjoint( + mods1, &conf->mouse.selection_override_modifiers)) + { collision_type = COLLISION_OVERRIDE; + } /* Does our binding collide with another binding? */ for (ssize_t j = i - 1; collision_type == COLLISION_NONE && j >= 0; j--) { - const struct config_mouse_binding *binding2 = &bindings->arr[j]; + const struct config_key_binding *binding2 = &bindings->arr[j]; xassert(binding2->action != BIND_ACTION_NONE); if (binding2->action == binding1->action) { @@ -2215,10 +2035,20 @@ resolve_mouse_binding_collisions(struct config *conf, const char *section_name, const struct config_key_modifiers *mods2 = &binding2->modifiers; bool mods_equal = modifiers_equal(mods1, mods2); - bool button_equal = binding1->button == binding2->button; - bool count_equal = binding1->count == binding2->count; + bool sym_equal; - if (!mods_equal || !button_equal || !count_equal) + switch (type) { + case KEY_BINDING: + sym_equal = binding1->k.sym == binding2->k.sym; + break; + + case MOUSE_BINDING: + sym_equal = (binding1->m.button == binding2->m.button && + binding1->m.count == binding2->m.count); + break; + } + + if (!mods_equal || !sym_equal) continue; collision_binding = binding2; @@ -2228,13 +2058,25 @@ resolve_mouse_binding_collisions(struct config *conf, const char *section_name, if (collision_type != COLLISION_NONE) { char *modifier_names = modifiers_to_str(mods1); - const char *button_name = mouse_button_code_to_name(binding1->button); + char sym_name[64]; - char click_count[16]; - if (binding1->count > 1) - sprintf(click_count, "-%d", binding1->count); - else - click_count[0] = '\0'; + switch (type){ + case KEY_BINDING: + xkb_keysym_get_name(binding1->k.sym, sym_name, sizeof(sym_name)); + break; + + case MOUSE_BINDING: { + const char *button_name = + mouse_button_code_to_name(binding1->m.button); + + if (binding1->m.count > 1) { + snprintf(sym_name, sizeof(sym_name), "%s-%d", + button_name, binding1->m.count); + } else + strcpy(sym_name, button_name); + break; + } + } switch (collision_type) { case COLLISION_NONE: @@ -2243,10 +2085,10 @@ resolve_mouse_binding_collisions(struct config *conf, const char *section_name, case COLLISION_BINDING: { bool has_pipe = collision_binding->pipe.argv.args != NULL; LOG_AND_NOTIFY_ERR( - "%s:%d: [%s].%s: %s%s%s already mapped to '%s%s%s%s'", + "%s:%d: [%s].%s: %s%s already mapped to '%s%s%s%s'", binding1->path, binding1->lineno, section_name, action_map[binding1->action], - modifier_names, button_name, click_count, + modifier_names, sym_name, action_map[collision_binding->action], has_pipe ? " [" : "", has_pipe ? collision_binding->pipe.argv.args[0] : "", @@ -2262,20 +2104,20 @@ resolve_mouse_binding_collisions(struct config *conf, const char *section_name, override_names[strlen(override_names) - 1] = '\0'; LOG_AND_NOTIFY_ERR( - "%s:%d: [%s].%s: %s%s%s: " + "%s:%d: [%s].%s: %s%s: " "modifiers conflict with 'selection-override-modifiers=%s'", binding1->path, binding1->lineno, section_name, action_map[binding1->action], - modifier_names, button_name, click_count, - override_names); + modifier_names, sym_name, override_names); free(override_names); break; } } + free(modifier_names); if (binding1->pipe.master_copy && i + 1 < bindings->count) { - struct config_mouse_binding *next = &bindings->arr[i + 1]; + struct config_key_binding *next = &bindings->arr[i + 1]; if (next->action == binding1->action && argv_compare(&binding1->pipe.argv, &next->pipe.argv) == 0) @@ -2286,7 +2128,7 @@ resolve_mouse_binding_collisions(struct config *conf, const char *section_name, } } - free_mouse_binding(binding1); + free_key_binding(binding1); /* Remove the most recent binding */ size_t move_count = bindings->count - (i + 1); @@ -2336,8 +2178,8 @@ parse_section_mouse_bindings(struct context *ctx) if (strcmp(key, binding_action_map[action]) != 0) continue; - if (!value_to_mouse_combos( - ctx, action, &pipe_argv, &conf->bindings.mouse)) + if (!value_to_key_combos( + ctx, action, &pipe_argv, &conf->bindings.mouse, MOUSE_BINDING)) { free_argv(&pipe_argv); return false; @@ -2743,21 +2585,21 @@ static void add_default_key_bindings(struct config *conf) { static const struct config_key_binding bindings[] = { - {BIND_ACTION_SCROLLBACK_UP_PAGE, m_shift, XKB_KEY_Prior}, - {BIND_ACTION_SCROLLBACK_DOWN_PAGE, m_shift, XKB_KEY_Next}, - {BIND_ACTION_CLIPBOARD_COPY, m_ctrl_shift, XKB_KEY_c}, - {BIND_ACTION_CLIPBOARD_PASTE, m_ctrl_shift, XKB_KEY_v}, - {BIND_ACTION_PRIMARY_PASTE, m_shift, XKB_KEY_Insert}, - {BIND_ACTION_SEARCH_START, m_ctrl_shift, XKB_KEY_r}, - {BIND_ACTION_FONT_SIZE_UP, m_ctrl, XKB_KEY_plus}, - {BIND_ACTION_FONT_SIZE_UP, m_ctrl, XKB_KEY_equal}, - {BIND_ACTION_FONT_SIZE_UP, m_ctrl, XKB_KEY_KP_Add}, - {BIND_ACTION_FONT_SIZE_DOWN, m_ctrl, XKB_KEY_minus}, - {BIND_ACTION_FONT_SIZE_DOWN, m_ctrl, XKB_KEY_KP_Subtract}, - {BIND_ACTION_FONT_SIZE_RESET, m_ctrl, XKB_KEY_0}, - {BIND_ACTION_FONT_SIZE_RESET, m_ctrl, XKB_KEY_KP_0}, - {BIND_ACTION_SPAWN_TERMINAL, m_ctrl_shift, XKB_KEY_n}, - {BIND_ACTION_SHOW_URLS_LAUNCH, m_ctrl_shift, XKB_KEY_u}, + {BIND_ACTION_SCROLLBACK_UP_PAGE, m_shift, {{XKB_KEY_Prior}}}, + {BIND_ACTION_SCROLLBACK_DOWN_PAGE, m_shift, {{XKB_KEY_Next}}}, + {BIND_ACTION_CLIPBOARD_COPY, m_ctrl_shift, {{XKB_KEY_c}}}, + {BIND_ACTION_CLIPBOARD_PASTE, m_ctrl_shift, {{XKB_KEY_v}}}, + {BIND_ACTION_PRIMARY_PASTE, m_shift, {{XKB_KEY_Insert}}}, + {BIND_ACTION_SEARCH_START, m_ctrl_shift, {{XKB_KEY_r}}}, + {BIND_ACTION_FONT_SIZE_UP, m_ctrl, {{XKB_KEY_plus}}}, + {BIND_ACTION_FONT_SIZE_UP, m_ctrl, {{XKB_KEY_equal}}}, + {BIND_ACTION_FONT_SIZE_UP, m_ctrl, {{XKB_KEY_KP_Add}}}, + {BIND_ACTION_FONT_SIZE_DOWN, m_ctrl, {{XKB_KEY_minus}}}, + {BIND_ACTION_FONT_SIZE_DOWN, m_ctrl, {{XKB_KEY_KP_Subtract}}}, + {BIND_ACTION_FONT_SIZE_RESET, m_ctrl, {{XKB_KEY_0}}}, + {BIND_ACTION_FONT_SIZE_RESET, m_ctrl, {{XKB_KEY_KP_0}}}, + {BIND_ACTION_SPAWN_TERMINAL, m_ctrl_shift, {{XKB_KEY_n}}}, + {BIND_ACTION_SHOW_URLS_LAUNCH, m_ctrl_shift, {{XKB_KEY_u}}}, }; conf->bindings.key.count = ALEN(bindings); @@ -2770,35 +2612,35 @@ static void add_default_search_bindings(struct config *conf) { static const struct config_key_binding bindings[] = { - {BIND_ACTION_SEARCH_CANCEL, m_ctrl, XKB_KEY_c}, - {BIND_ACTION_SEARCH_CANCEL, m_ctrl, XKB_KEY_g}, - {BIND_ACTION_SEARCH_CANCEL, m_none, XKB_KEY_Escape}, - {BIND_ACTION_SEARCH_COMMIT, m_none, XKB_KEY_Return}, - {BIND_ACTION_SEARCH_FIND_PREV, m_ctrl, XKB_KEY_r}, - {BIND_ACTION_SEARCH_FIND_NEXT, m_ctrl, XKB_KEY_s}, - {BIND_ACTION_SEARCH_EDIT_LEFT, m_none, XKB_KEY_Left}, - {BIND_ACTION_SEARCH_EDIT_LEFT, m_ctrl, XKB_KEY_b}, - {BIND_ACTION_SEARCH_EDIT_LEFT_WORD, m_ctrl, XKB_KEY_Left}, - {BIND_ACTION_SEARCH_EDIT_LEFT_WORD, m_alt, XKB_KEY_b}, - {BIND_ACTION_SEARCH_EDIT_RIGHT, m_none, XKB_KEY_Right}, - {BIND_ACTION_SEARCH_EDIT_RIGHT, m_ctrl, XKB_KEY_f}, - {BIND_ACTION_SEARCH_EDIT_RIGHT_WORD, m_ctrl, XKB_KEY_Right}, - {BIND_ACTION_SEARCH_EDIT_RIGHT_WORD, m_alt, XKB_KEY_f}, - {BIND_ACTION_SEARCH_EDIT_HOME, m_none, XKB_KEY_Home}, - {BIND_ACTION_SEARCH_EDIT_HOME, m_ctrl, XKB_KEY_a}, - {BIND_ACTION_SEARCH_EDIT_END, m_none, XKB_KEY_End}, - {BIND_ACTION_SEARCH_EDIT_END, m_ctrl, XKB_KEY_e}, - {BIND_ACTION_SEARCH_DELETE_PREV, m_none, XKB_KEY_BackSpace}, - {BIND_ACTION_SEARCH_DELETE_PREV_WORD, m_ctrl, XKB_KEY_BackSpace}, - {BIND_ACTION_SEARCH_DELETE_PREV_WORD, m_alt, XKB_KEY_BackSpace}, - {BIND_ACTION_SEARCH_DELETE_NEXT, m_none, XKB_KEY_Delete}, - {BIND_ACTION_SEARCH_DELETE_NEXT_WORD, m_ctrl, XKB_KEY_Delete}, - {BIND_ACTION_SEARCH_DELETE_NEXT_WORD, m_alt, XKB_KEY_d}, - {BIND_ACTION_SEARCH_EXTEND_WORD, m_ctrl, XKB_KEY_w}, - {BIND_ACTION_SEARCH_EXTEND_WORD_WS, m_ctrl_shift, XKB_KEY_w}, - {BIND_ACTION_SEARCH_CLIPBOARD_PASTE, m_ctrl, XKB_KEY_v}, - {BIND_ACTION_SEARCH_CLIPBOARD_PASTE, m_ctrl, XKB_KEY_y}, - {BIND_ACTION_SEARCH_PRIMARY_PASTE, m_shift, XKB_KEY_Insert}, + {BIND_ACTION_SEARCH_CANCEL, m_ctrl, {{XKB_KEY_c}}}, + {BIND_ACTION_SEARCH_CANCEL, m_ctrl, {{XKB_KEY_g}}}, + {BIND_ACTION_SEARCH_CANCEL, m_none, {{XKB_KEY_Escape}}}, + {BIND_ACTION_SEARCH_COMMIT, m_none, {{XKB_KEY_Return}}}, + {BIND_ACTION_SEARCH_FIND_PREV, m_ctrl, {{XKB_KEY_r}}}, + {BIND_ACTION_SEARCH_FIND_NEXT, m_ctrl, {{XKB_KEY_s}}}, + {BIND_ACTION_SEARCH_EDIT_LEFT, m_none, {{XKB_KEY_Left}}}, + {BIND_ACTION_SEARCH_EDIT_LEFT, m_ctrl, {{XKB_KEY_b}}}, + {BIND_ACTION_SEARCH_EDIT_LEFT_WORD, m_ctrl, {{XKB_KEY_Left}}}, + {BIND_ACTION_SEARCH_EDIT_LEFT_WORD, m_alt, {{XKB_KEY_b}}}, + {BIND_ACTION_SEARCH_EDIT_RIGHT, m_none, {{XKB_KEY_Right}}}, + {BIND_ACTION_SEARCH_EDIT_RIGHT, m_ctrl, {{XKB_KEY_f}}}, + {BIND_ACTION_SEARCH_EDIT_RIGHT_WORD, m_ctrl, {{XKB_KEY_Right}}}, + {BIND_ACTION_SEARCH_EDIT_RIGHT_WORD, m_alt, {{XKB_KEY_f}}}, + {BIND_ACTION_SEARCH_EDIT_HOME, m_none, {{XKB_KEY_Home}}}, + {BIND_ACTION_SEARCH_EDIT_HOME, m_ctrl, {{XKB_KEY_a}}}, + {BIND_ACTION_SEARCH_EDIT_END, m_none, {{XKB_KEY_End}}}, + {BIND_ACTION_SEARCH_EDIT_END, m_ctrl, {{XKB_KEY_e}}}, + {BIND_ACTION_SEARCH_DELETE_PREV, m_none, {{XKB_KEY_BackSpace}}}, + {BIND_ACTION_SEARCH_DELETE_PREV_WORD, m_ctrl, {{XKB_KEY_BackSpace}}}, + {BIND_ACTION_SEARCH_DELETE_PREV_WORD, m_alt, {{XKB_KEY_BackSpace}}}, + {BIND_ACTION_SEARCH_DELETE_NEXT, m_none, {{XKB_KEY_Delete}}}, + {BIND_ACTION_SEARCH_DELETE_NEXT_WORD, m_ctrl, {{XKB_KEY_Delete}}}, + {BIND_ACTION_SEARCH_DELETE_NEXT_WORD, m_alt, {{XKB_KEY_d}}}, + {BIND_ACTION_SEARCH_EXTEND_WORD, m_ctrl, {{XKB_KEY_w}}}, + {BIND_ACTION_SEARCH_EXTEND_WORD_WS, m_ctrl_shift, {{XKB_KEY_w}}}, + {BIND_ACTION_SEARCH_CLIPBOARD_PASTE, m_ctrl, {{XKB_KEY_v}}}, + {BIND_ACTION_SEARCH_CLIPBOARD_PASTE, m_ctrl, {{XKB_KEY_y}}}, + {BIND_ACTION_SEARCH_PRIMARY_PASTE, m_shift, {{XKB_KEY_Insert}}}, }; conf->bindings.search.count = ALEN(bindings); @@ -2810,11 +2652,11 @@ static void add_default_url_bindings(struct config *conf) { static const struct config_key_binding bindings[] = { - {BIND_ACTION_URL_CANCEL, m_ctrl, XKB_KEY_c}, - {BIND_ACTION_URL_CANCEL, m_ctrl, XKB_KEY_g}, - {BIND_ACTION_URL_CANCEL, m_ctrl, XKB_KEY_d}, - {BIND_ACTION_URL_CANCEL, m_none, XKB_KEY_Escape}, - {BIND_ACTION_URL_TOGGLE_URL_ON_JUMP_LABEL, m_none, XKB_KEY_t}, + {BIND_ACTION_URL_CANCEL, m_ctrl, {{XKB_KEY_c}}}, + {BIND_ACTION_URL_CANCEL, m_ctrl, {{XKB_KEY_g}}}, + {BIND_ACTION_URL_CANCEL, m_ctrl, {{XKB_KEY_d}}}, + {BIND_ACTION_URL_CANCEL, m_none, {{XKB_KEY_Escape}}}, + {BIND_ACTION_URL_TOGGLE_URL_ON_JUMP_LABEL, m_none, {{XKB_KEY_t}}}, }; conf->bindings.url.count = ALEN(bindings); @@ -2825,15 +2667,15 @@ add_default_url_bindings(struct config *conf) static void add_default_mouse_bindings(struct config *conf) { - static const struct config_mouse_binding bindings[] = { - {BIND_ACTION_PRIMARY_PASTE, m_none, BTN_MIDDLE, 1}, - {BIND_ACTION_SELECT_BEGIN, m_none, BTN_LEFT, 1}, - {BIND_ACTION_SELECT_BEGIN_BLOCK, m_ctrl, BTN_LEFT, 1}, - {BIND_ACTION_SELECT_EXTEND, m_none, BTN_RIGHT, 1}, - {BIND_ACTION_SELECT_EXTEND_CHAR_WISE, m_ctrl, BTN_RIGHT, 1}, - {BIND_ACTION_SELECT_WORD, m_none, BTN_LEFT, 2}, - {BIND_ACTION_SELECT_WORD_WS, m_ctrl, BTN_LEFT, 2}, - {BIND_ACTION_SELECT_ROW, m_none, BTN_LEFT, 3}, + static const struct config_key_binding bindings[] = { + {BIND_ACTION_PRIMARY_PASTE, m_none, {.m = {BTN_MIDDLE, 1}}}, + {BIND_ACTION_SELECT_BEGIN, m_none, {.m = {BTN_LEFT, 1}}}, + {BIND_ACTION_SELECT_BEGIN_BLOCK, m_ctrl, {.m = {BTN_LEFT, 1}}}, + {BIND_ACTION_SELECT_EXTEND, m_none, {.m = {BTN_RIGHT, 1}}}, + {BIND_ACTION_SELECT_EXTEND_CHAR_WISE, m_ctrl, {.m = {BTN_RIGHT, 1}}}, + {BIND_ACTION_SELECT_WORD, m_none, {.m = {BTN_LEFT, 2}}}, + {BIND_ACTION_SELECT_WORD_WS, m_ctrl, {.m = {BTN_LEFT, 2}}}, + {BIND_ACTION_SELECT_ROW, m_none, {.m = {BTN_LEFT, 3}}}, }; conf->bindings.mouse.count = ALEN(bindings); @@ -3066,16 +2908,16 @@ config_load(struct config *conf, const char *conf_path, if (ret && (!resolve_key_binding_collisions( conf, section_info[SECTION_KEY_BINDINGS].name, - binding_action_map, &conf->bindings.key) || + binding_action_map, &conf->bindings.key, KEY_BINDING) || !resolve_key_binding_collisions( conf, section_info[SECTION_SEARCH_BINDINGS].name, - search_binding_action_map, &conf->bindings.search) || + search_binding_action_map, &conf->bindings.search, KEY_BINDING) || !resolve_key_binding_collisions( conf, section_info[SECTION_URL_BINDINGS].name, - url_binding_action_map, &conf->bindings.url) || - !resolve_mouse_binding_collisions( + url_binding_action_map, &conf->bindings.url, KEY_BINDING) || + !resolve_key_binding_collisions( conf, section_info[SECTION_MOUSE_BINDINGS].name, - binding_action_map, &conf->bindings.mouse))) + binding_action_map, &conf->bindings.mouse, MOUSE_BINDING))) { ret = !errors_are_fatal; } @@ -3198,34 +3040,6 @@ key_binding_list_clone(struct config_key_binding_list *dst, } } -static void NOINLINE -mouse_binding_list_clone(struct config_mouse_binding_list *dst, - const struct config_mouse_binding_list *src) -{ - struct argv *last_master_argv = NULL; - - dst->count = src->count; - dst->arr = xmalloc(src->count * sizeof(dst->arr[0])); - - for (size_t i = 0; i < src->count; i++) { - const struct config_mouse_binding *old = &src->arr[i]; - struct config_mouse_binding *new = &dst->arr[i]; - - *new = *old; - - if (old->pipe.argv.args == NULL) - continue; - - if (old->pipe.master_copy) { - binding_pipe_clone(&new->pipe, &old->pipe); - last_master_argv = &new->pipe.argv; - } else { - xassert(last_master_argv != NULL); - new->pipe.argv = *last_master_argv; - } - } -} - struct config * config_clone(const struct config *old) { @@ -3257,7 +3071,7 @@ config_clone(const struct config *old) key_binding_list_clone(&conf->bindings.key, &old->bindings.key); key_binding_list_clone(&conf->bindings.search, &old->bindings.search); key_binding_list_clone(&conf->bindings.url, &old->bindings.url); - mouse_binding_list_clone(&conf->bindings.mouse, &old->bindings.mouse); + key_binding_list_clone(&conf->bindings.mouse, &old->bindings.mouse); conf->notifications.length = 0; conf->notifications.head = conf->notifications.tail = 0; @@ -3317,7 +3131,7 @@ config_free(struct config conf) free_key_binding_list(&conf.bindings.key); free_key_binding_list(&conf.bindings.search); free_key_binding_list(&conf.bindings.url); - free_mouse_binding_list(&conf.bindings.mouse); + free_key_binding_list(&conf.bindings.mouse); user_notifications_free(&conf.notifications); } diff --git a/config.h b/config.h index 153b9c27..4a724bec 100644 --- a/config.h +++ b/config.h @@ -40,10 +40,27 @@ struct config_binding_pipe { bool master_copy; }; +enum config_key_binding_type { + KEY_BINDING, + MOUSE_BINDING, +}; + struct config_key_binding { int action; /* One of the varios bind_action_* enums from wayland.h */ struct config_key_modifiers modifiers; - xkb_keysym_t sym; + union { + /* Key bindings */ + struct { + xkb_keysym_t sym; + } k; + + /* Mouse bindings */ + struct { + int button; + int count; + } m; + }; + struct config_binding_pipe pipe; /* For error messages in collision handling */ @@ -52,6 +69,7 @@ struct config_key_binding { }; DEFINE_LIST(struct config_key_binding); +#if 0 struct config_mouse_binding { enum bind_action_normal action; struct config_key_modifiers modifiers; @@ -64,6 +82,7 @@ struct config_mouse_binding { int lineno; }; DEFINE_LIST(struct config_mouse_binding); +#endif typedef tll(char *) config_override_t; @@ -207,7 +226,7 @@ struct config { struct { /* Bindings for "normal" mode */ struct config_key_binding_list key; - struct config_mouse_binding_list mouse; + struct config_key_binding_list mouse; /* * Special modes diff --git a/input.c b/input.c index 0bd4b347..3e029ab2 100644 --- a/input.c +++ b/input.c @@ -520,7 +520,7 @@ convert_key_binding(const struct seat *seat, key_binding_list_t *bindings) { xkb_mod_mask_t mods = conf_modifiers_to_mask(seat, &conf_binding->modifiers); - xkb_keysym_t sym = maybe_repair_key_combo(seat, conf_binding->sym, mods); + xkb_keysym_t sym = maybe_repair_key_combo(seat, conf_binding->k.sym, mods); struct key_binding binding = { .mods = mods, @@ -561,13 +561,13 @@ convert_url_bindings(const struct config *conf, struct seat *seat) static void convert_mouse_binding(struct seat *seat, - const struct config_mouse_binding *conf_binding) + const struct config_key_binding *conf_binding) { struct mouse_binding binding = { .action = conf_binding->action, .mods = conf_modifiers_to_mask(seat, &conf_binding->modifiers), - .button = conf_binding->button, - .count = conf_binding->count, + .button = conf_binding->m.button, + .count = conf_binding->m.count, .pipe_argv = conf_binding->pipe.argv.args, }; tll_push_back(seat->mouse.bindings, binding); @@ -577,9 +577,7 @@ static void convert_mouse_bindings(const struct config *conf, struct seat *seat) { for (size_t i = 0; i < conf->bindings.mouse.count; i++) { - const struct config_mouse_binding *binding = &conf->bindings.mouse.arr[i]; - if (binding->action == BIND_ACTION_NONE) - continue; + const struct config_key_binding *binding = &conf->bindings.mouse.arr[i]; convert_mouse_binding(seat, binding); } } @@ -2543,19 +2541,19 @@ wl_pointer_button(void *data, struct wl_pointer *wl_pointer, else { /* Seat does NOT have a keyboard - use mouse bindings *without* modifiers */ - const struct config_mouse_binding *match = NULL; + const struct config_key_binding *match = NULL; const struct config *conf = seat->wayl->conf; for (size_t i = 0; i < conf->bindings.mouse.count; i++) { - const struct config_mouse_binding *binding = + const struct config_key_binding *binding = &conf->bindings.mouse.arr[i]; - if (binding->button != button) { + if (binding->m.button != button) { /* Wrong button */ continue; } - if (binding->count > seat->mouse.count) { + if (binding->m.count > seat->mouse.count) { /* Incorrect click count */ continue; } @@ -2566,7 +2564,7 @@ wl_pointer_button(void *data, struct wl_pointer *wl_pointer, continue; } - if (match == NULL || binding->count > match->count) + if (match == NULL || binding->m.count > match->m.count) match = binding; } From 3512a7febf16c2cc6db25f81fc26c51b17d56c20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Mon, 6 Dec 2021 21:13:55 +0100 Subject: [PATCH 07/14] config: remove unneeded function prototype --- config.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/config.c b/config.c index cee4454c..5265a910 100644 --- a/config.c +++ b/config.c @@ -1365,10 +1365,6 @@ parse_section_cursor(struct context *ctx) return true; } -static bool -parse_modifiers(struct context *ctx, const char *text, size_t len, - struct config_key_modifiers *modifiers); - static bool parse_section_mouse(struct context *ctx) { From ff82a3900e0fd09a4c59612494920fdd6ecac1eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Mon, 6 Dec 2021 21:15:18 +0100 Subject: [PATCH 08/14] config: remove commented out struct definition --- config.h | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/config.h b/config.h index 4a724bec..84aee3ae 100644 --- a/config.h +++ b/config.h @@ -69,21 +69,6 @@ struct config_key_binding { }; DEFINE_LIST(struct config_key_binding); -#if 0 -struct config_mouse_binding { - enum bind_action_normal action; - struct config_key_modifiers modifiers; - int button; - int count; - struct config_binding_pipe pipe; - - /* For error messages in collision handling */ - const char *path; - int lineno; -}; -DEFINE_LIST(struct config_mouse_binding); -#endif - typedef tll(char *) config_override_t; struct config_spawn_template { From 6fa09d24e50132b65d1781ff085de8cffcc742df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Mon, 6 Dec 2021 21:20:03 +0100 Subject: [PATCH 09/14] =?UTF-8?q?config:=20use=20=E2=80=9C(default)?= =?UTF-8?q?=E2=80=9D=20as=20path=20in=20log=20message,=20if=20path=20is=20?= =?UTF-8?q?NULL?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- config.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/config.c b/config.c index 5265a910..644147c8 100644 --- a/config.c +++ b/config.c @@ -2102,7 +2102,8 @@ resolve_key_binding_collisions(struct config *conf, const char *section_name, LOG_AND_NOTIFY_ERR( "%s:%d: [%s].%s: %s%s: " "modifiers conflict with 'selection-override-modifiers=%s'", - binding1->path, binding1->lineno, section_name, + binding1->path != NULL ? binding1->path : "(default)", + binding1->lineno, section_name, action_map[binding1->action], modifier_names, sym_name, override_names); free(override_names); From 17250ec39303755954ea8cafc5c1e93ba89e74dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sat, 11 Dec 2021 20:00:32 +0100 Subject: [PATCH 10/14] =?UTF-8?q?config:=20resolve=20collisions:=20return?= =?UTF-8?q?=20=E2=80=98false=E2=80=99=20on=20failure?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- config.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/config.c b/config.c index 644147c8..8dab70e9 100644 --- a/config.c +++ b/config.c @@ -2089,6 +2089,7 @@ resolve_key_binding_collisions(struct config *conf, const char *section_name, has_pipe ? " [" : "", has_pipe ? collision_binding->pipe.argv.args[0] : "", has_pipe ? "]" : ""); + ret = false; break; } @@ -2106,6 +2107,7 @@ resolve_key_binding_collisions(struct config *conf, const char *section_name, binding1->lineno, section_name, action_map[binding1->action], modifier_names, sym_name, override_names); + ret = false; free(override_names); break; } From 2a02ba77a1d021e3af7d4e386db8e2e8c2569943 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sat, 11 Dec 2021 20:00:55 +0100 Subject: [PATCH 11/14] config: resolve collisions: argv_compare() returns -1,0,+1, not a bool --- config.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/config.c b/config.c index 8dab70e9..b0fffaf7 100644 --- a/config.c +++ b/config.c @@ -2024,8 +2024,11 @@ resolve_key_binding_collisions(struct config *conf, const char *section_name, xassert(binding2->action != BIND_ACTION_NONE); if (binding2->action == binding1->action) { - if (argv_compare(&binding1->pipe.argv, &binding2->pipe.argv)) + if (argv_compare( + &binding1->pipe.argv, &binding2->pipe.argv) == 0) + { continue; + } } const struct config_key_modifiers *mods2 = &binding2->modifiers; From 40249ab3a29dbdb0abb58a731296db5bcff0ec8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sat, 11 Dec 2021 20:01:16 +0100 Subject: [PATCH 12/14] test: config: port key/mouse binding tests to new API --- tests/test-config.c | 438 ++++++++++++++++++++++---------------------- 1 file changed, 214 insertions(+), 224 deletions(-) diff --git a/tests/test-config.c b/tests/test-config.c index befe4864..94e211f0 100644 --- a/tests/test-config.c +++ b/tests/test-config.c @@ -265,7 +265,8 @@ test_section_main(void) static void test_key_binding(struct context *ctx, bool (*parse_fun)(struct context *ctx), int action, int max_action, const char *const *map, - struct config_key_binding_list *bindings) + struct config_key_binding_list *bindings, + enum config_key_binding_type type) { xassert(map[action] != NULL); xassert(bindings->count == 0); @@ -280,20 +281,43 @@ test_key_binding(struct context *ctx, bool (*parse_fun)(struct context *ctx), /* Generate the modifier part of the ‘value’ */ char modifier_string[32]; - int chars = sprintf(modifier_string, "%s%s%s%s", - ctrl ? XKB_MOD_NAME_CTRL "+" : "", - alt ? XKB_MOD_NAME_ALT "+" : "", - shift ? XKB_MOD_NAME_SHIFT "+" : "", - super ? XKB_MOD_NAME_LOGO "+" : ""); + sprintf(modifier_string, "%s%s%s%s", + ctrl ? XKB_MOD_NAME_CTRL "+" : "", + alt ? XKB_MOD_NAME_ALT "+" : "", + shift ? XKB_MOD_NAME_SHIFT "+" : "", + super ? XKB_MOD_NAME_LOGO "+" : ""); - /* Use a unique symbol for this action */ - xkb_keysym_t sym = XKB_KEY_a + action; - char sym_name[8]; - xkb_keysym_get_name(sym, sym_name, sizeof(sym_name)); + /* Use a unique symbol for this action (key bindings) */ + const xkb_keysym_t sym = XKB_KEY_a + action; + + /* Mouse button (mouse bindings) */ + const int button_idx = action % ALEN(button_map); + const int button = button_map[button_idx].code; + const int click_count = action % 3 + 1; /* Finally, generate the ‘value’ (e.g. “Control+shift+x”) */ - char value[chars + strlen(sym_name) + 1]; - sprintf(value, "%s%s", modifier_string, sym_name); + char value[128]; + + switch (type) { + case KEY_BINDING: { + char sym_name[16]; + xkb_keysym_get_name(sym, sym_name, sizeof(sym_name)); + + snprintf(value, sizeof(value), "%s%s", modifier_string, sym_name); + break; + } + + case MOUSE_BINDING: { + const char *const button_name = button_map[button_idx].name; + int chars = snprintf( + value, sizeof(value), "%s%s", modifier_string, button_name); + + xassert(click_count > 0); + if (click_count > 1) + snprintf(&value[chars], sizeof(value) - chars, "-%d", click_count); + break; + } + } ctx->key = key; ctx->value = value; @@ -313,242 +337,153 @@ test_key_binding(struct context *ctx, bool (*parse_fun)(struct context *ctx), ctx->section, ctx->key, ctx->value, binding->action, action); } - if (binding->sym != sym) { - BUG("[%s].%s=%s: key symbol mismatch: %d != %d", - ctx->section, ctx->key, ctx->value, binding->sym, sym); - } - if (binding->modifiers.ctrl != ctrl || binding->modifiers.alt != alt || binding->modifiers.shift != shift || - binding->modifiers.meta != super) + binding->modifiers.super != super) { BUG("[%s].%s=%s: modifier mismatch:\n" " have: ctrl=%d, alt=%d, shift=%d, super=%d\n" " expected: ctrl=%d, alt=%d, shift=%d, super=%d", ctx->section, ctx->key, ctx->value, binding->modifiers.ctrl, binding->modifiers.alt, - binding->modifiers.shift, binding->modifiers.meta, + binding->modifiers.shift, binding->modifiers.super, ctrl, alt, shift, super); } - key_binding_list_free(bindings); - bindings->arr = NULL; - bindings->count = 0; + switch (type) { + case KEY_BINDING: + if (binding->k.sym != sym) { + BUG("[%s].%s=%s: key symbol mismatch: %d != %d", + ctx->section, ctx->key, ctx->value, binding->k.sym, sym); + } + break; - if (action >= max_action) - return; + case MOUSE_BINDING:; + if (binding->m.button != button) { + BUG("[%s].%s=%s: mouse button mismatch: %d != %d", + ctx->section, ctx->key, ctx->value, binding->m.button, button); + } - /* - * Test collisions - */ + if (binding->m.count != click_count) { + BUG("[%s].%s=%s: mouse button click count mismatch: %d != %d", + ctx->section, ctx->key, ctx->value, + binding->m.count, click_count); + } + break; + } + + + free_key_binding_list(bindings); +} + +enum collision_test_mode { + FAIL_DIFFERENT_ACTION, + FAIL_DIFFERENT_ARGV, + SUCCED_SAME_ACTION_AND_ARGV +}; + +static void +_test_binding_collisions(struct context *ctx, + int max_action, const char *const *map, + enum config_key_binding_type type, + enum collision_test_mode test_mode) +{ + struct config_key_binding *bindings_array = + xcalloc(2, sizeof(bindings_array[0])); + + struct config_key_binding_list bindings = { + .count = 2, + .arr = bindings_array, + }; /* First, verify we get a collision when trying to assign the same * key combo to multiple actions */ - bindings->count = 1; - bindings->arr = xmalloc(sizeof(bindings->arr[0])); - bindings->arr[0] = (struct config_key_binding){ - .action = action + 1, - .sym = XKB_KEY_a, - .modifiers = { - .ctrl = true, - }, + bindings.arr[0] = (struct config_key_binding){ + .action = (test_mode == FAIL_DIFFERENT_ACTION + ? max_action - 1 : max_action), + .modifiers = {.ctrl = true}, + .path = "unittest", + }; + bindings.arr[1] = (struct config_key_binding){ + .action = max_action, + .modifiers = {.ctrl = true}, + .path = "unittest", }; - xkb_keysym_get_name(XKB_KEY_a, sym_name, sizeof(sym_name)); + switch (type) { + case KEY_BINDING: + bindings.arr[0].k.sym = XKB_KEY_a; + bindings.arr[1].k.sym = XKB_KEY_a; + break; - char collision[128]; - snprintf(collision, sizeof(collision), "%s+%s", XKB_MOD_NAME_CTRL, sym_name); - - ctx->value = collision; - if (parse_fun(ctx)) { - BUG("[%s].%s=%s: key combo collision not detected", - ctx->section, ctx->key, ctx->value); + case MOUSE_BINDING: + bindings.arr[0].m.button = BTN_LEFT; + bindings.arr[0].m.count = 1; + bindings.arr[1].m.button = BTN_LEFT; + bindings.arr[1].m.count = 1; + break; } - /* Next, verify we get a collision when trying to assign the same - * key combo to the same action, but with different pipe argvs */ - bindings->arr[0].action = action; - bindings->arr[0].pipe.master_copy = true; - bindings->arr[0].pipe.argv.args = xmalloc(4 * sizeof(bindings->arr[0].pipe.argv.args[0])); - bindings->arr[0].pipe.argv.args[0] = xstrdup("/usr/bin/foobar"); - bindings->arr[0].pipe.argv.args[1] = xstrdup("hello"); - bindings->arr[0].pipe.argv.args[2] = xstrdup("world"); - bindings->arr[0].pipe.argv.args[3] = NULL; + switch (test_mode) { + case FAIL_DIFFERENT_ACTION: + break; - snprintf(collision, sizeof(collision), - "[/usr/bin/foobar hello] %s+%s", - XKB_MOD_NAME_CTRL, sym_name); + case FAIL_DIFFERENT_ARGV: + case SUCCED_SAME_ACTION_AND_ARGV: + bindings.arr[0].pipe.master_copy = true; + bindings.arr[0].pipe.argv.args = xcalloc( + 4, sizeof(bindings.arr[0].pipe.argv.args[0])); + bindings.arr[0].pipe.argv.args[0] = xstrdup("/usr/bin/foobar"); + bindings.arr[0].pipe.argv.args[1] = xstrdup("hello"); + bindings.arr[0].pipe.argv.args[2] = xstrdup("world"); - ctx->value = collision; - if (parse_fun(ctx)) { - BUG("[%s].%s=%s: key combo collision not detected", - ctx->section, ctx->key, ctx->value); + bindings.arr[1].pipe.master_copy = true; + bindings.arr[1].pipe.argv.args = xcalloc( + 4, sizeof(bindings.arr[1].pipe.argv.args[0])); + bindings.arr[1].pipe.argv.args[0] = xstrdup("/usr/bin/foobar"); + bindings.arr[1].pipe.argv.args[1] = xstrdup("hello"); + + if (test_mode == SUCCED_SAME_ACTION_AND_ARGV) + bindings.arr[1].pipe.argv.args[2] = xstrdup("world"); + break; } - /* Finally, verify we do *not* get a collision when assigning the - * same key combo to the same action, with matching argvs */ - snprintf(collision, sizeof(collision), - "[/usr/bin/foobar hello world] %s+%s", - XKB_MOD_NAME_CTRL, sym_name); + bool expected_result = + test_mode == SUCCED_SAME_ACTION_AND_ARGV ? true : false; - ctx->value = collision; - if (!parse_fun(ctx)) { - BUG("[%s].%s=%s: invalid key combo collision", - ctx->section, ctx->key, ctx->value); + if (resolve_key_binding_collisions( + ctx->conf, ctx->section, map, &bindings, type) != expected_result) + { + BUG("[%s].%s vs. %s: %s", + ctx->section, map[max_action - 1], map[max_action], + (expected_result == true + ? "invalid key combo collision detected" + : "key combo collision not detected")); } - key_binding_list_free(bindings); - bindings->arr = NULL; - bindings->count = 0; + if (expected_result == false) { + if (bindings.count != 1) + BUG("[%s]: colliding binding not removed", ctx->section); + + if (bindings.arr[0].action != + (test_mode == FAIL_DIFFERENT_ACTION ? max_action - 1 : max_action)) + { + BUG("[%s]: wrong binding removed", ctx->section); + } + } + + free_key_binding_list(&bindings); } static void -test_mouse_binding(struct context *ctx, bool (*parse_fun)(struct context *ctx), - int action, int max_action, const char *const *map, - struct config_mouse_binding_list *bindings) +test_binding_collisions(struct context *ctx, + int max_action, const char *const *map, + enum config_key_binding_type type) { - xassert(map[action] != NULL); - xassert(bindings->count == 0); - - const char *key = map[action]; - - /* “Randomize” which modifiers to enable */ - const bool ctrl = action % 2; - const bool alt = action % 3; - const bool shift = action % 4; - const bool super = action % 5; - - /* Generate the modifier part of the ‘value’ */ - char modifier_string[32]; - int chars = sprintf(modifier_string, "%s%s%s%s", - ctrl ? XKB_MOD_NAME_CTRL "+" : "", - alt ? XKB_MOD_NAME_ALT "+" : "", - shift ? XKB_MOD_NAME_SHIFT "+" : "", - super ? XKB_MOD_NAME_LOGO "+" : ""); - - const int button_idx = action % ALEN(button_map); - const int button = button_map[button_idx].code; - const char *const button_name = button_map[button_idx].name; - const int click_count = action % 3 + 1; - - xassert(click_count > 0); - - /* Finally, generate the ‘value’ (e.g. “Control+shift+x”) */ - char value[chars + strlen(button_name) + 2 + 1]; - chars = sprintf(value, "%s%s", modifier_string, button_name); - if (click_count > 1) - sprintf(&value[chars], "-%d", click_count); - - ctx->key = key; - ctx->value = value; - - if (!parse_fun(ctx)) { - BUG("[%s].%s=%s failed to parse", - ctx->section, ctx->key, ctx->value); - } - - const struct config_mouse_binding *binding = - &bindings->arr[bindings->count - 1]; - - xassert(binding->pipe.argv.args == NULL); - - if (binding->action != action) { - BUG("[%s].%s=%s: action mismatch: %d != %d", - ctx->section, ctx->key, ctx->value, binding->action, action); - } - - if (binding->button != button) { - BUG("[%s].%s=%s: button mismatch: %d != %d", - ctx->section, ctx->key, ctx->value, binding->button, button); - } - - if (binding->count != click_count) { - BUG("[%s].%s=%s: button click count mismatch: %d != %d", - ctx->section, ctx->key, ctx->value, binding->count, click_count); - } - - if (binding->modifiers.ctrl != ctrl || - binding->modifiers.alt != alt || - binding->modifiers.shift != shift || - binding->modifiers.meta != super) - { - BUG("[%s].%s=%s: modifier mismatch:\n" - " have: ctrl=%d, alt=%d, shift=%d, super=%d\n" - " expected: ctrl=%d, alt=%d, shift=%d, super=%d", - ctx->section, ctx->key, ctx->value, - binding->modifiers.ctrl, binding->modifiers.alt, - binding->modifiers.shift, binding->modifiers.meta, - ctrl, alt, shift, super); - } - - mouse_binding_list_free(bindings); - bindings->arr = NULL; - bindings->count = 0; - - if (action >= max_action) - return; - - /* - * Test collisions - */ - - /* First, verify we get a collision when trying to assign the same - * key combo to multiple actions */ - bindings->count = 1; - bindings->arr = xmalloc(sizeof(bindings->arr[0])); - bindings->arr[0] = (struct config_mouse_binding){ - .action = action + 1, - .button = BTN_LEFT, - .count = 1, - .modifiers = { - .ctrl = true, - }, - }; - - char collision[128]; - snprintf(collision, sizeof(collision), "%s+BTN_LEFT", XKB_MOD_NAME_CTRL); - - ctx->value = collision; - if (parse_fun(ctx)) { - BUG("[%s].%s=%s: mouse combo collision not detected", - ctx->section, ctx->key, ctx->value); - } - - /* Next, verify we get a collision when trying to assign the same - * key combo to the same action, but with different pipe argvs */ - bindings->arr[0].action = action; - bindings->arr[0].pipe.master_copy = true; - bindings->arr[0].pipe.argv.args = xmalloc(4 * sizeof(bindings->arr[0].pipe.argv.args[0])); - bindings->arr[0].pipe.argv.args[0] = xstrdup("/usr/bin/foobar"); - bindings->arr[0].pipe.argv.args[1] = xstrdup("hello"); - bindings->arr[0].pipe.argv.args[2] = xstrdup("world"); - bindings->arr[0].pipe.argv.args[3] = NULL; - - snprintf(collision, sizeof(collision), - "[/usr/bin/foobar hello] %s+BTN_LEFT", XKB_MOD_NAME_CTRL); - - ctx->value = collision; - if (parse_fun(ctx)) { - BUG("[%s].%s=%s: key combo collision not detected", - ctx->section, ctx->key, ctx->value); - } - -#if 0 /* BUG! (should be fixed by https://codeberg.org/dnkl/foot/pulls/832) */ - /* Finally, verify we do *not* get a collision when assigning the - * same key combo to the same action, with matching argvs */ - snprintf(collision, sizeof(collision), - "[/usr/bin/foobar hello world] %s+BTN_LEFT", XKB_MOD_NAME_CTRL); - - ctx->value = collision; - if (!parse_fun(ctx)) { - BUG("[%s].%s=%s: invalid mouse combo collision", - ctx->section, ctx->key, ctx->value); - } -#endif - mouse_binding_list_free(bindings); - bindings->arr = NULL; - bindings->count = 0; + _test_binding_collisions(ctx, max_action, map, type, FAIL_DIFFERENT_ACTION); + _test_binding_collisions(ctx, max_action, map, type, FAIL_DIFFERENT_ARGV); + _test_binding_collisions(ctx, max_action, map, type, SUCCED_SAME_ACTION_AND_ARGV); } static void @@ -567,13 +502,25 @@ test_section_key_bindings(void) test_key_binding( &ctx, &parse_section_key_bindings, action, BIND_ACTION_KEY_COUNT - 1, - binding_action_map, &conf.bindings.key); + binding_action_map, &conf.bindings.key, KEY_BINDING); } config_free(conf); } -#if 0 +static void +test_section_key_bindings_collisions(void) +{ + struct config conf = {0}; + struct context ctx = { + .conf = &conf, .section = "key-bindings", .path = "unittest"}; + + test_binding_collisions( + &ctx, BIND_ACTION_KEY_COUNT - 1, binding_action_map, KEY_BINDING); + + config_free(conf); +} + static void test_section_search_bindings(void) { @@ -590,12 +537,26 @@ test_section_search_bindings(void) test_key_binding( &ctx, &parse_section_search_bindings, action, BIND_ACTION_SEARCH_COUNT - 1, - search_binding_action_map, &conf.bindings.search); + search_binding_action_map, &conf.bindings.search, KEY_BINDING); } config_free(conf); } +static void +test_section_search_bindings_collisions(void) +{ + struct config conf = {0}; + struct context ctx = { + .conf = &conf, .section = "search-bindings", .path = "unittest"}; + + test_binding_collisions( + &ctx, + BIND_ACTION_SEARCH_COUNT - 1, search_binding_action_map, KEY_BINDING); + + config_free(conf); +} + static void test_section_url_bindings(void) { @@ -612,12 +573,25 @@ test_section_url_bindings(void) test_key_binding( &ctx, &parse_section_url_bindings, action, BIND_ACTION_URL_COUNT - 1, - url_binding_action_map, &conf.bindings.url); + url_binding_action_map, &conf.bindings.url, KEY_BINDING); } config_free(conf); } -#endif + +static void +test_section_url_bindings_collisions(void) +{ + struct config conf = {0}; + struct context ctx = { + .conf = &conf, .section = "url-bindings", .path = "unittest"}; + + test_binding_collisions( + &ctx, + BIND_ACTION_URL_COUNT - 1, url_binding_action_map, KEY_BINDING); + + config_free(conf); +} static void test_section_mouse_bindings(void) @@ -632,26 +606,42 @@ test_section_mouse_bindings(void) if (binding_action_map[action] == NULL) continue; - test_mouse_binding( + test_key_binding( &ctx, &parse_section_mouse_bindings, action, BIND_ACTION_COUNT - 1, - binding_action_map, &conf.bindings.mouse); + binding_action_map, &conf.bindings.mouse, MOUSE_BINDING); } config_free(conf); } +static void +test_section_mouse_bindings_collisions(void) +{ + struct config conf = {0}; + struct context ctx = { + .conf = &conf, .section = "mouse-bindings", .path = "unittest"}; + + test_binding_collisions( + &ctx, + BIND_ACTION_COUNT - 1, binding_action_map, MOUSE_BINDING); + + config_free(conf); +} + int main(int argc, const char *const *argv) { log_init(LOG_COLORIZE_AUTO, false, 0, LOG_CLASS_ERROR); test_section_main(); test_section_key_bindings(); -#if 0 + test_section_key_bindings_collisions(); test_section_search_bindings(); + test_section_search_bindings_collisions(); test_section_url_bindings(); -#endif + test_section_url_bindings_collisions(); test_section_mouse_bindings(); + test_section_mouse_bindings_collisions(); log_deinit(); return 0; } From f077a2e77a95c7c3226db1728b0e432dfb4ad9d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sat, 11 Dec 2021 20:44:01 +0100 Subject: [PATCH 13/14] test: config: verify collisions with the mouse override modifiers are detected --- tests/test-config.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/tests/test-config.c b/tests/test-config.c index 94e211f0..1e99b3c4 100644 --- a/tests/test-config.c +++ b/tests/test-config.c @@ -380,7 +380,8 @@ test_key_binding(struct context *ctx, bool (*parse_fun)(struct context *ctx), enum collision_test_mode { FAIL_DIFFERENT_ACTION, FAIL_DIFFERENT_ARGV, - SUCCED_SAME_ACTION_AND_ARGV + FAIL_MOUSE_OVERRIDE, + SUCCED_SAME_ACTION_AND_ARGV, }; static void @@ -429,6 +430,10 @@ _test_binding_collisions(struct context *ctx, case FAIL_DIFFERENT_ACTION: break; + case FAIL_MOUSE_OVERRIDE: + ctx->conf->mouse.selection_override_modifiers.ctrl = true; + break; + case FAIL_DIFFERENT_ARGV: case SUCCED_SAME_ACTION_AND_ARGV: bindings.arr[0].pipe.master_copy = true; @@ -484,6 +489,11 @@ test_binding_collisions(struct context *ctx, _test_binding_collisions(ctx, max_action, map, type, FAIL_DIFFERENT_ACTION); _test_binding_collisions(ctx, max_action, map, type, FAIL_DIFFERENT_ARGV); _test_binding_collisions(ctx, max_action, map, type, SUCCED_SAME_ACTION_AND_ARGV); + + if (type == MOUSE_BINDING) { + _test_binding_collisions( + ctx, max_action, map, type, FAIL_MOUSE_OVERRIDE); + } } static void From 6911a50df7cf18fd49969fde11525a702111a545 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sat, 11 Dec 2021 20:59:08 +0100 Subject: [PATCH 14/14] config: NOINLINE modifiers_to_str() --- config.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config.c b/config.c index b0fffaf7..50fb74e6 100644 --- a/config.c +++ b/config.c @@ -1767,7 +1767,7 @@ modifiers_disjoint(const struct config_key_modifiers *mods1, return !(shift || alt || ctrl || super); } -static char * +static char * NOINLINE modifiers_to_str(const struct config_key_modifiers *mods) { char *ret = xasprintf(