From 42756610206e5efd8ac1a0687b9d856d7b21e68a Mon Sep 17 00:00:00 2001 From: bashi8128 Date: Sun, 2 May 2021 13:47:36 +0900 Subject: [PATCH 1/5] tests/basename: add tests for --version, --help --- tests/by-util/test_basename.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/by-util/test_basename.rs b/tests/by-util/test_basename.rs index 8d32b4008..6c7a3ecae 100644 --- a/tests/by-util/test_basename.rs +++ b/tests/by-util/test_basename.rs @@ -1,6 +1,29 @@ use crate::common::util::*; use std::ffi::OsStr; +#[test] +fn test_help() { + for help_flg in vec!["-h", "--help"] { + new_ucmd!() + .arg(&help_flg) + .succeeds() + .no_stderr() + .stdout_contains("USAGE:"); + } +} + +#[test] +fn test_version() { + for version_flg in vec!["-V", "--version"] { + assert!(new_ucmd!() + .arg(&version_flg) + .succeeds() + .no_stderr() + .stdout_str() + .starts_with("basename")); + } +} + #[test] fn test_directory() { new_ucmd!() From 47a5dd0f9737cebe2859fc1ee5f9fff5e239f3e2 Mon Sep 17 00:00:00 2001 From: bashi8128 Date: Sun, 2 May 2021 17:08:14 +0900 Subject: [PATCH 2/5] basename: move from getopts to clap (#2117) Use clap for argument parsing instead of getopts Also, make the following changes * Use `executable!()` macro to output the name of utility * Add another usage to help message --- src/uu/basename/Cargo.toml | 1 + src/uu/basename/src/basename.rs | 91 +++++++++++++++++++++------------ 2 files changed, 58 insertions(+), 34 deletions(-) diff --git a/src/uu/basename/Cargo.toml b/src/uu/basename/Cargo.toml index 92d0ca4cd..0072619b7 100644 --- a/src/uu/basename/Cargo.toml +++ b/src/uu/basename/Cargo.toml @@ -15,6 +15,7 @@ edition = "2018" path = "src/basename.rs" [dependencies] +clap = "2.33.2" uucore = { version=">=0.0.8", package="uucore", path="../../uucore" } uucore_procs = { version=">=0.0.5", package="uucore_procs", path="../../uucore_procs" } diff --git a/src/uu/basename/src/basename.rs b/src/uu/basename/src/basename.rs index 68b705d53..b54a69f44 100644 --- a/src/uu/basename/src/basename.rs +++ b/src/uu/basename/src/basename.rs @@ -10,80 +10,103 @@ #[macro_use] extern crate uucore; +use clap::{App, Arg}; use std::path::{is_separator, PathBuf}; use uucore::InvalidEncodingHandling; -static NAME: &str = "basename"; -static SYNTAX: &str = "NAME [SUFFIX]"; +static VERSION: &str = env!("CARGO_PKG_VERSION"); static SUMMARY: &str = "Print NAME with any leading directory components removed - If specified, also remove a trailing SUFFIX"; -static LONG_HELP: &str = ""; +If specified, also remove a trailing SUFFIX"; + +fn get_usage() -> String { + format!( + "{0} NAME [SUFFIX] + {0} OPTION... NAME...", + executable!() + ) +} + +pub mod options { + pub static MULTIPLE: &str = "multiple"; + pub static NAME: &str = "name"; + pub static SUFFIX: &str = "suffix"; + pub static ZERO: &str = "zero"; +} pub fn uumain(args: impl uucore::Args) -> i32 { let args = args .collect_str(InvalidEncodingHandling::ConvertLossy) .accept_any(); + let usage = get_usage(); // // Argument parsing // - let matches = app!(SYNTAX, SUMMARY, LONG_HELP) - .optflag( - "a", - "multiple", - "Support more than one argument. Treat every argument as a name.", + let matches = App::new(executable!()) + .version(VERSION) + .about(SUMMARY) + .usage(&usage[..]) + .arg( + Arg::with_name(options::MULTIPLE) + .short("a") + .long(options::MULTIPLE) + .help("support multiple arguments and treat each as a NAME"), ) - .optopt( - "s", - "suffix", - "Remove a trailing suffix. This option implies the -a option.", - "SUFFIX", + .arg(Arg::with_name(options::NAME).multiple(true).hidden(true)) + .arg( + Arg::with_name(options::SUFFIX) + .short("s") + .long(options::SUFFIX) + .value_name("SUFFIX") + .help("remove a trailing SUFFIX; implies -a"), ) - .optflag( - "z", - "zero", - "Output a zero byte (ASCII NUL) at the end of each line, rather than a newline.", + .arg( + Arg::with_name(options::ZERO) + .short("z") + .long(options::ZERO) + .help("end each output line with NUL, not newline"), ) - .parse(args); + .get_matches_from(args); // too few arguments - if matches.free.is_empty() { + if !matches.is_present(options::NAME) { crash!( 1, "{0}: {1}\nTry '{0} --help' for more information.", - NAME, + executable!(), "missing operand" ); } - let opt_s = matches.opt_present("s"); - let opt_a = matches.opt_present("a"); - let opt_z = matches.opt_present("z"); + + let opt_s = matches.is_present(options::SUFFIX); + let opt_a = matches.is_present(options::MULTIPLE); + let opt_z = matches.is_present(options::ZERO); let multiple_paths = opt_s || opt_a; // too many arguments - if !multiple_paths && matches.free.len() > 2 { + if !multiple_paths && matches.occurrences_of(options::NAME) > 2 { crash!( 1, "{0}: extra operand '{1}'\nTry '{0} --help' for more information.", - NAME, - matches.free[2] + executable!(), + matches.values_of(options::NAME).unwrap().nth(2).unwrap() ); } let suffix = if opt_s { - matches.opt_str("s").unwrap() - } else if !opt_a && matches.free.len() > 1 { - matches.free[1].clone() + matches.value_of(options::SUFFIX).unwrap() + } else if !opt_a && matches.occurrences_of(options::NAME) > 1 { + matches.values_of(options::NAME).unwrap().nth(1).unwrap() } else { - "".to_owned() + "" }; // // Main Program Processing // - let paths = if multiple_paths { - &matches.free[..] + let paths: Vec<_> = if multiple_paths { + matches.values_of(options::NAME).unwrap().collect() } else { - &matches.free[0..1] + matches.values_of(options::NAME).unwrap().take(1).collect() }; let line_ending = if opt_z { "\0" } else { "\n" }; From 91c736bd95bc8eafdff1ff428bc68e3ed76fa34c Mon Sep 17 00:00:00 2001 From: bashi8128 Date: Mon, 3 May 2021 23:22:00 +0900 Subject: [PATCH 3/5] tests/basename: add tests for error messages --- tests/by-util/test_basename.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/by-util/test_basename.rs b/tests/by-util/test_basename.rs index 6c7a3ecae..2a40ba4b9 100644 --- a/tests/by-util/test_basename.rs +++ b/tests/by-util/test_basename.rs @@ -104,11 +104,25 @@ fn test_no_args() { expect_error(vec![]); } +#[test] +fn test_no_args_output() { + new_ucmd!() + .fails() + .stderr_is("basename: error: missing operand\nTry 'basename --help' for more information."); +} + #[test] fn test_too_many_args() { expect_error(vec!["a", "b", "c"]); } +#[test] +fn test_too_many_args_output() { + new_ucmd!().args(&["a", "b", "c"]).fails().stderr_is( + "basename: error: extra operand 'c'\nTry 'basename --help' for more information.", + ); +} + fn test_invalid_utf8_args(os_str: &OsStr) { let test_vec = vec![os_str.to_os_string()]; new_ucmd!().args(&test_vec).succeeds().stdout_is("fo�o\n"); From 74802f9f0fca4579315ce6f7097ff131534d211f Mon Sep 17 00:00:00 2001 From: bashi8128 Date: Mon, 3 May 2021 23:26:46 +0900 Subject: [PATCH 4/5] basename: improve error messages Remove duplicated utility name from error messages --- src/uu/basename/src/basename.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/uu/basename/src/basename.rs b/src/uu/basename/src/basename.rs index b54a69f44..d5512863f 100644 --- a/src/uu/basename/src/basename.rs +++ b/src/uu/basename/src/basename.rs @@ -71,7 +71,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { if !matches.is_present(options::NAME) { crash!( 1, - "{0}: {1}\nTry '{0} --help' for more information.", + "{1}\nTry '{0} --help' for more information.", executable!(), "missing operand" ); @@ -85,7 +85,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { if !multiple_paths && matches.occurrences_of(options::NAME) > 2 { crash!( 1, - "{0}: extra operand '{1}'\nTry '{0} --help' for more information.", + "extra operand '{1}'\nTry '{0} --help' for more information.", executable!(), matches.values_of(options::NAME).unwrap().nth(2).unwrap() ); From 5a4bb610ffec547fdac3f749a17f46b572f62962 Mon Sep 17 00:00:00 2001 From: bashi8128 Date: Mon, 3 May 2021 23:32:01 +0900 Subject: [PATCH 5/5] basename: rename variable names Rename variable names to be more explicit ones --- src/uu/basename/src/basename.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/uu/basename/src/basename.rs b/src/uu/basename/src/basename.rs index d5512863f..c20561b30 100644 --- a/src/uu/basename/src/basename.rs +++ b/src/uu/basename/src/basename.rs @@ -77,10 +77,10 @@ pub fn uumain(args: impl uucore::Args) -> i32 { ); } - let opt_s = matches.is_present(options::SUFFIX); - let opt_a = matches.is_present(options::MULTIPLE); - let opt_z = matches.is_present(options::ZERO); - let multiple_paths = opt_s || opt_a; + let opt_suffix = matches.is_present(options::SUFFIX); + let opt_multiple = matches.is_present(options::MULTIPLE); + let opt_zero = matches.is_present(options::ZERO); + let multiple_paths = opt_suffix || opt_multiple; // too many arguments if !multiple_paths && matches.occurrences_of(options::NAME) > 2 { crash!( @@ -91,9 +91,9 @@ pub fn uumain(args: impl uucore::Args) -> i32 { ); } - let suffix = if opt_s { + let suffix = if opt_suffix { matches.value_of(options::SUFFIX).unwrap() - } else if !opt_a && matches.occurrences_of(options::NAME) > 1 { + } else if !opt_multiple && matches.occurrences_of(options::NAME) > 1 { matches.values_of(options::NAME).unwrap().nth(1).unwrap() } else { "" @@ -109,7 +109,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { matches.values_of(options::NAME).unwrap().take(1).collect() }; - let line_ending = if opt_z { "\0" } else { "\n" }; + let line_ending = if opt_zero { "\0" } else { "\n" }; for path in paths { print!("{}{}", basename(&path, &suffix), line_ending); }