Skip to content
Merged

Fix esc #2661

Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions codex-rs/tui/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,13 +303,21 @@ impl App {
self.transcript_overlay = Some(TranscriptApp::new(self.transcript_lines.clone()));
tui.frame_requester().schedule_frame();
}
// Esc primes/advances backtracking when composer is empty.
// Esc primes/advances backtracking only in normal (not working) mode
// with an empty composer. In any other state, forward Esc so the
// active UI (e.g. status indicator, modals, popups) handles it.
KeyEvent {
code: KeyCode::Esc,
kind: KeyEventKind::Press | KeyEventKind::Repeat,
..
} => {
self.handle_backtrack_esc_key(tui);
if self.chat_widget.is_normal_backtrack_mode()
&& self.chat_widget.composer_is_empty()
{
self.handle_backtrack_esc_key(tui);
} else {
self.chat_widget.handle_key_event(key_event);
}
}
// Enter confirms backtrack when primed + count > 0. Otherwise pass to widget.
KeyEvent {
Expand Down
12 changes: 12 additions & 0 deletions codex-rs/tui/src/bottom_pane/chat_composer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,11 @@ impl ChatComposer {
result
}

/// Return true if either the slash-command popup or the file-search popup is active.
pub(crate) fn popup_active(&self) -> bool {
!matches!(self.active_popup, ActivePopup::None)
}

/// Handle key event when the slash-command popup is visible.
fn handle_key_event_with_slash_popup(&mut self, key_event: KeyEvent) -> (InputResult, bool) {
let ActivePopup::Command(popup) = &mut self.active_popup else {
Expand All @@ -313,6 +318,13 @@ impl ChatComposer {
popup.move_down();
(InputResult::None, true)
}
KeyEvent {
code: KeyCode::Esc, ..
} => {
// Dismiss the slash popup; keep the current input untouched.
self.active_popup = ActivePopup::None;
(InputResult::None, true)
Comment on lines 318 to +326
Copy link
Contributor

Choose a reason for hiding this comment

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

[P1] Esc dismissal of slash popup reopened immediately

In handle_key_event_with_slash_popup the Esc branch clears active_popup (lines 321‑327) to hide the slash command menu. But handle_key_event then unconditionally calls sync_command_popup (lines 280‑290), which recreates the popup whenever the text still starts with '/'. Thus Esc cannot actually close the popup, contradicting the intended behaviour.

Useful? React with 👍 / 👎.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do we want to esc hide pop up on the slash or not?
@edward-bayes @easong-openai

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ideally yes - sometimes you just want a /

}
KeyEvent {
code: KeyCode::Tab, ..
} => {
Expand Down
7 changes: 7 additions & 0 deletions codex-rs/tui/src/bottom_pane/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,13 @@ impl BottomPane {
self.is_task_running
}

/// Return true when the pane is in the regular composer state without any
/// overlays or popups and not running a task. This is the safe context to
/// use Esc-Esc for backtracking from the main view.
pub(crate) fn is_normal_backtrack_mode(&self) -> bool {
!self.is_task_running && self.active_view.is_none() && !self.composer.popup_active()
}

/// Update the *context-window remaining* indicator in the composer. This
/// is forwarded directly to the underlying `ChatComposer`.
pub(crate) fn set_token_usage(
Expand Down
7 changes: 7 additions & 0 deletions codex-rs/tui/src/chatwidget.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1083,6 +1083,13 @@ impl ChatWidget {
self.bottom_pane.composer_is_empty()
}

/// True when the UI is in the regular composer state with no running task,
/// no modal overlay (e.g. approvals or status indicator), and no composer popups.
/// In this state Esc-Esc backtracking is enabled.
pub(crate) fn is_normal_backtrack_mode(&self) -> bool {
self.bottom_pane.is_normal_backtrack_mode()
}

pub(crate) fn insert_str(&mut self, text: &str) {
self.bottom_pane.insert_str(text);
}
Expand Down