Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sylvestre merged 5 commits into from
Jan 21, 2026
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
03d39a1
export the locale grouping separator from i18n decimal
Kaua-Klassmann Jan 18, 2026
c4107c4
fix: numeric sort does not recognize thousand separators
Kaua-Klassmann Jan 18, 2026
1fa6681
fix: update tests to sort
Kaua-Klassmann Jan 19, 2026
49048b4
Merge branch 'uutils:main' into fix_numeric_sort_does_not_recognize_t…
Kaua-Klassmann Jan 21, 2026
e3a4b05
fix: validate if thousands_separator is not multi byte
Kaua-Klassmann Jan 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion src/uu/sort/src/sort.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -1086,11 +1086,22 @@ 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 {
Copy link
Collaborator

@ChrisDryden ChrisDryden Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mind adding a comment here about upstream GNU not being able to handle this, for people reading the code to understand why we're doing this

[b] => Some(*b),
_ => None,
};

// Parse NumInfo for this number.
let (info, num_range) = NumInfo::parse(
range_str,
&NumInfoParseSettings {
accept_si_units: self.settings.mode == SortMode::HumanNumeric,
thousands_separator,
..Default::default()
},
);
Expand Down
36 changes: 34 additions & 2 deletions src/uucore/src/lib/features/i18n/decimal.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}");
}
}
16 changes: 12 additions & 4 deletions tests/by-util/test_sort.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,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(
Expand Down Expand Up @@ -2359,18 +2367,18 @@ _
__
1
_
2,5
_
2.4
___
2,5
_
2.,,3
__
2.4
___
2,,3
_
2.4
___
2,,3
_
1a
_
2b
Expand Down
4 changes: 2 additions & 2 deletions tests/fixtures/sort/mixed_floats_ints_chars_numeric.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
12 changes: 6 additions & 6 deletions tests/fixtures/sort/mixed_floats_ints_chars_numeric.expected.debug
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,18 +67,18 @@ __
46.89
_____
_____
576,446.88800000
___
________________
576,446.890
___
___________
4567.
_____
____________________
>>>>37800
_____
_________
576,446.88800000
___
________________
576,446.890
___
___________
4798908.340000000000
____________________
____________________
Expand Down
4 changes: 2 additions & 2 deletions tests/fixtures/sort/mixed_floats_ints_chars_numeric_stable.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
8 changes: 4 additions & 4 deletions tests/fixtures/sort/mixed_floats_ints_chars_numeric_stable.expected.debug
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,14 @@ _____
__
46.89
_____
576,446.890
___
576,446.88800000
___
4567.
_____
>>>>37800
_____
576,446.88800000
___
576,446.890
___
4798908.340000000000
____________________
4798908.45
Expand Down
3 changes: 2 additions & 1 deletion tests/fixtures/sort/mixed_floats_ints_chars_numeric_unique.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
6 changes: 4 additions & 2 deletions tests/fixtures/sort/mixed_floats_ints_chars_numeric_unique.expected.debug
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,14 @@ _____
__
46.89
_____
576,446.890
___
4567.
_____
>>>>37800
_____
576,446.88800000
___
576,446.890
___
4798908.340000000000
____________________
4798908.45
Expand Down
3 changes: 2 additions & 1 deletion tests/fixtures/sort/mixed_floats_ints_chars_numeric_unique_reverse.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Expand Down
6 changes: 4 additions & 2 deletions tests/fixtures/sort/mixed_floats_ints_chars_numeric_unique_reverse.expected.debug
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,14 @@ _______________
__________
4798908.340000000000
____________________
576,446.890
___
576,446.88800000
___
>>>>37800
_____
4567.
_____
576,446.890
___
46.89
_____
45
Expand Down
4 changes: 2 additions & 2 deletions tests/fixtures/sort/multiple_decimals_numeric.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,15 @@ CARAvan
8.013
45
46.89
576,446.88800000
576,446.890
4567..457
4567.
4567.1
4567.34
37800
45670.89079.098
45670.89079.1
576,446.88800000
576,446.890
4798908.340000000000
4798908.45
4798908.8909800
12 changes: 6 additions & 6 deletions tests/fixtures/sort/multiple_decimals_numeric.expected.debug
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,6 @@ __
46.89
_____
_____
576,446.88800000
___
________________
576,446.890
___
___________
>>>>>>>>>>4567..457
_____
___________________
Expand All @@ -94,6 +88,12 @@ _____________________
>>>>>>45670.89079.1
___________
___________________
576,446.88800000
___
________________
576,446.890
___
___________
4798908.340000000000
____________________
____________________
Expand Down
15 changes: 15 additions & 0 deletions tests/fixtures/sort/multiple_groupings_numeric.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@



CARAvan
1.234
2.000
2.000,50
22
23,.
111
210
1,234
12,34
1,999.99
2,000
45 changes: 45 additions & 0 deletions tests/fixtures/sort/multiple_groupings_numeric.expected.debug
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@

^ no match for key
^ no match for key

^ no match for key
^ no match for key

^ no match for key
^ no match for key
CARAvan
^ no match for key
_______
>1.234
_____
______
2.000
_____
_____
2.000,50
_____
________
22
__
__
23,.
__
____
111
___
___
>210
___
____
1,234
_
_____
12,34
__
_____
>>1,999.99
_
__________
>>>2,000
_
________
15 changes: 15 additions & 0 deletions tests/fixtures/sort/multiple_groupings_numeric.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
1,234
12,34

1.234
2.000
2,000
111


CARAvan
22
23,.
210
1,999.99
2.000,50
Loading