From 697fb5de4d4fd254173e555428f4d61306d5958e Mon Sep 17 00:00:00 2001 From: Hunter Bown Date: Sun, 26 Apr 2026 17:29:05 -0500 Subject: [PATCH] feat(tui): external editor support (Ctrl+E) Bind Ctrl+E in the composer to suspend the TUI, spawn $VISUAL/$EDITOR (fallback `vi`) on a temp file pre-populated with the composer's current contents, then read the file back into the composer on save. - New `crates/tui/src/tui/external_editor.rs`: - `spawn_editor_for_input` toggles raw mode + alt-screen + mouse + bracketed-paste before/after the edit and forces a `terminal.clear()` on return so a SIGWINCH during the edit doesn't leave a stale viewport. - `run_editor_raw` is the testable core: writes seed -> spawns editor -> reads back -> returns `Edited(new) | Unchanged | Cancelled`. Uses `tempfile::NamedTempFile` so the temp file is unlinked on every path (success, non-zero exit, missing binary, IO error). - `tui/ui.rs`: split the existing `End | Ctrl+e` cursor-end keybinding so `End` still moves the cursor to the line end, and `Ctrl+E` now spawns the external editor. Status-line feedback: "Edited in ", "Editor closed (no changes)", or "Editor cancelled". - 7 new unit tests cover the resolver precedence (VISUAL > EDITOR > vi), the no-op / failure / missing-binary / edited paths, and an explicit temp-file-cleanup assertion via a captured editor argv. Fixes #91 --- crates/tui/src/tui/external_editor.rs | 314 ++++++++++++++++++++++++++ crates/tui/src/tui/mod.rs | 1 + crates/tui/src/tui/ui.rs | 45 +++- 3 files changed, 357 insertions(+), 3 deletions(-) create mode 100644 crates/tui/src/tui/external_editor.rs diff --git a/crates/tui/src/tui/external_editor.rs b/crates/tui/src/tui/external_editor.rs new file mode 100644 index 00000000..33057b10 --- /dev/null +++ b/crates/tui/src/tui/external_editor.rs @@ -0,0 +1,314 @@ +//! External editor support for the composer. +//! +//! Spawns `$VISUAL`/`$EDITOR` (fallback `vi`) on a temp file pre-populated with +//! the composer's current contents. The TUI is suspended for the duration of +//! the edit and re-entered on return. The temp file is cleaned up in all paths +//! (success, editor failure, IO error) via [`tempfile::NamedTempFile`]. +//! +//! Reference: codex-rs's `tui/src/external_editor.rs` — the design here mirrors +//! that approach but is synchronous (called inline from the TUI event loop) and +//! handles its own raw-mode toggling rather than relying on the caller. + +use std::env; +use std::fs; +use std::io::{self, Stdout, Write}; +use std::process::Command; + +use crossterm::{ + event::{DisableBracketedPaste, DisableMouseCapture, EnableBracketedPaste, EnableMouseCapture}, + execute, + terminal::{EnterAlternateScreen, LeaveAlternateScreen, disable_raw_mode, enable_raw_mode}, +}; +use ratatui::{Terminal, backend::CrosstermBackend}; +use tempfile::Builder; + +/// Outcome of a single external-editor invocation. +#[derive(Debug, PartialEq, Eq)] +pub enum EditorOutcome { + /// Editor exited cleanly and the file contents differ from the seed. + Edited(String), + /// Editor exited cleanly but the contents are unchanged (or empty after + /// trimming). The composer should be left as-is. + Unchanged, + /// Editor exited non-zero or could not be spawned. The composer should be + /// left as-is and a status toast shown. + Cancelled, +} + +/// Resolve the editor command, preferring `$VISUAL` over `$EDITOR`, falling +/// back to `vi`. Returns the raw string for the test path; [`spawn_editor`] +/// splits it via `shlex` (Unix) so users can set `EDITOR="code --wait"`. +fn resolve_editor() -> String { + env::var("VISUAL") + .ok() + .filter(|s| !s.trim().is_empty()) + .or_else(|| env::var("EDITOR").ok().filter(|s| !s.trim().is_empty())) + .unwrap_or_else(|| "vi".to_string()) +} + +#[cfg(unix)] +fn split_command(raw: &str) -> Option> { + shlex::split(raw) +} + +#[cfg(not(unix))] +fn split_command(raw: &str) -> Option> { + // On Windows we do not support shell-quoted editor commands; treat the + // full string as the program name. + if raw.trim().is_empty() { + None + } else { + Some(vec![raw.to_string()]) + } +} + +/// Run the external editor without touching terminal state. Exposed for tests. +/// +/// Returns: +/// - `Ok(EditorOutcome::Edited(new))` if the editor exited cleanly and the +/// contents differ from `seed`. +/// - `Ok(EditorOutcome::Unchanged)` if the editor exited cleanly but the +/// contents match `seed`. +/// - `Ok(EditorOutcome::Cancelled)` if the editor exited non-zero or could not +/// be spawned. +/// +/// The temp file is removed on every path because [`tempfile::NamedTempFile`] +/// is dropped at the end of the function. +pub fn run_editor_raw(seed: &str) -> io::Result { + let mut tmp = Builder::new() + .prefix("deepseek-edit-") + .suffix(".md") + .tempfile()?; + tmp.write_all(seed.as_bytes())?; + tmp.flush()?; + let path = tmp.path().to_path_buf(); + + let raw = resolve_editor(); + let parts = match split_command(&raw) { + Some(p) if !p.is_empty() => p, + _ => return Ok(EditorOutcome::Cancelled), + }; + + let mut cmd = Command::new(&parts[0]); + if parts.len() > 1 { + cmd.args(&parts[1..]); + } + cmd.arg(&path); + + let status = match cmd.status() { + Ok(s) => s, + Err(_) => return Ok(EditorOutcome::Cancelled), + }; + if !status.success() { + return Ok(EditorOutcome::Cancelled); + } + + let new = fs::read_to_string(&path)?; + // tmp goes out of scope here — file is unlinked. + if new == seed { + Ok(EditorOutcome::Unchanged) + } else { + Ok(EditorOutcome::Edited(new)) + } +} + +/// Suspend the TUI, run the external editor on `current`, then re-enter the +/// TUI. Returns the new composer text iff the user saved changes. +/// +/// On any error (raw-mode toggle, IO, editor spawn failure), the function +/// still attempts to fully restore the terminal before returning. +pub fn spawn_editor_for_input( + terminal: &mut Terminal>, + use_alt_screen: bool, + use_mouse_capture: bool, + use_bracketed_paste: bool, + current: &str, +) -> io::Result { + // 1. Suspend. + let _ = disable_raw_mode(); + if use_bracketed_paste { + let _ = execute!(terminal.backend_mut(), DisableBracketedPaste); + } + if use_mouse_capture { + let _ = execute!(terminal.backend_mut(), DisableMouseCapture); + } + if use_alt_screen { + let _ = execute!(terminal.backend_mut(), LeaveAlternateScreen); + } + + // 2. Run the editor (synchronous; inherits stdio). + let result = run_editor_raw(current); + + // 3. Resume — best-effort restoration regardless of `result`. + if use_alt_screen { + let _ = execute!(terminal.backend_mut(), EnterAlternateScreen); + } + if use_mouse_capture { + let _ = execute!(terminal.backend_mut(), EnableMouseCapture); + } + if use_bracketed_paste { + let _ = execute!(terminal.backend_mut(), EnableBracketedPaste); + } + let _ = enable_raw_mode(); + // Force a full repaint so a SIGWINCH during the edit doesn't leave the + // viewport stale. + let _ = terminal.clear(); + + result +} + +#[cfg(test)] +mod tests { + use super::*; + use std::ffi::OsString; + use std::sync::Mutex; + + /// Serialize tests that mutate process-global env vars. + static ENV_LOCK: Mutex<()> = Mutex::new(()); + + struct EnvGuard { + keys: Vec<(&'static str, Option)>, + } + impl EnvGuard { + fn new(keys: &[&'static str]) -> Self { + let saved: Vec<_> = keys.iter().map(|k| (*k, env::var_os(k))).collect(); + Self { keys: saved } + } + } + impl Drop for EnvGuard { + fn drop(&mut self) { + for (k, v) in &self.keys { + match v { + Some(val) => unsafe { env::set_var(k, val) }, + None => unsafe { env::remove_var(k) }, + } + } + } + } + + #[test] + fn resolve_editor_prefers_visual_over_editor() { + let _lock = ENV_LOCK.lock().unwrap(); + let _g = EnvGuard::new(&["VISUAL", "EDITOR"]); + unsafe { + env::set_var("VISUAL", "vis-cmd"); + env::set_var("EDITOR", "ed-cmd"); + } + assert_eq!(resolve_editor(), "vis-cmd"); + } + + #[test] + fn resolve_editor_falls_back_to_vi() { + let _lock = ENV_LOCK.lock().unwrap(); + let _g = EnvGuard::new(&["VISUAL", "EDITOR"]); + unsafe { + env::remove_var("VISUAL"); + env::remove_var("EDITOR"); + } + assert_eq!(resolve_editor(), "vi"); + } + + /// Editor that immediately exits 0 without touching the file ⇒ Unchanged. + #[test] + #[cfg(unix)] + fn run_editor_unchanged_when_editor_is_noop() { + let _lock = ENV_LOCK.lock().unwrap(); + let _g = EnvGuard::new(&["VISUAL", "EDITOR"]); + unsafe { + env::remove_var("VISUAL"); + env::set_var("EDITOR", "true"); + } + let out = run_editor_raw("seed text").expect("editor ok"); + assert_eq!(out, EditorOutcome::Unchanged); + } + + /// Editor that exits non-zero ⇒ Cancelled. + #[test] + #[cfg(unix)] + fn run_editor_cancelled_on_nonzero_exit() { + let _lock = ENV_LOCK.lock().unwrap(); + let _g = EnvGuard::new(&["VISUAL", "EDITOR"]); + unsafe { + env::remove_var("VISUAL"); + env::set_var("EDITOR", "false"); + } + let out = run_editor_raw("seed").expect("call ok"); + assert_eq!(out, EditorOutcome::Cancelled); + } + + /// Spawning an editor binary that doesn't exist ⇒ Cancelled (graceful). + #[test] + #[cfg(unix)] + fn run_editor_cancelled_when_editor_missing() { + let _lock = ENV_LOCK.lock().unwrap(); + let _g = EnvGuard::new(&["VISUAL", "EDITOR"]); + unsafe { + env::remove_var("VISUAL"); + env::set_var("EDITOR", "/nonexistent/deepseek-tui-test-editor"); + } + let out = run_editor_raw("seed").expect("call ok"); + assert_eq!(out, EditorOutcome::Cancelled); + } + + /// Editor that rewrites the file ⇒ Edited(new). + #[test] + #[cfg(unix)] + fn run_editor_returns_edited_contents() { + use std::os::unix::fs::PermissionsExt; + + let _lock = ENV_LOCK.lock().unwrap(); + let _g = EnvGuard::new(&["VISUAL", "EDITOR"]); + let dir = tempfile::tempdir().unwrap(); + let script = dir.path().join("ed.sh"); + fs::write(&script, "#!/bin/sh\nprintf 'edited body' > \"$1\"\n").unwrap(); + let mut perms = fs::metadata(&script).unwrap().permissions(); + perms.set_mode(0o755); + fs::set_permissions(&script, perms).unwrap(); + + unsafe { + env::remove_var("VISUAL"); + env::set_var("EDITOR", script.to_string_lossy().to_string()); + } + let out = run_editor_raw("seed body").expect("editor ok"); + assert_eq!(out, EditorOutcome::Edited("edited body".to_string())); + } + + /// Verify that the temp file is unlinked after `run_editor_raw` returns, + /// regardless of outcome. We test the success path with a script that + /// echoes the file path to a side channel before exiting. + #[test] + #[cfg(unix)] + fn run_editor_cleans_up_temp_file() { + use std::os::unix::fs::PermissionsExt; + + let _lock = ENV_LOCK.lock().unwrap(); + let _g = EnvGuard::new(&["VISUAL", "EDITOR"]); + let dir = tempfile::tempdir().unwrap(); + let path_capture = dir.path().join("capture.txt"); + let script = dir.path().join("ed.sh"); + fs::write( + &script, + format!( + "#!/bin/sh\nprintf '%s' \"$1\" > \"{}\"\nprintf 'x' > \"$1\"\n", + path_capture.display() + ), + ) + .unwrap(); + let mut perms = fs::metadata(&script).unwrap().permissions(); + perms.set_mode(0o755); + fs::set_permissions(&script, perms).unwrap(); + + unsafe { + env::remove_var("VISUAL"); + env::set_var("EDITOR", script.to_string_lossy().to_string()); + } + let _ = run_editor_raw("seed").expect("editor ok"); + + let captured = fs::read_to_string(&path_capture).expect("captured path"); + assert!(!captured.is_empty(), "editor should have received a path"); + assert!( + !std::path::Path::new(&captured).exists(), + "temp file {captured:?} should be cleaned up after run_editor_raw returns" + ); + } +} diff --git a/crates/tui/src/tui/mod.rs b/crates/tui/src/tui/mod.rs index ec15ab84..0d617674 100644 --- a/crates/tui/src/tui/mod.rs +++ b/crates/tui/src/tui/mod.rs @@ -9,6 +9,7 @@ pub mod clipboard; pub mod command_palette; pub mod diff_render; pub mod event_broker; +pub mod external_editor; pub mod file_mention; pub mod history; pub mod markdown_render; diff --git a/crates/tui/src/tui/ui.rs b/crates/tui/src/tui/ui.rs index bd8a79af..7edd0abd 100644 --- a/crates/tui/src/tui/ui.rs +++ b/crates/tui/src/tui/ui.rs @@ -1534,11 +1534,50 @@ async fn run_event_loop( { app.move_cursor_start(); } - KeyCode::End | KeyCode::Char('e') - if key.modifiers.contains(KeyModifiers::CONTROL) => - { + KeyCode::End => { app.move_cursor_end(); } + KeyCode::Char('e') if key.modifiers.contains(KeyModifiers::CONTROL) => { + // Ctrl+E: spawn $EDITOR on the composer contents (#91). + // Only fires when no modal is active (the !view_stack + // branch above already returns early in that case) and + // the composer is the focused input target. We accept the + // shortcut whether or not a model turn is streaming — + // editing the buffer never disturbs in-flight work. + let seed = app.input.clone(); + match super::external_editor::spawn_editor_for_input( + terminal, + app.use_alt_screen, + app.use_mouse_capture, + app.use_bracketed_paste, + &seed, + ) { + Ok(super::external_editor::EditorOutcome::Edited(new)) => { + app.input = new; + app.move_cursor_end(); + let editor = std::env::var("VISUAL") + .ok() + .filter(|s| !s.trim().is_empty()) + .or_else(|| { + std::env::var("EDITOR") + .ok() + .filter(|s| !s.trim().is_empty()) + }) + .unwrap_or_else(|| "vi".to_string()); + app.status_message = Some(format!("Edited in {editor}")); + } + Ok(super::external_editor::EditorOutcome::Unchanged) => { + app.status_message = Some("Editor closed (no changes)".to_string()); + } + Ok(super::external_editor::EditorOutcome::Cancelled) => { + app.status_message = Some("Editor cancelled".to_string()); + } + Err(err) => { + app.status_message = Some(format!("Editor error: {err}")); + } + } + app.needs_redraw = true; + } KeyCode::Up => { if key.modifiers.contains(KeyModifiers::CONTROL) { app.history_up();