diff --git a/src/bin/coreutils.rs b/src/bin/coreutils.rs index 59634849a6b..094738ad915 100644 --- a/src/bin/coreutils.rs +++ b/src/bin/coreutils.rs @@ -38,6 +38,24 @@ fn usage(utils: &UtilityMap, name: &str) { ); } +/// Entry into Coreutils +/// +/// # Arguments +/// * first arg needs to be the binary/executable. \ +/// This is usually coreutils, but can be the util name itself, e.g. 'ls'. \ +/// The util name will be checked against the list of enabled utils, where +/// * the name exactly matches the name of an applet/util or +/// * the name matches pattern, e.g. +/// 'my_own_directory_service_ls' as long as the last letters match the utility. +/// * coreutils arg: --list, --version, -V, --help, -h (or shortened long versions): \ +/// Output information about coreutils itself. \ +/// Multiple of these arguments, output limited to one, with help > version > list. +/// * util name and any number of arguments: \ +/// Will get passed on to the selected utility. \ +/// Error if util name is not recognized. +/// * --help or -h and a following util name: \ +/// Output help for that specific utility. \ +/// So 'coreutils sum --help' is the same as 'coreutils --help sum'. #[allow(clippy::cognitive_complexity)] fn main() { uucore::panic::mute_sigpipe_panic(); @@ -74,27 +92,7 @@ fn main() { validation::not_found(&util_os) }; - match util { - "--list" => { - // If --help is also present, show usage instead of list - if args.any(|arg| arg == "--help" || arg == "-h") { - usage(&utils, binary_as_util); - process::exit(0); - } - let utils: Vec<_> = utils.keys().collect(); - for util in utils { - println!("{util}"); - } - process::exit(0); - } - "--version" | "-V" => { - println!("{binary_as_util} {VERSION} (multi-call binary)"); - process::exit(0); - } - // Not a special command: fallthrough to calling a util - _ => {} - } - + #[allow(clippy::single_match_else)] match utils.get(util) { Some(&(uumain, _)) => { // TODO: plug the deactivation of the translation @@ -106,33 +104,40 @@ fn main() { process::exit(uumain(vec![util_os].into_iter().chain(args))); } None => { - if util == "--help" || util == "-h" { - // see if they want help on a specific util - if let Some(util_os) = args.next() { - let Some(util) = util_os.to_str() else { - validation::not_found(&util_os) - }; - - match utils.get(util) { + let (option, help_util) = find_dominant_option(&util_os, &mut args); + match option { + SelectedOption::Help => match help_util { + // see if they want help on a specific util and if it is valid + Some(u_os) => match utils.get(&u_os.to_string_lossy()) { Some(&(uumain, _)) => { let code = uumain( - vec![util_os, OsString::from("--help")] + vec![u_os, OsString::from("--help")] .into_iter() + // Function requires a chain like in the Some case, but + // the args are discarded as clap returns help immediately. .chain(args), ); io::stdout().flush().expect("could not flush stdout"); process::exit(code); } - None => validation::not_found(&util_os), + None => validation::not_found(&u_os), + }, + // show coreutils help + None => usage(&utils, binary_as_util), + }, + SelectedOption::Version => { + println!("{binary_as_util} {VERSION} (multi-call binary)"); + } + SelectedOption::List => { + let utils: Vec<_> = utils.keys().collect(); + for util in utils { + println!("{util}"); } } - usage(&utils, binary_as_util); - process::exit(0); - } else if util.starts_with('-') { - // Argument looks like an option but wasn't recognized - validation::unrecognized_option(binary_as_util, &util_os); - } else { - validation::not_found(&util_os); + SelectedOption::Unrecognized(arg) => { + // Argument looks like an option but wasn't recognized + validation::unrecognized_option(binary_as_util, &arg); + } } } } @@ -142,3 +147,91 @@ fn main() { process::exit(0); } } + +/// All defined coreutils options. +// Important: when changing then adapt also [identify_option_from_partial_text] +// as it works with the indices of this array. +const COREUTILS_OPTIONS: [&str; 5] = ["--help", "--list", "--version", "-h", "-V"]; + +/// The dominant selected option. +#[derive(Debug, Clone, PartialEq)] +enum SelectedOption { + Help, + Version, + List, + Unrecognized(OsString), +} + +/// Coreutils only accepts one single option, +/// if multiple are given, use the most dominant one. +/// +/// Help > Version > List (e.g. 'coreutils --list --version' will return version) +/// Unrecognized will return immediately. +/// +/// # Returns +/// (SelectedOption, Util for help request, if any) +fn find_dominant_option( + first_arg: &OsString, + args: &mut impl Iterator, +) -> (SelectedOption, Option) { + let mut sel = identify_option_from_partial_text(first_arg); + match sel { + SelectedOption::Help => return (SelectedOption::Help, args.next()), + SelectedOption::Unrecognized(_) => { + return (sel, None); + } + _ => {} + } + // check remaining options, allows multiple + while let Some(arg) = args.next() { + let so = identify_option_from_partial_text(&arg); + match so { + // most dominant, return directly + SelectedOption::Help => { + // if help is wanted, check if a tool was named + return (so, args.next()); + } + // best after help, can be set directly + SelectedOption::Version => sel = SelectedOption::Version, + SelectedOption::List => { + if sel != SelectedOption::Version { + sel = SelectedOption::List; + } + } + // unrecognized is not allowed + SelectedOption::Unrecognized(_) => { + return (so, None); + } + } + } + + (sel, None) +} + +// Will identify one, SelectedOption::None cannot be returned. +fn identify_option_from_partial_text(arg: &OsString) -> SelectedOption { + let mut option = &arg.to_string_lossy()[..]; + if let Some(p) = option.find('=') { + option = &option[0..p]; + } + let l = option.len(); + let possible_opts: Vec = COREUTILS_OPTIONS + .iter() + .enumerate() + .filter(|(_, it)| it.len() >= l && &it[0..l] == option) + .map(|(id, _)| id) + .collect(); + + match possible_opts.len() { + // exactly one hit + 1 => match &possible_opts[0] { + // number represents index of [COREUTILS_OPTIONS] + 0 | 3 => SelectedOption::Help, + 1 => SelectedOption::List, + 2 | 4 => SelectedOption::Version, + _ => SelectedOption::Help, + }, + // None or more hits. The latter can not happen with the allowed options. + _ => SelectedOption::Unrecognized(arg.clone()), + } +} diff --git a/src/uucore/locales/en-US.ftl b/src/uucore/locales/en-US.ftl index 15896209526..03fdcd50834 100644 --- a/src/uucore/locales/en-US.ftl +++ b/src/uucore/locales/en-US.ftl @@ -19,6 +19,9 @@ clap-error-missing-required-arguments = { $error_word }: the following required clap-error-possible-values = possible values clap-error-help-suggestion = For more information, try '{ $command } --help'. common-help-suggestion = For more information, try '--help'. +# For clap_localization +clap-error-ambiguous-argument=Error: Argument '{ $arg }' is ambiguous. + Did you mean one of these? # Common help text patterns help-flag-help = Print help information diff --git a/src/uucore/locales/fr-FR.ftl b/src/uucore/locales/fr-FR.ftl index e9ff4abe475..c94bd4138b2 100644 --- a/src/uucore/locales/fr-FR.ftl +++ b/src/uucore/locales/fr-FR.ftl @@ -19,6 +19,9 @@ clap-error-missing-required-arguments = { $error_word } : les arguments requis s clap-error-possible-values = valeurs possibles clap-error-help-suggestion = Pour plus d'informations, essayez '{ $command } --help'. common-help-suggestion = Pour plus d'informations, essayez '--help'. +# For clap_localization +clap-error-ambiguous-argument=Error: L'argument '{ $arg }' est ambigu. + Tu parlais d'un de ceux-ci? # Modèles de texte d'aide communs help-flag-help = Afficher les informations d'aide diff --git a/src/uucore/src/lib/mods/clap_localization.rs b/src/uucore/src/lib/mods/clap_localization.rs index fc4f838c216..7ad969a760d 100644 --- a/src/uucore/src/lib/mods/clap_localization.rs +++ b/src/uucore/src/lib/mods/clap_localization.rs @@ -432,7 +432,7 @@ where /// let result = handle_clap_result_with_exit_code(cmd, args, 125); /// ``` pub fn handle_clap_result_with_exit_code( - cmd: Command, + mut cmd: Command, itr: I, exit_code: i32, ) -> UResult @@ -440,10 +440,47 @@ where I: IntoIterator, T: Into + Clone, { - cmd.try_get_matches_from(itr).map_err(|e| { + // cloning args for double use in error case + let args = itr.into_iter().collect::>(); + let itr = args.clone(); + // using mut to avoid cloning cmd + cmd.try_get_matches_from_mut(itr).map_err(|e| { if e.exit_code() == 0 { e.into() // Preserve help/version } else { + if e.kind() == ErrorKind::UnknownArgument || e.kind() == ErrorKind::InvalidSubcommand { + // find ambiguous options + // Find the string the user actually typed (e.g., "--de") + // for arg in &itr {} + let args_str: Vec = args + .into_iter() + .map(|t| { + let o: OsString = t.into(); + o.to_string_lossy().to_string() + }) + .collect(); + if let Some(provided) = args_str.iter().find(|a| a.starts_with("--")) { + let search_term = provided.trim_start_matches("--"); + + // Manually filter all defined long arguments + let mut matches: Vec<_> = cmd + .get_arguments() + .filter_map(|arg| arg.get_long()) + .filter(|l| l.starts_with(search_term)) + .collect(); + + if matches.len() > 1 { + let mut msg = + translate!("clap-error-ambiguous-argument", "arg" => provided); + matches.sort(); + for m in matches { + msg.push_str(&format!("\n --{}", m)); + } + return USimpleError::new(exit_code, msg); + } + } + } + let formatter = ErrorFormatter::new(crate::util_name()); let code = formatter.print_error(&e, exit_code); USimpleError::new(code, "") diff --git a/tests/test_util_name.rs b/tests/test_util_name.rs index 17b9dc36e5e..8b6d26d3c99 100644 --- a/tests/test_util_name.rs +++ b/tests/test_util_name.rs @@ -192,7 +192,7 @@ fn util_version() { println!("Skipping test: Binary not found at {:?}", scenario.bin_path); return; } - for arg in ["-V", "--version"] { + for arg in ["-V", "--version", "--ver"] { let child = Command::new(&scenario.bin_path) .arg(arg) .stdin(Stdio::piped()) @@ -209,6 +209,56 @@ fn util_version() { } } +#[test] +fn util_help() { + use std::process::{Command, Stdio}; + + let scenario = TestScenario::new("--version"); + if !scenario.bin_path.exists() { + println!("Skipping test: Binary not found at {:?}", scenario.bin_path); + return; + } + for arg in ["-h", "--help", "--he"] { + let child = Command::new(&scenario.bin_path) + .arg(arg) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + let output = child.wait_with_output().unwrap(); + assert_eq!(output.status.code(), Some(0)); + assert_eq!(output.stderr, b""); + let output_str = String::from_utf8(output.stdout).unwrap(); + assert!(output_str.contains("Usage: coreutils")); + assert!(output_str.contains("lists all defined functions")); + } +} + +#[test] +fn util_arg_priority() { + use std::process::{Command, Stdio}; + + let scenario = TestScenario::new("--version"); + if !scenario.bin_path.exists() { + println!("Skipping test: Binary not found at {:?}", scenario.bin_path); + return; + } + let child = Command::new(&scenario.bin_path) + .arg("--list") + .arg("--version") + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + let output = child.wait_with_output().unwrap(); + assert_eq!(output.status.code(), Some(0)); + assert_eq!(output.stderr, b""); + let output_str = String::from_utf8(output.stdout).unwrap(); + let ver = env::var("CARGO_PKG_VERSION").unwrap(); + assert_eq!(format!("coreutils {ver} (multi-call binary)\n"), output_str); +} + #[test] #[cfg(target_env = "musl")] fn test_musl_no_dynamic_deps() {