fix: restore key auto-repeat in copy mode and other non-terminal modes (PoC) - #2
fix: restore key auto-repeat in copy mode and other non-terminal modes (PoC)#2shibayu36 wants to merge 3 commits into
Conversation
Prepare the input-context enum for a non-terminal variant: it is about to model the input context of a key press, terminal-bound or not, so the "terminal input" name would no longer describe it. TerminalInputTarget keeps its name because it is genuinely terminal-only. refs herdrdev#2371
Track copy mode as a non-terminal input context so held semantic keys (arrows, etc.) pass the press/repeat context comparison instead of being suppressed, and route reprocessed repeats through the non-terminal key handler in the headless pipeline. refs herdrdev#2371
…modes Map Mode::Navigator, Mode::Navigate, and Mode::KeybindHelp to non-terminal input contexts so held movement keys repeat in the quick jump modal, the workspaces sidebar, and the keybind help screen. All three were reported in the issue comments. Their confirm and exit keys change modes, so those repeats stay suppressed by the context comparison without extra handling. refs herdrdev#2371
|
|
||
| #[derive(Clone, Debug, PartialEq, Eq)] | ||
| pub(crate) enum TerminalInputContext { | ||
| pub(crate) enum InputContext { |
There was a problem hiding this comment.
Renamed TerminalInputContext to InputContext: the enum now has a NonTerminal variant, so the name "terminal input" no longer matches what it holds. TerminalInputTarget keeps its name because it is truly terminal-only. The rename is a mechanical replace, split into the first commit.
| } | ||
|
|
||
| #[derive(Clone, Debug, PartialEq, Eq)] | ||
| pub(crate) enum NonTerminalInputContext { |
There was a problem hiding this comment.
This enum lists only the modes where we want key repeat. Modes not listed here get None from input_context(), so their repeats stay suppressed as before. I added only the four modes needed so far, but listing all modes is also possible if that is preferred.
| Some(TerminalInputContext::Pane) | ||
| Some(InputContext::Pane) | ||
| } else if self.state.mode == Mode::Copy { | ||
| Some(InputContext::NonTerminal(NonTerminalInputContext::Copy)) |
There was a problem hiding this comment.
This line is the direct cause of the broken repeat in copy mode. Whether repeats are allowed is decided right after a press is handled, by comparing the context before the press with the context after it (complete_press). If both are the same Some, later repeats are reprocessed; if either is None, they are all dropped. Copy mode always had None here, so its repeats were always dropped. Returning Some makes this comparison succeed.
Keys that leave the mode (Enter/y/q/Esc, ...) change the context at press time, so they still do not repeat. Every repeat also re-checks that the context is still the same, so if the mode changes while the key is held, the repeat stops there.
| let initial_context = self.input_context(); | ||
| let target = if matches!( | ||
| initial_context, | ||
| Some(InputContext::Pane | InputContext::Popup(_)) |
There was a problem hiding this comment.
This used to be is_some(), which meant "if there is a context, forward the key to the terminal". That is no longer true once NonTerminal contexts exist, so the terminal variants are matched explicitly now. The reprocess loop had the same pattern; there it is handled by the added NonTerminal branch (a hunk above in this file).
1 participant
Summary
PoC fix for key auto-repeat not working while holding keys in copy mode, the navigator, and similar screens. Instead of maintaining a hand-written list of repeatable keys, it decides whether a repeat is allowed by looking at what the key press actually did: whether the input context changed.
Upstream issue: herdrdev#2371
Upstream PR taking the key-list approach: herdrdev#2372
Approach
The existing input lease ledger (src/app/input/lease.rs) already has the machinery: repeats are reprocessed only when the context at repeat time matches the context captured at press time. Copy mode simply had no context (None), which meant its repeats were always suppressed.
This PoC adds a NonTerminal variant to the input context and lets copy mode participate. That alone yields:
so there is no repeatable-key list to maintain (no hand-written exclusions like is_prefix_key).