From 58b0aeabee8cb137876cc2bcbf8fffb31a5cec6b Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sun, 18 Oct 2020 21:27:49 +0200 Subject: [PATCH 1/2] refactor(sort): move to clap --- Cargo.lock | 2 +- src/uu/sort/Cargo.toml | 2 +- src/uu/sort/src/sort.rs | 221 ++++++++++++++++++++++++---------------- 3 files changed, 138 insertions(+), 87 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 66fe84236..a0f89567f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2096,7 +2096,7 @@ dependencies = [ name = "uu_sort" 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)", "itertools 0.8.2 (registry+https://github.com/rust-lang/crates.io-index)", "semver 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)", "uucore 0.0.4 (git+https://github.com/uutils/uucore.git?branch=canary)", diff --git a/src/uu/sort/Cargo.toml b/src/uu/sort/Cargo.toml index c9e643cf3..fca462b04 100644 --- a/src/uu/sort/Cargo.toml +++ b/src/uu/sort/Cargo.toml @@ -15,7 +15,7 @@ edition = "2018" path = "src/sort.rs" [dependencies] -getopts = "0.2.18" +clap = "2.33" itertools = "0.8.0" semver = "0.9.0" uucore = { version="0.0.4", package="uucore", git="https://github.com/uutils/uucore.git", branch="canary", features=["fs"] } diff --git a/src/uu/sort/src/sort.rs b/src/uu/sort/src/sort.rs index 11d243bba..18e94f4a6 100644 --- a/src/uu/sort/src/sort.rs +++ b/src/uu/sort/src/sort.rs @@ -8,13 +8,14 @@ // spell-checker:ignore (ToDO) outfile nondictionary -extern crate getopts; +extern crate clap; extern crate semver; extern crate itertools; #[macro_use] extern crate uucore; +use clap::{App, Arg}; use itertools::Itertools; use semver::Version; use std::cmp::Ordering; @@ -26,8 +27,23 @@ use std::path::Path; use uucore::fs::is_stdin_interactive; // for Iterator::dedup() static NAME: &str = "sort"; +static ABOUT: &str = "sort lines of text files"; static VERSION: &str = env!("CARGO_PKG_VERSION"); +const OPT_NUMERIC_SORT: &str = "numeric-sort"; +const OPT_HUMAN_NUMERIC_SORT: &str = "human-numeric-sort"; +const OPT_MONTH_SORT: &str = "month-sort"; +const OPT_VERSION_SORT: &str = "version-sort"; +const OPT_OUTPUT: &str = "output"; +const OPT_FILES: &str = "files"; +const OPT_MERGE: &str = "merge"; +const OPT_REVERSE: &str = "reverse"; +const OPT_STABLE: &str = "stable"; +const OPT_UNIQUE: &str = "unique"; +const OPT_CHECK: &str = "check"; +const OPT_DICTIONARY_ORDER: &str = "dictionary-order"; +const OPT_IGNORE_CASE: &str = "ignore-case"; + const DECIMAL_PT: char = '.'; const THOUSANDS_SEP: char = ','; @@ -142,68 +158,9 @@ impl<'a> Iterator for FileMerger<'a> { } } } - -pub fn uumain(args: impl uucore::Args) -> i32 { - let args = args.collect_str(); - - let mut settings: Settings = Default::default(); - let mut opts = getopts::Options::new(); - - opts.optflag( - "d", - "dictionary-order", - "consider only blanks and alphanumeric characters", - ); - opts.optflag( - "f", - "ignore-case", - "fold lower case to upper case characters", - ); - opts.optflag( - "n", - "numeric-sort", - "compare according to string numerical value", - ); - opts.optflag( - "h", - "human-numeric-sort", - "compare according to human readable sizes, eg 1M > 100k", - ); - opts.optflag( - "M", - "month-sort", - "compare according to month name abbreviation", - ); - opts.optflag("r", "reverse", "reverse the output"); - opts.optflag("h", "help", "display this help and exit"); - opts.optflag("", "version", "output version information and exit"); - opts.optflag("m", "merge", "merge already sorted files; do not sort"); - opts.optopt( - "o", - "output", - "write output to FILENAME instead of stdout", - "FILENAME", - ); - opts.optflag( - "s", - "stable", - "stabilize sort by disabling last-resort comparison", - ); - opts.optflag("u", "unique", "output only the first of an equal run"); - opts.optflag( - "V", - "version-sort", - "Sort by SemVer version number, eg 1.12.2 > 1.1.2", - ); - opts.optflag("c", "check", "check for sorted input; do not sort"); - - let matches = match opts.parse(&args[1..]) { - Ok(m) => m, - Err(f) => crash!(1, "Invalid options\n{}", f), - }; - if matches.opt_present("help") { - let msg = format!( - "{0} {1} +fn get_usage() -> String { + format!( + "{0} {1} Usage: {0} [OPTION]... [FILE]... @@ -213,44 +170,138 @@ Write the sorted concatenation of all FILE(s) to standard output. Mandatory arguments for long options are mandatory for short options too. With no FILE, or when FILE is -, read standard input.", - NAME, VERSION - ); - print!("{}", opts.usage(&msg)); - return 0; - } + NAME, VERSION + ) +} - if matches.opt_present("version") { - println!("{} {}", NAME, VERSION); - return 0; - } +pub fn uumain(args: impl uucore::Args) -> i32 { + let args = args.collect_str(); + let usage = get_usage(); + let mut settings: Settings = Default::default(); - settings.mode = if matches.opt_present("numeric-sort") { + let matches = App::new(executable!()) + .version(VERSION) + .about(ABOUT) + .usage(&usage[..]) + .arg( + Arg::with_name(OPT_DICTIONARY_ORDER) + .short("d") + .long(OPT_DICTIONARY_ORDER) + .help("consider only blanks and alphanumeric characters"), + ) + .arg( + Arg::with_name(OPT_IGNORE_CASE) + .short("f") + .long(OPT_IGNORE_CASE) + .help("fold lower case to upper case characters"), + ) + .arg( + Arg::with_name(OPT_NUMERIC_SORT) + .short("n") + .long(OPT_NUMERIC_SORT) + .help("compare according to string numerical value"), + ) + .arg( + Arg::with_name(OPT_HUMAN_NUMERIC_SORT) + .short("h") + .long(OPT_HUMAN_NUMERIC_SORT) + .help("compare according to human readable sizes, eg 1M > 100k"), + ) + .arg( + Arg::with_name(OPT_MONTH_SORT) + .short("M") + .long(OPT_MONTH_SORT) + .help("compare according to month name abbreviation"), + ) + .arg( + Arg::with_name(OPT_REVERSE) + .short("r") + .long(OPT_REVERSE) + .help("reverse the output"), + ) + .arg( + Arg::with_name("h") + .long("help") + .help("display this help and exit"), + ) + .arg( + Arg::with_name("version") + .long("version") + .help("output version information and exit"), + ) + .arg( + Arg::with_name(OPT_MERGE) + .short("m") + .long(OPT_MERGE) + .help("merge already sorted files; do not sort"), + ) + .arg( + Arg::with_name(OPT_OUTPUT) + .short("o") + .long(OPT_OUTPUT) + .help("write output to FILENAME instead of stdout") + .takes_value(true) + .value_name("FILENAME"), + ) + .arg( + Arg::with_name(OPT_STABLE) + .short("s") + .long(OPT_STABLE) + .help("stabilize sort by disabling last-resort comparison"), + ) + .arg( + Arg::with_name(OPT_UNIQUE) + .short("u") + .long(OPT_UNIQUE) + .help("output only the first of an equal run"), + ) + .arg( + Arg::with_name(OPT_VERSION_SORT) + .short("V") + .long(OPT_VERSION_SORT) + .help("Sort by SemVer version number, eg 1.12.2 > 1.1.2"), + ) + .arg( + Arg::with_name(OPT_CHECK) + .short("c") + .long(OPT_CHECK) + .help("check for sorted input; do not sort"), + ) + .arg(Arg::with_name(OPT_FILES).multiple(true).takes_value(true)) + .get_matches_from(args); + + let mut files: Vec = matches + .values_of(OPT_FILES) + .map(|v| v.map(ToString::to_string).collect()) + .unwrap_or_default(); + + settings.mode = if matches.is_present(OPT_NUMERIC_SORT) { SortMode::Numeric - } else if matches.opt_present("human-numeric-sort") { + } else if matches.is_present(OPT_HUMAN_NUMERIC_SORT) { SortMode::HumanNumeric - } else if matches.opt_present("month-sort") { + } else if matches.is_present(OPT_MONTH_SORT) { SortMode::Month - } else if matches.opt_present("version-sort") { + } else if matches.is_present(OPT_VERSION_SORT) { SortMode::Version } else { SortMode::Default }; - settings.merge = matches.opt_present("merge"); - settings.reverse = matches.opt_present("reverse"); - settings.outfile = matches.opt_str("output"); - settings.stable = matches.opt_present("stable"); - settings.unique = matches.opt_present("unique"); - settings.check = matches.opt_present("check"); + settings.merge = matches.is_present(OPT_MERGE); + settings.reverse = matches.is_present(OPT_REVERSE); + settings.outfile = matches.value_of(OPT_OUTPUT).map(String::from); + settings.stable = matches.is_present(OPT_STABLE); + settings.unique = matches.is_present(OPT_UNIQUE); + settings.check = matches.is_present(OPT_CHECK); - if matches.opt_present("dictionary-order") { + if matches.is_present(OPT_DICTIONARY_ORDER) { settings.transform_fns.push(remove_nondictionary_chars); } - if matches.opt_present("ignore-case") { + if matches.is_present(OPT_IGNORE_CASE) { settings.transform_fns.push(|s| s.to_uppercase()); } - let mut files = matches.free; + //let mut files = matches.free; if files.is_empty() { /* if no file, default to stdin */ files.push("-".to_owned()); From dc4eb7932917b66b967be2cdc154d70e27296949 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Thu, 22 Oct 2020 23:31:04 +0200 Subject: [PATCH 2/2] refactor/sort ~ changes based on PR feedback - change `const`=>`static` and remove unneeded help/version (supplied by default by `clap`) - update of the ABOUT description - move to alphabetical order (where reasonable) - rename OPT_FILES => ARG_FILES - change the order of the declarations --- src/uu/sort/src/sort.rs | 130 +++++++++++++++++++--------------------- 1 file changed, 62 insertions(+), 68 deletions(-) diff --git a/src/uu/sort/src/sort.rs b/src/uu/sort/src/sort.rs index 18e94f4a6..5e88c7d3b 100644 --- a/src/uu/sort/src/sort.rs +++ b/src/uu/sort/src/sort.rs @@ -27,25 +27,27 @@ use std::path::Path; use uucore::fs::is_stdin_interactive; // for Iterator::dedup() static NAME: &str = "sort"; -static ABOUT: &str = "sort lines of text files"; +static ABOUT: &str = "Display sorted concatenation of all FILE(s)."; static VERSION: &str = env!("CARGO_PKG_VERSION"); -const OPT_NUMERIC_SORT: &str = "numeric-sort"; -const OPT_HUMAN_NUMERIC_SORT: &str = "human-numeric-sort"; -const OPT_MONTH_SORT: &str = "month-sort"; -const OPT_VERSION_SORT: &str = "version-sort"; -const OPT_OUTPUT: &str = "output"; -const OPT_FILES: &str = "files"; -const OPT_MERGE: &str = "merge"; -const OPT_REVERSE: &str = "reverse"; -const OPT_STABLE: &str = "stable"; -const OPT_UNIQUE: &str = "unique"; -const OPT_CHECK: &str = "check"; -const OPT_DICTIONARY_ORDER: &str = "dictionary-order"; -const OPT_IGNORE_CASE: &str = "ignore-case"; +static OPT_HUMAN_NUMERIC_SORT: &str = "human-numeric-sort"; +static OPT_MONTH_SORT: &str = "month-sort"; +static OPT_NUMERIC_SORT: &str = "numeric-sort"; +static OPT_VERSION_SORT: &str = "version-sort"; -const DECIMAL_PT: char = '.'; -const THOUSANDS_SEP: char = ','; +static OPT_DICTIONARY_ORDER: &str = "dictionary-order"; +static OPT_MERGE: &str = "merge"; +static OPT_CHECK: &str = "check"; +static OPT_IGNORE_CASE: &str = "ignore-case"; +static OPT_OUTPUT: &str = "output"; +static OPT_REVERSE: &str = "reverse"; +static OPT_STABLE: &str = "stable"; +static OPT_UNIQUE: &str = "unique"; + +static ARG_FILES: &str = "files"; + +static DECIMAL_PT: char = '.'; +static THOUSANDS_SEP: char = ','; enum SortMode { Numeric, @@ -183,24 +185,6 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .version(VERSION) .about(ABOUT) .usage(&usage[..]) - .arg( - Arg::with_name(OPT_DICTIONARY_ORDER) - .short("d") - .long(OPT_DICTIONARY_ORDER) - .help("consider only blanks and alphanumeric characters"), - ) - .arg( - Arg::with_name(OPT_IGNORE_CASE) - .short("f") - .long(OPT_IGNORE_CASE) - .help("fold lower case to upper case characters"), - ) - .arg( - Arg::with_name(OPT_NUMERIC_SORT) - .short("n") - .long(OPT_NUMERIC_SORT) - .help("compare according to string numerical value"), - ) .arg( Arg::with_name(OPT_HUMAN_NUMERIC_SORT) .short("h") @@ -214,20 +198,22 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .help("compare according to month name abbreviation"), ) .arg( - Arg::with_name(OPT_REVERSE) - .short("r") - .long(OPT_REVERSE) - .help("reverse the output"), + Arg::with_name(OPT_NUMERIC_SORT) + .short("n") + .long(OPT_NUMERIC_SORT) + .help("compare according to string numerical value"), ) .arg( - Arg::with_name("h") - .long("help") - .help("display this help and exit"), + Arg::with_name(OPT_VERSION_SORT) + .short("V") + .long(OPT_VERSION_SORT) + .help("Sort by SemVer version number, eg 1.12.2 > 1.1.2"), ) .arg( - Arg::with_name("version") - .long("version") - .help("output version information and exit"), + Arg::with_name(OPT_DICTIONARY_ORDER) + .short("d") + .long(OPT_DICTIONARY_ORDER) + .help("consider only blanks and alphanumeric characters"), ) .arg( Arg::with_name(OPT_MERGE) @@ -235,6 +221,18 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .long(OPT_MERGE) .help("merge already sorted files; do not sort"), ) + .arg( + Arg::with_name(OPT_CHECK) + .short("c") + .long(OPT_CHECK) + .help("check for sorted input; do not sort"), + ) + .arg( + Arg::with_name(OPT_IGNORE_CASE) + .short("f") + .long(OPT_IGNORE_CASE) + .help("fold lower case to upper case characters"), + ) .arg( Arg::with_name(OPT_OUTPUT) .short("o") @@ -243,6 +241,12 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .takes_value(true) .value_name("FILENAME"), ) + .arg( + Arg::with_name(OPT_REVERSE) + .short("r") + .long(OPT_REVERSE) + .help("reverse the output"), + ) .arg( Arg::with_name(OPT_STABLE) .short("s") @@ -255,52 +259,42 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .long(OPT_UNIQUE) .help("output only the first of an equal run"), ) - .arg( - Arg::with_name(OPT_VERSION_SORT) - .short("V") - .long(OPT_VERSION_SORT) - .help("Sort by SemVer version number, eg 1.12.2 > 1.1.2"), - ) - .arg( - Arg::with_name(OPT_CHECK) - .short("c") - .long(OPT_CHECK) - .help("check for sorted input; do not sort"), - ) - .arg(Arg::with_name(OPT_FILES).multiple(true).takes_value(true)) + .arg(Arg::with_name(ARG_FILES).multiple(true).takes_value(true)) .get_matches_from(args); let mut files: Vec = matches - .values_of(OPT_FILES) + .values_of(ARG_FILES) .map(|v| v.map(ToString::to_string).collect()) .unwrap_or_default(); - settings.mode = if matches.is_present(OPT_NUMERIC_SORT) { - SortMode::Numeric - } else if matches.is_present(OPT_HUMAN_NUMERIC_SORT) { + settings.mode = if matches.is_present(OPT_HUMAN_NUMERIC_SORT) { SortMode::HumanNumeric } else if matches.is_present(OPT_MONTH_SORT) { SortMode::Month + } else if matches.is_present(OPT_NUMERIC_SORT) { + SortMode::Numeric } else if matches.is_present(OPT_VERSION_SORT) { SortMode::Version } else { SortMode::Default }; - settings.merge = matches.is_present(OPT_MERGE); - settings.reverse = matches.is_present(OPT_REVERSE); - settings.outfile = matches.value_of(OPT_OUTPUT).map(String::from); - settings.stable = matches.is_present(OPT_STABLE); - settings.unique = matches.is_present(OPT_UNIQUE); - settings.check = matches.is_present(OPT_CHECK); - if matches.is_present(OPT_DICTIONARY_ORDER) { settings.transform_fns.push(remove_nondictionary_chars); } + + settings.merge = matches.is_present(OPT_MERGE); + settings.check = matches.is_present(OPT_CHECK); + if matches.is_present(OPT_IGNORE_CASE) { settings.transform_fns.push(|s| s.to_uppercase()); } + settings.outfile = matches.value_of(OPT_OUTPUT).map(String::from); + settings.reverse = matches.is_present(OPT_REVERSE); + settings.stable = matches.is_present(OPT_STABLE); + settings.unique = matches.is_present(OPT_UNIQUE); + //let mut files = matches.free; if files.is_empty() { /* if no file, default to stdin */