feat(tui): configurable keymaps and Vim mode#17245
Open
fcoury-oai wants to merge 22 commits intomainfrom
Open
Conversation
acee7ed to
8d0bb6d
Compare
b2c36ed to
a56b93a
Compare
Publish keymap system documentation first so the implementation stack can be reviewed against explicit behavior and invariants. This commit adds the keymap system guide, action matrix, default keymap template, and config/example documentation updates, plus a rollout plan used to stage the additive refactor and validation work.
Introduce keymap configuration types and schema support in core without wiring runtime key handling yet. This keeps behavior unchanged while adding the configuration surface needed by later commits.
Introduce the TUI runtime keymap resolver and keybinding matching helpers with a dedicated unit-test suite. This commit is additive only: it adds resolution logic, conflict validation, parser coverage, and documented macros without wiring input handlers to the new runtime map yet.
Add a dedicated footer line pointing to `[tui.keymap]` in `~/.codex/config.toml` so users can find where to rebind shortcuts. Refresh tooltips and snapshots to mention the config entry and the keymap template URL.
Actions placed directly under [tui.keymap] instead of a context sub-table (e.g. [tui.keymap.global]) were silently ignored because deny_unknown_fields was only on the schemars attribute, not serde.
Expose Vim bindings through the TUI keymap config and add a startup toggle so Vim mode can be enabled by default. Wire runtime keymap handling for Vim actions, add `tui.vim_mode_default` to config parsing/runtime config, apply the default in ChatWidget startup paths, and update docs/default keymap/schema accordingly.
Update `tui/src/bottom_pane/textarea.rs` so `Esc` in Vim insert mode moves the cursor to the previous atomic boundary before switching to normal mode. This matches Vim behavior at end-of-line and avoids leaving the cursor on a virtual trailing cell. Add regression coverage in `tui/src/bottom_pane/textarea.rs` and `tui/src/bottom_pane/chat_composer.rs`, plus a `vim_escape_cursor_position` snapshot to lock rendered cursor placement after `Esc`.
Make shifted letter bindings robust when terminals report uppercase letters without explicit SHIFT modifiers. This updates keybinding matching so bindings like `shift-i`, `shift-a`, and `shift-o` also match `I`, `A`, and `O` event forms, and adds regression tests covering Vim normal-mode actions for line-start insert, line-end append, and open-line-above with shift-only bindings.
Clarify that shifted letter bindings are matched compatibly when terminals emit uppercase letters without an explicit SHIFT modifier. Update config guidance, the default keymap template, and the action matrix with examples for shift-i, shift-a, and shift-o mapping to I, A, and O.
Preserve mainline submit/queue semantics, update approval tests for `SubmitThreadOp`, and keep permission-deny shortcuts from shadowing `Esc` cancellation. Add a `v2` keymap preset so `latest` can restore `alt-d` `delete_forward_word` without mutating frozen `v1` defaults, and refresh the generated config schema and docs.
Thread cursor style through the custom terminal and render tree so the chat composer can request an insert-like cursor while Vim mode is in Insert. Restore the terminal default cursor style on Normal/non-Vim frames and TUI teardown, and cover the escape sequences plus composer mode transitions.
Let Vim Insert handle plain `Esc` before the chat edit-previous and backtrack shortcut layers so an empty composer can still return to Normal mode. Keep existing empty-composer backtrack behavior after Vim has reached Normal mode, and cover both composer-local and app-level routing.
Regenerate the config schema after adding keymap rustdoc so the fixture matches the generated `config.toml` schema. Add the required argument comments for opaque literals in the vim and cursor tests so the branch passes the argument-comment lint.
Apply the pinned Prettier layout to the keymap handoff and rollout notes so the root `pnpm run format` check accepts the documentation.
Adds a `/keymap` flow for discovering actions, replacing root-level shortcut bindings, and removing custom bindings when users want to return to defaults. The picker uses responsive layouts so wide terminals show richer action details while narrow terminals keep the guidance compact.
Adds argument-name comments to the keymap capture snapshot helper so the argument-comment lint accepts the test literals.
Add `global.copy` to the TUI keymap schema, runtime keymap, and interactive remap action list so the Copy shortcut can be remapped or unbound like other global shortcuts. Move the existing `ctrl-o` default into a new `v3` preset while preserving older presets, and update docs plus coverage for the remapped copy behavior.
Set the manual `ChatWidget` test helper to use the default copy shortcut binding so helper-built widgets match normal construction. This fixes the CI compile failure from the new remappable copy field.
Add missing argument comments around positional boolean and numeric literals that the TUI keymap work now reaches. This keeps callsites self-documenting and satisfies the argument-comment lint in CI.
Use a non-copy chord in the queue shadowing regression test so it still reaches the intended composer conflict after `global.copy` claimed `ctrl-o`. Accept the keymap picker snapshots now that Copy is part of the configurable global keymap actions.
6793e76 to
638cde2
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TL;DR
/keymappicker for setting or removing root-level custom shortcuts from the TUI.Problem
TUI keybindings were hardcoded across dozens of match arms in
app.rs,textarea.rs, and overlay handlers. Adding or changing a single binding required touching multiple files, and users had no way to remap keys without forking the codebase or hand-editing undocumented TOML. Vim mode existed, but users could neither start directly in Vim normal mode nor customize the Vim keys. Shifted-letter Vim commands (I,A,O,D,Y) were also unreliable across terminal emulators because different terminals report the same physical keypress as eitherShift+lowercaseor plainUPPERCASE.Mental model
The keymap system is a three-layer resolver sitting between terminal input and application actions:
Config layer (
codex-rs/config/src/tui_keymap.rs) — defines the on-disk[tui.keymap]TOML contract. Each action lives under a context sub-table (global,chat,composer,editor,vim_normal,vim_operator,pager,list,approval,onboarding). Key specs are normalized at deserialization time into a canonicalctrl-alt-shift-<key>form. Unknown fields are rejected eagerly with#[serde(deny_unknown_fields)].Runtime resolver (
codex-rs/tui/src/keymap.rs) — convertsTuiKeymapconfig into aRuntimeKeymapofVec<KeyBinding>per action. Resolution precedence is: context-specific override →globalfallback (for selected chat/composer actions) → versioned preset defaults. After resolution, per-context conflict validation rejects duplicate key-to-action mappings within the same dispatch scope. Presets are frozen (v1) or additive (v2addsalt-das forward-word delete);latestis always an alias to the newest.Input matching (
codex-rs/tui/src/key_hint.rs) —KeyBinding::is_press()checks incomingKeyEvents against bindings. A shifted-letter compatibility path (matches_shifted_ascii_letter_compat) ensures that ashift-abinding matches bothShift+aand plainAregardless of terminal behavior, while preserving additional modifiers like Ctrl.A keypress flows:
CrosstermEvent→TuiEvent::Key→App::handle_key_event(app/chat bindings) →ChatWidget→ composer dispatch →TextArea::input→ modal dispatch (Vim insert vs normal vs operator-pending) or standard editor keymap. Conflict validation follows those dispatch scopes: app-level actions are checked against chat and composer actions that can be intercepted before the composer sees them; unrelated contexts likevim_normalandpagerare allowed to reuse the same key because they are never evaluated together.The
/keymapcommand adds a guided editing layer on top of that resolver rather than a second source of truth. It builds a searchable catalog of supported actions from the same runtime keymap, shows the current binding and source, captures one replacement key, validates the resultingTuiKeymap, then persists only the chosen root-level[tui.keymap.<context>]action. Users can also remove a custom root binding, which deletes that config entry and lets the action fall back to preset/default resolution again.Non-goals
config.tomlrequires restarting the TUI.vim_normalandpageris allowed because they never evaluate simultaneously./keymapflow writes root-level[tui.keymap.*]entries so shortcuts stay consistent across profiles. Profile-specific keymaps remain possible by editing config manually.Tradeoffs
shift(KeyCode::Char('a'))andplain(KeyCode::Char('A'))for Vim commands that use uppercase letters. This duplicates entries but guarantees correctness across iTerm2, Terminal.app, Kitty, Alacritty, and Windows Terminal which all disagree on shift reporting. The alternative — relying solely on thematches_shifted_ascii_letter_compatruntime fallback — would work for user config but would make the defaults silently depend on a non-obvious compatibility path.v1is immutable even if we discover better defaults later. Users who pinpreset = "v1"get stable behavior forever. New improvements go intov2+ andlatestrotates forward. The cost is carrying dead default tables; the benefit is that config files written today never silently change behavior.validate_conflicts()but is not enforced structurally.[tui].vim_mode_default = truestarts the composer in Vim normal mode.[tui.keymap]only decides which keypresses trigger actions; it does not implicitly enable Vim mode.Architecture
Observability
tui.keymap...path, the offending value/action, and a link to the canonical keymap template atdocs/default-keymap.toml. TOML shape errors such as unknown fields are rejected during config deserialization before runtime keymap resolution.-- NORMAL --/-- INSERT --when Vim mode is active, driven byvim_mode_label()intextarea.rs.[tui].vim_mode_defaultis loaded intoConfig::tui_vim_mode_default;ChatWidgetapplies it when constructing the bottom pane so users can launch directly into normal mode./keymapopens a searchable remap picker. Wide terminals show selected-action details in a side panel; narrow terminals keep a compact one-line description below the picker. Setting a binding replaces that action's root custom binding, while removing a binding clears only the root override and returns to defaults.Tests
misplaced_action_at_keymap_root_is_rejected), valid context placement.tui.vim_mode_defaultdeserialization and ChatWidget integration tests assert both insert-mode and normal-mode startup.[], function key range.shift-amatches bothShift+aand plainA;ctrl-shift-imatchesCtrl+I; no false positives on wrong case or unrelated uppercase letters.defaults_pass_conflict_validationensures built-in presets are self-consistent.[tui.keymap.*]paths; TUI tests cover the picker catalog, action menu, capture view, conflict handling, remove-custom-binding enablement, selected-detail updates, and wide/narrow snapshots.