Merge pull request #1632 from sylvestre/clap-rm

refactor(rm): Move to clap + add a test

Cargo.lock

@@ -1974,7 +1974,7 @@ dependencies = [
 name = "uu_rm"
 version = "0.0.1"
 dependencies = [
- "getopts 0.2.21 (registry+https://github.com/rust-lang/crates.io-index)",
+ "clap 2.33.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "remove_dir_all 0.5.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "uucore 0.0.4",
  "uucore_procs 0.0.4",

src/uu/rm/Cargo.toml

@@ -15,7 +15,7 @@ edition = "2018"
 path = "src/rm.rs"
 
 [dependencies]
-getopts = "0.2.18"
+clap = "2.33"
 walkdir = "2.2.8"
 remove_dir_all = "0.5.1"
 uucore = { version=">=0.0.4", package="uucore", path="../../uucore" }

src/uu/rm/src/rm.rs

@@ -7,13 +7,14 @@
 
 // spell-checker:ignore (ToDO) bitor ulong
 
-extern crate getopts;
+extern crate clap;
 extern crate remove_dir_all;
 extern crate walkdir;
 
 #[macro_use]
 extern crate uucore;
 
+use clap::{App, Arg};
 use remove_dir_all::remove_dir_all;
 use std::collections::VecDeque;
 use std::fs;
@@ -40,83 +41,139 @@ struct Options {
     verbose: bool,
 }
 
-static NAME: &str = "rm";
+static ABOUT: &str = "Remove (unlink) the FILE(s)";
 static VERSION: &str = env!("CARGO_PKG_VERSION");
+static OPT_DIR: &str = "dir";
+static OPT_INTERACTIVE: &str = "interactive";
+static OPT_FORCE: &str = "force";
+static OPT_NO_PRESERVE_ROOT: &str = "no-preserve-root";
+static OPT_ONE_FILE_SYSTEM: &str = "one-file-system";
+static OPT_PRESERVE_ROOT: &str = "preserve-root";
+static OPT_PROMPT: &str = "prompt";
+static OPT_PROMPT_MORE: &str = "prompt-more";
+static OPT_RECURSIVE: &str = "recursive";
+static OPT_VERBOSE: &str = "verbose";
+
+static ARG_FILES: &str = "files";
+
+fn get_usage() -> String {
+    format!("{0} [OPTION]... FILE...", executable!())
+}
+
+fn get_long_usage() -> String {
+    String::from(
+        "By default, rm does not remove directories.  Use the --recursive (-r)
+        option to remove each listed directory, too, along with all of its contents
+
+        To remove a file whose name starts with a '-', for example '-foo',
+        use one of these commands:
+        rm -- -foo
+
+        rm ./-foo
+
+        Note that if you use rm to remove a file, it might be possible to recover
+        some of its contents, given sufficient expertise and/or time.  For greater
+        assurance that the contents are truly unrecoverable, consider using shred.",
+    )
+}
 
 pub fn uumain(args: impl uucore::Args) -> i32 {
-    let args = args.collect_str();
+    let usage = get_usage();
+    let long_usage = get_long_usage();
 
+    let matches = App::new(executable!())
+        .version(VERSION)
+        .about(ABOUT)
+        .usage(&usage[..])
+        .after_help(&long_usage[..])
     // TODO: make getopts support -R in addition to -r
-    let mut opts = getopts::Options::new();
 
-    opts.optflag(
+        .arg(
-        "f",
+            Arg::with_name(OPT_FORCE)
-        "force",
+            .short("f")
-        "ignore nonexistent files and arguments, never prompt",
+            .long(OPT_FORCE)
-    );
+            .help("ignore nonexistent files and arguments, never prompt")
-    opts.optflag("i", "", "prompt before every removal");
+        )
-    opts.optflag("I", "", "prompt once before removing more than three files, or when removing recursively.  Less intrusive than -i, while still giving some protection against most mistakes");
+        .arg(
-    opts.optflagopt(
+            Arg::with_name(OPT_PROMPT)
-        "",
+            .short("i")
-        "interactive",
+            .long("prompt before every removal")
-        "prompt according to WHEN: never, once (-I), or always (-i).  Without WHEN, prompts always",
+        )
-        "WHEN",
+        .arg(
-    );
+            Arg::with_name(OPT_PROMPT_MORE)
-    opts.optflag("", "one-file-system", "when removing a hierarchy recursively, skip any directory that is on a file system different from that of the corresponding command line argument (NOT IMPLEMENTED)");
+            .short("I")
-    opts.optflag("", "no-preserve-root", "do not treat '/' specially");
+            .help("prompt once before removing more than three files, or when removing recursively. Less intrusive than -i, while still giving some protection against most mistakes")
-    opts.optflag("", "preserve-root", "do not remove '/' (default)");
+        )
-    opts.optflag(
+        .arg(
-        "r",
+            Arg::with_name(OPT_INTERACTIVE)
-        "recursive",
+            .long(OPT_INTERACTIVE)
-        "remove directories and their contents recursively",
+            .help("prompt according to WHEN: never, once (-I), or always (-i). Without WHEN, prompts always")
-    );
+            .value_name("WHEN")
-    opts.optflag("d", "dir", "remove empty directories");
+            .takes_value(true)
-    opts.optflag("v", "verbose", "explain what is being done");
+        )
-    opts.optflag("h", "help", "display this help and exit");
+        .arg(
-    opts.optflag("V", "version", "output version information and exit");
+            Arg::with_name(OPT_ONE_FILE_SYSTEM)
+            .long(OPT_ONE_FILE_SYSTEM)
+            .help("when removing a hierarchy recursively, skip any directory that is on a file system different from that of the corresponding command line argument (NOT IMPLEMENTED)")
+        )
+        .arg(
+            Arg::with_name(OPT_NO_PRESERVE_ROOT)
+            .long(OPT_NO_PRESERVE_ROOT)
+            .help("do not treat '/' specially")
+        )
+        .arg(
+            Arg::with_name(OPT_PRESERVE_ROOT)
+            .long(OPT_PRESERVE_ROOT)
+            .help("do not remove '/' (default)")
+        )
+        .arg(
+            Arg::with_name(OPT_RECURSIVE).short("r")
+            .long(OPT_RECURSIVE)
+            .help("remove directories and their contents recursively")
+        )
+        .arg(
+            Arg::with_name(OPT_DIR)
+            .short("d")
+            .long(OPT_DIR)
+            .help("remove empty directories")
+        )
+        .arg(
+            Arg::with_name(OPT_VERBOSE)
+            .short("v")
+            .long(OPT_VERBOSE)
+            .help("explain what is being done")
+        )
+        .arg(
+            Arg::with_name(ARG_FILES)
+            .multiple(true)
+            .takes_value(true)
+            .min_values(1)
+        )
+        .get_matches_from(args);
 
-    let matches = match opts.parse(&args[1..]) {
+    let files: Vec<String> = matches
-        Ok(m) => m,
+        .values_of(ARG_FILES)
-        Err(f) => crash!(1, "{}", f),
+        .map(|v| v.map(ToString::to_string).collect())
-    };
+        .unwrap_or_default();
 
-    let force = matches.opt_present("force");
+    let force = matches.is_present(OPT_FORCE);
 
-    if matches.opt_present("help") {
+    if files.is_empty() && !force {
-        println!("{} {}", NAME, VERSION);
+        // Still check by hand and not use clap
-        println!();
+        // Because "rm -f" is a thing
-        println!("Usage:");
-        println!("  {0} [OPTION]... [FILE]...", NAME);
-        println!();
-        println!("{}", opts.usage("Remove (unlink) the FILE(s)."));
-        println!("By default, rm does not remove directories.  Use the --recursive (-r)");
-        println!("option to remove each listed directory, too, along with all of its contents");
-        println!();
-        println!("To remove a file whose name starts with a '-', for example '-foo',");
-        println!("use one of these commands:");
-        println!("rm -- -foo");
-        println!();
-        println!("rm ./-foo");
-        println!();
-        println!("Note that if you use rm to remove a file, it might be possible to recover");
-        println!("some of its contents, given sufficient expertise and/or time.  For greater");
-        println!("assurance that the contents are truly unrecoverable, consider using shred.");
-    } else if matches.opt_present("version") {
-        println!("{} {}", NAME, VERSION);
-    } else if matches.free.is_empty() && !force {
         show_error!("missing an argument");
-        show_error!("for help, try '{0} --help'", NAME);
+        show_error!("for help, try '{0} --help'", executable!());
         return 1;
     } else {
         let options = Options {
             force,
             interactive: {
-                if matches.opt_present("i") {
+                if matches.is_present(OPT_PROMPT) {
                     InteractiveMode::Always
-                } else if matches.opt_present("I") {
+                } else if matches.is_present(OPT_PROMPT_MORE) {
                     InteractiveMode::Once
-                } else if matches.opt_present("interactive") {
+                } else if matches.is_present(OPT_INTERACTIVE) {
-                    match &matches.opt_str("interactive").unwrap()[..] {
+                    match &matches.value_of(OPT_INTERACTIVE).unwrap()[..] {
                         "none" => InteractiveMode::None,
                         "once" => InteractiveMode::Once,
                         "always" => InteractiveMode::Always,
@@ -126,15 +183,13 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
                     InteractiveMode::None
                 }
             },
-            one_fs: matches.opt_present("one-file-system"),
+            one_fs: matches.is_present(OPT_ONE_FILE_SYSTEM),
-            preserve_root: !matches.opt_present("no-preserve-root"),
+            preserve_root: !matches.is_present(OPT_NO_PRESERVE_ROOT),
-            recursive: matches.opt_present("recursive"),
+            recursive: matches.is_present(OPT_RECURSIVE),
-            dir: matches.opt_present("dir"),
+            dir: matches.is_present(OPT_DIR),
-            verbose: matches.opt_present("verbose"),
+            verbose: matches.is_present(OPT_VERBOSE),
         };
-        if options.interactive == InteractiveMode::Once
+        if options.interactive == InteractiveMode::Once && (options.recursive || files.len() > 3) {
-            && (options.recursive || matches.free.len() > 3)
-        {
             let msg = if options.recursive {
                 "Remove all arguments recursively? "
             } else {
@@ -145,7 +200,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
             }
         }
 
-        if remove(matches.free, options) {
+        if remove(files, options) {
             return 1;
         }
     }

tests/by-util/test_rm.rs

@@ -12,6 +12,14 @@ fn test_rm_one_file() {
     assert!(!at.file_exists(file));
 }
 
+#[test]
+fn test_rm_failed() {
+    let (_at, mut ucmd) = at_and_ucmd!();
+    let file = "test_rm_one_file";
+
+    ucmd.arg(file).fails(); // Doesn't exist
+}
+
 #[test]
 fn test_rm_multiple_files() {
     let (at, mut ucmd) = at_and_ucmd!();