Skip to content

date: take %p and %P from the locale - #15130

Merged
sylvestre merged 3 commits into
uutils:mainfrom
abendrothj:date-locale-ampm
Oct 9, 2026
Merged

sylvestre merged 3 commits into
uutils:mainfrom
abendrothj:date-locale-ampm

Conversation

@abendrothj

Copy link
Copy Markdown
Contributor

%p and %P printed jiff's AM/PM in every locale. They now use the locale's AM_STR/PM_STR, read once with nl_langinfo. Locales without markers (fr_FR, de_DE) get empty ones as in GNU 9.12, and the %p inside %r goes the same way, so fr_FR %r is 08:17:48 like GNU. Width and case flags keep their rules (%10p, %^p, %#p). It's one more arm in #11068's format scanner.

Based on #11068; until that merges, the diff here shows its commit too.

Closes #15117

Expand them to the locale's D_FMT, T_FMT and T_FMT_AMPM, read once,
instead of jiff's C formats. A width or flag applies to the whole
expansion, as in GNU date, and %x is expanded before an out-of-range
year is substituted into the default format.

Fixes uutils#10284
Use the locale's AM_STR and PM_STR, read once, instead of jiff's AM and
PM, for %p and %P and so for the %p inside %r. A locale without markers,
like fr_FR, gets empty ones, as in GNU date.
Copilot AI balanced review requested due to automatic review settings October 6, 2026 07:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Comment thread src/uu/date/src/date.rs Outdated
};
output.push_str(&format[copied..i]);
// The marker is put into a format, so a `%` in it has to be doubled.
for c in marker.chars() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

could be output.push_str(&marker.replace('%', "%%")) like below, no?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, changed.

Comment thread src/uu/date/src/locale.rs
static LOCALE_AMPM_MARKERS: LazyLock<[&'static str; 2]> = LazyLock::new(|| {
let mut markers = POSIX_AMPM_MARKERS;
for (marker, item) in markers.iter_mut().zip([libc::AM_STR, libc::PM_STR]) {
if let Some(value) = langinfo(item).and_then(|value| String::from_utf8(value).ok()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

with a non UTF-8 locale (zh_CN.GB18030) this silently falls back to AM/PM, is it ok?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Same as main, which prints PM there too. date doesn't handle non-UTF-8 locales in general: %B and %A come out in UTF-8 under zh_CN.GB18030, where GNU uses GB18030. I can open a separate issue for that.

Copilot AI balanced review requested due to automatic review settings October 6, 2026 09:02

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@codspeed

codspeed Bot commented Oct 6, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 3.28%

⚡ 4 improved benchmarks
❌ 6 regressed benchmarks
✅ 387 untouched benchmarks
🆕 2 new benchmarks
⏩ 54 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
❌ Simulation three_39_bit_primes 228.6 ms 466.6 ms -51.01%
❌ Simulation file_iso_dates 30.4 ms 32.5 ms -6.38%
❌ Simulation file_tz_abbreviations 34.2 ms 36.3 ms -5.72%
❌ Simulation file_custom_format 29.2 ms 30.6 ms -4.46%
❌ Simulation five_38_bit_primes 1.8 s 1.8 s -3.71%
❌ Simulation cksum_crc 32.9 ms 34 ms -3.11%
⚡ Simulation split_lines 11.1 ms 8.5 ms +30.93%
⚡ Simulation split_numeric_suffix 11.3 ms 8.7 ms +30.2%
⚡ Simulation du_wide_tree[(5000, 500)] 15.9 ms 15.2 ms +4.56%
⚡ Simulation tr_single_char_replace 14.5 ms 13.9 ms +4.33%
🆕 Memory tr_delete_single_char N/A 35.4 KB N/A
🆕 Simulation tr_delete_single_char N/A 103.9 ms N/A

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing abendrothj:date-locale-ampm (953c1da) with main (968853a)2

Open in CodSpeed

Footnotes

  1. 54 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. ↩

  2. No successful run was found on main (f29e308) during the generation of this report, so 968853a was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

@sylvestre
sylvestre merged commit 9e77354 into uutils:main Oct 9, 2026
190 of 191 checks passed
@xtqqczze

Copy link
Copy Markdown
Collaborator

the new test test_date_format_locale_date_and_time is failing:

---- test_date::test_date_format_locale_date_and_time stdout ----
bin: "/home/runner/work/coreutils/coreutils/target/wasm32-wasip1/debug/coreutils.wasm"
run: /home/runner/work/coreutils/coreutils/target/wasm32-wasip1/debug/coreutils.wasm date -d 1997-01-19 20:17:48 +%x

thread 'test_date::test_date_format_locale_date_and_time' (21995) panicked at tests/by-util/test_date.rs:2211:14:
assertion failed: `(left == right)`

Diff < left / right > :
<01/19/97
>01/19/1997
 


note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

@abendrothj

Copy link
Copy Markdown
Contributor Author

Thanks, my test. Fix in #15236.

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.

date: %p and %P ignore the locale's AM/PM markers

4 participants