Skip to content

Commit 712dd67

Browse files
malpernclaude
andcommitted
refactor: address code review feedback
- Replace Vec::from_iter with .collect() (idiomatic) - Simplify parse_key_list_from_option to plain for loop - Clean up debugging comment in test - Add test for multi-list priority order - Clarify PermissiveHold wording in docs Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent f411ac7 commit 712dd67

File tree

4 files changed

+42
-29
lines changed

4 files changed

+42
-29
lines changed

docs/config.adoc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2076,7 +2076,8 @@ This is useful for home row mods where fast typing should not trigger modifiers.
20762076
Options are: `(tap-on-press <keys...>)` activates `$tap-action` early if a listed key is pressed;
20772077
`(tap-on-press-release <keys...>)` activates `$tap-action` early if a listed key is pressed then released;
20782078
`(hold-on-press <keys...>)` activates `$hold-action` early if a listed key is pressed.
2079-
For any other key, activates `$hold-action` when the key is pressed and released (PermissiveHold behavior).
2079+
For any other key: if that key is pressed and released while this key is waiting,
2080+
`$hold-action` is triggered early (PermissiveHold behavior).
20802081
All list options are optional.
20812082

20822083
| `tap-hold-opposite-hand`

parser/src/cfg/custom_tap_hold.rs

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -140,15 +140,16 @@ pub(crate) fn custom_tap_hold_keys(
140140
keys_hold_on_press: &[OsCode],
141141
a: &Allocations,
142142
) -> &'static CustomTapHoldFn {
143-
let keys_tap_on_press = a.sref_vec(Vec::from_iter(
144-
keys_tap_on_press.iter().copied().map(u16::from),
145-
));
146-
let keys_tap_on_press_release = a.sref_vec(Vec::from_iter(
147-
keys_tap_on_press_release.iter().copied().map(u16::from),
148-
));
149-
let keys_hold_on_press = a.sref_vec(Vec::from_iter(
150-
keys_hold_on_press.iter().copied().map(u16::from),
151-
));
143+
let keys_tap_on_press = a.sref_vec(keys_tap_on_press.iter().copied().map(u16::from).collect());
144+
let keys_tap_on_press_release = a.sref_vec(
145+
keys_tap_on_press_release
146+
.iter()
147+
.copied()
148+
.map(u16::from)
149+
.collect(),
150+
);
151+
let keys_hold_on_press =
152+
a.sref_vec(keys_hold_on_press.iter().copied().map(u16::from).collect());
152153
a.sref(
153154
move |mut queued: QueuedIter, _coord: KCoord| -> (Option<WaitingAction>, bool) {
154155
while let Some(q) = queued.next() {

parser/src/cfg/tap_hold.rs

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -427,18 +427,15 @@ fn parse_key_list_from_option(
427427
kw
428428
);
429429
}
430-
option[1..].iter().try_fold(vec![], |mut keys, key| {
431-
key.atom(s.vars())
432-
.map(|a| -> Result<()> {
433-
keys.push(
434-
str_to_oscode(a)
435-
.ok_or_else(|| anyhow_expr!(key, "string of a known key is expected"))?,
436-
);
437-
Ok(())
438-
})
439-
.ok_or_else(|| {
440-
anyhow_expr!(key, "string of a known key is expected, found list instead")
441-
})??;
442-
Ok(keys)
443-
})
430+
let mut keys = Vec::new();
431+
for key in &option[1..] {
432+
let a = key.atom(s.vars()).ok_or_else(|| {
433+
anyhow_expr!(key, "string of a known key is expected, found list instead")
434+
})?;
435+
keys.push(
436+
str_to_oscode(a)
437+
.ok_or_else(|| anyhow_expr!(key, "string of a known key is expected"))?,
438+
);
439+
}
440+
Ok(keys)
444441
}

src/tests/sim_tests/tap_hold_tests.rs

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -169,11 +169,8 @@ fn tap_hold_keys_tap_on_press_release() {
169169
// tap-on-press-release key needs press+release to trigger tap
170170
let result = simulate(cfg, "d:a t:20 d:b u:b t:100").to_ascii();
171171
assert_eq!("t:20ms dn:X t:6ms dn:B t:1ms up:B", result);
172-
// tap-on-press-release key pressed but not released: no early decision,
173-
// falls through to PermissiveHold which triggers hold on press+release
174-
// Wait, it IS in tap-on-press-release list so it checks for release.
175-
// If not released, falls through to PermissiveHold check which also needs release.
176-
// So just pressing without releasing: no decision until timeout.
172+
// tap-on-press-release key pressed but not released: no early tap or hold,
173+
// waits for timeout.
177174
let result = simulate(cfg, "d:a t:20 d:b t:200").to_ascii();
178175
assert_eq!("t:100ms dn:Y t:1ms dn:B", result);
179176
}
@@ -241,6 +238,23 @@ fn tap_hold_keys_no_options() {
241238
assert_eq!("t:100ms dn:Y t:50ms up:Y", result);
242239
}
243240

241+
#[test]
242+
fn tap_hold_keys_priority_order() {
243+
// When a key appears in multiple lists, tap-on-press wins over hold-on-press.
244+
let cfg = "
245+
(defsrc a b)
246+
(deflayer l1
247+
(tap-hold-keys 100 100 x y
248+
(tap-on-press b)
249+
(hold-on-press b))
250+
b
251+
)
252+
";
253+
// b is in both lists; tap-on-press should win → triggers tap
254+
let result = simulate(cfg, "d:a t:20 d:b t:100").to_ascii();
255+
assert_eq!("t:20ms dn:X t:6ms dn:B", result);
256+
}
257+
244258
#[test]
245259
fn tap_hold_release_keys() {
246260
let cfg = "

0 commit comments

Comments
 (0)