From 4a305b32c6e77af76e1af3a42b72eeeac4855d23 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Wed, 21 Apr 2021 17:49:40 +0200 Subject: [PATCH 1/3] sort: disallow certain flags with -d and -i GNU sort disallows these combinations, presumably because they are likely not what the user really wants. Ignoring characters would cause things to be put together that aren't together in the input. For example, -dn would cause "0.12" or "0,12" to be parsed as "12" which is highly unexpected and confusing. --- src/uu/sort/src/sort.rs | 34 +++++++++++++++++++++++++++++----- tests/by-util/test_sort.rs | 27 +++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 5 deletions(-) diff --git a/src/uu/sort/src/sort.rs b/src/uu/sort/src/sort.rs index 07b852921..7090a98ed 100644 --- a/src/uu/sort/src/sort.rs +++ b/src/uu/sort/src/sort.rs @@ -84,7 +84,7 @@ static THOUSANDS_SEP: char = ','; static NEGATIVE: char = '-'; static POSITIVE: char = '+'; -#[derive(Eq, Ord, PartialEq, PartialOrd, Clone)] +#[derive(Eq, Ord, PartialEq, PartialOrd, Clone, Copy)] enum SortMode { Numeric, HumanNumeric, @@ -153,7 +153,7 @@ struct KeySettings { impl From<&GlobalSettings> for KeySettings { fn from(settings: &GlobalSettings) -> Self { Self { - mode: settings.mode.clone(), + mode: settings.mode, ignore_blanks: settings.ignore_blanks, ignore_case: settings.ignore_case, ignore_non_printing: settings.ignore_non_printing, @@ -407,6 +407,28 @@ impl KeyPosition { crash!(1, "invalid option for key: `{}`", c) } } + // All numeric sorts and month sort conflict with dictionary_order and ignore_non_printing. + // Instad of reporting an error, let them overwrite each other. + + // FIXME: This should only override if the overridden flag is a global flag. + // If conflicting flags are attached to the key, GNU sort crashes and we should probably too. + match option { + 'h' | 'n' | 'g' | 'M' => { + settings.dictionary_order = false; + settings.ignore_non_printing = false; + } + 'd' | 'i' => { + settings.mode = match settings.mode { + SortMode::Numeric + | SortMode::HumanNumeric + | SortMode::GeneralNumeric + | SortMode::Month => SortMode::Default, + // Only SortMode::Default and SortMode::Version work with dictionary_order and ignore_non_printing + m @ (SortMode::Default | SortMode::Version) => m, + } + } + _ => {} + } } // Strip away option characters from the original value so we can parse it later *value_with_options = &value_with_options[..options_start]; @@ -651,7 +673,8 @@ pub fn uumain(args: impl uucore::Args) -> i32 { Arg::with_name(OPT_DICTIONARY_ORDER) .short("d") .long(OPT_DICTIONARY_ORDER) - .help("consider only blanks and alphanumeric characters"), + .help("consider only blanks and alphanumeric characters") + .conflicts_with_all(&[OPT_NUMERIC_SORT, OPT_GENERAL_NUMERIC_SORT, OPT_HUMAN_NUMERIC_SORT, OPT_MONTH_SORT]), ) .arg( Arg::with_name(OPT_MERGE) @@ -679,9 +702,10 @@ pub fn uumain(args: impl uucore::Args) -> i32 { ) .arg( Arg::with_name(OPT_IGNORE_NONPRINTING) - .short("-i") + .short("i") .long(OPT_IGNORE_NONPRINTING) - .help("ignore nonprinting characters"), + .help("ignore nonprinting characters") + .conflicts_with_all(&[OPT_NUMERIC_SORT, OPT_GENERAL_NUMERIC_SORT, OPT_HUMAN_NUMERIC_SORT, OPT_MONTH_SORT]), ) .arg( Arg::with_name(OPT_IGNORE_BLANKS) diff --git a/tests/by-util/test_sort.rs b/tests/by-util/test_sort.rs index a4a9a383c..4ee8826e3 100644 --- a/tests/by-util/test_sort.rs +++ b/tests/by-util/test_sort.rs @@ -581,3 +581,30 @@ fn test_check_silent() { .fails() .stdout_is(""); } + +#[test] +fn test_dictionary_and_nonprinting_conflicts() { + let conflicting_args = ["n", "h", "g", "M"]; + for restricted_arg in &["d", "i"] { + for conflicting_arg in &conflicting_args { + new_ucmd!() + .arg(&format!("-{}{}", restricted_arg, conflicting_arg)) + .fails(); + } + for conflicting_arg in &conflicting_args { + new_ucmd!() + .args(&[ + format!("-{}", restricted_arg).as_str(), + "-k", + &format!("1,1{}", conflicting_arg), + ]) + .succeeds(); + } + for conflicting_arg in &conflicting_args { + // FIXME: this should ideally fail. + new_ucmd!() + .args(&["-k", &format!("1{},1{}", restricted_arg, conflicting_arg)]) + .succeeds(); + } + } +} From b08f92cfa5cec53ff99a902ea3e4b5689b1bd922 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Wed, 21 Apr 2021 17:50:22 +0200 Subject: [PATCH 2/3] remove unneeded 'static --- tests/by-util/test_sort.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/by-util/test_sort.rs b/tests/by-util/test_sort.rs index 4ee8826e3..0e6ac2e62 100644 --- a/tests/by-util/test_sort.rs +++ b/tests/by-util/test_sort.rs @@ -63,7 +63,7 @@ fn test_check_zero_terminated_success() { #[test] fn test_random_shuffle_len() { // check whether output is the same length as the input - const FILE: &'static str = "default_unsorted_ints.expected"; + const FILE: &str = "default_unsorted_ints.expected"; let (at, _ucmd) = at_and_ucmd!(); let result = new_ucmd!().arg("-R").arg(FILE).run().stdout_move_str(); let expected = at.read(FILE); @@ -75,7 +75,7 @@ fn test_random_shuffle_len() { #[test] fn test_random_shuffle_contains_all_lines() { // check whether lines of input are all in output - const FILE: &'static str = "default_unsorted_ints.expected"; + const FILE: &str = "default_unsorted_ints.expected"; let (at, _ucmd) = at_and_ucmd!(); let result = new_ucmd!().arg("-R").arg(FILE).run().stdout_move_str(); let expected = at.read(FILE); @@ -90,7 +90,7 @@ fn test_random_shuffle_two_runs_not_the_same() { // check to verify that two random shuffles are not equal; this has the // potential to fail in the very unlikely event that the random order is the same // as the starting order, or if both random sorts end up having the same order. - const FILE: &'static str = "default_unsorted_ints.expected"; + const FILE: &str = "default_unsorted_ints.expected"; let (at, _ucmd) = at_and_ucmd!(); let result = new_ucmd!().arg("-R").arg(FILE).run().stdout_move_str(); let expected = at.read(FILE); @@ -105,7 +105,7 @@ fn test_random_shuffle_contains_two_runs_not_the_same() { // check to verify that two random shuffles are not equal; this has the // potential to fail in the unlikely event that random order is the same // as the starting order, or if both random sorts end up having the same order. - const FILE: &'static str = "default_unsorted_ints.expected"; + const FILE: &str = "default_unsorted_ints.expected"; let (at, _ucmd) = at_and_ucmd!(); let result = new_ucmd!().arg("-R").arg(FILE).run().stdout_move_str(); let expected = at.read(FILE); From 8b906b954789acb42b0b1b2a43e8b679c93821b3 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Wed, 21 Apr 2021 18:00:01 +0200 Subject: [PATCH 3/3] remove feature use stabilized in 1.51 --- src/uu/sort/src/sort.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/sort/src/sort.rs b/src/uu/sort/src/sort.rs index 7090a98ed..7d9808c1b 100644 --- a/src/uu/sort/src/sort.rs +++ b/src/uu/sort/src/sort.rs @@ -424,7 +424,7 @@ impl KeyPosition { | SortMode::GeneralNumeric | SortMode::Month => SortMode::Default, // Only SortMode::Default and SortMode::Version work with dictionary_order and ignore_non_printing - m @ (SortMode::Default | SortMode::Version) => m, + m @ SortMode::Default | m @ SortMode::Version => m, } } _ => {}