From efe1850087a9c0a2a8b57c2385d09d62d5ba80a6 Mon Sep 17 00:00:00 2001 From: Mitchell Mebane Date: Tue, 1 Jun 2021 19:06:51 -0500 Subject: [PATCH 1/3] dircolors: replace getopts with clap Port argument parsing from getopts to clap. The only difference I have observed is that clap auto-generates -h and -V short options for help and version, and there is no way (in clap 2.x) to disable them. --- Cargo.lock | 1 + src/uu/dircolors/Cargo.toml | 1 + src/uu/dircolors/src/dircolors.rs | 118 +++++++++++++++++++++++------- 3 files changed, 93 insertions(+), 27 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 17fa9e2b7..01c154351 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1893,6 +1893,7 @@ dependencies = [ name = "uu_dircolors" version = "0.0.6" dependencies = [ + "clap", "glob 0.3.0", "uucore", "uucore_procs", diff --git a/src/uu/dircolors/Cargo.toml b/src/uu/dircolors/Cargo.toml index 5e822820e..7d47fa5c4 100644 --- a/src/uu/dircolors/Cargo.toml +++ b/src/uu/dircolors/Cargo.toml @@ -15,6 +15,7 @@ edition = "2018" path = "src/dircolors.rs" [dependencies] +clap = "2.33" glob = "0.3.0" 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/dircolors/src/dircolors.rs b/src/uu/dircolors/src/dircolors.rs index b6942c2d2..8e13e281c 100644 --- a/src/uu/dircolors/src/dircolors.rs +++ b/src/uu/dircolors/src/dircolors.rs @@ -1,6 +1,7 @@ // This file is part of the uutils coreutils package. // // (c) Jian Zeng +// (c) Mitchell Mebane // // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. @@ -15,6 +16,17 @@ use std::env; use std::fs::File; use std::io::{BufRead, BufReader}; +use clap::{App, Arg, crate_version}; + +mod options { + pub const SH: &str = "sh"; + pub const BOURNE_SHELL: &str = "bourne-shell"; + pub const CSH: &str = "csh"; + pub const C_SHELL: &str = "c-shell"; + pub const PRINT_DATABASE: &str = "print-database"; + pub const FILE: &str = "FILE"; +} + static SYNTAX: &str = "[OPTION]... [FILE]"; static SUMMARY: &str = "Output commands to set the LS_COLORS environment variable."; static LONG_HELP: &str = " @@ -52,28 +64,80 @@ pub fn guess_syntax() -> OutputFmt { } } +fn get_usage() -> String { + format!( + "{0} {1}", + executable!(), + SYNTAX + ) +} + pub fn uumain(args: impl uucore::Args) -> i32 { let args = args .collect_str(InvalidEncodingHandling::Ignore) .accept_any(); - let matches = app!(SYNTAX, SUMMARY, LONG_HELP) - .optflag("b", "sh", "output Bourne shell code to set LS_COLORS") - .optflag( - "", - "bourne-shell", - "output Bourne shell code to set LS_COLORS", - ) - .optflag("c", "csh", "output C shell code to set LS_COLORS") - .optflag("", "c-shell", "output C shell code to set LS_COLORS") - .optflag("p", "print-database", "print the byte counts") - .parse(args); + let usage = get_usage(); - if (matches.opt_present("csh") - || matches.opt_present("c-shell") - || matches.opt_present("sh") - || matches.opt_present("bourne-shell")) - && matches.opt_present("print-database") + /* Clap has .visible_alias, but it generates help like this + * -b, --sh output Bourne shell code to set LS_COLORS [aliases: bourne-shell] + * whereas we want help like this + * -b, --sh output Bourne shell code to set LS_COLORS + * --bourne-shell output Bourne shell code to set LS_COLORS + * (or preferably like the original, but that doesn't seem possible with clap:) + * -b, --sh, --bourne-shell output Bourne shell code to set LS_COLORS + * therefore, command aliases are defined manually as multiple commands + */ + let matches = App::new(executable!()) + .version(crate_version!()) + .about(SUMMARY) + .usage(&usage[..]) + .after_help(LONG_HELP) + .arg(Arg::with_name(options::SH) + .long("sh") + .short("b") + .help("output Bourne shell code to set LS_COLORS") + .display_order(1) + ) + .arg(Arg::with_name(options::BOURNE_SHELL) + .long("bourne-shell") + .help("output Bourne shell code to set LS_COLORS") + .display_order(2) + ) + .arg(Arg::with_name(options::CSH) + .long("csh") + .short("c") + .help("output C shell code to set LS_COLORS") + .display_order(3) + ) + .arg(Arg::with_name(options::C_SHELL) + .long("c-shell") + .help("output C shell code to set LS_COLORS") + .display_order(4) + ) + .arg(Arg::with_name(options::PRINT_DATABASE) + .long("print-database") + .short("p") + .help("print the byte counts") + .display_order(5) + ) + .arg(Arg::with_name(options::FILE) + .hidden(true) + .multiple(true) + ) + .get_matches_from(&args); + + let files = matches.values_of(options::FILE) + .map_or(vec![], |file_values| file_values.collect()); + + // clap provides .conflicts_with / .conflicts_with_all, but we want to + // manually handle conflicts so we can match the output of GNU coreutils + if (matches.is_present(options::CSH) + || matches.is_present(options::C_SHELL) + || matches.is_present(options::SH) + || matches.is_present(options::BOURNE_SHELL) + ) + && matches.is_present(options::PRINT_DATABASE) { show_usage_error!( "the options to output dircolors' internal database and\nto select a shell \ @@ -82,12 +146,12 @@ pub fn uumain(args: impl uucore::Args) -> i32 { return 1; } - if matches.opt_present("print-database") { - if !matches.free.is_empty() { + if matches.is_present(options::PRINT_DATABASE) { + if !files.is_empty() { show_usage_error!( "extra operand ‘{}’\nfile operands cannot be combined with \ --print-database (-p)", - matches.free[0] + files[0] ); return 1; } @@ -96,9 +160,9 @@ pub fn uumain(args: impl uucore::Args) -> i32 { } let mut out_format = OutputFmt::Unknown; - if matches.opt_present("csh") || matches.opt_present("c-shell") { + if matches.is_present(options::CSH) || matches.is_present(options::C_SHELL) { out_format = OutputFmt::CShell; - } else if matches.opt_present("sh") || matches.opt_present("bourne-shell") { + } else if matches.is_present(options::SH) || matches.is_present(options::BOURNE_SHELL) { out_format = OutputFmt::Shell; } @@ -113,24 +177,24 @@ pub fn uumain(args: impl uucore::Args) -> i32 { } let result; - if matches.free.is_empty() { + if files.is_empty() { result = parse(INTERNAL_DB.lines(), out_format, "") } else { - if matches.free.len() > 1 { - show_usage_error!("extra operand ‘{}’", matches.free[1]); + if files.len() > 1 { + show_usage_error!("extra operand ‘{}’", files[1]); return 1; } - match File::open(matches.free[0].as_str()) { + match File::open(files[0]) { Ok(f) => { let fin = BufReader::new(f); result = parse( fin.lines().filter_map(Result::ok), out_format, - matches.free[0].as_str(), + files[0], ) } Err(e) => { - show_error!("{}: {}", matches.free[0], e); + show_error!("{}: {}", files[0], e); return 1; } } From 850a56cceaa758b4122e5b644b28d9bf8aa3bb70 Mon Sep 17 00:00:00 2001 From: Mitchell Mebane Date: Tue, 1 Jun 2021 19:13:20 -0500 Subject: [PATCH 2/3] dircolors: rustfmt --- src/uu/dircolors/src/dircolors.rs | 80 ++++++++++++++----------------- 1 file changed, 37 insertions(+), 43 deletions(-) diff --git a/src/uu/dircolors/src/dircolors.rs b/src/uu/dircolors/src/dircolors.rs index 8e13e281c..078270791 100644 --- a/src/uu/dircolors/src/dircolors.rs +++ b/src/uu/dircolors/src/dircolors.rs @@ -16,7 +16,7 @@ use std::env; use std::fs::File; use std::io::{BufRead, BufReader}; -use clap::{App, Arg, crate_version}; +use clap::{crate_version, App, Arg}; mod options { pub const SH: &str = "sh"; @@ -65,11 +65,7 @@ pub fn guess_syntax() -> OutputFmt { } fn get_usage() -> String { - format!( - "{0} {1}", - executable!(), - SYNTAX - ) + format!("{0} {1}", executable!(), SYNTAX) } pub fn uumain(args: impl uucore::Args) -> i32 { @@ -93,50 +89,52 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .about(SUMMARY) .usage(&usage[..]) .after_help(LONG_HELP) - .arg(Arg::with_name(options::SH) - .long("sh") - .short("b") - .help("output Bourne shell code to set LS_COLORS") - .display_order(1) + .arg( + Arg::with_name(options::SH) + .long("sh") + .short("b") + .help("output Bourne shell code to set LS_COLORS") + .display_order(1), ) - .arg(Arg::with_name(options::BOURNE_SHELL) - .long("bourne-shell") - .help("output Bourne shell code to set LS_COLORS") - .display_order(2) + .arg( + Arg::with_name(options::BOURNE_SHELL) + .long("bourne-shell") + .help("output Bourne shell code to set LS_COLORS") + .display_order(2), ) - .arg(Arg::with_name(options::CSH) - .long("csh") - .short("c") - .help("output C shell code to set LS_COLORS") - .display_order(3) + .arg( + Arg::with_name(options::CSH) + .long("csh") + .short("c") + .help("output C shell code to set LS_COLORS") + .display_order(3), ) - .arg(Arg::with_name(options::C_SHELL) - .long("c-shell") - .help("output C shell code to set LS_COLORS") - .display_order(4) + .arg( + Arg::with_name(options::C_SHELL) + .long("c-shell") + .help("output C shell code to set LS_COLORS") + .display_order(4), ) - .arg(Arg::with_name(options::PRINT_DATABASE) - .long("print-database") - .short("p") - .help("print the byte counts") - .display_order(5) - ) - .arg(Arg::with_name(options::FILE) - .hidden(true) - .multiple(true) + .arg( + Arg::with_name(options::PRINT_DATABASE) + .long("print-database") + .short("p") + .help("print the byte counts") + .display_order(5), ) + .arg(Arg::with_name(options::FILE).hidden(true).multiple(true)) .get_matches_from(&args); - let files = matches.values_of(options::FILE) + let files = matches + .values_of(options::FILE) .map_or(vec![], |file_values| file_values.collect()); // clap provides .conflicts_with / .conflicts_with_all, but we want to // manually handle conflicts so we can match the output of GNU coreutils if (matches.is_present(options::CSH) - || matches.is_present(options::C_SHELL) - || matches.is_present(options::SH) - || matches.is_present(options::BOURNE_SHELL) - ) + || matches.is_present(options::C_SHELL) + || matches.is_present(options::SH) + || matches.is_present(options::BOURNE_SHELL)) && matches.is_present(options::PRINT_DATABASE) { show_usage_error!( @@ -187,11 +185,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { match File::open(files[0]) { Ok(f) => { let fin = BufReader::new(f); - result = parse( - fin.lines().filter_map(Result::ok), - out_format, - files[0], - ) + result = parse(fin.lines().filter_map(Result::ok), out_format, files[0]) } Err(e) => { show_error!("{}: {}", files[0], e); From 754082886c602414846f59bf9c3f9a9d04343a6b Mon Sep 17 00:00:00 2001 From: Mitchell Mebane Date: Thu, 3 Jun 2021 20:49:25 -0500 Subject: [PATCH 3/3] dircolors: Address code review comments --- src/uu/dircolors/src/dircolors.rs | 42 +++++++------------------------ 1 file changed, 9 insertions(+), 33 deletions(-) diff --git a/src/uu/dircolors/src/dircolors.rs b/src/uu/dircolors/src/dircolors.rs index 078270791..2fa2e8b91 100644 --- a/src/uu/dircolors/src/dircolors.rs +++ b/src/uu/dircolors/src/dircolors.rs @@ -19,9 +19,7 @@ use std::io::{BufRead, BufReader}; use clap::{crate_version, App, Arg}; mod options { - pub const SH: &str = "sh"; pub const BOURNE_SHELL: &str = "bourne-shell"; - pub const CSH: &str = "csh"; pub const C_SHELL: &str = "c-shell"; pub const PRINT_DATABASE: &str = "print-database"; pub const FILE: &str = "FILE"; @@ -75,52 +73,33 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let usage = get_usage(); - /* Clap has .visible_alias, but it generates help like this - * -b, --sh output Bourne shell code to set LS_COLORS [aliases: bourne-shell] - * whereas we want help like this - * -b, --sh output Bourne shell code to set LS_COLORS - * --bourne-shell output Bourne shell code to set LS_COLORS - * (or preferably like the original, but that doesn't seem possible with clap:) - * -b, --sh, --bourne-shell output Bourne shell code to set LS_COLORS - * therefore, command aliases are defined manually as multiple commands - */ let matches = App::new(executable!()) .version(crate_version!()) .about(SUMMARY) .usage(&usage[..]) .after_help(LONG_HELP) .arg( - Arg::with_name(options::SH) + Arg::with_name(options::BOURNE_SHELL) .long("sh") .short("b") + .visible_alias("bourne-shell") .help("output Bourne shell code to set LS_COLORS") .display_order(1), ) .arg( - Arg::with_name(options::BOURNE_SHELL) - .long("bourne-shell") - .help("output Bourne shell code to set LS_COLORS") - .display_order(2), - ) - .arg( - Arg::with_name(options::CSH) + Arg::with_name(options::C_SHELL) .long("csh") .short("c") + .visible_alias("c-shell") .help("output C shell code to set LS_COLORS") - .display_order(3), - ) - .arg( - Arg::with_name(options::C_SHELL) - .long("c-shell") - .help("output C shell code to set LS_COLORS") - .display_order(4), + .display_order(2), ) .arg( Arg::with_name(options::PRINT_DATABASE) .long("print-database") .short("p") .help("print the byte counts") - .display_order(5), + .display_order(3), ) .arg(Arg::with_name(options::FILE).hidden(true).multiple(true)) .get_matches_from(&args); @@ -131,10 +110,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { // clap provides .conflicts_with / .conflicts_with_all, but we want to // manually handle conflicts so we can match the output of GNU coreutils - if (matches.is_present(options::CSH) - || matches.is_present(options::C_SHELL) - || matches.is_present(options::SH) - || matches.is_present(options::BOURNE_SHELL)) + if (matches.is_present(options::C_SHELL) || matches.is_present(options::BOURNE_SHELL)) && matches.is_present(options::PRINT_DATABASE) { show_usage_error!( @@ -158,9 +134,9 @@ pub fn uumain(args: impl uucore::Args) -> i32 { } let mut out_format = OutputFmt::Unknown; - if matches.is_present(options::CSH) || matches.is_present(options::C_SHELL) { + if matches.is_present(options::C_SHELL) { out_format = OutputFmt::CShell; - } else if matches.is_present(options::SH) || matches.is_present(options::BOURNE_SHELL) { + } else if matches.is_present(options::BOURNE_SHELL) { out_format = OutputFmt::Shell; }