Skip to content

keymap: keep idle and time at physical press for nested pending - #707

Merged
rgoulter merged 3 commits into
masterfrom
fix-idle-timeout
Aug 22, 2026
Merged

keymap: keep idle and time at physical press for nested pending#707
rgoulter merged 3 commits into
masterfrom
fix-idle-timeout

Conversation

@rgoulter

Copy link
Copy Markdown
Owner

Fixes chorded passthrough inflating tap-hold required_idle/quick_tap.

  • Snapshot KeymapContext at the physical press that creates pending (idle, time, recent presses) in Keymap::pending_press_context
  • Reuse that snapshot for nested NewPressedKey so required_idle_time and quick_tap_ms are checked against the press, not the outer chord timeout (timeouts already backdated via pke.backdate)
  • Motivating test: tests/rust/chorded/nested_idle.rs demonstrates insufficient 50ms remains tap after 200ms chord timeout, sufficient 150ms still holds

Fixes timing issue where chorded timeout was effectively added to tap-hold idle check.

Comment thread smart-keymap-core/src/keymap.rs Outdated
Comment on lines +338 to +343
/// Snapshot of `KeymapContext` at the physical press that created
/// `pending_state`.
/// Used for nested replacements (e.g. chorded passthrough to
/// tap-hold) so `required_idle_time` and `quick_tap_ms` are
/// checked against the press, not the outer timeout.
pending_press_context: Option<KeymapContext>,

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ... apparently works.

But it surely seems like way too much to store; especially without other requirements for it.

My feeling is that storing something like "timings at pending" is appropriate, but the whole keymap context? e.g. also includes recent presses.

Comment thread smart-keymap-core/src/keymap.rs Outdated
Comment on lines +617 to +632
let nested_press_ctx = if let Some(ctx) = self.pending_press_context {
// Stored snapshot already excludes the current
// press from the ring; use it directly.
ctx
} else {
// Fallback: exclude the current press and backdate
// idle to the press time.
keymap_context_without_current_press(
self.recent_presses,
self.recent_press_count,
self.idle_time,
self.event_scheduler.schedule_counter,
pressed_modifiers,
keymap_index,
)
};

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting.

I wonder if this suggests it's worth looking into or rearranging the keymap context handling and how it relates to pending events.

rgoulter and others added 2 commits August 22, 2026 18:16
Model: muse-spark-1.2
Agent: Muse Code
Co-Authored-By: Muse Code powered by Meta Muse Spark <muse-code@meta.com>
Co-Authored-By: muse-spark-1.2 <muse-spark@meta.com>
Model: muse-spark-1.2
Agent: Muse Code
Co-Authored-By: Muse Code powered by Meta Muse Spark <muse-code@meta.com>
Co-Authored-By: muse-spark-1.2 <muse-spark@meta.com>
Model: muse-spark-1.2
Agent: Muse Code
Co-Authored-By: Muse Code powered by Meta Muse Spark <muse-code@meta.com>
Co-Authored-By: muse-spark-1.2 <muse-spark@meta.com>
@rgoulter
rgoulter merged commit 9079fbb into master Aug 22, 2026
13 checks passed
@rgoulter
rgoulter deleted the fix-idle-timeout branch August 22, 2026 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant