From 85edfa1f4881e86c564cc3d517cceef6ff9f2a12 Mon Sep 17 00:00:00 2001 From: Anomalocaridid <29845794+Anomalocaridid@users.noreply.github.com> Date: Sat, 16 Dec 2023 17:54:49 -0500 Subject: [PATCH 1/3] replace run_script with execute --- Cargo.lock | 132 +++++++++++++++++++---------------------------------- Cargo.toml | 5 +- 2 files changed, 50 insertions(+), 87 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3578ca08..b4916cf1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -123,6 +123,7 @@ dependencies = [ "content_inspector", "encoding_rs", "etcetera", + "execute", "expect-test", "flate2", "git2", @@ -134,12 +135,10 @@ dependencies = [ "nix", "nu-ansi-term", "once_cell", - "os_str_bytes", "path_abs", "plist", "predicates", "regex", - "run_script", "semver", "serde", "serde_derive", @@ -396,12 +395,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" @@ -461,6 +454,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.4.1" @@ -522,24 +552,12 @@ dependencies = [ ] [[package]] -name = "fsio" -version = "0.4.0" +name = "generic-array" +version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dad0ce30be0cc441b325c5d705c8b613a0ca0d92b6a8953d41bd236dc09a36d0" +checksum = "fe739944a5406424e080edccb6add95685130b9f160d5407c639c7df0c5836b0" 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]] @@ -837,15 +855,6 @@ dependencies = [ "pkg-config", ] -[[package]] -name = "os_str_bytes" -version = "6.6.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e2355d85b9a3786f481747ced0e0ff2ba35213a1f9bd406ed906554d7af805a1" -dependencies = [ - "memchr", -] - [[package]] name = "parking_lot" version = "0.12.1" @@ -910,12 +919,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.0" @@ -973,36 +976,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" @@ -1059,15 +1032,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" @@ -1426,6 +1390,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.8" @@ -1501,12 +1471,6 @@ dependencies = [ "winapi-util", ] -[[package]] -name = "wasi" -version = "0.11.0+wasi-snapshot-preview1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9c8d87e72b64a3b4db28d11ce29237c246188f4f51057d65a7eab63b7987e423" - [[package]] name = "wild" version = "2.2.1" diff --git a/Cargo.toml b/Cargo.toml index b45d7d88..a5cbe323 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"] # 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.2", optional = true } walkdir = { version = "2.4", optional = true } bytesize = { version = "1.3.0" } encoding_rs = "0.8.33" -os_str_bytes = { version = "~6.6", optional = true } -run_script = { version = "^0.10.1", optional = true} +execute = { version = "^0.2.13", optional = true } [dependencies.git2] version = "0.18" From e7f6bcb0fe54c9008b69925fd398a485f6ae4799 Mon Sep 17 00:00:00 2001 From: Anomalocaridid <29845794+Anomalocaridid@users.noreply.github.com> Date: Sat, 16 Dec 2023 17:55:04 -0500 Subject: [PATCH 2/3] refactor lessopen implementation --- src/lessopen.rs | 150 ++++++++++++++++++++++-------------------------- 1 file changed, 69 insertions(+), 81 deletions(-) diff --git a/src/lessopen.rs b/src/lessopen.rs index c8f5225d..cf004d49 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::from_string(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(); + stdin.read_to_end(&mut stdin_buffer)?; - 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); - } - }; + let mut lessopen_command = shell(self.lessopen.replacen("%s", "-", 1)); + lessopen_command.stdout(Stdio::piped()); - if lessopen_handle - .stdin - .as_mut() - .unwrap() - .write_all(&stdin_buffer.clone()) - .is_err() + let lessopen_output = match lessopen_command.execute_input_output(&stdin_buffer) { - return input.open(stdin, stdout_identifier); - } - - let lessopen_output = match lessopen_handle.wait_with_output() { 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,13 +159,17 @@ impl LessOpenPreprocessor { kind, reader: InputReader::new(BufReader::new( if matches!(self.kind, LessOpenKind::TempFile) { - // Remove newline at end of temporary file path returned by $LESSOPEN - let stdout = match lessopen_stdout.strip_suffix("\n") { - Some(stripped) => stripped.to_owned(), - None => lessopen_stdout, + 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_string.strip_suffix("\n") { + Some(stripped) => stripped.to_owned(), + None => lessopen_string, }; - - let stdout = stdout.into_os_string(); let file = match File::open(PathBuf::from(&stdout)) { Ok(file) => file, @@ -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); - run_script::run(command, &self.command_args, &self.command_options) - .expect("failed to run $LESSCLOSE to clean up file"); + let lessclose_output = match lessclose_command.execute_output() { + Ok(output) => output, + Err(_) => { + bat_warning!("failed to run $LESSCLOSE to clean up temporary file"); + return; + } + }; + + 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(); From 1a1c4c3c4d82925565185549d9a6613b84a099aa Mon Sep 17 00:00:00 2001 From: Anomalocaridid <29845794+Anomalocaridid@users.noreply.github.com> Date: Sat, 16 Dec 2023 18:18:06 -0500 Subject: [PATCH 3/3] update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c7ebdc55..1df232d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ - Fix handling of inputs with OSC ANSI escape sequences, see #2541 and #2544 (@eth-p) - Fix handling of inputs with combined ANSI color and attribute sequences, see #2185 and #2856 (@eth-p) - Fix panel width when line 10000 wraps, see #2854 (@eth-p) +- Fix bugs in `$LESSOPEN` support, see #2805 (@Anomalocaridid) ## Other