Merge pull request #2097 from miDeb/sort-disable-dictionary-mode

sort: disallow certain flags with -d and -i

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,
@@ -405,6 +405,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 | m @ 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];
@@ -649,7 +671,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)
@@ -677,9 +700,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)

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);
@@ -582,6 +582,33 @@ fn test_check_silent() {
         .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();
+        }
+    }
+}
+
 #[test]
 fn test_trailing_separator() {
     new_ucmd!()