Skip to content

Commit 56775b1

Browse files
authored
fix: tap-hold-opposite-hand-release skips same-hand keys when held (#2017)
When I used the `tap-hold-opposite-hand-release` and pressed `f`+`d` for ctrl and shift modifiers and then pressed `j`, it outputted ctrl+d and ignored the `f`. ## Problem `tap-hold-opposite-hand-release` with `(same-hand tap)` incorrectly resolves as Hold when two same-hand HRM keys are pressed together followed by an opposite-hand key. **Reproduction:** Given left-hand HRM keys `f`→ctrl and `d`→shift, and right-hand key `j`: ``` d:f t:5 d:d t:20 d:j t:20 u:j ``` **Expected:** `f` resolves as Tap (same-hand `d` was pressed) **Actual:** `f` resolves as Hold(ctrl), outputting ctrl+d ## Root cause The `-release` variant checks for both press+release in the queue **before** checking which hand a key belongs to: ```rust // Wait for the interrupting key's release before deciding. let release = Event::Release(i, j); if !queued.clone().copied().any(|q| q.event() == release) { continue; // ← same-hand keys skipped when still held! } // hand check happens here, too late ``` When `f` is waiting and the queue contains `[d↓, j↓, j↑]`: 1. `d↓` — looks for `d↑` — **not found** (d still held) → `continue` (skipped) 2. `j↓` — looks for `j↑` — **found** → opposite hand → **Hold** The same-hand key `d` is invisible because it hasn't been released yet. ## Fix Move the release check **after** the hand check. Only opposite-hand, neutral, and unknown-hand keys require press+release. Same-hand keys resolve immediately on press — matching `tap-hold-opposite-hand` (non-release variant) behavior for same-hand decisions. This is correct because the `-release` requirement exists to prevent misfires on fast **cross-hand** overlaps (e.g., `f↓ j↓ f↑ j↑` should be `fj`, not `ctrl+j`). Same-hand keys don't need this protection — they already resolve as Tap structurally.
1 parent 166551e commit 56775b1

File tree

2 files changed

+63
-19
lines changed

2 files changed

+63
-19
lines changed

parser/src/cfg/custom_tap_hold.rs

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -294,8 +294,14 @@ pub(crate) fn custom_tap_hold_opposite_hand(
294294
}
295295

296296
/// Like `custom_tap_hold_opposite_hand` but waits for the interrupting key's
297-
/// press+release before committing. This avoids misfires on fast same-hand
298-
/// rolls where keystrokes briefly overlap.
297+
/// press+release before committing to Hold. This avoids misfires on fast
298+
/// cross-hand overlaps where keystrokes briefly overlap.
299+
///
300+
/// The `-release` requirement only applies to opposite-hand and unknown-hand
301+
/// keys. Same-hand keys resolve immediately on press (no release needed),
302+
/// because requiring release would cause same-hand keys to be skipped when
303+
/// still held, allowing a later opposite-hand key+release to incorrectly
304+
/// trigger Hold.
299305
pub(crate) fn custom_tap_hold_opposite_hand_release(
300306
hand_map: &'static HandMap,
301307
same_hand: DecisionBehavior,
@@ -318,15 +324,14 @@ pub(crate) fn custom_tap_hold_opposite_hand_release(
318324
continue;
319325
}
320326

321-
// Wait for the interrupting key's release before deciding.
322-
let release = Event::Release(i, j);
323-
if !queued.clone().copied().any(|q| q.event() == release) {
324-
continue;
325-
}
326-
327327
// Check neutral-keys first (takes precedence over defhands)
328328
if let Some(osc) = OsCode::from_u16(j) {
329329
if neutral_keys.contains(&osc) {
330+
// Neutral keys require release before deciding
331+
let release = Event::Release(i, j);
332+
if !queued.clone().copied().any(|q| q.event() == release) {
333+
continue;
334+
}
330335
match neutral_behavior {
331336
DecisionBehavior::Tap => return (Some(WaitingAction::Tap), false),
332337
DecisionBehavior::Hold => return (Some(WaitingAction::Hold), false),
@@ -338,16 +343,29 @@ pub(crate) fn custom_tap_hold_opposite_hand_release(
338343
let pressed_hand = hand_map.get(j);
339344

340345
match (waiting_hand, pressed_hand) {
341-
(Hand::Left, Hand::Right) | (Hand::Right, Hand::Left) => {
342-
return (Some(WaitingAction::Hold), false);
343-
}
346+
// Same hand: resolve immediately on press (no release needed).
347+
// This prevents same-hand keys from being skipped while held,
348+
// which would let a later opposite-hand release trigger Hold.
344349
(Hand::Left, Hand::Left) | (Hand::Right, Hand::Right) => match same_hand {
345350
DecisionBehavior::Tap => return (Some(WaitingAction::Tap), false),
346351
DecisionBehavior::Hold => return (Some(WaitingAction::Hold), false),
347352
DecisionBehavior::Ignore => continue,
348353
},
354+
// Opposite hand: require release before committing to Hold
355+
(Hand::Left, Hand::Right) | (Hand::Right, Hand::Left) => {
356+
let release = Event::Release(i, j);
357+
if !queued.clone().copied().any(|q| q.event() == release) {
358+
continue;
359+
}
360+
return (Some(WaitingAction::Hold), false);
361+
}
349362
_ => {
350-
// At least one key is Neutral (not in defhands)
363+
// At least one key is Neutral (not in defhands):
364+
// require release before deciding
365+
let release = Event::Release(i, j);
366+
if !queued.clone().copied().any(|q| q.event() == release) {
367+
continue;
368+
}
351369
match unknown_hand {
352370
DecisionBehavior::Tap => return (Some(WaitingAction::Tap), false),
353371
DecisionBehavior::Hold => return (Some(WaitingAction::Hold), false),

src/tests/sim_tests/tap_hold_tests.rs

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -301,14 +301,14 @@ fn tap_hold_opposite_hand_release_same_hand_tap() {
301301
s j
302302
)
303303
";
304-
// Same-hand key pressed + released → tap
304+
// Same-hand key pressed + released → tap (resolves on press, not release)
305305
let result = simulate(cfg, "d:a t:20 d:s t:20 u:s t:100").to_ascii();
306-
assert_eq!("t:40ms dn:X t:6ms dn:S t:1ms up:S", result);
306+
assert_eq!("t:20ms dn:X t:6ms dn:S t:14ms up:S", result);
307307

308-
// Same-hand key pressed but NOT released → no decision (waits for release)
309-
// Eventually times out → timeout action (tap by default)
308+
// Same-hand key pressed but NOT released → tap immediately on press
309+
// (same-hand doesn't require release, only opposite-hand does)
310310
let result = simulate(cfg, "d:a t:20 d:s t:250").to_ascii();
311-
assert_eq!("t:200ms dn:X t:1ms dn:S", result);
311+
assert_eq!("t:20ms dn:X t:6ms dn:S", result);
312312
}
313313

314314
#[test]
@@ -400,7 +400,7 @@ fn tap_hold_opposite_hand_release_with_require_prior_idle() {
400400

401401
#[test]
402402
fn tap_hold_opposite_hand_release_same_hand_hold() {
403-
// (same-hand hold) should trigger hold when same-hand key is pressed+released
403+
// (same-hand hold) should trigger hold immediately on press (no release needed)
404404
let cfg = "
405405
(defhands
406406
(left a s d f)
@@ -412,7 +412,33 @@ fn tap_hold_opposite_hand_release_same_hand_hold() {
412412
)
413413
";
414414
let result = simulate(cfg, "d:a t:20 d:s t:20 u:s t:100").to_ascii();
415-
assert_eq!("t:40ms dn:Y t:6ms dn:S t:1ms up:S", result);
415+
assert_eq!("t:20ms dn:Y t:6ms dn:S t:14ms up:S", result);
416+
}
417+
418+
#[test]
419+
fn tap_hold_opposite_hand_release_same_hand_then_opposite() {
420+
// Regression test: pressing two same-hand HRM keys together then an
421+
// opposite-hand key should resolve the first as tap (via same-hand),
422+
// not as hold (via opposite-hand release).
423+
// Bug: the -release variant required ALL keys to have press+release before
424+
// considering them, so same-hand keys were skipped if still held, and the
425+
// opposite-hand key's release incorrectly triggered Hold.
426+
let cfg = "
427+
(defcfg concurrent-tap-hold yes process-unmapped-keys yes)
428+
(defhands
429+
(left a s d f)
430+
(right j k l ;))
431+
(defsrc d f j)
432+
(deflayer l1
433+
(tap-hold-opposite-hand-release 500 d lsft (same-hand tap) (timeout tap))
434+
(tap-hold-opposite-hand-release 500 f lctl (same-hand tap) (timeout tap))
435+
j
436+
)
437+
";
438+
// f↓ d↓ j↓ j↑ → f sees d (same-hand) → tap immediately; d sees j (opposite+release) → hold
439+
// Result: f (tap) at t=5, then lsft+j when j is released at t=45
440+
let result = simulate(cfg, "d:f t:5 d:d t:20 d:j t:20 u:j t:100").to_ascii();
441+
assert_eq!("t:5ms dn:F t:40ms dn:LShift t:6ms dn:J t:1ms up:J", result);
416442
}
417443

418444
#[test]

0 commit comments

Comments
 (0)