keyd icon indicating copy to clipboard operation
keyd copied to clipboard

overloaded keys do not use modifiers from pressed state

Open matthias-hmb opened this issue 2 years ago • 3 comments

Normally keys are send, when they are pressed: Shift + 'd' will result in 'D', no matter, if I release shift or 'd' first.

If I use some config setting like:

d = overload(control, d)

The key can only be send, when the overloaded key is released, so the result is different: Shift + 'd' ... release 'd' -> 'D' (, release 'shift') Shift + 'd' ... release shift, release 'd' -> 'd'

I'm training now to be more exact and acurate when releasing keys ... and I did a patch. I don't know, if it's interesting for you, but I wanted to share it. That way, it will save the layer states, at the moment, the overloaded key gets pressed. In case of sending, it will use that saved state and after sending restore the actual state.

Thanks for your wonderful program! Matthias

diff --git a/src/keyboard.c b/src/keyboard.c
index 4af48f5..905407c 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -590,10 +590,15 @@ static long process_descriptor(struct keyboard *kbd, uint8_t code,
                        clear(kbd);
                break;
        case OP_OVERLOAD:
+               size_t i;
+
                idx = d->args[0].idx;
                action = &kbd->config.descriptors[d->args[1].idx];
 
                if (pressed) {
+                       for (i = 0; i < kbd->config.nr_layers; i++) {
+                               kbd->layer_state[i].active_overload_pressed = kbd->layer_state[i].active;
+                       }
                        kbd->overload_start_time = time;
                        activate_layer(kbd, code, idx);
                        update_mods(kbd, -1, 0);
@@ -604,8 +609,16 @@ static long process_descriptor(struct keyboard *kbd, uint8_t code,
                        if (kbd->last_pressed_code == code &&
                            (!kbd->config.overload_tap_timeout ||
                             ((time - kbd->overload_start_time) < kbd->config.overload_tap_timeout))) {
+                               uint8_t layer_active[MAX_LAYERS];
+                               for (i = 0; i < kbd->config.nr_layers; i++) {
+                                       layer_active[i] = kbd->layer_state[i].active;
+                                       kbd->layer_state[i].active = kbd->layer_state[i].active_overload_pressed;
+                               }
                                process_descriptor(kbd, code, action, dl, 1, time);
                                process_descriptor(kbd, code, action, dl, 0, time);
+                               for (i = 0; i < kbd->config.nr_layers; i++) {
+                                       kbd->layer_state[i].active = layer_active[i];
+                               }
                        }
                }
 
diff --git a/src/keyboard.h b/src/keyboard.h
index 8bb8af3..9e072c8 100644
--- a/src/keyboard.h
+++ b/src/keyboard.h
@@ -116,6 +116,7 @@ struct keyboard {
                long activation_time;
 
                uint8_t active;
+               uint8_t active_overload_pressed;
                uint8_t toggled;
                uint8_t oneshot_depth;
        } layer_state[MAX_LAYERS];

matthias-hmb avatar Dec 16 '23 21:12 matthias-hmb

Can you provide a real world use case?

The example you gave would better be solved by using one of the overloadt* functions, since overloading letter keys involves a lot of potentially ambiguous sequences that necessitate a timeout (there are several lengthy discussions about the implementation buried amongst the issues).

overload is mainly intended for keys which tend to be typed in isolation, like backspace or tab which don't tend to suffer from problems better solved by timeouts.

rvaiya avatar Dec 17 '23 06:12 rvaiya

The setup, I'm experimenting with right now is the home-row-mods ( https://precondition.github.io/home-row-mods ) I tried right now with the overloadt, which is helping a lot, because now "rolling" (fast) over the characters doesn't use them as modifiers. Thanks.

It is more my expectation, that the key is used with the modifiers in the moment, it is pressed and maybe it doesn't fit into your design. But the statement of the ticket is still the case. If I use a modifier with an overload/overloadt key and then release the modifier first, the key will be send with the released modifier. (My patch is not complete, because it doesn't work with overloadt - maybe there is a better way anyway.)

matthias-hmb avatar Dec 17 '23 13:12 matthias-hmb

I suppose this could be construed as a bug, but implementing a proper fix is non trivial. Simply saving and restoring layer state doesn't work since the tap action might modify the layer stack. One could account for the simple case of a non action key by preserving modifier state (rather than layer state), but it might break some use cases, and I haven't fully considered all the implications.

In general, my inclination is to ward the user in the direction of the overloadt* functions, as the goal of overloading a key that is likely to be typed in tandem with another key will present other problems (in your example imagine dt is typed, but t is still held while d is actuated).

rvaiya avatar Dec 17 '23 19:12 rvaiya