Subcmd exec and signal handling

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"

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<SyntaxTheme>,
     pub tab_cfg: utils::tabs::TabCfg,
@@ -424,6 +425,7 @@ impl From<cli::Opt> 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,

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::<Vec<_>>(), 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::<Vec<_>>(),
         );
     }
@@ -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

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,29 +45,34 @@ 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()]
             },
         ),
     };
 
     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::<Vec<_>>();
-    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))
 }