Repository navigation
fix(offset): accept DST after a standard time zone name - #343
jayhemnani9910 wants to merge 3 commits into
Conversation
GNU date reads a standard time zone followed by the word DST as its daylight saving time zone, one hour ahead: "EST DST" is EDT. The module doc quotes that rule, but the parser rejected "2026-01-10 10:00 EST DST" as invalid. Add the hour when DST follows a zone that takes it; after a daylight saving time zone such as EDT, and after T, GNU rejects it, and so does this.
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #343 +/- ##
==========================================
+ Coverage 97.73% 98.89% +1.16%
==========================================
Files 21 21
Lines 4421 4540 +119
Branches 139 142 +3
==========================================
+ Hits 4321 4490 +169
+ Misses 99 49 -50
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:
|
| let start = input.checkpoint(); | ||
| let word: ModalResult<&str> = | ||
| s(take_while(1..=MAX_TZ_SIZE, AsChar::is_alpha)).parse_next(input); | ||
| if word.is_ok_and(|word| word.eq_ignore_ascii_case("dst")) { |
There was a problem hiding this comment.
same here, word == Ok("dst") should be enough
|
|
||
| /// Whether a named timezone can be followed by DST. GNU date rejects it after | ||
| /// a daylight saving time zone, and after `t`. | ||
| fn takes_dst(name: &str) -> bool { |
There was a problem hiding this comment.
this list duplicates names from timezone_name_to_offset
could the table carry a dst flag instead, no?
| /// a daylight saving time zone, and after `t`. | ||
| fn takes_dst(name: &str) -> bool { | ||
| !matches!( | ||
| name.to_ascii_lowercase().as_str(), |
There was a problem hiding this comment.
the input is already lowercased in mod.rs, do we need to_ascii_lowercase here?
| } | ||
|
|
||
| #[test] | ||
| fn timezone_name_with_dst() { |
There was a problem hiding this comment.
please add a test with a full date in tests/time.rs too, e.g. 2026-01-10 10:00 EST DST
Mark the zones that can be followed by DST in timezone_name_to_offset instead of keeping a second list of names, and compare the next word directly since the input is already lowercase. Test full dates with EST DST, CET DST and IST DST, and EDT DST as invalid.
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.
Parse each name in the time zone table, alone and followed by DST, and compare with GNU date 9.7, which rejects DST after the daylight saving time zones and T.
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.
|
Thanks. Pushed a5cc389:
Also pushed 888c6a6, a test that runs every name in the table, alone and with DST, against GNU date 9.7. It covers the table rows that codecov flagged. |
The doc comment at the top of
offset.rsquotes GNU's rule: "By following a non-daylight-saving time zone by the string 'DST' in a separate word (that is, separated by some white space), the corresponding daylight saving time zone may be specified." The parser doesn't do it:timezone_name_offsetnow checks for aDSTword after the zone name and adds one hour. GNU rejectsDSTafter a zone that is already daylight saving time (EDT DST,CEST DST, ...) and afterT. I got that list by trying every name intimezone_name_to_offsetwithDSTagainst GNU date, andtakes_dsthas the same list. In those cases the word is left unparsed, so the input is invalid as before.Testing:
timezone_name_with_dstcoversest dst, two spaces,utc,ist(minutes), a military letter, and the rejected forms (edt dst,t dst,estdst,est dstx). It fails without the change.cargo test: all pass, one more than main.cargo clippy --all-targetsandcargo fmt --checkclean.date9.7: every zone name in the table withDST, in four positions (after the time, before the date, between date and time, after aTtime). That's 288 inputs, all the same. AlsoDSTalone,UTC+1 DST,EST DST +1,EST DST DST,EST DST 1 hour agoandEST DST tomorrow: same.date(coreutils 865e575) against this branch:date -u -d '2026-01-10 10:00 EST DST'gives 14:00 UTC like GNU, andEDT DSTis still invalid.