diff --git a/Cargo.lock b/Cargo.lock index 1565342cc..02c02cb5b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -619,6 +619,7 @@ dependencies = [ "insta", "itertools", "lazy_static", + "libc", "palette", "pathdiff", "pretty_assertions", diff --git a/Cargo.toml b/Cargo.toml index 6401aaf21..9ea18df70 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -43,6 +43,7 @@ git2 = { version = "0.18.2", default-features = false, features = [] } grep-cli = "0.1.8" itertools = "0.10.5" lazy_static = "1.4" +libc = "*" palette = "0.7.2" pathdiff = "0.2.1" regex = "1.7.1" diff --git a/src/config.rs b/src/config.rs index a536d7e42..782a04f7b 100644 --- a/src/config.rs +++ b/src/config.rs @@ -128,6 +128,7 @@ pub struct Config { pub show_themes: bool, pub side_by_side_data: side_by_side::SideBySideData, pub side_by_side: bool, + pub stdout_is_term: bool, pub syntax_set: SyntaxSet, pub syntax_theme: Option, pub tab_cfg: utils::tabs::TabCfg, @@ -424,6 +425,7 @@ impl From for Config { relative_paths: opt.relative_paths, show_themes: opt.show_themes, side_by_side: opt.side_by_side && !handlers::hunk::is_word_diff(), + stdout_is_term: opt.computed.stdout_is_term, side_by_side_data, styles_map, syntax_set: opt.computed.syntax_set, diff --git a/src/main.rs b/src/main.rs index e952a3bcb..9531883b0 100644 --- a/src/main.rs +++ b/src/main.rs @@ -70,6 +70,7 @@ fn main() -> std::io::Result<()> { .unwrap_or_else(|err| eprintln!("Failed to set ctrl-c handler: {err}")); let exit_code = run_app(std::env::args_os().collect::>(), None)?; // when you call process::exit, no drop impls are called, so we want to do it only once, here + // (exception: a subcommand is exec'ed directly) process::exit(exit_code); } @@ -97,7 +98,7 @@ pub fn run_app( utils::process::set_calling_process( &cmd.args .iter() - .map(|arg| OsStr::to_string_lossy(arg).to_string()) + .map(|arg| OsStr::to_string_lossy(arg.as_ref()).to_string()) .collect::>(), ); } @@ -136,16 +137,35 @@ pub fn run_app( return Ok(0); }; - let _show_config = opt.show_config; + let show_config = opt.show_config; let config = config::Config::from(opt); - if _show_config { + if show_config { let stdout = io::stdout(); let mut stdout = stdout.lock(); subcommands::show_config::show_config(&config, &mut stdout)?; return Ok(0); } + let subcmd = match call { + Call::DeltaDiff(_, minus, plus) => { + match subcommands::diff::build_diff_cmd(&minus, &plus, &config) { + Err(code) => return Ok(code), + Ok(val) => val, + } + } + Call::SubCommand(_, subcmd) => subcmd, + Call::Delta(_) => SubCommand::none(), + Call::Help(_) | Call::Version(_) => delta_unreachable("help/version handled earlier"), + }; + + // Do the exec before the pager is set up. + // No subprocesses must exist (but other threads may), and no terminal queries must be outstanding. + #[cfg(not(test))] + if !subcmd.is_none() && !config.stdout_is_term { + subcmd.exec(); + } + // The following block structure is because of `writer` and related lifetimes: let pager_cfg = (&config).into(); let paging_mode = if capture_output.is_some() { @@ -161,18 +181,6 @@ pub fn run_app( output_type.handle().unwrap() }; - let subcmd = match call { - Call::DeltaDiff(_, minus, plus) => { - match subcommands::diff::build_diff_cmd(&minus, &plus, &config) { - Err(code) => return Ok(code), - Ok(val) => val, - } - } - Call::SubCommand(_, subcmd) => subcmd, - Call::Delta(_) => SubCommand::none(), - Call::Help(_) | Call::Version(_) => delta_unreachable("help/version handled earlier"), - }; - if subcmd.is_none() { // Default delta run: read input from stdin, write to stdout or pager (pager started already^). @@ -189,12 +197,14 @@ pub fn run_app( let res = delta(io::stdin().lock().byte_lines(), &mut writer, &config); if let Err(error) = res { - match error.kind() { - ErrorKind::BrokenPipe => return Ok(0), - _ => { - eprintln!("{error}"); - return Ok(config.error_exit_code); - } + if error.kind() == ErrorKind::BrokenPipe { + // Not the entire input was processed, so we should not return 0. + // Other unix utils with a default SIGPIPE handler would have their exit code + // set to this code by the shell, emulate it: + return Ok(128 + libc::SIGPIPE); + } else { + eprintln!("{error}"); + return Ok(config.error_exit_code); } } @@ -203,7 +213,9 @@ pub fn run_app( // First start a subcommand, and pipe input from it to delta(). Also handle // subcommand exit code and stderr (maybe truncate it, e.g. for git and diff logic). - let (subcmd_bin, subcmd_args) = subcmd.args.split_first().unwrap(); + let subcmd_args: Vec<&OsStr> = subcmd.args.iter().map(|arg| arg.as_ref()).collect(); + + let (subcmd_bin, subcmd_args) = subcmd_args.split_first().unwrap(); let subcmd_kind = subcmd.kind; // for easier {} formatting let subcmd_bin_path = match grep_cli::resolve_binary(std::path::PathBuf::from(subcmd_bin)) { @@ -236,25 +248,45 @@ pub fn run_app( if let Err(error) = res { let _ = cmd.wait(); // for clippy::zombie_processes - match error.kind() { - ErrorKind::BrokenPipe => return Ok(0), - _ => { - eprintln!("{error}"); - return Ok(config.error_exit_code); - } + if error.kind() == ErrorKind::BrokenPipe { + // (see non-subcommand block above) + return Ok(128 + libc::SIGPIPE); + } else { + eprintln!("{error}"); + return Ok(config.error_exit_code); } }; - let subcmd_status = cmd - .wait() - .unwrap_or_else(|_| { - delta_unreachable(&format!("{subcmd_kind:?} process not running.")); - }) - .code() - .unwrap_or_else(|| { - eprintln!("delta: {subcmd_kind:?} process terminated without exit status."); - config.error_exit_code - }); + let subcmd_status = cmd.wait().unwrap_or_else(|_| { + delta_unreachable(&format!("{subcmd_kind} process not running.")); + }); + + let subcmd_code = subcmd_status.code().unwrap_or_else(|| { + #[cfg(unix)] + { + // On unix no exit status usually means the process was terminated + // by a signal (not using MT-unsafe `libc::strsignal` to get its name). + use std::os::unix::process::ExitStatusExt; + + if let Some(signal) = subcmd_status.signal() { + // (note that by default the rust runtime blocks SIGPIPE) + if signal != libc::SIGPIPE { + eprintln!( + "delta: {subcmd_kind:?} received signal {signal}{}", + if subcmd_status.core_dumped() { + " (core dumped)" + } else { + "" + } + ); + } + // unix convention: 128 + signal + return 128 + signal; + } + } + eprintln!("delta: {subcmd_kind:?} process terminated without exit status."); + config.error_exit_code + }); let mut stderr_lines = io::BufReader::new( cmd.stderr @@ -272,8 +304,7 @@ pub fn run_app( // On `git diff` unknown option error: stop after printing the first line above (which is // an error message), because the entire --help text follows. - if !(subcmd_status == 129 - && matches!(subcmd_kind, SubCmdKind::GitDiff | SubCmdKind::Git(_))) + if !(subcmd_code == 129 && matches!(subcmd_kind, SubCmdKind::GitDiff | SubCmdKind::Git(_))) { for line in stderr_lines { eprintln!( @@ -283,22 +314,22 @@ pub fn run_app( } } - if matches!(subcmd_kind, SubCmdKind::GitDiff | SubCmdKind::Diff) && subcmd_status >= 2 { + if matches!(subcmd_kind, SubCmdKind::GitDiff | SubCmdKind::Diff) && subcmd_code >= 2 { eprintln!( - "{subcmd_kind:?} process failed with exit status {subcmd_status}. Command was: {}", + "delta: {subcmd_kind:?} process failed with exit status {subcmd_code}. Command was: {}", format_args!( "{} {}", subcmd_bin_path.display(), shell_words::join( subcmd_args .iter() - .map(|arg0: &OsString| std::ffi::OsStr::to_string_lossy(arg0)) + .map(|arg0: &&OsStr| OsStr::to_string_lossy(arg0)) ), ) ); } - Ok(subcmd_status) + Ok(subcmd_code) } // `output_type` drop impl runs here diff --git a/src/subcommands/diff.rs b/src/subcommands/diff.rs index 3346d9fe6..c240618ee 100644 --- a/src/subcommands/diff.rs +++ b/src/subcommands/diff.rs @@ -1,11 +1,9 @@ use std::path::Path; use crate::config::{self}; - -use crate::utils::git::retrieve_git_version; - +use crate::subcommands::external::SubCmdArg; use crate::subcommands::{SubCmdKind, SubCommand}; -use std::ffi::OsString; +use crate::utils::git::retrieve_git_version; /// Build `git diff` command for the files provided on the command line. Fall back to /// `diff` if the supplied "files" use process substitution. @@ -31,7 +29,7 @@ pub fn build_diff_cmd( .map(|arg| !arg.is_empty() && !arg.starts_with('-')) .unwrap_or(false) { - diff_args[0] = format!("-{}", diff_args[0]) + diff_args[0] = format!("-{}", diff_args[0]); } let via_process_substitution = @@ -47,15 +45,20 @@ pub fn build_diff_cmd( { ( SubCmdKind::GitDiff, - vec!["git", "diff", "--no-index", "--color"], + vec![ + "git".into(), + "diff".into(), + "--no-index".into(), + SubCmdArg::Added("--color".into()), + ], ) } _ => ( SubCmdKind::Diff, if diff_args_set_unified_context(&diff_args) { - vec!["diff"] + vec!["diff".into()] } else { - vec!["diff", "-U3"] + vec!["diff".into(), "-U3".into()] }, ), }; @@ -63,13 +66,13 @@ pub fn build_diff_cmd( diff_cmd.extend( diff_args .iter() - .filter(|s| !s.is_empty()) - .map(String::as_str), + .filter(|&s| !s.is_empty()) + .map(|s| s.as_str().into()), ); - diff_cmd.push("--"); - let mut diff_cmd = diff_cmd.iter().map(OsString::from).collect::>(); - diff_cmd.push(minus_file.into()); - diff_cmd.push(plus_file.into()); + diff_cmd.push("--".into()); + + diff_cmd.push(minus_file.as_os_str().into()); + diff_cmd.push(plus_file.as_os_str().into()); Ok(SubCommand::new(differ, diff_cmd)) } diff --git a/src/subcommands/external.rs b/src/subcommands/external.rs index dc98a20fb..b69d40b40 100644 --- a/src/subcommands/external.rs +++ b/src/subcommands/external.rs @@ -1,7 +1,10 @@ -use crate::cli::Opt; +use std::ffi::{OsStr, OsString}; +use std::process::{self, Command}; + use clap::CommandFactory; use clap::{ArgMatches, Error}; -use std::ffi::{OsStr, OsString}; + +use crate::cli::Opt; const RG: &str = "rg"; const GIT: &str = "git"; @@ -37,7 +40,7 @@ impl std::fmt::Debug for SubCmdKind { SubCmdKind::Git(Some(arg)) => { return formatter.write_fmt(format_args!("\"git {}\"", arg.escape_debug())) } - _ => format!("{}", self), + _ => format!("{self}"), }; formatter.write_str("\"")?; formatter.write_str(&s)?; @@ -45,14 +48,52 @@ impl std::fmt::Debug for SubCmdKind { } } +/// `SubCommand` call arguments, where an `Added()` argument was added by us for better +/// delta compatibility. They are only used when the subcommand output is directly +/// fed into `delta()`. If a subcommand is exec'ed these are omitted. +#[derive(Debug)] +pub enum SubCmdArg { + Original(OsString), + Added(OsString), +} + +impl SubCmdArg { + fn original(&self) -> Option<&OsStr> { + match self { + Self::Original(arg) => Some(arg.as_ref()), + Self::Added(_) => None, + } + } +} + +impl AsRef for SubCmdArg { + fn as_ref(&self) -> &OsStr { + match self { + Self::Original(arg) | Self::Added(arg) => arg.as_ref(), + } + } +} + +impl From<&OsStr> for SubCmdArg { + fn from(value: &OsStr) -> Self { + Self::Original(value.to_owned()) + } +} + +impl From<&str> for SubCmdArg { + fn from(value: &str) -> Self { + Self::Original(OsString::from(value)) + } +} + #[derive(Debug)] pub struct SubCommand { pub kind: SubCmdKind, - pub args: Vec, + pub args: Vec, } impl SubCommand { - pub fn new(kind: SubCmdKind, args: Vec) -> Self { + pub fn new(kind: SubCmdKind, args: Vec) -> Self { Self { kind, args } } @@ -66,12 +107,64 @@ impl SubCommand { pub fn is_none(&self) -> bool { matches!(self.kind, SubCmdKind::None) } + + pub fn exec(self) -> ! { + debug_assert!(!self.is_none()); + + fn exec_cmd(subcmd_args: Vec<&OsStr>) { + let (subcmd_bin, subcmd_args) = subcmd_args.split_first().unwrap(); + + let subcmd_bin_path = + match grep_cli::resolve_binary(std::path::PathBuf::from(subcmd_bin)) { + Ok(path) => path, + Err(err) => { + eprintln!("Failed to resolve command {subcmd_bin:?}: {err}"); + return; + } + }; + + let mut cmd = Command::new(subcmd_bin_path); + cmd.args(subcmd_args.iter()); + + #[cfg(unix)] + { + use std::os::unix::process::CommandExt; + let err = cmd.exec(); + eprintln!("Failed to start {subcmd_bin:?}: {err}"); + } + #[cfg(windows)] + { + // there is no `exec` on windows, so emulate it: + match cmd.spawn() { + Err(err) => { + eprintln!("Failed to start {subcmd_bin:?}: {err}"); + } + Ok(mut child) => match child.wait() { + Ok(result) if result.code().is_some() => { + process::exit(result.code().unwrap()); + } + Err(err) => { + eprintln!("Failed to wait for {subcmd_bin:?}: {err}"); + } + _ => {} + }, + } + } + } + + // Not using `config.error_exit_code` for better shell compatibility + const PROCESS_CREATION_ERROR: i32 = 127; + + let subcmd_args: Vec<&OsStr> = self.args.iter().filter_map(|arg| arg.original()).collect(); + exec_cmd(subcmd_args); + process::exit(PROCESS_CREATION_ERROR); + } } /// Find the first arg that is a registered external subcommand and return a /// tuple containing: /// - The args prior to that point (delta can understand these) -/// - A SubCommand representing the external subcommand and its subsequent args +/// - A `SubCommand` representing the external subcommand and its subsequent args pub fn extract(args: &[OsString], orig_error: Error) -> (ArgMatches, SubCommand) { for (subcmd_pos, arg) in args.iter().filter_map(|a| a.to_str()).enumerate() { if SUBCOMMANDS.contains(&arg) { @@ -84,14 +177,18 @@ pub fn extract(args: &[OsString], orig_error: Error) -> (ArgMatches, SubCommand) } Ok(matches) => { let (subcmd_args_index, kind, subcmd) = if arg == RG { - (subcmd_pos + 1, SubCmdKind::Rg, vec![RG, "--json"]) + ( + subcmd_pos + 1, + SubCmdKind::Rg, + vec![RG.into(), SubCmdArg::Added("--json".into())], + ) } else if arg == GIT { let subcmd_args_index = subcmd_pos + 1; let git_subcmd = args .get(subcmd_args_index) .and_then(|cmd| OsStr::to_str(cmd)) .and_then(|cmd| { - if cmd.starts_with("-") { + if cmd.starts_with('-') { None } else { Some(cmd.into()) @@ -102,7 +199,11 @@ pub fn extract(args: &[OsString], orig_error: Error) -> (ArgMatches, SubCommand) SubCmdKind::Git(git_subcmd), // git does not start the pager and sees that it does not write to a // terminal, so by default it will not use colors. Override it: - vec![GIT, "-c", "color.ui=always"], + vec![ + GIT.into(), + SubCmdArg::Added("-c".into()), + SubCmdArg::Added("color.ui=always".into()), + ], ) } else { unreachable!("arg must be in SUBCOMMANDS"); @@ -110,8 +211,11 @@ pub fn extract(args: &[OsString], orig_error: Error) -> (ArgMatches, SubCommand) let subcmd = subcmd .into_iter() - .map(OsString::from) - .chain(args[subcmd_args_index..].iter().map(OsString::from)) + .chain( + args[subcmd_args_index..] + .iter() + .map(|s| s.as_os_str().into()), + ) .collect(); return (matches, SubCommand::new(kind, subcmd)); @@ -140,6 +244,8 @@ pub fn extract(args: &[OsString], orig_error: Error) -> (ArgMatches, SubCommand) mod test { use super::RG; use crate::ansi::strip_ansi_codes; + use crate::subcommands::{SubCmdKind, SubCommand}; + use crate::tests::integration_test_utils::make_config_from_args; use std::ffi::OsString; use std::io::Cursor; @@ -160,6 +266,20 @@ mod test { } } + #[test] + #[ignore] // to compile .exec() in test cfg. Never useful to run, so always return early. + fn leave_the_test_suite_via_exec() { + let c = make_config_from_args(&[]); + let bye = SubCommand::new( + SubCmdKind::Rg, + [format!("{:?}", c.stdout_is_term).as_str().into()].into(), + ); + if !bye.is_none() { + return; + } + bye.exec(); + } + #[test] #[should_panic(expected = "unexpected delta argument")] fn just_delta_argument_error() {