diff --git a/src/cmp.rs b/src/cmp.rs index 0f7250c0..3ff716f0 100644 --- a/src/cmp.rs +++ b/src/cmp.rs @@ -74,7 +74,8 @@ pub fn parse_params>(mut opts: Peekable) -> Resu let executable = opts.next().ok_or("Usage: ".to_string())?; let executable_str = executable.to_string_lossy().to_string(); - let parse_skip = |param: &str, skip_desc: &str| -> Result { + // Sizes for --ignore-initial and --bytes, with the same unit suffixes. + let parse_size = |param: &str, option: &str, desc: &str| -> Result { let suffix_start = param .find(|b: char| !b.is_ascii_digit()) .unwrap_or(param.len()); @@ -83,7 +84,7 @@ pub fn parse_params>(mut opts: Peekable) -> Resu Err(e) if *e.kind() == std::num::IntErrorKind::PosOverflow => SkipU64::MAX, Err(_) => { return Err(format!( - "{executable_str}: invalid --ignore-initial value '{skip_desc}'" + "{executable_str}: invalid --{option} value '{desc}'" )) } }; @@ -111,7 +112,7 @@ pub fn parse_params>(mut opts: Peekable) -> Resu "Y" => SkipU64::MAX, // 1_208_925_819_614_629_174_706_176, _ => { return Err(format!( - "{executable_str}: invalid --ignore-initial value '{skip_desc}'" + "{executable_str}: invalid --{option} value '{desc}'" )); } }; @@ -172,17 +173,7 @@ pub fn parse_params>(mut opts: Peekable) -> Resu let (_, arg) = param_str.split_once('=').unwrap(); arg.to_string() }; - let max_bytes = match max_bytes.parse::() { - Ok(num) => num, - // TODO limit to MAX is dangerous, this should become an error like in GNU cmp. - Err(e) if *e.kind() == std::num::IntErrorKind::PosOverflow => BytesLimitU64::MAX, - Err(_) => { - return Err(format!( - "{executable_str}: invalid --bytes value '{max_bytes}'" - )) - } - }; - params.max_bytes = Some(max_bytes); + params.max_bytes = Some(parse_size(&max_bytes, "bytes", &max_bytes)?); continue; } if param == "-i" || param_str.starts_with("--ignore-initial=") { @@ -197,11 +188,11 @@ pub fn parse_params>(mut opts: Peekable) -> Resu }; let (skip_a, skip_b) = if let Some((skip_a, skip_b)) = skip_desc.split_once(':') { ( - parse_skip(skip_a, &skip_desc)?, - parse_skip(skip_b, &skip_desc)?, + parse_size(skip_a, "ignore-initial", &skip_desc)?, + parse_size(skip_b, "ignore-initial", &skip_desc)?, ) } else { - let skip = parse_skip(&skip_desc, &skip_desc)?; + let skip = parse_size(&skip_desc, "ignore-initial", &skip_desc)?; (skip, skip) }; params.skip_a = Some(skip_a); @@ -224,9 +215,9 @@ pub fn parse_params>(mut opts: Peekable) -> Resu } else if to.is_none() { to = Some(param); } else if skip_pos1.is_none() { - skip_pos1 = Some(parse_skip(¶m_str, ¶m_str)?); + skip_pos1 = Some(parse_size(¶m_str, "ignore-initial", ¶m_str)?); } else if skip_pos2.is_none() { - skip_pos2 = Some(parse_skip(¶m_str, ¶m_str)?); + skip_pos2 = Some(parse_size(¶m_str, "ignore-initial", ¶m_str)?); } else { return Err(usage_string(&executable_str)); } @@ -268,7 +259,7 @@ pub fn parse_params>(mut opts: Peekable) -> Resu params.skip_a = skip_pos1; } else if let Some(param) = opts.next() { let param_str = param.to_string_lossy().to_string(); - params.skip_a = Some(parse_skip(¶m_str, ¶m_str)?); + params.skip_a = Some(parse_size(¶m_str, "ignore-initial", ¶m_str)?); } }; if params.skip_b.is_none() { @@ -276,7 +267,7 @@ pub fn parse_params>(mut opts: Peekable) -> Resu params.skip_b = skip_pos2; } else if let Some(param) = opts.next() { let param_str = param.to_string_lossy().to_string(); - params.skip_b = Some(parse_skip(¶m_str, ¶m_str)?); + params.skip_b = Some(parse_size(¶m_str, "ignore-initial", ¶m_str)?); } } @@ -985,12 +976,33 @@ mod tests { ) ); + // Same unit suffixes as --ignore-initial + for (args, bytes) in [ + (&["-n", "1K"][..], 1_024), + (&["--bytes=1kB"][..], 1_000), + (&["--bytes=2M"][..], 2_097_152), + ] { + let mut opts = vec![os("cmp")]; + opts.extend(args.iter().map(|arg| os(arg))); + opts.extend([os("foo"), os("bar")]); + assert_eq!( + Ok(Params { + executable: os("cmp"), + from: os("foo"), + to: os("bar"), + max_bytes: Some(bytes), + ..Default::default() + }), + parse_params(opts.into_iter().peekable()), + "{args:?}" + ); + } + // Failure case - // TODO This is actually fine in GNU cmp. --bytes does not have a unit parser yet. assert_eq!( - Err("cmp: invalid --bytes value '1K'".to_string()), + Err("cmp: invalid --bytes value '1x'".to_string()), parse_params( - [os("cmp"), os("--bytes=1K"), os("foo"), os("bar")] + [os("cmp"), os("--bytes=1x"), os("foo"), os("bar")] .iter() .cloned() .peekable() diff --git a/tests/integration.rs b/tests/integration.rs index ad5efd92..28fd8ad9 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -669,6 +669,19 @@ mod cmp { cmd.arg("-n"); cmd.arg("13"); cmd.arg(&a_path).arg(&b_path); + cmd.assert() + .code(predicate::eq(1)) + .failure() + .stderr(predicate::str::is_empty()) + .stdout(predicate::eq(" 4 40 144 d\n 8 40 150 h\n")); + + let mut cmd = cargo_bin_cmd!("diffutils"); + cmd.arg("cmp"); + cmd.arg("-l"); + cmd.arg("-b"); + cmd.arg("-n"); + cmd.arg("1K"); + cmd.arg(&a_path).arg(&b_path); cmd.assert() .code(predicate::eq(1)) .failure()