diff --git a/src/uu/rm/locales/en-US.ftl b/src/uu/rm/locales/en-US.ftl index 38511105394..06246e200fd 100644 --- a/src/uu/rm/locales/en-US.ftl +++ b/src/uu/rm/locales/en-US.ftl @@ -51,6 +51,14 @@ rm-error-cannot-remove = cannot remove {$file} rm-error-cannot-remove-changed = cannot remove {$file}: File changed while removing rm-error-may-not-abbreviate-no-preserve-root = you may not abbreviate the --no-preserve-root option rm-error-standard-output = standard output: {$error} +rm-error-invalid-interactive-choice = invalid argument '{$arg}' for '--interactive' + Valid arguments are: + {$choices} + Try 'rm --help' for more information. +rm-error-ambiguous-interactive-choice = ambiguous argument '{$arg}' for '--interactive' + Valid arguments are: + {$choices} + Try 'rm --help' for more information. # Verbose messages rm-verbose-removed = removed {$file} diff --git a/src/uu/rm/locales/fr-FR.ftl b/src/uu/rm/locales/fr-FR.ftl index fdbf1275fec..b703a39835f 100644 --- a/src/uu/rm/locales/fr-FR.ftl +++ b/src/uu/rm/locales/fr-FR.ftl @@ -49,6 +49,14 @@ rm-error-cannot-remove = impossible de supprimer {$file} rm-error-cannot-remove-changed = impossible de supprimer {$file} : Le fichier a changé pendant la suppression rm-error-may-not-abbreviate-no-preserve-root = Vous ne pouvez pas abréger l'option --no-preserve-root rm-error-standard-output = sortie standard : {$error} +rm-error-invalid-interactive-choice = argument '{$arg}' invalide pour '--interactive' + Les arguments valides sont : + {$choices} + Essayez 'rm --help' pour plus d'informations. +rm-error-ambiguous-interactive-choice = argument '{$arg}' ambigu pour '--interactive' + Les arguments valides sont : + {$choices} + Essayez 'rm --help' pour plus d'informations. # Messages verbeux rm-verbose-removed = {$file} supprimé diff --git a/src/uu/rm/src/rm.rs b/src/uu/rm/src/rm.rs index f1436b4bd13..dc7b9727e50 100644 --- a/src/uu/rm/src/rm.rs +++ b/src/uu/rm/src/rm.rs @@ -5,7 +5,7 @@ // spell-checker:ignore (path) eacces inacc rm-r4 unlinkat fstatat rootlink -use clap::builder::{PossibleValue, ValueParser}; +use clap::builder::ValueParser; use clap::{Arg, ArgAction, Command, parser::ValueSource}; use indicatif::{ProgressBar, ProgressStyle}; use std::ffi::{OsStr, OsString}; @@ -22,7 +22,6 @@ use std::sync::atomic::{AtomicBool, Ordering}; use thiserror::Error; use uucore::display::Quotable; use uucore::error::{FromIo, UError, UResult, USimpleError, strip_errno}; -use uucore::parser::shortcut_value_parser::ShortcutValueParser; use uucore::quoting_style::{QuotingStyle, locale_aware_escape_name}; use uucore::translate; use uucore::{format_usage, os_str_as_bytes, prompt_yes, show_error}; @@ -144,7 +143,8 @@ pub enum InteractiveMode { PromptProtected, } -// We implement `From` instead of `TryFrom` because clap guarantees that we only receive valid values. +// We implement `From` instead of `TryFrom` because `resolve_interactive_choice` +// guarantees that we only receive valid, canonical values. // // The `PromptProtected` variant is not supposed to be created from a string. impl From<&str> for InteractiveMode { @@ -153,11 +153,72 @@ impl From<&str> for InteractiveMode { "never" => Self::Never, "once" => Self::Once, "always" => Self::Always, - _ => unreachable!("should be prevented by clap"), + _ => unreachable!("should be prevented by resolve_interactive_choice"), } } } +/// The choices `--interactive`'s value accepts, in the order GNU lists +/// them. Each choice's aliases are grouped on a single line in the error +/// message the way GNU's rm does. +const INTERACTIVE_CHOICE_GROUPS: &[(&str, &[&str])] = &[ + ("never", &["no", "none"]), + ("once", &[]), + ("always", &["yes"]), +]; + +/// The canonical name (first element of its group) `value` names among +/// `INTERACTIVE_CHOICE_GROUPS`, accepting any unambiguous abbreviation the +/// way GNU does (including one ambiguous only between aliases of the +/// *same* choice, e.g. 'n' among 'never'/'no'/'none'). +fn resolve_interactive_choice(value: &str) -> UResult<&'static str> { + let list = || { + INTERACTIVE_CHOICE_GROUPS + .iter() + .map(|(canonical, aliases)| { + let names = std::iter::once(*canonical).chain(aliases.iter().copied()); + format!( + " - {}", + names + .map(|name| format!("'{name}'")) + .collect::>() + .join(", ") + ) + }) + .collect::>() + .join("\n") + }; + let matches: Vec<(usize, &'static str)> = INTERACTIVE_CHOICE_GROUPS + .iter() + .enumerate() + .flat_map(|(i, (canonical, aliases))| { + std::iter::once(*canonical) + .chain(aliases.iter().copied()) + .map(move |name| (i, name)) + }) + .filter(|(_, name)| name.starts_with(value)) + .collect(); + + if !value.is_empty() + && let Some(&(group, _)) = matches.iter().find(|(_, name)| *name == value) + { + return Ok(INTERACTIVE_CHOICE_GROUPS[group].0); + } + match matches.first() { + Some(&(group, _)) if !value.is_empty() && matches.iter().all(|(g, _)| *g == group) => { + Ok(INTERACTIVE_CHOICE_GROUPS[group].0) + } + Some(_) => Err(USimpleError::new( + 1, + translate!("rm-error-ambiguous-interactive-choice", "arg" => value.to_string(), "choices" => list()), + )), + None => Err(USimpleError::new( + 1, + translate!("rm-error-invalid-interactive-choice", "arg" => value.to_string(), "choices" => list()), + )), + } +} + /// Options for the `rm` command /// /// All options are public so that the options can be programmatically @@ -278,8 +339,8 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { InteractiveMode::Always } else if matches.get_flag(OPT_PROMPT_ONCE) { InteractiveMode::Once - } else if matches.contains_id(OPT_INTERACTIVE) { - InteractiveMode::from(matches.get_one::(OPT_INTERACTIVE).unwrap().as_str()) + } else if let Some(value) = matches.get_one::(OPT_INTERACTIVE) { + InteractiveMode::from(resolve_interactive_choice(value)?) } else { InteractiveMode::PromptProtected } @@ -398,11 +459,6 @@ pub fn uu_app() -> Command { .long(OPT_INTERACTIVE) .help(translate!("rm-help-interactive")) .value_name("WHEN") - .value_parser(ShortcutValueParser::new([ - PossibleValue::new("always").alias("yes"), - PossibleValue::new("once"), - PossibleValue::new("never").alias("no").alias("none"), - ])) .num_args(0..=1) .require_equals(true) .default_missing_value("always") diff --git a/tests/by-util/test_rm.rs b/tests/by-util/test_rm.rs index c315d4112d5..4e326212214 100644 --- a/tests/by-util/test_rm.rs +++ b/tests/by-util/test_rm.rs @@ -542,6 +542,41 @@ fn test_interactive_never() { } } +#[test] +fn test_interactive_invalid_arg_message() { + new_ucmd!() + .arg("--interactive=bogus") + .arg("a") + .fails_with_code(1) + .stderr_is(concat!( + "rm: invalid argument 'bogus' for '--interactive'\n", + "Valid arguments are:\n", + " - 'never', 'no', 'none'\n", + " - 'once'\n", + " - 'always', 'yes'\n", + "Try 'rm --help' for more information.\n", + )); +} + +#[test] +fn test_interactive_abbreviation_same_meaning_accepted() { + // 'n' is ambiguous among 'never'/'no'/'none', but since they all mean + // the same choice, GNU (and this implementation) accepts it. + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + let file = "a"; + + at.touch(file); + scene + .ucmd() + .arg("--interactive=n") + .arg(file) + .succeeds() + .no_output(); + + assert!(!at.file_exists(file)); +} + #[test] fn test_interactive_always() { let scene = TestScenario::new(util_name!());