From db79b5abb2938f92ed0115030aea0f2c94c07ff0 Mon Sep 17 00:00:00 2001 From: Hanif Bin Ariffin Date: Fri, 9 Jul 2021 23:22:12 +0800 Subject: [PATCH 1/4] `tr` should error out when if user provides more than 2 sets Signed-off-by: Hanif Bin Ariffin --- src/uu/tr/src/tr.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/uu/tr/src/tr.rs b/src/uu/tr/src/tr.rs index 28ce70c22..7df404f6f 100644 --- a/src/uu/tr/src/tr.rs +++ b/src/uu/tr/src/tr.rs @@ -235,7 +235,7 @@ fn get_usage() -> String { } fn get_long_usage() -> String { - String::from( + format!( "Translate, squeeze, and/or delete characters from standard input, writing to standard output.", ) @@ -259,7 +259,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let squeeze_flag = matches.is_present(options::SQUEEZE); let truncate_flag = matches.is_present(options::TRUNCATE); - let sets: Vec = match matches.values_of(options::SETS) { + let sets = match matches.values_of(options::SETS) { Some(v) => v.map(|v| v.to_string()).collect(), None => vec![], }; @@ -281,6 +281,15 @@ pub fn uumain(args: impl uucore::Args) -> i32 { return 1; } + if sets.len() > 2 { + show_error!( + "extra operand '{}'\nTry `{} --help` for more information.", + sets[2], + executable!() + ); + return 1; + } + let stdin = stdin(); let mut locked_stdin = stdin.lock(); let stdout = stdout(); From c4c6819daea63fc32e641e83cfc5b3b9c5ccff19 Mon Sep 17 00:00:00 2001 From: Hanif Bin Ariffin Date: Fri, 9 Jul 2021 23:51:37 +0800 Subject: [PATCH 2/4] Reimplemented simply using args Signed-off-by: Hanif Bin Ariffin --- src/uu/tr/src/tr.rs | 16 ++++++---------- tests/by-util/test_tr.rs | 8 ++++++++ 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/uu/tr/src/tr.rs b/src/uu/tr/src/tr.rs index 7df404f6f..1a696f3b9 100644 --- a/src/uu/tr/src/tr.rs +++ b/src/uu/tr/src/tr.rs @@ -281,15 +281,6 @@ pub fn uumain(args: impl uucore::Args) -> i32 { return 1; } - if sets.len() > 2 { - show_error!( - "extra operand '{}'\nTry `{} --help` for more information.", - sets[2], - executable!() - ); - return 1; - } - let stdin = stdin(); let mut locked_stdin = stdin.lock(); let stdout = stdout(); @@ -369,5 +360,10 @@ pub fn uu_app() -> App<'static, 'static> { .short("t") .help("first truncate SET1 to length of SET2"), ) - .arg(Arg::with_name(options::SETS).multiple(true)) + .arg( + Arg::with_name(options::SETS) + .multiple(true) + .takes_value(true) + .max_values(2), + ) } diff --git a/tests/by-util/test_tr.rs b/tests/by-util/test_tr.rs index 936af2ca8..4107fe87d 100644 --- a/tests/by-util/test_tr.rs +++ b/tests/by-util/test_tr.rs @@ -275,3 +275,11 @@ fn test_interpret_backslash_at_eol_literally() { .succeeds() .stdout_is("\\"); } + +#[test] +fn test_more_than_2_sets() { + new_ucmd!() + .args(&["'abcdefgh'", "'a", "'b'"]) + .pipe_in("hello world") + .fails(); +} From 199b4e437b6102cf72abf6d93d1a1b6043d3ada5 Mon Sep 17 00:00:00 2001 From: Hanif Bin Ariffin Date: Sat, 10 Jul 2021 18:49:11 +0800 Subject: [PATCH 3/4] Removed manual checks to just using clap instead Also added tests for this. Signed-off-by: Hanif Bin Ariffin --- src/uu/tr/src/tr.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/uu/tr/src/tr.rs b/src/uu/tr/src/tr.rs index 1a696f3b9..153128bfd 100644 --- a/src/uu/tr/src/tr.rs +++ b/src/uu/tr/src/tr.rs @@ -364,6 +364,7 @@ pub fn uu_app() -> App<'static, 'static> { Arg::with_name(options::SETS) .multiple(true) .takes_value(true) + .min_values(1) .max_values(2), ) } From 15f6e351630ac46374c5c79effeef7995ad918a2 Mon Sep 17 00:00:00 2001 From: Hanif Bin Ariffin Date: Sat, 10 Jul 2021 18:51:24 +0800 Subject: [PATCH 4/4] Use map/default instead of match Signed-off-by: Hanif Bin Ariffin --- src/uu/tr/src/tr.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/uu/tr/src/tr.rs b/src/uu/tr/src/tr.rs index 153128bfd..eedf81d91 100644 --- a/src/uu/tr/src/tr.rs +++ b/src/uu/tr/src/tr.rs @@ -259,10 +259,10 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let squeeze_flag = matches.is_present(options::SQUEEZE); let truncate_flag = matches.is_present(options::TRUNCATE); - let sets = match matches.values_of(options::SETS) { - Some(v) => v.map(|v| v.to_string()).collect(), - None => vec![], - }; + let sets = matches + .values_of(options::SETS) + .map(|v| v.map(ToString::to_string).collect::>()) + .unwrap_or_default(); if sets.is_empty() { show_error!(