From 4673090f733cb1e691181e2c63f28f3c24cee752 Mon Sep 17 00:00:00 2001 From: sayan-oai Date: Mon, 22 Dec 2025 15:12:23 -0800 Subject: [PATCH] feat: open prompt in configured external editor (#7606) Add `ctrl+g` shortcut to enable opening current prompt in configured editor (`$VISUAL` or `$EDITOR`). - Prompt is updated with editor's content upon editor close. - Paste placeholders are automatically expanded when opening the external editor, and are not "recompressed" on close - They could be preserved in the editor, but it would be hard to prevent the user from modifying the placeholder text directly, which would drop the mapping to the `pending_paste` value - Image placeholders stay as-is - `ctrl+g` explanation added to shortcuts menu, snapshot tests updated https://github.com/user-attachments/assets/4ee05c81-fa49-4e99-8b07-fc9eef0bbfce --- codex-rs/Cargo.lock | 10 + codex-rs/tui/Cargo.toml | 9 + codex-rs/tui/src/app.rs | 92 +++++++ codex-rs/tui/src/app_event.rs | 3 + codex-rs/tui/src/bottom_pane/chat_composer.rs | 239 +++++++++++++++++- codex-rs/tui/src/bottom_pane/footer.rs | 13 + codex-rs/tui/src/bottom_pane/mod.rs | 19 ++ ...__tests__footer_mode_shortcut_overlay.snap | 9 +- ...tests__footer_shortcuts_shift_and_esc.snap | 9 +- codex-rs/tui/src/chatwidget.rs | 38 ++- codex-rs/tui/src/chatwidget/tests.rs | 1 + codex-rs/tui/src/external_editor.rs | 171 +++++++++++++ codex-rs/tui/src/lib.rs | 1 + codex-rs/tui/src/tui.rs | 127 +++++++++- 14 files changed, 723 insertions(+), 18 deletions(-) create mode 100644 codex-rs/tui/src/external_editor.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index fb63e758e..42c8c557d 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1756,6 +1756,7 @@ dependencies = [ "supports-color 3.0.2", "tempfile", "textwrap 0.16.2", + "thiserror 2.0.17", "tokio", "tokio-stream", "tokio-util", @@ -1770,6 +1771,9 @@ dependencies = [ "url", "uuid", "vt100", + "which", + "windows-sys 0.52.0", + "winsplit", ] [[package]] @@ -8379,6 +8383,12 @@ version = "0.0.19" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d135d17ab770252ad95e9a872d365cf3090e3be864a34ab46f48555993efc904" +[[package]] +name = "winsplit" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3ab703352da6a72f35c39a533526393725640575bb211f61987a2748323ad956" + [[package]] name = "wiremock" version = "0.6.5" diff --git a/codex-rs/tui/Cargo.toml b/codex-rs/tui/Cargo.toml index 06167c973..3248c7377 100644 --- a/codex-rs/tui/Cargo.toml +++ b/codex-rs/tui/Cargo.toml @@ -71,6 +71,7 @@ strum_macros = { workspace = true } supports-color = { workspace = true } tempfile = { workspace = true } textwrap = { workspace = true } +thiserror = { workspace = true } tokio = { workspace = true, features = [ "io-std", "macros", @@ -97,6 +98,14 @@ tokio-util = { workspace = true, features = ["time"] } [target.'cfg(unix)'.dependencies] libc = { workspace = true } +[target.'cfg(windows)'.dependencies] +which = { workspace = true } +windows-sys = { version = "0.52", features = [ + "Win32_Foundation", + "Win32_System_Console", +] } +winsplit = "0.1" + # Clipboard support via `arboard` is not available on Android/Termux. # Only include it for non-Android targets so the crate builds on Android. [target.'cfg(not(target_os = "android"))'.dependencies] diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 14b785ec9..c91eafcbe 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -3,9 +3,12 @@ use crate::app_event::AppEvent; use crate::app_event_sender::AppEventSender; use crate::bottom_pane::ApprovalRequest; use crate::chatwidget::ChatWidget; +use crate::chatwidget::ExternalEditorState; use crate::diff_render::DiffSummary; use crate::exec_command::strip_bash_lc_and_escape; +use crate::external_editor; use crate::file_search::FileSearchManager; +use crate::history_cell; use crate::history_cell::HistoryCell; use crate::model_migration::ModelMigrationOutcome; use crate::model_migration::migration_copy_for_models; @@ -62,6 +65,8 @@ use tokio::sync::mpsc::unbounded_channel; #[cfg(not(debug_assertions))] use crate::history_cell::UpdateAvailableHistoryCell; +const EXTERNAL_EDITOR_HINT: &str = "Save and close external editor to continue."; + #[derive(Debug, Clone)] pub struct AppExitInfo { pub token_usage: TokenUsage, @@ -542,6 +547,11 @@ impl App { } }, )?; + if self.chat_widget.external_editor_state() == ExternalEditorState::Requested { + self.chat_widget + .set_external_editor_state(ExternalEditorState::Active); + self.app_event_tx.send(AppEvent::LaunchExternalEditor); + } } } } @@ -796,6 +806,11 @@ impl App { AppEvent::OpenFeedbackConsent { category } => { self.chat_widget.open_feedback_consent(category); } + AppEvent::LaunchExternalEditor => { + if self.chat_widget.external_editor_state() == ExternalEditorState::Active { + self.launch_external_editor(tui).await; + } + } AppEvent::OpenWindowsSandboxEnablePrompt { preset } => { self.chat_widget.open_windows_sandbox_enable_prompt(preset); } @@ -1148,6 +1163,68 @@ impl App { self.config.model_reasoning_effort = effort; } + async fn launch_external_editor(&mut self, tui: &mut tui::Tui) { + let editor_cmd = match external_editor::resolve_editor_command() { + Ok(cmd) => cmd, + Err(external_editor::EditorError::MissingEditor) => { + self.chat_widget + .add_to_history(history_cell::new_error_event( + "Cannot open external editor: set $VISUAL or $EDITOR".to_string(), + )); + self.reset_external_editor_state(tui); + return; + } + Err(err) => { + self.chat_widget + .add_to_history(history_cell::new_error_event(format!( + "Failed to open editor: {err}", + ))); + self.reset_external_editor_state(tui); + return; + } + }; + + let seed = self.chat_widget.composer_text_with_pending(); + let editor_result = tui + .with_restored(tui::RestoreMode::KeepRaw, || async { + external_editor::run_editor(&seed, &editor_cmd).await + }) + .await; + self.reset_external_editor_state(tui); + + match editor_result { + Ok(new_text) => { + // Trim trailing whitespace + let cleaned = new_text.trim_end().to_string(); + self.chat_widget.apply_external_edit(cleaned); + } + Err(err) => { + self.chat_widget + .add_to_history(history_cell::new_error_event(format!( + "Failed to open editor: {err}", + ))); + } + } + tui.frame_requester().schedule_frame(); + } + + fn request_external_editor_launch(&mut self, tui: &mut tui::Tui) { + self.chat_widget + .set_external_editor_state(ExternalEditorState::Requested); + self.chat_widget.set_footer_hint_override(Some(vec![( + EXTERNAL_EDITOR_HINT.to_string(), + String::new(), + )])); + tui.frame_requester().schedule_frame(); + } + + fn reset_external_editor_state(&mut self, tui: &mut tui::Tui) { + self.chat_widget + .set_external_editor_state(ExternalEditorState::Closed); + self.chat_widget.set_footer_hint_override(None); + tui.frame_requester().schedule_frame(); + } + async fn handle_key_event(&mut self, tui: &mut tui::Tui, key_event: KeyEvent) { match key_event { KeyEvent { @@ -1161,6 +1238,21 @@ impl App { self.overlay = Some(Overlay::new_transcript(self.transcript_cells.clone())); tui.frame_requester().schedule_frame(); } + KeyEvent { + code: KeyCode::Char('g'), + modifiers: crossterm::event::KeyModifiers::CONTROL, + kind: KeyEventKind::Press, + .. + } => { + // Only launch the external editor if there is no overlay and the bottom pane is not in use. + // Note that it can be launched while a task is running to enable editing while the previous turn is ongoing. + if self.overlay.is_none() + && self.chat_widget.can_launch_external_editor() + && self.chat_widget.external_editor_state() == ExternalEditorState::Closed + { + self.request_external_editor_launch(tui); + } + } // Esc primes/advances backtracking only in normal (not working) mode // with the composer focused and empty. In any other state, forward // Esc so the active UI (e.g. status indicator, modals, popups) diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index 0be556de8..1f99e372e 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -181,6 +181,9 @@ pub(crate) enum AppEvent { OpenFeedbackConsent { category: FeedbackCategory, }, + + /// Launch the external editor after a normal draw has completed. + LaunchExternalEditor, } #[derive(Debug, Clone, Copy, PartialEq, Eq)] diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 17877e4bf..d6d01045b 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -282,6 +282,85 @@ impl ChatComposer { } } + /// Replace the composer content with text from an external editor. + /// Clears pending paste placeholders and keeps only attachments whose + /// placeholder labels still appear in the new text. Cursor is placed at + /// the end after rebuilding elements. + pub(crate) fn apply_external_edit(&mut self, text: String) { + self.pending_pastes.clear(); + + // Count placeholder occurrences in the new text. + let mut placeholder_counts: HashMap = HashMap::new(); + for placeholder in self.attached_images.iter().map(|img| &img.placeholder) { + if placeholder_counts.contains_key(placeholder) { + continue; + } + let count = text.match_indices(placeholder).count(); + if count > 0 { + placeholder_counts.insert(placeholder.clone(), count); + } + } + + // Keep attachments only while we have matching occurrences left. + let mut kept_images = Vec::new(); + for img in self.attached_images.drain(..) { + if let Some(count) = placeholder_counts.get_mut(&img.placeholder) + && *count > 0 + { + *count -= 1; + kept_images.push(img); + } + } + self.attached_images = kept_images; + + // Rebuild textarea so placeholders become elements again. + self.textarea.set_text(""); + let mut remaining: HashMap<&str, usize> = HashMap::new(); + for img in &self.attached_images { + *remaining.entry(img.placeholder.as_str()).or_insert(0) += 1; + } + + let mut occurrences: Vec<(usize, &str)> = Vec::new(); + for placeholder in remaining.keys() { + for (pos, _) in text.match_indices(placeholder) { + occurrences.push((pos, *placeholder)); + } + } + occurrences.sort_unstable_by_key(|(pos, _)| *pos); + + let mut idx = 0usize; + for (pos, ph) in occurrences { + let Some(count) = remaining.get_mut(ph) else { + continue; + }; + if *count == 0 { + continue; + } + if pos > idx { + self.textarea.insert_str(&text[idx..pos]); + } + self.textarea.insert_element(ph); + *count -= 1; + idx = pos + ph.len(); + } + if idx < text.len() { + self.textarea.insert_str(&text[idx..]); + } + + self.textarea.set_cursor(self.textarea.text().len()); + self.sync_popups(); + } + + pub(crate) fn current_text_with_pending(&self) -> String { + let mut text = self.textarea.text().to_string(); + for (placeholder, actual) in &self.pending_pastes { + if text.contains(placeholder) { + text = text.replace(placeholder, actual); + } + } + text + } + /// Override the footer hint items displayed beneath the composer. Passing /// `None` restores the default shortcut footer. pub(crate) fn set_footer_hint_override(&mut self, items: Option>) { @@ -321,7 +400,8 @@ impl ChatComposer { .file_name() .map(|name| name.to_string_lossy().into_owned()) .unwrap_or_else(|| "image".to_string()); - let placeholder = format!("[{file_label} {width}x{height}]"); + let base_placeholder = format!("{file_label} {width}x{height}"); + let placeholder = self.next_image_placeholder(&base_placeholder); // Insert as an element to match large paste placeholder behavior: // styled distinctly and treated atomically for cursor/mutations. self.textarea.insert_element(&placeholder); @@ -384,6 +464,22 @@ impl ChatComposer { } } + fn next_image_placeholder(&mut self, base: &str) -> String { + let text = self.textarea.text(); + let mut suffix = 1; + loop { + let placeholder = if suffix == 1 { + format!("[{base}]") + } else { + format!("[{base} #{suffix}]") + }; + if !text.contains(&placeholder) { + return placeholder; + } + suffix += 1; + } + } + pub(crate) fn insert_str(&mut self, text: &str) { self.textarea.insert_str(text); self.sync_popups(); @@ -3180,6 +3276,35 @@ mod tests { assert!(composer.attached_images.is_empty()); } + #[test] + fn duplicate_image_placeholders_get_suffix() { + let (tx, _rx) = unbounded_channel::(); + let sender = AppEventSender::new(tx); + let mut composer = ChatComposer::new( + true, + sender, + false, + "Ask Codex to do anything".to_string(), + false, + ); + let path = PathBuf::from("/tmp/image_dup.png"); + composer.attach_image(path.clone(), 10, 5, "PNG"); + composer.handle_paste(" ".into()); + composer.attach_image(path, 10, 5, "PNG"); + + let text = composer.textarea.text().to_string(); + assert!(text.contains("[image_dup.png 10x5]")); + assert!(text.contains("[image_dup.png 10x5 #2]")); + assert_eq!( + composer.attached_images[0].placeholder, + "[image_dup.png 10x5]" + ); + assert_eq!( + composer.attached_images[1].placeholder, + "[image_dup.png 10x5 #2]" + ); + } + #[test] fn image_placeholder_backspace_behaves_like_text_placeholder() { let (tx, _rx) = unbounded_channel::(); @@ -4029,4 +4154,116 @@ mod tests { "'/zzz' should not activate slash popup because it is not a prefix of any built-in command" ); } + + #[test] + fn apply_external_edit_rebuilds_text_and_attachments() { + let (tx, _rx) = unbounded_channel::(); + let sender = AppEventSender::new(tx); + let mut composer = ChatComposer::new( + true, + sender, + false, + "Ask Codex to do anything".to_string(), + false, + ); + + let placeholder = "[image 10x10]".to_string(); + composer.textarea.insert_element(&placeholder); + composer.attached_images.push(AttachedImage { + placeholder: placeholder.clone(), + path: PathBuf::from("img.png"), + }); + composer + .pending_pastes + .push(("[Pasted]".to_string(), "data".to_string())); + + composer.apply_external_edit(format!("Edited {placeholder} text")); + + assert_eq!( + composer.current_text(), + format!("Edited {placeholder} text") + ); + assert!(composer.pending_pastes.is_empty()); + assert_eq!(composer.attached_images.len(), 1); + assert_eq!(composer.attached_images[0].placeholder, placeholder); + assert_eq!(composer.textarea.cursor(), composer.current_text().len()); + } + + #[test] + fn apply_external_edit_drops_missing_attachments() { + let (tx, _rx) = unbounded_channel::(); + let sender = AppEventSender::new(tx); + let mut composer = ChatComposer::new( + true, + sender, + false, + "Ask Codex to do anything".to_string(), + false, + ); + + let placeholder = "[image 10x10]".to_string(); + composer.textarea.insert_element(&placeholder); + composer.attached_images.push(AttachedImage { + placeholder: placeholder.clone(), + path: PathBuf::from("img.png"), + }); + + composer.apply_external_edit("No images here".to_string()); + + assert_eq!(composer.current_text(), "No images here".to_string()); + assert!(composer.attached_images.is_empty()); + } + + #[test] + fn current_text_with_pending_expands_placeholders() { + let (tx, _rx) = unbounded_channel::(); + let sender = AppEventSender::new(tx); + let mut composer = ChatComposer::new( + true, + sender, + false, + "Ask Codex to do anything".to_string(), + false, + ); + + let placeholder = "[Pasted Content 5 chars]".to_string(); + composer.textarea.insert_element(&placeholder); + composer + .pending_pastes + .push((placeholder.clone(), "hello".to_string())); + + assert_eq!( + composer.current_text_with_pending(), + "hello".to_string(), + "placeholder should expand to actual text" + ); + } + + #[test] + fn apply_external_edit_limits_duplicates_to_occurrences() { + let (tx, _rx) = unbounded_channel::(); + let sender = AppEventSender::new(tx); + let mut composer = ChatComposer::new( + true, + sender, + false, + "Ask Codex to do anything".to_string(), + false, + ); + + let placeholder = "[image 10x10]".to_string(); + composer.textarea.insert_element(&placeholder); + composer.attached_images.push(AttachedImage { + placeholder: placeholder.clone(), + path: PathBuf::from("img.png"), + }); + + composer.apply_external_edit(format!("{placeholder} extra {placeholder}")); + + assert_eq!( + composer.current_text(), + format!("{placeholder} extra {placeholder}") + ); + assert_eq!(composer.attached_images.len(), 1); + } } diff --git a/codex-rs/tui/src/bottom_pane/footer.rs b/codex-rs/tui/src/bottom_pane/footer.rs index d47ffec98..c3f2da0e3 100644 --- a/codex-rs/tui/src/bottom_pane/footer.rs +++ b/codex-rs/tui/src/bottom_pane/footer.rs @@ -162,6 +162,7 @@ fn shortcut_overlay_lines(state: ShortcutsState) -> Vec> { let mut newline = Line::from(""); let mut file_paths = Line::from(""); let mut paste_image = Line::from(""); + let mut external_editor = Line::from(""); let mut edit_previous = Line::from(""); let mut quit = Line::from(""); let mut show_transcript = Line::from(""); @@ -173,6 +174,7 @@ fn shortcut_overlay_lines(state: ShortcutsState) -> Vec> { ShortcutId::InsertNewline => newline = text, ShortcutId::FilePaths => file_paths = text, ShortcutId::PasteImage => paste_image = text, + ShortcutId::ExternalEditor => external_editor = text, ShortcutId::EditPrevious => edit_previous = text, ShortcutId::Quit => quit = text, ShortcutId::ShowTranscript => show_transcript = text, @@ -185,6 +187,7 @@ fn shortcut_overlay_lines(state: ShortcutsState) -> Vec> { newline, file_paths, paste_image, + external_editor, edit_previous, quit, Line::from(""), @@ -261,6 +264,7 @@ enum ShortcutId { InsertNewline, FilePaths, PasteImage, + ExternalEditor, EditPrevious, Quit, ShowTranscript, @@ -381,6 +385,15 @@ const SHORTCUTS: &[ShortcutDescriptor] = &[ prefix: "", label: " to paste images", }, + ShortcutDescriptor { + id: ShortcutId::ExternalEditor, + bindings: &[ShortcutBinding { + key: key_hint::ctrl(KeyCode::Char('g')), + condition: DisplayCondition::Always, + }], + prefix: "", + label: " to edit in external editor", + }, ShortcutDescriptor { id: ShortcutId::EditPrevious, bindings: &[ShortcutBinding { diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index ea599698b..4ad117082 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -274,6 +274,20 @@ impl BottomPane { self.composer.current_text() } + pub(crate) fn composer_text_with_pending(&self) -> String { + self.composer.current_text_with_pending() + } + + pub(crate) fn apply_external_edit(&mut self, text: String) { + self.composer.apply_external_edit(text); + self.request_redraw(); + } + + pub(crate) fn set_footer_hint_override(&mut self, items: Option>) { + self.composer.set_footer_hint_override(items); + self.request_redraw(); + } + /// Update the animated header shown to the left of the brackets in the /// status indicator (defaults to "Working"). No-ops if the status /// indicator is not active. @@ -428,6 +442,11 @@ impl BottomPane { !self.is_task_running && self.view_stack.is_empty() && !self.composer.popup_active() } + /// Return true when no popups or modal views are active, regardless of task state. + pub(crate) fn can_launch_external_editor(&self) -> bool { + self.view_stack.is_empty() && !self.composer.popup_active() + } + pub(crate) fn show_view(&mut self, view: Box) { self.push_view(view); } diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_mode_shortcut_overlay.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_mode_shortcut_overlay.snap index 3b6782d06..7d05a9223 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_mode_shortcut_overlay.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_mode_shortcut_overlay.snap @@ -10,7 +10,8 @@ expression: terminal.backend() " " " " " " -" / for commands shift + enter for newline " -" @ for file paths ctrl + v to paste images " -" esc again to edit previous message ctrl + c to exit " -" ctrl + t to view transcript " +" / for commands shift + enter for newline " +" @ for file paths ctrl + v to paste images " +" ctrl + g to edit in external editor esc again to edit previous message " +" ctrl + c to exit " +" ctrl + t to view transcript " diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_shortcuts_shift_and_esc.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_shortcuts_shift_and_esc.snap index 264515a6c..445fa4448 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_shortcuts_shift_and_esc.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_shortcuts_shift_and_esc.snap @@ -2,7 +2,8 @@ source: tui/src/bottom_pane/footer.rs expression: terminal.backend() --- -" / for commands shift + enter for newline " -" @ for file paths ctrl + v to paste images " -" esc again to edit previous message ctrl + c to exit " -" ctrl + t to view transcript " +" / for commands shift + enter for newline " +" @ for file paths ctrl + v to paste images " +" ctrl + g to edit in external editor esc again to edit previous message " +" ctrl + c to exit " +" ctrl + t to view transcript " diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index f0afed3bf..aad4e44ad 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -302,6 +302,14 @@ enum RateLimitSwitchPromptState { Shown, } +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub(crate) enum ExternalEditorState { + #[default] + Closed, + Requested, + Active, +} + pub(crate) struct ChatWidget { app_event_tx: AppEventSender, codex_op_tx: UnboundedSender, @@ -360,6 +368,7 @@ pub(crate) struct ChatWidget { feedback: codex_feedback::CodexFeedback, // Current session rollout path (if known) current_rollout_path: Option, + external_editor_state: ExternalEditorState, } struct UserMessage { @@ -1469,6 +1478,7 @@ impl ChatWidget { last_rendered_width: std::cell::Cell::new(None), feedback, current_rollout_path: None, + external_editor_state: ExternalEditorState::Closed, }; widget.prefetch_rate_limits(); @@ -1555,6 +1565,7 @@ impl ChatWidget { last_rendered_width: std::cell::Cell::new(None), feedback, current_rollout_path: None, + external_editor_state: ExternalEditorState::Closed, }; widget.prefetch_rate_limits(); @@ -1653,6 +1664,31 @@ impl ChatWidget { self.request_redraw(); } + pub(crate) fn composer_text_with_pending(&self) -> String { + self.bottom_pane.composer_text_with_pending() + } + + pub(crate) fn apply_external_edit(&mut self, text: String) { + self.bottom_pane.apply_external_edit(text); + self.request_redraw(); + } + + pub(crate) fn external_editor_state(&self) -> ExternalEditorState { + self.external_editor_state + } + + pub(crate) fn set_external_editor_state(&mut self, state: ExternalEditorState) { + self.external_editor_state = state; + } + + pub(crate) fn set_footer_hint_override(&mut self, items: Option>) { + self.bottom_pane.set_footer_hint_override(items); + } + + pub(crate) fn can_launch_external_editor(&self) -> bool { + self.bottom_pane.can_launch_external_editor() + } + fn dispatch_command(&mut self, cmd: SlashCommand) { if !cmd.available_during_task() && self.bottom_pane.is_task_running() { let message = format!( @@ -1854,7 +1890,7 @@ impl ChatWidget { .send(AppEvent::InsertHistoryCell(Box::new(cell))); } - fn add_to_history(&mut self, cell: impl HistoryCell + 'static) { + pub(crate) fn add_to_history(&mut self, cell: impl HistoryCell + 'static) { self.add_boxed_history(Box::new(cell)); } diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 5e595d330..5aa0e5d06 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -407,6 +407,7 @@ async fn make_chatwidget_manual( last_rendered_width: std::cell::Cell::new(None), feedback: codex_feedback::CodexFeedback::new(), current_rollout_path: None, + external_editor_state: ExternalEditorState::Closed, }; (widget, rx, op_rx) } diff --git a/codex-rs/tui/src/external_editor.rs b/codex-rs/tui/src/external_editor.rs new file mode 100644 index 000000000..2818503d0 --- /dev/null +++ b/codex-rs/tui/src/external_editor.rs @@ -0,0 +1,171 @@ +use std::env; +use std::fs; +use std::process::Stdio; + +use color_eyre::eyre::Report; +use color_eyre::eyre::Result; +use tempfile::Builder; +use thiserror::Error; +use tokio::process::Command; + +#[derive(Debug, Error)] +pub(crate) enum EditorError { + #[error("neither VISUAL nor EDITOR is set")] + MissingEditor, + #[cfg(not(windows))] + #[error("failed to parse editor command")] + ParseFailed, + #[error("editor command is empty")] + EmptyCommand, +} + +/// Tries to resolve the full path to a Windows program, respecting PATH + PATHEXT. +/// Falls back to the original program name if resolution fails. +#[cfg(windows)] +fn resolve_windows_program(program: &str) -> std::path::PathBuf { + // On Windows, `Command::new("code")` will not resolve `code.cmd` shims on PATH. + // Use `which` so we respect PATH + PATHEXT (e.g., `code` -> `code.cmd`). + which::which(program).unwrap_or_else(|_| std::path::PathBuf::from(program)) +} + +/// Resolve the editor command from environment variables. +/// Prefers `VISUAL` over `EDITOR`. +pub(crate) fn resolve_editor_command() -> std::result::Result, EditorError> { + let raw = env::var("VISUAL") + .or_else(|_| env::var("EDITOR")) + .map_err(|_| EditorError::MissingEditor)?; + let parts = { + #[cfg(windows)] + { + winsplit::split(&raw) + } + #[cfg(not(windows))] + { + shlex::split(&raw).ok_or(EditorError::ParseFailed)? + } + }; + if parts.is_empty() { + return Err(EditorError::EmptyCommand); + } + Ok(parts) +} + +/// Write `seed` to a temp file, launch the editor command, and return the updated content. +pub(crate) async fn run_editor(seed: &str, editor_cmd: &[String]) -> Result { + if editor_cmd.is_empty() { + return Err(Report::msg("editor command is empty")); + } + + // Convert to TempPath immediately so no file handle stays open on Windows. + let temp_path = Builder::new().suffix(".md").tempfile()?.into_temp_path(); + fs::write(&temp_path, seed)?; + + let mut cmd = { + #[cfg(windows)] + { + // handles .cmd/.bat shims + Command::new(resolve_windows_program(&editor_cmd[0])) + } + #[cfg(not(windows))] + { + Command::new(&editor_cmd[0]) + } + }; + if editor_cmd.len() > 1 { + cmd.args(&editor_cmd[1..]); + } + let status = cmd + .arg(&temp_path) + .stdin(Stdio::inherit()) + .stdout(Stdio::inherit()) + .stderr(Stdio::inherit()) + .status() + .await?; + + if !status.success() { + return Err(Report::msg(format!("editor exited with status {status}"))); + } + + let contents = fs::read_to_string(&temp_path)?; + Ok(contents) +} + +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + use serial_test::serial; + #[cfg(unix)] + use tempfile::tempdir; + + struct EnvGuard { + visual: Option, + editor: Option, + } + + impl EnvGuard { + fn new() -> Self { + Self { + visual: env::var("VISUAL").ok(), + editor: env::var("EDITOR").ok(), + } + } + } + + impl Drop for EnvGuard { + fn drop(&mut self) { + restore_env("VISUAL", self.visual.take()); + restore_env("EDITOR", self.editor.take()); + } + } + + fn restore_env(key: &str, value: Option) { + match value { + Some(val) => unsafe { env::set_var(key, val) }, + None => unsafe { env::remove_var(key) }, + } + } + + #[test] + #[serial] + fn resolve_editor_prefers_visual() { + let _guard = EnvGuard::new(); + unsafe { + env::set_var("VISUAL", "vis"); + env::set_var("EDITOR", "ed"); + } + let cmd = resolve_editor_command().unwrap(); + assert_eq!(cmd, vec!["vis".to_string()]); + } + + #[test] + #[serial] + fn resolve_editor_errors_when_unset() { + let _guard = EnvGuard::new(); + unsafe { + env::remove_var("VISUAL"); + env::remove_var("EDITOR"); + } + assert!(matches!( + resolve_editor_command(), + Err(EditorError::MissingEditor) + )); + } + + #[tokio::test] + #[cfg(unix)] + async fn run_editor_returns_updated_content() { + use std::os::unix::fs::PermissionsExt; + + let dir = tempdir().unwrap(); + let script_path = dir.path().join("edit.sh"); + fs::write(&script_path, "#!/bin/sh\nprintf \"edited\" > \"$1\"\n").unwrap(); + let mut perms = fs::metadata(&script_path).unwrap().permissions(); + perms.set_mode(0o755); + fs::set_permissions(&script_path, perms).unwrap(); + + let cmd = vec![script_path.to_string_lossy().to_string()]; + let result = run_editor("seed", &cmd).await.unwrap(); + assert_eq!(result, "edited".to_string()); + } +} diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index b18754cab..5f34c6f62 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -47,6 +47,7 @@ pub mod custom_terminal; mod diff_render; mod exec_cell; mod exec_command; +mod external_editor; mod file_search; mod frames; mod get_git_diff; diff --git a/codex-rs/tui/src/tui.rs b/codex-rs/tui/src/tui.rs index 0770b7664..e35e192a5 100644 --- a/codex-rs/tui/src/tui.rs +++ b/codex-rs/tui/src/tui.rs @@ -1,4 +1,5 @@ use std::fmt; +use std::future::Future; use std::io::IsTerminal; use std::io::Result; use std::io::Stdout; @@ -118,18 +119,86 @@ impl Command for DisableAlternateScroll { } } -/// Restore the terminal to its original state. -/// Inverse of `set_modes`. -pub fn restore() -> Result<()> { +fn restore_common(should_disable_raw_mode: bool) -> Result<()> { // Pop may fail on platforms that didn't support the push; ignore errors. let _ = execute!(stdout(), PopKeyboardEnhancementFlags); execute!(stdout(), DisableBracketedPaste)?; let _ = execute!(stdout(), DisableFocusChange); - disable_raw_mode()?; + if should_disable_raw_mode { + disable_raw_mode()?; + } let _ = execute!(stdout(), crossterm::cursor::Show); Ok(()) } +/// Restore the terminal to its original state. +/// Inverse of `set_modes`. +pub fn restore() -> Result<()> { + let should_disable_raw_mode = true; + restore_common(should_disable_raw_mode) +} + +/// Restore the terminal to its original state, but keep raw mode enabled. +pub fn restore_keep_raw() -> Result<()> { + let should_disable_raw_mode = false; + restore_common(should_disable_raw_mode) +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum RestoreMode { + #[allow(dead_code)] + Full, // Fully restore the terminal (disables raw mode). + KeepRaw, // Restore the terminal but keep raw mode enabled. +} + +impl RestoreMode { + fn restore(self) -> Result<()> { + match self { + RestoreMode::Full => restore(), + RestoreMode::KeepRaw => restore_keep_raw(), + } + } +} + +/// Flush the underlying stdin buffer to clear any input that may be buffered at the terminal level. +/// For example, clears any user input that occurred while the crossterm EventStream was dropped. +#[cfg(unix)] +fn flush_terminal_input_buffer() { + // Safety: flushing the stdin queue is safe and does not move ownership. + let result = unsafe { libc::tcflush(libc::STDIN_FILENO, libc::TCIFLUSH) }; + if result != 0 { + let err = std::io::Error::last_os_error(); + tracing::warn!("failed to tcflush stdin: {err}"); + } +} + +/// Flush the underlying stdin buffer to clear any input that may be buffered at the terminal level. +/// For example, clears any user input that occurred while the crossterm EventStream was dropped. +#[cfg(windows)] +fn flush_terminal_input_buffer() { + use windows_sys::Win32::Foundation::GetLastError; + use windows_sys::Win32::Foundation::INVALID_HANDLE_VALUE; + use windows_sys::Win32::System::Console::FlushConsoleInputBuffer; + use windows_sys::Win32::System::Console::GetStdHandle; + use windows_sys::Win32::System::Console::STD_INPUT_HANDLE; + + let handle = unsafe { GetStdHandle(STD_INPUT_HANDLE) }; + if handle == INVALID_HANDLE_VALUE || handle == 0 { + let err = unsafe { GetLastError() }; + tracing::warn!("failed to get stdin handle for flush: error {err}"); + return; + } + + let result = unsafe { FlushConsoleInputBuffer(handle) }; + if result == 0 { + let err = unsafe { GetLastError() }; + tracing::warn!("failed to flush stdin buffer: error {err}"); + } +} + +#[cfg(not(any(unix, windows)))] +pub(crate) fn flush_terminal_input_buffer() {} + /// Initialize the terminal (inline viewport; history stays in normal scrollback) pub fn init() -> Result { if !stdin().is_terminal() { @@ -215,18 +284,60 @@ impl Tui { self.enhanced_keys_supported } - // todo(sayan) unused for now; intend to use to enable opening external editors - #[allow(unused)] + pub fn is_alt_screen_active(&self) -> bool { + self.alt_screen_active.load(Ordering::Relaxed) + } + + // Drop crossterm EventStream to avoid stdin conflicts with other processes. pub fn pause_events(&mut self) { self.event_broker.pause_events(); } - // todo(sayan) unused for now; intend to use to enable opening external editors - #[allow(unused)] + // Resume crossterm EventStream to resume stdin polling. + // Inverse of `pause_events`. pub fn resume_events(&mut self) { self.event_broker.resume_events(); } + /// Temporarily restore terminal state to run an external interactive program `f`. + /// + /// This pauses crossterm's stdin polling by dropping the underlying event stream, restores + /// terminal modes (optionally keeping raw mode enabled), then re-applies Codex TUI modes and + /// flushes pending stdin input before resuming events. + pub async fn with_restored(&mut self, mode: RestoreMode, f: F) -> R + where + F: FnOnce() -> Fut, + Fut: Future, + { + // Pause crossterm events to avoid stdin conflicts with external program `f`. + self.pause_events(); + + // Leave alt screen if active to avoid conflicts with external program `f`. + let was_alt_screen = self.is_alt_screen_active(); + if was_alt_screen { + let _ = self.leave_alt_screen(); + } + + if let Err(err) = mode.restore() { + tracing::warn!("failed to restore terminal modes before external program: {err}"); + } + + let output = f().await; + + if let Err(err) = set_modes() { + tracing::warn!("failed to re-enable terminal modes after external program: {err}"); + } + // After the external program `f` finishes, reset terminal state and flush any buffered keypresses. + flush_terminal_input_buffer(); + + if was_alt_screen { + let _ = self.enter_alt_screen(); + } + + self.resume_events(); + output + } + /// Emit a desktop notification now if the terminal is unfocused. /// Returns true if a notification was posted. pub fn notify(&mut self, message: impl AsRef) -> bool {