⚠ This page is served via a proxy. Original site: https://github.com
This service does not collect credentials or authentication data.
Skip to content

Conversation

@mattsu2020
Copy link
Contributor

Implement NumericLocaleSettings to handle thousands separators and decimal points based on locale. Update tokenization logic to accommodate blank thousands separators for numeric and human-numeric modes, ensuring proper parsing of numbers with locale-specific formatting. This enhances compatibility with international number representations.

@codspeed-hq
Copy link

codspeed-hq bot commented Dec 25, 2025

CodSpeed Performance Report

Merging this PR will degrade performance by 12.71%

Comparing mattsu2020:sort_sort-h-thousands-sep.sh (9688011) with main (038a08b)

Summary

⚡ 4 improved benchmarks
❌ 2 regressed benchmarks
✅ 276 untouched benchmarks
⏩ 38 skipped benchmarks1

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Memory numfmt_large_numbers_si[10000] 4.8 MB 4.6 MB +4.08%
Simulation sort_long_line[160000] 1.7 ms 1.9 ms -12.71%
Memory sort_case_sensitive[500000] 17.4 MB 16.9 MB +3.02%
Memory sort_long_line[160000] 981.4 KB 719.3 KB +36.44%
Memory sort_reverse_locale[500000] 27.5 MB 30.5 MB -9.82%
Memory dd_copy_default 130.8 KB 126 KB +3.78%

Footnotes

  1. 38 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@github-actions
Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/sort/sort-h-thousands-sep is no longer failing!
Note: The gnu test tests/tail/tail-n0f is now being skipped but was previously passing.

@github-actions
Copy link

GNU testsuite comparison:

GNU test failed: tests/sort/sort-stale-thread-mem. tests/sort/sort-stale-thread-mem is passing on 'main'. Maybe you have to rebase?
Congrats! The gnu test tests/sort/sort-h-thousands-sep is no longer failing!

@github-actions
Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/sort/sort-h-thousands-sep is no longer failing!

@sylvestre
Copy link
Contributor

it needs tests

@mattsu2020 mattsu2020 force-pushed the sort_sort-h-thousands-sep.sh branch from 4e5498a to be1ea65 Compare January 13, 2026 23:30
@github-actions
Copy link

GNU testsuite comparison:

Skipping an intermittent issue tests/tty/tty-eof (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/sort/sort-h-thousands-sep is no longer failing!

@github-actions
Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/sort/sort-h-thousands-sep is no longer failing!

Implement NumericLocaleSettings to handle thousands separators and decimal points based on locale. Update tokenization logic to accommodate blank thousands separators for numeric and human-numeric modes, improving parsing of locale-specific numbers. Also refactor numeric locale detection for safety/readability and clean up related initialization/spell-checker ignore.
@mattsu2020 mattsu2020 force-pushed the sort_sort-h-thousands-sep.sh branch from ff7e69e to 22fa140 Compare January 16, 2026 02:50
@mattsu2020 mattsu2020 requested a review from sylvestre January 16, 2026 03:29
@github-actions
Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/sort/sort-h-thousands-sep is no longer failing!

…in sv_SE locale

Add a new test function `test_human_numeric_blank_thousands_sep_locale` to verify that the sort utility correctly handles human-readable numeric sorting when the locale's thousands separator is a blank space (e.g., in sv_SE.UTF-8 or sv_SE). This ensures proper behavior of the `-h` flag with key-based sorting in such locales, preventing potential sorting errors with space-separated numeric strings.
Use array slice for trim_end_matches and String::len for length check to improve readability and efficiency in test_human_numeric_blank_thousands_sep_locale.
@github-actions
Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/sort/sort-h-thousands-sep is no longer failing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants