mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
feat(tui): add configurable keymap support (#18593)
## Why The TUI currently handles keyboard shortcuts as hard-coded event matches spread across app, composer, pager, list, approval, and navigation code. That makes shortcuts hard to customize, makes displayed hints easy to drift from actual behavior, and makes future keymap work riskier because there is no central action inventory. This PR adds the foundation for configurable, action-based keymaps without adding the interactive remapping UI yet. Onboarding intentionally stays on fixed startup shortcuts because users cannot reasonably configure keymaps before completing onboarding. This is PR1 in the keymap stack: - PR1: #18593: configurable keymap foundation - PR2: #18594: `/keymap` picker and guided remapping UI - PR3: #18595: Vim composer mode and the remap option ## Design Notes The new model resolves named actions into concrete runtime bindings once from config, then passes those bindings to the UI surfaces that handle input or render shortcut hints. The main concepts are: - **Context**: a scope where an action is active, such as `global`, `chat`, `composer`, `editor`, `pager`, `list`, or `approval`. - **Action**: a named operation inside a context, such as `global.open_transcript`, `composer.submit`, or `pager.close`. - **Binding**: one or more single-key shortcuts assigned to an action, written as config strings such as `ctrl-t`, `alt-backspace`, or `page-down`. Multi-step sequences such as `ctrl-x ctrl-s`, `g g`, or leader-key flows are not part of this PR. - **Resolution order**: context-specific config wins first, supported global fallbacks come next, and built-in defaults fill in anything unset. - **Explicit unbinding**: an empty array removes an action binding in that scope and does not fall through to a fallback binding. - **Conflict validation**: a resolved keymap rejects duplicate active bindings inside the same scope so one keypress cannot dispatch two actions. ## What Changed - Added `TuiKeymap` config support under `[tui.keymap]`, including typed contexts/actions, key alias normalization, generated schema coverage, and user-facing config errors. - Added `RuntimeKeymap` resolution in `codex-rs/tui/src/keymap.rs`, including fallback precedence, built-in defaults, explicit unbinding, and per-context conflict validation. - Rewired existing TUI handlers to consume resolved keymap actions instead of directly matching hard-coded keys in each component. - Updated key hint rendering and footer/pager/list surfaces so displayed shortcuts follow the resolved keymap. - Kept onboarding shortcuts fixed in `codex-rs/tui/src/onboarding/keys.rs` instead of exposing them through `[tui.keymap]`. ## Validation The branch includes focused coverage for config parsing, key normalization, runtime fallback resolution, explicit unbinding, duplicate-key conflict validation, default keymap consistency, onboarding startup key behavior, and UI hint snapshots affected by resolved key bindings.
This commit is contained in:
@@ -937,9 +937,10 @@ async fn restore_thread_input_state_syncs_sleep_inhibitor_state() {
|
||||
#[tokio::test]
|
||||
async fn alt_up_edits_most_recent_queued_message() {
|
||||
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
chat.queued_message_edit_binding = crate::key_hint::alt(KeyCode::Up);
|
||||
chat.chat_keymap.edit_queued_message = vec![crate::key_hint::alt(KeyCode::Up)];
|
||||
chat.queued_message_edit_hint_binding = Some(crate::key_hint::alt(KeyCode::Up));
|
||||
chat.bottom_pane
|
||||
.set_queued_message_edit_binding(crate::key_hint::alt(KeyCode::Up));
|
||||
.set_queued_message_edit_binding(chat.queued_message_edit_hint_binding);
|
||||
|
||||
// Simulate a running task so messages would normally be queued.
|
||||
chat.bottom_pane.set_task_running(/*running*/ true);
|
||||
@@ -967,6 +968,24 @@ async fn alt_up_edits_most_recent_queued_message() {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn unbound_queued_message_edit_does_not_fall_back_to_alt_up() {
|
||||
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
chat.chat_keymap.edit_queued_message = Vec::new();
|
||||
chat.queued_message_edit_hint_binding = None;
|
||||
chat.bottom_pane
|
||||
.set_queued_message_edit_binding(chat.queued_message_edit_hint_binding);
|
||||
chat.bottom_pane.set_task_running(/*running*/ true);
|
||||
chat.queued_user_messages
|
||||
.push_back(UserMessage::from("queued".to_string()).into());
|
||||
chat.refresh_pending_input_preview();
|
||||
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::ALT));
|
||||
|
||||
assert!(chat.bottom_pane.composer_text().is_empty());
|
||||
assert_eq!(chat.queued_user_messages.len(), 1);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn shift_left_edits_most_recent_queued_message_in_apple_terminal() {
|
||||
assert_shift_left_edits_most_recent_queued_message_for_terminal(TerminalInfo {
|
||||
|
||||
@@ -211,6 +211,7 @@ pub(super) async fn make_chatwidget_manual(
|
||||
stream_controller: None,
|
||||
plan_stream_controller: None,
|
||||
clipboard_lease: None,
|
||||
copy_last_response_binding: crate::keymap::RuntimeKeymap::defaults().app.copy,
|
||||
pending_guardian_review_status: PendingGuardianReviewStatus::default(),
|
||||
recent_auto_review_denials: RecentAutoReviewDenials::default(),
|
||||
terminal_title_status_kind: TerminalTitleStatusKind::Working,
|
||||
@@ -275,7 +276,8 @@ pub(super) async fn make_chatwidget_manual(
|
||||
rejected_steer_history_records: VecDeque::new(),
|
||||
pending_steers: VecDeque::new(),
|
||||
submit_pending_steers_after_interrupt: false,
|
||||
queued_message_edit_binding: crate::key_hint::alt(KeyCode::Up),
|
||||
chat_keymap: crate::keymap::RuntimeKeymap::defaults().chat,
|
||||
queued_message_edit_hint_binding: Some(crate::key_hint::alt(KeyCode::Up)),
|
||||
suppress_session_configured_redraw: false,
|
||||
suppress_initial_user_message_submit: false,
|
||||
pending_notification: None,
|
||||
@@ -739,9 +741,10 @@ pub(super) async fn assert_shift_left_edits_most_recent_queued_message_for_termi
|
||||
terminal_info: TerminalInfo,
|
||||
) {
|
||||
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
chat.queued_message_edit_binding = queued_message_edit_binding_for_terminal(terminal_info);
|
||||
chat.queued_message_edit_hint_binding =
|
||||
Some(queued_message_edit_binding_for_terminal(terminal_info));
|
||||
chat.bottom_pane
|
||||
.set_queued_message_edit_binding(chat.queued_message_edit_binding);
|
||||
.set_queued_message_edit_binding(chat.queued_message_edit_hint_binding);
|
||||
|
||||
// Simulate a running task so messages would normally be queued.
|
||||
chat.bottom_pane.set_task_running(/*running*/ true);
|
||||
|
||||
@@ -1341,6 +1341,65 @@ async fn ctrl_o_copy_reports_when_no_agent_response_exists() {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn keymap_capture_can_capture_current_copy_shortcut() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
let runtime_keymap = crate::keymap::RuntimeKeymap::defaults();
|
||||
chat.open_keymap_capture(
|
||||
"composer".to_string(),
|
||||
"submit".to_string(),
|
||||
crate::app_event::KeymapEditIntent::ReplaceAll,
|
||||
&runtime_keymap,
|
||||
);
|
||||
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Char('o'), KeyModifiers::CONTROL));
|
||||
|
||||
let AppEvent::KeymapCaptured {
|
||||
context,
|
||||
action,
|
||||
key,
|
||||
intent,
|
||||
} = rx.try_recv().expect("captured key event")
|
||||
else {
|
||||
panic!("expected keymap capture event");
|
||||
};
|
||||
assert_eq!(context, "composer");
|
||||
assert_eq!(action, "submit");
|
||||
assert_eq!(key, "ctrl-o");
|
||||
assert_eq!(intent, crate::app_event::KeymapEditIntent::ReplaceAll);
|
||||
assert!(
|
||||
drain_insert_history(&mut rx).is_empty(),
|
||||
"copy shortcut should not run while key capture is active"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn copy_shortcut_can_be_remapped() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
let mut keymap_config = chat.config_ref().tui_keymap.clone();
|
||||
keymap_config.global.copy = Some(codex_config::types::KeybindingsSpec::One(
|
||||
codex_config::types::KeybindingSpec("ctrl-x".to_string()),
|
||||
));
|
||||
let runtime_keymap =
|
||||
crate::keymap::RuntimeKeymap::from_config(&keymap_config).expect("valid copy remap");
|
||||
chat.apply_keymap_update(keymap_config, &runtime_keymap);
|
||||
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Char('o'), KeyModifiers::CONTROL));
|
||||
assert!(
|
||||
drain_insert_history(&mut rx).is_empty(),
|
||||
"old copy shortcut should no longer copy"
|
||||
);
|
||||
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Char('x'), KeyModifiers::CONTROL));
|
||||
let cells = drain_insert_history(&mut rx);
|
||||
assert_eq!(cells.len(), 1, "expected one info message");
|
||||
let rendered = lines_to_single_string(&cells[0]);
|
||||
assert!(
|
||||
rendered.contains("No agent response to copy"),
|
||||
"expected remapped copy shortcut to run, got {rendered:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn slash_copy_stores_clipboard_lease_and_preserves_it_on_failure() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
|
||||
Reference in New Issue
Block a user