From 6d9b165fb442003375c6de3a6c2d03b10d487ba9 Mon Sep 17 00:00:00 2001 From: Michael Stapelberg Date: Wed, 24 May 2017 20:40:17 +0200 Subject: [PATCH 1/4] =?UTF-8?q?no-op=20change:=20don=E2=80=99t=20compare?= =?UTF-8?q?=20keycode=20for=20every=20modifier?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/bindings.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/bindings.c b/src/bindings.c index a8b897af..0a994f55 100644 --- a/src/bindings.c +++ b/src/bindings.c @@ -261,7 +261,6 @@ static Binding *get_binding(i3_event_state_mask_t state_filtered, bool is_releas continue; } - xcb_keycode_t input_keycode = (xcb_keycode_t)input_code; bool found_keycode = false; struct Binding_Keycode *binding_keycode; TAILQ_FOREACH(binding_keycode, &(bind->keycodes_head), keycodes) { @@ -269,7 +268,7 @@ static Binding *get_binding(i3_event_state_mask_t state_filtered, bool is_releas const bool mods_match = modifiers_match(modifiers_mask, modifiers_state); DLOG("binding_keycode->modifiers = %d, modifiers_mask = %d, modifiers_state = %d, mods_match = %s\n", binding_keycode->modifiers, modifiers_mask, modifiers_state, (mods_match ? "yes" : "no")); - if (binding_keycode->keycode == input_keycode && mods_match) { + if (mods_match) { found_keycode = true; break; } From 0acd11a8d71a120edf3d2cfe327ec678bb1b63af Mon Sep 17 00:00:00 2001 From: Michael Stapelberg Date: Wed, 24 May 2017 20:41:17 +0200 Subject: [PATCH 2/4] no-op change: move bind->release check into loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We don’t have to negate the check anymore, making it more readable. --- src/bindings.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/bindings.c b/src/bindings.c index 0a994f55..e9aec2c6 100644 --- a/src/bindings.c +++ b/src/bindings.c @@ -268,14 +268,12 @@ static Binding *get_binding(i3_event_state_mask_t state_filtered, bool is_releas const bool mods_match = modifiers_match(modifiers_mask, modifiers_state); DLOG("binding_keycode->modifiers = %d, modifiers_mask = %d, modifiers_state = %d, mods_match = %s\n", binding_keycode->modifiers, modifiers_mask, modifiers_state, (mods_match ? "yes" : "no")); - if (mods_match) { + if (mods_match || (bind->release == B_UPON_KEYRELEASE_IGNORE_MODS && is_release)) { found_keycode = true; break; } } - if (!found_keycode && - (bind->release != B_UPON_KEYRELEASE_IGNORE_MODS || - !is_release)) { + if (!found_keycode) { continue; } } From 26f5edb97fb3dcc0659c3ae5fff5b816f80d57e5 Mon Sep 17 00:00:00 2001 From: Michael Stapelberg Date: Wed, 24 May 2017 20:41:55 +0200 Subject: [PATCH 3/4] no-op change: store |button| in the correct data type --- src/bindings.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/bindings.c b/src/bindings.c index e9aec2c6..7cba556e 100644 --- a/src/bindings.c +++ b/src/bindings.c @@ -483,8 +483,9 @@ void translate_keysyms(void) { ELOG("Could not translate string to button: \"%s\"\n", bind->symbol); } - bind->keycode = button; - ADD_TRANSLATED_KEY(button, bind->event_state_mask); + xcb_keycode_t key = button; + bind->keycode = key; + ADD_TRANSLATED_KEY(key, bind->event_state_mask); continue; } From a4f6387911b4b79ca864052012c5cbace584eb2e Mon Sep 17 00:00:00 2001 From: Michael Stapelberg Date: Wed, 24 May 2017 20:42:27 +0200 Subject: [PATCH 4/4] compare modifiers for equality, not subset (+test) Subset comparison was introduced with the rather large commit bf3cd41b5ddf1e757515ab5fbf811be56e5f69cc, but I now think we should use equality. In other words, the following key binding: bindsym Mod4+x nop Mod4+x previously would have been triggered when pressing Mod3+Mod4+x. Strictly speaking, this is a change of behavior, but it breaks none of our tests, and using equality instead of subset comparison enables more use-cases. fixes #2002 --- src/bindings.c | 15 ++------------ testcases/t/264-keypress-numlock.t | 32 ++++++++++++++++++++++++++++-- 2 files changed, 32 insertions(+), 15 deletions(-) diff --git a/src/bindings.c b/src/bindings.c index 7cba556e..14eaaf5e 100644 --- a/src/bindings.c +++ b/src/bindings.c @@ -191,17 +191,6 @@ void regrab_all_buttons(xcb_connection_t *conn) { xcb_ungrab_server(conn); } -static bool modifiers_match(const uint32_t modifiers_mask, const uint32_t modifiers_state) { - /* modifiers_mask is a special case: a value of 0 does not mean “match - * all”, but rather “match exactly when no modifiers are present”. */ - if (modifiers_mask == 0) { - /* Verify no modifiers are pressed. A bitwise AND would lead to - * false positives, see issue #2002. */ - return (modifiers_state == 0); - } - return ((modifiers_state & modifiers_mask) == modifiers_mask); -} - /* * Returns a pointer to the Binding with the specified modifiers and * keycode or NULL if no such binding exists. @@ -244,7 +233,7 @@ static Binding *get_binding(i3_event_state_mask_t state_filtered, bool is_releas struct Binding_Keycode *binding_keycode; TAILQ_FOREACH(binding_keycode, &(bind->keycodes_head), keycodes) { const uint32_t modifiers_mask = (binding_keycode->modifiers & 0x0000FFFF); - const bool mods_match = modifiers_match(modifiers_mask, modifiers_state); + const bool mods_match = (modifiers_mask == modifiers_state); DLOG("binding_keycode->modifiers = %d, modifiers_mask = %d, modifiers_state = %d, mods_match = %s\n", binding_keycode->modifiers, modifiers_mask, modifiers_state, (mods_match ? "yes" : "no")); if (binding_keycode->keycode == input_keycode && mods_match) { @@ -265,7 +254,7 @@ static Binding *get_binding(i3_event_state_mask_t state_filtered, bool is_releas struct Binding_Keycode *binding_keycode; TAILQ_FOREACH(binding_keycode, &(bind->keycodes_head), keycodes) { const uint32_t modifiers_mask = (binding_keycode->modifiers & 0x0000FFFF); - const bool mods_match = modifiers_match(modifiers_mask, modifiers_state); + const bool mods_match = (modifiers_mask == modifiers_state); DLOG("binding_keycode->modifiers = %d, modifiers_mask = %d, modifiers_state = %d, mods_match = %s\n", binding_keycode->modifiers, modifiers_mask, modifiers_state, (mods_match ? "yes" : "no")); if (mods_match || (bind->release == B_UPON_KEYRELEASE_IGNORE_MODS && is_release)) { diff --git a/testcases/t/264-keypress-numlock.t b/testcases/t/264-keypress-numlock.t index 90a403af..2fdafb83 100644 --- a/testcases/t/264-keypress-numlock.t +++ b/testcases/t/264-keypress-numlock.t @@ -214,6 +214,8 @@ $config = <