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

gh-66571: Expand matches for %Z in strptime #93486

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

eugenetriguba
Copy link

@eugenetriguba eugenetriguba commented Jun 4, 2022

I had sent a message a while back to the datetime-sig mailing list in regard to this issue and got some more direction from there. That also prompted a brief discussion with @pganssle on the original ticket.

In particular, @pganssle had suggested simply expanding the range of inputs that %Z will accept. Paul had mentioned any 3-4 characters, but I had noticed there was some timezone abbreviations with 2 characters (such as CT) and some with as many as 5 (such as CHADT) so I made that the max here.

The PR tries to take a more conservative approach by simply adding on the 2-5 character match if the machine's locale and UTC/GMT check first fail.

  • If we wanted to, I would assume this 2-5 character match should encompass the set of values that the machine's locale timezones may have and UTC/GMT. So we could possibly just have (?P<Z>[a-z]{2,5}).
  • Furthermore, does 2 to 5 characters make sense? Or, at this point, would it make more sense to simply match on any word given for %Z rather than have this middle ground where there are some values that are matched on that the caller could expect to be valid but some that the caller would need to check because of this more lax 2-5 character match. In reality, I would assume that simply means if the caller wants to be strict here, they need to get the full range of possible abbreviations and validate them all against tzname. And at that point, is there a point in us matching on utc/gmt/machine's locale timezones as well instead of simply accepting whatever word is giving for %Z?

Closes #66571

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.

2 participants