feat(behaviors): sticky keys no longer sticky after being held

[nguyendown]

Awesome suggestion from RexxStone.
https://discord.com/channels/719497620560543766/1112676462089207828/1112676462089207828

A tap will stick.

      press            release
        ^                 ^
========|-----------------|===========================
          no-sticky-after-hold-ms
        <------------------------->

                            release-after-ms
                          <------------------>
========|-----------------|------------------|========
        v                                    v
     activate                            deactivate

A hold won't stick.

      press                        release
        ^                             ^
========|-----------------------------|===============
          no-sticky-after-hold-ms
        <------------------------->
========|-----------------------------|===============
        v                             v
     activate                     deactivate

Sticky keys will act like normal keys if they are held for longer than a configurable amount of time. I am deciding on a name for this property. Please help me.

• no-sticky-after-hold-ms
• max-hold-ms
• deactivate-from-hold-ms

Currently, the property is optional and won't break existing configs. The default value is set to be the same as release-after-ms.

[petejohanson]

I don't see how this is accounting for the comment above about adjusting for being held.. The timestamp in the event might be "older" than the actual time if the release was captured then re-released.

Did you test this in combination of hold taps?

[nguyendown]

I included this change by mistake. I wanted to test if it would break.

Did you test this in combination of hold taps?

I planned to, but I haven't had the chance to test it yet.

[theol0403]

What was your reasoning for deleting this? I believe it stands as a failsafe for an edge case for a laggy timer, but I'm not 100% sure exactly when it is being used.

[nguyendown]

release_at is calculated upon key press. This failsafe deactivates sticky keys while they are being held. The unit test 13-active-until-release covers this.

I have been using this for a month now without noticing any issues. Perhaps my keymap isn't complicated enough to cause any problems?

There is certainly a way to keep the failsafe but I haven't thought much about it. Personally, I prefer not to rely on failsafes and believe that unexpected behaviors should be fixed and verified through testing. (Maybe I'm just being lazy.) Let me know what you think.

[theol0403]

For sure... I don't know enough to say for sure why the failsafe is there, but I think it would be wise to really understand why it was needed in the first place and whether it is safe to remove.

[theol0403]

This implementation looks good to me, and considers everything discussed in #1636

[theol0403]

Small cleanup

Suggested change

	if (ms_left < 0) {
	k_work_schedule(&sticky_key->release_timer, K_MSEC(ms_left > 0 ? ms_left : 0));

[nguyendown]

Compilers probably optimize this already. Even if they do not, it's not worth sacrificing readability here, considering that the code is not run often.

[theol0403]

Personally, I preferred the PR without the config option. I think this change is a sane default and I can't imagine any usecase for the timeout after release. I think the config option adds redundant complexity.

Edit: I read the usecase about using it instead of a tap hold for "don't hold sticky if held". While I think this is a good feature, I think that the tap-hold approach is superior, as a) it composes existing features without introducing new ones b) it handles edge cases including that happens if another key is pressed during held, hold-while-undecided, etc.

[nguyendown]

usecase for the timeout after release

My focus is on no-sticky-after-hold. Whether the release (deactivate) timer starts on binding press or binding release, it is a separate issue. The reason I changed it in d1a0027 is that I could make use of release_at to solve two issues at once.

tap-hold approach

Sticky key needs a standalone feature and does not rely on any workarounds or other behaviors. Besides, keymaps could be much more simple.

&sk {
    no-sticky-after-hold-ms = <200>;
};

And I don't have to read through the Hold-Tap documentation or understand ten different hold-tap flavors. (I did read it but am still confused btw.)

[kvietcong]

Are there any blockers in getting this merged? I love this feature but it seems to be quite behind master at this point.