Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature Request] Permissive Hold and Ignore Mod Tap Interrupt for LT #8971

Closed
1 of 4 tasks
ShadowProgr opened this issue Apr 29, 2020 · 27 comments
Closed
1 of 4 tasks
Labels
enhancement help wanted stale Issues or pull requests that have become inactive without resolution.

Comments

@ShadowProgr
Copy link
Contributor

ShadowProgr commented Apr 29, 2020

Feature Request Type

  • Core functionality
  • Add-on hardware support (eg. audio, RGB, OLED screen, etc.)
  • Alteration (enhancement/optimization) of existing feature(s)
  • New behavior

Description

Currently Permissive Hold and Ignore Mod Tap Interrupt only affects MT() and not LT(). It would be nice to have that functionality extended to LT() since it's very similar to MT()

@nathanvercaemert
Copy link
Contributor

Can I work on this?

@ShadowProgr
Copy link
Contributor Author

Can I work on this?

please do

@sigprof
Copy link
Contributor

sigprof commented May 2, 2020

From my testing it seems that PERMISSIVE_HOLD actually works for LT(). Note that you need to test the behavior with #define TAPPING_TERM 499 or lower, because the code in process_tapping() forcibly enables PERMISSIVE_HOLD if TAPPING_TERM >= 500, unless the PERMISSIVE_HOLD_PER_KEY feature is used.

The PERMISSIVE_HOLD mode takes effect when the LT() key is held down, and then another key is pressed and released before the TAPPING_TERM expires. Without PERMISSIVE_HOLD this action would be handled as two taps if the LT() key is then also released before TAPPING_TERM expires (if the LT() key is held longer, the tap events for the mapping of the second key on another layer are emitted when TAPPING_TERM expires). With PERMISSIVE_HOLD the second release event immediately causes the layer switch to happen, and the tap events for the mapping of the second key on another layer are emitted at this time without waiting for TAPPING_TERM to expire. All this seems to behave exactly as documented.

However, I cannot obtain the documented effect of the ”no PERMISSIVE_HOLD mode” with MT(MOD_LCTL, KC_CAPS) and without enabling IGNORE_MOD_TAP_INTERRUPT — without PERMISSIVE_HOLD I still get events with the left Ctrl modifier applied if i release all keys before TAPPING_TERM; with PERMISSIVE_HOLD the events are emitted as soon as I release the other key. Only after enabling the IGNORE_MOD_TAP_INTERRUPT option the behavior of MT() becomes the same as the behavior of LT() described above.

What is missing now is the mode for LT() which corresponds to the default (IGNORE_MOD_TAP_INTERRUPT not enabled) behavior of MT().

@nathanvercaemert
Copy link
Contributor

nathanvercaemert commented May 3, 2020

@sigprof I think @ShadowProgr was referring to the combination of PERMISSIVE_HOLD and IGNORE_MOD_TAP_INTERRUPT. This combination is supposed to affect the sequence where the first key down (MT) is also the first key up (and both keys are up before the TAPPING_TERM.) This "down down up up" sequence (based on the documentation) sends the modified key even though the modifier was not held for the TAPPING_TERM. I do not see this effect even with MT, though! If I am interpreting the documentation correctly, this entire "combination functionality" is broken.

@ShadowProgr, have you tried this functionality with MT?

This is further confused by incorrect documentation. For the following sequence without PERMISSIVE_HOLD or IGNORE_MOD_TAP:

SFT_T(KC_A) Down
KC_X Down
SFT_T(KC_A) Up
KC_X Up

the documentation says that a capital "X" is sent, when lowercase "ax" is actually sent.

@sigprof
Copy link
Contributor

sigprof commented May 3, 2020

@nathanvercaemert
Yes, IGNORE_MOD_TAP_INTERRUPT is documented to affect the sequence where the MT key is released before the other key, and it appears to affect that sequence as documented. One issue is that the documentation for PERMISSIVE_HOLD is apparently written with the assumption that IGNORE_MOD_TAP_INTERRUPT is also enabled; however, when IGNORE_MOD_TAP_INTERRUPT is not enabled, the effect of PERMISSIVE_HOLD on MT keys is limited to reducing the delay before the events for the host are emitted.

I tried the suggested sequence with the current QMK master and without PERMISSIVE_HOLD or IGNORE_MOD_TAP_INTERRUPT (just #define TAPPING_TERM 499 to make testing easier), and it definitely sends a capital ”X”, although with a significant delay (looks like the Shift press event is sent when actual Shift is released, but the “X” tap is sent only after the tapping term timer expires). When I enable PERMISSIVE_HOLD, I get the same capital “X”, but faster (the Shift press event is sent when the actual Shift is released, then the “X” tap is sent when X is released without waiting for the tapping term timer to expire). I don't think that both of these behavior variations are really good (looks like in both cases the decision to consider the MT action to be a hold is effectively made when X is pressed, and therefore delaying the events any further is not really useful, but apparently the implementation of “mod tap interrupt” in process_action() gets invoked only after the MT key is released, and cannot even push the queued events through until the process_tapping() code decides to send them on its own — which also depends on PERMISSIVE_HOLD).

I think that PERMISSIVE_HOLD should really be named like HOLD_ON_OTHER_KEY_RELEASE (because this is what it actually does — it chooses the hold action immediately when another key is pressed and released during tapping term), and there should be another similar option HOLD_ON_OTHER_KEY_PRESS, which chooses the hold action immediately when another key is pressed during tapping term. That HOLD_ON_OTHER_KEY_PRESS option should provide the behavior which is similar to the current MT behavior, but without any extra delays, and this behavior would also be available for LT and other more obscure dual-use keys.

@ShadowProgr
Copy link
Contributor Author

@nathanvercaemert I was referring to all 3 cases where PERMISSIVE_HOLD and IGNORE_MOD_TAP_INTERRUPT are used separately and together. Since they didn't seem to affect how LT() behaved on my side at all, but I was using TAPPING_TERM 1000 to test so that could have been an issue.

Seems like right now the "problem" is that LT and MT behaves differently and those options also affect them differently. Which is very confusing to understand how LT and MT actually work

@sigprof
Copy link
Contributor

sigprof commented May 4, 2020

There are also other more obscure keycodes which use the same process_tapping() handling code. The actual set of actions is defined by is_tap_action(), but it shows action_t codes, not QMK keycodes. Mapping back to QMK keycodes gives the following list:

  • MT(mod, kc)
  • OSM(mod)
  • the QMK keycode for ACTION_MODS_TAP_TOGGLE(mods) does not seem to exist, but could probably be faked with MT(mod, MODS_TAP_TOGGLE)
  • LT(layer, kc)
  • OSL(layer)
  • TT(layer)
  • SH_T(kc)
  • SH_TT
  • MACROTAP(kc) — probably obsolete
  • FUNC(kc) could be anything, but it's obsolete too

The swap hands actions are unique in that they also have the process_record_tap_hint() handler.

@nathanvercaemert
Copy link
Contributor

@sigprof and @ShadowProgr , are you experiencing any difference between PERMISSIVE_HOLD and PERMISSIVE_HOLD/IGNORE_MOD_TAP_INTERRUPT combo (with the TAPPING_TERM @499?) What is the difference that you are observing?

@sigprof
Copy link
Contributor

sigprof commented May 4, 2020

I configured Caps Lock as MT(MOD_LCTL, KC_Z) for testing.

Without any extra options except #define TAPPING_TERM 499:

  • Single tap less than tapping term:
    • Caps Lock pressed → no output
    • Caps Lock released → Z down, Z up
  • Nested press of another key (everything during the tapping term):
    • Caps Lock pressed → no output
    • A pressed → no output
    • A released → no output
    • Caps Lock released → LCTL down
    • Tapping term expired → A down, A up, LCTL up
  • Nested press of another key (Caps Lock held longer than the tapping term):
    • Caps Lock pressed → no output
    • A pressed → no output
    • A released → no output
    • Tapping term expired → LCTL down, A down, A up
    • Caps Lock released → LCTL up
  • Rolling press of another key (everything during the tapping term):
    • Caps Lock pressed → no output
    • A pressed → no output
    • Caps Lock released → LCTL down
    • A released → A down, LCTL up, A up
  • Rolling press of another key (A held longer than the tapping term):
    • Caps Lock pressed → no output
    • A pressed → no output
    • Caps Lock released → LCTL down
    • Tapping term expired → A down, LCTL up
    • A released → A up
  • Rolling press of another key (both keys held longer than the tapping term):
    • Caps Lock pressed → no output
    • A pressed → no output
    • Tapping term expired → LCTL down, A down
    • Caps Lock released → LCTL up
    • A released → A up

With just IGNORE_MOD_TAP_INTERRUPT:

  • Single tap less than tapping term:
    • Caps Lock pressed → no output
    • Caps Lock released → Z down, Z up
  • Nested press of another key (everything during the tapping term):
    • Caps Lock pressed → no output
    • A pressed → no output
    • A released → no output
    • Caps Lock released → Z down, A down, A up, Z up
  • Nested press of another key (Caps Lock held longer than the tapping term):
    • Caps Lock pressed → no output
    • A pressed → no output
    • A released → no output
    • Tapping term expired → LCTL down, A down, A up
    • Caps Lock released → LCTL up
  • Rolling press of another key (everything during the tapping term):
    • Caps Lock pressed → no output
    • A pressed → no output
    • Caps Lock released → Z down, A down, Z up
    • A released → A up
  • Rolling press of another key (A held longer than the tapping term):
    • Caps Lock pressed → no output
    • A pressed → no output
    • Caps Lock released → Z down, A down, Z up
    • Tapping term expired → no output
    • A released → A up
  • Rolling press of another key (both keys held longer than the tapping term):
    • Caps Lock pressed → no output
    • A pressed → no output
    • Tapping term expired → LCTL down, A down
    • Caps Lock released → LCTL up
    • A released → A up

With PERMISSIVE_HOLD:

  • Single tap less than tapping term:
    • Caps Lock pressed → no output
    • Caps Lock released → Z down, Z up
  • Nested press of another key (everything during the tapping term):
    • Caps Lock pressed → no output
    • A pressed → no output
    • A released → LCTL down, A down, A up
    • Caps Lock released → LCTL up
  • Nested press of another key (Caps Lock held longer than the tapping term):
    • Caps Lock pressed → no output
    • A pressed → no output
    • A released → LCTL down, A down, A up
    • Tapping term expired → no output
    • Caps Lock released → LCTL up
  • Rolling press of another key (everything during the tapping term):
    • Caps Lock pressed → no output
    • A pressed → no output
    • Caps Lock released → LCTL down
    • A released → A down, LCTL up, A up
  • Rolling press of another key (A held longer than the tapping term):
    • Caps Lock pressed → no output
    • A pressed → no output
    • Caps Lock released → LCTL down
    • Tapping term expired → A down, LCTL up
    • A released → A up
  • Rolling press of another key (both keys held longer than the tapping term):
    • Caps Lock pressed → no output
    • A pressed → no output
    • Tapping term expired → LCTL down, A down
    • Caps Lock released → LCTL up
    • A released → A up

With both PERMISSIVE_HOLD and IGNORE_MOD_TAP_INTERRUPT:

  • Single tap less than tapping term:
    • Caps Lock pressed → no output
    • Caps Lock released → Z down, Z up
  • Nested press of another key (everything during the tapping term):
    • Caps Lock pressed → no output
    • A pressed → no output
    • A released → LCTL down, A down, A up
    • Caps Lock released → LCTL up
  • Nested press of another key (Caps Lock held longer than the tapping term):
    • Caps Lock pressed → no output
    • A pressed → no output
    • A released → LCTL down, A down, A up
    • Tapping term expired → no output
    • Caps Lock released → LCTL up
  • Rolling press of another key (everything during the tapping term):
    • Caps Lock pressed → no output
    • A pressed → no output
    • Caps Lock released → Z down, A down, Z up
    • A released → A up
  • Rolling press of another key (A held longer than the tapping term):
    • Caps Lock pressed → no output
    • A pressed → no output
    • Caps Lock released → Z down, A down, Z up
    • Tapping term expired → no output
    • A released → A up
  • Rolling press of another key (both keys held longer than the tapping term):
    • Caps Lock pressed → no output
    • A pressed → no output
    • Tapping term expired → LCTL down, A down
    • Caps Lock released → LCTL up
    • A released → A up

@sigprof
Copy link
Contributor

sigprof commented May 4, 2020

Trying to summarize the above wall of text:

  • If PERMISSIVE_HOLD is enabled, changing IGNORE_MOD_TAP_INTERRUPT affects only the handling of rolling presses when the MT key is held less than the tapping term — without IGNORE_MOD_TAP_INTERRUPT such rolling presses are handled as MT hold actions, but enabling IGNORE_MOD_TAP_INTERRUPT changes them to MT tap actions.
  • If PERMISSIVE_HOLD is not enabled, changing IGNORE_MOD_TAP_INTERRUPT still affects rolling presses in the same way, but also affects nested presses when the MT key is held less than the tapping term — enabling IGNORE_MOD_TAP_INTERRUPT also changes those presses to MT tap actions.

Or, if we want to look at the changes from PERMISSIVE_HOLD:

  • If IGNORE_MOD_TAP_INTERRUPT is enabled, enabling PERMISSIVE_HOLD affects only the nested presses: a nested press when the MT key is held less than the tapping term is converted from MT tap to MT hold, and a nested press when the MT key is held more than the tapping term is still handled as MT hold, but with slightly different timings of generated events.
  • If IGNORE_MOD_TAP_INTERRUPT is not enabled, enabling PERMISSIVE_HOLD affects only some timings of generated events for both kind of nested presses, but does not really affect the generated keypresses.

@sigprof
Copy link
Contributor

sigprof commented May 4, 2020

I made another series of tests for Caps Lock mapped to LT(1, KC_Z) and the A key mapped to KC_F1 on layer 1.

Without any extra options except #define TAPPING_TERM 499:

  • Single tap less than tapping term:
    • Caps Lock pressed → no output
    • Caps Lock released → Z down, Z up
  • Nested press of another key (everything during the tapping term):
    • Caps Lock pressed → no output
    • A pressed → no output
    • A released → no output
    • Caps Lock released → Z down, A down, A up, Z up
  • Nested press of another key (Caps Lock held longer than the tapping term):
    • Caps Lock pressed → no output
    • A pressed → no output
    • A released → no output
    • Tapping term expired → F1 down, F1 up
    • Caps Lock released → no output
  • Rolling press of another key (everything during the tapping term):
    • Caps Lock pressed → no output
    • A pressed → no output
    • Caps Lock released → Z down, A down, Z up
    • A released → A up
  • Rolling press of another key (A held longer than the tapping term):
    • Caps Lock pressed → no output
    • A pressed → no output
    • Caps Lock released → Z down, A down, Z up
    • Tapping term expired → no output
    • A released → A up
  • Rolling press of another key (both keys held longer than the tapping term):
    • Caps Lock pressed → no output
    • A pressed → no output
    • Tapping term expired → F1 down
    • Caps Lock released → no output
    • A released → F1 up

With PERMISSIVE_HOLD:

  • Single tap less than tapping term:
    • Caps Lock pressed → no output
    • Caps Lock released → Z down, Z up
  • Nested press of another key (everything during the tapping term):
    • Caps Lock pressed → no output
    • A pressed → no output
    • A released → F1 down, F1 Up
    • Caps Lock released → no output
  • Nested press of another key (Caps Lock held longer than the tapping term):
    • Caps Lock pressed → no output
    • A pressed → no output
    • A released → F1 down, F1 up
    • Tapping term expired → no output
    • Caps Lock released → no output
  • Rolling press of another key (everything during the tapping term):
    • Caps Lock pressed → no output
    • A pressed → no output
    • Caps Lock released → Z down, A down, Z up
    • A released → A up
  • Rolling press of another key (A held longer than the tapping term):
    • Caps Lock pressed → no output
    • A pressed → no output
    • Caps Lock released → Z down, A down, Z up
    • Tapping term expired → no output
    • A released → A up
  • Rolling press of another key (both keys held longer than the tapping term):
    • Caps Lock pressed → no output
    • A pressed → no output
    • Tapping term expired → F1 down
    • Caps Lock released → no output
    • A released → F1 up

@sigprof
Copy link
Contributor

sigprof commented May 4, 2020

Summarizing the above:

  • For LT the PERMISSIVE_HOLD option changes the behavior only for the nested press case: a nested press when the LT key is held less than the tapping term is converted from LT tap to LT hold, and a nested press when the LT key is held more than the tapping term is still handled as LT hold, but with slightly different timings of generated events.
  • The behavior of LT with PERMISSIVE_HOLD is identlcal to the behavior of MT with PERMISSIVE_HOLD+IGNORE_MOD_TAP_INTERRUPT, except the layer switches themselves are not visible as key events.
  • The behavior of LT without PERMISSIVE_HOLD is also identlcal to the behavior of MT with IGNORE_MOD_TAP_INTERRUPT.

@sigprof
Copy link
Contributor

sigprof commented May 4, 2020

I tried to make this hack, but don't have the time to test it properly at the moment:

diff --git a/tmk_core/common/action_tapping.c b/tmk_core/common/action_tapping.c
index 34f08d890..4d5202d1b 100644
--- a/tmk_core/common/action_tapping.c
+++ b/tmk_core/common/action_tapping.c
@@ -35,6 +35,10 @@ __attribute__((weak)) bool get_tapping_force_hold(uint16_t keycode, keyrecord_t
 __attribute__((weak)) bool get_permissive_hold(uint16_t keycode, keyrecord_t *record) { return false; }
 #    endif
 
+#    ifdef HOLD_ON_OTHER_KEY_PRESS_PER_KEY
+__attribute__((weak)) bool get_hold_on_other_key_press(uint16_t keycode, keyrecord_t *record) { return false; }
+#    endif
+
 static keyrecord_t tapping_key                         = {};
 static keyrecord_t waiting_buffer[WAITING_BUFFER_SIZE] = {};
 static uint8_t     waiting_buffer_head                 = 0;
@@ -163,6 +167,19 @@ bool process_tapping(keyrecord_t *keyp) {
                     // set interrupted flag when other key preesed during tapping
                     if (event.pressed) {
                         tapping_key.tap.interrupted = true;
+#    if defined(HOLD_ON_OTHER_KEY_PRESS) || defined(HOLD_ON_OTHER_KEY_PRESS_PER_KEY)
+#        if defined(HOLD_ON_OTHER_KEY_PRESS_PER_KEY)
+                        if (get_hold_on_other_key_press(get_event_keycode(tapping_key.event, false), &tapping_key))
+#        endif
+                        {
+                            debug("Tapping: End. No tap. Interfered by pressed key\n");
+                            process_record(&tapping_key);
+                            tapping_key = (keyrecord_t){};
+                            debug_tapping_key();
+                            // enqueue
+                            return false;
+                        }
+#    endif
                     }
                     // enqueue
                     return false;

@sigprof
Copy link
Contributor

sigprof commented May 4, 2020

Testing MT(MOD_LCTL, KC_Z) again, this time with the patch.

With just HOLD_ON_OTHER_KEY_PRESS enabled:

  • Single tap less than tapping term:
    • Caps Lock pressed → no output
    • Caps Lock released → Z down, Z up
  • Nested press of another key (everything during the tapping term):
    • Caps Lock pressed → no output
    • A pressed → LCTL down, A down
    • A released → A up
    • Caps Lock released → LCTL up
  • Nested press of another key (Caps Lock held longer than the tapping term):
    • Caps Lock pressed → no output
    • A pressed → LCTL down, A down
    • A released → A up
    • Tapping term expired → no output
    • Caps Lock released → LCTL up
  • Rolling press of another key (everything during the tapping term):
    • Caps Lock pressed → no output
    • A pressed → LCTL down, A down
    • Caps Lock released → LCTL up
    • A released → A up
  • Rolling press of another key (A held longer than the tapping term):
    • Caps Lock pressed → no output
    • A pressed → LCTL down, A down
    • Caps Lock released → LCTL up
    • Tapping term expired → no output
    • A released → A up
  • Rolling press of another key (both keys held longer than the tapping term):
    • Caps Lock pressed → no output
    • A pressed → LCTL down, A down
    • Tapping term expired → no output
    • Caps Lock released → LCTL up
    • A released → A up

With both HOLD_ON_OTHER_KEY_PRESS and IGNORE_MOD_TAP_INTERRUPT enabled:

  • Single tap less than tapping term:
    • Caps Lock pressed → no output
    • Caps Lock released → Z down, Z up
  • Nested press of another key (everything during the tapping term):
    • Caps Lock pressed → no output
    • A pressed → LCTL down, A down
    • A released → A up
    • Caps Lock released → LCTL up
  • Nested press of another key (Caps Lock held longer than the tapping term):
    • Caps Lock pressed → no output
    • A pressed → LCTL down, A down
    • A released → A up
    • Tapping term expired → no output
    • Caps Lock released → LCTL up
  • Rolling press of another key (everything during the tapping term):
    • Caps Lock pressed → no output
    • A pressed → LCTL down, A down
    • Caps Lock released → LCTL up
    • A released → A up
  • Rolling press of another key (A held longer than the tapping term):
    • Caps Lock pressed → no output
    • A pressed → LCTL down, A down
    • Caps Lock released → LCTL up
    • Tapping term expired → no output
    • A released → A up
  • Rolling press of another key (both keys held longer than the tapping term):
    • Caps Lock pressed → no output
    • A pressed → LCTL down, A down
    • Tapping term expired → no output
    • Caps Lock released → LCTL up
    • A released → A up

@sigprof
Copy link
Contributor

sigprof commented May 4, 2020

What the above patch does for MT:

  • If HOLD_ON_OTHER_KEY_PRESS is enabled, IGNORE_MOD_TAP_INTERRUPT does not change the behavior of MT anymore.
  • The behavior of MT with HOLD_ON_OTHER_KEY_PRESS is almost the same as the behavior of MT with PERMISSIVE_HOLD and without IGNORE_MOD_TAP_INTERRUPT, except some events are generated earlier (with PERMISSIVE_HOLD events are delayed until either one of the keys is released, or the tapping term expires; with HOLD_ON_OTHER_KEY_PRESS events are generated immediately after the second key is pressed).

@sigprof
Copy link
Contributor

sigprof commented May 4, 2020

Testing the patch with Caps Lock mapped to LT(1, KC_Z) and the A key mapped to KC_F1 on layer 1.

With just HOLD_ON_OTHER_KEY_PRESS enabled:

  • Single tap less than tapping term:
    • Caps Lock pressed → no output
    • Caps Lock released → Z down, Z up
  • Nested press of another key (everything during the tapping term):
    • Caps Lock pressed → no output
    • A pressed → F1 down
    • A released → F1 up
    • Caps Lock released → no output
  • Nested press of another key (Caps Lock held longer than the tapping term):
    • Caps Lock pressed → no output
    • A pressed → F1 down
    • A released → F1 up
    • Tapping term expired → no output
    • Caps Lock released → no output
  • Rolling press of another key (everything during the tapping term):
    • Caps Lock pressed → no output
    • A pressed → F1 down
    • Caps Lock released → no output
    • A released → F1 up
  • Rolling press of another key (A held longer than the tapping term):
    • Caps Lock pressed → no output
    • A pressed → F1 down
    • Caps Lock released → no output
    • Tapping term expired → no output
    • A released → F1 up
  • Rolling press of another key (both keys held longer than the tapping term):
    • Caps Lock pressed → no output
    • A pressed → F1 down
    • Tapping term expired → no output
    • Caps Lock released → no output
    • A released → F1 up

@sigprof
Copy link
Contributor

sigprof commented May 4, 2020

What the above patch does for LT:

  • The behavior with HOLD_ON_OTHER_KEY_PRESS is different from the behavior with PERMISSIVE_HOLD:
    • A rolling press when the LT key is held for less than the tapping term is handled as LT tap with PERMISSIVE_HOLD, but as LT hold with HOLD_ON_OTHER_KEY_PRESS.
    • With PERMISSIVE_HOLD events are delayed until either one of the keys is released, or the tapping term expires; with HOLD_ON_OTHER_KEY_PRESS events are generated immediately after the second key is pressed.
  • The behavior of LT with HOLD_ON_OTHER_KEY_PRESS is similar to the behavior of MT with PERMISSIVE_HOLD and without IGNORE_MOD_TAP_INTERRUPT — all situations when two keys are pressed simultaneously are handled as LT/MT hold, and the only difference is that with HOLD_ON_OTHER_KEY_PRESS some events are generated earlier.

@sigprof
Copy link
Contributor

sigprof commented May 4, 2020

The situation with tap-hold configuration options with the above patch would be as follows:

  • The default mode of hold detection is strict — the hold action is chosen only when the dual-use key is held for more than the tapping term. In some cases this means that the generated key events need to be delayed until the tapping term expires, so that the hold detection could be completed.
  • The PERMISSIVE_HOLD option (which could be renamed to HOLD_ON_OTHER_KEY_TAP) relaxes the hold detection: if another key is tapped (pressed and released) after the dual-use key was pressed, the hold action will be chosen for the dual-use key immediately after the tap (without waiting for the tapping term to expire). If another key is just pressed and not released before the dual-use key is released (and this happens before the tapping term expires), this action is not considered to be a hold.
  • The HOLD_ON_OTHER_KEY_PRESS option relaxes the hold detection even more: if another key is pressed after the dual-use key was pressed, the hold action will be chosen for the dual-use key immediately after the press (without waiting for the tapping term to expire).

Enabling the IGNORE_MOD_TAP_INTERRUPT option makes MT keys behave exactly like other dual-use keys. When IGNORE_MOD_TAP_INTERRUPT is not enabled, MT keys are handled in some legacy way that is similar to HOLD_ON_OTHER_KEY_PRESS, but with extra delay for some events if some other mode of hold detection is selected. I'm not sure how this legacy mode should be handled in the future (ideally it should just be removed, but that would mean either changing the default behavior of MT keys, or switching to HOLD_ON_OTHER_KEY_PRESS as a default for all dual-use keys).

@ShadowProgr
Copy link
Contributor Author

Thanks for the hard work on testing all those cases, @sigprof! I made a table out of your test results to compare the behaviors more easily.

I would be nice if MT and LT can behave similarly in all cases, if it's feasible

@sigprof
Copy link
Contributor

sigprof commented May 5, 2020

Yes, a table is a good idea here. I decided to make another table, where I rearranged the columns in a more logical order (first 6 columns show the proposed “new style” modes, last 3 columns show the “legacy mod-tap” behavior), and added the Summary sheet to show the tap/hold decisions done in various cases.

@sigprof
Copy link
Contributor

sigprof commented May 6, 2020

I pushed the commit adding HOLD_ON_OTHER_KEY_PRESS to https://github.com/sigprof/qmk_firmware/tree/tap-hold-options; did not make a PR out of it yet, because I still need to add some documentation (which then would collide with #8348), and I'm still not sure that PERMISSIVE_HOLD and HOLD_ON_OTHER_KEY_PRESS are good names for these configuration options.

@jmding8
Copy link

jmding8 commented May 12, 2020

FWIW, from the perspective of someone new to QMK, "PERMISSIVE_HOLD" is not a particularly intuitive name for this configuration option. "HOLD_ON_OTHER_KEY_PRESS", although not perfect, is definitely better (for me personally, I think "PERFORM_HOLD_ACTION_ON_OTHER_KEY_PRESS" is a little easier to understand). I'm really looking forward to making use of your changes, and think it makes sense pushing since it is both more aligned with the user's expected behavior, and also improves on the status quo configuration option naming (even if its not perfect). A case of "don't let perfect be the enemy of good."

@aaronjensen
Copy link

I'm testing the tap-hold-options branch and it's doing exactly what I want. There's one LT key combination where I actually want it to behave differently (I want it to send the tap key and then the pressed key when I roll) and I'm able to cover that with SEND_STRING on the activated layer, which is great (though it would be nice if there was a way to do this that was built in... not sure how that would work, exactly).

@sigprof
Copy link
Contributor

sigprof commented Jun 13, 2020

I opened PR #9404 with this code and some hopefully understandable documentation, but found a bug (OSM(MOD_RCTL) on a layer enabled by LT(_FN, KC_APP) does not work with a rolling press for some reason). It turned out that the problem with OSM on a layer was already there even when using a simple MO to switch layers, and is completely unrelated to my PR.

@stale
Copy link

stale bot commented Sep 12, 2020

This issue has been automatically marked as stale because it has not had activity in the last 90 days. It will be closed in the next 30 days unless it is tagged properly or other activity occurs.
For maintainers: Please label with bug, in progress, on hold, discussion or to do to prevent the issue from being re-flagged.

@stale stale bot added the stale Issues or pull requests that have become inactive without resolution. label Sep 12, 2020
@stale
Copy link

stale bot commented Oct 12, 2020

This issue has been automatically closed because it has not had activity in the last 30 days. If this issue is still valid, re-open the issue and let us know.

@ThreepE0
Copy link

ThreepE0 commented Mar 2, 2021

What is the status of this? Will it be pulled into QMK at some point, or are there conflicts? and does it still work?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement help wanted stale Issues or pull requests that have become inactive without resolution.
Projects
None yet
Development

No branches or pull requests

6 participants