travelagent 1.10.3

Agent-first TUI code review tool
//! External editor integration for comment editing.
//!
//! When the user presses Ctrl+E (or runs `:edit`) in Comment mode, the current
//! comment buffer is written to a temp file and opened in `$VISUAL`/`$EDITOR`
//! (fallback: `vi`). After the editor exits, the buffer is re-read and used to
//! replace the in-memory comment buffer.
//!
//! The terminal must be suspended (raw mode off, alternate screen left) before
//! the editor is spawned; that is the main loop's responsibility. This module
//! only handles resolution, tempfile plumbing, and child-process execution.

use std::io::Write;
use std::process::Command;

use anyhow::{Context, Result, anyhow};

/// Resolve the editor command to run, returning `(binary, args)`.
///
/// Resolution order:
/// 1. `$VISUAL` (if set and non-empty)
/// 2. `$EDITOR` (if set and non-empty)
/// 3. Fallback to `vi`
///
/// The editor value is split with `shlex` to support commands with arguments
/// (e.g., `nvim --clean`, `code --wait`). If shell splitting fails, the raw
/// value is treated as a single binary path.
///
/// The `get_var` closure is injected so this function can be unit-tested
/// without mutating process environment variables.
pub fn resolve_editor(get_var: &dyn Fn(&str) -> Option<String>) -> (String, Vec<String>) {
    let raw = get_var("VISUAL")
        .filter(|s| !s.trim().is_empty())
        .or_else(|| get_var("EDITOR").filter(|s| !s.trim().is_empty()))
        .unwrap_or_else(|| "vi".to_string());

    // shlex::split returns None on unbalanced quotes; fall back to a single
    // token in that case so we still invoke *something*.
    let parts = shlex::split(&raw).unwrap_or_else(|| vec![raw.clone()]);
    let mut iter = parts.into_iter();
    let command = iter.next().unwrap_or_else(|| "vi".to_string());
    let args: Vec<String> = iter.collect();
    (command, args)
}

/// Write `initial` to a temp file, invoke the user's editor on it, and return
/// the file's contents after the editor exits.
///
/// Caller responsibilities (i.e., `main.rs`):
///   * Disable raw mode, leave alternate screen, and show the cursor *before*
///     calling this function.
///   * Re-enter alternate screen, re-enable raw mode, and hide the cursor
///     *after* this function returns.
pub fn edit_text(initial: &str) -> Result<String> {
    let mut temp = tempfile::Builder::new()
        .prefix("trv-comment-")
        .suffix(".md")
        .tempfile()
        .context("failed to create temp file for external editor")?;

    temp.write_all(initial.as_bytes())
        .context("failed to write initial comment buffer to temp file")?;
    temp.flush()
        .context("failed to flush temp file before spawning editor")?;

    let path = temp.path().to_path_buf();
    let (editor, extra_args) = resolve_editor(&|key| std::env::var(key).ok());

    let status = Command::new(&editor)
        .args(&extra_args)
        .arg(&path)
        .status()
        .with_context(|| format!("failed to spawn editor '{editor}'"))?;

    if !status.success() {
        return Err(anyhow!(
            "editor '{editor}' exited with non-zero status: {status}"
        ));
    }

    let content = std::fs::read_to_string(&path)
        .with_context(|| format!("failed to re-read temp file at {}", path.display()))?;

    // NamedTempFile is dropped here; the temp file is cleaned up automatically.
    Ok(content)
}

#[cfg(test)]
mod tests {
    use super::*;
    use std::collections::HashMap;

    /// Build an env lookup closure from a fixed map.
    fn env_from(pairs: &[(&str, &str)]) -> HashMap<String, String> {
        pairs
            .iter()
            .map(|(k, v)| ((*k).to_string(), (*v).to_string()))
            .collect()
    }

    fn lookup(env: &HashMap<String, String>) -> impl Fn(&str) -> Option<String> + '_ {
        move |key: &str| env.get(key).cloned()
    }

    #[test]
    fn resolves_visual_over_editor() {
        let env = env_from(&[("VISUAL", "nvim"), ("EDITOR", "vim")]);
        let (cmd, args) = resolve_editor(&lookup(&env));
        assert_eq!(cmd, "nvim");
        assert!(args.is_empty());
    }

    #[test]
    fn falls_back_to_editor() {
        let env = env_from(&[("EDITOR", "emacs")]);
        let (cmd, args) = resolve_editor(&lookup(&env));
        assert_eq!(cmd, "emacs");
        assert!(args.is_empty());
    }

    #[test]
    fn falls_back_to_vi_when_neither_set() {
        let env: HashMap<String, String> = HashMap::new();
        let (cmd, args) = resolve_editor(&lookup(&env));
        assert_eq!(cmd, "vi");
        assert!(args.is_empty());
    }

    #[test]
    fn empty_visual_falls_through_to_editor() {
        // Users sometimes have VISUAL="" (or whitespace); treat that as unset.
        let env = env_from(&[("VISUAL", "   "), ("EDITOR", "nano")]);
        let (cmd, args) = resolve_editor(&lookup(&env));
        assert_eq!(cmd, "nano");
        assert!(args.is_empty());
    }

    #[test]
    fn splits_command_with_args() {
        let env = env_from(&[("VISUAL", "nvim --clean -u NONE")]);
        let (cmd, args) = resolve_editor(&lookup(&env));
        assert_eq!(cmd, "nvim");
        assert_eq!(
            args,
            vec!["--clean".to_string(), "-u".to_string(), "NONE".to_string()]
        );
    }

    #[test]
    fn unbalanced_quotes_fall_back_to_raw_value() {
        // `shlex::split` returns None on unbalanced quotes. The documented
        // contract is that we still return *something* — the raw value as a
        // single-token command — rather than crashing or silently dropping
        // back to `vi`. Regression guard for the fallback closure.
        let env = env_from(&[("VISUAL", "nvim \"unclosed")]);
        let (cmd, args) = resolve_editor(&lookup(&env));
        assert_eq!(cmd, "nvim \"unclosed");
        assert!(args.is_empty());
    }

    #[test]
    fn edit_text_round_trips_content_through_true_command() {
        // Use `true` (a POSIX no-op command) as the editor: it exits 0 without
        // touching the file, so we should read back exactly what we wrote.
        // SAFETY: set_var is unsafe in edition 2024 because env is a global
        // resource. This is a single-threaded test and no other tests mutate
        // VISUAL / EDITOR concurrently (the unit tests above do not invoke
        // edit_text). We restore the originals afterwards for good measure.
        let old_visual = std::env::var("VISUAL").ok();
        let old_editor = std::env::var("EDITOR").ok();
        unsafe {
            std::env::set_var("VISUAL", "true");
            std::env::remove_var("EDITOR");
        }

        let result = edit_text("hello from trv\n");

        unsafe {
            match old_visual {
                Some(v) => std::env::set_var("VISUAL", v),
                None => std::env::remove_var("VISUAL"),
            }
            match old_editor {
                Some(v) => std::env::set_var("EDITOR", v),
                None => std::env::remove_var("EDITOR"),
            }
        }

        let content = result.expect("edit_text should succeed when editor exits 0");
        assert_eq!(content, "hello from trv\n");
    }
}