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

Special-case Z, Z[Etc/UTC] and Z[Etc/GMT] in IXDTF parser #5757

Merged
merged 4 commits into from
Oct 31, 2024

Conversation

robertbastian
Copy link
Member

@robertbastian robertbastian commented Oct 31, 2024

These are the only Zs that don't require any calculation.

@robertbastian robertbastian requested review from sffc, zbraniecki and a team as code owners October 31, 2024 13:52
@robertbastian robertbastian changed the title Deduplicate location names against und Special-case Z, Z[Etc/UTC] and Z[Etc/GMT] in IXDTF parser Oct 31, 2024
sffc
sffc previously approved these changes Oct 31, 2024
components/timezone/src/ixdtf.rs Outdated Show resolved Hide resolved
@@ -262,21 +298,42 @@ impl<'a> Intermediate<'a> {
self.time.minute,
self.time.second,
)?;
Copy link
Member

Choose a reason for hiding this comment

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

Should we now be checking for is_z and failing with MismatchedTimeZoneFields in location_only?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think so.

if self.is_z {
if let Some(offset) = self.offset {
if offset != UtcOffsetRecord::zero() {
return Err(ParseError::RequiresCalculation);
Copy link
Member

Choose a reason for hiding this comment

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

This is rejecting Z[-0800], right? Seems like RequiresCalculation isn't quite the right error case?

Copy link
Member Author

Choose a reason for hiding this comment

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

We need to subtract 8h from the timestamp, which is definitely a calculation. The only non-calculation offset is Z[+0000].

Co-authored-by: Shane F. Carr <shane@unicode.org>
@robertbastian robertbastian requested a review from sffc October 31, 2024 17:13
@robertbastian robertbastian merged commit ed143c9 into unicode-org:main Oct 31, 2024
28 checks passed
@robertbastian robertbastian deleted the utc branch October 31, 2024 21:53
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.

3 participants