getreuer / qmk-keymap

My keymap & reusable QMK gems
Apache License 2.0
301 stars 45 forks source link

Add retro tapping support #55

Closed akaralar closed 6 months ago

akaralar commented 7 months ago

I tried to add support for features where a key is held and then released without pressing another key to fix https://github.com/getreuer/qmk-keymap/issues/51

In this PR, we try to circumvent the Achordion timeout if no other keypress happens between the press and release of the tap-hold key. When a tap-hold is pressed and released without the user pressing any other key in between, Achordion normally waits until the timeout finishes to settle the key as held and to send the events back to QMK to be processed. With this PR, If no other keypress happens until the tap-hold key is released, we simulate a hold and then release event to make Achordion play nice with these features.

getreuer commented 6 months ago

@akaralar thank you so much for this contribution! I very much appreciate this support for retro tap, this rounds out Achordion nicely.

The implementation looks good. Just a couple minor comments.

akaralar commented 6 months ago

@getreuer Thanks for the review! I have since changed the implementation to be more generic in my userspace. As far as I understood the problem was that when there isn't a second keypress (Retro tapping, retro shift, autoshift, custom macros for hold action all fall in this category) Achordion holds back the hold event for TAPPING_TERM + get_achordion_timeout amount of time. I think this is less than ideal because we don't actually need Achordion when there isn't a second keypress. Let me push those changes to see if you think they would fit in this repo.

akaralar commented 6 months ago

@getreuer I pushed a new commit, changed the PR description and added comments to the code to explain the changes, let me know what you think. Thanks for taking the time!

wolfwood commented 6 months ago

current version works for me 👍

hitting a second key on the same hand while holding the first immediately resolves both as taps.

getreuer commented 6 months ago

@akaralar I agree with your points! I'll merge the PR in its current form. Thank you again for your contribution!

akaralar commented 6 months ago

@getreuer glad I could help! thank you again for making such a great library and making home row mods tolerable 😃