Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ISO 8601 date parser #1296

Closed
wants to merge 1 commit into from

Conversation

pitdicker
Copy link
Collaborator

@djc #1143 seems a bit too large to review. But the first few commits make sense on their own.
This is the commit that adds an ISO 8601 date parser.

The parser would be used by NaiveDate::parse_from_iso8601().
It is a component for the ISO 8601 parsers for NaiveDateTime and DateTime<FixedOffset>.

But ISO 8601 also specifies that the date components of a duration can use the date formats supported by this parser, with the exception of the formats for a week date.

@codecov
Copy link

codecov bot commented Sep 15, 2023

Codecov Report

Merging #1296 (37ba280) into 0.4.x (e730c6a) will increase coverage by 0.05%.
The diff coverage is 97.14%.

@@            Coverage Diff             @@
##            0.4.x    #1296      +/-   ##
==========================================
+ Coverage   91.35%   91.41%   +0.05%     
==========================================
  Files          38       39       +1     
  Lines       17034    17173     +139     
==========================================
+ Hits        15562    15698     +136     
- Misses       1472     1475       +3     
Files Changed Coverage Δ
src/format/mod.rs 85.04% <ø> (ø)
src/format/parse_iso8601.rs 96.39% <96.39%> (ø)
src/format/parse.rs 97.30% <100.00%> (+0.06%) ⬆️
src/naive/date.rs 96.35% <100.00%> (+0.05%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@pitdicker
Copy link
Collaborator Author

pitdicker commented Sep 15, 2023

Maybe we should move all the ISO 8601 parsing code into a module of its own?

Edit: moved to a new module.

@@ -224,6 +224,103 @@ pub(crate) fn parse_rfc3339<'a>(parsed: &mut Parsed, mut s: &'a str) -> ParseRes
Ok((s, ()))
}

#[derive(Copy, Clone, PartialEq, Eq)]
pub(crate) enum Iso8601Format {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This enum serves no purpose yet. We need it to make sure that the date, time and offset components use the same format when parsing to a NaiveDateTime, DateTime or duration.

pub(crate) fn parse_iso8601_date<'a>(
parsed: &mut Parsed,
mut s: &'a str,
) -> ParseResult<(&'a str, Iso8601Format)> {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To reuse this method for parsing a duration it should take a flag to indicate it should not parse week date formats.

@djc
Copy link
Contributor

djc commented Sep 18, 2023

My main motivation for adding an ISO 8601 parser is that chrono is currently unable to parse is own Debug output if the year is outside the 0..=9999 range supported by RFC 3339. Edit: I forgot the FromString implementation of DateTime is very forgiving and accepts this.

A second motivation is that this crate frequently mentions ISO 8601. Then it is strange it does not even have a parser for that format 😆.

As far as motivations go, I find these to be fairly weak, so I don't think I'm going to prioritize reviewing these for the near future.

@pitdicker pitdicker closed this Jan 30, 2024
@pitdicker pitdicker deleted the iso8601_date_parser branch January 30, 2024 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants