mirror of
https://github.com/openai/codex.git
synced 2026-10-21 12:10:06 +00:00
Recover Vim escape input in legacy terminals (#42584)
## Why Legacy terminals encode `Alt` plus a character the same way as `Esc` followed by that character. In Vim insert or replace mode, typing a command immediately after `Esc` could therefore leave the composer in the wrong mode. ## What changed Recover ambiguous input as separate `Esc` and character events when Vim mode can consume the escape. Preserve active `Alt` bindings, key chords, image paste, agent navigation, enhanced-key terminals, and AltGr input. ## Testing Add coverage for recovered Vim commands and for each shortcut and terminal case that must retain its existing behavior. GitOrigin-RevId: 3d8464d805f95f677cce5c46ae00338e611d776a
This commit is contained in:
committed by
copyberry
parent
1d74c3ba1e
commit
1d6727c0b5
+10
-1
@@ -837,7 +837,16 @@ impl App {
|
||||
self.handle_draw_pre_render(tui, screen_size)?;
|
||||
}
|
||||
|
||||
let event = if let TuiEvent::Key(key_event) = event {
|
||||
let event = if let TuiEvent::Key(mut key_event) = event {
|
||||
let escape = KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE);
|
||||
if self.should_recover_vim_insert_escape(key_event) {
|
||||
// Restore both strokes before chords or global shortcuts can consume them.
|
||||
if let Some(escape) = self.route_key_chord_event(tui, escape) {
|
||||
self.handle_key_event(tui, app_server, escape).await;
|
||||
}
|
||||
key_event.modifiers.remove(KeyModifiers::ALT);
|
||||
}
|
||||
|
||||
let Some(key_event) = self.route_key_chord_event(tui, key_event) else {
|
||||
return Ok(AppRunControl::Continue);
|
||||
};
|
||||
|
||||
@@ -5,8 +5,50 @@
|
||||
|
||||
use super::*;
|
||||
use crate::app_backtrack::SIDE_EDIT_PREVIOUS_UNAVAILABLE_MESSAGE;
|
||||
use crate::keymap::bindings_for_action;
|
||||
use crate::keymap::keymap_action_ids;
|
||||
|
||||
impl App {
|
||||
pub(super) fn should_recover_vim_insert_escape(&self, key_event: KeyEvent) -> bool {
|
||||
let active_contexts = self.active_keymap_contexts();
|
||||
// Legacy terminals encode Alt+character and Escape+character identically. Active
|
||||
// bindings and either stroke of a chord win; inactive bindings do not consume input.
|
||||
cfg!(unix)
|
||||
&& !self.enhanced_keys_supported
|
||||
&& self.overlay.is_none()
|
||||
&& self.chat_widget.no_modal_or_popup_active()
|
||||
&& matches!(key_event.code, KeyCode::Char(_))
|
||||
&& matches!(key_event.kind, KeyEventKind::Press | KeyEventKind::Repeat)
|
||||
&& (key_event.modifiers == KeyModifiers::ALT
|
||||
|| key_event.modifiers == (KeyModifiers::ALT | KeyModifiers::SHIFT))
|
||||
&& self
|
||||
.chat_widget
|
||||
.should_handle_vim_insert_escape(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE))
|
||||
// ChatWidget's fixed image-paste shortcut is not in the configurable keymap.
|
||||
&& !(key_event.kind == KeyEventKind::Press
|
||||
&& matches!(key_event.code, KeyCode::Char('v' | 'V')))
|
||||
// Empty-draft agent navigation also has fixed legacy-terminal fallbacks.
|
||||
&& !(self.chat_widget.composer_text_with_pending().is_empty()
|
||||
&& (previous_agent_shortcut_matches(key_event, /*allow_word_motion_fallback*/ true)
|
||||
|| next_agent_shortcut_matches(key_event, /*allow_word_motion_fallback*/ true)))
|
||||
&& !keymap_action_ids()
|
||||
.filter(|action| active_contexts.contains(action.context))
|
||||
.any(|action| {
|
||||
bindings_for_action(&self.keymap, action.context.config_name(), action.action)
|
||||
.is_some_and(|bindings| bindings.is_pressed(key_event))
|
||||
})
|
||||
&& !matches!(
|
||||
self.key_chord_matcher.clone().advance(
|
||||
key_event,
|
||||
&self.keymap.chords,
|
||||
active_contexts,
|
||||
tokio::time::Instant::now(),
|
||||
),
|
||||
crate::keymap::KeyChordMatch::Pending(_)
|
||||
| crate::keymap::KeyChordMatch::Completed(_)
|
||||
)
|
||||
}
|
||||
|
||||
pub(super) fn route_key_chord_event(
|
||||
&mut self,
|
||||
tui: &mut tui::Tui,
|
||||
|
||||
@@ -158,6 +158,317 @@ async fn wrong_second_stroke_passes_through_but_escape_is_consumed() -> Result<(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[tokio::test]
|
||||
async fn legacy_terminal_recovers_vim_escape_before_normal_commands() -> Result<()> {
|
||||
for (key, expected) in [
|
||||
(KeyEvent::new(KeyCode::Char('x'), KeyModifiers::ALT), "ab"),
|
||||
(
|
||||
KeyEvent::new(KeyCode::Char('D'), KeyModifiers::ALT | KeyModifiers::SHIFT),
|
||||
"ab",
|
||||
),
|
||||
(KeyEvent::new(KeyCode::Char('h'), KeyModifiers::ALT), "abc"),
|
||||
] {
|
||||
let (mut app, mut tui, mut app_server) = chord_app().await?;
|
||||
app.chat_widget.toggle_vim_mode_and_notify();
|
||||
app.chat_widget.insert_str("abc");
|
||||
press(
|
||||
&mut app,
|
||||
&mut tui,
|
||||
&mut app_server,
|
||||
KeyCode::Char('i').into(),
|
||||
)
|
||||
.await?;
|
||||
|
||||
press(&mut app, &mut tui, &mut app_server, key).await?;
|
||||
|
||||
assert_eq!(app.chat_widget.composer_text_with_pending(), expected);
|
||||
assert!(
|
||||
!app.chat_widget
|
||||
.should_handle_vim_insert_escape(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE))
|
||||
);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[tokio::test]
|
||||
async fn legacy_terminal_preserves_default_and_configured_editor_alt_bindings() -> Result<()> {
|
||||
for (key, expected) in [
|
||||
('b', "!abc def"),
|
||||
('f', "abc! def"),
|
||||
('d', "! def"),
|
||||
('k', "!"),
|
||||
] {
|
||||
let (mut app, mut tui, mut app_server) = chord_app().await?;
|
||||
if key == 'k' {
|
||||
let mut config = TuiKeymap::default();
|
||||
config.editor.kill_line_end =
|
||||
Some(KeybindingsSpec::One(KeybindingSpec("alt-k".to_string())));
|
||||
let runtime = RuntimeKeymap::from_config(&config)
|
||||
.map_err(|error| color_eyre::eyre::eyre!(error))?;
|
||||
app.chat_widget.apply_keymap_update(config, &runtime);
|
||||
app.keymap = runtime;
|
||||
}
|
||||
app.chat_widget.toggle_vim_mode_and_notify();
|
||||
app.chat_widget.insert_str("abc def");
|
||||
let insert = KeyCode::Char('i').into();
|
||||
press(&mut app, &mut tui, &mut app_server, insert).await?;
|
||||
press(&mut app, &mut tui, &mut app_server, ctrl('a')).await?;
|
||||
press(
|
||||
&mut app,
|
||||
&mut tui,
|
||||
&mut app_server,
|
||||
KeyEvent::new(KeyCode::Char(key), KeyModifiers::ALT),
|
||||
)
|
||||
.await?;
|
||||
|
||||
assert!(
|
||||
app.chat_widget
|
||||
.should_handle_vim_insert_escape(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE))
|
||||
);
|
||||
app.chat_widget.insert_str("!");
|
||||
assert_eq!(app.chat_widget.composer_text_with_pending(), expected);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[tokio::test]
|
||||
async fn legacy_terminal_preserves_active_global_alt_shortcuts() -> Result<()> {
|
||||
let (mut app, mut tui, mut app_server) = chord_app().await?;
|
||||
app.chat_widget.toggle_vim_mode_and_notify();
|
||||
app.chat_widget.insert_str("abc");
|
||||
press(
|
||||
&mut app,
|
||||
&mut tui,
|
||||
&mut app_server,
|
||||
KeyCode::Char('i').into(),
|
||||
)
|
||||
.await?;
|
||||
|
||||
press(
|
||||
&mut app,
|
||||
&mut tui,
|
||||
&mut app_server,
|
||||
KeyEvent::new(KeyCode::Char('r'), KeyModifiers::ALT),
|
||||
)
|
||||
.await?;
|
||||
|
||||
assert!(app.chat_widget.raw_output_mode());
|
||||
assert!(
|
||||
app.chat_widget
|
||||
.should_handle_vim_insert_escape(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE))
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[tokio::test]
|
||||
async fn legacy_terminal_preserves_configured_composer_alt_shortcuts() -> Result<()> {
|
||||
let (mut app, mut tui, mut app_server) = chord_app().await?;
|
||||
let mut config = TuiKeymap::default();
|
||||
config.composer.history_search_previous =
|
||||
Some(KeybindingsSpec::One(KeybindingSpec("alt-q".to_string())));
|
||||
let runtime =
|
||||
RuntimeKeymap::from_config(&config).map_err(|error| color_eyre::eyre::eyre!(error))?;
|
||||
app.chat_widget.apply_keymap_update(config, &runtime);
|
||||
app.keymap = runtime;
|
||||
app.chat_widget.toggle_vim_mode_and_notify();
|
||||
app.chat_widget.insert_str("abc");
|
||||
press(
|
||||
&mut app,
|
||||
&mut tui,
|
||||
&mut app_server,
|
||||
KeyCode::Char('i').into(),
|
||||
)
|
||||
.await?;
|
||||
|
||||
press(
|
||||
&mut app,
|
||||
&mut tui,
|
||||
&mut app_server,
|
||||
KeyEvent::new(KeyCode::Char('q'), KeyModifiers::ALT),
|
||||
)
|
||||
.await?;
|
||||
|
||||
assert!(render_bottom_popup(&app.chat_widget, /*width*/ 80).contains("reverse-i-search:"));
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[tokio::test]
|
||||
async fn legacy_terminal_preserves_active_alt_chords() -> Result<()> {
|
||||
for (binding, prefix, completion) in [
|
||||
(
|
||||
"alt-q r",
|
||||
KeyEvent::new(KeyCode::Char('q'), KeyModifiers::ALT),
|
||||
KeyCode::Char('r').into(),
|
||||
),
|
||||
(
|
||||
"ctrl-x alt-h",
|
||||
ctrl('x'),
|
||||
KeyEvent::new(KeyCode::Char('h'), KeyModifiers::ALT),
|
||||
),
|
||||
] {
|
||||
let (mut app, mut tui, mut app_server) = chord_app().await?;
|
||||
let mut config = TuiKeymap::default();
|
||||
config.global.toggle_raw_output =
|
||||
Some(KeybindingsSpec::One(KeybindingSpec(binding.to_string())));
|
||||
let runtime =
|
||||
RuntimeKeymap::from_config(&config).map_err(|error| color_eyre::eyre::eyre!(error))?;
|
||||
app.chat_widget.apply_keymap_update(config, &runtime);
|
||||
app.keymap = runtime;
|
||||
app.chat_widget.toggle_vim_mode_and_notify();
|
||||
app.chat_widget.insert_str("abc");
|
||||
press(
|
||||
&mut app,
|
||||
&mut tui,
|
||||
&mut app_server,
|
||||
KeyCode::Char('i').into(),
|
||||
)
|
||||
.await?;
|
||||
|
||||
press(&mut app, &mut tui, &mut app_server, prefix).await?;
|
||||
assert!(app.key_chord_matcher.is_pending());
|
||||
press(&mut app, &mut tui, &mut app_server, completion).await?;
|
||||
|
||||
assert!(app.chat_widget.raw_output_mode());
|
||||
assert!(!app.key_chord_matcher.is_pending());
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[tokio::test]
|
||||
async fn legacy_terminal_preserves_image_paste_without_reading_clipboard() -> Result<()> {
|
||||
for mode in ['i', 'R'] {
|
||||
let (mut app, mut tui, mut app_server) = chord_app().await?;
|
||||
app.chat_widget.toggle_vim_mode_and_notify();
|
||||
app.chat_widget.insert_str("abc");
|
||||
press(
|
||||
&mut app,
|
||||
&mut tui,
|
||||
&mut app_server,
|
||||
KeyCode::Char(mode).into(),
|
||||
)
|
||||
.await?;
|
||||
for key in [
|
||||
KeyEvent::new(KeyCode::Char('v'), KeyModifiers::ALT),
|
||||
KeyEvent::new(KeyCode::Char('V'), KeyModifiers::ALT | KeyModifiers::SHIFT),
|
||||
] {
|
||||
assert!(!app.should_recover_vim_insert_escape(key));
|
||||
}
|
||||
assert!(app.should_recover_vim_insert_escape(KeyEvent::new(
|
||||
KeyCode::Char('x'),
|
||||
KeyModifiers::ALT,
|
||||
)));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(target_os = "macos")]
|
||||
#[tokio::test]
|
||||
async fn legacy_terminal_preserves_agent_shortcuts_without_editor_word_bindings() -> Result<()> {
|
||||
for mode in ['i', 'R'] {
|
||||
let (mut app, mut tui, mut app_server) = chord_app().await?;
|
||||
let mut config = TuiKeymap::default();
|
||||
config.editor.move_word_left = Some(KeybindingsSpec::Many(vec![]));
|
||||
config.editor.move_word_right = Some(KeybindingsSpec::Many(vec![]));
|
||||
let runtime =
|
||||
RuntimeKeymap::from_config(&config).map_err(|error| color_eyre::eyre::eyre!(error))?;
|
||||
app.chat_widget.apply_keymap_update(config, &runtime);
|
||||
app.keymap = runtime;
|
||||
app.chat_widget.toggle_vim_mode_and_notify();
|
||||
press(
|
||||
&mut app,
|
||||
&mut tui,
|
||||
&mut app_server,
|
||||
KeyCode::Char(mode).into(),
|
||||
)
|
||||
.await?;
|
||||
|
||||
for key in ['b', 'f'] {
|
||||
let event = KeyEvent::new(KeyCode::Char(key), KeyModifiers::ALT);
|
||||
assert!(!app.should_recover_vim_insert_escape(event));
|
||||
}
|
||||
app.chat_widget.insert_str("abc");
|
||||
for key in ['b', 'f'] {
|
||||
let event = KeyEvent::new(KeyCode::Char(key), KeyModifiers::ALT);
|
||||
assert!(app.should_recover_vim_insert_escape(event));
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn vim_escape_recovery_preserves_enhanced_terminals_and_altgr() -> Result<()> {
|
||||
for (enhanced_keys_supported, modifiers) in [
|
||||
(true, KeyModifiers::ALT),
|
||||
(false, KeyModifiers::ALT | KeyModifiers::CONTROL),
|
||||
] {
|
||||
let (mut app, mut tui, mut app_server) = chord_app().await?;
|
||||
app.enhanced_keys_supported = enhanced_keys_supported;
|
||||
app.chat_widget.toggle_vim_mode_and_notify();
|
||||
app.chat_widget.insert_str("abc");
|
||||
press(
|
||||
&mut app,
|
||||
&mut tui,
|
||||
&mut app_server,
|
||||
KeyCode::Char('i').into(),
|
||||
)
|
||||
.await?;
|
||||
|
||||
press(
|
||||
&mut app,
|
||||
&mut tui,
|
||||
&mut app_server,
|
||||
KeyEvent::new(KeyCode::Char('x'), modifiers),
|
||||
)
|
||||
.await?;
|
||||
|
||||
assert!(
|
||||
app.chat_widget
|
||||
.should_handle_vim_insert_escape(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE))
|
||||
);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[tokio::test]
|
||||
async fn recovered_vim_escape_cancels_a_pending_key_chord_first() -> Result<()> {
|
||||
let (mut app, mut tui, mut app_server) = chord_app().await?;
|
||||
app.chat_widget.toggle_vim_mode_and_notify();
|
||||
app.chat_widget.insert_str("abc");
|
||||
press(
|
||||
&mut app,
|
||||
&mut tui,
|
||||
&mut app_server,
|
||||
KeyCode::Char('i').into(),
|
||||
)
|
||||
.await?;
|
||||
press(&mut app, &mut tui, &mut app_server, ctrl('x')).await?;
|
||||
assert!(app.key_chord_matcher.is_pending());
|
||||
|
||||
press(
|
||||
&mut app,
|
||||
&mut tui,
|
||||
&mut app_server,
|
||||
KeyEvent::new(KeyCode::Char('x'), KeyModifiers::ALT),
|
||||
)
|
||||
.await?;
|
||||
|
||||
assert!(!app.key_chord_matcher.is_pending());
|
||||
press(&mut app, &mut tui, &mut app_server, KeyCode::Left.into()).await?;
|
||||
assert_eq!(app.chat_widget.composer_text_with_pending(), "abcx");
|
||||
assert!(
|
||||
app.chat_widget
|
||||
.should_handle_vim_insert_escape(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE))
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn physical_dispatch_band_events_are_dropped() -> Result<()> {
|
||||
let (mut app, mut tui, mut app_server) = chord_app().await?;
|
||||
|
||||
@@ -44,6 +44,7 @@ mod conflict_tests;
|
||||
pub(crate) use bindings::KeymapContext;
|
||||
pub(crate) use bindings::bindings_for_action;
|
||||
pub(crate) use bindings::keymap_action_id;
|
||||
pub(crate) use bindings::keymap_action_ids;
|
||||
use bindings::runtime_action_bindings;
|
||||
pub(crate) use chords::KEY_CHORD_TIMEOUT;
|
||||
pub(crate) use chords::KeyChordMatch;
|
||||
|
||||
@@ -231,7 +231,7 @@ struct PendingChord {
|
||||
}
|
||||
|
||||
/// Tracks one pending two-stroke chord without buffering ordinary input.
|
||||
#[derive(Debug, Default)]
|
||||
#[derive(Clone, Debug, Default)]
|
||||
pub(crate) struct KeyChordMatcher {
|
||||
pending: Option<PendingChord>,
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user