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

Skip IANA TZ names when parsing %Z specifier #473

Merged
merged 3 commits into from
Aug 21, 2020

Conversation

abashurov
Copy link
Contributor

@abashurov abashurov commented Aug 16, 2020

Thanks for contributing to chrono!

  • Have you added yourself and the change to the changelog? (Don't worry
    about adding the PR number)
  • If this pull request fixes a bug, does it add a test that verifies that
    we can't reintroduce it?

This PR prevents %Z specifier from causing an instant parsing failure, instead, skipping chars until the next whiltespace-like char, as implemented in glibc strptime.

Copy link
Contributor

@quodlibetor quodlibetor left a comment

Choose a reason for hiding this comment

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

Thanks for this! Could you also modify the %Z row in src/format/strftime.rs to state that in parsing mode this discards all non-whitespace characters?

Aside: it is kind of wild that that is the behavior of glibc, I had to confirm it myself.

@abashurov
Copy link
Contributor Author

Thanks for this! Could you also modify the %Z row in src/format/strftime.rs to state that in parsing mode this discards all non-whitespace characters?

Done and done

Aside: it is kind of wild that that is the behavior of glibc, I had to confirm it myself.

Well, it's a lot of work with so little payoff. Also, I probably should have provided a link to the glibc source code to simplify the verification, sorry for that

@quodlibetor quodlibetor merged commit f4c42b4 into chronotope:main Aug 21, 2020
@quodlibetor
Copy link
Contributor

Also, I probably should have provided a link to the glibc source code to simplify the verification, sorry for that.

No apologies necessary, it was straightforward to verify. Thanks, very much for this!

erickt added a commit to erickt/valico that referenced this pull request Jan 19, 2022
In chronotope/chrono#473, chrono changed how %Z is
handled, and was changed to simply skip over any non-whitespace characters
in the timezone section, just like glibc's `strptime`.

I'm not sure if this is the right fix, but it gets the tests to pass.
erickt added a commit to erickt/valico that referenced this pull request Jan 19, 2022
In chronotope/chrono#473, chrono changed how %Z is
handled, and was changed to simply skip over any non-whitespace characters
in the timezone section, just like glibc's `strptime`.

I'm not sure if this is the right fix, but it gets the tests to pass.
pickfire pushed a commit to pickfire/chrono that referenced this pull request Jan 22, 2022
botahamec pushed a commit to botahamec/chrono that referenced this pull request May 26, 2022
pickfire pushed a commit to pickfire/chrono that referenced this pull request Jul 5, 2022
fourkbomb pushed a commit to fourkbomb/valico that referenced this pull request Nov 11, 2022
In chronotope/chrono#473, chrono changed how %Z is
handled, and was changed to simply skip over any non-whitespace characters
in the timezone section, just like glibc's `strptime`.

I'm not sure if this is the right fix, but it gets the tests to pass.
s-panferov pushed a commit to s-panferov/valico that referenced this pull request Feb 2, 2023
In chronotope/chrono#473, chrono changed how %Z is
handled, and was changed to simply skip over any non-whitespace characters
in the timezone section, just like glibc's `strptime`.

I'm not sure if this is the right fix, but it gets the tests to pass.
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.

None yet

2 participants