Skip to content

ls: honor LC_NUMERIC for the -h decimal separator - #14264

Open
Jorge-Polanco-Roque wants to merge 1 commit into
uutils:mainfrom
Jorge-Polanco-Roque:fix/ls-h-locale-decimal-separator
Open

ls: honor LC_NUMERIC for the -h decimal separator#14264
Jorge-Polanco-Roque wants to merge 1 commit into
uutils:mainfrom
Jorge-Polanco-Roque:fix/ls-h-locale-decimal-separator

Conversation

@Jorge-Polanco-Roque

Copy link
Copy Markdown

Fixes the ls part of #14232.

ls -lh (and -h) rendered the fractional size with a period regardless of LC_NUMERIC, so under a comma-decimal locale it disagreed with sort -h — breaking the common ls -h / du -h | sort -h use case.

$ LC_ALL=fr_FR.UTF-8 ls -lh 8500-byte-file   # before: 8.4K   after: 8,4K   (GNU: 8,4K)

Root cause: ls formats sizes through uucore's human_readable, which already localizes the separator via localize_decimal, but the ls crate did not enable the i18n-decimal uucore feature, so the localization compiled out. Enabling it mirrors what du already does (#12357); du and numfmt were already handled, so this closes the remaining ls gap.

Testing: added test_ls_h_locale_decimal_separator mirroring test_du_h_locale_decimal_separator. Verified on the standalone ls binary too (with the feature 8,4K, without it 8.4K). The full ls suite, cargo fmt, and clippy -D warnings pass.

@github-actions

Copy link
Copy Markdown

Binary size comparison:

Individual binary size comparison VS main (threshold: >=5% AND >=4 KB).

Total size of compared binaries: 152.18 MB (+1004 KB, +0.65%)

Significant per-binary changes:
  comm     1.12 MB ->    2.33 MB  (+1.21 MB, +107.29%)

@github-actions

github-actions Bot commented Aug 30, 2026

Copy link
Copy Markdown

GNU testsuite comparison:

Skip an intermittent issue tests/pr/bounded-memory (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/tail/inotify-dir-recreate (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/misc/io-errors (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/tail/tail-n0f (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/cut/cut-huge-range is now passing!
Skip an intermittent issue tests/cut/bounded-memory (was skipped on 'main', now failing)

@Jorge-Polanco-Roque

Copy link
Copy Markdown
Author

The failing checks look unrelated to this change. The macOS feat_os_unix job fails only on test_tail::test_follow_name_truncate3 (5312 passed, 1 failed) — a known flaky, timing-sensitive tail test — and the wasm32-wasip1 job hit a 403 Forbidden fetching packages.microsoft.com (CI infra). This PR only touches ls (adds the i18n-decimal uucore feature). Happy to rebase or re-run if that helps.

@Jorge-Polanco-Roque

Copy link
Copy Markdown
Author

Heads-up: both red builds look unrelated to this change.

  • wasm32-wasip1: the job failed during environment setup (apt-get hit a 403 from packages.microsoft.com), before cargo compiled anything — no Compiling line and no error[E...] in the log.
  • macos x86_64: the only failure is test_tail::test_follow_name_truncate3, a timing-sensitive tail --follow --name test unrelated to ls. This PR's own test, test_ls_h_locale_decimal_separator, passed on that same macOS run.

Could someone re-run the two failed jobs? I ran test_follow_name_truncate3 locally 3×, passing each time, so it looks like a pre-existing flake.

ls -lh (and -h) rendered the fractional size with a period regardless of
LC_NUMERIC, so under e.g. fr_FR.UTF-8 it printed "8.4K" where GNU prints
"8,4K". ls already formats sizes through uucore's human_readable, which
localizes the separator, but the ls crate did not enable the
`i18n-decimal` uucore feature, so the localization compiled out.

Enable it, matching what du does (see uutils#12357). This makes ls -h agree
with sort -h under a comma-decimal locale.

Fixes uutils#14232
@Jorge-Polanco-Roque
Jorge-Polanco-Roque force-pushed the fix/ls-h-locale-decimal-separator branch from a2e014c to 1e539da Compare September 3, 2026 13:21
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.

1 participant