Repository: jtroo/kanata (rust)

Issue:
Behavior of "concurrent-tap-hold yes" seems incorrect
Hello everyone. I am new to kanata and am currently optimizing my configuration. I noticed a behavior that I would like to ask you if it is intended and correct or if it is perhaps a bug.

Since I wanted to use `defchordsv2`, I had to set `concurrent-tap-hold` to `yes`. However, this setting affects the behavior when using "concurrent" `tap-hold`s in a way that is undesirable for me.

The following configuration clarifies my question:

```
(defcfg
 concurrent-tap-hold yes 
)

(defsrc
 a j 
)

(deflayer base
 @a @j
)

(defalias
 a (tap-hold 200 1000 a lctl)
 j (tap-hold 200 500 j lsft)
)
```

If I now hold down the `a` key for over 1000 ms and only tap `j` briefly, `lsft` is still triggered. According to the documentation, the timeout for the second `tap-hold` action should start immediately when `concurrent-tap-hold` is activated. However, it looks as if the timeout for this second action is not waited for at all, but the `$hold-action` is executed immediately. Do you have an explanation for this and any tips on how I can avoid this behavior?

_Originally posted by @smoky19x in https://github.com/jtroo/kanata/discussions/1671_


Patch:
diff --git a/keyberon/src/layout.rs b/keyberon/src/layout.rs
index 9f8ab73fc..e9996ee0f 100644
--- a/keyberon/src/layout.rs
+++ b/keyberon/src/layout.rs
@@ -433,6 +433,7 @@ enum WaitingConfig<'a, T: 'a + std::fmt::Debug> {
 pub struct WaitingState<'a, T: 'a + std::fmt::Debug> {
     coord: KCoord,
     timeout: u16,
+    original_timeout: u16,
     delay: u16,
     ticks: u16,
     hold: &'a Action<'a, T>,
@@ -535,11 +536,17 @@ impl<'a, T: std::fmt::Debug> WaitingState<'a, T> {
                 skip_timeout = local_skip;
             }
         }
-        if let Some(&Queued { since, .. }) = queued
+        if let Some(&Queued {
+            since: since_release,
+            ..
+        }) = queued
             .iter()
             .find(|s| self.is_corresponding_release(&s.event))
         {
-            if self.timeout > self.delay.saturating_sub(since) {
+            let gap_between_press_and_release = self.delay.saturating_sub(since_release);
+            if self.original_timeout > 0 && self.original_timeout >= gap_between_press_and_release
+                || self.timeout > gap_between_press_and_release
+            {
                 Some(WaitingAction::Tap)
             } else {
                 Some(WaitingAction::Timeout)
@@ -1673,6 +1680,10 @@ impl<'a, const C: usize, const R: usize, T: 'a + Copy + std::fmt::Debug> Layout<
                             delay
                         },
                         ticks: 0,
+                        original_timeout: match self.quick_tap_hold_timeout {
+                            true => *timeout,
+                            false => 0,
+                        },
                         hold,
                         tap,
                         timeout_action,
@@ -1727,6 +1738,7 @@ impl<'a, const C: usize, const R: usize, T: 'a + Copy + std::fmt::Debug> Layout<
                             hold: &Action::NoOp,
                             tap: &Action::NoOp,
                             timeout_action: &Action::NoOp,
+                            original_timeout: 0,
                             config: WaitingConfig::TapDance(TapDanceState {
                                 actions: td.actions,
                                 timeout: td.timeout,
@@ -1773,6 +1785,7 @@ impl<'a, const C: usize, const R: usize, T: 'a + Copy + std::fmt::Debug> Layout<
                     hold: &Action::NoOp,
                     tap: &Action::NoOp,
                     timeout_action: &Action::NoOp,
+                    original_timeout: 0,
                     config: WaitingConfig::Chord(chords),
                     layer_stack: layer_stack.collect(),
                     prev_queue_len: QueueLen::MAX,


Test patch:
diff --git a/src/tests/sim_tests/chord_sim_tests.rs b/src/tests/sim_tests/chord_sim_tests.rs
index 984ca4e79..9a28ceb78 100644
--- a/src/tests/sim_tests/chord_sim_tests.rs
+++ b/src/tests/sim_tests/chord_sim_tests.rs
@@ -232,11 +232,11 @@ static CHORD_INTO_TAP_HOLD_CFG: &str = "\
 fn sim_chord_into_tap_hold() {
     let result = simulate(
         CHORD_INTO_TAP_HOLD_CFG,
-        "d:a t:50 d:b t:149 u:a u:b t:5 \
-         d:a t:50 d:b t:148 u:a u:b t:1000",
+        "d:a t:50 d:b t:150 u:a u:b t:5 \
+         d:a t:50 d:b t:149 u:a u:b t:1000",
     );
     assert_eq!(
-        "t:199ms\nout:↓Y\nt:10ms\nout:↑Y\nt:193ms\nout:↓X\nt:10ms\nout:↑X",
+        "t:199ms\nout:↓Y\nt:10ms\nout:↑Y\nt:195ms\nout:↓X\nt:10ms\nout:↑X",
         result
     );
 }
diff --git a/src/tests/sim_tests/mod.rs b/src/tests/sim_tests/mod.rs
index 2880b5f58..2e2af2d81 100644
--- a/src/tests/sim_tests/mod.rs
+++ b/src/tests/sim_tests/mod.rs
@@ -24,6 +24,7 @@ mod release_sim_tests;
 mod repeat_sim_tests;
 mod seq_sim_tests;
 mod switch_sim_tests;
+mod tap_hold_tests;
 mod template_sim_tests;
 mod timing_tests;
 mod unicode_sim_tests;
diff --git a/src/tests/sim_tests/tap_hold_tests.rs b/src/tests/sim_tests/tap_hold_tests.rs
new file mode 100644
index 000000000..020d711f4
--- /dev/null
+++ b/src/tests/sim_tests/tap_hold_tests.rs
@@ -0,0 +1,18 @@
+use super::*;
+
+#[test]
+fn nested_template() {
+    let result = simulate(
+        "
+        (defcfg concurrent-tap-hold yes )
+        (defsrc a j )
+        (deflayer base @a @j)
+        (defalias
+         a (tap-hold 200 1000 a lctl)
+         j (tap-hold 200 500 j lsft))
+        ",
+        "d:a t:100 d:j t:10 u:j t:1100 u:a t:50",
+    )
+    .to_ascii();
+    assert_eq!("t:999ms dn:LCtrl t:2ms dn:J t:6ms up:J t:203ms up:LCtrl", result);
+}