From c904a8b24cb1371b6503b8c92de613082c59b28c Mon Sep 17 00:00:00 2001 From: Olivier Tilloy Date: Tue, 23 Apr 2024 20:02:34 +0200 Subject: [PATCH] Make error message consistent with GNU diff's implementation when failing to read input file(s) --- src/main.rs | 33 ++++++++++++++++++++++---- src/params.rs | 55 +++++++++++++++++++++++++++++++++++++++----- tests/integration.rs | 21 ++++++++++++----- 3 files changed, 93 insertions(+), 16 deletions(-) diff --git a/src/main.rs b/src/main.rs index f1bd1e2..7e221ea 100644 --- a/src/main.rs +++ b/src/main.rs @@ -4,6 +4,7 @@ // files that was distributed with this source code. use crate::params::{parse_params, Format}; +use regex::Regex; use std::env; use std::ffi::OsString; use std::fs; @@ -18,6 +19,22 @@ mod params; mod unified_diff; mod utils; +fn report_failure_to_read_input_file( + executable: &OsString, + filepath: &OsString, + error: &std::io::Error, +) { + // std::io::Error's display trait outputs "{detail} (os error {code})" + // but we want only the {detail} (error string) part + let error_code_re = Regex::new(r"\ \(os\ error\ \d+\)$").unwrap(); + eprintln!( + "{}: {}: {}", + executable.to_string_lossy(), + filepath.to_string_lossy(), + error_code_re.replace(error.to_string().as_str(), ""), + ); +} + // Exit codes are documented at // https://www.gnu.org/software/diffutils/manual/html_node/Invoking-diff.html. // An exit status of 0 means no differences were found, @@ -45,6 +62,7 @@ fn main() -> ExitCode { maybe_report_identical_files(); return ExitCode::SUCCESS; } + // read files fn read_file_contents(filepath: &OsString) -> io::Result> { if filepath == "-" { @@ -54,20 +72,27 @@ fn main() -> ExitCode { fs::read(filepath) } } + let mut io_error = false; let from_content = match read_file_contents(¶ms.from) { Ok(from_content) => from_content, Err(e) => { - eprintln!("Failed to read from-file: {e}"); - return ExitCode::from(2); + report_failure_to_read_input_file(¶ms.executable, ¶ms.from, &e); + io_error = true; + vec![] } }; let to_content = match read_file_contents(¶ms.to) { Ok(to_content) => to_content, Err(e) => { - eprintln!("Failed to read to-file: {e}"); - return ExitCode::from(2); + report_failure_to_read_input_file(¶ms.executable, ¶ms.to, &e); + io_error = true; + vec![] } }; + if io_error { + return ExitCode::from(2); + } + // run diff let result: Vec = match params.format { Format::Normal => normal_diff::diff(&from_content, &to_content, ¶ms), diff --git a/src/params.rs b/src/params.rs index 88feb47..770a78e 100644 --- a/src/params.rs +++ b/src/params.rs @@ -13,6 +13,7 @@ pub enum Format { #[derive(Clone, Debug, Eq, PartialEq)] pub struct Params { + pub executable: OsString, pub from: OsString, pub to: OsString, pub format: Format, @@ -26,6 +27,7 @@ pub struct Params { impl Default for Params { fn default() -> Self { Self { + executable: OsString::default(), from: OsString::default(), to: OsString::default(), format: Format::default(), @@ -42,10 +44,13 @@ pub fn parse_params>(opts: I) -> Result ".to_string()); }; - let mut params = Params::default(); + let mut params = Params { + executable, + ..Default::default() + }; let mut from = None; let mut to = None; let mut format = None; @@ -62,7 +67,10 @@ pub fn parse_params>(opts: I) -> Result ", exe.to_string_lossy())); + return Err(format!( + "Usage: {} ", + params.executable.to_string_lossy() + )); } continue; } @@ -154,7 +162,10 @@ pub fn parse_params>(opts: I) -> Result ", exe.to_string_lossy())); + return Err(format!( + "Usage: {} ", + params.executable.to_string_lossy() + )); } } params.from = if let Some(from) = from { @@ -162,14 +173,20 @@ pub fn parse_params>(opts: I) -> Result ", exe.to_string_lossy())); + return Err(format!( + "Usage: {} ", + params.executable.to_string_lossy() + )); }; params.to = if let Some(to) = to { to } else if let Some(param) = opts.next() { param } else { - return Err(format!("Usage: {} ", exe.to_string_lossy())); + return Err(format!( + "Usage: {} ", + params.executable.to_string_lossy() + )); }; params.format = format.unwrap_or(Format::default()); if let Some(context_count) = context { @@ -286,6 +303,7 @@ mod tests { fn basics() { assert_eq!( Ok(Params { + executable: os("diff"), from: os("foo"), to: os("bar"), ..Default::default() @@ -294,6 +312,7 @@ mod tests { ); assert_eq!( Ok(Params { + executable: os("diff"), from: os("foo"), to: os("bar"), ..Default::default() @@ -310,6 +329,7 @@ mod tests { for arg in ["-e", "--ed"] { assert_eq!( Ok(Params { + executable: os("diff"), from: os("foo"), to: os("bar"), format: Format::Ed, @@ -327,6 +347,7 @@ mod tests { params.extend(["foo", "bar"]); assert_eq!( Ok(Params { + executable: os("diff"), from: os("foo"), to: os("bar"), format: Format::Context, @@ -347,6 +368,7 @@ mod tests { params.extend(["foo", "bar"]); assert_eq!( Ok(Params { + executable: os("diff"), from: os("foo"), to: os("bar"), format: Format::Context, @@ -384,6 +406,7 @@ mod tests { params.extend(["foo", "bar"]); assert_eq!( Ok(Params { + executable: os("diff"), from: os("foo"), to: os("bar"), format: Format::Unified, @@ -404,6 +427,7 @@ mod tests { params.extend(["foo", "bar"]); assert_eq!( Ok(Params { + executable: os("diff"), from: os("foo"), to: os("bar"), format: Format::Unified, @@ -437,6 +461,7 @@ mod tests { fn context_count() { assert_eq!( Ok(Params { + executable: os("diff"), from: os("foo"), to: os("bar"), format: Format::Unified, @@ -451,6 +476,7 @@ mod tests { ); assert_eq!( Ok(Params { + executable: os("diff"), from: os("foo"), to: os("bar"), format: Format::Unified, @@ -465,6 +491,7 @@ mod tests { ); assert_eq!( Ok(Params { + executable: os("diff"), from: os("foo"), to: os("bar"), format: Format::Unified, @@ -479,6 +506,7 @@ mod tests { ); assert_eq!( Ok(Params { + executable: os("diff"), from: os("foo"), to: os("bar"), format: Format::Context, @@ -496,6 +524,7 @@ mod tests { fn report_identical_files() { assert_eq!( Ok(Params { + executable: os("diff"), from: os("foo"), to: os("bar"), ..Default::default() @@ -504,6 +533,7 @@ mod tests { ); assert_eq!( Ok(Params { + executable: os("diff"), from: os("foo"), to: os("bar"), report_identical_files: true, @@ -513,6 +543,7 @@ mod tests { ); assert_eq!( Ok(Params { + executable: os("diff"), from: os("foo"), to: os("bar"), report_identical_files: true, @@ -534,6 +565,7 @@ mod tests { fn brief() { assert_eq!( Ok(Params { + executable: os("diff"), from: os("foo"), to: os("bar"), ..Default::default() @@ -542,6 +574,7 @@ mod tests { ); assert_eq!( Ok(Params { + executable: os("diff"), from: os("foo"), to: os("bar"), brief: true, @@ -551,6 +584,7 @@ mod tests { ); assert_eq!( Ok(Params { + executable: os("diff"), from: os("foo"), to: os("bar"), brief: true, @@ -567,6 +601,7 @@ mod tests { fn expand_tabs() { assert_eq!( Ok(Params { + executable: os("diff"), from: os("foo"), to: os("bar"), ..Default::default() @@ -576,6 +611,7 @@ mod tests { for option in ["-t", "--expand-tabs"] { assert_eq!( Ok(Params { + executable: os("diff"), from: os("foo"), to: os("bar"), expand_tabs: true, @@ -593,6 +629,7 @@ mod tests { fn tabsize() { assert_eq!( Ok(Params { + executable: os("diff"), from: os("foo"), to: os("bar"), ..Default::default() @@ -601,6 +638,7 @@ mod tests { ); assert_eq!( Ok(Params { + executable: os("diff"), from: os("foo"), to: os("bar"), tabsize: 0, @@ -614,6 +652,7 @@ mod tests { ); assert_eq!( Ok(Params { + executable: os("diff"), from: os("foo"), to: os("bar"), tabsize: 42, @@ -671,6 +710,7 @@ mod tests { fn double_dash() { assert_eq!( Ok(Params { + executable: os("diff"), from: os("-g"), to: os("-h"), ..Default::default() @@ -682,6 +722,7 @@ mod tests { fn default_to_stdin() { assert_eq!( Ok(Params { + executable: os("diff"), from: os("foo"), to: os("-"), ..Default::default() @@ -690,6 +731,7 @@ mod tests { ); assert_eq!( Ok(Params { + executable: os("diff"), from: os("-"), to: os("bar"), ..Default::default() @@ -698,6 +740,7 @@ mod tests { ); assert_eq!( Ok(Params { + executable: os("diff"), from: os("-"), to: os("-"), ..Default::default() diff --git a/tests/integration.rs b/tests/integration.rs index 6afa7c5..4f66574 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -35,21 +35,30 @@ fn cannot_read_files() -> Result<(), Box> { cmd.assert() .code(predicate::eq(2)) .failure() - .stderr(predicate::str::starts_with("Failed to read from-file")); + .stderr(predicate::str::ends_with(format!( + ": {}: No such file or directory\n", + &nopath.as_os_str().to_string_lossy() + ))); let mut cmd = Command::cargo_bin("diffutils")?; cmd.arg(file.path()).arg(&nopath); cmd.assert() .code(predicate::eq(2)) .failure() - .stderr(predicate::str::starts_with("Failed to read to-file")); + .stderr(predicate::str::ends_with(format!( + ": {}: No such file or directory\n", + &nopath.as_os_str().to_string_lossy() + ))); let mut cmd = Command::cargo_bin("diffutils")?; cmd.arg(&nopath).arg(&nopath); - cmd.assert() - .code(predicate::eq(2)) - .failure() - .stderr(predicate::str::starts_with("Failed to read from-file")); + cmd.assert().code(predicate::eq(2)).failure().stderr( + predicate::str::contains(format!( + ": {}: No such file or directory\n", + &nopath.as_os_str().to_string_lossy() + )) + .count(2), + ); Ok(()) }