diff --git a/src/uu/rm/Cargo.toml b/src/uu/rm/Cargo.toml index 01056009a..a7d959977 100644 --- a/src/uu/rm/Cargo.toml +++ b/src/uu/rm/Cargo.toml @@ -20,7 +20,7 @@ path = "src/rm.rs" [dependencies] thiserror = { workspace = true } clap = { workspace = true } -uucore = { workspace = true, features = ["fs"] } +uucore = { workspace = true, features = ["fs", "parser"] } fluent = { workspace = true } [target.'cfg(unix)'.dependencies] diff --git a/src/uu/rm/locales/en-US.ftl b/src/uu/rm/locales/en-US.ftl index 5a1231806..de12feae4 100644 --- a/src/uu/rm/locales/en-US.ftl +++ b/src/uu/rm/locales/en-US.ftl @@ -32,7 +32,6 @@ rm-help-verbose = explain what is being done # Error messages rm-error-missing-operand = missing operand Try '{$util_name} --help' for more information. -rm-error-invalid-interactive-argument = Invalid argument to interactive ({$arg}) rm-error-cannot-remove-no-such-file = cannot remove {$file}: No such file or directory rm-error-cannot-remove-permission-denied = cannot remove {$file}: Permission denied rm-error-cannot-remove-is-directory = cannot remove {$file}: Is a directory diff --git a/src/uu/rm/locales/fr-FR.ftl b/src/uu/rm/locales/fr-FR.ftl index 80842e8bc..2bff4a66f 100644 --- a/src/uu/rm/locales/fr-FR.ftl +++ b/src/uu/rm/locales/fr-FR.ftl @@ -32,7 +32,6 @@ rm-help-verbose = expliquer ce qui est fait # Messages d'erreur rm-error-missing-operand = opérande manquant Essayez '{$util_name} --help' pour plus d'informations. -rm-error-invalid-interactive-argument = Argument invalide pour interactive ({$arg}) rm-error-cannot-remove-no-such-file = impossible de supprimer {$file} : Aucun fichier ou répertoire de ce type rm-error-cannot-remove-permission-denied = impossible de supprimer {$file} : Permission refusée rm-error-cannot-remove-is-directory = impossible de supprimer {$file} : C'est un répertoire diff --git a/src/uu/rm/src/rm.rs b/src/uu/rm/src/rm.rs index 18c963611..f7971aa80 100644 --- a/src/uu/rm/src/rm.rs +++ b/src/uu/rm/src/rm.rs @@ -5,7 +5,8 @@ // spell-checker:ignore (path) eacces inacc rm-r4 -use clap::{Arg, ArgAction, Command, builder::ValueParser, parser::ValueSource}; +use clap::builder::{PossibleValue, ValueParser}; +use clap::{Arg, ArgAction, Command, parser::ValueSource}; use std::ffi::{OsStr, OsString}; use std::fs::{self, Metadata}; use std::io::{IsTerminal, stdin}; @@ -19,6 +20,7 @@ use std::path::{Path, PathBuf}; use thiserror::Error; use uucore::display::Quotable; use uucore::error::{FromIo, UError, UResult}; +use uucore::parser::shortcut_value_parser::ShortcutValueParser; use uucore::translate; use uucore::{format_usage, os_str_as_bytes, prompt_yes, show_error}; @@ -27,8 +29,6 @@ use uucore::{format_usage, os_str_as_bytes, prompt_yes, show_error}; enum RmError { #[error("{}", translate!("rm-error-missing-operand", "util_name" => uucore::execution_phrase()))] MissingOperand, - #[error("{}", translate!("rm-error-invalid-interactive-argument", "arg" => _0.clone()))] - InvalidInteractiveArgument(String), #[error("{}", translate!("rm-error-cannot-remove-no-such-file", "file" => _0.quote()))] CannotRemoveNoSuchFile(String), #[error("{}", translate!("rm-error-cannot-remove-permission-denied", "file" => _0.quote()))] @@ -59,6 +59,20 @@ pub enum InteractiveMode { PromptProtected, } +// We implement `From` instead of `TryFrom` because clap guarantees that we only receive valid values. +// +// The `PromptProtected` variant is not supposed to be created from a string. +impl From<&str> for InteractiveMode { + fn from(s: &str) -> Self { + match s { + "never" => Self::Never, + "once" => Self::Once, + "always" => Self::Always, + _ => unreachable!("should be prevented by clap"), + } + } +} + /// Options for the `rm` command /// /// All options are public so that the options can be programmatically @@ -165,14 +179,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { } else if matches.get_flag(OPT_PROMPT_ONCE) { InteractiveMode::Once } else if matches.contains_id(OPT_INTERACTIVE) { - match matches.get_one::(OPT_INTERACTIVE).unwrap().as_str() { - "never" => InteractiveMode::Never, - "once" => InteractiveMode::Once, - "always" => InteractiveMode::Always, - val => { - return Err(RmError::InvalidInteractiveArgument(val.to_string()).into()); - } - } + InteractiveMode::from(matches.get_one::(OPT_INTERACTIVE).unwrap().as_str()) } else { InteractiveMode::PromptProtected } @@ -249,6 +256,11 @@ 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 5589286e7..ec7de9136 100644 --- a/tests/by-util/test_rm.rs +++ b/tests/by-util/test_rm.rs @@ -6,8 +6,7 @@ use std::process::Stdio; -use uutests::util::TestScenario; -use uutests::{at_and_ucmd, new_ucmd, util_name}; +use uutests::{at_and_ucmd, new_ucmd, util::TestScenario, util_name}; #[test] fn test_invalid_arg() { @@ -379,42 +378,53 @@ fn test_silently_accepts_presume_input_tty2() { fn test_interactive_never() { let scene = TestScenario::new(util_name!()); let at = &scene.fixtures; + let file = "a"; - let file_2 = "test_rm_interactive"; + for arg in ["never", "no", "none"] { + at.touch(file); + #[cfg(feature = "chmod")] + scene.ccmd("chmod").arg("0").arg(file).succeeds(); - at.touch(file_2); - #[cfg(feature = "chmod")] - scene.ccmd("chmod").arg("0").arg(file_2).succeeds(); + scene + .ucmd() + .arg(format!("--interactive={arg}")) + .arg(file) + .succeeds() + .no_output(); - scene - .ucmd() - .arg("--interactive=never") - .arg(file_2) - .succeeds() - .stdout_is(""); - - assert!(!at.file_exists(file_2)); + assert!(!at.file_exists(file)); + } } #[test] -fn test_interactive_missing_value() { - // `--interactive` is equivalent to `--interactive=always` or `-i` - let (at, mut ucmd) = at_and_ucmd!(); +fn test_interactive_always() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; - let file1 = "test_rm_interactive_missing_value_file1"; - let file2 = "test_rm_interactive_missing_value_file2"; + let file_a = "a"; + let file_b = "b"; - at.touch(file1); - at.touch(file2); + for arg in [ + "-i", + "--interactive", + "--interactive=always", + "--interactive=yes", + ] { + at.touch(file_a); + at.touch(file_b); - ucmd.arg("--interactive") - .arg(file1) - .arg(file2) - .pipe_in("y\ny") - .succeeds(); + scene + .ucmd() + .arg(arg) + .arg(file_a) + .arg(file_b) + .pipe_in("y\ny") + .succeeds() + .no_stdout(); - assert!(!at.file_exists(file1)); - assert!(!at.file_exists(file2)); + assert!(!at.file_exists(file_a)); + assert!(!at.file_exists(file_b)); + } } #[test]