From 3d517fbd00ed357572f473b8fef6a00cdabfbd2e Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Wed, 13 May 2026 12:33:27 -0300 Subject: [PATCH] feat(tui): standardize picker navigation keys (#22347) ## Why Picker-style UI in the TUI has accumulated a mix of hardcoded navigation keys. Some lists supported page movement, some did not; some accepted Vim-like keys, while others only accepted arrows; and tabbed or horizontally adjustable pickers had no shared keymap action for left/right movement. This PR makes picker/list navigation consistent and configurable so users can rely on the same defaults across the TUI. ## What Changed - Adds shared list keymap actions for: - vertical movement: `move_up`, `move_down` - horizontal movement: `move_left`, `move_right` - paging and jumps: `page_up`, `page_down`, `jump_top`, `jump_bottom` - Adds defaults: - Up/down: arrows, `Ctrl+P/N`, `Ctrl+K/J`, and plain `k/j` where text input is not active - Page up/down: `PageUp/PageDown` and `Ctrl+B/F` - First/last: `Home/End` - Left/right: `Left/Right` and `Ctrl+H/L` - Wires the shared list keymap through picker and list surfaces including session resume, multi-select, tabbed selection lists, settings-style lists, app-link selection, MCP elicitation, request-user-input, and the OSS selection wizard. - Keeps search behavior intact by reserving printable characters for query text in searchable pickers. - Updates keymap setup actions, config schema, snapshots, and focused coverage for the new list actions. ## How to Test 1. Start Codex from this branch and open the session picker, for example with an existing session history. 2. In the session list, verify that `Ctrl+J/K` moves the selection down/up. 3. Verify that `Ctrl+F/B` pages down/up and `Home/End` jumps to the first/last visible session. 4. Type printable search text such as `j` or `k` and confirm it updates the query instead of navigating. 5. Focus a picker control that changes values horizontally, such as a session picker toolbar control, and verify `Ctrl+H/L` changes the focused value like left/right arrows. Targeted tests run: - `cargo test -p codex-tui keymap::tests::` - `cargo test -p codex-tui keymap_setup::tests::` - `cargo test -p codex-tui horizontal_list_keys` - `cargo test -p codex-tui page_and_jump_navigation_use_list_keymap` - `cargo test -p codex-tui ctrl_h_l_move_provider_selection` - `cargo test -p codex-tui scroll_state::tests` - `cargo test -p codex-tui switching_tabs_changes_visible_items_and_clears_search` - `cargo test -p codex-tui toggle_sort_key_reloads_with_new_sort` Also ran `just write-config-schema`, `just fmt`, `just fix -p codex-tui`, `just argument-comment-lint`, and `git diff --check`. Note: `cargo test -p codex-tui` was attempted and still aborts in the pre-existing `tests::fork_last_filters_latest_session_by_cwd_unless_show_all` stack overflow, which is unrelated to this branch. --- codex-rs/config/src/tui_keymap.rs | 12 + codex-rs/core/config.schema.json | 64 +++- codex-rs/tui/src/bottom_pane/app_link_view.rs | 105 +++++- .../bottom_pane/experimental_features_view.rs | 83 ++-- .../tui/src/bottom_pane/hooks_browser_view.rs | 72 ++-- .../src/bottom_pane/list_selection_view.rs | 240 ++++++++++-- .../src/bottom_pane/mcp_server_elicitation.rs | 82 ++++ .../src/bottom_pane/memories_settings_view.rs | 82 ++-- codex-rs/tui/src/bottom_pane/mod.rs | 13 +- .../src/bottom_pane/multi_select_picker.rs | 244 +++++++++--- .../src/bottom_pane/request_user_input/mod.rs | 73 +++- codex-rs/tui/src/bottom_pane/scroll_state.rs | 113 +++++- .../tui/src/bottom_pane/skills_toggle_view.rs | 148 +++++--- ..._snapshot_uses_runtime_preview_values.snap | 2 +- ...up__tests__terminal_title_setup_basic.snap | 2 +- .../tui/src/bottom_pane/status_line_setup.rs | 8 +- codex-rs/tui/src/bottom_pane/title_setup.rs | 15 +- codex-rs/tui/src/chatwidget.rs | 15 +- codex-rs/tui/src/chatwidget/hooks.rs | 1 + codex-rs/tui/src/chatwidget/skills.rs | 6 +- ...tatus_line_setup_popup_hardcoded_only.snap | 2 +- ...ts__status_line_setup_popup_live_only.snap | 2 +- ..._tests__status_line_setup_popup_mixed.snap | 2 +- ...inal_title_setup_popup_hardcoded_only.snap | 2 +- ..._terminal_title_setup_popup_live_only.snap | 2 +- ...sts__terminal_title_setup_popup_mixed.snap | 2 +- .../chatwidget/tests/popups_and_settings.rs | 7 +- codex-rs/tui/src/key_hint.rs | 27 ++ codex-rs/tui/src/keymap.rs | 354 +++++++++++++++++- codex-rs/tui/src/keymap_setup/actions.rs | 18 + codex-rs/tui/src/oss_selection.rs | 61 ++- codex-rs/tui/src/resume_picker.rs | 190 ++++++---- ...ap_setup__tests__keymap_picker_custom.snap | 2 +- ...ests__keymap_picker_fast_mode_enabled.snap | 2 +- ...p__tests__keymap_picker_first_actions.snap | 4 +- ...ap_setup__tests__keymap_picker_narrow.snap | 2 +- ...ymap_setup__tests__keymap_picker_wide.snap | 2 +- 37 files changed, 1635 insertions(+), 426 deletions(-) diff --git a/codex-rs/config/src/tui_keymap.rs b/codex-rs/config/src/tui_keymap.rs index 90d82da93..5b1e7d4cc 100644 --- a/codex-rs/config/src/tui_keymap.rs +++ b/codex-rs/config/src/tui_keymap.rs @@ -306,6 +306,18 @@ pub struct TuiListKeymap { pub move_up: Option, /// Move list selection down. pub move_down: Option, + /// Move horizontally left in list pickers that support horizontal actions. + pub move_left: Option, + /// Move horizontally right in list pickers that support horizontal actions. + pub move_right: Option, + /// Move list selection up by one page. + pub page_up: Option, + /// Move list selection down by one page. + pub page_down: Option, + /// Jump to the first list item. + pub jump_top: Option, + /// Jump to the last list item. + pub jump_bottom: Option, /// Accept current selection. pub accept: Option, /// Cancel and close selection view. diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 1d3ca3210..8a7cb01b5 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -2606,8 +2606,14 @@ "list": { "accept": null, "cancel": null, + "jump_bottom": null, + "jump_top": null, "move_down": null, - "move_up": null + "move_left": null, + "move_right": null, + "move_up": null, + "page_down": null, + "page_up": null }, "pager": { "close": null, @@ -3252,8 +3258,14 @@ "default": { "accept": null, "cancel": null, + "jump_bottom": null, + "jump_top": null, "move_down": null, - "move_up": null + "move_left": null, + "move_right": null, + "move_up": null, + "page_down": null, + "page_up": null } }, "pager": { @@ -3350,6 +3362,22 @@ ], "description": "Cancel and close selection view." }, + "jump_bottom": { + "allOf": [ + { + "$ref": "#/definitions/KeybindingsSpec" + } + ], + "description": "Jump to the last list item." + }, + "jump_top": { + "allOf": [ + { + "$ref": "#/definitions/KeybindingsSpec" + } + ], + "description": "Jump to the first list item." + }, "move_down": { "allOf": [ { @@ -3358,6 +3386,22 @@ ], "description": "Move list selection down." }, + "move_left": { + "allOf": [ + { + "$ref": "#/definitions/KeybindingsSpec" + } + ], + "description": "Move horizontally left in list pickers that support horizontal actions." + }, + "move_right": { + "allOf": [ + { + "$ref": "#/definitions/KeybindingsSpec" + } + ], + "description": "Move horizontally right in list pickers that support horizontal actions." + }, "move_up": { "allOf": [ { @@ -3365,6 +3409,22 @@ } ], "description": "Move list selection up." + }, + "page_down": { + "allOf": [ + { + "$ref": "#/definitions/KeybindingsSpec" + } + ], + "description": "Move list selection down by one page." + }, + "page_up": { + "allOf": [ + { + "$ref": "#/definitions/KeybindingsSpec" + } + ], + "description": "Move list selection up by one page." } }, "type": "object" diff --git a/codex-rs/tui/src/bottom_pane/app_link_view.rs b/codex-rs/tui/src/bottom_pane/app_link_view.rs index 3702849fc..7fe180a16 100644 --- a/codex-rs/tui/src/bottom_pane/app_link_view.rs +++ b/codex-rs/tui/src/bottom_pane/app_link_view.rs @@ -29,6 +29,8 @@ use crate::app::app_server_requests::ResolvedAppServerRequest; use crate::app_event::AppEvent; use crate::app_event_sender::AppEventSender; use crate::key_hint; +use crate::key_hint::KeyBindingListExt; +use crate::keymap::ListKeymap; use crate::render::Insets; use crate::render::RectExt as _; use crate::style::user_message_style; @@ -238,10 +240,24 @@ pub(crate) struct AppLinkView { screen: AppLinkScreen, selected_action: usize, complete: bool, + list_keymap: ListKeymap, } impl AppLinkView { + #[cfg(test)] pub(crate) fn new(params: AppLinkViewParams, app_event_tx: AppEventSender) -> Self { + Self::new_with_keymap( + params, + app_event_tx, + crate::keymap::RuntimeKeymap::defaults().list, + ) + } + + pub(crate) fn new_with_keymap( + params: AppLinkViewParams, + app_event_tx: AppEventSender, + list_keymap: ListKeymap, + ) -> Self { let AppLinkViewParams { app_id, title, @@ -269,6 +285,7 @@ impl AppLinkView { screen: AppLinkScreen::Link, selected_action: 0, complete: false, + list_keymap, } } @@ -679,10 +696,6 @@ impl BottomPaneView for AppLinkView { KeyEvent { code: KeyCode::Up, .. } - | KeyEvent { - code: KeyCode::Left, - .. - } | KeyEvent { code: KeyCode::BackTab, .. @@ -691,20 +704,12 @@ impl BottomPaneView for AppLinkView { code: KeyCode::Char('k'), modifiers: KeyModifiers::NONE, .. - } - | KeyEvent { - code: KeyCode::Char('h'), - modifiers: KeyModifiers::NONE, - .. } => self.move_selection_prev(), + _ if self.list_keymap.move_left.is_pressed(key_event) => self.move_selection_prev(), KeyEvent { code: KeyCode::Down, .. } - | KeyEvent { - code: KeyCode::Right, - .. - } | KeyEvent { code: KeyCode::Tab, .. } @@ -712,12 +717,8 @@ impl BottomPaneView for AppLinkView { code: KeyCode::Char('j'), modifiers: KeyModifiers::NONE, .. - } - | KeyEvent { - code: KeyCode::Char('l'), - modifiers: KeyModifiers::NONE, - .. } => self.move_selection_next(), + _ if self.list_keymap.move_right.is_pressed(key_event) => self.move_selection_next(), KeyEvent { code: KeyCode::Char(c), modifiers: KeyModifiers::NONE, @@ -1087,6 +1088,74 @@ mod tests { assert!(view.terminal_title_requires_action()); } + #[test] + fn horizontal_list_keys_move_action_selection() { + let (tx_raw, _rx) = unbounded_channel::(); + let tx = AppEventSender::new(tx_raw); + let mut view = AppLinkView::new( + AppLinkViewParams { + app_id: "connector_1".to_string(), + title: "Notion".to_string(), + description: None, + instructions: "Manage app".to_string(), + url: "https://example.test/notion".to_string(), + is_installed: true, + is_enabled: true, + suggest_reason: None, + suggestion_type: None, + elicitation_target: None, + }, + tx, + ); + + assert_eq!(view.selected_action, 0); + view.handle_key_event(KeyEvent::new(KeyCode::Char('l'), KeyModifiers::CONTROL)); + assert_eq!(view.selected_action, 1); + view.handle_key_event(KeyEvent::new(KeyCode::Char('h'), KeyModifiers::CONTROL)); + assert_eq!(view.selected_action, 0); + } + + #[test] + fn remapped_horizontal_list_keys_control_action_selection() { + let (tx_raw, _rx) = unbounded_channel::(); + let tx = AppEventSender::new(tx_raw); + let mut list_keymap = crate::keymap::RuntimeKeymap::defaults().list; + list_keymap.move_left = vec![key_hint::plain(KeyCode::Char('x'))]; + list_keymap.move_right = vec![key_hint::plain(KeyCode::Char('z'))]; + let mut view = AppLinkView::new_with_keymap( + AppLinkViewParams { + app_id: "connector_1".to_string(), + title: "Notion".to_string(), + description: None, + instructions: "Manage app".to_string(), + url: "https://example.test/notion".to_string(), + is_installed: true, + is_enabled: true, + suggest_reason: None, + suggestion_type: None, + elicitation_target: None, + }, + tx, + list_keymap, + ); + + assert_eq!(view.selected_action, 0); + view.handle_key_event(KeyEvent::new(KeyCode::Char('l'), KeyModifiers::NONE)); + assert_eq!(view.selected_action, 0); + view.handle_key_event(KeyEvent::new(KeyCode::Right, KeyModifiers::NONE)); + assert_eq!(view.selected_action, 0); + + view.handle_key_event(KeyEvent::new(KeyCode::Char('z'), KeyModifiers::NONE)); + assert_eq!(view.selected_action, 1); + view.handle_key_event(KeyEvent::new(KeyCode::Char('h'), KeyModifiers::NONE)); + assert_eq!(view.selected_action, 1); + view.handle_key_event(KeyEvent::new(KeyCode::Left, KeyModifiers::NONE)); + assert_eq!(view.selected_action, 1); + + view.handle_key_event(KeyEvent::new(KeyCode::Char('x'), KeyModifiers::NONE)); + assert_eq!(view.selected_action, 0); + } + #[test] fn toggle_action_sends_set_app_enabled_and_updates_label() { let (tx_raw, mut rx) = unbounded_channel::(); diff --git a/codex-rs/tui/src/bottom_pane/experimental_features_view.rs b/codex-rs/tui/src/bottom_pane/experimental_features_view.rs index c36d70c9f..6d57e58b6 100644 --- a/codex-rs/tui/src/bottom_pane/experimental_features_view.rs +++ b/codex-rs/tui/src/bottom_pane/experimental_features_view.rs @@ -13,6 +13,8 @@ use ratatui::widgets::Widget; use crate::app_event::AppEvent; use crate::app_event_sender::AppEventSender; use crate::key_hint; +use crate::key_hint::KeyBindingListExt; +use crate::keymap::ListKeymap; use crate::render::Insets; use crate::render::RectExt as _; use crate::render::renderable::ColumnRenderable; @@ -43,12 +45,14 @@ pub(crate) struct ExperimentalFeaturesView { app_event_tx: AppEventSender, header: Box, footer_hint: Line<'static>, + keymap: ListKeymap, } impl ExperimentalFeaturesView { pub(crate) fn new( features: Vec, app_event_tx: AppEventSender, + keymap: ListKeymap, ) -> Self { let mut header = ColumnRenderable::new(); header.push(Line::from("Experimental features".bold())); @@ -63,6 +67,7 @@ impl ExperimentalFeaturesView { app_event_tx, header: Box::new(header), footer_hint: experimental_popup_hint_line(), + keymap, }; view.initialize_selection(); view @@ -119,6 +124,30 @@ impl ExperimentalFeaturesView { self.state.ensure_visible(len, MAX_POPUP_ROWS.min(len)); } + fn page_up(&mut self) { + let len = self.visible_len(); + let visible = MAX_POPUP_ROWS.min(len); + self.state.page_up_clamped(len, visible); + } + + fn page_down(&mut self) { + let len = self.visible_len(); + let visible = MAX_POPUP_ROWS.min(len); + self.state.page_down_clamped(len, visible); + } + + fn jump_top(&mut self) { + let len = self.visible_len(); + let visible = MAX_POPUP_ROWS.min(len); + self.state.jump_top(len, visible); + } + + fn jump_bottom(&mut self) { + let len = self.visible_len(); + let visible = MAX_POPUP_ROWS.min(len); + self.state.jump_bottom(len, visible); + } + fn toggle_selected(&mut self) { let Some(selected_idx) = self.state.selected_idx else { return; @@ -137,56 +166,20 @@ impl ExperimentalFeaturesView { impl BottomPaneView for ExperimentalFeaturesView { fn handle_key_event(&mut self, key_event: KeyEvent) { match key_event { - KeyEvent { - code: KeyCode::Up, .. - } - | KeyEvent { - code: KeyCode::Char('p'), - modifiers: KeyModifiers::CONTROL, - .. - } - | KeyEvent { - code: KeyCode::Char('\u{0010}'), - modifiers: KeyModifiers::NONE, - .. - } => self.move_up(), - KeyEvent { - code: KeyCode::Char('k'), - modifiers: KeyModifiers::NONE, - .. - } => self.move_up(), - KeyEvent { - code: KeyCode::Down, - .. - } - | KeyEvent { - code: KeyCode::Char('n'), - modifiers: KeyModifiers::CONTROL, - .. - } - | KeyEvent { - code: KeyCode::Char('\u{000e}'), - modifiers: KeyModifiers::NONE, - .. - } => self.move_down(), - KeyEvent { - code: KeyCode::Char('j'), - modifiers: KeyModifiers::NONE, - .. - } => self.move_down(), + _ if self.keymap.move_up.is_pressed(key_event) => self.move_up(), + _ if self.keymap.move_down.is_pressed(key_event) => self.move_down(), + _ if self.keymap.page_up.is_pressed(key_event) => self.page_up(), + _ if self.keymap.page_down.is_pressed(key_event) => self.page_down(), + _ if self.keymap.jump_top.is_pressed(key_event) => self.jump_top(), + _ if self.keymap.jump_bottom.is_pressed(key_event) => self.jump_bottom(), KeyEvent { code: KeyCode::Char(' '), modifiers: KeyModifiers::NONE, .. } => self.toggle_selected(), - KeyEvent { - code: KeyCode::Enter, - modifiers: KeyModifiers::NONE, - .. - } - | KeyEvent { - code: KeyCode::Esc, .. - } => { + _ if self.keymap.accept.is_pressed(key_event) + || self.keymap.cancel.is_pressed(key_event) => + { self.on_ctrl_c(); } _ => {} diff --git a/codex-rs/tui/src/bottom_pane/hooks_browser_view.rs b/codex-rs/tui/src/bottom_pane/hooks_browser_view.rs index c9c7a8c61..462e9b286 100644 --- a/codex-rs/tui/src/bottom_pane/hooks_browser_view.rs +++ b/codex-rs/tui/src/bottom_pane/hooks_browser_view.rs @@ -29,6 +29,8 @@ use crate::app_event_sender::AppEventSender; use crate::hooks_rpc::HookTrustUpdate; use crate::hooks_rpc::hook_needs_review; use crate::key_hint; +use crate::key_hint::KeyBindingListExt; +use crate::keymap::ListKeymap; use crate::line_truncation::truncate_line_with_ellipsis_if_overflow; use crate::render::renderable::Renderable; use crate::status::format_directory_display; @@ -50,6 +52,7 @@ pub(crate) struct HooksBrowserView { state: ScrollState, complete: bool, app_event_tx: AppEventSender, + keymap: ListKeymap, } impl HooksBrowserView { @@ -68,10 +71,15 @@ impl HooksBrowserView { errors, }, app_event_tx, + crate::keymap::RuntimeKeymap::defaults().list, ) } - pub(crate) fn from_entry(mut entry: HooksListEntry, app_event_tx: AppEventSender) -> Self { + pub(crate) fn from_entry( + mut entry: HooksListEntry, + app_event_tx: AppEventSender, + keymap: ListKeymap, + ) -> Self { entry.hooks.sort_by_key(|hook| hook.display_order); let mut view = Self { entry, @@ -79,6 +87,7 @@ impl HooksBrowserView { state: ScrollState::new(), complete: false, app_event_tx, + keymap, }; if view.page_len() > 0 { view.state.selected_idx = Some( @@ -165,6 +174,26 @@ impl HooksBrowserView { self.state.ensure_visible(len, self.max_visible_rows()); } + fn page_up(&mut self) { + let len = self.page_len(); + self.state.page_up_clamped(len, self.max_visible_rows()); + } + + fn page_down(&mut self) { + let len = self.page_len(); + self.state.page_down_clamped(len, self.max_visible_rows()); + } + + fn jump_top(&mut self) { + let len = self.page_len(); + self.state.jump_top(len, self.max_visible_rows()); + } + + fn jump_bottom(&mut self) { + let len = self.page_len(); + self.state.jump_bottom(len, self.max_visible_rows()); + } + fn page_len(&self) -> usize { match self.page { HooksBrowserPage::Events => codex_protocol::protocol::HookEventName::iter().count(), @@ -533,33 +562,18 @@ impl HooksBrowserView { impl BottomPaneView for HooksBrowserView { fn handle_key_event(&mut self, key_event: KeyEvent) { match key_event { - KeyEvent { - code: KeyCode::Up, .. + _ if self.keymap.move_up.is_pressed(key_event) => self.move_up(), + _ if self.keymap.move_down.is_pressed(key_event) => self.move_down(), + _ if self.keymap.page_up.is_pressed(key_event) => self.page_up(), + _ if self.keymap.page_down.is_pressed(key_event) => self.page_down(), + _ if self.keymap.jump_top.is_pressed(key_event) => self.jump_top(), + _ if self.keymap.jump_bottom.is_pressed(key_event) => self.jump_bottom(), + _ if self.keymap.accept.is_pressed(key_event) + && self.page == HooksBrowserPage::Events => + { + self.open_selected_event() } - | KeyEvent { - code: KeyCode::Char('p'), - modifiers: KeyModifiers::CONTROL, - .. - } => self.move_up(), - KeyEvent { - code: KeyCode::Down, - .. - } - | KeyEvent { - code: KeyCode::Char('n'), - modifiers: KeyModifiers::CONTROL, - .. - } => self.move_down(), - KeyEvent { - code: KeyCode::Enter, - modifiers: KeyModifiers::NONE, - .. - } if self.page == HooksBrowserPage::Events => self.open_selected_event(), - KeyEvent { - code: KeyCode::Enter, - modifiers: KeyModifiers::NONE, - .. - } => { + _ if self.keymap.accept.is_pressed(key_event) => { if let HooksBrowserPage::Handlers(event_name) = self.page { self.toggle_selected_hook(event_name); } @@ -581,9 +595,7 @@ impl BottomPaneView for HooksBrowserView { HooksBrowserPage::Events => self.trust_all_hooks(), HooksBrowserPage::Handlers(event_name) => self.trust_selected_hook(event_name), }, - KeyEvent { - code: KeyCode::Esc, .. - } => match self.page { + _ if self.keymap.cancel.is_pressed(key_event) => match self.page { HooksBrowserPage::Events => self.close(), HooksBrowserPage::Handlers(_) => self.return_to_events(), }, diff --git a/codex-rs/tui/src/bottom_pane/list_selection_view.rs b/codex-rs/tui/src/bottom_pane/list_selection_view.rs index 2d0ac3717..94ef15cde 100644 --- a/codex-rs/tui/src/bottom_pane/list_selection_view.rs +++ b/codex-rs/tui/src/bottom_pane/list_selection_view.rs @@ -17,6 +17,7 @@ use super::selection_popup_common::wrap_styled_line; use crate::app_event_sender::AppEventSender; use crate::key_hint::KeyBinding; use crate::key_hint::KeyBindingListExt; +use crate::key_hint::is_plain_text_key_event; use crate::keymap::ListKeymap; use crate::render::renderable::ColumnRenderable; use crate::render::renderable::Renderable; @@ -655,8 +656,8 @@ impl ListSelectionView { let len = self.visible_len(); self.state.move_up_wrap(len); let visible = Self::max_visible_rows(len); - self.state.ensure_visible(len, visible); self.skip_disabled_up(); + self.state.ensure_visible(len, visible); if self.selected_actual_idx() != before { self.fire_selection_changed(); } @@ -667,8 +668,56 @@ impl ListSelectionView { let len = self.visible_len(); self.state.move_down_wrap(len); let visible = Self::max_visible_rows(len); - self.state.ensure_visible(len, visible); self.skip_disabled_down(); + self.state.ensure_visible(len, visible); + if self.selected_actual_idx() != before { + self.fire_selection_changed(); + } + } + + fn page_up(&mut self) { + let before = self.selected_actual_idx(); + let len = self.visible_len(); + let visible = Self::max_visible_rows(len); + self.state.page_up_clamped(len, visible); + self.skip_disabled_up_clamped(); + self.state.ensure_visible(len, visible); + if self.selected_actual_idx() != before { + self.fire_selection_changed(); + } + } + + fn page_down(&mut self) { + let before = self.selected_actual_idx(); + let len = self.visible_len(); + let visible = Self::max_visible_rows(len); + self.state.page_down_clamped(len, visible); + self.skip_disabled_down_clamped(); + self.state.ensure_visible(len, visible); + if self.selected_actual_idx() != before { + self.fire_selection_changed(); + } + } + + fn jump_top(&mut self) { + let before = self.selected_actual_idx(); + let len = self.visible_len(); + let visible = Self::max_visible_rows(len); + self.state.jump_top(len, visible); + self.skip_disabled_down_clamped(); + self.state.ensure_visible(len, visible); + if self.selected_actual_idx() != before { + self.fire_selection_changed(); + } + } + + fn jump_bottom(&mut self) { + let before = self.selected_actual_idx(); + let len = self.visible_len(); + let visible = Self::max_visible_rows(len); + self.state.jump_bottom(len, visible); + self.skip_disabled_up_clamped(); + self.state.ensure_visible(len, visible); if self.selected_actual_idx() != before { self.fire_selection_changed(); } @@ -788,13 +837,7 @@ impl ListSelectionView { fn skip_disabled_down(&mut self) { let len = self.visible_len(); for _ in 0..len { - if let Some(idx) = self.state.selected_idx - && let Some(actual_idx) = self.filtered_indices.get(idx) - && self - .active_items() - .get(*actual_idx) - .is_some_and(|item| item.disabled_reason.is_some() || item.is_disabled) - { + if self.selected_visible_idx_is_disabled() { self.state.move_down_wrap(len); } else { break; @@ -805,34 +848,70 @@ impl ListSelectionView { fn skip_disabled_up(&mut self) { let len = self.visible_len(); for _ in 0..len { - if let Some(idx) = self.state.selected_idx - && let Some(actual_idx) = self.filtered_indices.get(idx) - && self - .active_items() - .get(*actual_idx) - .is_some_and(|item| item.disabled_reason.is_some() || item.is_disabled) - { + if self.selected_visible_idx_is_disabled() { self.state.move_up_wrap(len); } else { break; } } } + + fn skip_disabled_down_clamped(&mut self) { + let Some(start) = self.state.selected_idx else { + return; + }; + if !self.visible_idx_is_disabled(start) { + return; + } + + let len = self.visible_len(); + self.state.selected_idx = ((start + 1)..len) + .find(|idx| !self.visible_idx_is_disabled(*idx)) + .or_else(|| { + (0..start) + .rev() + .find(|idx| !self.visible_idx_is_disabled(*idx)) + }) + .or(Some(start)); + } + + fn skip_disabled_up_clamped(&mut self) { + let Some(start) = self.state.selected_idx else { + return; + }; + if !self.visible_idx_is_disabled(start) { + return; + } + + let len = self.visible_len(); + self.state.selected_idx = (0..start) + .rev() + .find(|idx| !self.visible_idx_is_disabled(*idx)) + .or_else(|| ((start + 1)..len).find(|idx| !self.visible_idx_is_disabled(*idx))) + .or(Some(start)); + } + + fn selected_visible_idx_is_disabled(&self) -> bool { + self.state + .selected_idx + .is_some_and(|idx| self.visible_idx_is_disabled(idx)) + } + + fn visible_idx_is_disabled(&self, idx: usize) -> bool { + self.filtered_indices + .get(idx) + .and_then(|actual_idx| self.active_items().get(*actual_idx)) + .is_some_and(|item| item.disabled_reason.is_some() || item.is_disabled) + } } impl BottomPaneView for ListSelectionView { fn handle_key_event(&mut self, key_event: KeyEvent) { - let is_plain_text_char = matches!( - key_event, - KeyEvent { - code: KeyCode::Char(ch), - modifiers, - .. - } if !ch.is_ascii_control() - && !modifiers.contains(KeyModifiers::CONTROL) - && !modifiers.contains(KeyModifiers::ALT) - ); - let allow_plain_char_navigation = !self.is_searchable || !is_plain_text_char; + // Searchable lists reserve printable characters for query input. This + // keeps vim-style plain j/k/h/l useful in non-search lists without + // making those letters impossible to type into a filter. + let allow_plain_char_navigation = + !self.is_searchable || !is_plain_text_key_event(key_event); match key_event { _ if allow_plain_char_navigation && self.keymap.move_up.is_pressed(key_event) => { @@ -841,14 +920,30 @@ impl BottomPaneView for ListSelectionView { _ if allow_plain_char_navigation && self.keymap.move_down.is_pressed(key_event) => { self.move_down() } - KeyEvent { - code: KeyCode::Left, - .. - } if self.tabs_enabled() => self.switch_tab(/*step*/ -1), - KeyEvent { - code: KeyCode::Right, - .. - } if self.tabs_enabled() => self.switch_tab(/*step*/ 1), + _ if allow_plain_char_navigation && self.keymap.page_up.is_pressed(key_event) => { + self.page_up() + } + _ if allow_plain_char_navigation && self.keymap.page_down.is_pressed(key_event) => { + self.page_down() + } + _ if allow_plain_char_navigation && self.keymap.jump_top.is_pressed(key_event) => { + self.jump_top() + } + _ if allow_plain_char_navigation && self.keymap.jump_bottom.is_pressed(key_event) => { + self.jump_bottom() + } + _ if allow_plain_char_navigation + && self.tabs_enabled() + && self.keymap.move_left.is_pressed(key_event) => + { + self.switch_tab(/*step*/ -1) + } + _ if allow_plain_char_navigation + && self.tabs_enabled() + && self.keymap.move_right.is_pressed(key_event) => + { + self.switch_tab(/*step*/ 1) + } KeyEvent { code: KeyCode::Backspace, .. @@ -1618,6 +1713,10 @@ mod tests { assert_eq!(view.active_tab_id(), Some("alpha")); assert_eq!(view.search_query, ""); + view.handle_key_event(KeyEvent::new(KeyCode::Char('l'), KeyModifiers::CONTROL)); + assert_eq!(view.active_tab_id(), Some("beta")); + view.handle_key_event(KeyEvent::new(KeyCode::Char('h'), KeyModifiers::CONTROL)); + assert_eq!(view.active_tab_id(), Some("alpha")); let rendered = render_lines(&view); assert!( rendered.contains("Alpha Item") && !rendered.contains("Beta Item"), @@ -2059,6 +2158,77 @@ mod tests { assert_eq!(view.selected_actual_idx(), Some(1)); } + #[test] + fn page_and_jump_navigation_use_list_keymap() { + let (tx_raw, _rx) = unbounded_channel::(); + let tx = AppEventSender::new(tx_raw); + let mut keymap = crate::keymap::RuntimeKeymap::defaults().list; + keymap.page_down = vec![crate::key_hint::ctrl(KeyCode::Char('d'))]; + keymap.page_up = vec![crate::key_hint::ctrl(KeyCode::Char('u'))]; + keymap.jump_bottom = vec![crate::key_hint::ctrl(KeyCode::Char('e'))]; + keymap.jump_top = vec![crate::key_hint::ctrl(KeyCode::Char('a'))]; + let mut view = ListSelectionView::new( + SelectionViewParams { + items: (0..12) + .map(|idx| SelectionItem { + name: format!("Item {idx}"), + ..Default::default() + }) + .collect(), + ..Default::default() + }, + tx, + keymap, + ); + + view.handle_key_event(KeyEvent::from(KeyCode::PageDown)); + assert_eq!(view.selected_actual_idx(), Some(0)); + + view.handle_key_event(KeyEvent::new(KeyCode::Char('d'), KeyModifiers::CONTROL)); + assert_eq!(view.selected_actual_idx(), Some(8)); + + view.handle_key_event(KeyEvent::new(KeyCode::Char('u'), KeyModifiers::CONTROL)); + assert_eq!(view.selected_actual_idx(), Some(0)); + + view.handle_key_event(KeyEvent::new(KeyCode::Char('e'), KeyModifiers::CONTROL)); + assert_eq!(view.selected_actual_idx(), Some(11)); + + view.handle_key_event(KeyEvent::new(KeyCode::Char('a'), KeyModifiers::CONTROL)); + assert_eq!(view.selected_actual_idx(), Some(0)); + } + + #[test] + fn page_and_jump_navigation_skip_trailing_disabled_rows_without_wrapping() { + let (tx_raw, _rx) = unbounded_channel::(); + let tx = AppEventSender::new(tx_raw); + let mut view = ListSelectionView::new( + SelectionViewParams { + items: (0..12) + .map(|idx| SelectionItem { + name: format!("Item {idx}"), + is_disabled: idx >= 8, + ..Default::default() + }) + .collect(), + ..Default::default() + }, + tx, + crate::keymap::RuntimeKeymap::defaults().list, + ); + + view.handle_key_event(KeyEvent::from(KeyCode::PageDown)); + assert_eq!(view.selected_actual_idx(), Some(7)); + let selected = view.state.selected_idx.expect("selection should be set"); + assert!(view.state.scroll_top <= selected); + assert!(selected < view.state.scroll_top + ListSelectionView::max_visible_rows(/*len*/ 12)); + + view.handle_key_event(KeyEvent::from(KeyCode::End)); + assert_eq!(view.selected_actual_idx(), Some(7)); + let selected = view.state.selected_idx.expect("selection should be set"); + assert!(view.state.scroll_top <= selected); + assert!(selected < view.state.scroll_top + ListSelectionView::max_visible_rows(/*len*/ 12)); + } + #[test] fn wraps_long_option_without_overflowing_columns() { let (tx_raw, _rx) = unbounded_channel::(); diff --git a/codex-rs/tui/src/bottom_pane/mcp_server_elicitation.rs b/codex-rs/tui/src/bottom_pane/mcp_server_elicitation.rs index 95e8e8ca1..447ab8e6c 100644 --- a/codex-rs/tui/src/bottom_pane/mcp_server_elicitation.rs +++ b/codex-rs/tui/src/bottom_pane/mcp_server_elicitation.rs @@ -49,6 +49,8 @@ use crate::bottom_pane::selection_popup_common::menu_surface_inset; use crate::bottom_pane::selection_popup_common::menu_surface_padding_height; use crate::bottom_pane::selection_popup_common::render_menu_surface; use crate::bottom_pane::selection_popup_common::render_rows; +use crate::key_hint::KeyBindingListExt; +use crate::keymap::ListKeymap; use crate::render::renderable::Renderable; use crate::text_formatting::format_json_compact; use crate::text_formatting::truncate_text; @@ -711,15 +713,35 @@ pub(crate) struct McpServerElicitationOverlay { current_idx: usize, done: bool, validation_error: Option, + list_keymap: ListKeymap, } impl McpServerElicitationOverlay { + #[cfg(test)] pub(crate) fn new( request: McpServerElicitationFormRequest, app_event_tx: AppEventSender, has_input_focus: bool, enhanced_keys_supported: bool, disable_paste_burst: bool, + ) -> Self { + Self::new_with_keymap( + request, + app_event_tx, + has_input_focus, + enhanced_keys_supported, + disable_paste_burst, + crate::keymap::RuntimeKeymap::defaults().list, + ) + } + + pub(crate) fn new_with_keymap( + request: McpServerElicitationFormRequest, + app_event_tx: AppEventSender, + has_input_focus: bool, + enhanced_keys_supported: bool, + disable_paste_burst: bool, + list_keymap: ListKeymap, ) -> Self { let mut composer = ChatComposer::new_with_config( has_input_focus, @@ -739,6 +761,7 @@ impl McpServerElicitationOverlay { current_idx: 0, done: false, validation_error: None, + list_keymap, }; overlay.reset_for_request(); overlay.restore_current_draft(); @@ -1529,18 +1552,40 @@ impl BottomPaneView for McpServerElicitationOverlay { code: KeyCode::Left, modifiers: KeyModifiers::NONE, .. + } + | KeyEvent { + code: KeyCode::Char('h'), + modifiers: KeyModifiers::NONE, + .. } if self.current_field_is_select() => { self.move_field(/*next*/ false); return; } + _ if self.current_field_is_select() + && self.list_keymap.move_left.is_pressed(key_event) => + { + self.move_field(/*next*/ false); + return; + } KeyEvent { code: KeyCode::Right, modifiers: KeyModifiers::NONE, .. + } + | KeyEvent { + code: KeyCode::Char('l'), + modifiers: KeyModifiers::NONE, + .. } if self.current_field_is_select() => { self.move_field(/*next*/ true); return; } + _ if self.current_field_is_select() + && self.list_keymap.move_right.is_pressed(key_event) => + { + self.move_field(/*next*/ true); + return; + } _ => {} } @@ -2155,6 +2200,43 @@ mod tests { ); } + #[test] + fn horizontal_list_keys_move_between_select_fields() { + let (tx, _rx) = test_sender(); + let request = from_form_request( + ThreadId::default(), + form_request( + "Choose values", + serde_json::json!({ + "type": "object", + "properties": { + "first": { + "type": "boolean", + "title": "First", + }, + "second": { + "type": "boolean", + "title": "Second", + } + }, + "required": ["first", "second"], + }), + /*meta*/ None, + ), + ) + .expect("expected supported form"); + let mut overlay = McpServerElicitationOverlay::new( + request, tx, /*has_input_focus*/ true, /*enhanced_keys_supported*/ false, + /*disable_paste_burst*/ false, + ); + + assert_eq!(overlay.current_idx, 0); + overlay.handle_key_event(KeyEvent::new(KeyCode::Char('l'), KeyModifiers::CONTROL)); + assert_eq!(overlay.current_idx, 1); + overlay.handle_key_event(KeyEvent::new(KeyCode::Char('h'), KeyModifiers::CONTROL)); + assert_eq!(overlay.current_idx, 0); + } + #[test] fn empty_tool_approval_schema_session_choice_sets_persist_meta() { let (tx, mut rx) = test_sender(); diff --git a/codex-rs/tui/src/bottom_pane/memories_settings_view.rs b/codex-rs/tui/src/bottom_pane/memories_settings_view.rs index d2d3c00ce..06e9b31eb 100644 --- a/codex-rs/tui/src/bottom_pane/memories_settings_view.rs +++ b/codex-rs/tui/src/bottom_pane/memories_settings_view.rs @@ -14,6 +14,8 @@ use crate::app_event::AppEvent; use crate::app_event_sender::AppEventSender; use crate::bottom_pane::popup_consts::standard_popup_hint_line; use crate::key_hint; +use crate::key_hint::KeyBindingListExt; +use crate::keymap::ListKeymap; use crate::render::Insets; use crate::render::RectExt as _; use crate::render::renderable::ColumnRenderable; @@ -62,6 +64,7 @@ pub(crate) struct MemoriesSettingsView { complete: bool, app_event_tx: AppEventSender, docs_link: Line<'static>, + keymap: ListKeymap, } impl MemoriesSettingsView { @@ -69,6 +72,7 @@ impl MemoriesSettingsView { use_memories: bool, generate_memories: bool, app_event_tx: AppEventSender, + keymap: ListKeymap, ) -> Self { let mut view = Self { items: vec![ @@ -98,6 +102,7 @@ impl MemoriesSettingsView { "Learn more: ".dim(), MEMORIES_DOC_URL.cyan().underlined(), ]), + keymap, }; view.initialize_selection(); view @@ -216,6 +221,30 @@ impl MemoriesSettingsView { state.ensure_visible(len, MAX_POPUP_ROWS.min(len)); } + fn page_up(&mut self) { + let len = self.visible_len(); + let visible = MAX_POPUP_ROWS.min(len); + self.active_state_mut().page_up_clamped(len, visible); + } + + fn page_down(&mut self) { + let len = self.visible_len(); + let visible = MAX_POPUP_ROWS.min(len); + self.active_state_mut().page_down_clamped(len, visible); + } + + fn jump_top(&mut self) { + let len = self.visible_len(); + let visible = MAX_POPUP_ROWS.min(len); + self.active_state_mut().jump_top(len, visible); + } + + fn jump_bottom(&mut self) { + let len = self.visible_len(); + let visible = MAX_POPUP_ROWS.min(len); + self.active_state_mut().jump_bottom(len, visible); + } + fn toggle_selected(&mut self) { if self.reset_confirmation.is_some() { return; @@ -271,56 +300,19 @@ impl MemoriesSettingsView { impl BottomPaneView for MemoriesSettingsView { fn handle_key_event(&mut self, key_event: KeyEvent) { match key_event { - KeyEvent { - code: KeyCode::Up, .. - } - | KeyEvent { - code: KeyCode::Char('p'), - modifiers: KeyModifiers::CONTROL, - .. - } - | KeyEvent { - code: KeyCode::Char('\u{0010}'), - modifiers: KeyModifiers::NONE, - .. - } => self.move_up(), - KeyEvent { - code: KeyCode::Char('k'), - modifiers: KeyModifiers::NONE, - .. - } => self.move_up(), - KeyEvent { - code: KeyCode::Down, - .. - } - | KeyEvent { - code: KeyCode::Char('n'), - modifiers: KeyModifiers::CONTROL, - .. - } - | KeyEvent { - code: KeyCode::Char('\u{000e}'), - modifiers: KeyModifiers::NONE, - .. - } => self.move_down(), - KeyEvent { - code: KeyCode::Char('j'), - modifiers: KeyModifiers::NONE, - .. - } => self.move_down(), + _ if self.keymap.move_up.is_pressed(key_event) => self.move_up(), + _ if self.keymap.move_down.is_pressed(key_event) => self.move_down(), + _ if self.keymap.page_up.is_pressed(key_event) => self.page_up(), + _ if self.keymap.page_down.is_pressed(key_event) => self.page_down(), + _ if self.keymap.jump_top.is_pressed(key_event) => self.jump_top(), + _ if self.keymap.jump_bottom.is_pressed(key_event) => self.jump_bottom(), KeyEvent { code: KeyCode::Char(' '), modifiers: KeyModifiers::NONE, .. } => self.toggle_selected(), - KeyEvent { - code: KeyCode::Enter, - modifiers: KeyModifiers::NONE, - .. - } => self.save(), - KeyEvent { - code: KeyCode::Esc, .. - } => self.cancel(), + _ if self.keymap.accept.is_pressed(key_event) => self.save(), + _ if self.keymap.cancel.is_pressed(key_event) => self.cancel(), _ => {} } } diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index 5a0f967ee..3faea3f81 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -1082,6 +1082,10 @@ impl BottomPane { popup_consts::standard_popup_hint_line_for_keymap(&self.keymap.list) } + pub(crate) fn list_keymap(&self) -> crate::keymap::ListKeymap { + self.keymap.list.clone() + } + /// Replace one or more active views whose IDs are in `view_ids` with a /// generic list selection view. pub(crate) fn replace_active_views_with_selection_view( @@ -1298,12 +1302,13 @@ impl BottomPane { request }; - let modal = RequestUserInputOverlay::new( + let modal = RequestUserInputOverlay::new_with_keymap( request, self.app_event_tx.clone(), self.has_input_focus, self.enhanced_keys_supported, self.disable_paste_burst, + self.keymap.list.clone(), ); self.pause_status_timer_for_modal(); self.set_composer_input_enabled( @@ -1342,7 +1347,7 @@ impl BottomPane { tool_suggestion.suggest_type, mcp_server_elicitation::ToolSuggestionType::Enable ); - let view = AppLinkView::new( + let view = AppLinkView::new_with_keymap( AppLinkViewParams { app_id: tool_suggestion.tool_id.clone(), title: tool_suggestion.tool_name.clone(), @@ -1373,6 +1378,7 @@ impl BottomPane { }), }, self.app_event_tx.clone(), + self.keymap.list.clone(), ); self.pause_status_timer_for_modal(); self.set_composer_input_enabled( @@ -1383,12 +1389,13 @@ impl BottomPane { return; } - let modal = McpServerElicitationOverlay::new( + let modal = McpServerElicitationOverlay::new_with_keymap( request, self.app_event_tx.clone(), self.has_input_focus, self.enhanced_keys_supported, self.disable_paste_burst, + self.keymap.list.clone(), ); self.pause_status_timer_for_modal(); self.set_composer_input_enabled( diff --git a/codex-rs/tui/src/bottom_pane/multi_select_picker.rs b/codex-rs/tui/src/bottom_pane/multi_select_picker.rs index 6b046e7ea..619ff31b5 100644 --- a/codex-rs/tui/src/bottom_pane/multi_select_picker.rs +++ b/codex-rs/tui/src/bottom_pane/multi_select_picker.rs @@ -61,6 +61,11 @@ use crate::bottom_pane::popup_consts::MAX_POPUP_ROWS; use crate::bottom_pane::scroll_state::ScrollState; use crate::bottom_pane::selection_popup_common::render_rows_single_line; use crate::key_hint; +use crate::key_hint::KeyBindingListExt; +use crate::key_hint::is_plain_text_key_event; +use crate::keymap::ListKeymap; +use crate::keymap::RuntimeKeymap; +use crate::keymap::primary_binding; use crate::line_truncation::truncate_line_with_ellipsis_if_overflow; use crate::render::Insets; use crate::render::RectExt; @@ -182,6 +187,9 @@ pub(crate) struct MultiSelectPicker { /// Whether left/right arrow reordering is enabled. ordering_enabled: bool, + /// Shared list keybindings for navigation and completion. + keymap: ListKeymap, + /// Optional callback to generate a preview line from current item states. preview_builder: Option, @@ -352,6 +360,30 @@ impl MultiSelectPicker { self.state.ensure_visible(len, visible); } + fn page_up(&mut self) { + let len = self.visible_len(); + let visible = Self::max_visible_rows(len); + self.state.page_up_clamped(len, visible); + } + + fn page_down(&mut self) { + let len = self.visible_len(); + let visible = Self::max_visible_rows(len); + self.state.page_down_clamped(len, visible); + } + + fn jump_top(&mut self) { + let len = self.visible_len(); + let visible = Self::max_visible_rows(len); + self.state.jump_top(len, visible); + } + + fn jump_bottom(&mut self) { + let len = self.visible_len(); + let visible = Self::max_visible_rows(len); + self.state.jump_bottom(len, visible); + } + /// Toggles the enabled state of the currently selected item. /// /// Updates the preview line and invokes the `on_change` callback if set. @@ -493,50 +525,41 @@ impl BottomPaneView for MultiSelectPicker { } fn handle_key_event(&mut self, key_event: KeyEvent) { + // Printable characters always feed search. Movement aliases such as + // plain j/k only apply through non-text events or modified bindings. + let allow_plain_char_navigation = !is_plain_text_key_event(key_event); + match key_event { - KeyEvent { code: KeyCode::Left, .. } if self.ordering_enabled => { + _ if allow_plain_char_navigation + && self.ordering_enabled + && self.keymap.move_left.is_pressed(key_event) => + { self.move_selected_item(Direction::Up); } - KeyEvent { code: KeyCode::Right, .. } if self.ordering_enabled => { + _ if allow_plain_char_navigation + && self.ordering_enabled + && self.keymap.move_right.is_pressed(key_event) => + { self.move_selected_item(Direction::Down); } - KeyEvent { - code: KeyCode::Up, .. + _ if allow_plain_char_navigation && self.keymap.move_up.is_pressed(key_event) => { + self.move_up() } - | KeyEvent { - code: KeyCode::Char('p'), - modifiers: KeyModifiers::CONTROL, - .. + _ if allow_plain_char_navigation && self.keymap.move_down.is_pressed(key_event) => { + self.move_down() } - | KeyEvent { - code: KeyCode::Char('k'), - modifiers: KeyModifiers::CONTROL, - .. + _ if allow_plain_char_navigation && self.keymap.page_up.is_pressed(key_event) => { + self.page_up() } - | KeyEvent { - code: KeyCode::Char('\u{0010}'), - modifiers: KeyModifiers::NONE, - .. - } /* ^P */ => self.move_up(), - KeyEvent { - code: KeyCode::Down, - .. + _ if allow_plain_char_navigation && self.keymap.page_down.is_pressed(key_event) => { + self.page_down() } - | KeyEvent { - code: KeyCode::Char('j'), - modifiers: KeyModifiers::CONTROL, - .. + _ if allow_plain_char_navigation && self.keymap.jump_top.is_pressed(key_event) => { + self.jump_top() } - | KeyEvent { - code: KeyCode::Char('n'), - modifiers: KeyModifiers::CONTROL, - .. + _ if allow_plain_char_navigation && self.keymap.jump_bottom.is_pressed(key_event) => { + self.jump_bottom() } - | KeyEvent { - code: KeyCode::Char('\u{000e}'), - modifiers: KeyModifiers::NONE, - .. - } /* ^N */ => self.move_down(), KeyEvent { code: KeyCode::Backspace, .. @@ -549,15 +572,8 @@ impl BottomPaneView for MultiSelectPicker { modifiers: KeyModifiers::NONE, .. } => self.toggle_selected(), - KeyEvent { - code: KeyCode::Enter, - .. - } => self.confirm_selection(), - KeyEvent { - code: KeyCode::Esc, .. - } => { - self.close(); - } + _ if self.keymap.accept.is_pressed(key_event) => self.confirm_selection(), + _ if self.keymap.cancel.is_pressed(key_event) => self.close(), KeyEvent { code: KeyCode::Char(c), modifiers, @@ -703,6 +719,7 @@ pub(crate) struct MultiSelectPickerBuilder { items: Vec, ordering_enabled: bool, app_event_tx: AppEventSender, + keymap: ListKeymap, preview_builder: Option, on_change: Option, on_confirm: Option, @@ -719,6 +736,7 @@ impl MultiSelectPickerBuilder { items: Vec::new(), ordering_enabled: false, app_event_tx, + keymap: RuntimeKeymap::defaults().list, preview_builder: None, on_change: None, on_confirm: None, @@ -732,15 +750,6 @@ impl MultiSelectPickerBuilder { self } - /// Sets custom instruction spans for the footer hint line. - /// - /// If not set, default instructions are shown (Space to toggle, Enter to - /// confirm, Escape to close). - pub fn instructions(mut self, instructions: Vec>) -> Self { - self.instructions = instructions; - self - } - /// Enables left/right arrow keys for reordering items. /// /// Reordering is only active when the search query is empty. @@ -749,6 +758,12 @@ impl MultiSelectPickerBuilder { self } + /// Sets the shared list keymap used for navigation and completion. + pub fn list_keymap(mut self, keymap: ListKeymap) -> Self { + self.keymap = keymap; + self + } + /// Sets a callback to generate a preview line from the current item states. /// /// The callback receives all items and should return a [`Line`] to display, @@ -806,15 +821,34 @@ impl MultiSelectPickerBuilder { } let instructions = if self.instructions.is_empty() { - vec![ + let mut spans = vec![ "Press ".into(), key_hint::plain(KeyCode::Char(' ')).into(), - " to toggle; ".into(), - key_hint::plain(KeyCode::Enter).into(), - " to confirm and close; ".into(), - key_hint::plain(KeyCode::Esc).into(), - " to close".into(), - ] + " to toggle".into(), + ]; + if self.ordering_enabled + && let (Some(move_left), Some(move_right)) = ( + primary_binding(&self.keymap.move_left), + primary_binding(&self.keymap.move_right), + ) + { + spans.push("; ".into()); + spans.push(move_left.into()); + spans.push("/".into()); + spans.push(move_right.into()); + spans.push(" to move".into()); + } + if let Some(accept) = primary_binding(&self.keymap.accept) { + spans.push("; ".into()); + spans.push(accept.into()); + spans.push(" to confirm and close".into()); + } + if let Some(cancel) = primary_binding(&self.keymap.cancel) { + spans.push("; ".into()); + spans.push(cancel.into()); + spans.push(" to close".into()); + } + spans } else { self.instructions }; @@ -827,6 +861,7 @@ impl MultiSelectPickerBuilder { header: Box::new(header), footer_hint: Line::from(instructions), ordering_enabled: self.ordering_enabled, + keymap: self.keymap, search_query: String::new(), filtered_indices: Vec::new(), preview_builder: self.preview_builder, @@ -940,6 +975,38 @@ mod tests { ); } + #[test] + fn horizontal_list_keys_reorder_orderable_items() { + let mut picker = test_picker(vec![ + item( + "model", /*orderable*/ true, /*section_break_after*/ false, + ), + item( + "branch", /*orderable*/ true, /*section_break_after*/ false, + ), + ]); + + picker.handle_key_event(KeyEvent::new(KeyCode::Char('l'), KeyModifiers::CONTROL)); + assert_eq!( + picker + .items + .iter() + .map(|item| item.id.as_str()) + .collect::>(), + vec!["branch", "model"] + ); + + picker.handle_key_event(KeyEvent::new(KeyCode::Char('h'), KeyModifiers::CONTROL)); + assert_eq!( + picker + .items + .iter() + .map(|item| item.id.as_str()) + .collect::>(), + vec!["model", "branch"] + ); + } + #[test] fn section_break_after_item_renders_separator_row() { let picker = test_picker(vec![ @@ -964,4 +1031,65 @@ mod tests { ); assert_eq!(rows.state.selected_idx, Some(0)); } + + #[test] + fn searchable_plain_j_updates_query_instead_of_navigating() { + let mut picker = test_picker(vec![ + item( + "alpha", /*orderable*/ true, /*section_break_after*/ false, + ), + item( + "jupiter", /*orderable*/ true, /*section_break_after*/ false, + ), + ]); + + picker.handle_key_event(KeyEvent::new(KeyCode::Char('j'), KeyModifiers::NONE)); + + assert_eq!(picker.search_query, "j"); + assert_eq!(picker.filtered_indices, vec![1]); + assert_eq!(picker.state.selected_idx, Some(0)); + } + + #[test] + fn page_and_jump_navigation_use_list_keymap() { + let (tx, _rx) = unbounded_channel::(); + let mut keymap = RuntimeKeymap::defaults().list; + keymap.page_down = vec![key_hint::ctrl(KeyCode::Char('d'))]; + keymap.page_up = vec![key_hint::ctrl(KeyCode::Char('u'))]; + keymap.jump_bottom = vec![key_hint::ctrl(KeyCode::Char('e'))]; + keymap.jump_top = vec![key_hint::ctrl(KeyCode::Char('a'))]; + let mut picker = MultiSelectPicker::builder( + "Test".to_string(), + /*subtitle*/ None, + AppEventSender::new(tx), + ) + .items( + (0..12) + .map(|idx| { + item( + &format!("item-{idx}"), + /*orderable*/ true, + /*section_break_after*/ false, + ) + }) + .collect(), + ) + .list_keymap(keymap) + .build(); + + picker.handle_key_event(KeyEvent::from(KeyCode::PageDown)); + assert_eq!(picker.state.selected_idx, Some(0)); + + picker.handle_key_event(KeyEvent::new(KeyCode::Char('d'), KeyModifiers::CONTROL)); + assert_eq!(picker.state.selected_idx, Some(8)); + + picker.handle_key_event(KeyEvent::new(KeyCode::Char('u'), KeyModifiers::CONTROL)); + assert_eq!(picker.state.selected_idx, Some(0)); + + picker.handle_key_event(KeyEvent::new(KeyCode::Char('e'), KeyModifiers::CONTROL)); + assert_eq!(picker.state.selected_idx, Some(11)); + + picker.handle_key_event(KeyEvent::new(KeyCode::Char('a'), KeyModifiers::CONTROL)); + assert_eq!(picker.state.selected_idx, Some(0)); + } } diff --git a/codex-rs/tui/src/bottom_pane/request_user_input/mod.rs b/codex-rs/tui/src/bottom_pane/request_user_input/mod.rs index 1b9e8c975..68c6755e4 100644 --- a/codex-rs/tui/src/bottom_pane/request_user_input/mod.rs +++ b/codex-rs/tui/src/bottom_pane/request_user_input/mod.rs @@ -29,6 +29,8 @@ use crate::bottom_pane::scroll_state::ScrollState; use crate::bottom_pane::selection_popup_common::GenericDisplayRow; use crate::bottom_pane::selection_popup_common::measure_rows_height; use crate::history_cell; +use crate::key_hint::KeyBindingListExt; +use crate::keymap::ListKeymap; use crate::render::renderable::Renderable; #[cfg(test)] @@ -138,15 +140,35 @@ pub(crate) struct RequestUserInputOverlay { done: bool, pending_submission_draft: Option, confirm_unanswered: Option, + list_keymap: ListKeymap, } impl RequestUserInputOverlay { + #[cfg(test)] pub(crate) fn new( request: ToolRequestUserInputParams, app_event_tx: AppEventSender, has_input_focus: bool, enhanced_keys_supported: bool, disable_paste_burst: bool, + ) -> Self { + Self::new_with_keymap( + request, + app_event_tx, + has_input_focus, + enhanced_keys_supported, + disable_paste_burst, + crate::keymap::RuntimeKeymap::defaults().list, + ) + } + + pub(crate) fn new_with_keymap( + request: ToolRequestUserInputParams, + app_event_tx: AppEventSender, + has_input_focus: bool, + enhanced_keys_supported: bool, + disable_paste_burst: bool, + list_keymap: ListKeymap, ) -> Self { // Use the same composer widget, but disable popups/slash-commands and // image-path attachment so it behaves like a focused notes field. @@ -171,6 +193,7 @@ impl RequestUserInputOverlay { done: false, pending_submission_draft: None, confirm_unanswered: None, + list_keymap, }; overlay.reset_for_request(); overlay.ensure_focus_available(); @@ -1063,11 +1086,8 @@ impl BottomPaneView for RequestUserInputOverlay { code: KeyCode::Char('h'), modifiers: KeyModifiers::NONE, .. - } if self.has_options() && matches!(self.focus, Focus::Options) => { - self.move_question(/*next*/ false); - return; } - KeyEvent { + | KeyEvent { code: KeyCode::Left, modifiers: KeyModifiers::NONE, .. @@ -1075,19 +1095,30 @@ impl BottomPaneView for RequestUserInputOverlay { self.move_question(/*next*/ false); return; } + _ if self.has_options() + && matches!(self.focus, Focus::Options) + && self.list_keymap.move_left.is_pressed(key_event) => + { + self.move_question(/*next*/ false); + return; + } KeyEvent { code: KeyCode::Char('l'), modifiers: KeyModifiers::NONE, .. + } + | KeyEvent { + code: KeyCode::Right, + modifiers: KeyModifiers::NONE, + .. } if self.has_options() && matches!(self.focus, Focus::Options) => { self.move_question(/*next*/ true); return; } - KeyEvent { - code: KeyCode::Right, - modifiers: KeyModifiers::NONE, - .. - } if self.has_options() && matches!(self.focus, Focus::Options) => { + _ if self.has_options() + && matches!(self.focus, Focus::Options) + && self.list_keymap.move_right.is_pressed(key_event) => + { self.move_question(/*next*/ true); return; } @@ -1886,6 +1917,30 @@ mod tests { assert_eq!(overlay.current_index(), 0); } + #[test] + fn horizontal_list_keys_move_between_questions_in_options() { + let (tx, _rx) = test_sender(); + let mut overlay = RequestUserInputOverlay::new( + request_event( + "turn-1", + vec![ + question_with_options("q1", "Pick one"), + question_with_options("q2", "Pick two"), + ], + ), + tx, + /*has_input_focus*/ true, + /*enhanced_keys_supported*/ false, + /*disable_paste_burst*/ false, + ); + + assert_eq!(overlay.current_index(), 0); + overlay.handle_key_event(KeyEvent::new(KeyCode::Char('l'), KeyModifiers::CONTROL)); + assert_eq!(overlay.current_index(), 1); + overlay.handle_key_event(KeyEvent::new(KeyCode::Char('h'), KeyModifiers::CONTROL)); + assert_eq!(overlay.current_index(), 0); + } + #[test] fn options_notes_focus_hides_question_navigation_tip() { let (tx, _rx) = test_sender(); diff --git a/codex-rs/tui/src/bottom_pane/scroll_state.rs b/codex-rs/tui/src/bottom_pane/scroll_state.rs index a9728d1a0..a76f3821b 100644 --- a/codex-rs/tui/src/bottom_pane/scroll_state.rs +++ b/codex-rs/tui/src/bottom_pane/scroll_state.rs @@ -4,6 +4,13 @@ /// - Optional selection (None when list is empty) /// - Wrap-around navigation on Up/Down /// - Maintaining a scroll window (`scroll_top`) so the selected row stays visible +/// +/// Callers own the filtered row count and the visible window size. Every +/// mutation method takes those values instead of caching them here, so list +/// views can apply filters, pagination, or density changes without this helper +/// knowing about their data model. Passing a stale length after filtering would +/// leave selection pointing at the wrong row, so callers should clamp or move +/// through this type immediately after changing their visible row set. #[derive(Debug, Default, Clone, Copy)] pub(crate) struct ScrollState { pub selected_idx: Option, @@ -26,20 +33,15 @@ impl ScrollState { /// Clamp selection to be within the [0, len-1] range, or None when empty. pub fn clamp_selection(&mut self, len: usize) { - self.selected_idx = match len { - 0 => None, - _ => Some(self.selected_idx.unwrap_or(0).min(len - 1)), - }; - if len == 0 { - self.scroll_top = 0; + if self.clear_if_empty(len) { + return; } + self.selected_idx = Some(self.selected_idx.unwrap_or(0).min(len - 1)); } /// Move selection up by one, wrapping to the bottom when necessary. pub fn move_up_wrap(&mut self, len: usize) { - if len == 0 { - self.selected_idx = None; - self.scroll_top = 0; + if self.clear_if_empty(len) { return; } self.selected_idx = Some(match self.selected_idx { @@ -51,9 +53,7 @@ impl ScrollState { /// Move selection down by one, wrapping to the top when necessary. pub fn move_down_wrap(&mut self, len: usize) { - if len == 0 { - self.selected_idx = None; - self.scroll_top = 0; + if self.clear_if_empty(len) { return; } self.selected_idx = Some(match self.selected_idx { @@ -62,6 +62,63 @@ impl ScrollState { }); } + /// Move selection up by one visible page, clamping at the first row. + /// + /// Page movement intentionally does not wrap. It mirrors terminal list + /// behavior where repeated page-up/page-down converges at the nearest edge + /// while still keeping the selected row visible. + pub fn page_up_clamped(&mut self, len: usize, visible_rows: usize) { + if self.clear_if_empty(len) { + return; + } + let step = visible_rows.max(1); + let current = self.selected_idx.unwrap_or(0).min(len - 1); + self.selected_idx = Some(current.saturating_sub(step)); + self.ensure_visible(len, visible_rows); + } + + /// Move selection down by one visible page, clamping at the last row. + /// + /// Page movement intentionally does not wrap. It mirrors terminal list + /// behavior where repeated page-up/page-down converges at the nearest edge + /// while still keeping the selected row visible. + pub fn page_down_clamped(&mut self, len: usize, visible_rows: usize) { + if self.clear_if_empty(len) { + return; + } + let step = visible_rows.max(1); + let current = self.selected_idx.unwrap_or(0).min(len - 1); + self.selected_idx = Some(current.saturating_add(step).min(len - 1)); + self.ensure_visible(len, visible_rows); + } + + /// Jump selection to the first row. + pub fn jump_top(&mut self, len: usize, visible_rows: usize) { + if self.clear_if_empty(len) { + return; + } + self.selected_idx = Some(0); + self.ensure_visible(len, visible_rows); + } + + /// Jump selection to the last row. + pub fn jump_bottom(&mut self, len: usize, visible_rows: usize) { + if self.clear_if_empty(len) { + return; + } + self.selected_idx = Some(len - 1); + self.ensure_visible(len, visible_rows); + } + + fn clear_if_empty(&mut self, len: usize) -> bool { + if len != 0 { + return false; + } + self.selected_idx = None; + self.scroll_top = 0; + true + } + /// Adjust `scroll_top` so that the current `selected_idx` is visible within /// the window of `visible_rows`. pub fn ensure_visible(&mut self, len: usize, visible_rows: usize) { @@ -112,4 +169,36 @@ mod tests { assert_eq!(s.selected_idx, Some(0)); assert_eq!(s.scroll_top, 0); } + + #[test] + fn page_and_jump_navigation_clamps() { + let mut s = ScrollState::new(); + let len = 10; + let vis = 4; + + s.clamp_selection(len); + s.page_down_clamped(len, vis); + assert_eq!(s.selected_idx, Some(4)); + assert_eq!(s.scroll_top, 1); + + s.page_down_clamped(len, vis); + assert_eq!(s.selected_idx, Some(8)); + assert_eq!(s.scroll_top, 5); + + s.page_down_clamped(len, vis); + assert_eq!(s.selected_idx, Some(9)); + assert_eq!(s.scroll_top, 6); + + s.page_up_clamped(len, vis); + assert_eq!(s.selected_idx, Some(5)); + assert_eq!(s.scroll_top, 5); + + s.jump_top(len, vis); + assert_eq!(s.selected_idx, Some(0)); + assert_eq!(s.scroll_top, 0); + + s.jump_bottom(len, vis); + assert_eq!(s.selected_idx, Some(9)); + assert_eq!(s.scroll_top, 6); + } } diff --git a/codex-rs/tui/src/bottom_pane/skills_toggle_view.rs b/codex-rs/tui/src/bottom_pane/skills_toggle_view.rs index 85c014f66..5a5414ed4 100644 --- a/codex-rs/tui/src/bottom_pane/skills_toggle_view.rs +++ b/codex-rs/tui/src/bottom_pane/skills_toggle_view.rs @@ -14,6 +14,10 @@ use ratatui::widgets::Widget; use crate::app_event::AppEvent; use crate::app_event_sender::AppEventSender; use crate::key_hint; +use crate::key_hint::KeyBindingListExt; +use crate::key_hint::is_plain_text_key_event; +use crate::keymap::ListKeymap; +use crate::keymap::primary_binding; use crate::render::Insets; use crate::render::RectExt as _; use crate::render::renderable::ColumnRenderable; @@ -49,10 +53,15 @@ pub(crate) struct SkillsToggleView { footer_hint: Line<'static>, search_query: String, filtered_indices: Vec, + keymap: ListKeymap, } impl SkillsToggleView { - pub(crate) fn new(items: Vec, app_event_tx: AppEventSender) -> Self { + pub(crate) fn new( + items: Vec, + app_event_tx: AppEventSender, + keymap: ListKeymap, + ) -> Self { let mut header = ColumnRenderable::new(); header.push(Line::from("Enable/Disable Skills".bold())); header.push(Line::from( @@ -65,9 +74,10 @@ impl SkillsToggleView { complete: false, app_event_tx, header: Box::new(header), - footer_hint: skills_toggle_hint_line(), + footer_hint: skills_toggle_hint_line(&keymap), search_query: String::new(), filtered_indices: Vec::new(), + keymap, }; view.apply_filter(); view @@ -161,6 +171,30 @@ impl SkillsToggleView { self.state.ensure_visible(len, visible); } + fn page_up(&mut self) { + let len = self.visible_len(); + let visible = Self::max_visible_rows(len); + self.state.page_up_clamped(len, visible); + } + + fn page_down(&mut self) { + let len = self.visible_len(); + let visible = Self::max_visible_rows(len); + self.state.page_down_clamped(len, visible); + } + + fn jump_top(&mut self) { + let len = self.visible_len(); + let visible = Self::max_visible_rows(len); + self.state.jump_top(len, visible); + } + + fn jump_bottom(&mut self) { + let len = self.visible_len(); + let visible = Self::max_visible_rows(len); + self.state.jump_bottom(len, visible); + } + fn toggle_selected(&mut self) { let Some(idx) = self.state.selected_idx else { return; @@ -200,34 +234,29 @@ impl SkillsToggleView { impl BottomPaneView for SkillsToggleView { fn handle_key_event(&mut self, key_event: KeyEvent) { + // Printable characters always feed search. Movement aliases such as + // plain j/k only apply through non-text events or modified bindings. + let allow_plain_char_navigation = !is_plain_text_key_event(key_event); + match key_event { - KeyEvent { - code: KeyCode::Up, .. + _ if allow_plain_char_navigation && self.keymap.move_up.is_pressed(key_event) => { + self.move_up() } - | KeyEvent { - code: KeyCode::Char('p'), - modifiers: KeyModifiers::CONTROL, - .. + _ if allow_plain_char_navigation && self.keymap.move_down.is_pressed(key_event) => { + self.move_down() } - | KeyEvent { - code: KeyCode::Char('\u{0010}'), - modifiers: KeyModifiers::NONE, - .. - } /* ^P */ => self.move_up(), - KeyEvent { - code: KeyCode::Down, - .. + _ if allow_plain_char_navigation && self.keymap.page_up.is_pressed(key_event) => { + self.page_up() } - | KeyEvent { - code: KeyCode::Char('n'), - modifiers: KeyModifiers::CONTROL, - .. + _ if allow_plain_char_navigation && self.keymap.page_down.is_pressed(key_event) => { + self.page_down() + } + _ if allow_plain_char_navigation && self.keymap.jump_top.is_pressed(key_event) => { + self.jump_top() + } + _ if allow_plain_char_navigation && self.keymap.jump_bottom.is_pressed(key_event) => { + self.jump_bottom() } - | KeyEvent { - code: KeyCode::Char('\u{000e}'), - modifiers: KeyModifiers::NONE, - .. - } /* ^N */ => self.move_down(), KeyEvent { code: KeyCode::Backspace, .. @@ -239,15 +268,9 @@ impl BottomPaneView for SkillsToggleView { code: KeyCode::Char(' '), modifiers: KeyModifiers::NONE, .. - } - | KeyEvent { - code: KeyCode::Enter, - modifiers: KeyModifiers::NONE, - .. } => self.toggle_selected(), - KeyEvent { - code: KeyCode::Esc, .. - } => { + _ if self.keymap.accept.is_pressed(key_event) => self.toggle_selected(), + _ if self.keymap.cancel.is_pressed(key_event) => { self.on_ctrl_c(); } KeyEvent { @@ -364,16 +387,37 @@ impl Renderable for SkillsToggleView { } } -fn skills_toggle_hint_line() -> Line<'static> { - Line::from(vec![ - "Press ".into(), - key_hint::plain(KeyCode::Char(' ')).into(), - " or ".into(), - key_hint::plain(KeyCode::Enter).into(), - " to toggle; ".into(), - key_hint::plain(KeyCode::Esc).into(), - " to close".into(), - ]) +fn skills_toggle_hint_line(keymap: &ListKeymap) -> Line<'static> { + let space = key_hint::plain(KeyCode::Char(' ')); + let accept = primary_binding(&keymap.accept).filter(|binding| *binding != space); + let cancel = primary_binding(&keymap.cancel); + + match (accept, cancel) { + (Some(accept), Some(cancel)) => Line::from(vec![ + "Press ".into(), + space.into(), + " or ".into(), + accept.into(), + " to toggle; ".into(), + cancel.into(), + " to close".into(), + ]), + (Some(accept), None) => Line::from(vec![ + "Press ".into(), + space.into(), + " or ".into(), + accept.into(), + " to toggle".into(), + ]), + (None, Some(cancel)) => Line::from(vec![ + "Press ".into(), + space.into(), + " to toggle; ".into(), + cancel.into(), + " to close".into(), + ]), + (None, None) => Line::from(vec!["Press ".into(), space.into(), " to toggle".into()]), + } } #[cfg(test)] @@ -429,7 +473,23 @@ mod tests { path: test_path_buf("/tmp/skills/changelog_writer.toml").abs(), }, ]; - let view = SkillsToggleView::new(items, tx); + let view = SkillsToggleView::new(items, tx, crate::keymap::RuntimeKeymap::defaults().list); assert_snapshot!("skills_toggle_basic", render_lines(&view, /*width*/ 72)); } + + #[test] + fn footer_hint_uses_list_keymap_accept_and_cancel() { + let (tx_raw, _rx) = unbounded_channel::(); + let tx = AppEventSender::new(tx_raw); + let mut keymap = crate::keymap::RuntimeKeymap::defaults().list; + keymap.accept = vec![key_hint::ctrl(KeyCode::Char('t'))]; + keymap.cancel = vec![key_hint::ctrl(KeyCode::Char('x'))]; + let view = SkillsToggleView::new(Vec::new(), tx, keymap); + let rendered = render_lines(&view, /*width*/ 72); + + assert!(rendered.contains("ctrl + t")); + assert!(rendered.contains("ctrl + x")); + assert!(!rendered.contains("enter")); + assert!(!rendered.contains("esc")); + } } diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__status_line_setup__tests__setup_view_snapshot_uses_runtime_preview_values.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__status_line_setup__tests__setup_view_snapshot_uses_runtime_preview_values.snap index db86cf8a7..50eb41ed4 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__status_line_setup__tests__setup_view_snapshot_uses_runtime_preview_values.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__status_line_setup__tests__setup_view_snapshot_uses_runtime_preview_values.snap @@ -18,4 +18,4 @@ expression: "render_lines(&view, 72)" [ ] pull-request-number Open pull request number for the current … gpt-5-codex · ~/codex-rs · jif/statusline-preview - Use ↑↓ to navigate, ←→ to move, space to select, enter to confirm, esc + Press space to toggle; ←/→ to move; enter to confirm and close; esc to diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__title_setup__tests__terminal_title_setup_basic.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__title_setup__tests__terminal_title_setup_basic.snap index 598fa418a..be5cecebf 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__title_setup__tests__terminal_title_setup_basic.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__title_setup__tests__terminal_title_setup_basic.snap @@ -18,4 +18,4 @@ expression: "render_lines(&view, 84)" [ ] context-remaining Percentage of context window remaining (omitted when unk… [ ! ] Action Required | my-project | Working | thread title - Use ↑↓ to navigate, ←→ to move, space to select, enter to confirm, esc to cancel. + Press space to toggle; ←/→ to move; enter to confirm and close; esc to close diff --git a/codex-rs/tui/src/bottom_pane/status_line_setup.rs b/codex-rs/tui/src/bottom_pane/status_line_setup.rs index d96bc286a..5ce13cd21 100644 --- a/codex-rs/tui/src/bottom_pane/status_line_setup.rs +++ b/codex-rs/tui/src/bottom_pane/status_line_setup.rs @@ -35,6 +35,7 @@ use crate::bottom_pane::multi_select_picker::MultiSelectItem; use crate::bottom_pane::multi_select_picker::MultiSelectPicker; use crate::bottom_pane::status_surface_preview::StatusSurfacePreviewData; use crate::bottom_pane::status_surface_preview::StatusSurfacePreviewItem; +use crate::keymap::ListKeymap; use crate::render::renderable::Renderable; const STATUS_LINE_USE_THEME_COLORS_ITEM_ID: &str = "status-line-use-theme-colors"; @@ -246,6 +247,7 @@ impl StatusLineSetupView { use_theme_colors: bool, preview_data: StatusSurfacePreviewData, app_event_tx: AppEventSender, + list_keymap: ListKeymap, ) -> Self { let mut used_ids = HashSet::new(); let mut items = vec![MultiSelectItem { @@ -284,10 +286,7 @@ impl StatusLineSetupView { Some("Select which items to display in the status line.".to_string()), app_event_tx, ) - .instructions(vec![ - "Use ↑↓ to navigate, ←→ to move, space to select, enter to confirm, esc to cancel." - .into(), - ]) + .list_keymap(list_keymap) .items(items) .enable_ordering() .on_preview(move |items| { @@ -621,6 +620,7 @@ mod tests { ), ]), AppEventSender::new(tx_raw), + crate::keymap::RuntimeKeymap::defaults().list, ); assert_snapshot!(render_lines(&view, /*width*/ 72)); diff --git a/codex-rs/tui/src/bottom_pane/title_setup.rs b/codex-rs/tui/src/bottom_pane/title_setup.rs index ae3c56ca3..f37991bc0 100644 --- a/codex-rs/tui/src/bottom_pane/title_setup.rs +++ b/codex-rs/tui/src/bottom_pane/title_setup.rs @@ -26,6 +26,7 @@ use crate::bottom_pane::multi_select_picker::MultiSelectItem; use crate::bottom_pane::multi_select_picker::MultiSelectPicker; use crate::bottom_pane::status_surface_preview::StatusSurfacePreviewData; use crate::bottom_pane::status_surface_preview::StatusSurfacePreviewItem; +use crate::keymap::ListKeymap; use crate::render::renderable::Renderable; /// Available items that can be displayed in the terminal title. @@ -244,6 +245,7 @@ impl TerminalTitleSetupView { title_items: Option<&[String]>, preview_data: StatusSurfacePreviewData, app_event_tx: AppEventSender, + list_keymap: ListKeymap, ) -> Self { let selected_items = title_items .into_iter() @@ -271,10 +273,7 @@ impl TerminalTitleSetupView { Some("Select which items to display in the terminal title.".to_string()), app_event_tx, ) - .instructions(vec![ - "Use ↑↓ to navigate, ←→ to move, space to select, enter to confirm, esc to cancel." - .into(), - ]) + .list_keymap(list_keymap) .items(items) .enable_ordering() .on_preview(move |items| { @@ -387,8 +386,12 @@ mod tests { "run-state".to_string(), "thread-title".to_string(), ]; - let view = - TerminalTitleSetupView::new(Some(&selected), StatusSurfacePreviewData::default(), tx); + let view = TerminalTitleSetupView::new( + Some(&selected), + StatusSurfacePreviewData::default(), + tx, + crate::keymap::RuntimeKeymap::defaults().list, + ); assert_snapshot!( "terminal_title_setup_basic", render_lines(&view, /*width*/ 84) diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index b73459c6f..2894e79c8 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -1441,7 +1441,11 @@ impl ChatWidget { } pub(crate) fn open_app_link_view(&mut self, params: crate::bottom_pane::AppLinkViewParams) { - let view = crate::bottom_pane::AppLinkView::new(params, self.app_event_tx.clone()); + let view = crate::bottom_pane::AppLinkView::new_with_keymap( + params, + self.app_event_tx.clone(), + self.bottom_pane.list_keymap(), + ); self.bottom_pane.show_view(Box::new(view)); self.request_redraw(); } @@ -1899,6 +1903,7 @@ impl ChatWidget { self.config.memories.use_memories, self.config.memories.generate_memories, self.app_event_tx.clone(), + self.bottom_pane.list_keymap(), ); self.bottom_pane.show_view(Box::new(view)); } @@ -4981,6 +4986,7 @@ impl ChatWidget { self.config.tui_status_line_use_colors, self.status_surface_preview_data(), self.app_event_tx.clone(), + self.bottom_pane.list_keymap(), ); self.bottom_pane.show_view(Box::new(view)); } @@ -4992,6 +4998,7 @@ impl ChatWidget { Some(configured_terminal_title_items.as_slice()), self.terminal_title_preview_data(), self.app_event_tx.clone(), + self.bottom_pane.list_keymap(), ); self.bottom_pane.show_view(Box::new(view)); } @@ -6517,7 +6524,11 @@ impl ChatWidget { }) .collect(); - let view = ExperimentalFeaturesView::new(features, self.app_event_tx.clone()); + let view = ExperimentalFeaturesView::new( + features, + self.app_event_tx.clone(), + self.bottom_pane.list_keymap(), + ); self.bottom_pane.show_view(Box::new(view)); } diff --git a/codex-rs/tui/src/chatwidget/hooks.rs b/codex-rs/tui/src/chatwidget/hooks.rs index f8c52f74e..3cc638a8d 100644 --- a/codex-rs/tui/src/chatwidget/hooks.rs +++ b/codex-rs/tui/src/chatwidget/hooks.rs @@ -36,6 +36,7 @@ impl ChatWidget { .show_view(Box::new(HooksBrowserView::from_entry( entry, self.app_event_tx.clone(), + self.bottom_pane.list_keymap(), ))); self.request_redraw(); } diff --git a/codex-rs/tui/src/chatwidget/skills.rs b/codex-rs/tui/src/chatwidget/skills.rs index bc431d0db..356e191b8 100644 --- a/codex-rs/tui/src/chatwidget/skills.rs +++ b/codex-rs/tui/src/chatwidget/skills.rs @@ -94,7 +94,11 @@ impl ChatWidget { }) .collect(); - let view = SkillsToggleView::new(items, self.app_event_tx.clone()); + let view = SkillsToggleView::new( + items, + self.app_event_tx.clone(), + self.bottom_pane.list_keymap(), + ); self.bottom_pane.show_view(Box::new(view)); } diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_line_setup_popup_hardcoded_only.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_line_setup_popup_hardcoded_only.snap index 652c914ad..10c1132c3 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_line_setup_popup_hardcoded_only.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_line_setup_popup_hardcoded_only.snap @@ -17,4 +17,4 @@ expression: status_line_popup_snapshot(&mut chat) [ ] current-dir Current working directory my-project · feat/awesome-feature · thread title - Use ↑↓ to navigate, ←→ to move, space to select, enter to confirm, esc to cancel. + Press space to toggle; ←/→ to move; enter to confirm and close; esc to close diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_line_setup_popup_live_only.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_line_setup_popup_live_only.snap index fc713c4cd..3106adea2 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_line_setup_popup_live_only.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_line_setup_popup_live_only.snap @@ -17,4 +17,4 @@ expression: status_line_popup_snapshot(&mut chat) [ ] current-dir Current working directory preview-live-root · feature/live-preview-branch · Live preview thread - Use ↑↓ to navigate, ←→ to move, space to select, enter to confirm, esc to cancel. + Press space to toggle; ←/→ to move; enter to confirm and close; esc to close diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_line_setup_popup_mixed.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_line_setup_popup_mixed.snap index 547f70daa..1cb2dcb5a 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_line_setup_popup_mixed.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_line_setup_popup_mixed.snap @@ -17,4 +17,4 @@ expression: status_line_popup_snapshot(&mut chat) [ ] current-dir Current working directory my-project · feature/mixed-preview · Mixed preview thread - Use ↑↓ to navigate, ←→ to move, space to select, enter to confirm, esc to cancel. + Press space to toggle; ←/→ to move; enter to confirm and close; esc to close diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__terminal_title_setup_popup_hardcoded_only.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__terminal_title_setup_popup_hardcoded_only.snap index cf2248ffd..8ff422750 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__terminal_title_setup_popup_hardcoded_only.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__terminal_title_setup_popup_hardcoded_only.snap @@ -17,4 +17,4 @@ expression: terminal_title_popup_snapshot(&mut chat) [ ] run-state Compact session run-state text (Ready, Working, Thinking) thread title | feat/awesome-feature | Tasks 0/0 - Use ↑↓ to navigate, ←→ to move, space to select, enter to confirm, esc to cancel. + Press space to toggle; ←/→ to move; enter to confirm and close; esc to close diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__terminal_title_setup_popup_live_only.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__terminal_title_setup_popup_live_only.snap index c22a70f71..8c149b8cc 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__terminal_title_setup_popup_live_only.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__terminal_title_setup_popup_live_only.snap @@ -17,4 +17,4 @@ expression: terminal_title_popup_snapshot(&mut chat) [ ] run-state Compact session run-state text (Ready, Working, Thinking) preview-live-root | Live preview thread | feature/live-preview-branch | Tasks 2/5 - Use ↑↓ to navigate, ←→ to move, space to select, enter to confirm, esc to cancel. + Press space to toggle; ←/→ to move; enter to confirm and close; esc to close diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__terminal_title_setup_popup_mixed.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__terminal_title_setup_popup_mixed.snap index 06b988811..b04e07bd9 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__terminal_title_setup_popup_mixed.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__terminal_title_setup_popup_mixed.snap @@ -17,4 +17,4 @@ expression: terminal_title_popup_snapshot(&mut chat) [ ] git-branch Current Git branch (omitted when unavailable) project | Mixed preview thread | Tasks 0/0 - Use ↑↓ to navigate, ←→ to move, space to select, enter to confirm, esc to cancel. + Press space to toggle; ←/→ to move; enter to confirm and close; esc to close diff --git a/codex-rs/tui/src/chatwidget/tests/popups_and_settings.rs b/codex-rs/tui/src/chatwidget/tests/popups_and_settings.rs index a2dd3b995..4f4398d34 100644 --- a/codex-rs/tui/src/chatwidget/tests/popups_and_settings.rs +++ b/codex-rs/tui/src/chatwidget/tests/popups_and_settings.rs @@ -2181,7 +2181,11 @@ async fn experimental_features_popup_snapshot() { enabled: true, }, ]; - let view = ExperimentalFeaturesView::new(features, chat.app_event_tx.clone()); + let view = ExperimentalFeaturesView::new( + features, + chat.app_event_tx.clone(), + crate::keymap::RuntimeKeymap::defaults().list, + ); chat.bottom_pane.show_view(Box::new(view)); let popup = render_bottom_popup(&chat, /*width*/ 80); @@ -2201,6 +2205,7 @@ async fn experimental_features_toggle_saves_on_exit() { enabled: false, }], chat.app_event_tx.clone(), + crate::keymap::RuntimeKeymap::defaults().list, ); chat.bottom_pane.show_view(Box::new(view)); diff --git a/codex-rs/tui/src/key_hint.rs b/codex-rs/tui/src/key_hint.rs index 1008a00aa..da5a33236 100644 --- a/codex-rs/tui/src/key_hint.rs +++ b/codex-rs/tui/src/key_hint.rs @@ -5,6 +5,12 @@ //! cross-terminal inconsistencies in how shifted letters and raw C0 control //! characters are reported. //! +//! List and picker code should match navigation through these helpers instead +//! of comparing `KeyEvent` values directly. The matcher owns compatibility for +//! terminals that report control chords as C0 characters, while +//! `is_plain_text_key_event` gives searchable pickers a shared boundary between +//! text input and navigation commands. +//! //! It also supplies rendering helpers that convert bindings into styled //! `ratatui::text::Span` values for UI hint display. @@ -122,6 +128,27 @@ impl KeyBindingListExt for [KeyBinding] { } } +/// Returns whether an event should be treated as literal text input. +/// +/// Searchable pickers use this to avoid stealing plain printable characters for +/// navigation when the same character might be a valid query. For example, a +/// list may bind `j` and `k` for movement, but a searchable list must let +/// plain `j` update the query while still allowing `Ctrl+J` to move. Calling +/// this after normalizing keybindings would blur that distinction and cause +/// printable search input to disappear. +pub(crate) fn is_plain_text_key_event(event: KeyEvent) -> bool { + matches!( + event, + KeyEvent { + code: KeyCode::Char(ch), + modifiers, + .. + } if !ch.is_ascii_control() + && !modifiers.contains(KeyModifiers::CONTROL) + && !modifiers.contains(KeyModifiers::ALT) + ) +} + pub(crate) const fn plain(key: KeyCode) -> KeyBinding { KeyBinding::new(key, KeyModifiers::NONE) } diff --git a/codex-rs/tui/src/keymap.rs b/codex-rs/tui/src/keymap.rs index c12d24ca2..fbac16096 100644 --- a/codex-rs/tui/src/keymap.rs +++ b/codex-rs/tui/src/keymap.rs @@ -201,10 +201,24 @@ pub(crate) struct PagerKeymap { } /// Generic list picker keybindings shared across popup list views. +/// +/// These actions describe list intent rather than a specific widget layout. +/// Vertical actions move the highlighted row, page and jump actions move within +/// the current filtered row set, and horizontal actions are available to views +/// that expose adjacent choices such as tabs, toolbar values, or ordered item +/// movement. Views that also accept search text are responsible for checking +/// `is_plain_text_key_event` before dispatching plain-character bindings so a +/// configured `j`, `k`, `h`, or `l` does not steal query input. #[derive(Clone, Debug)] pub(crate) struct ListKeymap { pub(crate) move_up: Vec, pub(crate) move_down: Vec, + pub(crate) move_left: Vec, + pub(crate) move_right: Vec, + pub(crate) page_up: Vec, + pub(crate) page_down: Vec, + pub(crate) jump_top: Vec, + pub(crate) jump_bottom: Vec, pub(crate) accept: Vec, pub(crate) cancel: Vec, } @@ -505,13 +519,6 @@ impl RuntimeKeymap { close_transcript: resolve_local!(keymap, defaults, pager, close_transcript), }; - let list = ListKeymap { - move_up: resolve_local!(keymap, defaults, list, move_up), - move_down: resolve_local!(keymap, defaults, list, move_down), - accept: resolve_local!(keymap, defaults, list, accept), - cancel: resolve_local!(keymap, defaults, list, cancel), - }; - let approval = ApprovalKeymap { open_fullscreen: resolve_local!(keymap, defaults, approval, open_fullscreen), open_thread: resolve_local!(keymap, defaults, approval, open_thread), @@ -523,6 +530,121 @@ impl RuntimeKeymap { cancel: resolve_local!(keymap, defaults, approval, cancel), }; + let list_move_up = resolve_local!(keymap, defaults, list, move_up); + let list_move_down = resolve_local!(keymap, defaults, list, move_down); + let list_accept = resolve_local!(keymap, defaults, list, accept); + let list_cancel = resolve_local!(keymap, defaults, list, cancel); + let mut configured_bindings_to_preserve = Vec::new(); + for (configured, resolved) in [ + ( + keymap.global.open_transcript.as_ref(), + app.open_transcript.as_slice(), + ), + ( + keymap.global.open_external_editor.as_ref(), + app.open_external_editor.as_slice(), + ), + (keymap.global.copy.as_ref(), app.copy.as_slice()), + ( + keymap.global.clear_terminal.as_ref(), + app.clear_terminal.as_slice(), + ), + ( + keymap.global.toggle_vim_mode.as_ref(), + app.toggle_vim_mode.as_slice(), + ), + ( + keymap.global.toggle_fast_mode.as_ref(), + app.toggle_fast_mode.as_slice(), + ), + ( + keymap.global.toggle_raw_output.as_ref(), + app.toggle_raw_output.as_slice(), + ), + (keymap.list.move_up.as_ref(), list_move_up.as_slice()), + (keymap.list.move_down.as_ref(), list_move_down.as_slice()), + (keymap.list.accept.as_ref(), list_accept.as_slice()), + (keymap.list.cancel.as_ref(), list_cancel.as_slice()), + ( + keymap.approval.open_fullscreen.as_ref(), + approval.open_fullscreen.as_slice(), + ), + ( + keymap.approval.open_thread.as_ref(), + approval.open_thread.as_slice(), + ), + ( + keymap.approval.approve.as_ref(), + approval.approve.as_slice(), + ), + ( + keymap.approval.approve_for_session.as_ref(), + approval.approve_for_session.as_slice(), + ), + ( + keymap.approval.approve_for_prefix.as_ref(), + approval.approve_for_prefix.as_slice(), + ), + (keymap.approval.deny.as_ref(), approval.deny.as_slice()), + ( + keymap.approval.decline.as_ref(), + approval.decline.as_slice(), + ), + (keymap.approval.cancel.as_ref(), approval.cancel.as_slice()), + ] { + if configured.is_none() { + continue; + } + for binding in resolved { + if !configured_bindings_to_preserve.contains(binding) { + configured_bindings_to_preserve.push(*binding); + } + } + } + + let list = ListKeymap { + move_up: list_move_up, + move_down: list_move_down, + move_left: resolve_new_list_bindings( + keymap.list.move_left.as_ref(), + &defaults.list.move_left, + &configured_bindings_to_preserve, + "tui.keymap.list.move_left", + )?, + move_right: resolve_new_list_bindings( + keymap.list.move_right.as_ref(), + &defaults.list.move_right, + &configured_bindings_to_preserve, + "tui.keymap.list.move_right", + )?, + page_up: resolve_new_list_bindings( + keymap.list.page_up.as_ref(), + &defaults.list.page_up, + &configured_bindings_to_preserve, + "tui.keymap.list.page_up", + )?, + page_down: resolve_new_list_bindings( + keymap.list.page_down.as_ref(), + &defaults.list.page_down, + &configured_bindings_to_preserve, + "tui.keymap.list.page_down", + )?, + jump_top: resolve_new_list_bindings( + keymap.list.jump_top.as_ref(), + &defaults.list.jump_top, + &configured_bindings_to_preserve, + "tui.keymap.list.jump_top", + )?, + jump_bottom: resolve_new_list_bindings( + keymap.list.jump_bottom.as_ref(), + &defaults.list.jump_bottom, + &configured_bindings_to_preserve, + "tui.keymap.list.jump_bottom", + )?, + accept: list_accept, + cancel: list_cancel, + }; + let resolved = Self { app, chat, @@ -711,13 +833,21 @@ impl RuntimeKeymap { move_up: default_bindings![ plain(KeyCode::Up), ctrl(KeyCode::Char('p')), + ctrl(KeyCode::Char('k')), plain(KeyCode::Char('k')) ], move_down: default_bindings![ plain(KeyCode::Down), ctrl(KeyCode::Char('n')), + ctrl(KeyCode::Char('j')), plain(KeyCode::Char('j')) ], + move_left: default_bindings![plain(KeyCode::Left), ctrl(KeyCode::Char('h'))], + move_right: default_bindings![plain(KeyCode::Right), ctrl(KeyCode::Char('l'))], + page_up: default_bindings![plain(KeyCode::PageUp), ctrl(KeyCode::Char('b'))], + page_down: default_bindings![plain(KeyCode::PageDown), ctrl(KeyCode::Char('f'))], + jump_top: default_bindings![plain(KeyCode::Home)], + jump_bottom: default_bindings![plain(KeyCode::End)], accept: default_bindings![plain(KeyCode::Enter)], cancel: default_bindings![plain(KeyCode::Esc)], }, @@ -835,7 +965,7 @@ impl RuntimeKeymap { MAIN_RESERVED_BINDINGS, )?; - validate_no_shadow( + validate_no_shadow_with_allowed_overlaps( "app", [ ("open_transcript", self.app.open_transcript.as_slice()), @@ -852,6 +982,12 @@ impl RuntimeKeymap { [ ("list.move_up", self.list.move_up.as_slice()), ("list.move_down", self.list.move_down.as_slice()), + ("list.move_left", self.list.move_left.as_slice()), + ("list.move_right", self.list.move_right.as_slice()), + ("list.page_up", self.list.page_up.as_slice()), + ("list.page_down", self.list.page_down.as_slice()), + ("list.jump_top", self.list.jump_top.as_slice()), + ("list.jump_bottom", self.list.jump_bottom.as_slice()), ("list.accept", self.list.accept.as_slice()), ("list.cancel", self.list.cancel.as_slice()), ( @@ -872,6 +1008,11 @@ impl RuntimeKeymap { ("approval.decline", self.approval.decline.as_slice()), ("approval.cancel", self.approval.cancel.as_slice()), ], + [( + "clear_terminal", + "list.move_right", + key_hint::ctrl(KeyCode::Char('l')), + )], )?; // While the composer is focused, these main-surface handlers always @@ -1123,6 +1264,12 @@ impl RuntimeKeymap { [ ("move_up", self.list.move_up.as_slice()), ("move_down", self.list.move_down.as_slice()), + ("move_left", self.list.move_left.as_slice()), + ("move_right", self.list.move_right.as_slice()), + ("page_up", self.list.page_up.as_slice()), + ("page_down", self.list.page_down.as_slice()), + ("jump_top", self.list.jump_top.as_slice()), + ("jump_bottom", self.list.jump_bottom.as_slice()), ("accept", self.list.accept.as_slice()), ("cancel", self.list.cancel.as_slice()), ], @@ -1152,6 +1299,12 @@ impl RuntimeKeymap { for (action, bindings) in [ ("list.move_up", self.list.move_up.as_slice()), ("list.move_down", self.list.move_down.as_slice()), + ("list.move_left", self.list.move_left.as_slice()), + ("list.move_right", self.list.move_right.as_slice()), + ("list.page_up", self.list.page_up.as_slice()), + ("list.page_down", self.list.page_down.as_slice()), + ("list.jump_top", self.list.jump_top.as_slice()), + ("list.jump_bottom", self.list.jump_bottom.as_slice()), ("list.accept", self.list.accept.as_slice()), ("list.cancel", self.list.cancel.as_slice()), ( @@ -1221,14 +1374,6 @@ See the Codex keymap documentation for supported actions and examples." Ok(()) } -fn validate_no_shadow( - context: &str, - primary: [(&'static str, &[KeyBinding]); N], - shadowed: [(&'static str, &[KeyBinding]); M], -) -> Result<(), String> { - validate_no_shadow_with_allowed_overlaps(context, primary, shadowed, []) -} - fn validate_no_shadow_with_allowed_overlaps( context: &str, primary: [(&'static str, &[KeyBinding]); N], @@ -1372,6 +1517,22 @@ fn resolve_bindings( parse_bindings(spec, path) } +fn resolve_new_list_bindings( + configured: Option<&KeybindingsSpec>, + fallback: &[KeyBinding], + configured_bindings_to_preserve: &[KeyBinding], + path: &str, +) -> Result, String> { + let Some(spec) = configured else { + return Ok(fallback + .iter() + .copied() + .filter(|binding| !configured_bindings_to_preserve.contains(binding)) + .collect()); + }; + parse_bindings(spec, path) +} + /// Parse one keybinding value (`string` or `list[string]`) into concrete bindings. /// /// Duplicate entries are de-duplicated while preserving first-seen order so the @@ -1659,6 +1820,152 @@ mod tests { assert_eq!(runtime.editor.kill_whole_line, Vec::new()); } + #[test] + fn defaults_include_list_page_and_jump_actions() { + let runtime = RuntimeKeymap::defaults(); + + assert_eq!( + runtime.list.move_up, + vec![ + key_hint::plain(KeyCode::Up), + key_hint::ctrl(KeyCode::Char('p')), + key_hint::ctrl(KeyCode::Char('k')), + key_hint::plain(KeyCode::Char('k')), + ] + ); + assert_eq!( + runtime.list.move_down, + vec![ + key_hint::plain(KeyCode::Down), + key_hint::ctrl(KeyCode::Char('n')), + key_hint::ctrl(KeyCode::Char('j')), + key_hint::plain(KeyCode::Char('j')), + ] + ); + assert_eq!( + runtime.list.move_left, + vec![ + key_hint::plain(KeyCode::Left), + key_hint::ctrl(KeyCode::Char('h')), + ] + ); + assert_eq!( + runtime.list.move_right, + vec![ + key_hint::plain(KeyCode::Right), + key_hint::ctrl(KeyCode::Char('l')), + ] + ); + assert_eq!( + runtime.list.page_up, + vec![ + key_hint::plain(KeyCode::PageUp), + key_hint::ctrl(KeyCode::Char('b')), + ] + ); + assert_eq!( + runtime.list.page_down, + vec![ + key_hint::plain(KeyCode::PageDown), + key_hint::ctrl(KeyCode::Char('f')), + ] + ); + assert_eq!(runtime.list.jump_top, vec![key_hint::plain(KeyCode::Home)]); + assert_eq!( + runtime.list.jump_bottom, + vec![key_hint::plain(KeyCode::End)] + ); + } + + #[test] + fn configured_legacy_list_bindings_prune_new_default_overlaps() { + let mut keymap = TuiKeymap::default(); + keymap.list.move_up = Some(one("page-up")); + keymap.list.move_down = Some(one("page-down")); + + let runtime = RuntimeKeymap::from_config(&keymap).expect("config should parse"); + + assert_eq!(runtime.list.move_up, vec![key_hint::plain(KeyCode::PageUp)]); + assert_eq!( + runtime.list.move_down, + vec![key_hint::plain(KeyCode::PageDown)] + ); + assert_eq!( + runtime.list.page_up, + vec![key_hint::ctrl(KeyCode::Char('b'))] + ); + assert_eq!( + runtime.list.page_down, + vec![key_hint::ctrl(KeyCode::Char('f'))] + ); + } + + #[test] + fn configured_legacy_list_bindings_can_prune_all_new_default_keys() { + let mut keymap = TuiKeymap::default(); + keymap.list.move_up = Some(KeybindingsSpec::Many(vec![ + KeybindingSpec("page-up".to_string()), + KeybindingSpec("ctrl-b".to_string()), + ])); + + let runtime = RuntimeKeymap::from_config(&keymap).expect("config should parse"); + + assert_eq!( + runtime.list.move_up, + vec![ + key_hint::plain(KeyCode::PageUp), + key_hint::ctrl(KeyCode::Char('b')), + ] + ); + assert_eq!(runtime.list.page_up, Vec::new()); + } + + #[test] + fn explicit_new_list_bindings_still_conflict_with_legacy_bindings() { + let mut keymap = TuiKeymap::default(); + keymap.list.move_up = Some(one("page-up")); + keymap.list.page_up = Some(one("page-up")); + + expect_conflict(&keymap, "move_up", "page_up"); + } + + #[test] + fn configured_app_bindings_prune_new_list_default_overlaps() { + let mut keymap = TuiKeymap::default(); + keymap.global.copy = Some(one("page-down")); + + let runtime = RuntimeKeymap::from_config(&keymap).expect("config should parse"); + + assert_eq!(runtime.app.copy, vec![key_hint::plain(KeyCode::PageDown)]); + assert_eq!( + runtime.list.page_down, + vec![key_hint::ctrl(KeyCode::Char('f'))] + ); + } + + #[test] + fn configured_approval_bindings_prune_new_list_default_overlaps() { + let mut keymap = TuiKeymap::default(); + keymap.approval.approve = Some(one("home")); + + let runtime = RuntimeKeymap::from_config(&keymap).expect("config should parse"); + + assert_eq!( + runtime.approval.approve, + vec![key_hint::plain(KeyCode::Home)] + ); + assert_eq!(runtime.list.jump_top, Vec::new()); + } + + #[test] + fn explicit_new_list_bindings_still_conflict_with_configured_approval_bindings() { + let mut keymap = TuiKeymap::default(); + keymap.approval.approve = Some(one("home")); + keymap.list.jump_top = Some(one("home")); + + expect_conflict(&keymap, "list.jump_top", "approval.approve"); + } + #[test] fn vim_normal_defaults_include_insert_and_arrow_aliases() { let runtime = RuntimeKeymap::defaults(); @@ -1734,6 +2041,21 @@ mod tests { keymap.list.move_down = Some(one("up")); expect_conflict(&keymap, "move_up", "move_down"); + + let mut keymap = TuiKeymap::default(); + keymap.list.move_left = Some(one("left")); + keymap.list.move_right = Some(one("left")); + + expect_conflict(&keymap, "move_left", "move_right"); + } + + #[test] + fn rejects_conflicting_list_page_and_jump_bindings() { + let mut keymap = TuiKeymap::default(); + keymap.list.page_up = Some(one("home")); + keymap.list.jump_top = Some(one("home")); + + expect_conflict(&keymap, "page_up", "jump_top"); } #[test] diff --git a/codex-rs/tui/src/keymap_setup/actions.rs b/codex-rs/tui/src/keymap_setup/actions.rs index 8e11c5537..9ce827b8a 100644 --- a/codex-rs/tui/src/keymap_setup/actions.rs +++ b/codex-rs/tui/src/keymap_setup/actions.rs @@ -164,6 +164,12 @@ pub(super) const KEYMAP_ACTIONS: &[KeymapActionDescriptor] = &[ action("pager", "Pager", "close_transcript", "Close the transcript overlay."), action("list", "List", "move_up", "Move list selection up."), action("list", "List", "move_down", "Move list selection down."), + action("list", "List", "move_left", "Move horizontally left in list pickers."), + action("list", "List", "move_right", "Move horizontally right in list pickers."), + action("list", "List", "page_up", "Move list selection up by one page."), + action("list", "List", "page_down", "Move list selection down by one page."), + action("list", "List", "jump_top", "Jump to the first list item."), + action("list", "List", "jump_bottom", "Jump to the last list item."), action("list", "List", "accept", "Accept the current list selection."), action("list", "List", "cancel", "Cancel and close selection views."), action("approval", "Approval", "open_fullscreen", "Open approval details fullscreen."), @@ -286,6 +292,12 @@ pub(super) fn binding_slot<'a>( ("pager", "close_transcript") => Some(&mut keymap.pager.close_transcript), ("list", "move_up") => Some(&mut keymap.list.move_up), ("list", "move_down") => Some(&mut keymap.list.move_down), + ("list", "move_left") => Some(&mut keymap.list.move_left), + ("list", "move_right") => Some(&mut keymap.list.move_right), + ("list", "page_up") => Some(&mut keymap.list.page_up), + ("list", "page_down") => Some(&mut keymap.list.page_down), + ("list", "jump_top") => Some(&mut keymap.list.jump_top), + ("list", "jump_bottom") => Some(&mut keymap.list.jump_bottom), ("list", "accept") => Some(&mut keymap.list.accept), ("list", "cancel") => Some(&mut keymap.list.cancel), ("approval", "open_fullscreen") => Some(&mut keymap.approval.open_fullscreen), @@ -390,6 +402,12 @@ pub(super) fn bindings_for_action<'a>( ("pager", "close_transcript") => Some(runtime_keymap.pager.close_transcript.as_slice()), ("list", "move_up") => Some(runtime_keymap.list.move_up.as_slice()), ("list", "move_down") => Some(runtime_keymap.list.move_down.as_slice()), + ("list", "move_left") => Some(runtime_keymap.list.move_left.as_slice()), + ("list", "move_right") => Some(runtime_keymap.list.move_right.as_slice()), + ("list", "page_up") => Some(runtime_keymap.list.page_up.as_slice()), + ("list", "page_down") => Some(runtime_keymap.list.page_down.as_slice()), + ("list", "jump_top") => Some(runtime_keymap.list.jump_top.as_slice()), + ("list", "jump_bottom") => Some(runtime_keymap.list.jump_bottom.as_slice()), ("list", "accept") => Some(runtime_keymap.list.accept.as_slice()), ("list", "cancel") => Some(runtime_keymap.list.cancel.as_slice()), ("approval", "open_fullscreen") => Some(runtime_keymap.approval.open_fullscreen.as_slice()), diff --git a/codex-rs/tui/src/oss_selection.rs b/codex-rs/tui/src/oss_selection.rs index 5396f061c..ab9176ffb 100644 --- a/codex-rs/tui/src/oss_selection.rs +++ b/codex-rs/tui/src/oss_selection.rs @@ -1,6 +1,9 @@ use std::io; use std::sync::LazyLock; +use crate::key_hint; +use crate::key_hint::KeyBinding; +use crate::key_hint::KeyBindingListExt; use crate::legacy_core::config::set_default_oss_provider; use codex_model_provider_info::DEFAULT_LMSTUDIO_PORT; use codex_model_provider_info::DEFAULT_OLLAMA_PORT; @@ -10,6 +13,7 @@ use crossterm::event::Event; use crossterm::event::KeyCode; use crossterm::event::KeyEvent; use crossterm::event::KeyEventKind; +use crossterm::event::KeyModifiers; use crossterm::event::{self}; use crossterm::execute; use crossterm::terminal::EnterAlternateScreen; @@ -77,6 +81,18 @@ static OSS_SELECT_OPTIONS: LazyLock> = LazyLock::new(|| { ] }); +// This startup wizard runs before the main TUI runtime keymap is available, so +// it mirrors the built-in horizontal list defaults instead of reading config. +// The shared matcher still covers raw C0 Ctrl-H/Ctrl-L terminal reports. +const MOVE_LEFT_KEYS: [KeyBinding; 2] = [ + key_hint::plain(KeyCode::Left), + key_hint::ctrl(KeyCode::Char('h')), +]; +const MOVE_RIGHT_KEYS: [KeyBinding; 2] = [ + key_hint::plain(KeyCode::Right), + key_hint::ctrl(KeyCode::Char('l')), +]; + pub struct OssSelectionWidget<'a> { select_options: &'a Vec, confirmation_prompt: Paragraph<'a>, @@ -178,29 +194,35 @@ impl OssSelectionWidget<'_> { } fn handle_select_key(&mut self, key_event: KeyEvent) { - match key_event.code { - KeyCode::Char('c') - if key_event - .modifiers - .contains(crossterm::event::KeyModifiers::CONTROL) => - { + match key_event { + KeyEvent { + code: KeyCode::Char('c'), + modifiers, + .. + } if modifiers.contains(KeyModifiers::CONTROL) => { self.send_decision("__CANCELLED__".to_string()); } - KeyCode::Left => { + _ if MOVE_LEFT_KEYS.is_pressed(key_event) => { self.selected_option = (self.selected_option + self.select_options.len() - 1) % self.select_options.len(); } - KeyCode::Right => { + _ if MOVE_RIGHT_KEYS.is_pressed(key_event) => { self.selected_option = (self.selected_option + 1) % self.select_options.len(); } - KeyCode::Enter => { + KeyEvent { + code: KeyCode::Enter, + .. + } => { let opt = &self.select_options[self.selected_option]; self.send_decision(opt.provider_id.to_string()); } - KeyCode::Esc => { + KeyEvent { + code: KeyCode::Esc, .. + } => { self.send_decision(LMSTUDIO_OSS_PROVIDER_ID.to_string()); } - other => { + KeyEvent { code, .. } => { + let other = code; let normalized = Self::normalize_keycode(other); if let Some(opt) = self .select_options @@ -371,3 +393,20 @@ async fn check_port_status(port: u16) -> io::Result { Err(_) => Ok(false), // Connection failed = not running } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn ctrl_h_l_move_provider_selection() { + let mut widget = OssSelectionWidget::new(ProviderStatus::Unknown, ProviderStatus::Unknown) + .expect("widget should initialize"); + + assert_eq!(widget.selected_option, 0); + widget.handle_key_event(KeyEvent::new(KeyCode::Char('l'), KeyModifiers::CONTROL)); + assert_eq!(widget.selected_option, 1); + widget.handle_key_event(KeyEvent::new(KeyCode::Char('h'), KeyModifiers::CONTROL)); + assert_eq!(widget.selected_option, 0); + } +} diff --git a/codex-rs/tui/src/resume_picker.rs b/codex-rs/tui/src/resume_picker.rs index 5d5e88839..527c84a8e 100644 --- a/codex-rs/tui/src/resume_picker.rs +++ b/codex-rs/tui/src/resume_picker.rs @@ -10,6 +10,9 @@ use crate::app_server_session::AppServerSession; use crate::color::blend; use crate::color::is_light; use crate::git_action_directives::parse_assistant_markdown; +use crate::key_hint::KeyBindingListExt; +use crate::key_hint::is_plain_text_key_event; +use crate::keymap::ListKeymap; use crate::keymap::PagerKeymap; use crate::keymap::RuntimeKeymap; use crate::legacy_core::config::Config; @@ -282,6 +285,7 @@ struct SessionPickerRunOptions { initial_density: SessionListDensity, view_persistence: Option, pager_keymap: PagerKeymap, + list_keymap: ListKeymap, } /// Interactive session picker that lists app-server threads with simple search, @@ -354,7 +358,7 @@ async fn run_resume_picker_with_launch_context( ); let local_filter_cwd = local_picker_cwd_filter(&cwd_filter, is_remote); let provider_filter = picker_provider_filter(config, is_remote); - let pager_keymap = picker_pager_keymap(config)?; + let runtime_keymap = picker_runtime_keymap(config)?; let options = SessionPickerRunOptions { show_all, filter_cwd: cwd_filter, @@ -367,7 +371,8 @@ async fn run_resume_picker_with_launch_context( codex_home: config.codex_home.to_path_buf(), active_profile: config.active_profile.clone(), }), - pager_keymap, + pager_keymap: runtime_keymap.pager, + list_keymap: runtime_keymap.list, }; run_session_picker_with_loader( tui, @@ -399,7 +404,7 @@ pub async fn run_fork_picker_with_app_server( ); let local_filter_cwd = local_picker_cwd_filter(&cwd_filter, is_remote); let provider_filter = picker_provider_filter(config, is_remote); - let pager_keymap = picker_pager_keymap(config)?; + let runtime_keymap = picker_runtime_keymap(config)?; let options = SessionPickerRunOptions { show_all, filter_cwd: cwd_filter, @@ -412,7 +417,8 @@ pub async fn run_fork_picker_with_app_server( codex_home: config.codex_home.to_path_buf(), active_profile: config.active_profile.clone(), }), - pager_keymap, + pager_keymap: runtime_keymap.pager, + list_keymap: runtime_keymap.list, }; run_session_picker_with_loader( tui, @@ -447,6 +453,7 @@ async fn run_session_picker_with_loader( state.density = options.initial_density; state.view_persistence = options.view_persistence; state.pager_keymap = options.pager_keymap; + state.list_keymap = options.list_keymap; state.launch_context = options.launch_context; state.start_initial_load(); state.request_frame(); @@ -516,9 +523,8 @@ fn picker_provider_filter(config: &Config, is_remote: bool) -> ProviderFilter { } } -fn picker_pager_keymap(config: &Config) -> Result { +fn picker_runtime_keymap(config: &Config) -> Result { RuntimeKeymap::from_config(&config.tui_keymap) - .map(|keymap| keymap.pager) .map_err(|err| color_eyre::eyre::eyre!("invalid keymap configuration: {err}")) } @@ -656,6 +662,7 @@ struct PickerState { transcript_loading_frame_shown: bool, overlay: Option, pager_keymap: PagerKeymap, + list_keymap: ListKeymap, } struct PaginationState { @@ -928,6 +935,7 @@ impl PickerState { transcript_loading_frame_shown: false, overlay: None, pager_keymap: RuntimeKeymap::defaults().pager, + list_keymap: RuntimeKeymap::defaults().list, } } @@ -1028,18 +1036,14 @@ impl PickerState { if self.is_transcript_loading() { return Ok(self.handle_transcript_loading_key(key)); } - if !matches!(key.code, KeyCode::PageDown) { + if !self.list_keymap.page_down.is_pressed(key) { self.pending_page_down_target = None; } + // The session picker is always searchable, so plain text belongs to + // the query first. Modified list bindings still route through the + // runtime keymap below. + let allow_plain_char_navigation = !is_plain_text_key_event(key); match key { - KeyEvent { - code: KeyCode::Esc, .. - } => { - if self.query.is_empty() { - return Ok(Some(SessionSelection::StartFresh)); - } - self.clear_query_preserving_selection(); - } KeyEvent { code: KeyCode::Char('c'), modifiers, @@ -1047,6 +1051,12 @@ impl PickerState { } if modifiers.contains(KeyModifiers::CONTROL) => { return Ok(Some(SessionSelection::Exit)); } + _ if self.list_keymap.cancel.is_pressed(key) => { + if self.query.is_empty() { + return Ok(Some(SessionSelection::StartFresh)); + } + self.clear_query_preserving_selection(); + } KeyEvent { code: KeyCode::Char('t'), modifiers, @@ -1089,10 +1099,7 @@ impl PickerState { } /* ^O */ => { self.toggle_density().await; } - KeyEvent { - code: KeyCode::Enter, - .. - } => { + _ if self.list_keymap.accept.is_pressed(key) => { if let Some(row) = self.filtered_rows.get(self.selected) { let path = row.path.clone(); let thread_id = match row.thread_id { @@ -1119,39 +1126,14 @@ impl PickerState { self.request_frame(); } } - KeyEvent { - code: KeyCode::Up, .. - } - | KeyEvent { - code: KeyCode::Char('p'), - modifiers: KeyModifiers::CONTROL, - .. - } - | KeyEvent { - code: KeyCode::Char('\u{0010}'), - modifiers: KeyModifiers::NONE, - .. - } /* ^P */ => { + _ if allow_plain_char_navigation && self.list_keymap.move_up.is_pressed(key) => { if self.selected > 0 { self.selected -= 1; self.ensure_selected_visible(); } self.request_frame(); } - KeyEvent { - code: KeyCode::Down, - .. - } - | KeyEvent { - code: KeyCode::Char('n'), - modifiers: KeyModifiers::CONTROL, - .. - } - | KeyEvent { - code: KeyCode::Char('\u{000e}'), - modifiers: KeyModifiers::NONE, - .. - } /* ^N */ => { + _ if allow_plain_char_navigation && self.list_keymap.move_down.is_pressed(key) => { if self.selected + 1 < self.filtered_rows.len() { self.selected += 1; self.ensure_selected_visible(); @@ -1159,10 +1141,7 @@ impl PickerState { self.maybe_load_more_for_scroll(); self.request_frame(); } - KeyEvent { - code: KeyCode::PageUp, - .. - } => { + _ if allow_plain_char_navigation && self.list_keymap.page_up.is_pressed(key) => { let step = self.view_rows.unwrap_or(10).max(1); if self.selected > 0 { self.selected = self.selected.saturating_sub(step); @@ -1170,19 +1149,14 @@ impl PickerState { self.request_frame(); } } - KeyEvent { - code: KeyCode::Home, - .. - } => { + _ if allow_plain_char_navigation && self.list_keymap.jump_top.is_pressed(key) => { if !self.filtered_rows.is_empty() { self.selected = 0; self.ensure_selected_visible(); self.request_frame(); } } - KeyEvent { - code: KeyCode::End, .. - } => { + _ if allow_plain_char_navigation && self.list_keymap.jump_bottom.is_pressed(key) => { if !self.filtered_rows.is_empty() { self.selected = self.filtered_rows.len().saturating_sub(1); self.ensure_selected_visible(); @@ -1190,10 +1164,7 @@ impl PickerState { self.request_frame(); } } - KeyEvent { - code: KeyCode::PageDown, - .. - } => { + _ if allow_plain_char_navigation && self.list_keymap.page_down.is_pressed(key) => { if !self.filtered_rows.is_empty() { let step = self.view_rows.unwrap_or(10).max(1); let target = self.selected.saturating_add(step); @@ -1222,14 +1193,10 @@ impl PickerState { self.focus_previous_toolbar_control(); self.request_frame(); } - KeyEvent { - code: KeyCode::Left, - .. - } - | KeyEvent { - code: KeyCode::Right, - .. - } => { + _ if allow_plain_char_navigation + && (self.list_keymap.move_left.is_pressed(key) + || self.list_keymap.move_right.is_pressed(key)) => + { self.change_focused_toolbar_value(); self.request_frame(); } @@ -5382,7 +5349,7 @@ session_picker_view = "dense" .await .unwrap(); state - .handle_key(KeyEvent::new(KeyCode::Right, KeyModifiers::NONE)) + .handle_key(KeyEvent::new(KeyCode::Char('l'), KeyModifiers::CONTROL)) .await .unwrap(); @@ -5554,6 +5521,89 @@ session_picker_view = "dense" assert_eq!(state.selected, 0); } + #[tokio::test] + async fn page_and_jump_navigation_use_list_keymap() { + let loader = page_only_loader(|_| {}); + let mut state = PickerState::new( + FrameRequester::test_dummy(), + loader, + ProviderFilter::MatchDefault(String::from("openai")), + /*show_all*/ true, + /*filter_cwd*/ None, + SessionPickerAction::Resume, + ); + state.list_keymap.page_down = vec![crate::key_hint::ctrl(KeyCode::Char('d'))]; + state.list_keymap.page_up = vec![crate::key_hint::ctrl(KeyCode::Char('u'))]; + state.list_keymap.jump_bottom = vec![crate::key_hint::ctrl(KeyCode::Char('y'))]; + state.list_keymap.jump_top = vec![crate::key_hint::ctrl(KeyCode::Char('a'))]; + + let mut items = Vec::new(); + for idx in 0..20 { + let ts = format!("2025-01-{:02}T00:00:00Z", idx + 1); + let preview = format!("item-{idx}"); + let path = format!("/tmp/item-{idx}.jsonl"); + items.push(make_row(&path, &ts, &preview)); + } + + state.reset_pagination(); + state.ingest_page(page( + items, /*next_cursor*/ None, /*num_scanned_files*/ 20, + /*reached_scan_cap*/ false, + )); + state.update_viewport(/*rows*/ 5, /*width*/ 80); + + state + .handle_key(KeyEvent::new(KeyCode::PageDown, KeyModifiers::NONE)) + .await + .unwrap(); + assert_eq!(state.selected, 0); + + state + .handle_key(KeyEvent::new(KeyCode::Char('d'), KeyModifiers::CONTROL)) + .await + .unwrap(); + assert_eq!(state.selected, 5); + + state + .handle_key(KeyEvent::new(KeyCode::Char('u'), KeyModifiers::CONTROL)) + .await + .unwrap(); + assert_eq!(state.selected, 0); + + state + .handle_key(KeyEvent::new(KeyCode::Char('y'), KeyModifiers::CONTROL)) + .await + .unwrap(); + assert_eq!(state.selected, 19); + + state + .handle_key(KeyEvent::new(KeyCode::Char('a'), KeyModifiers::CONTROL)) + .await + .unwrap(); + assert_eq!(state.selected, 0); + } + + #[tokio::test] + async fn ctrl_c_exits_even_when_cancel_is_remapped_to_ctrl_c() { + let loader = page_only_loader(|_| {}); + let mut state = PickerState::new( + FrameRequester::test_dummy(), + loader, + ProviderFilter::MatchDefault(String::from("openai")), + /*show_all*/ true, + /*filter_cwd*/ None, + SessionPickerAction::Resume, + ); + state.list_keymap.cancel = vec![crate::key_hint::ctrl(KeyCode::Char('c'))]; + + let selection = state + .handle_key(KeyEvent::new(KeyCode::Char('c'), KeyModifiers::CONTROL)) + .await + .unwrap(); + + assert!(matches!(selection, Some(SessionSelection::Exit))); + } + #[tokio::test] async fn end_jumps_to_last_known_row_and_starts_loading_more() { let recorded_requests: Arc>> = Arc::new(Mutex::new(Vec::new())); diff --git a/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_custom.snap b/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_custom.snap index f58386a8d..898108c75 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_custom.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_custom.snap @@ -5,7 +5,7 @@ expression: "render_picker(params, 120)" Keymap All configurable shortcuts. - 87 actions, 1 customized, 2 unbound. + 93 actions, 1 customized, 2 unbound. [All] Common Customized (1) Unbound (2) App Composer Editor Vim Navigation Approval Debug diff --git a/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_fast_mode_enabled.snap b/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_fast_mode_enabled.snap index 1c247951f..4d9440cb4 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_fast_mode_enabled.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_fast_mode_enabled.snap @@ -5,7 +5,7 @@ expression: "render_picker(params, 120)" Keymap All configurable shortcuts. - 88 actions, 0 customized, 3 unbound. + 94 actions, 0 customized, 3 unbound. [All] Common Customized (0) Unbound (3) App Composer Editor Vim Navigation Approval Debug diff --git a/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_first_actions.snap b/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_first_actions.snap index e094edad9..58b1b8080 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_first_actions.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_first_actions.snap @@ -2,7 +2,7 @@ source: tui/src/keymap_setup.rs expression: snapshot --- -tab: All (87 selectable) +tab: All (93 selectable) tab: Common (19 selectable) tab: Customized (0) (0 selectable) tab: Unbound (2) (2 selectable) @@ -10,7 +10,7 @@ tab: App (9 selectable) tab: Composer (5 selectable) tab: Editor (17 selectable) tab: Vim (34 selectable) -tab: Navigation (14 selectable) +tab: Navigation (20 selectable) tab: Approval (8 selectable) tab: Debug (1 selectable) Open Transcript | ctrl-t | Global open_transcript Open Transcript Open the transcript overlay. ctrl-t Default diff --git a/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_narrow.snap b/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_narrow.snap index f146c7eed..6a98da923 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_narrow.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_narrow.snap @@ -5,7 +5,7 @@ expression: "render_picker(params, 78)" Keymap All configurable shortcuts. - 87 actions, 0 customized, 2 unbound. + 93 actions, 0 customized, 2 unbound. [All] Common Customized (0) Unbound (2) App Composer Editor Vim Navigation Approval Debug diff --git a/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_wide.snap b/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_wide.snap index c9b8be80f..9d651ee4f 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_wide.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_wide.snap @@ -5,7 +5,7 @@ expression: "render_picker(params, 120)" Keymap All configurable shortcuts. - 87 actions, 0 customized, 2 unbound. + 93 actions, 0 customized, 2 unbound. [All] Common Customized (0) Unbound (2) App Composer Editor Vim Navigation Approval Debug