-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
date(locale): fix hardcoded formats with nl_langinfo(D_T_FMT) #9756
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.
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
Conversation
bf9778c to
4c0695d
Compare
d8a7568 to
5155ed1
Compare
|
|
||
| #[test] | ||
| #[cfg(unix)] | ||
| fn test_date_locale_format_structure() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we have another tests with french as locale? thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added test_date_locale_fr_french
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice
but i was also thinking about the day & month in french
5155ed1 to
119ede1
Compare
Replace hardcoded format string selection with direct nl_langinfo(D_T_FMT) usage. This ensures locale-specific formatting details (leading zeros, component ordering, hour formats) are properly respected from system locale data instead of detecting 12/24-hour preference and returning hardcoded alternatives. Changes to locale.rs: - Remove detect_12_hour_format() and uses_12_hour_format() - Simplify get_locale_default_format() to use D_T_FMT directly - Add timezone injection if %Z missing from locale format - Add use nix::libc import Add test coverage (tests/by-util/test_date.rs): - 4 new unit tests for locale format structure validation - 7 new integration tests verifying locale-specific behavior - Tests prevent regression to hardcoded format strings Addresses feedback from PR uutils#9654 comment #3676971020
119ede1 to
a85255c
Compare
|
The output will still be slightly different. We want |
|
hmm macOS does support |
|
GNU testsuite comparison: |
|
https://man7.org/linux/man-pages/man3/nl_langinfo.3.html https://github.com/lattera/glibc/blob/master/locale/langinfo.h#L232 |
|
Even though the test is passing, this is still incorrect: |
|
bizarre, on my system: date (GNU coreutils) 9.7 rust: |
Fixes #9654
where we used hardcoded format strings. Now we grab the actual locale format from
nl_langinfo(D_T_FMT).Verified
# Test case from comment #3676971020 $ LC_ALL=en_US.UTF-8 date -u -d 2025-12-14T1:00Main branch (PR #9654):
GNU coreutils:
$ LC_ALL=en_US.UTF-8 gdate -u -d 2025-12-14T1:00 Sun Dec 14 01:00:00 UTC 2025 # Has leading zeroOur fix (test/date-locale-tests):
Test 2: nl_NL.UTF-8
Addresses #9654 comment