From 96115ea0319fd32d87ffdae4874cee1f581d755c Mon Sep 17 00:00:00 2001 From: Valentin Isipchuk Date: Wed, 7 Jan 2026 18:18:04 +0700 Subject: [PATCH 01/10] Extract shared code to utils --- src/diff.rs | 42 +++++++++++------------------------------- src/utils.rs | 28 ++++++++++++++++++---------- 2 files changed, 29 insertions(+), 41 deletions(-) diff --git a/src/diff.rs b/src/diff.rs index f4c0614..93304ce 100644 --- a/src/diff.rs +++ b/src/diff.rs @@ -4,12 +4,10 @@ // files that was distributed with this source code. use crate::params::{parse_params, Format}; -use crate::utils::report_failure_to_read_input_file; +use crate::utils; use crate::{context_diff, ed_diff, normal_diff, side_diff, unified_diff}; use std::env::ArgsOs; -use std::ffi::OsString; -use std::fs; -use std::io::{self, stdout, Read, Write}; +use std::io::{self, stdout, Write}; use std::iter::Peekable; use std::process::{exit, ExitCode}; @@ -23,6 +21,7 @@ pub fn main(opts: Peekable) -> ExitCode { eprintln!("{error}"); exit(2); }); + // if from and to are the same file, no need to perform any comparison let maybe_report_identical_files = || { if params.report_identical_files { @@ -40,35 +39,16 @@ pub fn main(opts: Peekable) -> ExitCode { return ExitCode::SUCCESS; } - // read files - fn read_file_contents(filepath: &OsString) -> io::Result> { - if filepath == "-" { - let mut content = Vec::new(); - io::stdin().read_to_end(&mut content).and(Ok(content)) - } else { - fs::read(filepath) - } - } - let mut io_error = false; - let from_content = match read_file_contents(¶ms.from) { - Ok(from_content) => from_content, - Err(e) => { - 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) => { - report_failure_to_read_input_file(¶ms.executable, ¶ms.to, &e); - io_error = true; - vec![] + let (from_content, to_content) = match utils::read_both_files(¶ms.from, ¶ms.to) { + Ok(contents) => contents, + Err((filepath, error)) => { + eprintln!( + "{}", + utils::format_failure_to_read_input_file(¶ms.executable, &filepath, &error) + ); + return ExitCode::from(2); } }; - if io_error { - return ExitCode::from(2); - } // run diff let result: Vec = match params.format { diff --git a/src/utils.rs b/src/utils.rs index daca18d..d4074a4 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -4,7 +4,8 @@ // files that was distributed with this source code. use regex::Regex; -use std::{ffi::OsString, io::Write}; +use std::io::{self, Error, Read, Write}; +use std::{ffi::OsString, fs}; use unicode_width::UnicodeWidthStr; /// Replace tabs by spaces in the input line. @@ -87,15 +88,22 @@ pub fn format_failure_to_read_input_file( ) } -pub fn report_failure_to_read_input_file( - executable: &OsString, - filepath: &OsString, - error: &std::io::Error, -) { - eprintln!( - "{}", - format_failure_to_read_input_file(executable, filepath, error) - ); +pub fn read_file_contents(filepath: &OsString) -> io::Result> { + if filepath == "-" { + let mut content = Vec::new(); + io::stdin().read_to_end(&mut content).and(Ok(content)) + } else { + fs::read(filepath) + } +} + +pub fn read_both_files( + from: &OsString, + to: &OsString, +) -> Result<(Vec, Vec), (OsString, Error)> { + let from_content = read_file_contents(from).map_err(|e| (from.clone(), e))?; + let to_content = read_file_contents(to).map_err(|e| (to.clone(), e))?; + Ok((from_content, to_content)) } #[cfg(test)] From 4be21f187c647dd21a1361eef3f35aa34cffefab Mon Sep 17 00:00:00 2001 From: Valentin Isipchuk Date: Wed, 7 Jan 2026 18:44:37 +0700 Subject: [PATCH 02/10] Make side_diff use separate set of formatting params --- src/diff.rs | 11 ++++++++++- src/side_diff.rs | 9 +++++++-- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/src/diff.rs b/src/diff.rs index 93304ce..1933686 100644 --- a/src/diff.rs +++ b/src/diff.rs @@ -61,7 +61,16 @@ pub fn main(opts: Peekable) -> ExitCode { }), Format::SideBySide => { let mut output = stdout().lock(); - side_diff::diff(&from_content, &to_content, &mut output, ¶ms) + side_diff::diff( + &from_content, + &to_content, + &mut output, + &side_diff::Params { + tabsize: params.tabsize, + width: params.width, + expand_tabs: params.expand_tabs, + }, + ) } }; if params.brief && !result.is_empty() { diff --git a/src/side_diff.rs b/src/side_diff.rs index 56953d2..082b774 100644 --- a/src/side_diff.rs +++ b/src/side_diff.rs @@ -8,8 +8,6 @@ use diff::Result; use std::{io::Write, vec}; use unicode_width::UnicodeWidthStr; -use crate::params::Params; - const GUTTER_WIDTH_MIN: usize = 3; struct CharIter<'a> { @@ -306,6 +304,13 @@ fn push_output( Ok(()) } +#[derive(Default)] +pub struct Params { + pub width: usize, + pub tabsize: usize, + pub expand_tabs: bool, +} + pub fn diff( from_file: &[u8], to_file: &[u8], From ed9f154794220bdc70e1cf5cb47ffaa6f705d13c Mon Sep 17 00:00:00 2001 From: Valentin Isipchuk Date: Fri, 9 Jan 2026 14:32:08 +0700 Subject: [PATCH 03/10] Add sdiff initiall implementation --- src/sdiff.rs | 221 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 221 insertions(+) create mode 100644 src/sdiff.rs diff --git a/src/sdiff.rs b/src/sdiff.rs new file mode 100644 index 0000000..56a7391 --- /dev/null +++ b/src/sdiff.rs @@ -0,0 +1,221 @@ +// This file is part of the uutils diffutils package. +// +// For the full copyright and license information, please view the LICENSE-* +// files that was distributed with this source code. + +use regex::Regex; + +// use crate::params::parse_params; +use crate::side_diff; +use crate::utils; +use std::env::ArgsOs; +use std::ffi::OsString; +use std::io::{self, stdout, Write}; +use std::iter::Peekable; +use std::process::{exit, ExitCode}; + +#[derive(Eq, PartialEq, Debug)] +pub struct Params { + pub executable: OsString, + pub from: OsString, + pub to: OsString, + pub expand_tabs: bool, + pub tabsize: usize, + pub width: usize, +} + +impl Default for Params { + fn default() -> Self { + Self { + executable: OsString::default(), + from: OsString::default(), + to: OsString::default(), + expand_tabs: false, + tabsize: 8, + width: 130, + } + } +} + +pub fn parse_params>(mut opts: Peekable) -> Result { + let Some(executable) = opts.next() else { + return Err("Usage: ".to_string()); + }; + + let mut params = Params { + executable, + ..Default::default() + }; + + let mut from = None; + let mut to = None; + let tabsize_re = Regex::new(r"^--tabsize=(?\d+)$").unwrap(); + let width_re = Regex::new(r"--width=(?P\d+)$").unwrap(); + + while let Some(param) = opts.next() { + if param == "-" { + if from.is_none() { + from = Some(param); + } else if to.is_none() { + to = Some(param); + } else { + return Err(format!( + "Usage: {} ", + params.executable.to_string_lossy() + )); + } + continue; + } + + if param == "-t" || param == "--expand-tabs" { + params.expand_tabs = true; + continue; + } + + if tabsize_re.is_match(param.to_string_lossy().as_ref()) { + // Because param matches the regular expression, + // it is safe to assume it is valid UTF-8. + let param = param.into_string().unwrap(); + let tabsize_str = tabsize_re + .captures(param.as_str()) + .unwrap() + .name("num") + .unwrap() + .as_str(); + params.tabsize = match tabsize_str.parse::() { + Ok(num) => { + if num == 0 { + return Err("invalid tabsize «0»".to_string()); + } + + num + } + Err(_) => return Err(format!("invalid tabsize «{tabsize_str}»")), + }; + + continue; + } + + if width_re.is_match(param.to_string_lossy().as_ref()) { + let param = param.into_string().unwrap(); + let width_str: &str = width_re + .captures(param.as_str()) + .unwrap() + .name("long") + .unwrap() + .as_str(); + + params.width = match width_str.parse::() { + Ok(num) => { + if num == 0 { + return Err("invalid width «0»".to_string()); + } + + num + } + Err(_) => return Err(format!("invalid width «{width_str}»")), + }; + continue; + } + + if from.is_none() { + from = Some(param); + } else if to.is_none() { + to = Some(param); + } else { + return Err(format!( + "Usage: {} ", + params.executable.to_string_lossy() + )); + } + } + + params.from = if let Some(from) = from { + from + } else if let Some(param) = opts.next() { + param + } else { + return Err(format!("Err")); + }; + + params.to = if let Some(to) = to { + to + } else if let Some(param) = opts.next() { + param + } else { + return Err(format!("Err")); + }; + + Ok(params) +} + +pub fn main(opts: Peekable) -> ExitCode { + let params = parse_params(opts).unwrap_or_else(|error| { + eprintln!("{error}"); + exit(2); + }); + + if params.from == "-" && params.to == "-" + || same_file::is_same_file(¶ms.from, ¶ms.to).unwrap_or(false) + { + return ExitCode::SUCCESS; + } + + let (from_content, to_content) = match utils::read_both_files(¶ms.from, ¶ms.to) { + Ok(contents) => contents, + Err((filepath, error)) => { + eprintln!( + "{}", + utils::format_failure_to_read_input_file(¶ms.executable, &filepath, &error) + ); + return ExitCode::from(2); + } + }; + + // run diff + let mut output = stdout().lock(); + let result = side_diff::diff( + &from_content, + &to_content, + &mut output, + &side_diff::Params { + tabsize: params.tabsize, + width: params.width, + expand_tabs: params.expand_tabs, + }, + ); + + io::stdout().write_all(&result).unwrap(); + if result.is_empty() { + ExitCode::SUCCESS + } else { + ExitCode::from(1) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn os(s: &str) -> OsString { + OsString::from(s) + } + + #[test] + fn sdiff_params() { + assert_eq!( + Ok(Params { + executable: os("sdiff"), + from: os("foo"), + to: os("bar"), + ..Default::default() + }), + parse_params( + [os("sdiff"), os("foo"), os("bar")] + .iter() + .cloned() + .peekable() + ) + ) + } +} From 7355680ad7532c403d9f2de71cfa7e87a8cbc434 Mon Sep 17 00:00:00 2001 From: Valentin Isipchuk Date: Fri, 9 Jan 2026 14:33:31 +0700 Subject: [PATCH 04/10] Add call of sdiff to main and basic integration test --- src/main.rs | 2 ++ tests/integration.rs | 26 +++++++++++++++++++++++++- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/src/main.rs b/src/main.rs index b7c2712..792ce95 100644 --- a/src/main.rs +++ b/src/main.rs @@ -18,6 +18,7 @@ mod ed_diff; mod macros; mod normal_diff; mod params; +mod sdiff; mod side_diff; mod unified_diff; mod utils; @@ -72,6 +73,7 @@ fn main() -> ExitCode { match util_name.to_str() { Some("diff") => diff::main(args), Some("cmp") => cmp::main(args), + Some("sdiff") => sdiff::main(args), Some(name) => { eprintln!("{name}: utility not supported"); ExitCode::from(2) diff --git a/tests/integration.rs b/tests/integration.rs index b37d7e6..b6c3c88 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -57,7 +57,7 @@ mod common { #[cfg(windows)] let error_message = "The system cannot find the file specified."; - for subcmd in ["diff", "cmp"] { + for subcmd in ["diff", "cmp", "sdiff"] { let mut cmd = cargo_bin_cmd!("diffutils"); cmd.arg(subcmd); cmd.arg(&nopath).arg(file.path()); @@ -888,3 +888,27 @@ mod cmp { Ok(()) } } + +mod sdiff { + use super::*; + + #[test] + fn differences() -> Result<(), Box> { + let mut file1 = NamedTempFile::new()?; + file1.write_all("foo\n".as_bytes())?; + + let mut file2 = NamedTempFile::new()?; + file2.write_all("bar\n".as_bytes())?; + + let mut cmd = cargo_bin_cmd!("diffutils"); + cmd.arg("diff"); + cmd.arg(file1.path()).arg(file2.path()); + + cmd.assert() + .code(predicate::eq(1)) + .failure() + .stdout(predicate::str::is_empty().not()); + + Ok(()) + } +} From a5a1baf23ed8268085a59f8df7845ed1ceb5ed08 Mon Sep 17 00:00:00 2001 From: Valentin Isipchuk Date: Sat, 10 Jan 2026 11:27:44 +0700 Subject: [PATCH 05/10] Fix usage side_diff in fuzzers --- fuzz/fuzz_targets/fuzz_side.rs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/fuzz/fuzz_targets/fuzz_side.rs b/fuzz/fuzz_targets/fuzz_side.rs index 8a69c07..050a015 100644 --- a/fuzz/fuzz_targets/fuzz_side.rs +++ b/fuzz/fuzz_targets/fuzz_side.rs @@ -2,11 +2,10 @@ #[macro_use] extern crate libfuzzer_sys; -use diffutilslib::side_diff; +use diffutilslib::side_diff::{self, Params}; use std::fs::File; use std::io::Write; -use diffutilslib::params::Params; fuzz_target!(|x: (Vec, Vec, /* usize, usize */ bool)| { let (original, new, /* width, tabsize, */ expand) = x; @@ -22,7 +21,16 @@ fuzz_target!(|x: (Vec, Vec, /* usize, usize */ bool)| { ..Default::default() }; let mut output_buf = vec![]; - side_diff::diff(&original, &new, &mut output_buf, ¶ms); + side_diff::diff( + &original, + &new, + &mut output_buf, + &Params { + width: params.width, + tabsize: params.tabsize, + expand_tabs: params.expand_tabs, + }, + ); File::create("target/fuzz.file.original") .unwrap() .write_all(&original) @@ -39,4 +47,5 @@ fuzz_target!(|x: (Vec, Vec, /* usize, usize */ bool)| { .unwrap() .write_all(&output_buf) .unwrap(); -}); \ No newline at end of file +}); + From f3c4c783b1f1de0b70b3c0ca8230e5cd233ceb39 Mon Sep 17 00:00:00 2001 From: Valentin Isipchuk Date: Sat, 10 Jan 2026 11:32:05 +0700 Subject: [PATCH 06/10] Fix codestyle --- src/sdiff.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/sdiff.rs b/src/sdiff.rs index 56a7391..5ede436 100644 --- a/src/sdiff.rs +++ b/src/sdiff.rs @@ -52,7 +52,7 @@ pub fn parse_params>(mut opts: Peekable) -> Resu let tabsize_re = Regex::new(r"^--tabsize=(?\d+)$").unwrap(); let width_re = Regex::new(r"--width=(?P\d+)$").unwrap(); - while let Some(param) = opts.next() { + for param in opts.by_ref() { if param == "-" { if from.is_none() { from = Some(param); @@ -135,7 +135,10 @@ pub fn parse_params>(mut opts: Peekable) -> Resu } else if let Some(param) = opts.next() { param } else { - return Err(format!("Err")); + return Err(format!( + "Usage: {} ", + params.executable.to_string_lossy() + )); }; params.to = if let Some(to) = to { @@ -143,7 +146,10 @@ pub fn parse_params>(mut opts: Peekable) -> Resu } else if let Some(param) = opts.next() { param } else { - return Err(format!("Err")); + return Err(format!( + "Usage: {} ", + params.executable.to_string_lossy() + )); }; Ok(params) From 17bc12c57306aafd01257a26aa6d6449e07cb407 Mon Sep 17 00:00:00 2001 From: Valentin Isipchuk Date: Sat, 10 Jan 2026 15:44:57 +0700 Subject: [PATCH 07/10] Fix failed test --- tests/integration.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/integration.rs b/tests/integration.rs index b6c3c88..3935181 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -84,13 +84,13 @@ mod common { let mut cmd = cargo_bin_cmd!("diffutils"); cmd.arg("diff"); cmd.arg(&nopath).arg(&nopath); - cmd.assert().code(predicate::eq(2)).failure().stderr( - predicate::str::contains(format!( + cmd.assert() + .code(predicate::eq(2)) + .failure() + .stderr(predicate::str::contains(format!( ": {}: {error_message}\n", &nopath.as_os_str().to_string_lossy() - )) - .count(2), - ); + ))); Ok(()) } From 06e1d926ff678cb35dfee4c8ea1623669f105bd7 Mon Sep 17 00:00:00 2001 From: Valentin Isipchuk Date: Sun, 11 Jan 2026 16:08:59 +0700 Subject: [PATCH 08/10] Add more tests for new code --- src/sdiff.rs | 123 +++++++++++++++++++++++++++++++++++++++++++++++++++ src/utils.rs | 46 +++++++++++++++++++ 2 files changed, 169 insertions(+) diff --git a/src/sdiff.rs b/src/sdiff.rs index 5ede436..7446c34 100644 --- a/src/sdiff.rs +++ b/src/sdiff.rs @@ -222,6 +222,129 @@ mod tests { .cloned() .peekable() ) + ); + + assert_eq!( + Ok(Params { + executable: os("sdiff"), + from: os("foo"), + to: os("bar"), + ..Default::default() + }), + parse_params( + [os("sdiff"), os("foo"), os("bar")] + .iter() + .cloned() + .peekable() + ) + ); + for option in ["-t", "--expand-tabs"] { + assert_eq!( + Ok(Params { + executable: os("sdiff"), + from: os("foo"), + to: os("bar"), + expand_tabs: true, + ..Default::default() + }), + parse_params( + [os("sdiff"), os(option), os("foo"), os("bar")] + .iter() + .cloned() + .peekable() + ) + ); + } + assert_eq!( + Ok(Params { + executable: os("sdiff"), + from: os("foo"), + to: os("bar"), + ..Default::default() + }), + parse_params( + [os("sdiff"), os("foo"), os("bar")] + .iter() + .cloned() + .peekable() + ) + ); + assert_eq!( + Ok(Params { + executable: os("sdiff"), + from: os("foo"), + to: os("bar"), + tabsize: 1, + ..Default::default() + }), + parse_params( + [os("sdiff"), os("--tabsize=1"), os("foo"), os("bar")] + .iter() + .cloned() + .peekable() + ) + ); + assert_eq!( + Ok(Params { + executable: os("sdiff"), + from: os("foo"), + to: os("bar"), + tabsize: 42, + ..Default::default() + }), + parse_params( + [os("sdiff"), os("--tabsize=42"), os("foo"), os("bar")] + .iter() + .cloned() + .peekable() + ) + ); + assert!(parse_params( + [os("sdiff"), os("--tabsize"), os("foo"), os("bar")] + .iter() + .cloned() + .peekable() + ) + .is_err()); + assert!(parse_params( + [os("sdiff"), os("--tabsize="), os("foo"), os("bar")] + .iter() + .cloned() + .peekable() + ) + .is_err()); + assert!(parse_params( + [os("sdiff"), os("--tabsize=r2"), os("foo"), os("bar")] + .iter() + .cloned() + .peekable() + ) + .is_err()); + assert!(parse_params( + [os("sdiff"), os("--tabsize=-1"), os("foo"), os("bar")] + .iter() + .cloned() + .peekable() + ) + .is_err()); + assert!(parse_params( + [os("sdiff"), os("--tabsize=r2"), os("foo"), os("bar")] + .iter() + .cloned() + .peekable() + ) + .is_err()); + assert!(parse_params( + [ + os("sdiff"), + os("--tabsize=92233720368547758088"), + os("foo"), + os("bar") + ] + .iter() + .cloned() + .peekable() ) + .is_err()); } } diff --git a/src/utils.rs b/src/utils.rs index d4074a4..2e80814 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -153,6 +153,52 @@ mod tests { } } + mod read_file { + use super::*; + use tempfile::NamedTempFile; + + #[test] + fn read_two_valid_files() { + let content1 = "content-1"; + let content2 = "content-2"; + + let mut from_file = NamedTempFile::new().unwrap(); + let mut to_file = NamedTempFile::new().unwrap(); + + from_file.write_all(content1.as_bytes()).unwrap(); + to_file.write_all(content2.as_bytes()).unwrap(); + + let from_path = OsString::from(from_file.path()); + let to_path = OsString::from(to_file.path()); + + let res = read_both_files(&from_path, &to_path); + + assert!(res.is_ok()); + let (from_content, to_content) = res.unwrap(); + assert_eq!(from_content, content1.as_bytes()); + assert_eq!(to_content, content2.as_bytes()); + } + + #[test] + fn read_not_exist_file() { + let mut file = NamedTempFile::new().unwrap(); + file.write_all(b"valid-file").unwrap(); + let exist_file_path = OsString::from(file.path()); + + let non_exist_file_path = OsString::from("non-exist-file"); + + let res = read_both_files(&non_exist_file_path, &exist_file_path); + assert!(res.is_err()); + let (err_path, _) = res.unwrap_err(); + assert_eq!(err_path, non_exist_file_path); + + let res = read_both_files(&exist_file_path, &non_exist_file_path); + assert!(res.is_err()); + let (err_path, _) = res.unwrap_err(); + assert_eq!(err_path, non_exist_file_path); + } + } + mod write_line { use super::*; use pretty_assertions::assert_eq; From 9983fb21cd68b3173d6f6418e7c4c61e0106b67e Mon Sep 17 00:00:00 2001 From: Valentin Isipchuk Date: Tue, 13 Jan 2026 13:50:02 +0700 Subject: [PATCH 09/10] More tests for sdiff params parsing --- src/sdiff.rs | 72 +++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 63 insertions(+), 9 deletions(-) diff --git a/src/sdiff.rs b/src/sdiff.rs index 7446c34..4a8721d 100644 --- a/src/sdiff.rs +++ b/src/sdiff.rs @@ -5,7 +5,6 @@ use regex::Regex; -// use crate::params::parse_params; use crate::side_diff; use crate::utils; use std::env::ArgsOs; @@ -227,17 +226,21 @@ mod tests { assert_eq!( Ok(Params { executable: os("sdiff"), - from: os("foo"), - to: os("bar"), + from: os("-"), + to: os("-"), ..Default::default() }), - parse_params( - [os("sdiff"), os("foo"), os("bar")] - .iter() - .cloned() - .peekable() - ) + parse_params([os("sdiff"), os("-"), os("-")].iter().cloned().peekable()) ); + + assert!(parse_params( + [os("sdiff"), os("foo"), os("bar"), os("-"), os("-")] + .iter() + .cloned() + .peekable() + ) + .is_err()); + for option in ["-t", "--expand-tabs"] { assert_eq!( Ok(Params { @@ -255,6 +258,39 @@ mod tests { ) ); } + + assert_eq!( + Ok(Params { + executable: os("sdiff"), + from: os("foo"), + to: os("bar"), + width: 10, + ..Default::default() + }), + parse_params( + [os("sdiff"), os("--width=10"), os("foo"), os("bar")] + .iter() + .cloned() + .peekable() + ) + ); + + assert!(parse_params( + [os("sdiff"), os("--width=0"), os("foo"), os("bar")] + .iter() + .cloned() + .peekable() + ) + .is_err()); + + assert!(parse_params( + [os("sdiff"), os("--width=.1"), os("foo"), os("bar")] + .iter() + .cloned() + .peekable() + ) + .is_err()); + assert_eq!( Ok(Params { executable: os("sdiff"), @@ -269,6 +305,7 @@ mod tests { .peekable() ) ); + assert_eq!( Ok(Params { executable: os("sdiff"), @@ -284,6 +321,15 @@ mod tests { .peekable() ) ); + + assert!(parse_params( + [os("sdiff"), os("--tabsize=0"), os("foo"), os("bar")] + .iter() + .cloned() + .peekable() + ) + .is_err()); + assert_eq!( Ok(Params { executable: os("sdiff"), @@ -299,6 +345,9 @@ mod tests { .peekable() ) ); + + assert!(parse_params([os("sdiff")].iter().cloned().peekable()).is_err()); + assert!(parse_params( [os("sdiff"), os("--tabsize"), os("foo"), os("bar")] .iter() @@ -306,6 +355,7 @@ mod tests { .peekable() ) .is_err()); + assert!(parse_params( [os("sdiff"), os("--tabsize="), os("foo"), os("bar")] .iter() @@ -313,6 +363,7 @@ mod tests { .peekable() ) .is_err()); + assert!(parse_params( [os("sdiff"), os("--tabsize=r2"), os("foo"), os("bar")] .iter() @@ -320,6 +371,7 @@ mod tests { .peekable() ) .is_err()); + assert!(parse_params( [os("sdiff"), os("--tabsize=-1"), os("foo"), os("bar")] .iter() @@ -327,6 +379,7 @@ mod tests { .peekable() ) .is_err()); + assert!(parse_params( [os("sdiff"), os("--tabsize=r2"), os("foo"), os("bar")] .iter() @@ -334,6 +387,7 @@ mod tests { .peekable() ) .is_err()); + assert!(parse_params( [ os("sdiff"), From 038f423d48c14924c5d1fd9a21374edd29aa56fb Mon Sep 17 00:00:00 2001 From: Gunter Schmidt Date: Wed, 4 Mar 2026 21:53:45 +0100 Subject: [PATCH 10/10] fix: Restore file not found test and correct prg. Instead of 'fixing' the test, the program now acts like the GNU original and returns up to two error messages, one for each file. --- src/diff.rs | 4 +-- src/sdiff.rs | 4 +-- src/utils.rs | 61 ++++++++++++++++++++++++++++++++++++-------- tests/integration.rs | 26 +++++++++++-------- 4 files changed, 69 insertions(+), 26 deletions(-) diff --git a/src/diff.rs b/src/diff.rs index 1933686..c22691b 100644 --- a/src/diff.rs +++ b/src/diff.rs @@ -41,10 +41,10 @@ pub fn main(opts: Peekable) -> ExitCode { let (from_content, to_content) = match utils::read_both_files(¶ms.from, ¶ms.to) { Ok(contents) => contents, - Err((filepath, error)) => { + Err(e) => { eprintln!( "{}", - utils::format_failure_to_read_input_file(¶ms.executable, &filepath, &error) + utils::format_failure_to_read_input_files(¶ms.executable, &e) ); return ExitCode::from(2); } diff --git a/src/sdiff.rs b/src/sdiff.rs index 4a8721d..df5af54 100644 --- a/src/sdiff.rs +++ b/src/sdiff.rs @@ -168,10 +168,10 @@ pub fn main(opts: Peekable) -> ExitCode { let (from_content, to_content) = match utils::read_both_files(¶ms.from, ¶ms.to) { Ok(contents) => contents, - Err((filepath, error)) => { + Err(e) => { eprintln!( "{}", - utils::format_failure_to_read_input_file(¶ms.executable, &filepath, &error) + utils::format_failure_to_read_input_files(¶ms.executable, &e) ); return ExitCode::from(2); } diff --git a/src/utils.rs b/src/utils.rs index 2e80814..2b38b80 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -88,6 +88,28 @@ pub fn format_failure_to_read_input_file( ) } +/// Formats the error messages of both files. +pub fn format_failure_to_read_input_files( + executable: &OsString, + errors: &[(OsString, Error)], +) -> String { + let mut msg = format_failure_to_read_input_file( + executable, + &errors[0].0, // filepath, + &errors[0].1, // &error, + ); + if errors.len() > 1 { + msg.push('\n'); + msg.push_str(&format_failure_to_read_input_file( + executable, + &errors[1].0, // filepath, + &errors[1].1, // &error, + )); + } + + msg +} + pub fn read_file_contents(filepath: &OsString) -> io::Result> { if filepath == "-" { let mut content = Vec::new(); @@ -97,13 +119,30 @@ pub fn read_file_contents(filepath: &OsString) -> io::Result> { } } -pub fn read_both_files( - from: &OsString, - to: &OsString, -) -> Result<(Vec, Vec), (OsString, Error)> { - let from_content = read_file_contents(from).map_err(|e| (from.clone(), e))?; - let to_content = read_file_contents(to).map_err(|e| (to.clone(), e))?; - Ok((from_content, to_content)) +pub type ResultReadBothFiles = Result<(Vec, Vec), Vec<(OsString, Error)>>; +/// Reads both files and returns the files or a list of errors, as both files can produce a separate error. +pub fn read_both_files(from: &OsString, to: &OsString) -> ResultReadBothFiles { + let mut read_errors = Vec::new(); + let from_content = match read_file_contents(from).map_err(|e| (from.clone(), e)) { + Ok(r) => r, + Err(e) => { + read_errors.push(e); + Vec::new() + } + }; + let to_content = match read_file_contents(to).map_err(|e| (to.clone(), e)) { + Ok(r) => r, + Err(e) => { + read_errors.push(e); + Vec::new() + } + }; + + if read_errors.is_empty() { + Ok((from_content, to_content)) + } else { + Err(read_errors) + } } #[cfg(test)] @@ -189,13 +228,13 @@ mod tests { let res = read_both_files(&non_exist_file_path, &exist_file_path); assert!(res.is_err()); - let (err_path, _) = res.unwrap_err(); - assert_eq!(err_path, non_exist_file_path); + let err_path = res.unwrap_err(); + assert_eq!(err_path[0].0, non_exist_file_path); let res = read_both_files(&exist_file_path, &non_exist_file_path); assert!(res.is_err()); - let (err_path, _) = res.unwrap_err(); - assert_eq!(err_path, non_exist_file_path); + let err_path = res.unwrap_err(); + assert_eq!(err_path[0].0, non_exist_file_path); } } diff --git a/tests/integration.rs b/tests/integration.rs index 3935181..236f9df 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -79,18 +79,22 @@ mod common { ": {}: {error_message}\n", &nopath.as_os_str().to_string_lossy() ))); - } - let mut cmd = cargo_bin_cmd!("diffutils"); - cmd.arg("diff"); - cmd.arg(&nopath).arg(&nopath); - cmd.assert() - .code(predicate::eq(2)) - .failure() - .stderr(predicate::str::contains(format!( - ": {}: {error_message}\n", - &nopath.as_os_str().to_string_lossy() - ))); + // TODO test fails for cmp + if subcmd == "cmp" { + continue; + } + let mut cmd = cargo_bin_cmd!("diffutils"); + cmd.arg(subcmd); + cmd.arg(&nopath).arg(&nopath); + cmd.assert().code(predicate::eq(2)).failure().stderr( + predicate::str::contains(format!( + ": {}: {error_message}\n", + &nopath.as_os_str().to_string_lossy() + )) + .count(2), + ); + } Ok(()) }