fix: numeric sort (-n) does not recognize thousand separators (#10339)

Author: Kaua-Klassmann

src/uu/sort/src/sort.rs

@@ -47,7 +47,6 @@ use uucore::display::Quotable;
 use uucore::error::{FromIo, strip_errno};
 use uucore::error::{UError, UResult, USimpleError, UUsageError};
 use uucore::extendedbigdecimal::ExtendedBigDecimal;
-use uucore::format_usage;
 #[cfg(feature = "i18n-collator")]
 use uucore::i18n::collator::locale_cmp;
 use uucore::i18n::decimal::locale_decimal_separator;
@@ -59,6 +58,7 @@ use uucore::posix::{MODERN, TRADITIONAL};
 use uucore::show_error;
 use uucore::translate;
 use uucore::version_cmp::version_cmp;
+use uucore::{format_usage, i18n};
 
 use crate::buffer_hint::automatic_buffer_size;
 use crate::tmp_dir::TmpDirWrapper;
@@ -1200,11 +1200,21 @@ impl FieldSelector {
         };
         let mut range_str = &line[self.get_range(line, tokens)];
         if self.settings.mode == SortMode::Numeric || self.settings.mode == SortMode::HumanNumeric {
+            // Get the thousands separator from the locale, handling cases where the separator is empty or multi-character
+            let locale_thousands_separator = i18n::decimal::locale_grouping_separator().as_bytes();
+
+            // Upstream GNU coreutils ignore multibyte thousands separators
+            // (FIXME in C source). We keep the same single-byte behavior.
+            let thousands_separator = match locale_thousands_separator {
+                [b] => Some(*b),
+                _ => None,
+            };
+
             // Parse NumInfo for this number.
-            let (info, num_range) = NumInfo::parse(
-                range_str,
-                &numeric_locale.num_info_settings(self.settings.mode == SortMode::HumanNumeric),
-            );
+            let mut parse_settings =
+                numeric_locale.num_info_settings(self.settings.mode == SortMode::HumanNumeric);
+            parse_settings.thousands_separator = thousands_separator;
+            let (info, num_range) = NumInfo::parse(range_str, &parse_settings);
             // Shorten the range to what we need to pass to numeric_str_cmp later.
             range_str = &range_str[num_range];
             Selection::WithNumInfo(range_str, info)

src/uucore/src/lib/features/i18n/decimal.rs

@@ -37,15 +37,47 @@ pub fn locale_decimal_separator() -> &'static str {
     DECIMAL_SEP.get_or_init(|| get_decimal_separator(get_numeric_locale().0.clone()))
 }
 
+/// Return the grouping separator for the given locale
+fn get_grouping_separator(loc: Locale) -> String {
+    let data_locale = DataLocale::from(loc);
+
+    let request = DataRequest {
+        id: DataIdentifierBorrowed::for_locale(&data_locale),
+        metadata: DataRequestMetadata::default(),
+    };
+
+    let response: DataResponse<DecimalSymbolsV1> =
+        icu_decimal::provider::Baked.load(request).unwrap();
+
+    response.payload.get().grouping_separator().to_string()
+}
+
+/// Return the grouping separator from the language we're working with.
+/// Example:
+///  Say we need to format 1,000
+///     en_US: 1,000 -> grouping separator is ','
+///     fr_FR: 1 000 -> grouping separator is '\u{202f}'
+pub fn locale_grouping_separator() -> &'static str {
+    static GROUPING_SEP: OnceLock<String> = OnceLock::new();
+
+    GROUPING_SEP.get_or_init(|| get_grouping_separator(get_numeric_locale().0.clone()))
+}
+
 #[cfg(test)]
 mod tests {
     use icu_locale::locale;
 
-    use super::get_decimal_separator;
+    use super::{get_decimal_separator, get_grouping_separator};
 
     #[test]
-    fn test_simple_separator() {
+    fn test_simple_decimal_separator() {
         assert_eq!(get_decimal_separator(locale!("en")), ".");
         assert_eq!(get_decimal_separator(locale!("fr")), ",");
     }
+
+    #[test]
+    fn test_simple_grouping_separator() {
+        assert_eq!(get_grouping_separator(locale!("en")), ",");
+        assert_eq!(get_grouping_separator(locale!("fr")), "\u{202f}");
+    }
 }

tests/by-util/test_sort.rs

@@ -260,6 +260,14 @@ fn test_multiple_decimals_numeric() {
     );
 }
 
+#[test]
+fn test_multiple_groupings_numeric() {
+    test_helper(
+        "multiple_groupings_numeric",
+        &["-n", "--numeric-sort", "--sort=numeric", "--sort=n"],
+    );
+}
+
 #[test]
 fn test_numeric_with_trailing_invalid_chars() {
     test_helper(
@@ -2424,18 +2432,18 @@ _
 __
 1
 _
-2,5
-_
 2.4
 ___
+2,5
+_
 2.,,3
 __
 2.4
 ___
-2,,3
-_
 2.4
 ___
+2,,3
+_
 1a
 _
 2b

tests/fixtures/sort/mixed_floats_ints_chars_numeric.expected

@@ -21,10 +21,10 @@ CARAvan
 8.013
 45
 46.89
-576,446.88800000
-576,446.890
                4567.
 				37800
+576,446.88800000
+576,446.890
 4798908.340000000000
 4798908.45
 4798908.8909800

tests/fixtures/sort/mixed_floats_ints_chars_numeric.expected.debug

@@ -67,18 +67,18 @@ __
 46.89
 _____
 _____
-576,446.88800000
-___
-________________
-576,446.890
-___
-___________
                4567.
                _____
 ____________________
 >>>>37800
     _____
 _________
+576,446.88800000
+___
+________________
+576,446.890
+___
+___________
 4798908.340000000000
 ____________________
 ____________________

tests/fixtures/sort/mixed_floats_ints_chars_numeric_stable.expected

@@ -24,10 +24,10 @@ CARAvan
 8.013
 45
 46.89
-576,446.890
-576,446.88800000
                4567.
 				37800
+576,446.88800000
+576,446.890
 4798908.340000000000
 4798908.45
 4798908.8909800

tests/fixtures/sort/mixed_floats_ints_chars_numeric_stable.expected.debug

@@ -50,14 +50,14 @@ _____
 __
 46.89
 _____
-576,446.890
-___
-576,446.88800000
-___
                4567.
                _____
 >>>>37800
     _____
+576,446.88800000
+___
+576,446.890
+___
 4798908.340000000000
 ____________________
 4798908.45

tests/fixtures/sort/mixed_floats_ints_chars_numeric_unique.expected

@@ -11,9 +11,10 @@
 8.013
 45
 46.89
-576,446.890
                4567.
 				37800
+576,446.88800000
+576,446.890
 4798908.340000000000
 4798908.45
 4798908.8909800

tests/fixtures/sort/mixed_floats_ints_chars_numeric_unique.expected.debug

@@ -24,12 +24,14 @@ _____
 __
 46.89
 _____
-576,446.890
-___
                4567.
                _____
 >>>>37800
     _____
+576,446.88800000
+___
+576,446.890
+___
 4798908.340000000000
 ____________________
 4798908.45

tests/fixtures/sort/mixed_floats_ints_chars_numeric_unique_reverse.expected

@@ -1,9 +1,10 @@
 4798908.8909800
 4798908.45
 4798908.340000000000
+576,446.890
+576,446.88800000
 				37800
                4567.
-576,446.890
 46.89
 45
 8.013