From 4a305b32c6e77af76e1af3a42b72eeeac4855d23 Mon Sep 17 00:00:00 2001
From: Michael Debertol <michael.debertol@gmail.com>
Date: Wed, 21 Apr 2021 17:49:40 +0200
Subject: [PATCH] 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();
+        }
+    }
+}