date: support european dates and out-of-range years - #13818
Conversation
Merging this PR will not alter performance
Comparing Footnotes
|
|
GNU testsuite comparison: |
|
These tests are copied from GNU coreutils and should be licensed as such. |
Ok I gave attribution |
As per CONTRIBUTING.md, we cannot accept any changes based on the GNU source code. I recommend closing this PR and starting again from scratch. |
|
yeah, you need to rewrite the tests |
Ok doing it.... |
|
Binary size comparison: |
c918fea to
e991362
Compare
|
@sylvestre @collinfunk @xtqqczze Sorry for ping, can you guys check now? |
|
@MuntasirSZN Could you please confirm that no code in this PR has been derived from GNU coreutils or any other GPL-licensed source, including small snippets, helper structures, test fixtures, or comments? If you used an AI assistant during development, please also verify its output carefully, as AI-generated code may reproduce material from GPL-licensed sources, potentially without making the provenance apparent. |
No, @xtqqczze. I verified myself. |
7362f2a to
5c25094
Compare
|
@sylvestre @cakebaker review pls? Sorry for ping. |
|
@sylvestre @cakebaker is there anything else that is needed? |
|
@xtqqczze @sylvestre @cakebaker This is another ping... |
There was a problem hiding this comment.
🟡 Changes recommended
There are correctness and UX issues in the new parsing/formatting paths (e.g., overly-permissive European token matching and incomplete extended-year substitution for format specifiers).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 3
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
Extended-year formatting substitution currently misses GNU modifier forms (e.g. %+Y, %10Y) and composite specifiers (e.g. %x), so “any format string” extended-year support is not yet correct/covered.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Lite
There was a problem hiding this comment.
🟢 Approval recommended
The changes are well-covered by new tests and look consistent with the stated behavior, with only a minor comment/documentation nit identified.
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
The European-date rewrite logic appears to mis-handle separate-year token positioning and 2-digit year semantics compared to GNU/POSIX expectations.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
src/uu/date/src/date.rs:1629
- These assertions/documentation currently encode the behavior that 2-digit years are used verbatim (year 99), but GNU/POSIX
datetreats 2-digit years as 1969–2068. Ifrewrite_european_dateis updated to expand 2-digit years, this test should be updated accordingly.
// Inline year variants: one or two digits are passed through untouched,
// longer years are kept whole; a following token (e.g. a timezone) is
// not consumed as part of the date.
assert_eq!(rewrite("11.6.99"), "99-06-11");
assert_eq!(rewrite("11.6.12000"), "12000-06-11");
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Lite
`date` accepts DAY.MONTH[.YEAR] dates (yearless forms use the current year) and years beyond 9999 with any format. uutils rejected both, failing parts of tests/date/date.pl. Rewrite european dates to ISO before parsing; accept extended years for all output formats. Adds unit and integration tests. Signed-off-by: MuntasirSZN <muntasir.joypurhat@gmail.com
dateacceptsDAY.MONTH[.YEAR]dates (yearless forms use the currentyear) and years beyond 9999 with any format. uutils/coreutils rejected both,
failing parts of tests/date/date.pl.
Rewrite european dates to ISO before parsing; accept extended years for
all output formats. Adds unit and integration tests.
tests/date/date.pl gnu test now passes.
Signed-off-by: MuntasirSZN muntasir.joypurhat@gmail.com