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

Parsing explicitly space-padded format specifiers appears broken #1112

Closed
Joey9801 opened this issue May 30, 2023 · 5 comments
Closed

Parsing explicitly space-padded format specifiers appears broken #1112

Joey9801 opened this issue May 30, 2023 · 5 comments

Comments

@Joey9801
Copy link

Joey9801 commented May 30, 2023

Hi,

It looks like a recent change to Chrono has broken parsing of the %e/%_d format specifiers ("Same as %d but space-padded"). Rather than expecting a space padding character, the various strptime-like methods now return a Chrono(ParseError(Invalid)) error.

The following repro works as I would expect when built against chrono 0.4.24, and fails against chrono 0.4.25.

use chrono::{FixedOffset, DateTime, TimeZone};

fn main() {
    let s = "Mon Apr  3 22:32:00 BST 2023";
    let fmt = "%a %b %e %T %Z %Y";
    let offset = FixedOffset::east_opt(3600).unwrap();

    let parsed = offset.datetime_from_str(s, fmt).unwrap();
    let expected = offset.with_ymd_and_hms(2023, 4, 3, 22, 32, 0).unwrap();

    assert_eq!(expected, parsed);
}

The changes in #807 look suspiciously related to this issue, though I am not familiar enough with this codebase to know exactly what the issue is.

Many thanks,
Joe

@djc
Copy link
Member

djc commented May 30, 2023

#807 was not merged into the 0.4.x branch directly, but it's possible some of those changes made it into 0.4.25 via a backport.

@pitdicker
Copy link
Collaborator

pitdicker commented May 30, 2023

Oops, that would be my fault. Should we reverse that?
Or do we want to work an proper support for padding (currently the format specifiers for padding are fully ignored when parsing)?

@djc
Copy link
Member

djc commented May 30, 2023

Can you submit a PR to revert? Can try to get out a 0.4.26 tonight.

@Joey9801
Copy link
Author

I can confirm that the revert has fixed this issue for me in 0.4.26. Thank you so much @djc and @pitdicker for the incredibly speedy response!

Joe

@jtmoon79
Copy link
Contributor

jtmoon79 commented Jun 1, 2023

FYI I'm still planning to look into this in-depth, as this should have been caught by the tests. I just haven't had the opportunity to sit down for an extended period. I hope to do so soon.

jtmoon79 added a commit to jtmoon79/chrono that referenced this issue Jun 3, 2023
Add exhaustive set of tests for `DateTime<FixedOffset>::parse_from_str`
function and all strftime specifiers.

Issue chronotope#1112
jtmoon79 added a commit to jtmoon79/chrono that referenced this issue Jun 3, 2023
Add exhaustive set of tests for `DateTime<FixedOffset>::parse_from_str`
function and all strftime specifiers.

Issue chronotope#1112
jtmoon79 added a commit to jtmoon79/chrono that referenced this issue Feb 11, 2024
Add exhaustive set of tests for `DateTime<FixedOffset>::parse_from_str`
function and all strftime specifiers.

Issue chronotope#1112
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

No branches or pull requests

4 participants