Merge pull request #8419 from cakebaker/rm_interactive_arguments

rm: support the `--interactive` arg aliases

Author: drinkcat

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]

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

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

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::<String>(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::<String>(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")

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("--interactive=never")
-        .arg(file_2)
-        .succeeds()
-        .stdout_is("");
+        scene
+            .ucmd()
+            .arg(format!("--interactive={arg}"))
+            .arg(file)
+            .succeeds()
+            .no_output();
 
-    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!();
-
-    let file1 = "test_rm_interactive_missing_value_file1";
-    let file2 = "test_rm_interactive_missing_value_file2";
-
-    at.touch(file1);
-    at.touch(file2);
-
-    ucmd.arg("--interactive")
-        .arg(file1)
-        .arg(file2)
-        .pipe_in("y\ny")
-        .succeeds();
+fn test_interactive_always() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
 
-    assert!(!at.file_exists(file1));
-    assert!(!at.file_exists(file2));
+    let file_a = "a";
+    let file_b = "b";
+
+    for arg in [
+        "-i",
+        "--interactive",
+        "--interactive=always",
+        "--interactive=yes",
+    ] {
+        at.touch(file_a);
+        at.touch(file_b);
+
+        scene
+            .ucmd()
+            .arg(arg)
+            .arg(file_a)
+            .arg(file_b)
+            .pipe_in("y\ny")
+            .succeeds()
+            .no_stdout();
+
+        assert!(!at.file_exists(file_a));
+        assert!(!at.file_exists(file_b));
+    }
 }
 
 #[test]