Skip to content

Commit d52d321

Browse files
committed
fix(morse): keep a continuation breadcrumb after a flow-tapped tap
A morse/tap-hold key resolved as a tap by flow-tap fired its tap action and then, on release, removed itself from the held buffer, leaving no trace. A subsequent press-and-hold of the same key was therefore a brand-new press and resolved to the plain hold action instead of the hold-after-tap continuation, so e.g. a "tap, then press-and-hold to repeat the tap action" gesture only worked when the first tap had been resolved by the early-fire path (which does leave an EarlyFired breadcrumb), not by flow-tap. Whether flow-tap kicked in depends on how recently other keys were pressed, so the behaviour was inconsistent. Give flow-tapped entries their own held-buffer state (FlowTapped) and, on release, downgrade them to EarlyFired when the key has a hold-after-tap action, exactly like the early-fire path. Plain tap-hold keys gain nothing from a breadcrumb so they are just removed as before. Adds a regression test.
1 parent cf611fb commit d52d321

4 files changed

Lines changed: 66 additions & 1 deletion

File tree

rmk/src/keyboard.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -450,7 +450,7 @@ impl<'a> Keyboard<'a> {
450450
self.held_buffer.push(HeldKey::new(
451451
event,
452452
*key_action,
453-
KeyState::ProcessedButReleaseNotReportedYet(action),
453+
KeyState::FlowTapped(action),
454454
now,
455455
time_out,
456456
));

rmk/src/keyboard/held_buffer.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,11 @@ pub enum KeyState {
110110
/// remains in the buffer to allow hold_after_tap continuation.
111111
EarlyFired(MorsePattern),
112112

113+
/// After flow-tap resolved the key as a tap: the tap action's press HID report
114+
/// is sent and held while the key is physically held. On release the action is
115+
/// released and the key is kept as `EarlyFired` so hold_after_tap can continue.
116+
FlowTapped(Action),
117+
113118
/// The corresponding action is already executed (so the Pressed HID report is sent),
114119
/// but the release HID report is not sent yet (will be sent only when the corresponding
115120
/// key is really released).

rmk/src/keyboard/morse.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,26 @@ impl<'a> Keyboard<'a> {
192192
debug!("[morse] Releasing morse key: {:?}", event);
193193
self.process_key_action_normal(action, event).await;
194194
}
195+
KeyState::FlowTapped(action) => {
196+
// Flow-tap fired the tap action and is holding it down; release it now.
197+
debug!("[morse] Releasing flow-tapped morse key: {:?}", event);
198+
self.process_key_action_normal(action, event).await;
199+
// If the key has a hold-after-tap action, keep it in the buffer as if it
200+
// had been early-fired so a re-press within the gap timeout continues into
201+
// hold-after-tap (the tap-then-hold repeat). Without this a flow-tapped
202+
// tap leaves no trace and the next press-and-hold resolves as a fresh hold.
203+
if Self::action_from_pattern(self.keymap, key_action, TAP.followed_by_hold()) != Action::No {
204+
let now = Instant::now();
205+
let timeout = Self::morse_timeout(self.keymap, key_action, false);
206+
if let Some(k) = self.held_buffer.find_pos_mut(event.pos) {
207+
k.state = KeyState::EarlyFired(TAP);
208+
k.press_time = now;
209+
k.timeout_time = now + timeout;
210+
}
211+
} else {
212+
let _ = self.held_buffer.remove(event.pos);
213+
}
214+
}
195215
_ => {}
196216
};
197217
}

rmk/tests/keyboard_morse_tap_dance_test.rs

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -650,3 +650,43 @@ fn test_flow_tap_after_early_fire_does_not_jam() {
650650
]
651651
};
652652
}
653+
654+
/// Regression test: a tap resolved by flow-tap (e.g. right after a burst of typing) must
655+
/// still allow a hold-after-tap continuation, so press-and-hold after that tap repeats the
656+
/// tap action instead of resolving as a fresh hold.
657+
///
658+
/// Before the fix, flow-tap fired the tap and removed the key from the held buffer on
659+
/// release, leaving no trace. A subsequent press-and-hold was therefore a brand-new press
660+
/// and resolved to the hold action (RShift here) instead of hold-after-tap (Backspace). The
661+
/// early-fire path did not have this problem because it leaves an EarlyFired breadcrumb; the
662+
/// fix makes flow-tapped taps leave the same breadcrumb when a hold-after-tap action exists.
663+
#[test]
664+
fn test_flow_tapped_tap_then_hold_after_tap() {
665+
key_sequence_test! {
666+
keyboard: create_flow_tap_early_fire_keyboard(),
667+
sequence: [
668+
// Type A, then tap td!(0) within prior_idle_time so the tap is resolved by flow-tap.
669+
[0, 1, true, 200],
670+
[0, 1, false, 30],
671+
[0, 0, true, 50],
672+
[0, 0, false, 30],
673+
// Re-press td!(0) within the gap timeout and hold past the hold timeout.
674+
// With the fix this continues into hold-after-tap (Backspace held); before it
675+
// resolved as a fresh hold (RShift).
676+
[0, 0, true, 150],
677+
[0, 0, false, 400],
678+
],
679+
expected_reports: [
680+
// Type A.
681+
[0, [kc_to_u8!(A), 0, 0, 0, 0, 0]],
682+
[0, [0, 0, 0, 0, 0, 0]],
683+
// Flow-tapped tap: Backspace press (held) then release on key-up.
684+
[0, [kc_to_u8!(Backspace), 0, 0, 0, 0, 0]],
685+
[0, [0, 0, 0, 0, 0, 0]],
686+
// Re-press held: hold-after-tap fires Backspace (held), released on key-up.
687+
// RShift would mean the continuation breadcrumb was lost.
688+
[0, [kc_to_u8!(Backspace), 0, 0, 0, 0, 0]],
689+
[0, [0, 0, 0, 0, 0, 0]]
690+
]
691+
};
692+
}

0 commit comments

Comments
 (0)