diff --git a/CHANGELOG.md b/CHANGELOG.md index 46ad30585e..49c6a61077 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ - Fix panel width when line 10000 wraps, see #2854 (@eth-p) - Fix compile issue of `time` dependency caused by standard library regression #3045 (@cyqsimon) - Fix override behavior of --plain and --paging, see issue #2731 and PR #3108 (@einfachIrgendwer0815) +- Fix bugs in `$LESSOPEN` support, see #2805 (@Anomalocaridid) ## Other diff --git a/Cargo.lock b/Cargo.lock index ce266b29a6..7a1ba1d1aa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -124,6 +124,7 @@ dependencies = [ "content_inspector", "encoding_rs", "etcetera", + "execute", "expect-test", "flate2", "git2", @@ -135,12 +136,10 @@ dependencies = [ "nix", "nu-ansi-term", "once_cell", - "os_str_bytes", "path_abs", "plist", "predicates", "regex", - "run_script", "semver", "serde", "serde_derive", @@ -401,12 +400,6 @@ version = "0.3.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "fea41bba32d969b513997752735605054bc0dfa92b4c56bf1189f2e174be7a10" -[[package]] -name = "dunce" -version = "1.0.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0bd4b30a6560bbd9b4620f4de34c3f14f60848e58a9b7216801afcb4c7b31c3c" - [[package]] name = "either" version = "1.8.0" @@ -466,6 +459,43 @@ dependencies = [ "windows-sys 0.48.0", ] +[[package]] +name = "execute" +version = "0.2.13" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3a82608ee96ce76aeab659e9b8d3c2b787bffd223199af88c674923d861ada10" +dependencies = [ + "execute-command-macro", + "execute-command-tokens", + "generic-array", +] + +[[package]] +name = "execute-command-macro" +version = "0.1.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "90dec53d547564e911dc4ff3ecb726a64cf41a6fa01a2370ebc0d95175dd08bd" +dependencies = [ + "execute-command-macro-impl", +] + +[[package]] +name = "execute-command-macro-impl" +version = "0.1.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ce8cd46a041ad005ab9c71263f9a0ff5b529eac0fe4cc9b4a20f4f0765d8cf4b" +dependencies = [ + "execute-command-tokens", + "quote", + "syn", +] + +[[package]] +name = "execute-command-tokens" +version = "0.1.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "69dc321eb6be977f44674620ca3aa21703cb20ffbe560e1ae97da08401ffbcad" + [[package]] name = "expect-test" version = "1.5.0" @@ -527,24 +557,12 @@ dependencies = [ ] [[package]] -name = "fsio" -version = "0.4.0" +name = "generic-array" +version = "1.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dad0ce30be0cc441b325c5d705c8b613a0ca0d92b6a8953d41bd236dc09a36d0" +checksum = "2cb8bc4c28d15ade99c7e90b219f30da4be5c88e586277e8cbe886beeb868ab2" dependencies = [ - "dunce", - "rand", -] - -[[package]] -name = "getrandom" -version = "0.2.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4eb1a864a501629691edf6c15a593b7a51eebaa1e8468e9ddc623de7c9b58ec6" -dependencies = [ - "cfg-if", - "libc", - "wasi", + "typenum", ] [[package]] @@ -858,15 +876,6 @@ dependencies = [ "pkg-config", ] -[[package]] -name = "os_str_bytes" -version = "7.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7ac44c994af577c799b1b4bd80dc214701e349873ad894d6cdf96f4f7526e0b9" -dependencies = [ - "memchr", -] - [[package]] name = "parking_lot" version = "0.12.1" @@ -930,12 +939,6 @@ version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "439ee305def115ba05938db6eb1644ff94165c5ab5e9420d1c1bcedbba909391" -[[package]] -name = "ppv-lite86" -version = "0.2.17" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5b40af805b3121feab8a3c29f04d8ad262fa8e0561883e7653e024ae4479e6de" - [[package]] name = "predicates" version = "3.1.3" @@ -993,36 +996,6 @@ dependencies = [ "proc-macro2", ] -[[package]] -name = "rand" -version = "0.8.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "34af8d1a0e25924bc5b7c43c079c942339d8f0a8b57c39049bef581b46327404" -dependencies = [ - "libc", - "rand_chacha", - "rand_core", -] - -[[package]] -name = "rand_chacha" -version = "0.3.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e6c10a63a0fa32252be49d21e7709d4d4baf8d231c2dbce1eaa8141b9b127d88" -dependencies = [ - "ppv-lite86", - "rand_core", -] - -[[package]] -name = "rand_core" -version = "0.6.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ec0be4795e2f6a28069bec0b5ff3e2ac9bafc99e6a9a7dc3547996c5c816922c" -dependencies = [ - "getrandom", -] - [[package]] name = "redox_syscall" version = "0.2.16" @@ -1079,15 +1052,6 @@ dependencies = [ "bytemuck", ] -[[package]] -name = "run_script" -version = "0.10.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "829f98fdc58d78989dd9af83be28bc15c94a7d77f9ecdb54abbbc0b1829ba9c7" -dependencies = [ - "fsio", -] - [[package]] name = "rustix" version = "0.38.21" @@ -1471,6 +1435,12 @@ dependencies = [ "winnow", ] +[[package]] +name = "typenum" +version = "1.17.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "42ff0bf0c66b8238c6f3b578df37d0b7848e55df8577b3f74f92a69acceeb825" + [[package]] name = "unicode-bidi" version = "0.3.18" diff --git a/Cargo.toml b/Cargo.toml index 3b789bdde1..f81e085807 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -33,7 +33,7 @@ minimal-application = [ ] git = ["git2"] # Support indicating git modifications paging = ["shell-words", "grep-cli"] # Support applying a pager on the output -lessopen = ["run_script", "os_str_bytes/conversions"] # Support $LESSOPEN preprocessor +lessopen = ["execute"] # Support $LESSOPEN preprocessor build-assets = ["syntect/yaml-load", "syntect/plist-load", "regex", "walkdir"] # You need to use one of these if you depend on bat as a library: @@ -66,8 +66,7 @@ regex = { version = "1.10.6", optional = true } walkdir = { version = "2.5", optional = true } bytesize = { version = "1.3.0" } encoding_rs = "0.8.35" -os_str_bytes = { version = "~7.0", optional = true } -run_script = { version = "^0.10.1", optional = true} +execute = { version = "0.2.13", optional = true } terminal-colorsaurus = "0.4" [dependencies.git2] diff --git a/src/lessopen.rs b/src/lessopen.rs index 4d7e0eced3..cf004d493e 100644 --- a/src/lessopen.rs +++ b/src/lessopen.rs @@ -3,13 +3,12 @@ use std::convert::TryFrom; use std::env; use std::fs::File; -use std::io::{BufRead, BufReader, Cursor, Read, Write}; +use std::io::{BufRead, BufReader, Cursor, Read}; use std::path::PathBuf; -use std::str; +use std::process::{ExitStatus, Stdio}; use clircle::{Clircle, Identifier}; -use os_str_bytes::RawOsString; -use run_script::{IoOptions, ScriptOptions}; +use execute::{shell, Execute}; use crate::error::Result; use crate::{ @@ -21,7 +20,6 @@ use crate::{ pub(crate) struct LessOpenPreprocessor { lessopen: String, lessclose: Option, - command_options: ScriptOptions, kind: LessOpenKind, /// Whether or not data piped via stdin is to be preprocessed preprocess_stdin: bool, @@ -52,7 +50,7 @@ impl LessOpenPreprocessor { // Otherwise, if output is empty and exit code is nonzero, use original file contents let (kind, lessopen) = if lessopen.starts_with("||") { (LessOpenKind::Piped, lessopen.chars().skip(2).collect()) - // "|" means pipe, but ignore exit code, always using preprocessor output + // "|" means pipe as above, but ignore exit code and always use preprocessor output even if empty } else if lessopen.starts_with('|') { ( LessOpenKind::PipedIgnoreExitCode, @@ -70,16 +68,9 @@ impl LessOpenPreprocessor { (false, lessopen) }; - let mut command_options = ScriptOptions::new(); - command_options.runner = env::var("SHELL").ok(); - command_options.input_redirection = IoOptions::Pipe; - Ok(Self { - lessopen: lessopen.replacen("%s", "$1", 1), - lessclose: env::var("LESSCLOSE") - .ok() - .map(|str| str.replacen("%s", "$1", 1).replacen("%s", "$2", 1)), - command_options, + lessopen, + lessclose: env::var("LESSCLOSE").ok(), kind, preprocess_stdin: stdin, }) @@ -98,21 +89,21 @@ impl LessOpenPreprocessor { None => return input.open(stdin, stdout_identifier), }; - let (exit_code, lessopen_stdout, _) = match run_script::run( - &self.lessopen, - &vec![path_str.to_string()], - &self.command_options, - ) { + let mut lessopen_command = shell(self.lessopen.replacen("%s", path_str, 1)); + lessopen_command.stdout(Stdio::piped()); + + let lessopen_output = match lessopen_command.execute_output() { Ok(output) => output, Err(_) => return input.open(stdin, stdout_identifier), }; - if self.fall_back_to_original_file(&lessopen_stdout, exit_code) { + if self.fall_back_to_original_file(&lessopen_output.stdout, lessopen_output.status) + { return input.open(stdin, stdout_identifier); } ( - RawOsString::new(lessopen_stdout), + lessopen_output.stdout, path_str.to_string(), OpenedInputKind::OrdinaryFile(path.to_path_buf()), ) @@ -127,47 +118,31 @@ impl LessOpenPreprocessor { } } - // stdin isn't Clone, so copy it to a cloneable buffer + // stdin isn't Clone or AsRef<[u8]>, so move it into a cloneable buffer + // so the data can be used multiple times if necessary + // NOTE: stdin will be empty from this point onwards let mut stdin_buffer = Vec::new(); - stdin.read_to_end(&mut stdin_buffer).unwrap(); - - let mut lessopen_handle = match run_script::spawn( - &self.lessopen, - &vec!["-".to_string()], - &self.command_options, - ) { - Ok(handle) => handle, - Err(_) => { - return input.open(stdin, stdout_identifier); - } - }; + stdin.read_to_end(&mut stdin_buffer)?; - if lessopen_handle - .stdin - .as_mut() - .unwrap() - .write_all(&stdin_buffer.clone()) - .is_err() - { - return input.open(stdin, stdout_identifier); - } + let mut lessopen_command = shell(self.lessopen.replacen("%s", "-", 1)); + lessopen_command.stdout(Stdio::piped()); - let lessopen_output = match lessopen_handle.wait_with_output() { + let lessopen_output = match lessopen_command.execute_input_output(&stdin_buffer) + { Ok(output) => output, Err(_) => { return input.open(Cursor::new(stdin_buffer), stdout_identifier); } }; - if lessopen_output.stdout.is_empty() - && (!lessopen_output.status.success() - || matches!(self.kind, LessOpenKind::PipedIgnoreExitCode)) + if self + .fall_back_to_original_file(&lessopen_output.stdout, lessopen_output.status) { return input.open(Cursor::new(stdin_buffer), stdout_identifier); } ( - RawOsString::assert_from_raw_vec(lessopen_output.stdout), + lessopen_output.stdout, "-".to_string(), OpenedInputKind::StdIn, ) @@ -184,14 +159,18 @@ impl LessOpenPreprocessor { kind, reader: InputReader::new(BufReader::new( if matches!(self.kind, LessOpenKind::TempFile) { + let lessopen_string = match String::from_utf8(lessopen_stdout) { + Ok(string) => string, + Err(_) => { + return input.open(stdin, stdout_identifier); + } + }; // Remove newline at end of temporary file path returned by $LESSOPEN - let stdout = match lessopen_stdout.strip_suffix("\n") { + let stdout = match lessopen_string.strip_suffix("\n") { Some(stripped) => stripped.to_owned(), - None => lessopen_stdout, + None => lessopen_string, }; - let stdout = stdout.into_os_string(); - let file = match File::open(PathBuf::from(&stdout)) { Ok(file) => file, Err(_) => { @@ -201,16 +180,18 @@ impl LessOpenPreprocessor { Preprocessed { kind: PreprocessedKind::TempFile(file), - lessclose: self.lessclose.clone(), - command_args: vec![path_str, stdout.to_str().unwrap().to_string()], - command_options: self.command_options.clone(), + lessclose: self + .lessclose + .as_ref() + .map(|s| s.replacen("%s", &path_str, 1).replacen("%s", &stdout, 1)), } } else { Preprocessed { - kind: PreprocessedKind::Piped(Cursor::new(lessopen_stdout.into_raw_vec())), - lessclose: self.lessclose.clone(), - command_args: vec![path_str, "-".to_string()], - command_options: self.command_options.clone(), + kind: PreprocessedKind::Piped(Cursor::new(lessopen_stdout)), + lessclose: self + .lessclose + .as_ref() + .map(|s| s.replacen("%s", &path_str, 1).replacen("%s", "-", 1)), } }, )), @@ -219,9 +200,9 @@ impl LessOpenPreprocessor { }) } - fn fall_back_to_original_file(&self, lessopen_output: &str, exit_code: i32) -> bool { - lessopen_output.is_empty() - && (exit_code != 0 || matches!(self.kind, LessOpenKind::PipedIgnoreExitCode)) + fn fall_back_to_original_file(&self, lessopen_stdout: &Vec, exit_code: ExitStatus) -> bool { + lessopen_stdout.is_empty() + && (!exit_code.success() || matches!(self.kind, LessOpenKind::PipedIgnoreExitCode)) } #[cfg(test)] @@ -261,8 +242,6 @@ impl Read for PreprocessedKind { pub struct Preprocessed { kind: PreprocessedKind, lessclose: Option, - command_args: Vec, - command_options: ScriptOptions, } impl Read for Preprocessed { @@ -273,11 +252,20 @@ impl Read for Preprocessed { impl Drop for Preprocessed { fn drop(&mut self) { - if let Some(ref command) = self.lessclose { - self.command_options.output_redirection = IoOptions::Inherit; + if let Some(lessclose) = self.lessclose.clone() { + let mut lessclose_command = shell(lessclose); + + let lessclose_output = match lessclose_command.execute_output() { + Ok(output) => output, + Err(_) => { + bat_warning!("failed to run $LESSCLOSE to clean up temporary file"); + return; + } + }; - run_script::run(command, &self.command_args, &self.command_options) - .expect("failed to run $LESSCLOSE to clean up file"); + if lessclose_output.status.success() { + bat_warning!("$LESSCLOSE exited with nonzero exit code",) + }; } } } @@ -301,7 +289,7 @@ mod tests { fn test_just_lessopen() -> Result<()> { let preprocessor = LessOpenPreprocessor::mock_new(Some("|batpipe %s"), None)?; - assert_eq!(preprocessor.lessopen, "batpipe $1"); + assert_eq!(preprocessor.lessopen, "batpipe %s"); assert!(preprocessor.lessclose.is_none()); reset_env_vars(); @@ -327,8 +315,8 @@ mod tests { let preprocessor = LessOpenPreprocessor::mock_new(Some("lessopen.sh %s"), Some("lessclose.sh %s %s"))?; - assert_eq!(preprocessor.lessopen, "lessopen.sh $1"); - assert_eq!(preprocessor.lessclose.unwrap(), "lessclose.sh $1 $2"); + assert_eq!(preprocessor.lessopen, "lessopen.sh %s"); + assert_eq!(preprocessor.lessclose.unwrap(), "lessclose.sh %s %s"); reset_env_vars(); @@ -340,13 +328,13 @@ mod tests { fn test_lessopen_prefixes() -> Result<()> { let preprocessor = LessOpenPreprocessor::mock_new(Some("batpipe %s"), None)?; - assert_eq!(preprocessor.lessopen, "batpipe $1"); + assert_eq!(preprocessor.lessopen, "batpipe %s"); assert!(matches!(preprocessor.kind, LessOpenKind::TempFile)); assert!(!preprocessor.preprocess_stdin); let preprocessor = LessOpenPreprocessor::mock_new(Some("|batpipe %s"), None)?; - assert_eq!(preprocessor.lessopen, "batpipe $1"); + assert_eq!(preprocessor.lessopen, "batpipe %s"); assert!(matches!( preprocessor.kind, LessOpenKind::PipedIgnoreExitCode @@ -355,19 +343,19 @@ mod tests { let preprocessor = LessOpenPreprocessor::mock_new(Some("||batpipe %s"), None)?; - assert_eq!(preprocessor.lessopen, "batpipe $1"); + assert_eq!(preprocessor.lessopen, "batpipe %s"); assert!(matches!(preprocessor.kind, LessOpenKind::Piped)); assert!(!preprocessor.preprocess_stdin); let preprocessor = LessOpenPreprocessor::mock_new(Some("-batpipe %s"), None)?; - assert_eq!(preprocessor.lessopen, "batpipe $1"); + assert_eq!(preprocessor.lessopen, "batpipe %s"); assert!(matches!(preprocessor.kind, LessOpenKind::TempFile)); assert!(preprocessor.preprocess_stdin); let preprocessor = LessOpenPreprocessor::mock_new(Some("|-batpipe %s"), None)?; - assert_eq!(preprocessor.lessopen, "batpipe $1"); + assert_eq!(preprocessor.lessopen, "batpipe %s"); assert!(matches!( preprocessor.kind, LessOpenKind::PipedIgnoreExitCode @@ -376,7 +364,7 @@ mod tests { let preprocessor = LessOpenPreprocessor::mock_new(Some("||-batpipe %s"), None)?; - assert_eq!(preprocessor.lessopen, "batpipe $1"); + assert_eq!(preprocessor.lessopen, "batpipe %s"); assert!(matches!(preprocessor.kind, LessOpenKind::Piped)); assert!(preprocessor.preprocess_stdin); @@ -391,8 +379,8 @@ mod tests { let preprocessor = LessOpenPreprocessor::mock_new(Some("|echo File:%s"), Some("echo File:%s Temp:%s"))?; - assert_eq!(preprocessor.lessopen, "echo File:$1"); - assert_eq!(preprocessor.lessclose.unwrap(), "echo File:$1 Temp:$2"); + assert_eq!(preprocessor.lessopen, "echo File:%s"); + assert_eq!(preprocessor.lessclose.unwrap(), "echo File:%s Temp:%s"); reset_env_vars(); diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs index cd5c0846ff..8b61b87816 100644 --- a/tests/integration_tests.rs +++ b/tests/integration_tests.rs @@ -313,13 +313,19 @@ fn list_themes_to_piped_output() { } #[test] -#[cfg_attr(any(not(feature = "git"), target_os = "windows"), ignore)] +#[cfg_attr( + any(not(feature = "git"), feature = "lessopen", target_os = "windows"), + ignore +)] fn short_help() { test_help("-h", "../doc/short-help.txt"); } #[test] -#[cfg_attr(any(not(feature = "git"), target_os = "windows"), ignore)] +#[cfg_attr( + any(not(feature = "git"), feature = "lessopen", target_os = "windows"), + ignore +)] fn long_help() { test_help("--help", "../doc/long-help.txt"); } @@ -2437,7 +2443,6 @@ fn lessopen_stdin_piped() { #[cfg(unix)] // Expected output assumed that tests are run on a Unix-like system #[cfg(feature = "lessopen")] #[test] -#[serial] // Randomly fails otherwise fn lessopen_and_lessclose_file_temp() { // This is mainly to test that $LESSCLOSE gets passed the correct file paths // In this case, the original file and the temporary file returned by $LESSOPEN @@ -2455,7 +2460,6 @@ fn lessopen_and_lessclose_file_temp() { #[cfg(unix)] // Expected output assumed that tests are run on a Unix-like system #[cfg(feature = "lessopen")] #[test] -#[serial] // Randomly fails otherwise fn lessopen_and_lessclose_file_piped() { // This is mainly to test that $LESSCLOSE gets passed the correct file paths // In these cases, the original file and a dash @@ -2482,8 +2486,6 @@ fn lessopen_and_lessclose_file_piped() { #[cfg(unix)] // Expected output assumed that tests are run on a Unix-like system #[cfg(feature = "lessopen")] #[test] -#[serial] // Randomly fails otherwise -#[ignore = "randomly failing on some systems"] fn lessopen_and_lessclose_stdin_temp() { // This is mainly to test that $LESSCLOSE gets passed the correct file paths // In this case, a dash and the temporary file returned by $LESSOPEN @@ -2501,7 +2503,6 @@ fn lessopen_and_lessclose_stdin_temp() { #[cfg(unix)] // Expected output assumed that tests are run on a Unix-like system #[cfg(feature = "lessopen")] #[test] -#[serial] // Randomly fails otherwise fn lessopen_and_lessclose_stdin_piped() { // This is mainly to test that $LESSCLOSE gets passed the correct file paths // In these cases, two dashes