fix(tui): disable double-press quit shortcut (#9220)
Disables the default Ctrl+C/Ctrl+D double-press quit UX (keeps the code path behind a const) while we rethink the quit/interrupt flow. Tests: - just fmt - cargo clippy --fix --all-features --tests --allow-dirty --allow-no-vcs -p codex-tui - cargo test -p codex-tui --lib
This commit is contained in:
parent
0471ddbe74
commit
27da8a68d3
6 changed files with 88 additions and 70 deletions
|
|
@ -71,6 +71,13 @@ pub(crate) use feedback_view::FeedbackNoteView;
|
|||
/// Keeping a single value ensures Ctrl+C and Ctrl+D behave identically.
|
||||
pub(crate) const QUIT_SHORTCUT_TIMEOUT: Duration = Duration::from_secs(1);
|
||||
|
||||
/// Whether Ctrl+C/Ctrl+D require a second press to quit.
|
||||
///
|
||||
/// This UX experiment was enabled by default, but requiring a double press to quit feels janky in
|
||||
/// practice (especially for users accustomed to shells and other TUIs). Disable it for now while we
|
||||
/// rethink a better quit/interrupt design.
|
||||
pub(crate) const DOUBLE_PRESS_QUIT_SHORTCUT_ENABLED: bool = false;
|
||||
|
||||
/// The result of offering a cancellation key to a bottom-pane surface.
|
||||
///
|
||||
/// This is primarily used for Ctrl+C routing: active views can consume the key to dismiss
|
||||
|
|
@ -359,6 +366,10 @@ impl BottomPane {
|
|||
/// after [`QUIT_SHORTCUT_TIMEOUT`] so the hint disappears even if the user
|
||||
/// stops typing and no other events trigger a draw.
|
||||
pub(crate) fn show_quit_shortcut_hint(&mut self, key: KeyBinding) {
|
||||
if !DOUBLE_PRESS_QUIT_SHORTCUT_ENABLED {
|
||||
return;
|
||||
}
|
||||
|
||||
self.composer
|
||||
.show_quit_shortcut_hint(key, self.has_input_focus);
|
||||
let frame_requester = self.frame_requester.clone();
|
||||
|
|
@ -695,7 +706,7 @@ mod tests {
|
|||
}
|
||||
|
||||
#[test]
|
||||
fn ctrl_c_on_modal_consumes_and_shows_quit_hint() {
|
||||
fn ctrl_c_on_modal_consumes_without_showing_quit_hint() {
|
||||
let (tx_raw, _rx) = unbounded_channel::<AppEvent>();
|
||||
let tx = AppEventSender::new(tx_raw);
|
||||
let features = Features::with_defaults();
|
||||
|
|
@ -711,7 +722,7 @@ mod tests {
|
|||
});
|
||||
pane.push_approval_request(exec_request(), &features);
|
||||
assert_eq!(CancellationEvent::Handled, pane.on_ctrl_c());
|
||||
assert!(pane.quit_shortcut_hint_visible());
|
||||
assert!(!pane.quit_shortcut_hint_visible());
|
||||
assert_eq!(CancellationEvent::NotHandled, pane.on_ctrl_c());
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -124,6 +124,7 @@ use crate::bottom_pane::BetaFeatureItem;
|
|||
use crate::bottom_pane::BottomPane;
|
||||
use crate::bottom_pane::BottomPaneParams;
|
||||
use crate::bottom_pane::CancellationEvent;
|
||||
use crate::bottom_pane::DOUBLE_PRESS_QUIT_SHORTCUT_ENABLED;
|
||||
use crate::bottom_pane::ExperimentalFeaturesView;
|
||||
use crate::bottom_pane::InputResult;
|
||||
use crate::bottom_pane::QUIT_SHORTCUT_TIMEOUT;
|
||||
|
|
@ -3989,12 +3990,23 @@ impl ChatWidget {
|
|||
let key = key_hint::ctrl(KeyCode::Char('c'));
|
||||
let modal_or_popup_active = !self.bottom_pane.no_modal_or_popup_active();
|
||||
if self.bottom_pane.on_ctrl_c() == CancellationEvent::Handled {
|
||||
if modal_or_popup_active {
|
||||
self.quit_shortcut_expires_at = None;
|
||||
self.quit_shortcut_key = None;
|
||||
self.bottom_pane.clear_quit_shortcut_hint();
|
||||
if DOUBLE_PRESS_QUIT_SHORTCUT_ENABLED {
|
||||
if modal_or_popup_active {
|
||||
self.quit_shortcut_expires_at = None;
|
||||
self.quit_shortcut_key = None;
|
||||
self.bottom_pane.clear_quit_shortcut_hint();
|
||||
} else {
|
||||
self.arm_quit_shortcut(key);
|
||||
}
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
if !DOUBLE_PRESS_QUIT_SHORTCUT_ENABLED {
|
||||
if self.is_cancellable_work_active() {
|
||||
self.submit_op(Op::Interrupt);
|
||||
} else {
|
||||
self.arm_quit_shortcut(key);
|
||||
self.request_quit_without_confirmation();
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
|
@ -4019,6 +4031,16 @@ impl ChatWidget {
|
|||
/// Otherwise it should be routed to the active view and not attempt to quit.
|
||||
fn on_ctrl_d(&mut self) -> bool {
|
||||
let key = key_hint::ctrl(KeyCode::Char('d'));
|
||||
if !DOUBLE_PRESS_QUIT_SHORTCUT_ENABLED {
|
||||
if !self.bottom_pane.composer_is_empty() || !self.bottom_pane.no_modal_or_popup_active()
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
self.request_quit_without_confirmation();
|
||||
return true;
|
||||
}
|
||||
|
||||
if self.quit_shortcut_active_for(key) {
|
||||
self.quit_shortcut_expires_at = None;
|
||||
self.quit_shortcut_key = None;
|
||||
|
|
|
|||
|
|
@ -1109,46 +1109,22 @@ async fn streaming_final_answer_keeps_task_running_state() {
|
|||
Ok(Op::Interrupt) => {}
|
||||
other => panic!("expected Op::Interrupt, got {other:?}"),
|
||||
}
|
||||
assert!(chat.bottom_pane.quit_shortcut_hint_visible());
|
||||
assert!(!chat.bottom_pane.quit_shortcut_hint_visible());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn ctrl_c_shutdown_ignores_caps_lock() {
|
||||
async fn ctrl_c_shutdown_works_with_caps_lock() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await;
|
||||
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Char('C'), KeyModifiers::CONTROL));
|
||||
|
||||
assert_matches!(rx.try_recv(), Err(TryRecvError::Empty));
|
||||
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Char('c'), KeyModifiers::CONTROL));
|
||||
|
||||
assert_matches!(rx.try_recv(), Ok(AppEvent::Exit(ExitMode::ShutdownFirst)));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn ctrl_d_double_press_quits_without_prompt() {
|
||||
async fn ctrl_d_quits_without_prompt() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await;
|
||||
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Char('d'), KeyModifiers::CONTROL));
|
||||
assert_matches!(rx.try_recv(), Err(TryRecvError::Empty));
|
||||
|
||||
let width = 100;
|
||||
let height = chat.desired_height(width);
|
||||
let mut terminal =
|
||||
ratatui::Terminal::new(VT100Backend::new(width, height)).expect("create terminal");
|
||||
terminal.set_viewport_area(Rect::new(0, 0, width, height));
|
||||
terminal
|
||||
.draw(|f| chat.render(f.area(), f.buffer_mut()))
|
||||
.expect("draw after ctrl+d");
|
||||
assert!(
|
||||
terminal
|
||||
.backend()
|
||||
.vt100()
|
||||
.screen()
|
||||
.contents()
|
||||
.contains("ctrl + d again to quit")
|
||||
);
|
||||
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Char('d'), KeyModifiers::CONTROL));
|
||||
assert_matches!(rx.try_recv(), Ok(AppEvent::Exit(ExitMode::ShutdownFirst)));
|
||||
}
|
||||
|
|
@ -1179,7 +1155,7 @@ async fn ctrl_c_cleared_prompt_is_recoverable_via_history() {
|
|||
chat.handle_key_event(KeyEvent::new(KeyCode::Char('c'), KeyModifiers::CONTROL));
|
||||
assert!(chat.bottom_pane.composer_text().is_empty());
|
||||
assert_matches!(op_rx.try_recv(), Err(TryRecvError::Empty));
|
||||
assert!(chat.bottom_pane.quit_shortcut_hint_visible());
|
||||
assert!(!chat.bottom_pane.quit_shortcut_hint_visible());
|
||||
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::NONE));
|
||||
let restored_text = chat.bottom_pane.composer_text();
|
||||
|
|
|
|||
|
|
@ -68,6 +68,13 @@ pub(crate) use feedback_view::FeedbackNoteView;
|
|||
/// Keeping a single value ensures Ctrl+C and Ctrl+D behave identically.
|
||||
pub(crate) const QUIT_SHORTCUT_TIMEOUT: Duration = Duration::from_secs(1);
|
||||
|
||||
/// Whether Ctrl+C/Ctrl+D require a second press to quit.
|
||||
///
|
||||
/// This UX experiment was enabled by default, but requiring a double press to quit feels janky in
|
||||
/// practice (especially for users accustomed to shells and other TUIs). Disable it for now while we
|
||||
/// rethink a better quit/interrupt design.
|
||||
pub(crate) const DOUBLE_PRESS_QUIT_SHORTCUT_ENABLED: bool = false;
|
||||
|
||||
/// The result of offering a cancellation key to a bottom-pane surface.
|
||||
///
|
||||
/// This is primarily used for Ctrl+C routing: active views can consume the key to dismiss
|
||||
|
|
@ -337,6 +344,10 @@ impl BottomPane {
|
|||
/// after [`QUIT_SHORTCUT_TIMEOUT`] so the hint disappears even if the user
|
||||
/// stops typing and no other events trigger a draw.
|
||||
pub(crate) fn show_quit_shortcut_hint(&mut self, key: KeyBinding) {
|
||||
if !DOUBLE_PRESS_QUIT_SHORTCUT_ENABLED {
|
||||
return;
|
||||
}
|
||||
|
||||
self.composer
|
||||
.show_quit_shortcut_hint(key, self.has_input_focus);
|
||||
let frame_requester = self.frame_requester.clone();
|
||||
|
|
@ -681,7 +692,7 @@ mod tests {
|
|||
}
|
||||
|
||||
#[test]
|
||||
fn ctrl_c_on_modal_consumes_and_shows_quit_hint() {
|
||||
fn ctrl_c_on_modal_consumes_without_showing_quit_hint() {
|
||||
let (tx_raw, _rx) = unbounded_channel::<AppEvent>();
|
||||
let tx = AppEventSender::new(tx_raw);
|
||||
let features = Features::with_defaults();
|
||||
|
|
@ -697,7 +708,7 @@ mod tests {
|
|||
});
|
||||
pane.push_approval_request(exec_request(), &features);
|
||||
assert_eq!(CancellationEvent::Handled, pane.on_ctrl_c());
|
||||
assert!(pane.quit_shortcut_hint_visible());
|
||||
assert!(!pane.quit_shortcut_hint_visible());
|
||||
assert_eq!(CancellationEvent::NotHandled, pane.on_ctrl_c());
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -119,6 +119,7 @@ use crate::bottom_pane::ApprovalRequest;
|
|||
use crate::bottom_pane::BottomPane;
|
||||
use crate::bottom_pane::BottomPaneParams;
|
||||
use crate::bottom_pane::CancellationEvent;
|
||||
use crate::bottom_pane::DOUBLE_PRESS_QUIT_SHORTCUT_ENABLED;
|
||||
use crate::bottom_pane::InputResult;
|
||||
use crate::bottom_pane::QUIT_SHORTCUT_TIMEOUT;
|
||||
use crate::bottom_pane::SelectionAction;
|
||||
|
|
@ -3662,12 +3663,23 @@ impl ChatWidget {
|
|||
let key = key_hint::ctrl(KeyCode::Char('c'));
|
||||
let modal_or_popup_active = !self.bottom_pane.no_modal_or_popup_active();
|
||||
if self.bottom_pane.on_ctrl_c() == CancellationEvent::Handled {
|
||||
if modal_or_popup_active {
|
||||
self.quit_shortcut_expires_at = None;
|
||||
self.quit_shortcut_key = None;
|
||||
self.bottom_pane.clear_quit_shortcut_hint();
|
||||
if DOUBLE_PRESS_QUIT_SHORTCUT_ENABLED {
|
||||
if modal_or_popup_active {
|
||||
self.quit_shortcut_expires_at = None;
|
||||
self.quit_shortcut_key = None;
|
||||
self.bottom_pane.clear_quit_shortcut_hint();
|
||||
} else {
|
||||
self.arm_quit_shortcut(key);
|
||||
}
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
if !DOUBLE_PRESS_QUIT_SHORTCUT_ENABLED {
|
||||
if self.is_cancellable_work_active() {
|
||||
self.submit_op(Op::Interrupt);
|
||||
} else {
|
||||
self.arm_quit_shortcut(key);
|
||||
self.request_quit_without_confirmation();
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
|
@ -3692,6 +3704,16 @@ impl ChatWidget {
|
|||
/// Otherwise it should be routed to the active view and not attempt to quit.
|
||||
fn on_ctrl_d(&mut self) -> bool {
|
||||
let key = key_hint::ctrl(KeyCode::Char('d'));
|
||||
if !DOUBLE_PRESS_QUIT_SHORTCUT_ENABLED {
|
||||
if !self.bottom_pane.composer_is_empty() || !self.bottom_pane.no_modal_or_popup_active()
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
self.request_quit_without_confirmation();
|
||||
return true;
|
||||
}
|
||||
|
||||
if self.quit_shortcut_active_for(key) {
|
||||
self.quit_shortcut_expires_at = None;
|
||||
self.quit_shortcut_key = None;
|
||||
|
|
|
|||
|
|
@ -1061,46 +1061,22 @@ async fn streaming_final_answer_keeps_task_running_state() {
|
|||
Ok(Op::Interrupt) => {}
|
||||
other => panic!("expected Op::Interrupt, got {other:?}"),
|
||||
}
|
||||
assert!(chat.bottom_pane.quit_shortcut_hint_visible());
|
||||
assert!(!chat.bottom_pane.quit_shortcut_hint_visible());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn ctrl_c_shutdown_ignores_caps_lock() {
|
||||
async fn ctrl_c_shutdown_works_with_caps_lock() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await;
|
||||
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Char('C'), KeyModifiers::CONTROL));
|
||||
|
||||
assert_matches!(rx.try_recv(), Err(TryRecvError::Empty));
|
||||
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Char('c'), KeyModifiers::CONTROL));
|
||||
|
||||
assert_matches!(rx.try_recv(), Ok(AppEvent::Exit(ExitMode::ShutdownFirst)));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn ctrl_d_double_press_quits_without_prompt() {
|
||||
async fn ctrl_d_quits_without_prompt() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await;
|
||||
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Char('d'), KeyModifiers::CONTROL));
|
||||
assert_matches!(rx.try_recv(), Err(TryRecvError::Empty));
|
||||
|
||||
let width = 100;
|
||||
let height = chat.desired_height(width);
|
||||
let mut terminal =
|
||||
ratatui::Terminal::new(VT100Backend::new(width, height)).expect("create terminal");
|
||||
terminal.set_viewport_area(Rect::new(0, 0, width, height));
|
||||
terminal
|
||||
.draw(|f| chat.render(f.area(), f.buffer_mut()))
|
||||
.expect("draw after ctrl+d");
|
||||
assert!(
|
||||
terminal
|
||||
.backend()
|
||||
.vt100()
|
||||
.screen()
|
||||
.contents()
|
||||
.contains("ctrl + d again to quit")
|
||||
);
|
||||
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Char('d'), KeyModifiers::CONTROL));
|
||||
assert_matches!(rx.try_recv(), Ok(AppEvent::Exit(ExitMode::ShutdownFirst)));
|
||||
}
|
||||
|
|
@ -1131,7 +1107,7 @@ async fn ctrl_c_cleared_prompt_is_recoverable_via_history() {
|
|||
chat.handle_key_event(KeyEvent::new(KeyCode::Char('c'), KeyModifiers::CONTROL));
|
||||
assert!(chat.bottom_pane.composer_text().is_empty());
|
||||
assert_matches!(op_rx.try_recv(), Err(TryRecvError::Empty));
|
||||
assert!(chat.bottom_pane.quit_shortcut_hint_visible());
|
||||
assert!(!chat.bottom_pane.quit_shortcut_hint_visible());
|
||||
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::NONE));
|
||||
let restored_text = chat.bottom_pane.composer_text();
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue