who: move from getopts to clap (#2124)

src/uu/who/Cargo.toml

@@ -17,6 +17,7 @@ path = "src/who.rs"
 [dependencies]
 uucore = { version=">=0.0.8", package="uucore", path="../../uucore", features=["utmpx"] }
 uucore_procs = { version=">=0.0.5", package="uucore_procs", path="../../uucore_procs" }
+clap = "3.0.0-beta.2"
 
 [[bin]]
 name = "who"

src/uu/who/src/who.rs

@@ -12,79 +12,164 @@ extern crate uucore;
 use uucore::libc::{ttyname, STDIN_FILENO, S_IWGRP};
 use uucore::utmpx::{self, time, Utmpx};
 
+use clap::{App, Arg};
 use std::borrow::Cow;
 use std::ffi::CStr;
 use std::os::unix::fs::MetadataExt;
 use std::path::PathBuf;
 use uucore::InvalidEncodingHandling;
 
-static SYNTAX: &str = "[OPTION]... [ FILE | ARG1 ARG2 ]";
-static SUMMARY: &str = "Print information about users who are currently logged in.";
-static LONG_HELP: &str = "
-  -a, --all         same as -b -d --login -p -r -t -T -u
-  -b, --boot        time of last system boot
-  -d, --dead        print dead processes
-  -H, --heading     print line of column headings
-  -l, --login       print system login processes
-      --lookup      attempt to canonicalize hostnames via DNS
-  -m                only hostname and user associated with stdin
-  -p, --process     print active processes spawned by init
-  -q, --count       all login names and number of users logged on
-  -r, --runlevel    print current runlevel (not available on BSDs)
-  -s, --short       print only name, line, and time (default)
-  -t, --time        print last system clock change
-  -T, -w, --mesg    add user's message status as +, - or ?
-  -u, --users       list users logged in
-      --message     same as -T
-      --writable    same as -T
-      --help     display this help and exit
-      --version  output version information and exit
+mod options {
+    pub const ALL: &str = "all";
+    pub const BOOT: &str = "boot";
+    pub const DEAD: &str = "dead";
+    pub const HEADING: &str = "heading";
+    pub const LOGIN: &str = "login";
+    pub const LOOKUP: &str = "lookup";
+    pub const ONLY_HOSTNAME_USER: &str = "only_hostname_user";
+    pub const PROCESS: &str = "process";
+    pub const COUNT: &str = "count";
+    #[cfg(any(target_vendor = "apple", target_os = "linux", target_os = "android"))]
+    pub const RUNLEVEL: &str = "runlevel";
+    pub const SHORT: &str = "short";
+    pub const TIME: &str = "time";
+    pub const USERS: &str = "users";
+    pub const MESG: &str = "mesg"; // aliases: --message, --writable
+    pub const FILE: &str = "FILE"; // if length=1: FILE, if length=2: ARG1 ARG2
+}
 
-If FILE is not specified, use /var/run/utmp.  /var/log/wtmp as FILE is common.
-If ARG1 ARG2 given, -m presumed: 'am i' or 'mom likes' are usual.
-";
+static VERSION: &str = env!("CARGO_PKG_VERSION");
+static ABOUT: &str = "Print information about users who are currently logged in.";
+
+fn get_usage() -> String {
+    format!("{0} [OPTION]... [ FILE | ARG1 ARG2 ]", executable!())
+}
+
+fn get_long_usage() -> String {
+    String::from(
+        "If FILE is not specified, use /var/run/utmp.  /var/log/wtmp as FILE is common.\n\
+If ARG1 ARG2 given, -m presumed: 'am i' or 'mom likes' are usual.",
+    )
+}
 
 pub fn uumain(args: impl uucore::Args) -> i32 {
     let args = args
         .collect_str(InvalidEncodingHandling::Ignore)
         .accept_any();
 
-    let mut opts = app!(SYNTAX, SUMMARY, LONG_HELP);
-    opts.optflag("a", "all", "same as -b -d --login -p -r -t -T -u");
-    opts.optflag("b", "boot", "time of last system boot");
-    opts.optflag("d", "dead", "print dead processes");
-    opts.optflag("H", "heading", "print line of column headings");
-    opts.optflag("l", "login", "print system login processes");
-    opts.optflag("", "lookup", "attempt to canonicalize hostnames via DNS");
-    opts.optflag("m", "", "only hostname and user associated with stdin");
-    opts.optflag("p", "process", "print active processes spawned by init");
-    opts.optflag(
-        "q",
-        "count",
-        "all login names and number of users logged on",
-    );
-    #[cfg(any(target_vendor = "apple", target_os = "linux", target_os = "android"))]
-    opts.optflag("r", "runlevel", "print current runlevel");
-    opts.optflag("s", "short", "print only name, line, and time (default)");
-    opts.optflag("t", "time", "print last system clock change");
-    opts.optflag("u", "users", "list users logged in");
-    opts.optflag("w", "mesg", "add user's message status as +, - or ?");
-    // --message, --writable are the same as --mesg
-    opts.optflag("T", "message", "");
-    opts.optflag("T", "writable", "");
+    let usage = get_usage();
+    let after_help = get_long_usage();
 
-    opts.optflag("", "help", "display this help and exit");
-    opts.optflag("", "version", "output version information and exit");
+    let matches = App::new(executable!())
+        .version(VERSION)
+        .about(ABOUT)
+        .override_usage(&usage[..])
+        .after_help(&after_help[..])
+        .arg(
+            Arg::new(options::ALL)
+                .long(options::ALL)
+                .short('a')
+                .about("same as -b -d --login -p -r -t -T -u"),
+        )
+        .arg(
+            Arg::new(options::BOOT)
+                .long(options::BOOT)
+                .short('b')
+                .about("time of last system boot"),
+        )
+        .arg(
+            Arg::new(options::DEAD)
+                .long(options::DEAD)
+                .short('d')
+                .about("print dead processes"),
+        )
+        .arg(
+            Arg::new(options::HEADING)
+                .long(options::HEADING)
+                .short('H')
+                .about("print line of column headings"),
+        )
+        .arg(
+            Arg::new(options::LOGIN)
+                .long(options::LOGIN)
+                .short('l')
+                .about("print system login processes"),
+        )
+        .arg(
+            Arg::new(options::LOOKUP)
+                .long(options::LOOKUP)
+                .about("attempt to canonicalize hostnames via DNS"),
+        )
+        .arg(
+            Arg::new(options::ONLY_HOSTNAME_USER)
+                .short('m')
+                .about("only hostname and user associated with stdin"),
+        )
+        .arg(
+            Arg::new(options::PROCESS)
+                .long(options::PROCESS)
+                .short('p')
+                .about("print active processes spawned by init"),
+        )
+        .arg(
+            Arg::new(options::COUNT)
+                .long(options::COUNT)
+                .short('q')
+                .about("all login names and number of users logged on"),
+        )
+        .arg(
+            #[cfg(any(target_vendor = "apple", target_os = "linux", target_os = "android"))]
+            Arg::new(options::RUNLEVEL)
+                .long(options::RUNLEVEL)
+                .short('r')
+                .about("print current runlevel"),
+        )
+        .arg(
+            Arg::new(options::SHORT)
+                .long(options::SHORT)
+                .short('s')
+                .about("print only name, line, and time (default)"),
+        )
+        .arg(
+            Arg::new(options::TIME)
+                .long(options::TIME)
+                .short('t')
+                .about("print last system clock change"),
+        )
+        .arg(
+            Arg::new(options::USERS)
+                .long(options::USERS)
+                .short('u')
+                .about("list users logged in"),
+        )
+        .arg(
+            Arg::new(options::MESG)
+                .long(options::MESG)
+                .short('T')
+                .visible_short_alias('w')
+                .visible_aliases(&["message", "writable"])
+                .about("add user's message status as +, - or ?"),
+        )
+        .arg(
+            Arg::new(options::FILE)
+                .takes_value(true)
+                .min_values(1)
+                .max_values(2),
+        )
+        .get_matches_from(args);
 
-    let matches = opts.parse(args);
+    let files: Vec<String> = matches
+        .values_of(options::FILE)
+        .map(|v| v.map(ToString::to_string).collect())
+        .unwrap_or_default();
 
     // If true, attempt to canonicalize hostnames via a DNS lookup.
-    let do_lookup = matches.opt_present("lookup");
+    let do_lookup = matches.is_present(options::LOOKUP);
 
     // If true, display only a list of usernames and count of
     // the users logged on.
     // Ignored for 'who am i'.
-    let short_list = matches.opt_present("q");
+    let short_list = matches.is_present(options::COUNT);
 
     // If true, display only name, line, and time fields.
     let mut short_output = false;
@@ -95,12 +180,11 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
     let mut include_idle = false;
 
     // If true, display a line at the top describing each field.
-    let include_heading = matches.opt_present("H");
+    let include_heading = matches.is_present(options::HEADING);
 
     // If true, display a '+' for each user if mesg y, a '-' if mesg n,
     // or a '?' if their tty cannot be statted.
-    let include_mesg =
-        matches.opt_present("a") || matches.opt_present("T") || matches.opt_present("w");
+    let include_mesg = matches.is_present(options::ALL) || matches.is_present(options::MESG);
 
     // If true, display process termination & exit status.
     let mut include_exit = false;
@@ -133,7 +217,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
 
     #[allow(clippy::useless_let_if_seq)]
     {
-        if matches.opt_present("a") {
+        if matches.is_present(options::ALL) {
             need_boottime = true;
             need_deadprocs = true;
             need_login = true;
@@ -146,49 +230,49 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
             assumptions = false;
         }
 
-        if matches.opt_present("b") {
+        if matches.is_present(options::BOOT) {
             need_boottime = true;
             assumptions = false;
         }
 
-        if matches.opt_present("d") {
+        if matches.is_present(options::DEAD) {
             need_deadprocs = true;
             include_idle = true;
             include_exit = true;
             assumptions = false;
         }
 
-        if matches.opt_present("l") {
+        if matches.is_present(options::LOGIN) {
             need_login = true;
             include_idle = true;
             assumptions = false;
         }
 
-        if matches.opt_present("m") || matches.free.len() == 2 {
+        if matches.is_present(options::ONLY_HOSTNAME_USER) || files.len() == 2 {
             my_line_only = true;
         }
 
-        if matches.opt_present("p") {
+        if matches.is_present(options::PROCESS) {
             need_initspawn = true;
             assumptions = false;
         }
 
-        if matches.opt_present("r") {
+        if matches.is_present(options::RUNLEVEL) {
             need_runlevel = true;
             include_idle = true;
             assumptions = false;
         }
 
-        if matches.opt_present("s") {
+        if matches.is_present(options::SHORT) {
             short_output = true;
         }
 
-        if matches.opt_present("t") {
+        if matches.is_present(options::TIME) {
             need_clockchange = true;
             assumptions = false;
         }
 
-        if matches.opt_present("u") {
+        if matches.is_present(options::USERS) {
             need_users = true;
             include_idle = true;
             assumptions = false;
@@ -202,11 +286,6 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
         if include_exit {
             short_output = false;
         }
-
-        if matches.free.len() > 2 {
-            show_usage_error!("{}", msg_wrong_number_of_arguments!());
-            exit!(1);
-        }
     }
 
     let mut who = Who {
@@ -225,7 +304,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
         need_runlevel,
         need_users,
         my_line_only,
-        args: matches.free,
+        args: files,
     };
 
     who.exec();

tests/by-util/test_who.rs

@@ -1,11 +1,13 @@
-#[cfg(target_os = "linux")]
 use crate::common::util::*;
 
 #[cfg(target_os = "linux")]
 #[test]
 fn test_count() {
     for opt in vec!["-q", "--count"] {
-        new_ucmd!().arg(opt).run().stdout_is(expected_result(opt));
+        new_ucmd!()
+            .arg(opt)
+            .succeeds()
+            .stdout_is(expected_result(opt));
     }
 }
 
@@ -13,17 +15,21 @@ fn test_count() {
 #[test]
 fn test_boot() {
     for opt in vec!["-b", "--boot"] {
-        new_ucmd!().arg(opt).run().stdout_is(expected_result(opt));
+        new_ucmd!()
+            .arg(opt)
+            .succeeds()
+            .stdout_is(expected_result(opt));
     }
 }
 
 #[cfg(target_os = "linux")]
 #[test]
 fn test_heading() {
-    for opt in vec!["-H"] {
+    for opt in vec!["-H", "--heading"] {
         // allow whitespace variation
-        // * minor whitespace differences occur between platform built-in outputs; specifically number of TABs between "TIME" and "COMMENT" may be variant
-        let actual = new_ucmd!().arg(opt).run().stdout_move_str();
+        // * minor whitespace differences occur between platform built-in outputs;
+        //   specifically number of TABs between "TIME" and "COMMENT" may be variant
+        let actual = new_ucmd!().arg(opt).succeeds().stdout_move_str();
         let expect = expected_result(opt);
         println!("actual: {:?}", actual);
         println!("expect: {:?}", expect);
@@ -37,7 +43,10 @@ fn test_heading() {
 #[test]
 fn test_short() {
     for opt in vec!["-s", "--short"] {
-        new_ucmd!().arg(opt).run().stdout_is(expected_result(opt));
+        new_ucmd!()
+            .arg(opt)
+            .succeeds()
+            .stdout_is(expected_result(opt));
     }
 }
 
@@ -45,7 +54,10 @@ fn test_short() {
 #[test]
 fn test_login() {
     for opt in vec!["-l", "--login"] {
-        new_ucmd!().arg(opt).run().stdout_is(expected_result(opt));
+        new_ucmd!()
+            .arg(opt)
+            .succeeds()
+            .stdout_is(expected_result(opt));
     }
 }
 
@@ -53,7 +65,109 @@ fn test_login() {
 #[test]
 fn test_m() {
     for opt in vec!["-m"] {
-        new_ucmd!().arg(opt).run().stdout_is(expected_result(opt));
+        new_ucmd!()
+            .arg(opt)
+            .succeeds()
+            .stdout_is(expected_result(opt));
+    }
+}
+
+#[cfg(target_os = "linux")]
+#[test]
+fn test_process() {
+    for opt in vec!["-p", "--process"] {
+        new_ucmd!()
+            .arg(opt)
+            .succeeds()
+            .stdout_is(expected_result(opt));
+    }
+}
+
+#[cfg(target_os = "linux")]
+#[test]
+fn test_runlevel() {
+    for opt in vec!["-r", "--runlevel"] {
+        new_ucmd!()
+            .arg(opt)
+            .succeeds()
+            .stdout_is(expected_result(opt));
+    }
+}
+
+#[cfg(target_os = "linux")]
+#[test]
+fn test_time() {
+    for opt in vec!["-t", "--time"] {
+        new_ucmd!()
+            .arg(opt)
+            .succeeds()
+            .stdout_is(expected_result(opt));
+    }
+}
+
+#[cfg(target_os = "linux")]
+#[test]
+fn test_mesg() {
+    for opt in vec!["-w", "-T", "--users", "--message", "--writable"] {
+        new_ucmd!()
+            .arg(opt)
+            .succeeds()
+            .stdout_is(expected_result(opt));
+    }
+}
+
+#[cfg(target_os = "linux")]
+#[test]
+fn test_arg1_arg2() {
+    let scene = TestScenario::new(util_name!());
+
+    let expected = scene
+        .cmd_keepenv(util_name!())
+        .env("LANGUAGE", "C")
+        .arg("am")
+        .arg("i")
+        .succeeds();
+
+    scene
+        .ucmd()
+        .arg("am")
+        .arg("i")
+        .succeeds()
+        .stdout_is(expected.stdout_str());
+}
+
+#[test]
+fn test_too_many_args() {
+    let expected =
+        "error: The value 'u' was provided to '<FILE>...' but it wasn't expecting any more values";
+
+    new_ucmd!()
+        .arg("am")
+        .arg("i")
+        .arg("u")
+        .fails()
+        .stderr_contains(expected);
+}
+
+#[cfg(target_os = "linux")]
+#[test]
+fn test_users() {
+    for opt in vec!["-u", "--users"] {
+        new_ucmd!()
+            .arg(opt)
+            .succeeds()
+            .stdout_is(expected_result(opt));
+    }
+}
+
+#[cfg(target_os = "linux")]
+#[test]
+fn test_lookup() {
+    for opt in vec!["--lookup"] {
+        new_ucmd!()
+            .arg(opt)
+            .succeeds()
+            .stdout_is(expected_result(opt));
     }
 }
 
@@ -61,15 +175,60 @@ fn test_m() {
 #[test]
 fn test_dead() {
     for opt in vec!["-d", "--dead"] {
-        new_ucmd!().arg(opt).run().stdout_is(expected_result(opt));
+        new_ucmd!()
+            .arg(opt)
+            .succeeds()
+            .stdout_is(expected_result(opt));
     }
 }
 
+#[cfg(target_os = "linux")]
+#[test]
+fn test_all_separately() {
+    // -a, --all         same as -b -d --login -p -r -t -T -u
+    let scene = TestScenario::new(util_name!());
+
+    let expected = scene
+        .cmd_keepenv(util_name!())
+        .env("LANGUAGE", "C")
+        .arg("-b")
+        .arg("-d")
+        .arg("--login")
+        .arg("-p")
+        .arg("-r")
+        .arg("-t")
+        .arg("-T")
+        .arg("-u")
+        .succeeds();
+
+    scene
+        .ucmd()
+        .arg("-b")
+        .arg("-d")
+        .arg("--login")
+        .arg("-p")
+        .arg("-r")
+        .arg("-t")
+        .arg("-T")
+        .arg("-u")
+        .succeeds()
+        .stdout_is(expected.stdout_str());
+
+    scene
+        .ucmd()
+        .arg("--all")
+        .succeeds()
+        .stdout_is(expected.stdout_str());
+}
+
 #[cfg(target_os = "linux")]
 #[test]
 fn test_all() {
     for opt in vec!["-a", "--all"] {
-        new_ucmd!().arg(opt).run().stdout_is(expected_result(opt));
+        new_ucmd!()
+            .arg(opt)
+            .succeeds()
+            .stdout_is(expected_result(opt));
     }
 }
 
@@ -79,6 +238,6 @@ fn expected_result(arg: &str) -> String {
         .cmd_keepenv(util_name!())
         .env("LANGUAGE", "C")
         .args(&[arg])
-        .run()
+        .succeeds()
         .stdout_move_str()
 }