From 887b7edb706e3bafdbc5deeb3368f6fd73670628 Mon Sep 17 00:00:00 2001 From: Jonathan Davies Date: Fri, 7 Aug 2026 10:42:38 +0000 Subject: [PATCH] fix(keyboard): consume one-shot modifier on next key press, not release A key rolled over before the first shifted key released still saw `Single`, so it got shifted too. Consume on press instead; release reports never included `Single` anyway, so the shifted key is unaffected. A held OSL now also consumes a pending OSM at its own press, matching the already-tapped case in `osm_then_osl`. --- docs/docs/main/docs/configuration/behavior.md | 4 +-- rmk/src/keyboard/oneshot.rs | 10 +++--- rmk/tests/scenarios/one_shot.toml | 35 +++++++++++++++++++ 3 files changed, 41 insertions(+), 8 deletions(-) diff --git a/docs/docs/main/docs/configuration/behavior.md b/docs/docs/main/docs/configuration/behavior.md index 5b69af996..7d8c6a777 100644 --- a/docs/docs/main/docs/configuration/behavior.md +++ b/docs/docs/main/docs/configuration/behavior.md @@ -40,8 +40,8 @@ If you press One-Shot Modifier again, it will be sent as a normal modifier key p The `quick_release` option controls when the one-shot modifier is released: -- `false` (default): the modifier is released when the next key is **released** (chain mode, equivalent to ZMK `&skn`). The modifier stays active for the entire duration of the next keypress, including key repeat. -- `true`: the modifier is released when the next key is **pressed** (equivalent to ZMK `&skq`). Only the initial press of the next key is modified; key repeat will not include the modifier. +- `false` (default): the modifier is included in the next key's press report and stays part of that report for as long as the key is held, including key repeat (chain mode, equivalent to ZMK `&skn`). No separate report is sent when the key is released. +- `true`: an extra report is sent right after the next key's press with the modifier removed (equivalent to ZMK `&skq`). Only the initial press of the next key is modified; key repeat will not include the modifier. Default values: diff --git a/rmk/src/keyboard/oneshot.rs b/rmk/src/keyboard/oneshot.rs index aa8b1b75f..80d035c24 100644 --- a/rmk/src/keyboard/oneshot.rs +++ b/rmk/src/keyboard/oneshot.rs @@ -163,17 +163,15 @@ impl<'a> Keyboard<'a> { /// Update OSM state based on the keyboard event. /// Returns `true` if the OSM was consumed (transitioned from Single to None). pub(crate) fn update_osm(&mut self, event: KeyboardEvent) -> bool { - let quick_release = self.keymap.one_shot_modifiers_config().quick_release; match self.osm_state { OneShotState::Initial(m) => { self.osm_state = OneShotState::Held(m); false } - OneShotState::Single(_) if quick_release && event.pressed => { - self.osm_state = OneShotState::None; - true - } - OneShotState::Single(_) if !quick_release && !event.pressed => { + // Consume on press so a key rolled over before this one releases doesn't + // also see the modifier (resolve_explicit_modifiers only applies `Single` + // on the pressed report anyway, so the release report is unaffected). + OneShotState::Single(_) if event.pressed => { self.osm_state = OneShotState::None; true } diff --git a/rmk/tests/scenarios/one_shot.toml b/rmk/tests/scenarios/one_shot.toml index f5022bcac..7351834e9 100644 --- a/rmk/tests/scenarios/one_shot.toml +++ b/rmk/tests/scenarios/one_shot.toml @@ -52,6 +52,25 @@ expect = [ [], ] +# Rollover: a second key goes down before the first shifted key comes up. +# `update_osm` in oneshot.rs consumes the one-shot state on that first key's +# press, so a key rolled over before the release doesn't also get shifted. +[[test]] +name = "osm_rollover_second_key_not_shifted" +steps = [ + { tap = { pos = [0, 0], duration = 10 } }, # OSM(LShift) + { press = [0, 2] }, # Press A + { press = [1, 3] }, # Press W before A is released (rollover) + { release = [0, 2] }, + { release = [1, 3] }, +] +expect = [ + ["LShift", "A"], # A gets the one-shot modifier + ["A", "W"], # W does not, even though A is still held + ["W"], + [], +] + # OSM released while the tap-hold key is still down: the tap still carries it. [[test]] name = "osm_rolling_with_tap_hold" @@ -160,6 +179,22 @@ steps = [ ] expect = [["C"], []] +# A held (not tapped) OSL consumes a pending OSM at the OSL's own press, same +# as the tapped case above. The key typed under the held layer must not see +# the modifier either. +[[test]] +name = "osm_then_held_osl" +steps = [ + { tap = { pos = [0, 0], duration = 10 } }, # OSM(LShift) + { press = [0, 1] }, # Press OSL(1), held rather than tapped + { tap = { pos = [0, 2], duration = 10 } }, # C from layer 1 + { release = [0, 1] }, # Release OSL(1) +] +expect = [ + ["C"], # OSM already consumed by the OSL press + [], +] + # OSL first, so (0,0) resolves to layer 1's OSM(LShift | LCtrl); the layer is # spent by then, so the next key comes from layer 0. [[test]]