diff --git a/src/params.rs b/src/params.rs index 239ebca..9325a1f 100644 --- a/src/params.rs +++ b/src/params.rs @@ -79,37 +79,40 @@ pub fn parse_params>(mut opts: Peekable) -> Resu } continue; } - if param == "-s" || param == "--report-identical-files" { + // A word of short options taking no argument, such as `-qst`. Other + // words are left whole for legacy numeric options and their diagnostics. + if let Some(short) = param + .to_str() + .and_then(|arg| arg.strip_prefix('-')) + .filter(|short| !short.is_empty() && short.bytes().all(|b| SHORT_OPTIONS.contains(&b))) + { + for option in short.bytes() { + apply_short_option(option, &mut params, &mut 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 +159,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 +177,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 +250,42 @@ 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(()) +} + +/// The short options that take no argument, and can be grouped. +const SHORT_OPTIONS: &[u8] = b"ceqstuy"; + +fn apply_short_option( + option: u8, + params: &mut Params, + format: &mut Option, +) -> Result<(), String> { + 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)?, + // Callers only pass bytes of SHORT_OPTIONS. + _ => unreachable!("not a short option: {}", char::from(option)), + } + Ok(()) +} + struct DiffStyleMatch { is_match: bool, context_count: Option, @@ -258,7 +295,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 +303,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 +337,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 +345,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 +752,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 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 +1056,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/by-util/test_diff.rs b/tests/by-util/test_diff.rs index fbdd494..70f997d 100644 --- a/tests/by-util/test_diff.rs +++ b/tests/by-util/test_diff.rs @@ -118,6 +118,63 @@ fn differences_brief() -> Result<(), Box> { Ok(()) } +#[test] +fn short_option_clusters() -> Result<(), Box> { + let mut left = NamedTempFile::new()?; + left.write_all(b"one\ttwo\n")?; + let mut right = NamedTempFile::new()?; + right.write_all(b"one two\n")?; + let mut same = NamedTempFile::new()?; + same.write_all(b"one\ttwo\n")?; + + let mut cmd = cargo_bin_cmd!("diffutils"); + cmd.arg("diff") + .arg("-qs") + .arg(left.path()) + .arg(right.path()); + cmd.assert().code(predicate::eq(1)).stdout(format!( + "Files {} and {} differ\n", + left.path().to_string_lossy(), + right.path().to_string_lossy() + )); + + let mut cmd = cargo_bin_cmd!("diffutils"); + cmd.arg("diff").arg("-sq").arg(left.path()).arg(same.path()); + cmd.assert().code(predicate::eq(0)).stdout(format!( + "Files {} and {} are identical\n", + left.path().to_string_lossy(), + same.path().to_string_lossy() + )); + + let mut cmd = cargo_bin_cmd!("diffutils"); + cmd.arg("diff") + .arg("-te") + .arg(right.path()) + .arg(left.path()); + cmd.assert() + .code(predicate::eq(1)) + .stdout("1c\none two\n.\n"); + + let mut cmd = cargo_bin_cmd!("diffutils"); + cmd.arg("diff") + .arg("-ue") + .arg(left.path()) + .arg(right.path()); + cmd.assert() + .code(predicate::eq(2)) + .stderr(predicate::str::contains("Conflicting output style options")); + + let mut cmd = cargo_bin_cmd!("diffutils"); + cmd.arg("diff") + .arg("-qz") + .arg(left.path()) + .arg(right.path()); + cmd.assert() + .code(predicate::eq(2)) + .stderr(predicate::str::contains("unrecognized option '-qz'")); + Ok(()) +} + #[test] fn missing_newline() -> Result<(), Box> { let mut file1 = NamedTempFile::new()?;