Editorial changes in preparation for incorporating IETF draft #2385
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## main #2385 +/- ##
=======================================
Coverage 95.02% 95.02%
=======================================
Files 20 20
Lines 10820 10822 +2
Branches 1929 1926 -3
=======================================
+ Hits 10282 10284 +2
Misses 504 504
Partials 34 34
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Now that annotations are an integral part of IXDTF strings, it's better to handle them at the same time inside of ParseISODateTime. This work is preparatory to the normative change that we will have to do to accept the annotation format defined in IXDTF. (IXDTF is the working abbreviation of the IETF draft; it stands for Internet Extended Date-Time Format) Previously we would call ParseISODateTime on a string and subsequently ParseTemporalTimeZoneString on the same string. In addition to the benefit mentioned above, the previous situation could also lead to confusion since it parsed the string twice, and ParseTemporalTimeZoneString also accepts a bare time zone identifier (even though that branch of the grammar is not used in this case, because a bare time zone identifier can never be a valid ISO string.) Now, the Record returned by ParseISODateTime gains a [[TimeZone]] field which contains another Record of the same type that is returned by ParseTemporalTimeZoneString.
ptomato
force-pushed
the
editorial-ietf-draft
branch
from
04022a3 to
a0e50d7
Compare
Ms2ger
approved these changes
Aug 31, 2022
Use early returns and combine similar conditions to make the control flow through this operation slightly easier to follow.
This was a bit of a mess. Rewrite the operation so that it tries to parse the string as a bare time zone identifier _first_, and if that fails, as an ISO string, throwing if the ISO string doesn't contain any time zone (whether Z designator, UTC offset, or IANA name). This was what the operation previously did, just much more difficult to follow. The reference polyfill mostly already worked like this, but move the IANA name canonicalization step out of ParseTemporalTimeZoneString and into its only caller, to correspond better with the spec text. This also requires bringing the time zone ID regex better in line with the grammar's |TimeZoneIdentifier| production by adding the IANA legacy names.
This helps when debugging which property is not being parsed correctly.
ptomato
force-pushed
the
editorial-ietf-draft
branch
from
a0e50d7 to
45975fd
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Now that the IETF draft for the date-time format with annotations (codenamed "IXDTF") is ready, I'm preparing a normative PR to incorporate its conclusions into the proposal (#1450). I'm still chasing down a few loose ends on that, but here is a PR with editorial changes that prepare the way for it.
Best reviewed commit by commit.