Skip to content

fix(offset): treat a numeric offset after a time of day as a zone correction - #326

Open
Socialpranker wants to merge 1 commit into
uutils:mainfrom
Socialpranker:fix-numeric-offset-before-relative
Open

Socialpranker wants to merge 1 commit into
uutils:mainfrom
Socialpranker:fix-numeric-offset-before-relative

Conversation

@Socialpranker

@Socialpranker Socialpranker commented Sep 5, 2026 •

Copy link
Copy Markdown
Contributor

2026-08-27 12:00 +3 hours is parsed as a relative shift of three hours. GNU date reads +3 as the zone correction +03:00 and the bare hours as one hour:

$ date -u -d '2026-08-27 12:00 +3 hours'   # GNU 9.11
2026-08-27 10:00:00 UTC

12:00 +3 alone 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 seconds is a relative item in GNU), 12 +3 hours and 12:00 utc +3 hours are unchanged, and 12:00 +30 hours is 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.rs and src/items/mod.rs. cargo test, cargo fmt --check and cargo clippy --all-targets pass.

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.

@codspeed

codspeed Bot commented Sep 5, 2026 •

Copy link
Copy Markdown
Contributor

Merging this PR will improve performance by 18.76%

1 warning

⚠️ 13 benchmarks are dominated by system calls

System calls cannot be simulated consistently and are left out of the measure. The reported value understates the real cost. Switch to the Walltime instrument to measure them.

Learn more about system calls

See affected benchmarks

⚡ 3 improved benchmarks
✅ 18 untouched benchmarks

Performance Changes

Benchmark BASE HEAD Efficiency
⚡ parse_iso_datetime_t_separator 50.8 µs 40.6 µs +25.06%
⚡ parse_iso_datetime 50.2 µs 40.5 µs +23.96%
⚡ parse_datetime_with_delta 76.6 µs 70.9 µs +8.04%

Tip

Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.


Comparing Socialpranker:fix-numeric-offset-before-relative (62a9b02) with main (5f7897d)

Open in CodSpeed

@codecov

codecov Bot commented Sep 5, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.78%. Comparing base (d5c7380) to head (62a9b02).
⚠️ Report is 1 commits behind head on main.

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              
Flag Coverage Δ
macos_latest 97.78% <100.00%> (+0.04%) ⬆️
ubuntu_latest 97.78% <100.00%> (+0.04%) ⬆️
windows_latest 12.49% <29.78%> (+0.16%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread src/items/mod.rs

// `+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());

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.

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

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.

It's an error, like GNU's invalid date. Covered by a new case in test_time_invalid.

Comment thread src/items/offset.rs Outdated
//
// 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`

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.

4 lines of comment for this guard, could you please make it one or two?

Comment thread src/items/offset.rs Outdated
one_of(AsChar::is_dec_digit),
))
.parse_next(input);
if fraction.is_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.

fraction for a Result reads odd, has_fraction and inline the peek, no?

@Socialpranker

Copy link
Copy Markdown
Contributor Author

switched the peek to has_fraction and cut the comment down

Comment thread src/items/offset.rs Outdated
@@ -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> {

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.

this changes a lot of parses, did you run the gnu date tests from coreutils with it?

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.

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.

Comment thread src/items/mod.rs
/// `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() {

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.

please add a few of these cases in tests/time.rs too, that's where we keep the gnu-compared expectations

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.

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.
@Socialpranker
Socialpranker force-pushed the fix-numeric-offset-before-relative branch from 5bd8ef5 to 62a9b02 Compare October 22, 2026 05:31
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.

2 participants