Remove IGNORE_MOD_TAP_INTERRUPT_PER_KEY in favour of HOLD_ON_OTHER_KEY_PRESS_PER_KEY (#15741)

6 files changed, 131 insertions, 44 deletions

diff --git a/docs/ChangeLog/20230226/PR15741.md b/docs/ChangeLog/20230226/PR15741.md
new file mode 100644
index 0000000000..385816d65b
--- /dev/null
+++ b/docs/ChangeLog/20230226/PR15741.md
@@ -0,0 +1,43 @@
+`IGNORE_MOD_TAP_INTERRUPT_PER_KEY` has been removed and `IGNORE_MOD_TAP_INTERRUPT` deprecated as a stepping stone towards making `IGNORE_MOD_TAP_INTERRUPT` the new default behavior for mod-taps in the future.
+
+In place of the now removed `IGNORE_MOD_TAP_INTERRUPT_PER_KEY`, one must use the pre-existing `HOLD_ON_OTHER_KEY_PRESS` option.
+
+In most cases, updating `get_ignore_mod_tap_interrupt` to `get_hold_on_other_key_press` is simply a matter of renaming the function and swapping every `true` by `false` and vice versa. The one subtlety you may need to look out for is that the `get_ignore_mod_tap_interrupt` was only ever called with mod-taps passed in as the `keycode` argument, while the `keycode` argument of `get_hold_on_other_key_press` can be any dual-role key. This includes not only mod-taps, but also layer-taps, one shot keys, `TT(layer)` and more. This has an impact on the effect of the `default` case in a typical per-key configuration making use of a `switch(keycode)` statement.
+
+To illustrate, let's take the example of a configuration where we'd want all mod-taps to activate the modifier if another key is pressed while held with the exception of `LCTL_T(KC_A)`, which should ignore keys pressed while it is held and activate the modifier only if it has been held for longer than the tapping term. In addition, we would like to keep the default "ignore-interrupt" behavior of layer taps.
+
+An old way to do this would be via the following code:
+
+```c
+bool get_ignore_mod_tap_interrupt(uint16_t keycode, keyrecord_t *record) {
+    switch(keycode) {
+        case LCTL_T(KC_A):
+            return true;
+        default:
+            return false;
+    }
+}
+```
+
+The correct way to update this code without accidentally changing how the layer-taps work would be the following:
+
+```c
+bool get_hold_on_other_key_press(uint16_t keycode, keyrecord_t *record) {
+    switch(keycode) {
+        // Capture all mod-tap keycodes.
+        case QK_MOD_TAP ... QK_MOD_TAP_MAX:
+            if (keycode == LCTL_T(KC_A)) {
+                // Disable HOLD_ON_OTHER_KEY_PRESS for LCTL_T(KC_A)
+                // aka enable IGNORE_MOD_TAP_INTERRUPT for LCTL_T(KC_A).
+                return false;
+            } else {
+                // Enable HOLD_ON_OTHER_KEY_PRESS for every other mod-tap keycode.
+                return true;
+            }
+        default:
+            return false;
+    }
+}
+```
+
+For more information, you are invited to read the sections on [IGNORE_MOD_TAP_INTERRUPT](tap_hold.md#ignore-mod-tap-interrupt) and [HOLD_ON_OTHER_KEY_PRESS](tap_hold.md#hold-on-other-key-press) in the page on [Tap-Hold configuration options](tap_hold.md).
diff --git a/docs/config_options.md b/docs/config_options.md
index edaa739201..7a91160bcd 100644
--- a/docs/config_options.md
+++ b/docs/config_options.md
@@ -169,8 +169,6 @@ If you define these options you will enable the associated feature, which may in
 * `#define IGNORE_MOD_TAP_INTERRUPT`
   * makes it possible to do rolling combos (zx) with keys that convert to other keys on hold, by enforcing the `TAPPING_TERM` for both keys.
   * See [Ignore Mod Tap Interrupt](tap_hold.md#ignore-mod-tap-interrupt) for details
-* `#define IGNORE_MOD_TAP_INTERRUPT_PER_KEY`
-  * enables handling for per key `IGNORE_MOD_TAP_INTERRUPT` settings
 * `#define QUICK_TAP_TERM 100`
   * tap-then-hold timing to use a dual role key to repeat keycode
   * See [Quick Tap Term](tap_hold.md#quick-tap-term)
@@ -178,6 +176,11 @@ If you define these options you will enable the associated feature, which may in
   * Defaults to `TAPPING_TERM` if not defined
 * `#define QUICK_TAP_TERM_PER_KEY`
   * enables handling for per key `QUICK_TAP_TERM` settings
+* `#define HOLD_ON_OTHER_KEY_PRESS`
+  * selects the hold action of a dual-role key as soon as the tap of the dual-role key is interrupted by the press of another key.
+  * See "[hold on other key press](tap_hold.md#hold-on-other-key-press)" for details
+* `#define HOLD_ON_OTHER_KEY_PRESS_PER_KEY`
+  * enables handling for per key `HOLD_ON_OTHER_KEY_PRESS` settings
 * `#define LEADER_TIMEOUT 300`
   * how long before the leader key times out
     * If you're having issues finishing the sequence before it times out, you may need to increase the timeout setting. Or you may want to enable the `LEADER_PER_KEY_TIMING` option, which resets the timeout after each key is tapped.
diff --git a/docs/feature_auto_shift.md b/docs/feature_auto_shift.md
index d3437a9c60..1719807e26 100644
--- a/docs/feature_auto_shift.md
+++ b/docs/feature_auto_shift.md
@@ -281,16 +281,7 @@ Tap Hold Configurations work a little differently when using Retro Shift.
 Referencing `TAPPING_TERM` makes little sense, as holding longer would result in
 shifting one of the keys.
 
-`IGNORE_MOD_TAP_INTERRUPT` changes *only* rolling from a mod tap (releasing it
-first), sending both keys instead of the modifier on the second. Its effects on
-nested presses are ignored.
-
-As nested taps were changed to act as though `PERMISSIVE_HOLD` is set unless only
-`IGNORE_MOD_TAP_INTERRUPT` is (outside of Retro Shift), and Retro Shift ignores
-`IGNORE_MOD_TAP_INTERRUPT`, `PERMISSIVE_HOLD` has no effect on Mod Taps.
-
-Nested taps will *always* act as though the `TAPPING_TERM` was exceeded for both
-Mod and Layer Tap keys.
+`RETRO_SHIFT` enables [`PERMISSIVE_HOLD`-like behaviour](tap_hold.md#permissive-hold) (even if not explicitly enabled) on all mod-taps for which `RETRO_SHIFT` applies.
 
 ## Using Auto Shift Setup
 
diff --git a/docs/ja/config_options.md b/docs/ja/config_options.md
index c95753bd5d..6135721a42 100644
--- a/docs/ja/config_options.md
+++ b/docs/ja/config_options.md
@@ -162,8 +162,6 @@ QMK での全ての利用可能な設定にはデフォルトがあります。�
 * `#define IGNORE_MOD_TAP_INTERRUPT`
   * 両方のキーに `TAPPING_TERM` を適用することで、ホールド時に他のキーに変換するキーを使ってローリングコンボ (zx) をすることができるようにします
   * 詳細は [Ignore Mod Tap Interrupt](ja/tap_hold.md#ignore-mod-tap-interrupt) を見てください
-* `#define IGNORE_MOD_TAP_INTERRUPT_PER_KEY`
-  * キーごとの `IGNORE_MOD_TAP_INTERRUPT` 設定の処理を有効にします
 * `#define TAPPING_FORCE_HOLD`
   * タップされた直後に、デュアルロールキーを修飾子として使用できるようにします
   * [Tapping Force Hold](ja/tap_hold.md#tapping-force-hold)を見てください
diff --git a/docs/ja/tap_hold.md b/docs/ja/tap_hold.md
index 07242821a9..ac64fe6ce3 100644
--- a/docs/ja/tap_hold.md
+++ b/docs/ja/tap_hold.md
@@ -110,25 +110,6 @@ bool get_permissive_hold(uint16_t keycode, keyrecord_t *record) {
 
 ?> `許容ホールド`を有効にすると、これは両方がどのように動作するかを変更します。通常のキーには、最初のキーが最初に放された場合、あるいは両方のキーが  `TAPPING_TERM` より長くホールドされた場合に、修飾キーが追加されます。
 
-この機能をより細かく制御するために、以下を `config.h` に追加することができます:
-
-```c
-#define IGNORE_MOD_TAP_INTERRUPT_PER_KEY
-```
-
-そして、以下の関数をキーマップに追加します:
-
-```c
-bool get_ignore_mod_tap_interrupt(uint16_t keycode, keyrecord_t *record) {
-    switch (keycode) {
-        case SFT_T(KC_SPC):
-            return true;
-        default:
-            return false;
-    }
-}
-```
-
 ## タッピング強制ホールド
 
 `タッピング強制ホールド` を有効にするには、以下を `config.h` に追加します:
diff --git a/docs/tap_hold.md b/docs/tap_hold.md
index fa6c6abc14..348e2655eb 100644
--- a/docs/tap_hold.md
+++ b/docs/tap_hold.md
@@ -118,7 +118,7 @@ The reason is that `TAPPING_TERM` is a macro that expands to a constant integer
 
 The code which decides between the tap and hold actions of dual-role keys supports three different modes, in increasing order of preference for the hold action:
 
-1. The default mode selects the hold action only if the dual-role key is held down longer than the tapping term.  In this mode pressing other keys while the dual-role key is held down does not influence the tap-or-hold decision.
+1. The default mode selects the hold action only if the dual-role key is held down longer than the tapping term.  In this mode pressing other keys while the dual-role key is held down does not influence the tap-or-hold decision. In other words, this mode ignores interrupts.
 
 2. The “permissive hold” mode, in addition to the default behavior, immediately selects the hold action when another key is tapped (pressed and then released) while the dual-role key is held down, even if this happens earlier than the tapping term.  If another key is just pressed, but then the dual-role key is released before that other key (and earlier than the tapping term), this mode will still select the tap action.
 
@@ -126,6 +126,73 @@ The code which decides between the tap and hold actions of dual-role keys suppor
 
 Note that until the tap-or-hold decision completes (which happens when either the dual-role key is released, or the tapping term has expired, or the extra condition for the selected decision mode is satisfied), key events are delayed and not transmitted to the host immediately.  The default mode gives the most delay (if the dual-role key is held down, this mode always waits for the whole tapping term), and the other modes may give less delay when other keys are pressed, because the hold action may be selected earlier.
 
+### Comparison :id=comparison
+
+To better illustrate the tap-or-hold decision modes, let us compare the expected output of each decision mode in a handful of tapping scenarios involving a mod-tap key (`LSFT_T(KC_A)`) and a regular key (`KC_B`) with the `TAPPING_TERM` set to 200ms.
+
+By default, mod-taps behave like `HOLD_ON_OTHER_KEY_PRESS`, while layer-taps behave like "Ignore Interrupt" out of the box. If you want "Ignore Interrupt"-like behaviour for mod-taps, you must enable `IGNORE_MOD_TAP_INTERRUPT`, or return `false` in the `get_hold_on_other_key_press` function for all mod-taps.
+
+Note: "`kc` held" in the "Physical key event" column means that the key wasn't physically released yet at this point in time.
+
+#### Distinct taps (AABB) :id=distinct-taps
+
+| Time | Physical key event |Ignore Interrupt| `PERMISSIVE_HOLD` |  `HOLD_ON_OTHER_KEY_PRESS` |
+|------|--------------------|----------------|-------------------|----------------------------|
+| 0    | `LSFT_T(KC_A)` down|                |                   |                            |
+| 199  | `LSFT_T(KC_A)` up  | a              | a                 |  a                         |
+| 210  | `KC_B`      down   | ab             | ab                |  ab                        |
+| 220  | `KC_B`      up     | ab             | ab                |  ab                        |
+
+| Time | Physical key event |Ignore Interrupt| `PERMISSIVE_HOLD` |  `HOLD_ON_OTHER_KEY_PRESS` |
+|------|--------------------|----------------|-------------------|----------------------------|
+| 0    | `LSFT_T(KC_A)` down|                |                   |                            |
+| 200  | `LSFT_T(KC_A)` held|<kbd>Shift</kbd>| <kbd>Shift</kbd>  | <kbd>Shift</kbd>           |
+| 201  | `LSFT_T(KC_A)` up  |<kbd>Shift</kbd>| <kbd>Shift</kbd>  | <kbd>Shift</kbd>           |
+| 205  | `KC_B`      down   | b              | b                 |  b                         |
+| 210  | `KC_B`      up     | b              | b                 |  b                         |
+
+#### Nested tap (ABBA) :id=nested-tap
+
+| Time | Physical key event |Ignore Interrupt| `PERMISSIVE_HOLD` |  `HOLD_ON_OTHER_KEY_PRESS` |
+|------|--------------------|----------------|-------------------|----------------------------|
+| 0    | `LSFT_T(KC_A)` down|                |                   |                            |
+| 110  | `KC_B` down        |                |                   | B                          |
+| 120  | `KC_B` up          |                | B                 | B                          |
+| 199  | `LSFT_T(KC_A)` up  | ab             | B                 | B                          |
+
+| Time | Physical key event |Ignore Interrupt| `PERMISSIVE_HOLD` |  `HOLD_ON_OTHER_KEY_PRESS` |
+|------|--------------------|----------------|-------------------|----------------------------|
+| 0    | `LSFT_T(KC_A)` down|                |                   |                            |
+| 110  | `KC_B` down        |                |                   | B                          |
+| 120  | `KC_B` up          |                | B                 | B                          |
+| 200  | `LSFT_T(KC_A)` held| B              | B                 | B                          |
+| 210  | `LSFT_T(KC_A)` up  | B              | B                 | B                          |
+
+| Time | Physical key event |Ignore Interrupt| `PERMISSIVE_HOLD` |  `HOLD_ON_OTHER_KEY_PRESS` |
+|------|--------------------|----------------|-------------------|----------------------------|
+| 0    | `LSFT_T(KC_A)` down|                |                   |                            |
+| 200  | `LSFT_T(KC_A)` held|<kbd>Shift</kbd>| <kbd>Shift</kbd>  | <kbd>Shift</kbd>           |
+| 205  | `KC_B` down        | B              | B                 | B                          |
+| 210  | `KC_B` up          | B              | B                 | B                          |
+| 220  | `LSFT_T(KC_A)` up  | B              | B                 | B                          |
+
+#### Rolling keys (ABAB) :id=rolling-keys
+
+| Time | Physical key event |Ignore Interrupt| `PERMISSIVE_HOLD` |  `HOLD_ON_OTHER_KEY_PRESS` |
+|------|--------------------|----------------|-------------------|----------------------------|
+| 0    | `LSFT_T(KC_A)` down|                |                   |                            |
+| 110  | `KC_B` down        |                |                   | B                          |
+| 130  | `LSFT_T(KC_A)` up  | ab             | ab                | B                          |
+| 140  | `KC_B` up          | ab             | ab                | B                          |
+
+| Time | Physical key event |Ignore Interrupt| `PERMISSIVE_HOLD` |  `HOLD_ON_OTHER_KEY_PRESS` |
+|------|--------------------|----------------|-------------------|----------------------------|
+| 0    | `LSFT_T(KC_A)` down|                |                   |                            |
+| 110  | `KC_B` down        |                |                   | B                          |
+| 200  | `LSFT_T(KC_A)` held| B              | B                 | B                          |
+| 205  | `LSFT_T(KC_A)` up  | B              | B                 | B                          |
+| 210  | `KC_B` up          | B              | B                 | B                          |
+
 ### Default Mode
 Example sequence 1 (the `L` key is also mapped to `KC_RGHT` on layer 2):
 
@@ -179,8 +246,6 @@ since `SFT_T(KC_A)` is NOT held longer than the `TAPPING_TERM`.
 However, the actual output would be capital `X` (`SHIFT` + `x`) due to reasons
 explained under [Ignore Mod Tap Interrupt](#ignore-mod-tap-interrupt).
 
-
-
 ### Permissive Hold
 
 The “permissive hold” mode can be enabled for all dual-role keys by adding the corresponding option to `config.h`:
@@ -212,7 +277,7 @@ An example of a sequence that is affected by the “permissive hold” mode:
   +---------------------------|--------+
 ```
 
-Normally, if you do all this within the `TAPPING_TERM` (default: 200ms), this will be registered as `al` by the firmware and host system.  With the `PERMISSIVE_HOLD` option enabled, the Layer Tap key is considered as a layer switch if another key is tapped, and the above sequence would be registered as `KC_RGHT` (the mapping of `L` on layer 2). We could describe this sequence as a “nested press” (the modified key's key down and key up events are “nested” between the dual-role key's key down and key up events).
+Normally, if you do all this within the `TAPPING_TERM` (default: 200ms), this will be registered as `al` by the firmware and host system.  With the `PERMISSIVE_HOLD` option enabled, the Layer Tap key is considered as a layer switch if another key is tapped, and the above sequence would be registered as `KC_RGHT` (the mapping of `L` on layer 2). We could describe this sequence as a “nested tap” (the modified key's key down and key up events are “nested” between the dual-role key's key down and key up events).
 
 However, this slightly different sequence will not be affected by the “permissive hold” mode:
 
@@ -235,7 +300,7 @@ However, this slightly different sequence will not be affected by the “permiss
 
 In the sequence above the dual-role key is released before the other key is released, and if that happens within the tapping term, the “permissive hold” mode will still choose the tap action for the dual-role key, and the sequence will be registered as `al` by the host. We could describe this as a “rolling press” (the two keys' key down and key up events behave as if you were rolling a ball across the two keys, first pressing each key down in sequence and then releasing them in the same order).
 
-?> The `PERMISSIVE_HOLD` option also affects Mod Tap keys, but this may not be noticeable if you do not also enable the `IGNORE_MOD_TAP_INTERRUPT` option for those keys, because the default handler for Mod Tap keys also considers both the “nested press” and “rolling press” sequences like shown above as a modifier hold, not the tap action.  If you do not enable `IGNORE_MOD_TAP_INTERRUPT`, the effect of `PERMISSIVE_HOLD` on Mod Tap keys would be limited to reducing the delay before the key events are made visible to the host.
+?> The `PERMISSIVE_HOLD` option is not noticeable if you also enable `HOLD_ON_OTHER_KEY_PRESS` because the latter option considers both the “nested tap” and “rolling press” sequences like shown above as a hold action, not the tap action. `HOLD_ON_OTHER_KEY_PRESS` makes the Tap-Or-Hold decision earlier in the chain of key events, thus taking a precedence over `PERMISSIVE_HOLD`.  This remark also applies to default mod-taps.
 
 For more granular control of this feature, you can add the following to your `config.h`:
 
@@ -291,7 +356,7 @@ An example of a sequence that is affected by the “hold on other key press” m
 
 Normally, if you do all this within the `TAPPING_TERM` (default: 200ms), this will be registered as `al` by the firmware and host system.  With the `HOLD_ON_OTHER_KEY_PRESS` option enabled, the Layer Tap key is considered as a layer switch if another key is pressed, and the above sequence would be registered as `KC_RGHT` (the mapping of `L` on layer 2).
 
-?> The `HOLD_ON_OTHER_KEY_PRESS` option also affects Mod Tap keys, but this may not be noticeable if you do not also enable the `IGNORE_MOD_TAP_INTERRUPT` option for those keys, because the default handler for Mod Tap keys also considers the “rolling press” sequence like shown above as a modifier hold, not the tap action.  If you do not enable `IGNORE_MOD_TAP_INTERRUPT`, the effect of `HOLD_ON_OTHER_KEY_PRESS` on Mod Tap keys would be limited to reducing the delay before the key events are made visible to the host.
+?> The `HOLD_ON_OTHER_KEY_PRESS` option is essentially redundant with the default mod-tap behaviour. The only notable difference is that `HOLD_ON_OTHER_KEY_PRESS` reduces the delay before the key events are made visible to the host.
 
 For more granular control of this feature, you can add the following to your `config.h`:
 
@@ -355,26 +420,32 @@ However, if the `HOLD_ON_OTHER_KEY_PRESS` option is enabled in addition to `IGNO
 For more granular control of this feature, you can add the following to your `config.h`:
 
 ```c
-#define IGNORE_MOD_TAP_INTERRUPT_PER_KEY
+#define HOLD_ON_OTHER_KEY_PRESS_PER_KEY
 ```
 
+?> This option affects *all* dual-role keys.
+
 You can then add the following function to your keymap:
 
 ```c
-bool get_ignore_mod_tap_interrupt(uint16_t keycode, keyrecord_t *record) {
+bool get_hold_on_other_key_press(uint16_t keycode, keyrecord_t *record) {
     switch (keycode) {
         case SFT_T(KC_SPC):
             // Do not force the mod-tap key press to be handled as a modifier
             // if any other key was pressed while the mod-tap key is held down.
-            return true;
+            return false;
         default:
-            // Force the mod-tap key press to be handled as a modifier if any
+            // Force the dual-role key press to be handled as a modifier if any
             // other key was pressed while the mod-tap key is held down.
-            return false;
+            return true;
     }
 }
 ```
 
+Note that you must return `false` in `get_hold_on_other_key_press` in order to apply `IGNORE_MOD_TAP_INTERRUPT` for a certain mod-tap key.
+
+?> `IGNORE_MOD_TAP_INTERRUPT[_PER_KEY]` is being progressively phased out to align the (default) behavior and configuration of mod-taps with the rest of dual-role keys.
+
 ## Quick Tap Term
 
 When the user holds a key after tapping it, the tapping function is repeated by default, rather than activating the hold function. This allows keeping the ability to auto-repeat the tapping function of a dual-role key. `QUICK_TAP_TERM` enables fine tuning of that ability. If set to `0`, it will remove the auto-repeat ability and activate the hold function instead.