Skip to content

fix: parse offset seconds in FromStr so Display round-trips#1801

Open
nvxbug wants to merge 1 commit into
chronotope:mainfrom
nvxbug:offset-seconds-roundtrip
Open

fix: parse offset seconds in FromStr so Display round-trips#1801
nvxbug wants to merge 1 commit into
chronotope:mainfrom
nvxbug:offset-seconds-roundtrip

Conversation

@nvxbug

@nvxbug nvxbug commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

FixedOffset's Display and Debug print a sub-minute offset as +HH:MM:SS, for example +00:00:30, but scan::timezone_offset only reads +HH:MM. That scanner backs FromStr for both FixedOffset and DateTime (through parse_rfc3339_relaxed), so a value chrono prints cannot be parsed back: DateTime::from_str rejects its own to_string() with TooLong on the trailing :SS, and FixedOffset::from_str silently drops the seconds and returns a truncated offset.

The fix teaches timezone_offset to consume an optional seconds component behind a new allow_seconds flag, applying the sign inside the scanner so negative offsets like -00:00:30 stay correct, and turns it on only for the two FromStr paths. Keeping it in the scanner fixes both round-trips in one place while strict parse_from_rfc3339, parse_from_rfc2822, and the %z family keep rejecting seconds. Regression tests cover the FixedOffset and DateTime paths across positive and negative sub-minute offsets.

@codecov

codecov Bot commented Jun 22, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.10345% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.01%. Comparing base (3ffcd1b) to head (aef386c).

Files with missing lines Patch % Lines
src/format/scan.rs 86.66% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1801   +/-   ##
=======================================
  Coverage   90.01%   90.01%           
=======================================
  Files          35       35           
  Lines       13831    13855   +24     
=======================================
+ Hits        12450    12472   +22     
- Misses       1381     1383    +2     

☔ 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.

@djc

djc commented Jun 22, 2026

Copy link
Copy Markdown
Member

I'm going to stop responding to these PRs for edge case bugs. There are probably a lot more; you should direct your efforts towards other implementations.

@nvxbug

nvxbug commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

Makes sense, I'll redirect to other things. Happy to close this if you'd rather not keep it on the pile, otherwise it's here if the round-trip ever matters.

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