diff --git a/src/uu/checksum_common/src/cli.rs b/src/uu/checksum_common/src/cli.rs index a5e979e3057..50beb3f3b5a 100644 --- a/src/uu/checksum_common/src/cli.rs +++ b/src/uu/checksum_common/src/cli.rs @@ -131,6 +131,7 @@ impl ChecksumCommand for Command { .long(options::BINARY) .short('b') .hide(true) + .overrides_with(options::TEXT) .action(ArgAction::SetTrue), ) } @@ -169,6 +170,7 @@ impl ChecksumCommand for Command { Arg::new(options::UNTAGGED) .long(options::UNTAGGED) .help(translate!("ck-common-help-untagged")) + .overrides_with(options::TAG) .action(ArgAction::SetTrue), ) } diff --git a/src/uu/checksum_common/src/lib.rs b/src/uu/checksum_common/src/lib.rs index 4de122d7e12..eea6a7b4879 100644 --- a/src/uu/checksum_common/src/lib.rs +++ b/src/uu/checksum_common/src/lib.rs @@ -74,7 +74,10 @@ pub fn standalone_with_length_main( .transpose()? .flatten(); - let format = OutputFormat::from_standalone(std::env::args_os()); + //todo: deduplicate matches.get_flag + let text = !matches.get_flag(options::BINARY); + let tag = matches.get_flag(options::TAG); + let format = OutputFormat::from_standalone(text, tag); checksum_main(algo, length, matches, format?) } @@ -83,8 +86,10 @@ pub fn standalone_with_length_main( pub fn standalone_main(algo: AlgoKind, cmd: Command, args: impl uucore::Args) -> UResult<()> { let matches = uucore::clap_localization::handle_clap_result(cmd, args)?; let algo = Some(algo); - - let format = OutputFormat::from_standalone(std::env::args_os()); + //todo: deduplicate matches.get_flag + let text = !matches.get_flag(options::BINARY); + let tag = matches.get_flag(options::TAG); + let format = OutputFormat::from_standalone(text, tag); checksum_main(algo, None, matches, format?) } @@ -155,6 +160,9 @@ pub fn checksum_main( let quiet = check_flag("quiet")?; let strict = check_flag("strict")?; let status = check_flag("status")?; + let text_flag = matches.get_flag(options::TEXT); + let binary_flag = matches.get_flag(options::BINARY); + let tag = matches.get_flag(options::TAG); // clap provides the default value -. So we unwrap() safety. let files = matches @@ -162,17 +170,20 @@ pub fn checksum_main( .unwrap() .map(Borrow::borrow); + if text_flag && tag { + return Err(ChecksumError::TextAfterTag.into()); + } + if check { // cksum does not support '--check'ing legacy algorithms if algo.is_some_and(AlgoKind::is_legacy) { return Err(ChecksumError::AlgorithmNotSupportedWithCheck.into()); } - - let text_flag = matches.get_flag(options::TEXT); - let binary_flag = matches.get_flag(options::BINARY); - let tag = matches.get_flag(options::TAG); - - if tag || binary_flag || text_flag { + // Maybe, we should just use clap + if tag { + return Err(ChecksumError::TagCheck.into()); + } + if binary_flag || text_flag { return Err(ChecksumError::BinaryTextConflict.into()); } diff --git a/src/uu/cksum/src/cksum.rs b/src/uu/cksum/src/cksum.rs index 51dd83d3a27..1e638d8bcdb 100644 --- a/src/uu/cksum/src/cksum.rs +++ b/src/uu/cksum/src/cksum.rs @@ -5,7 +5,6 @@ // spell-checker:ignore (ToDO) fname, algo, bitlen -use std::ffi::OsStr; use std::io::{Write, stderr}; use clap::Command; @@ -44,48 +43,6 @@ fn print_cpu_debug_info() { } } -/// cksum has a bunch of legacy behavior. We handle this in this function to -/// make sure they are self contained and "easier" to understand. -/// -/// Returns a pair of boolean. The first one indicates if we should use tagged -/// output format, the second one indicates if we should use the binary flag in -/// the untagged case. -fn handle_tag_text_binary_flags>( - args: impl Iterator, -) -> UResult<(bool, bool)> { - let mut tag = true; - let mut binary = false; - let mut text = false; - - // --binary, --tag and --untagged are tight together: none of them - // conflicts with each other but --tag will reset "binary" and "text" and - // set "tag". - - for arg in args { - let arg = arg.as_ref(); - if arg == "-b" || arg == "--binary" { - text = false; - binary = true; - } else if arg == "--text" { - text = true; - binary = false; - } else if arg == "--tag" { - tag = true; - binary = false; - text = false; - } else if arg == "--untagged" { - tag = false; - } - } - - // Specifying --text without ever mentioning --untagged fails. - if text && tag { - return Err(ChecksumError::TextWithoutUntagged.into()); - } - - Ok((tag, binary)) -} - /// Sanitize the `--length` argument depending on `--algorithm` and `--length`. fn maybe_sanitize_length( algo_cli: Option, @@ -134,8 +91,14 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { .map(String::as_str); let length = maybe_sanitize_length(algo_cli, input_length)?; + let tag = !matches.get_flag(options::UNTAGGED); + let binary = matches.get_flag(options::BINARY); + let text = matches.get_flag(options::TEXT); - let (tag, binary) = handle_tag_text_binary_flags(std::env::args_os())?; + //Specifying --text without ever mentioning --untagged fails. + if text && tag { + return Err(ChecksumError::TextWithoutUntagged.into()); + } let output_format = OutputFormat::from_cksum( algo_cli.unwrap_or(AlgoKind::Crc), diff --git a/src/uucore/src/lib/features/checksum/compute.rs b/src/uucore/src/lib/features/checksum/compute.rs index 4a51f81d790..317af63ef78 100644 --- a/src/uucore/src/lib/features/checksum/compute.rs +++ b/src/uucore/src/lib/features/checksum/compute.rs @@ -5,7 +5,7 @@ // spell-checker:ignore bitlen -use std::ffi::{OsStr, OsString}; +use std::ffi::OsStr; use std::fs::File; use std::io::{self, BufReader, Read, Write}; use std::path::Path; @@ -115,27 +115,8 @@ impl OutputFormat { /// /// Since standalone utils can't use the Raw or Legacy output format, it is /// decided only using the --tag, --binary and --text arguments. - pub fn from_standalone(args: impl Iterator) -> UResult { - let mut text = true; - let mut tag = false; - - for arg in args { - if arg == "--" { - break; - } else if arg == "--tag" { - tag = true; - text = false; - } else if arg == "--binary" || arg == "-b" { - text = false; - } else if arg == "--text" || arg == "-t" { - // Finding a `--text` after `--tag` is an error. - if tag { - return Err(ChecksumError::TextAfterTag.into()); - } - text = true; - } - } - + #[allow(clippy::unnecessary_wraps)] + pub fn from_standalone(text: bool, tag: bool) -> UResult { if tag { Ok(Self::Tagged(DigestFormat::Hexadecimal)) } else { diff --git a/src/uucore/src/lib/features/checksum/mod.rs b/src/uucore/src/lib/features/checksum/mod.rs index 13b0680dfcf..6dd18ee9a32 100644 --- a/src/uucore/src/lib/features/checksum/mod.rs +++ b/src/uucore/src/lib/features/checksum/mod.rs @@ -403,6 +403,8 @@ pub enum ChecksumError { BinaryTextConflict, #[error("--text mode is only supported with --untagged")] TextWithoutUntagged, + #[error("the --tag option is meaningless when verifying checksums")] + TagCheck, #[error("--tag does not support --text mode")] TextAfterTag, #[error("--check is not supported with --algorithm={{bsd,sysv,crc,crc32b}}")] diff --git a/tests/by-util/test_cksum.rs b/tests/by-util/test_cksum.rs index fac6ab67965..f375bcb98b0 100644 --- a/tests/by-util/test_cksum.rs +++ b/tests/by-util/test_cksum.rs @@ -1012,13 +1012,13 @@ fn test_reset_binary() { scene .ucmd() - .arg("--binary") // should disappear because of the following option + .arg("--binary") .arg("--tag") .arg("--untagged") .arg("--algorithm=md5") .arg(at.subdir.join("f")) .succeeds() - .stdout_contains("d41d8cd98f00b204e9800998ecf8427e "); + .stdout_contains("d41d8cd98f00b204e9800998ecf8427e *"); } #[test] @@ -1040,7 +1040,6 @@ fn test_reset_binary_but_set() { .stdout_contains("d41d8cd98f00b204e9800998ecf8427e *"); } -/// Test legacy behaviors with --tag, --untagged, --binary and --text mod output_format { use super::*; @@ -1049,13 +1048,11 @@ mod output_format { let (at, mut ucmd) = at_and_ucmd!(); at.touch("f"); - ucmd.arg("--text") // should disappear because of the following option + ucmd.arg("--text") .arg("--tag") .args(&["-a", "md5"]) .arg(at.subdir.join("f")) - .succeeds() - // Tagged output is used - .stdout_contains("f) = d41d8cd98f00b204e9800998ecf8427e"); + .fails(); } #[test] @@ -1181,7 +1178,7 @@ fn test_binary_file() { .arg("--untagged") .arg("lorem_ipsum.txt") .succeeds() - .stdout_is("cd724690f7dc61775dfac400a71f2caa lorem_ipsum.txt\n"); + .stdout_is("cd724690f7dc61775dfac400a71f2caa *lorem_ipsum.txt\n"); } #[test] @@ -1229,9 +1226,7 @@ fn test_conflicting_options() { .arg("md5") .fails_with_code(1) .no_stdout() - .stderr_contains( - "cksum: the --binary and --text options are meaningless when verifying checksums", - ); + .stderr_contains("cksum: the --tag option is meaningless when verifying checksums"); } #[test]