[Core] Add TAPPING_RELEASE_HOLD_TERM option for Tapping Force Hold

[filterpaper]

Description

This change adds TAPPING_RELEASE_HOLD_TERM option to the Tapping Force Hold function. It will allow force hold to be interrupted when user holds a tap-hold key (quickly) after tapping within TAPPING_RELEASE_HOLD_TERM duration in milliseconds.

When used together with TAPPING_FORCE_HOLD_PER_KEY, it will only apply to tap-hold keycodes define by the get_tapping_force_hold function. The value must be less than TAPPING_TERM, or Force Hold will be disabled completely.

Test cases

• Without TAPPING_FORCE_HOLD:
  • Tap and hold action on all mod-tap keys should trigger auto-repeat normally.
• TAPPING_FORCE_HOLD only:
  • Tap and hold action on all mod-tap keys should never trigger auto-repeat.
• TAPPING_FORCE_HOLD_PER_KEY:
  • Tap and hold action on keycodes defined in get_tapping_force_hold should never trigger auto-repeat.
  • All other mod-tap keys should trigger auto-repeat normally.
• TAPPING_RELEASE_HOLD_TERM 100:
  • Tap and hold action on all mod-tap keys should trigger auto-repeat with quick deliberate action.
  • Reducing TAPPING_RELEASE_HOLD_TERM to 50 should make them even harder to trigger.
• TAPPING_FORCE_HOLD_PER_KEY and TAPPING_RELEASE_HOLD_TERM 100:
  • Tap and hold action on keycodes defined in get_tapping_force_hold should trigger auto-repeat with quick deliberate action.
  • Reducing TAPPING_RELEASE_HOLD_TERM to 50 should make those keycodes even harder to trigger.
  • All other mod-tap keys should trigger auto-repeat normally, unaffected by TAPPING_RELEASE_HOLD_TERM values.
• Regression test: all other features and normal operations associated with mod-tap / tap-hold keys should remain unaffected.

Types of Changes

• Core
• New feature
• Enhancement/optimization
• Documentation

Checklist

• My code follows the code style of this project: C, Python
• I have read the PR Checklist document and have made the appropriate changes.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• I have tested the changes and verified that they work and don't break anything (as well as I can manage).

[precondition]

The code looks good to me but in my opinion, the logic should be reversed. When tap-then-holding a key quickly, I would prefer to force the hold (e.g. writing the word "camelCase" with L as a shift modtap). If I want to auto-repeat a key, that's a much more deliberate action so that should be the thing that requires TAPPING_FORCE_HOLD_TERM to pass. It would also make more sense in regards to the proposed name : "The tapping force hold term is the time window in which the hold is forced".

However, I can see how my suggestion could be problematic in an example scenario like this:
Assume I have LALT_T(KC_A) in my keymap. Let's say I typed the letter 'a' in a text editor and I then got up and left the room for 10 minutes. When I come back, I want to Alt+Tab out of my text editor so I hold LALT_T(KC_A) but I get a bunch of spammed A's instead.

This could be solved by a time-out but that would further increase the complexity of the already very complex tap-hold mechanics.

[precondition]

Suggested change

	When a tap-hold key is held again after tapping within `TAPPING_FORCE_HOLD_TERM` in milliseconds, force hold will be interrupted. This will provide user the ability to deliberately override force hold with quick tap and hold action to auto-repeat a key. `TAPPING_FORCE_HOLD_TERM` must be less than `TAPPING_TERM` or tapping force hold will be disabled.
	When a tap-hold key is held again after tapping within `TAPPING_FORCE_HOLD_TERM` in milliseconds, the hold action is selected. If you want to auto-repeat the keycode associated to the dual-role key, you'd need to tap the dual-role key and then wait for the `TAPPING_FORCE_HOLD_TERM` to expire before holding it.
	`TAPPING_FORCE_HOLD_TERM` must be less than `TAPPING_TERM` or else tapping force hold will be disabled.

[precondition]

Suggested change

	(!get_tapping_force_hold(tapping_keycode, keyp) || WITHIN_FORCE_HOLD_TERM(event)) &&
	# elif defined(TAPPING_FORCE_HOLD_PER_KEY) && !defined(TAPPING_FORCE_HOLD_TERM)
	!get_tapping_force_hold(tapping_keycode, keyp) &&
	# elif !defined(TAPPING_FORCE_HOLD_PER_KEY) && defined(TAPPING_FORCE_HOLD_TERM)
	WITHIN_FORCE_HOLD_TERM(event) &&
	(!get_tapping_force_hold(tapping_keycode, keyp) || !WITHIN_FORCE_HOLD_TERM(event)) &&
	# elif defined(TAPPING_FORCE_HOLD_PER_KEY) && !defined(TAPPING_FORCE_HOLD_TERM)
	!get_tapping_force_hold(tapping_keycode, keyp) &&
	# elif !defined(TAPPING_FORCE_HOLD_PER_KEY) && defined(TAPPING_FORCE_HOLD_TERM)
	!WITHIN_FORCE_HOLD_TERM(event) &&

[filterpaper]

The code looks good to me but in my opinion, the logic should be reversed.

I agree that your logic solves a different use-case. This PR solves a personal use-case where slow same finger bigram on mod-tap keys are prevented from auto-repeating unless they are deliberately double-tap-held.

That said, logic for TAPPING_FORCE_HOLD_TERM can be reversed by introducing another flag/definition (in a subsequent PR) to swap that if condition.

[precondition]

(TAPPING_FORCE_HOLD_TERM might be a misnomer because it disables the force hold function—open to suggestions for a better label)

1. DOUBLE_PRESS_AUTO_REPEAT_TERM
2. TAP_AND_HOLD_TERM
3. TAPPING_FORCE_REPEATED_TAP_TERM
4. REPEATED_TAP_TERM
5. REPEATED_TAP_ON_HOLD_TERM
6. ANTI_TAPPING_FORCE_HOLD_TERM
7. TAP_THEN_HOLD_TERM
8. REPEAT_TAP_ON_DOUBLE_PRESS_TERM
9. ALLOW_AUTO_REPEAT_TERM
10. SPAM_TAP_TERM
11. ALLOW_SPAM_TAP_TERM

In any case, I don't think we should use "double-tap" to describe the action:

key down
key up
key down

Because a "tap" means pressing and releasing the key while in this scenario, you tap once and follow the tap with a press of the key (that you keep held down for a relatively long time).

[filterpaper]

I had these in my notes as well:

TAPPING_FORCE_UNHOLD_TERM
TAPPING_FORCE_HOLD_RELEASE_TERM
TAPPING_RELEASE_HOLD_TERM

[stale]

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

[drashna]

Please add to the testing suite.

Eg, see tests/tap_hold_configurations/ for existing tests

[filterpaper]

Please add to the testing suite.

I hope that's the right syntax for a test case.

[drashna]

IIRC, mods cause issues with the testing stuff. using layers may be ... better.
(eg, I was looking at some of that stuff this week)

[filterpaper]

The unit test tap_mod_tap_hold_key_twice_and_hold_on_second_time that started failing was there prior to my change. Are you recommending LT() for these tests?

[precondition]

I'm being annoying but do you really want to keep the name "TAPPING_RELEASE_HOLD_TERM"? It sounds like "the time window in which you must tap (i.e. press and release) the key in order to 'release' (i.e. send a key-up event) of the 'hold' action of the dual-role key". That's not really its true meaning. I like "DOUBLE_PRESS_AUTO_REPEAT_TERM" because you can infer from the name that it is "the time window in which you must double press — as opposed to a double tap which is a different thing — the key in order for it to auto-repeat".

While thinking about it, I also came to realise that the current default behaviour is akin to #define DOUBLE_PRESS_AUTO_REPEAT_TERM TAPPING_TERM while TAPPING_FORCE_HOLD would be equivalent to #define DOUBLE_PRESS_AUTO_REPEAT_TERM 0. There is potential here for a refactor that would simplify action_tapping and the documentation. That would imply deprecating TAPPING_FORCE_HOLD in favour of DOUBLE_PRESS_AUTO_REPEAT_TERM, which in my opinion is a lot clearer and will finally give a name to the default behaviour of auto-repeat after a double-press/tap-then-hold.

As for code size considerations, the compiler can easily infer that g_tapping_term (a uint16_t) can never be strictly below 0 (the value of DOUBLE_PRESS_AUTO_REPEAT_TERM if a TAPPING_FORCE_HOLD-like experience is desired) so it will not include the unreachable code in the produced binary so we can remove those messy #ifdefs currently sprinkled in the code for handling auto-repeat vs hold.

[filterpaper]

While thinking about it, I also came to realise that the current default behaviour is akin to #define DOUBLE_PRESS_AUTO_REPEAT_TERM TAPPING_TERM while TAPPING_FORCE_HOLD would be equivalent to #define DOUBLE_PRESS_AUTO_REPEAT_TERM 0. There is potential here for a refactor that would simplify action_tapping and the documentation. That would imply deprecating TAPPING_FORCE_HOLD in favour of DOUBLE_PRESS_AUTO_REPEAT_TERM, which in my opinion is a lot clearer and will finally give a name to the default behaviour of auto-repeat after a double-press/tap-then-hold.

You are not being annoying—the point of PRs is to solicit review and feedback. To be honest, this simple change is a partial implementation of a feature I use in ZMK. What you proposed above is exactly how TAPPING_FORCE_HOLD feature is actually implemented in ZMK as quick-tap-ms.

[precondition]

What you proposed above is exactly how TAPPING_FORCE_HOLD feature is actually implemented in ZMK as quick-tap-ms.

Oh neat, I like how they generalised the idea even further and found a way to reduce visual lag along the way.

[filterpaper]

As it stands now, the presence of TAPPING_RELEASE_HOLD_TERM overrides TAPPING_FORCE_HOLD. It is in a position to replace tapping force hold feature in the following manner for tap-hold keys:

• TAPPING_RELEASE_HOLD_TERM not set: repeat key when double-tapped and hold within TAPPING_TERM.
• TAPPING_RELEASE_HOLD_TERM < TAPPING_TERM: repeat key when double-tapped and hold within TAPPING_RELEASE_HOLD_TERM.
• TAPPING_RELEASE_HOLD_TERM < 1: no repeat key when double-tapped and hold.

I'm in favour of replacing TAPPING_FORCE_HOLD with a timer based QUICK_TAP_TERM to simplify its implementation, and use a QUICK_TAP_PER_KEY to replace TAPPING_FORCE_HOLD_PER_KEY.

[filterpaper]

@precondition Iteration of this feature is now in #17007