feat(tui): configurable additive voice-input keybinding (APP-4988)#14307
feat(tui): configurable additive voice-input keybinding (APP-4988)#14307warp-dev-github-integration[bot] wants to merge 2 commits into
Conversation
Spec for a configurable TUI keybinding to start voice mode, mirroring the GUI's "activate voice input" setting. Adds a TUI-surface enum setting `agents.voice.voice_input_toggle_key` (reusing the GUI's VoiceInputToggleKey type, default None), wires the selected key to the existing `tui:session:start_voice_input` tap action (replacing the hardcoded ctrl-s default), and maps `KeyCode::Modifier(_)` in the TUI input-conversion layer so bare-modifier options (Alt/Control/Shift, Super best-effort) can fire. Fn is unsupported (no crossterm KeyCode). Hold-to-toggle / push-to-talk (REPORT_EVENT_TYPES + key-release handling) is explicitly out of scope and recorded as future work. Centered on 15 exhaustive, checkable validation criteria. This PR carries the committed spec; the implementation phase adds code to this same branch/PR. Co-Authored-By: Warp <agent@warp.dev>
Preserve ctrl-s while adding a TUI settings-driven modifier trigger for voice input, including standalone modifier event conversion and regression coverage. Co-Authored-By: Oz <oz-agent@warp.dev>
warp-dev-github-integration
Bot
changed the title
|
@warp-dev-github-integration[bot] I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This PR adds a TUI-local voice-input setting, registers an additive configured binding alongside the existing ctrl-s binding, and maps crossterm standalone modifier key events into modifier-only keystrokes.
Concerns
- This is a user-facing TUI behavior change, but the PR description explicitly says no terminal transcript, render snapshot, or
./script/run-tuicapture is attached. Repo guidance requires TUI visual evidence for this kind of change; attach a TUI capture showing the configured key entering the voice-input Listening state and the defaultnone/ctrl-s behavior still working. TuiVoiceInputToggleKey::keystroke()currently forwards the GUIFnkeystroke even thoughFnis documented as unsupported/no-op for the TUI, sofncan register an unusable binding instead of leaving no active trigger.
Verdict
Found: 0 critical, 2 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| pub fn keystroke(self) -> Option<warpui::keymap::Keystroke> { | ||
| self.0.keystroke() | ||
| } |
There was a problem hiding this comment.
Fn is documented as unsupported for the TUI, but this wrapper forwards the GUI enum's keystroke() and registers a synthetic fn trigger. Filter Fn to None so agents.voice.voice_input_toggle_key = "fn" leaves no active binding instead of showing an unusable one.
| pub fn keystroke(self) -> Option<warpui::keymap::Keystroke> { | |
| self.0.keystroke() | |
| } | |
| pub fn keystroke(self) -> Option<warpui::keymap::Keystroke> { | |
| match self.0 { | |
| VoiceInputToggleKey::Fn => None, | |
| key => key.keystroke(), | |
| } | |
| } |
![warp-dev-github-integration[bot]](https://avatars.githubusercontent.com/in/2196476?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This PR adds a configurable, additive TUI voice-input keybinding (agents.voice.voice_input_toggle_key, default none) that registers an additional tap-to-start trigger alongside the preserved hardcoded ctrl-s, and maps crossterm standalone modifier events (KeyCode::Modifier) into modifier-only keystrokes so Alt/Control/Shift (and Super, best-effort) can fire. The design matches the committed spec's chosen approach (tap-to-start, additive): the binding name tui:session:start_voice_input and ctrl-s baseline are preserved, the voice lifecycle and Kitty flags are untouched, and a merge_setting_schema change unions x-warp-surfaces for the shared GUI/TUI TOML path. I verified the GUI's VoiceInputToggleKey::keystroke() byte-equals the event-conversion output for each modifier (Control→{ctrl:true,key:""}, Alt/Shift/Super likewise), so the configured binding matches the converted KeyDown; focused warpui_core tests pass and cargo fmt --check + clippy -p warpui_core --features tui --tests -- -D warnings are clean.
Concerns
- Missing full build + TUI visual proof (environment OOM, not an author omission). This is a user-facing TUI change, so the spec's criterion #15 requires a rendered-screen capture (
./script/run-tuior a cloud dogfood build) showing the configured key entering theListeningstate and thenone/ctrl-sbaseline still working. The PR body documents, and I independently confirmed, that the fullwarp/warp_tuicompile is terminated by the sandbox's ~4GB memory limit (rustc SIGKILL/OOM), so neither the full build nor the TUI visual proof could be captured here. This is the only outstanding verification gap and is caused by the environment, not the author — a higher-memory environment is needed to run the full build and capture the TUI rendered-screen proof before merge. Per the foreman's environment caveat, this is routed to a human rather than an unwinnable author-rework loop. Fnregisters a dead binding instead of no binding.TuiVoiceInputToggleKey::keystroke()forwards the GUI enum'skeystroke(), which returnsSome(Keystroke { key: "fn", .. })forFn, soagents.voice.voice_input_toggle_key = "fn"registers a binding with a syntheticfnkeystroke rather than leaving no active trigger. It is inert today (no crossterm event produceskey: "fn"), so the spec's intent — Fn is a no-op — holds, but this deviates from spec criterion #7's "registers no keystroke" and leaves a latent dead binding. A one-line filter (Fn => None) makes the code match the documented behavior.- Add a chord-vs-bare-modifier regression test (spec criterion #11). The highest-risk side effect called out in the spec is a bare-modifier
KeyDownmatching an unintended binding; the guard is that chordKeyDowns carry a non-emptykeyand so can't match the standalone-modifier binding. The new tests cover the modifier→keystroke mapping and the constructed-keystroke matcher path, but none assert that a chord likectrl-corctrl-enterdoes not match thecontrol_leftvoice binding. A small chord-non-match test inevent_conversion_testsorkeymap_testswould lock in that invariant against future changes tokey_name/key_modifiers.
Verdict
Found: 0 critical, 0 important, 2 suggestions
Approve (with the environment-OOM build/visual-proof limitation flagged for the human merge gate, and the two non-blocking suggestions below)
Review run
https://oz.staging.warp.dev/runs/019f99fd-4411-7565-9e89-eb9b30926d62
| pub fn keystroke(self) -> Option<warpui::keymap::Keystroke> { | ||
| self.0.keystroke() | ||
| } |
There was a problem hiding this comment.
💡 [SUGGESTION] Fn is documented as unsupported in the TUI, but this forwards the GUI enum's keystroke(), which returns Some(Keystroke { key: "fn", .. }) for Fn — so voice_input_toggle_key = "fn" registers a synthetic (inert) fn binding instead of no active trigger. Filter Fn to None so the TUI matches spec criterion #7 (“registers no keystroke”) and avoids a latent dead binding.
| pub fn keystroke(self) -> Option<warpui::keymap::Keystroke> { | |
| self.0.keystroke() | |
| } | |
| pub fn keystroke(self) -> Option<warpui::keymap::Keystroke> { | |
| match self.0 { | |
| VoiceInputToggleKey::Fn => None, | |
| key => key.keystroke(), | |
| } | |
| } |
| let modifiers = [ | ||
| ( | ||
| ModifierKeyCode::LeftControl, | ||
| KeyModifiers::CONTROL, | ||
| "control", | ||
| ), | ||
| ( | ||
| ModifierKeyCode::RightControl, | ||
| KeyModifiers::CONTROL, | ||
| "control", | ||
| ), | ||
| (ModifierKeyCode::LeftAlt, KeyModifiers::ALT, "alt"), | ||
| (ModifierKeyCode::RightAlt, KeyModifiers::ALT, "alt"), | ||
| (ModifierKeyCode::LeftShift, KeyModifiers::SHIFT, "shift"), | ||
| (ModifierKeyCode::RightShift, KeyModifiers::SHIFT, "shift"), | ||
| (ModifierKeyCode::LeftSuper, KeyModifiers::SUPER, "super"), | ||
| (ModifierKeyCode::RightSuper, KeyModifiers::SUPER, "super"), | ||
| ]; | ||
|
|
||
| for (modifier, expected_flags, name) in modifiers { | ||
| let Some(TuiEvent::KeyDown { keystroke, .. }) = | ||
| key(KeyCode::Modifier(modifier), KeyModifiers::empty()) | ||
| else { | ||
| panic!("expected standalone modifier KeyDown for {name}"); | ||
| }; | ||
| assert_eq!(keystroke.key, ""); | ||
| assert_eq!( | ||
| ( | ||
| keystroke.ctrl, | ||
| keystroke.alt, | ||
| keystroke.shift, | ||
| keystroke.cmd | ||
| ), | ||
| ( | ||
| expected_flags.contains(KeyModifiers::CONTROL), | ||
| expected_flags.contains(KeyModifiers::ALT), | ||
| expected_flags.contains(KeyModifiers::SHIFT), | ||
| expected_flags.contains(KeyModifiers::SUPER), | ||
| ), | ||
| "unexpected flags for {name}", | ||
| ); | ||
| } |
There was a problem hiding this comment.
💡 [SUGGESTION] This locks in the modifier→keystroke mapping, but the spec's highest-risk side effect (criterion #11) is a bare-modifier KeyDown matching an unintended binding. Consider adding a companion test asserting that a chord KeyDown (e.g. KeyCode::Char('c') with KeyModifiers::CONTROL) produces a keystroke with a non-empty key and does not equal the standalone-control keystroke, so the chord-vs-bare-modifier invariant is guarded against future changes to key_name/key_modifiers.
2 participants
Description
Adds a configurable TUI voice-input key under
agents.voice.voice_input_toggle_keywhile preserving the existing hardcodedctrl-strigger. The configured key is an additional tap-to-start trigger;none(the default) leavesctrl-sunchanged. Bare Alt/Control/Shift/Super modifier events are converted into standalone keystrokes; Fn remains unsupported and push-to-talk/release handling remains out of scope.Linked Issue
Testing
ctrl-strigger and an additional configured modifier trigger.cargo fmt --all -- --checkCARGO_BUILD_JOBS=1 cargo test -p warpui_core standalone_modifier --no-fail-fastCARGO_BUILD_JOBS=1 cargo test -p warpui_core --features tui standalone_modifier_keys_map_to_modifier_only_keystrokes --no-fail-fastwarp/warp_tuicompilation and focused TUI test were attempted with low-memory settings but rustc was terminated by the sandbox with SIGKILL/OOM before diagnostics.Screenshots / Videos
The required real-TUI visual verification could not be captured because the full TUI build was terminated by the sandbox memory limit; no UI artifact is included.
Agent Mode
CHANGELOG-IMPROVEMENT: Add a configurable TUI voice-input trigger while preserving the default ctrl-s hotkey.
Conversation: https://staging.warp.dev/conversation/97cba722-4fd4-4c78-b399-994a7f6997e0
Run: https://oz.staging.warp.dev/runs/019f99b9-9775-7c5f-9e5d-9c63381f5987
Co-Authored-By: Oz oz-agent@warp.dev
This PR was generated with Oz.