date: skip '-' flag for composite strftime specifiers - #14163
MadeNavaneeth wants to merge 2 commits into
Conversation
| // GNU treats composite specifiers (D, F, T, r, R, c, x, X) | ||
| // as atomic: the `-` flag applies to the whole expansion, | ||
| // not to inner sub-fields. Since jiff already expands them | ||
| // correctly, strip the `-` flag so apply_modifiers doesn't | ||
| // remove leading zeros from inner fields like %m in %D. | ||
| if !parsed.flags.is_empty() || parsed.width.is_some() { | ||
| // GNU treats composite specifiers (D, F, T, r, R, c, x, X) | ||
| // as atomic: flags like `-` apply to the whole expansion, | ||
| // not to inner sub-fields. Since jiff already expands them | ||
| // correctly, strip the `-` flag so apply_modifiers doesn't | ||
| // remove leading zeros from inner fields like %m in %D. |
There was a problem hiding this comment.
i don't think it needs 10 lines of comment
There was a problem hiding this comment.
please make this comment shorter
|
needs a test too |
|
GNU testsuite comparison: |
c055443 to
b4d5b00
Compare
| // not to inner sub-fields. Since jiff already expands them | ||
| // correctly, strip the `-` flag so apply_modifiers doesn't | ||
| // remove leading zeros from inner fields like %m in %D. | ||
| if !parsed.flags.is_empty() || parsed.width.is_some() { |
There was a problem hiding this comment.
This condition is identical to the condition on line 238.
GNU date treats composite strftime specifiers (%D, %F, %T, %r, %R, %c, %x, %X) as atomic: flags like '-' apply to the whole expansion, not to inner sub-fields. uutils let the '-' flag propagate into inner specifiers, producing e.g. '6/15/24' for %-D instead of '06/15/24'. Strip the '-' flag from composite specifiers before applying modifiers, so inner fields keep their default padding while width and other flags still work. Fixes uutils#11657
b4d5b00 to
3e494fa
Compare
|
Addressed all feedback: removed the duplicated comment and condition, shortened the is_composite_specifier doc comment to one line. The redundant check on line 238 is gone. |
|
Test already added -- |
* tests/date/date.pl: Add tests ensuring that %-D, %-F, %-T, %-R, %-r, %-x and %-X keep the default padding of their sub-fields, while the other flags and the field width still apply to the whole expansion. uutils/coreutils#14163 Link: #339
|
sorry but it needs to be rebased |
| spec: parsed.spec, | ||
| len: parsed.len, | ||
| } | ||
| } else { |
There was a problem hiding this comment.
this else branch is almost just a copy of parsed, do we need it?
|
|
||
| /// Returns true if the specifier is composite (multi-field, e.g. %D = %m/%d/%y). | ||
| fn is_composite_specifier(spec: &str) -> bool { | ||
| // strip leading colons (e.g. ":z" → "z") |
There was a problem hiding this comment.
why strip colons? none of these letters take one
Summary
Fixes #11657
The problem
GNU
datetreats composite strftime specifiers (%D,%F,%T,%r,%R,%c,%x,%X) as atomic: flags like-apply to the whole expansion, not to inner sub-fields. uutils let the-flag propagate into inner specifiers:The fix
Strip the
-flag from composite specifiers before applying modifiers, so inner fields keep their default padding while width and other flags still work.Verification
%-Dnow correctly outputs06/15/24(matching GNU)