Skip to content

Commit a248108

Browse files
tompassarelliclaude
andcommitted
feat: replace tap-hold-press-gap with tap-hold-release-order
Pure event-driven tap-hold disambiguation with zero parameters. After both keys are down, the first release determines intent: - Other key releases first (modifier still held) → Hold - Modifier releases first (other key still held) → Tap No timers, no thresholds. Decision on the 3rd event only. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 260f794 commit a248108

File tree

5 files changed

+47
-95
lines changed

5 files changed

+47
-95
lines changed

keyberon/src/action.rs

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -61,11 +61,11 @@ pub enum HoldTapConfig<'a> {
6161
/// not used in the flow of typing, like escape for example. If
6262
/// you are annoyed by accidental tap, you can try this behavior.
6363
HoldOnOtherKeyPress,
64-
/// Like HoldOnOtherKeyPress, but requires a minimum gap (in ticks/ms)
65-
/// between the tap-hold keydown and the next keydown before activating hold.
66-
/// If the next key arrives before the gap, resolves as tap instead.
67-
/// This filters fast-typing overlap from intentional modifier holds.
68-
HoldOnOtherKeyPressWithGap(u16),
64+
/// Resolves based on release order after both keys are down.
65+
/// If the other key releases first (modifier still held) → Hold.
66+
/// If the modifier releases first (other key still held) → Tap.
67+
/// No timers, no thresholds — purely event-driven.
68+
ReleaseOrder,
6969
/// If there is a press and release of another key, the hold
7070
/// action is activated.
7171
///
@@ -105,9 +105,7 @@ impl Debug for HoldTapConfig<'_> {
105105
match self {
106106
HoldTapConfig::Default => f.write_str("Default"),
107107
HoldTapConfig::HoldOnOtherKeyPress => f.write_str("HoldOnOtherKeyPress"),
108-
HoldTapConfig::HoldOnOtherKeyPressWithGap(gap) => {
109-
write!(f, "HoldOnOtherKeyPressWithGap({gap})")
110-
}
108+
HoldTapConfig::ReleaseOrder => f.write_str("ReleaseOrder"),
111109
HoldTapConfig::PermissiveHold => f.write_str("PermissiveHold"),
112110
HoldTapConfig::Custom(_) => f.write_str("Custom"),
113111
}
@@ -121,10 +119,7 @@ impl PartialEq for HoldTapConfig<'_> {
121119
(HoldTapConfig::Default, HoldTapConfig::Default)
122120
| (HoldTapConfig::HoldOnOtherKeyPress, HoldTapConfig::HoldOnOtherKeyPress)
123121
| (HoldTapConfig::PermissiveHold, HoldTapConfig::PermissiveHold) => true,
124-
(
125-
HoldTapConfig::HoldOnOtherKeyPressWithGap(a),
126-
HoldTapConfig::HoldOnOtherKeyPressWithGap(b),
127-
) => a == b,
122+
(HoldTapConfig::ReleaseOrder, HoldTapConfig::ReleaseOrder) => true,
128123
_ => false,
129124
}
130125
}

keyberon/src/layout.rs

Lines changed: 30 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -544,12 +544,18 @@ impl<'a, T: std::fmt::Debug> WaitingState<'a, T> {
544544
return Some(WaitingAction::Hold);
545545
}
546546
}
547-
HoldTapConfig::HoldOnOtherKeyPressWithGap(min_gap) => {
548-
if queued.iter().any(|s| s.event.is_press()) {
549-
if self.ticks >= min_gap {
550-
return Some(WaitingAction::Hold);
551-
} else {
552-
return Some(WaitingAction::Tap);
547+
HoldTapConfig::ReleaseOrder => {
548+
// Like PermissiveHold: if another key was pressed AND released
549+
// (while modifier is still held), resolve as Hold.
550+
// If modifier is released first, the fallthrough below handles Tap.
551+
let mut queued = queued.iter();
552+
while let Some(q) = queued.next() {
553+
if q.event.is_press() {
554+
let (i, j) = q.event.coord();
555+
let target = Event::Release(i, j);
556+
if queued.clone().any(|q| q.event == target) {
557+
return Some(WaitingAction::Hold);
558+
}
553559
}
554560
}
555561
}
@@ -2520,28 +2526,25 @@ mod test {
25202526
}
25212527

25222528
#[test]
2523-
fn hold_on_press_with_gap_clean_tap() {
2524-
// Clean tap: press and release the tap-hold key with no other keys.
2525-
// Should emit the tap action (Space).
2529+
fn release_order_clean_tap() {
2530+
// Press and release modifier with no other keys → Tap.
25262531
static LAYERS: Layers<2, 1> = &[[[
25272532
HoldTap(&HoldTapAction {
25282533
on_press_reset_timeout_to: None,
25292534
timeout: u16::MAX,
25302535
hold: k(LAlt),
25312536
timeout_action: k(Space),
25322537
tap: k(Space),
2533-
config: HoldTapConfig::HoldOnOtherKeyPressWithGap(100),
2538+
config: HoldTapConfig::ReleaseOrder,
25342539
tap_hold_interval: 0,
25352540
}),
25362541
k(Enter),
25372542
]]];
25382543
let mut layout = Layout::new(LAYERS);
25392544

2540-
// Press the tap-hold key
25412545
layout.event(Press(0, 0));
25422546
assert_eq!(CustomEvent::NoEvent, layout.tick());
25432547
assert_keys(&[], layout.keycodes());
2544-
// Wait a bit, then release (no other key pressed)
25452548
for _ in 0..50 {
25462549
assert_eq!(CustomEvent::NoEvent, layout.tick());
25472550
assert_keys(&[], layout.keycodes());
@@ -2554,116 +2557,71 @@ mod test {
25542557
}
25552558

25562559
#[test]
2557-
fn hold_on_press_with_gap_fast_typing_overlap() {
2558-
// Fast typing overlap: another key pressed within min_gap (100ms).
2559-
// Should resolve as tap: emit Space then Enter.
2560+
fn release_order_hold() {
2561+
// Modifier down → other down → other up first → Hold.
25602562
static LAYERS: Layers<2, 1> = &[[[
25612563
HoldTap(&HoldTapAction {
25622564
on_press_reset_timeout_to: None,
25632565
timeout: u16::MAX,
25642566
hold: k(LAlt),
25652567
timeout_action: k(Space),
25662568
tap: k(Space),
2567-
config: HoldTapConfig::HoldOnOtherKeyPressWithGap(100),
2569+
config: HoldTapConfig::ReleaseOrder,
25682570
tap_hold_interval: 0,
25692571
}),
25702572
k(Enter),
25712573
]]];
25722574
let mut layout = Layout::new(LAYERS);
25732575

2574-
// Press the tap-hold key
25752576
layout.event(Press(0, 0));
25762577
assert_eq!(CustomEvent::NoEvent, layout.tick());
25772578
assert_keys(&[], layout.keycodes());
2578-
// Wait only 30ms, then press another key (within the 100ms gap)
2579-
for _ in 0..29 {
2580-
assert_eq!(CustomEvent::NoEvent, layout.tick());
2581-
assert_keys(&[], layout.keycodes());
2582-
}
25832579
layout.event(Press(0, 1));
2584-
// Next tick: gap is 30 ticks < 100 min_gap, so resolves as Tap
2585-
assert_eq!(CustomEvent::NoEvent, layout.tick());
2586-
assert_keys(&[Space], layout.keycodes());
2587-
// Buffered Enter gets dequeued
2588-
assert_eq!(CustomEvent::NoEvent, layout.tick());
2589-
assert_keys(&[Space, Enter], layout.keycodes());
2590-
}
2591-
2592-
#[test]
2593-
fn hold_on_press_with_gap_intentional_hold() {
2594-
// Intentional hold: another key pressed after min_gap (100ms).
2595-
// Should resolve as hold: activate LAlt, then Enter is processed with LAlt active.
2596-
static LAYERS: Layers<2, 1> = &[[[
2597-
HoldTap(&HoldTapAction {
2598-
on_press_reset_timeout_to: None,
2599-
timeout: u16::MAX,
2600-
hold: k(LAlt),
2601-
timeout_action: k(Space),
2602-
tap: k(Space),
2603-
config: HoldTapConfig::HoldOnOtherKeyPressWithGap(100),
2604-
tap_hold_interval: 0,
2605-
}),
2606-
k(Enter),
2607-
]]];
2608-
let mut layout = Layout::new(LAYERS);
2609-
2610-
// Press the tap-hold key
2611-
layout.event(Press(0, 0));
26122580
assert_eq!(CustomEvent::NoEvent, layout.tick());
26132581
assert_keys(&[], layout.keycodes());
2614-
// Wait 150ms (past the 100ms gap threshold)
2615-
for _ in 0..149 {
2616-
assert_eq!(CustomEvent::NoEvent, layout.tick());
2617-
assert_keys(&[], layout.keycodes());
2618-
}
2619-
layout.event(Press(0, 1));
2620-
// Next tick: gap is 150 ticks >= 100 min_gap, so resolves as Hold
2582+
// Other key releases first → Hold
2583+
layout.event(Release(0, 1));
26212584
assert_eq!(CustomEvent::NoEvent, layout.tick());
26222585
assert_keys(&[LAlt], layout.keycodes());
2623-
// Buffered Enter gets dequeued
26242586
assert_eq!(CustomEvent::NoEvent, layout.tick());
26252587
assert_keys(&[LAlt, Enter], layout.keycodes());
2626-
// Release both
2627-
layout.event(Release(0, 1));
26282588
assert_eq!(CustomEvent::NoEvent, layout.tick());
26292589
assert_keys(&[LAlt], layout.keycodes());
2590+
// Release modifier
26302591
layout.event(Release(0, 0));
26312592
assert_eq!(CustomEvent::NoEvent, layout.tick());
26322593
assert_keys(&[], layout.keycodes());
26332594
}
26342595

26352596
#[test]
2636-
fn hold_on_press_with_gap_exact_boundary() {
2637-
// Exactly at min_gap: should resolve as hold (>= check).
2597+
fn release_order_tap() {
2598+
// Modifier down → other down → modifier up first → Tap.
26382599
static LAYERS: Layers<2, 1> = &[[[
26392600
HoldTap(&HoldTapAction {
26402601
on_press_reset_timeout_to: None,
26412602
timeout: u16::MAX,
26422603
hold: k(LAlt),
26432604
timeout_action: k(Space),
26442605
tap: k(Space),
2645-
config: HoldTapConfig::HoldOnOtherKeyPressWithGap(100),
2606+
config: HoldTapConfig::ReleaseOrder,
26462607
tap_hold_interval: 0,
26472608
}),
26482609
k(Enter),
26492610
]]];
26502611
let mut layout = Layout::new(LAYERS);
26512612

2652-
// Press the tap-hold key
26532613
layout.event(Press(0, 0));
26542614
assert_eq!(CustomEvent::NoEvent, layout.tick());
26552615
assert_keys(&[], layout.keycodes());
2656-
// Wait exactly 100 ticks
2657-
for _ in 0..99 {
2658-
assert_eq!(CustomEvent::NoEvent, layout.tick());
2659-
assert_keys(&[], layout.keycodes());
2660-
}
26612616
layout.event(Press(0, 1));
2662-
// Next tick: gap is exactly 100 ticks == 100 min_gap, resolves as Hold
26632617
assert_eq!(CustomEvent::NoEvent, layout.tick());
2664-
assert_keys(&[LAlt], layout.keycodes());
2618+
assert_keys(&[], layout.keycodes());
2619+
// Modifier releases first → Tap
2620+
layout.event(Release(0, 0));
26652621
assert_eq!(CustomEvent::NoEvent, layout.tick());
2666-
assert_keys(&[LAlt, Enter], layout.keycodes());
2622+
assert_keys(&[Space], layout.keycodes());
2623+
assert_eq!(CustomEvent::NoEvent, layout.tick());
2624+
assert_keys(&[Space, Enter], layout.keycodes());
26672625
}
26682626

26692627
#[test]

parser/src/cfg/list_actions.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ pub const CLIPBOARD_RESTORE: &str = "clipboard-restore";
136136
pub const CLIPBOARD_SAVE_SET: &str = "clipboard-save-set";
137137
pub const CLIPBOARD_SAVE_CMD_SET: &str = "clipboard-save-cmd-set";
138138
pub const CLIPBOARD_SAVE_SWAP: &str = "clipboard-save-swap";
139-
pub const TAP_HOLD_PRESS_GAP: &str = "tap-hold-press-gap";
139+
pub const TAP_HOLD_RELEASE_ORDER: &str = "tap-hold-release-order";
140140
pub const TAP_HOLD_OPPOSITE_HAND: &str = "tap-hold-opposite-hand";
141141

142142
pub fn is_list_action(ac: &str) -> bool {
@@ -273,7 +273,7 @@ pub fn is_list_action(ac: &str) -> bool {
273273
CLIPBOARD_SAVE_SET,
274274
CLIPBOARD_SAVE_CMD_SET,
275275
CLIPBOARD_SAVE_SWAP,
276-
TAP_HOLD_PRESS_GAP,
276+
TAP_HOLD_RELEASE_ORDER,
277277
TAP_HOLD_OPPOSITE_HAND,
278278
];
279279
LIST_ACTIONS.contains(&ac)

parser/src/cfg/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1508,7 +1508,7 @@ fn parse_action_list(ac: &[SExpr], s: &ParserState) -> Result<&'static KanataAct
15081508
TAP_HOLD_PRESS | TAP_HOLD_PRESS_A => {
15091509
parse_tap_hold(&ac[1..], s, HoldTapConfig::HoldOnOtherKeyPress)
15101510
}
1511-
TAP_HOLD_PRESS_GAP => parse_tap_hold_press_gap(&ac[1..], s),
1511+
TAP_HOLD_RELEASE_ORDER => parse_tap_hold_release_order(&ac[1..], s),
15121512
TAP_HOLD_RELEASE | TAP_HOLD_RELEASE_A => {
15131513
parse_tap_hold(&ac[1..], s, HoldTapConfig::PermissiveHold)
15141514
}

parser/src/cfg/tap_hold.rs

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -94,26 +94,25 @@ pub(crate) fn parse_tap_hold_timeout(
9494
}))))
9595
}
9696

97-
pub(crate) fn parse_tap_hold_press_gap(
97+
pub(crate) fn parse_tap_hold_release_order(
9898
ac_params: &[SExpr],
9999
s: &ParserState,
100100
) -> Result<&'static KanataAction> {
101-
if ac_params.len() != 3 {
101+
if ac_params.len() != 2 {
102102
bail!(
103-
r"tap-hold-press-gap expects 3 items after it, got {}.
103+
r"tap-hold-release-order expects 2 items after it, got {}.
104104
Params in order:
105-
<min-gap-ms> <tap-action> <hold-action>",
105+
<tap-action> <hold-action>",
106106
ac_params.len(),
107107
)
108108
}
109-
let min_gap = parse_non_zero_u16(&ac_params[0], s, "min gap threshold")?;
110-
let tap_action = parse_action(&ac_params[1], s)?;
111-
let hold_action = parse_action(&ac_params[2], s)?;
109+
let tap_action = parse_action(&ac_params[0], s)?;
110+
let hold_action = parse_action(&ac_params[1], s)?;
112111
if matches!(tap_action, Action::HoldTap { .. }) {
113112
bail!("tap-hold does not work in the tap-action of tap-hold")
114113
}
115114
Ok(s.a.sref(Action::HoldTap(s.a.sref(HoldTapAction {
116-
config: HoldTapConfig::HoldOnOtherKeyPressWithGap(min_gap),
115+
config: HoldTapConfig::ReleaseOrder,
117116
tap_hold_interval: 0,
118117
timeout: u16::MAX,
119118
tap: *tap_action,

0 commit comments

Comments
 (0)