Repository navigation
fix(offset): treat a numeric offset after a time of day as a zone correction - #326
Socialpranker wants to merge 1 commit into
Conversation
Merging this PR will improve performance by 18.76%1 warning
Performance Changes
Tip Curious why performance improved? Comment Comparing |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #326 +/- ##
==========================================
+ Coverage 97.73% 97.78% +0.04%
==========================================
Files 21 21
Lines 4421 4467 +46
Branches 139 140 +1
==========================================
+ Hits 4321 4368 +47
+ Misses 99 98 -1
Partials 1 1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
|
||
| // `+30` is not a valid zone correction, and GNU rejects the whole | ||
| // input rather than falling back to a relative item. | ||
| assert!(parse(&mut "2026-08-27 12:00 +30 hours").is_err()); |
There was a problem hiding this comment.
out of range is a hard error now, so please check what date -d '12:00 +30 hours' prints, it should still look like gnu's invalid date
There was a problem hiding this comment.
It's an error, like GNU's invalid date. Covered by a new case in test_time_invalid.
| // | ||
| // Return early if the input can be parsed as a relative time. | ||
| if peek(relative::parse).parse_next(input).is_ok() { | ||
| // A number with a fractional part is never a zone correction: GNU `date` |
There was a problem hiding this comment.
4 lines of comment for this guard, could you please make it one or two?
| one_of(AsChar::is_dec_digit), | ||
| )) | ||
| .parse_next(input); | ||
| if fraction.is_ok() { |
There was a problem hiding this comment.
fraction for a Result reads odd, has_fraction and inline the peek, no?
|
switched the peek to has_fraction and cut the comment down |
| @@ -193,12 +193,17 @@ pub(super) fn parse_local(input: &mut &str) -> ModalResult<()> { | |||
|
|
|||
| /// Parse a timezone starting with `+` or `-`. | |||
| pub(super) fn timezone_offset(input: &mut &str) -> ModalResult<Offset> { | |||
There was a problem hiding this comment.
this changes a lot of parses, did you run the gnu date tests from coreutils with it?
There was a problem hiding this comment.
Not the GNU test suite itself. I ran 2457 inputs (8 time-of-day forms or none × 13 numeric offsets × 21 relative units, all on 2026-08-27) through GNU date 9.11 and through main and this branch. main matches GNU on 2136, the branch on 2388, with 0 regressions. My first version did regress 117 of them, all the bare-hour form (12 +3 hours, where GNU keeps +3 relative), so the correction now only applies after a time with a colon. The 69 remaining differences are the same on main.
| /// `2026-08-27 12:00 +3 hours` as the zone `+03:00` plus a bare `hours` | ||
| /// (which means one hour), i.e. 10:00 UTC. | ||
| #[test] | ||
| fn numeric_offset_before_relative_item() { |
There was a problem hiding this comment.
please add a few of these cases in tests/time.rs too, that's where we keep the gnu-compared expectations
There was a problem hiding this comment.
Added 12 cases to tests/time.rs, expected values taken from GNU 9.11, plus +30 hours in test_time_invalid. Also rebased onto main and squashed to one commit (62a9b02): time_zone_correction is now separate from timezone_offset, which keeps its relative lookahead for UTC +8 years.
GNU reads '12:00 +3 hours' as the zone +03:00 plus a bare 'hours', not as a relative item. The correction after a colon time no longer yields to a following relative unit; a fractional number still does, and the bare-hour and named-zone forms are unchanged.
5bd8ef5 to
62a9b02
Compare
2026-08-27 12:00 +3 hoursis parsed as a relative shift of three hours. GNU date reads+3as the zone correction+03:00and the barehoursas one hour:12:00 +3alone already parsed as a zone, so the meaning of the number depended on whether a unit word followed. After a time with a colon, the correction no longer yields to a following relative unit. A fractional number still does (12:00 +1.5 secondsis a relative item in GNU),12 +3 hoursand12:00 utc +3 hoursare unchanged, and12:00 +30 hoursis an error, as in GNU.Ran 2457 inputs against GNU 9.11: main matches on 2136, this branch on 2388, no regressions. New cases are in
tests/time.rsandsrc/items/mod.rs.cargo test,cargo fmt --checkandcargo clippy --all-targetspass.Prepared with AI assistance (Claude Sonnet 5.5, via Claude Code), per the uutils AI policy. GNU behavior was taken from running the installed binary, not from reading its source.