From 385935e4fc18a92cb6549e6157dce39258ebfbe0 Mon Sep 17 00:00:00 2001 From: Sichen Date: Tue, 13 Oct 2026 14:45:34 +0000 Subject: [PATCH] diff: parse clusters of supported short options Handle the seven short options that take no argument through the same path in their separate and clustered forms. Keeping the existing fallback when a word contains an unsupported option, and share the output-style conflict checks between both forms. Add parser and command-output tests for clustered options. --- src/params.rs | 172 +++++++++++++++++++++++++++++++++++-------- tests/integration.rs | 74 +++++++++++++++++++ 2 files changed, 217 insertions(+), 29 deletions(-) diff --git a/src/params.rs b/src/params.rs index 239ebca..686e47b 100644 --- a/src/params.rs +++ b/src/params.rs @@ -79,37 +79,58 @@ pub fn parse_params>(mut opts: Peekable) -> Resu } continue; } - if param == "-s" || param == "--report-identical-files" { + if let Some(short) = param + .to_str() + .and_then(|arg| arg.strip_prefix('-')) + .filter(|arg| !arg.starts_with('-')) + { + if short.len() == 1 { + if apply_short_option(short.as_bytes()[0], &mut params, &mut format)? { + continue; + } + } else if short.len() > 1 { + // Leave mixed words whole for legacy numeric options and their diagnostics. + let all_recognized = short.bytes().all(|option| { + let mut isolated_format = None; + matches!( + apply_short_option(option, &mut Params::default(), &mut isolated_format), + Ok(true) + ) + }); + if all_recognized { + let mut clustered_params = params.clone(); + let mut clustered_format = format; + for option in short.bytes() { + apply_short_option(option, &mut clustered_params, &mut clustered_format)?; + } + params = clustered_params; + format = clustered_format; + continue; + } + } + } + if param == "--report-identical-files" { params.report_identical_files = true; continue; } - if param == "-q" || param == "--brief" { + if param == "--brief" { params.brief = true; continue; } - if param == "-t" || param == "--expand-tabs" { + if param == "--expand-tabs" { params.expand_tabs = true; continue; } if param == "--normal" { - if format.is_some() && format != Some(Format::Normal) { - return Err("Conflicting output style options".to_string()); - } - format = Some(Format::Normal); + set_format(&mut format, Format::Normal)?; continue; } - if param == "-e" || param == "--ed" { - if format.is_some() && format != Some(Format::Ed) { - return Err("Conflicting output style options".to_string()); - } - format = Some(Format::Ed); + if param == "--ed" { + set_format(&mut format, Format::Ed)?; continue; } - if param == "-y" || param == "--side-by-side" { - if format.is_some() && format != Some(Format::SideBySide) { - return Err("Conflicting output style option".to_string()); - } - format = Some(Format::SideBySide); + if param == "--side-by-side" { + set_format(&mut format, Format::SideBySide)?; continue; } if width_re.is_match(param.to_string_lossy().as_ref()) { @@ -156,14 +177,13 @@ pub fn parse_params>(mut opts: Peekable) -> Resu continue; } - match match_context_diff_params(¶m, next_param, format) { + match match_context_diff_params(¶m, next_param, &mut format) { Ok(DiffStyleMatch { is_match, context_count, next_param_consumed, }) => { if is_match { - format = Some(Format::Context); if context_count.is_some() { context = context_count; } @@ -175,14 +195,13 @@ pub fn parse_params>(mut opts: Peekable) -> Resu } Err(error) => return Err(error), } - match match_unified_diff_params(¶m, next_param, format) { + match match_unified_diff_params(¶m, next_param, &mut format) { Ok(DiffStyleMatch { is_match, context_count, next_param_consumed, }) => { if is_match { - format = Some(Format::Unified); if context_count.is_some() { context = context_count; } @@ -249,6 +268,38 @@ pub fn parse_params>(mut opts: Peekable) -> Resu Ok(params) } +fn set_format(format: &mut Option, requested: Format) -> Result<(), String> { + if format.is_some() && *format != Some(requested) { + // Preserve the singular side-by-side diagnostic for compatibility. + let message = if requested == Format::SideBySide { + "Conflicting output style option" + } else { + "Conflicting output style options" + }; + return Err(message.to_string()); + } + *format = Some(requested); + Ok(()) +} + +fn apply_short_option( + option: u8, + params: &mut Params, + format: &mut Option, +) -> Result { + match option { + b'c' => set_format(format, Format::Context)?, + b'e' => set_format(format, Format::Ed)?, + b'q' => params.brief = true, + b's' => params.report_identical_files = true, + b't' => params.expand_tabs = true, + b'u' => set_format(format, Format::Unified)?, + b'y' => set_format(format, Format::SideBySide)?, + _ => return Ok(false), + } + Ok(true) +} + struct DiffStyleMatch { is_match: bool, context_count: Option, @@ -258,7 +309,7 @@ struct DiffStyleMatch { fn match_context_diff_params( param: &OsString, next_param: Option<&OsString>, - format: Option, + format: &mut Option, ) -> Result { const CONTEXT_RE: &str = r"^(-[cC](?\d*)|--context(=(?\d*))?|-(?\d+)c)$"; let regex = Regex::new(CONTEXT_RE).unwrap(); @@ -266,9 +317,7 @@ fn match_context_diff_params( let mut context_count = None; let mut next_param_consumed = false; if is_match { - if format.is_some() && format != Some(Format::Context) { - return Err("Conflicting output style options".to_string()); - } + set_format(format, Format::Context)?; let captures = regex.captures(param.to_str().unwrap()).unwrap(); let num = captures .name("num1") @@ -302,7 +351,7 @@ fn match_context_diff_params( fn match_unified_diff_params( param: &OsString, next_param: Option<&OsString>, - format: Option, + format: &mut Option, ) -> Result { const UNIFIED_RE: &str = r"^(-[uU](?\d*)|--unified(=(?\d*))?|-(?\d+)u)$"; let regex = Regex::new(UNIFIED_RE).unwrap(); @@ -310,9 +359,7 @@ fn match_unified_diff_params( let mut context_count = None; let mut next_param_consumed = false; if is_match { - if format.is_some() && format != Some(Format::Unified) { - return Err("Conflicting output style options".to_string()); - } + set_format(format, Format::Unified)?; let captures = regex.captures(param.to_str().unwrap()).unwrap(); let num = captures .name("num1") @@ -719,6 +766,64 @@ mod tests { } } #[test] + fn short_option_clusters() { + for (cluster, format, brief, report_identical_files, expand_tabs) in [ + ("-cc", Format::Context, false, false, false), + ("-ee", Format::Ed, false, false, false), + ("-qq", Format::Normal, true, false, false), + ("-ss", Format::Normal, false, true, false), + ("-tt", Format::Normal, false, false, true), + ("-uu", Format::Unified, false, false, false), + ("-yy", Format::SideBySide, false, false, false), + ("-cqt", Format::Context, true, false, true), + ("-eqs", Format::Ed, true, true, false), + ("-qst", Format::Normal, true, true, true), + ("-uqt", Format::Unified, true, false, true), + ("-yqs", Format::SideBySide, true, true, false), + ] { + assert_eq!( + Ok(Params { + executable: os("diff"), + from: os("foo"), + to: os("bar"), + format, + brief, + report_identical_files, + expand_tabs, + ..Default::default() + }), + parse_params( + [os("diff"), os(cluster), os("foo"), os("bar")] + .iter() + .cloned() + .peekable() + ), + "{cluster}" + ); + } + } + #[test] + fn control_partly_recognized_short_option_clusters() { + for options in [ + vec!["-qg"], + vec!["-u", "-cX"], + vec!["-u", "-qcX"], + vec!["-c", "-uX"], + ] { + let unknown = options.last().unwrap().to_string(); + let args = ["diff"] + .into_iter() + .chain(options) + .chain(["foo", "bar"]) + .map(os) + .collect::>(); + assert_eq!( + Err(format!("unrecognized option '{unknown}'")), + parse_params(args.into_iter().peekable()) + ); + } + } + #[test] fn tabsize() { assert_eq!( Ok(Params { @@ -965,5 +1070,14 @@ mod tests { ) .is_err()); } + assert_eq!( + Err("Conflicting output style options".to_string()), + parse_params( + [os("diff"), os("-c"), os("-ut"), os("foo"), os("bar")] + .iter() + .cloned() + .peekable() + ) + ); } } diff --git a/tests/integration.rs b/tests/integration.rs index ad5efd9..058cacb 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -208,6 +208,80 @@ mod diff { Ok(()) } + #[test] + fn short_option_clusters_match_split_options() -> Result<(), Box> { + let mut left = NamedTempFile::new()?; + left.write_all(b"a\tb\n")?; + let mut right = NamedTempFile::new()?; + right.write_all(b"a b\n")?; + + let equivalent = |cluster: &[&str], + split: &[&str], + other: &std::path::Path| + -> Result> { + let run = |options: &[&str]| { + cargo_bin_cmd!("diffutils") + .arg("diff") + .args(options) + .arg(left.path()) + .arg(other) + .output() + }; + let clustered = run(cluster)?; + let separate = run(split)?; + assert_eq!( + clustered.status.code(), + separate.status.code(), + "{cluster:?}" + ); + assert_eq!(clustered.stdout, separate.stdout, "{cluster:?}"); + assert_eq!(clustered.stderr, separate.stderr, "{cluster:?}"); + Ok(clustered) + }; + + for (cluster, split, expected_code) in [ + (&["-cc"][..], &["-c", "-c"][..], Some(1)), + (&["-ee"], &["-e", "-e"], Some(1)), + (&["-qq"], &["-q", "-q"], Some(1)), + (&["-ss"], &["-s", "-s"], Some(1)), + (&["-tt"], &["-t", "-t"], Some(1)), + (&["-uu"], &["-u", "-u"], Some(1)), + (&["-cqt"], &["-c", "-q", "-t"], Some(1)), + (&["-eqs"], &["-e", "-q", "-s"], Some(1)), + (&["-qst"], &["-q", "-s", "-t"], Some(1)), + (&["-uqt"], &["-u", "-q", "-t"], Some(1)), + (&["-cu"], &["-c", "-u"], Some(2)), + (&["-uc"], &["-u", "-c"], Some(2)), + (&["-eu"], &["-e", "-u"], Some(2)), + (&["-c", "-ut"], &["-c", "-u", "-t"], Some(2)), + (&["-ut", "-c"], &["-u", "-t", "-c"], Some(2)), + ] { + let clustered = equivalent(cluster, split, right.path())?; + assert_eq!(clustered.status.code(), expected_code, "{cluster:?}"); + } + + let mut same = NamedTempFile::new()?; + same.write_all(b"a\tb\n")?; + let identical = format!( + "Files {} and {} are identical\n", + left.path().to_string_lossy(), + same.path().to_string_lossy() + ); + for (cluster, split) in [ + (&["-ss"][..], &["-s", "-s"][..]), + (&["-qs"], &["-q", "-s"]), + (&["-ys"], &["-y", "-s"]), + ] { + let clustered = equivalent(cluster, split, same.path())?; + assert_eq!(clustered.status.code(), Some(0), "{cluster:?}"); + assert!( + clustered.stdout.ends_with(identical.as_bytes()), + "{cluster:?}" + ); + } + Ok(()) + } + #[test] fn missing_newline() -> Result<(), Box> { let mut file1 = NamedTempFile::new()?;