From decc655d48fec56c09c3b0b904d821a4ff765de8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Thu, 4 Mar 2021 09:42:36 +0100 Subject: [PATCH 01/10] =?UTF-8?q?input:=20ask=20XKB=20for=20consumed=20mod?= =?UTF-8?q?ifiers,=20but=20don=E2=80=99t=20actually=20use=20them=20just=20?= =?UTF-8?q?yet?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- input.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/input.c b/input.c index d8a9333b..907a99bb 100644 --- a/input.c +++ b/input.c @@ -822,20 +822,22 @@ key_press_release(struct seat *seat, struct terminal *term, uint32_t serial, xkb_mod_mask_t mods = xkb_state_serialize_mods( seat->kbd.xkb_state, XKB_STATE_MODS_DEPRESSED); - //xkb_mod_mask_t consumed = xkb_state_key_get_consumed_mods(seat->kbd.xkb_state, key); - xkb_mod_mask_t consumed = 0x0; + xkb_mod_mask_t consumed = xkb_state_key_get_consumed_mods( + seat->kbd.xkb_state, key); + xkb_mod_mask_t significant = ctrl | alt | shift | meta; - xkb_mod_mask_t effective_mods = mods & ~consumed & significant; + mods &= significant; + consumed &= significant; if (term->is_searching) { if (should_repeat) start_repeater(seat, key); - search_input(seat, term, key, sym, effective_mods, serial); + search_input(seat, term, key, sym, mods, serial); return; } else if (urls_mode_is_active(term)) { if (should_repeat) start_repeater(seat, key); - urls_input(seat, term, key, sym, effective_mods, serial); + urls_input(seat, term, key, sym, mods, serial); return; } @@ -853,16 +855,15 @@ key_press_release(struct seat *seat, struct terminal *term, uint32_t serial, #endif LOG_DBG("%s (%u/0x%x): seat=%s, term=%p, serial=%u, " - "mod=0x%08x, consumed=0x%08x, significant=0x%08x, " - "effective=0x%08x, repeats=%d", + "mods=0x%08x, consumed=0x%08x, repeats=%d", sym_name, sym, sym, seat->name, (void *)term, serial, - mods, consumed, significant, effective_mods, should_repeat); + mods, consumed, should_repeat); /* * User configurable bindings */ tll_foreach(seat->kbd.bindings.key, it) { - if (it->item.mods != effective_mods) + if (it->item.mods != mods) continue; /* Match symbol */ @@ -972,7 +973,7 @@ key_press_release(struct seat *seat, struct terminal *term, uint32_t serial, } else { - if (effective_mods & alt) { + if (mods & alt) { /* * When the alt modifier is pressed, we do one out of three things: * From 5e64e06a550c2499a23f03749986791136cbbcb9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sat, 27 Feb 2021 20:42:31 +0100 Subject: [PATCH 02/10] =?UTF-8?q?input:=20rewrite=20of=20how=20we=20match?= =?UTF-8?q?=20foot=E2=80=99s=20own=20key=20bindings?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bindings are matched in one out of three ways: * By translated (by XKB) symbols * By untranslated symbols * By raw key codes A translated symbol is affected by pressed modifiers, some of which can be “consumed”. Consumed modifiers to not partake in the comparison with the binding’s modifiers. In this mode, ctrl+shift+2 maps to ctrl+@ on a US layout. Untranslated symbols, or un-shifted symbols refer to the “base” symbol of the pressed key, i.e. it’s unaffected by modifiers. In this mode, consumed modifiers *do* partake in the comparison with the binding’s modifiers, and ctrl+shift+2 maps to ctrl+shift+2 on a US layout. More examples: ctrl+shift+u maps to ctrl+U in the translated lookup, while ctrl+shift+u maps to ctrl+shift+u in the untranslated lookup. Finally, we also match raw key codes. This allows our bindings to work using the same physical keys when the user switches between latin and non-latin layouts. This means key bindings in foot.ini *must* not include both +shift+ and a *shifted* key. I.e. ctrl+shift+U is not a valid combo as it cannot be triggered. Unfortunately, this was how you were supposed to write bindings up until now... so, we try to detect such bindings, log a deprecation warning and then “fix” the binding for the user. When specifying bindings in foot.ini, both ctrl+U and ctrl+shift+u are valid, and will work. The latter is preferred though, since we cannot detect the raw key code for the former variant. Personally, I also prefer the latter one because it is more explicit; it’s more obvious which keys are involved. However, in some cases it makes more sense to use the other variant. Typically for non-letter combos. --- config.c | 35 ++++++++++++++++++++++++++--------- input.c | 50 ++++++++++++++++++++++++++++++++++++-------------- search.c | 34 ++++++++++++++++++++++++++-------- search.h | 7 +++++-- url-mode.c | 29 +++++++++++++++++++++-------- url-mode.h | 4 +++- 6 files changed, 117 insertions(+), 42 deletions(-) diff --git a/config.c b/config.c index 13673123..b1653916 100644 --- a/config.c +++ b/config.c @@ -1168,6 +1168,7 @@ out: static bool parse_key_combos(struct config *conf, const char *combos, key_combo_list_t *key_combos, + const char *section, const char *option, const char *path, unsigned lineno) { xassert(tll_length(*key_combos) == 0); @@ -1179,7 +1180,7 @@ parse_key_combos(struct config *conf, const char *combos, key_combo_list_t *key_ combo = strtok_r(NULL, " ", &tok_ctx)) { struct config_key_modifiers modifiers = {0}; - const char *key = strrchr(combo, '+'); + char *key = strrchr(combo, '+'); if (key == NULL) { /* No modifiers */ @@ -1190,11 +1191,26 @@ parse_key_combos(struct config *conf, const char *combos, key_combo_list_t *key_ key++; /* Skip past the '+' */ } + if (modifiers.shift && strlen(key) == 1 && (*key >= 'A' && *key <= 'Z')) { + LOG_WARN( + "%s:%d: [%s]: %s: %s: " + "upper case keys not supported with explicit 'Shift' modifier", + path, lineno, section, option, combo); + user_notification_add( + &conf->notifications, USER_NOTIFICATION_DEPRECATED, + "%s:%d: [%s]: %s: \033[1m%s\033[m: " + "shifted keys not supported with explicit \033[1mShift\033[m " + "modifier", + path, lineno, section, option, combo); + *key = tolower(*key); + } + /* Translate key name to symbol */ xkb_keysym_t sym = xkb_keysym_from_name(key, 0); if (sym == XKB_KEY_NoSymbol) { - LOG_AND_NOTIFY_ERR("%s:%d: %s: key is not a valid XKB key name", - path, lineno, key); + LOG_AND_NOTIFY_ERR( + "%s:%d: [%s]: %s: ]%s: key is not a valid XKB key name", + path, lineno, section, option, key); goto err; } @@ -1373,7 +1389,8 @@ parse_key_binding_section( } key_combo_list_t key_combos = tll_init(); - if (!parse_key_combos(conf, value, &key_combos, path, lineno) || + if (!parse_key_combos( + conf, value, &key_combos, section, key, path, lineno) || has_key_binding_collisions( conf, action, binding_action_map, bindings, &key_combos, path, lineno)) @@ -2040,10 +2057,10 @@ add_default_key_bindings(struct config *conf) add_binding(BIND_ACTION_SCROLLBACK_UP_PAGE, shift, XKB_KEY_Page_Up); add_binding(BIND_ACTION_SCROLLBACK_DOWN_PAGE, shift, XKB_KEY_Page_Down); - add_binding(BIND_ACTION_CLIPBOARD_COPY, ctrl_shift, XKB_KEY_C); - add_binding(BIND_ACTION_CLIPBOARD_PASTE, ctrl_shift, XKB_KEY_V); + add_binding(BIND_ACTION_CLIPBOARD_COPY, ctrl_shift, XKB_KEY_c); + add_binding(BIND_ACTION_CLIPBOARD_PASTE, ctrl_shift, XKB_KEY_v); add_binding(BIND_ACTION_PRIMARY_PASTE, shift, XKB_KEY_Insert); - add_binding(BIND_ACTION_SEARCH_START, ctrl_shift, XKB_KEY_R); + add_binding(BIND_ACTION_SEARCH_START, ctrl_shift, XKB_KEY_r); add_binding(BIND_ACTION_FONT_SIZE_UP, ctrl, XKB_KEY_plus); add_binding(BIND_ACTION_FONT_SIZE_UP, ctrl, XKB_KEY_equal); add_binding(BIND_ACTION_FONT_SIZE_UP, ctrl, XKB_KEY_KP_Add); @@ -2051,8 +2068,8 @@ add_default_key_bindings(struct config *conf) add_binding(BIND_ACTION_FONT_SIZE_DOWN, ctrl, XKB_KEY_KP_Subtract); add_binding(BIND_ACTION_FONT_SIZE_RESET, ctrl, XKB_KEY_0); add_binding(BIND_ACTION_FONT_SIZE_RESET, ctrl, XKB_KEY_KP_0); - add_binding(BIND_ACTION_SPAWN_TERMINAL, ctrl_shift, XKB_KEY_N); - add_binding(BIND_ACTION_SHOW_URLS_LAUNCH, ctrl_shift, XKB_KEY_U); + add_binding(BIND_ACTION_SPAWN_TERMINAL, ctrl_shift, XKB_KEY_n); + add_binding(BIND_ACTION_SHOW_URLS_LAUNCH, ctrl_shift, XKB_KEY_u); #undef add_binding } diff --git a/input.c b/input.c index 907a99bb..db84c8e4 100644 --- a/input.c +++ b/input.c @@ -350,20 +350,18 @@ key_codes_for_xkb_sym(struct xkb_keymap *keymap, xkb_keysym_t sym) xkb_keycode_list_t key_codes = tll_init(); /* - * Find all key codes that map to the lower case - * version of the symbol. + * Find all key codes that map to this symbol. * * This allows us to match bindings in other layouts * too. */ - xkb_keysym_t lower_sym = xkb_keysym_to_lower(sym); struct xkb_state *state = xkb_state_new(keymap); for (xkb_keycode_t code = xkb_keymap_min_keycode(keymap); code <= xkb_keymap_max_keycode(keymap); code++) { - if (xkb_state_key_get_one_sym(state, code) == lower_sym) + if (xkb_state_key_get_one_sym(state, code) == sym) tll_push_back(key_codes, code); } @@ -829,15 +827,24 @@ key_press_release(struct seat *seat, struct terminal *term, uint32_t serial, mods &= significant; consumed &= significant; + xkb_layout_index_t layout_idx = + xkb_state_key_get_layout(seat->kbd.xkb_state, key); + + const xkb_keysym_t *raw_syms = NULL; + size_t raw_count = xkb_keymap_key_get_syms_by_level( + seat->kbd.xkb_keymap, key, layout_idx, 0, &raw_syms); + if (term->is_searching) { if (should_repeat) start_repeater(seat, key); - search_input(seat, term, key, sym, mods, serial); + search_input( + seat, term, key, sym, mods, consumed, raw_syms, raw_count, serial); return; } else if (urls_mode_is_active(term)) { if (should_repeat) start_repeater(seat, key); - urls_input(seat, term, key, sym, mods, serial); + urls_input( + seat, term, key, sym, mods, consumed, raw_syms, raw_count, serial); return; } @@ -863,20 +870,35 @@ key_press_release(struct seat *seat, struct terminal *term, uint32_t serial, * User configurable bindings */ tll_foreach(seat->kbd.bindings.key, it) { - if (it->item.mods != mods) + const struct key_binding *bind = &it->item; + + /* Match translated symbol */ + if (bind->sym == sym && + bind->mods == (mods & ~consumed) && + execute_binding( + seat, term, bind->action, bind->pipe_argv, serial)) + { + goto maybe_repeat; + } + + if (bind->mods != mods) continue; - /* Match symbol */ - if (it->item.sym == sym) { - if (execute_binding(seat, term, it->item.action, it->item.pipe_argv, serial)) + /* Match untranslated symbols */ + for (size_t i = 0; i < raw_count; i++) { + if (bind->sym == raw_syms[i] && execute_binding( + seat, term, bind->action, bind->pipe_argv, serial)) + { goto maybe_repeat; + } } /* Match raw key code */ - tll_foreach(it->item.key_codes, code) { - if (code->item == key) { - if (execute_binding(seat, term, it->item.action, it->item.pipe_argv, serial)) - goto maybe_repeat; + tll_foreach(bind->key_codes, code) { + if (code->item == key && execute_binding( + seat, term, bind->action, bind->pipe_argv, serial)) + { + goto maybe_repeat; } } } diff --git a/search.c b/search.c index 5dfa7b58..30a50ae5 100644 --- a/search.c +++ b/search.c @@ -783,7 +783,9 @@ execute_binding(struct seat *seat, struct terminal *term, void search_input(struct seat *seat, struct terminal *term, uint32_t key, - xkb_keysym_t sym, xkb_mod_mask_t mods, uint32_t serial) + xkb_keysym_t sym, xkb_mod_mask_t mods, xkb_mod_mask_t consumed, + const xkb_keysym_t *raw_syms, size_t raw_count, + uint32_t serial) { LOG_DBG("search: input: sym=%d/0x%x, mods=0x%08x", sym, sym, mods); @@ -795,12 +797,13 @@ search_input(struct seat *seat, struct terminal *term, uint32_t key, /* Key bindings */ tll_foreach(seat->kbd.bindings.search, it) { - if (it->item.mods != mods) - continue; + const struct key_binding *bind = &it->item; - /* Match symbol */ - if (it->item.sym == sym) { - if (execute_binding(seat, term, it->item.action, serial, + /* Match translated symbol */ + if (bind->sym == sym && + bind->mods == (mods & ~consumed)) { + + if (execute_binding(seat, term, bind->action, serial, &update_search_result, &redraw)) { goto update_search; @@ -808,10 +811,25 @@ search_input(struct seat *seat, struct terminal *term, uint32_t key, return; } + if (bind->mods != mods) + continue; + + /* Match untranslated symbols */ + for (size_t i = 0; i < raw_count; i++) { + if (bind->sym == raw_syms[i]) { + if (execute_binding(seat, term, bind->action, serial, + &update_search_result, &redraw)) + { + goto update_search; + } + return; + } + } + /* Match raw key code */ - tll_foreach(it->item.key_codes, code) { + tll_foreach(bind->key_codes, code) { if (code->item == key) { - if (execute_binding(seat, term, it->item.action, serial, + if (execute_binding(seat, term, bind->action, serial, &update_search_result, &redraw)) { goto update_search; diff --git a/search.h b/search.h index ece0d68a..5db1610d 100644 --- a/search.h +++ b/search.h @@ -5,6 +5,9 @@ void search_begin(struct terminal *term); void search_cancel(struct terminal *term); -void search_input(struct seat *seat, struct terminal *term, uint32_t key, - xkb_keysym_t sym, xkb_mod_mask_t mods, uint32_t serial); +void search_input( + struct seat *seat, struct terminal *term, uint32_t key, + xkb_keysym_t sym, xkb_mod_mask_t mods, xkb_mod_mask_t consumed, + const xkb_keysym_t *raw_syms, size_t raw_count, + uint32_t serial); void search_add_chars(struct terminal *term, const char *text, size_t len); diff --git a/url-mode.c b/url-mode.c index 791b4ce0..34866fb8 100644 --- a/url-mode.c +++ b/url-mode.c @@ -115,23 +115,36 @@ activate_url(struct seat *seat, struct terminal *term, const struct url *url) void urls_input(struct seat *seat, struct terminal *term, uint32_t key, - xkb_keysym_t sym, xkb_mod_mask_t mods, uint32_t serial) + xkb_keysym_t sym, xkb_mod_mask_t mods, xkb_mod_mask_t consumed, + const xkb_keysym_t *raw_syms, size_t raw_count, + uint32_t serial) { /* Key bindings */ tll_foreach(seat->kbd.bindings.url, it) { - if (it->item.mods != mods) - continue; + const struct key_binding *bind = &it->item; - /* Match symbol */ - if (it->item.sym == sym) { - execute_binding(seat, term, it->item.action, serial); + /* Match translated symbol */ + if (bind->sym == sym && + bind->mods == (mods & ~consumed)) + { + execute_binding(seat, term, bind->action, serial); return; } + if (bind->mods != mods) + continue; + + for (size_t i = 0; i < raw_count; i++) { + if (bind->sym == raw_syms[i]) { + execute_binding(seat, term, bind->action, serial); + return; + } + } + /* Match raw key code */ - tll_foreach(it->item.key_codes, code) { + tll_foreach(bind->key_codes, code) { if (code->item == key) { - execute_binding(seat, term, it->item.action, serial); + execute_binding(seat, term, bind->action, serial); return; } } diff --git a/url-mode.h b/url-mode.h index 28abde18..b1314780 100644 --- a/url-mode.h +++ b/url-mode.h @@ -20,4 +20,6 @@ void urls_render(struct terminal *term); void urls_reset(struct terminal *term); void urls_input(struct seat *seat, struct terminal *term, uint32_t key, - xkb_keysym_t sym, xkb_mod_mask_t mods, uint32_t serial); + xkb_keysym_t sym, xkb_mod_mask_t mods, xkb_mod_mask_t consumed, + const xkb_keysym_t *raw_syms, size_t raw_count, + uint32_t serial); From 47fe27ca5d22a7cb203f5428cc6232434f18a9c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sat, 27 Feb 2021 20:55:28 +0100 Subject: [PATCH 03/10] doc: foot.ini: key combos must *not* include shift and be in upper case --- doc/foot.ini.5.scd | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/foot.ini.5.scd b/doc/foot.ini.5.scd index 586bdd8b..014f1981 100644 --- a/doc/foot.ini.5.scd +++ b/doc/foot.ini.5.scd @@ -437,9 +437,9 @@ may have one or more key combinations, space separated. Each combination is on the form _mod1+mod2+key_. The names of the modifiers and the key *must* be valid XKB key names. -Note that if *Shift* is one of the modifiers, the _key_ *must* be in -upper case. For example, *Control+Shift+v* will never trigger, but -*Control+Shift+V* will. +Note that if *Shift* is one of the modifiers, the _key_ *must not* be +in upper case. For example, *Control+Shift+V* will never trigger, but +*Control+Shift+v* will. Note that *Alt* is usually called *Mod1*. From cfa82bc920bcc8990e4b2af5f3d2d8e4565e37f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sat, 27 Feb 2021 21:33:52 +0100 Subject: [PATCH 04/10] config: fix default key binding for extend-to-next-whitespace Bindings now use lower-cased keys --- config.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config.c b/config.c index b1653916..633d1ad9 100644 --- a/config.c +++ b/config.c @@ -2114,7 +2114,7 @@ add_default_search_bindings(struct config *conf) add_binding(BIND_ACTION_SEARCH_DELETE_NEXT_WORD, ctrl, XKB_KEY_Delete); add_binding(BIND_ACTION_SEARCH_DELETE_NEXT_WORD, alt, XKB_KEY_d); add_binding(BIND_ACTION_SEARCH_EXTEND_WORD, ctrl, XKB_KEY_w); - add_binding(BIND_ACTION_SEARCH_EXTEND_WORD_WS, ctrl_shift, XKB_KEY_W); + add_binding(BIND_ACTION_SEARCH_EXTEND_WORD_WS, ctrl_shift, XKB_KEY_w); add_binding(BIND_ACTION_SEARCH_CLIPBOARD_PASTE, ctrl, XKB_KEY_v); add_binding(BIND_ACTION_SEARCH_CLIPBOARD_PASTE, ctrl, XKB_KEY_y); add_binding(BIND_ACTION_SEARCH_PRIMARY_PASTE, shift, XKB_KEY_Insert); From 3a4f6c469bf0de5d064457a5b217d70d0937252a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sat, 27 Feb 2021 21:34:37 +0100 Subject: [PATCH 05/10] doc: foot.ini: update default key bindings - use lower case keys --- doc/foot.ini.5.scd | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/doc/foot.ini.5.scd b/doc/foot.ini.5.scd index 014f1981..ac815b23 100644 --- a/doc/foot.ini.5.scd +++ b/doc/foot.ini.5.scd @@ -469,17 +469,17 @@ e.g. *search-start=none*. Scroll down/forward a single line in history. Default: _not set_. *clipboard-copy* - Copies the current selection into the _clipboard_. Default: _Control+Shift+C_. + Copies the current selection into the _clipboard_. Default: _Control+Shift+c_. *clipboard-paste* - Pastes from the _clipboard_. Default: _Control+Shift+V_. + Pastes from the _clipboard_. Default: _Control+Shift+v_. *primary-paste* Pastes from the _primary selection_. Default: _Shift+Insert_ (also defined in *mouse-bindings*). *search-start* - Starts a scrollback/history search. Default: _Control+Shift+R_. + Starts a scrollback/history search. Default: _Control+Shift+r_. *font-increase* Increases the font size by 0.5pt. Default: _Control+plus @@ -495,7 +495,7 @@ e.g. *search-start=none*. *spawn-terminal* Spawns a new terminal. If the shell has been configured to emit the OSC 7 escape sequence, the new terminal will start in the - current working directory. Default: _Control+Shift+N_. + current working directory. Default: _Control+Shift+n_. *minimize* Minimizes the window. Default: _not bound_. @@ -527,7 +527,7 @@ e.g. *search-start=none*. *show-urls-launch* Enter URL mode, where all currently visible URLs are tagged with a jump label with a key sequence that will open the URL. Default: - _Control+Shift+U_. + _Control+Shift+u_. *show-urls-copy* Enter URL mode, where all currently visible URLs are tagged with a @@ -548,7 +548,7 @@ scrollback search mode. The syntax is exactly the same as the regular *commit* Exit search mode and copy current selection into the _primary selection_. Viewport is **not** restored. To copy the selection to - the regular _clipboard_, use *Control+Shift+C*. Default: _Return_. + the regular _clipboard_, use *Control+Shift+c*. Default: _Return_. *find-prev* Search **backwards** in the scrollback history for the next @@ -602,7 +602,7 @@ scrollback search mode. The syntax is exactly the same as the regular *extend-to-next-whitespace* Extend the current selection to the next whitespace. Default: - _Control+Shift+W_. + _Control+Shift+w_. *clipboard-paste* Paste from the _clipboard_ into the search buffer. Default: From 1b4d9eade004c9050a918563e00d56de965d97e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sun, 28 Feb 2021 11:38:33 +0100 Subject: [PATCH 06/10] config: parse_key_combos: lower case manually, instead of calling tolower() tolower() is locale dependent, something we do not want in this case. --- config.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config.c b/config.c index 633d1ad9..927d9450 100644 --- a/config.c +++ b/config.c @@ -1202,7 +1202,7 @@ parse_key_combos(struct config *conf, const char *combos, key_combo_list_t *key_ "shifted keys not supported with explicit \033[1mShift\033[m " "modifier", path, lineno, section, option, combo); - *key = tolower(*key); + *key = *key - 'A' + 'a'; } /* Translate key name to symbol */ From a5b554761a8b376657be2a7e613b37fd5803920c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Tue, 2 Mar 2021 17:50:06 +0100 Subject: [PATCH 07/10] input: repair key combos containing both explicit modifier and shifted symbol MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If a user-provided key combo contains both an explicit modifier, and the shifted symbol it produces, replace the symbol with its un-shifted version. Example: Control+Shift+U contains both ‘Shift’, and ‘U’, where ‘Shift’ is the modifier that shifts ‘u’ to ‘U’. Such modifiers are “consumed”, i.e. filtered out, when matching key combos. As such, Control+Shift+U will never trigger since we’ll never be able to match the consumed modifier ‘Shift’. The above combo can be written in two ways: - Control+U - Control+Shift+u To be able to detect that Control+Shift+U is an invalid combination, we need to check all *shifted* symbols for all key *codes*. Once we’ve detected a shifted symbol (and where it differs from the un-shifted symbol), we loop all modifier sets that produce the shift level where our shifted symbol is. For each such set, check if it intersects with the user-provided key combo’s modifier set. If there is an intersection, it means the user provided a key combo containing a modifier and symbol, such that the modifier is consumed when the symbol is produced. In this case, we replace the symbol with its un-shifted version. --- config.c | 3 +- input.c | 128 ++++++++++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 125 insertions(+), 6 deletions(-) diff --git a/config.c b/config.c index 927d9450..9babe8eb 100644 --- a/config.c +++ b/config.c @@ -1191,6 +1191,7 @@ parse_key_combos(struct config *conf, const char *combos, key_combo_list_t *key_ key++; /* Skip past the '+' */ } +#if 0 if (modifiers.shift && strlen(key) == 1 && (*key >= 'A' && *key <= 'Z')) { LOG_WARN( "%s:%d: [%s]: %s: %s: " @@ -1204,7 +1205,7 @@ parse_key_combos(struct config *conf, const char *combos, key_combo_list_t *key_ path, lineno, section, option, combo); *key = *key - 'A' + 'a'; } - +#endif /* Translate key name to symbol */ xkb_keysym_t sym = xkb_keysym_from_name(key, 0); if (sym == XKB_KEY_NoSymbol) { diff --git a/input.c b/input.c index db84c8e4..98abac9c 100644 --- a/input.c +++ b/input.c @@ -369,16 +369,134 @@ key_codes_for_xkb_sym(struct xkb_keymap *keymap, xkb_keysym_t sym) return key_codes; } +static xkb_keysym_t +maybe_repair_key_combo(const struct seat *seat, + xkb_keysym_t sym, xkb_mod_mask_t mods) +{ + /* + * Detect combos containing a shifted symbol and the corresponding + * modifier, and replace the shifted symbol with its unshifted + * variant. + * + * For example, the combo is “Control+Shift+U”. In this case, + * Shift is the modifier used to “shift” ‘u’ to ‘U’, after which + * ‘Shift’ will have been “consumed”. Since we filter out consumed + * modifiers when matching key combos, this key combo will never + * trigger (we will never be able to match the ‘Shift’ modifier). + * + * There are two correct variants of the above key combo: + * - “Control+U” (upper case ‘U’) + * - “Control+Shift+u” (lower case ‘u’) + * + * What we do here is, for each key *code*, check if there are any + * (shifted) levels where it produces ‘sym’. If there are, check + * *which* sets of modifiers are needed to produce it, and compare + * with ‘mods’. + * + * If there is at least one common modifier, it means ‘sym’ is a + * “shifted” symbol, with the corresponding shifting modifier + * explicitly included in the key combo. I.e. the key combo will + * never trigger. + * + * We then proceed and “repair” the key combo by replacing ‘sym’ + * with the corresponding unshifted symbol. + * + * To reduce the noise, we ignore all key codes where the shifted + * symbol is the same as the unshifted symbol. + */ + + for (xkb_keycode_t code = xkb_keymap_min_keycode(seat->kbd.xkb_keymap); + code <= xkb_keymap_max_keycode(seat->kbd.xkb_keymap); + code++) + { + xkb_layout_index_t layout_idx = + xkb_state_key_get_layout(seat->kbd.xkb_state, code); + + /* Get all unshifted symbols for this key */ + const xkb_keysym_t *base_syms = NULL; + size_t base_count = xkb_keymap_key_get_syms_by_level( + seat->kbd.xkb_keymap, code, layout_idx, 0, &base_syms); + + if (base_count == 0 || sym == base_syms[0]) { + /* No unshifted symbols, or unshifted symbol is same as ‘sym’ */ + continue; + } + + /* Name of the unshifted symbol, for logging */ + char base_name[100]; + xkb_keysym_get_name(base_syms[0], base_name, sizeof(base_name)); + + /* Iterate all shift levels */ + for (xkb_level_index_t level_idx = 1; + level_idx < xkb_keymap_num_levels_for_key( + seat->kbd.xkb_keymap, code, layout_idx); + level_idx++) { + + /* Get all symbols for current shift level */ + const xkb_keysym_t *shifted_syms = NULL; + size_t shifted_count = xkb_keymap_key_get_syms_by_level( + seat->kbd.xkb_keymap, code, + layout_idx, level_idx, &shifted_syms); + + for (size_t i = 0; i < shifted_count; i++) { + if (shifted_syms[i] != sym) + continue; + + /* Get modifier sets that produces the current shift level */ + xkb_mod_mask_t mod_masks[16]; + size_t mod_mask_count = xkb_keymap_key_get_mods_for_level( + seat->kbd.xkb_keymap, code, layout_idx, level_idx, + mod_masks, ALEN(mod_masks)); + + /* Check if key combo’s modifier set intersects */ + for (size_t j = 0; j < mod_mask_count; j++) { + if (!(mod_masks[j] & mods)) + continue; + + char combo[64] = {0}; + + for (int k = 0; k < sizeof(xkb_mod_mask_t) * 8; k++) { + if (!(mods & (1u << k))) + continue; + + const char *mod_name = xkb_keymap_mod_get_name( + seat->kbd.xkb_keymap, k); + strcat(combo, mod_name); + strcat(combo, "+"); + } + + size_t len = strlen(combo); + xkb_keysym_get_name( + sym, &combo[len], sizeof(combo) - len); + + LOG_WARN( + "%s: combo with both explicit modifier and shifted symbol " + "(level=%d, mod-mask=0x%08x), " + "replacing with %s", + combo, level_idx, mod_masks[j], base_name); + + /* Replace with unshifted symbol */ + return base_syms[0]; + } + } + } + } + + return sym; +} + static void -convert_key_binding(struct seat *seat, +convert_key_binding(const struct seat *seat, const struct config_key_binding *conf_binding, 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); + struct key_binding binding = { - .mods = conf_modifiers_to_mask(seat, &conf_binding->modifiers), - .sym = conf_binding->sym, - .key_codes = key_codes_for_xkb_sym( - seat->kbd.xkb_keymap, conf_binding->sym), + .mods = mods, + .sym = sym, + .key_codes = key_codes_for_xkb_sym(seat->kbd.xkb_keymap, sym), .action = conf_binding->action, .pipe_argv = conf_binding->pipe.argv, }; From be00de4849c81dce67aac032f073a3d131b40307 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Thu, 4 Mar 2021 08:59:37 +0100 Subject: [PATCH 08/10] foot.ini: lower case Shift+X default bindings --- foot.ini | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/foot.ini b/foot.ini index fd42d908..0c2d3f5b 100644 --- a/foot.ini +++ b/foot.ini @@ -87,21 +87,21 @@ # scrollback-down-page=Shift+Page_Down # scrollback-down-half-page=none # scrollback-down-line=none -# clipboard-copy=Control+Shift+C -# clipboard-paste=Control+Shift+V +# clipboard-copy=Control+Shift+c +# clipboard-paste=Control+Shift+v # primary-paste=Shift+Insert -# search-start=Control+Shift+R +# search-start=Control+Shift+r # font-increase=Control+plus Control+equal Control+KP_Add # font-decrease=Control+minus Control+KP_Subtract # font-reset=Control+0 Control+KP_0 -# spawn-terminal=Control+Shift+N +# spawn-terminal=Control+Shift+n # minimize=none # maximize=none # fullscreen=none # pipe-visible=[sh -c "xurls | fuzzel | xargs -r firefox"] none # pipe-scrollback=[sh -c "xurls | fuzzel | xargs -r firefox"] none # pipe-selected=[xargs -r firefox] none -# show-urls-launch=Control+Shift+U +# show-urls-launch=Control+Shift+u # show-urls-copy=none [search-bindings] @@ -120,7 +120,7 @@ # delete-next=Delete # delete-next-word=Mod1+d Control+Delete # extend-to-word-boundary=Control+w -# extend-to-next-whitespace=Control+Shift+W +# extend-to-next-whitespace=Control+Shift+w # clipboard-paste=Control+v Control+y # primary-paste=Shift+Insert From 93b02cf2b85e080269d6822678df8bbbd94efd42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Thu, 4 Mar 2021 09:36:25 +0100 Subject: [PATCH 09/10] url-mode: ignore keys with modifiers --- url-mode.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/url-mode.c b/url-mode.c index 34866fb8..362ea3af 100644 --- a/url-mode.c +++ b/url-mode.c @@ -161,6 +161,9 @@ urls_input(struct seat *seat, struct terminal *term, uint32_t key, return; } + if (mods & ~consumed) + return; + wchar_t wc = xkb_state_key_get_utf32(seat->kbd.xkb_state, key); /* From b796965c3d34b4e93252e9633e140193dc43c4c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Thu, 4 Mar 2021 10:12:39 +0100 Subject: [PATCH 10/10] changelog: new key binding matching logic --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a4f4aebd..f4112765 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -74,6 +74,10 @@ timing can be tweaked, or completely disabled, by setting `resize-delay-ms` (https://codeberg.org/dnkl/foot/issues/301). * `CSI 13 ; 2 t` now reports (0,0). +* Key binding matching logic; key combinations like `Control+Shift+C` + **must** now be written as either `Control+C` or `Control+Shift+c`, + the latter being the preferred + variant. (https://codeberg.org/dnkl/foot/issues/376) ### Deprecated