-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Editorial: simplify and clarify time zone identifier spec text #3035
Conversation
0996db1
to
92305c1
Compare
2fdd226
to
ff4246a
Compare
Sorry for the flurry of commit spam to fix formatting and typecheck complaints. Also rebased to main while I was in the neighborhood. Should be ready for review now. |
18394ab
to
16ebedc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @bakkot for the review! I think I've resolved the concerns you raised, but let me know if you'd like to see more changes. Thanks again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm pretty sure HostTimeZoneIdentifier should be added to the list in D.1, and probably also AvailableTimeZoneIdentifiers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gibson042 Thanks for the review! All your suggestions are included in the latest commit.
I'm pretty sure HostTimeZoneIdentifier should be added to the list in D.1, and probably also AvailableTimeZoneIdentifiers.
I added HostTimeZoneIdentifier to D.1 (and changed its type to host-defined AO).
AvailableTimeZoneIdentifiers, as implemented today in all major implementations, is not host-dependent. The list of identifiers and their rules are baked into each implementation and are the same for every host running the same version of the implementation. So I left AvailableTimeZoneIdentifiers as implementation-defined, not host-defined.
Holler if that's a problem.
Otherwise I think I've resolved all open issues reported by all reviewers so far.
d83bab4
to
7f970e7
Compare
Hi All - I believe that all review comments have been addressed, and I just rebased to main. Let me know if there's any more changes needed. Thanks! |
In ECMA-262, "host" refers not to an operating system but rather to consumers of the spec that define a specific subset of implementation-defined operations (e.g., HTML is such a host). So I suppose my original suggestion was in error and Annex D should not be changed, since we don't expect HTML or other specifications to define HostTimeZoneIdentifier or AvailableTimeZoneIdentifiers. I apologize for the churn. |
We discussed this in the editor call, and our thoughts are:
For now we're not requesting any additional work to be done on this PR. Please stay tuned while we chat and review this more. |
Yep, agreed. I changed it to a host hook in response to @gibson042's review, but per his later comment (and your note too), I'll plan to remove the line in Annex D and revert Should we also rename it to something not
Great question. The main challenge is that it was decided (@gibson042 or @ptomato should know the history) that an ECMAScript implementation can support IANA time zones without supporting all of 402. I believe this was for systems that need time-zone-aware arithmetic but don't need localized formatting for end users. Presumably the 262 spec should give implementers guidance for that case. Here's three possible options to do that:
My preference is (1) because I think it'd simplify things for implementers if all identifier-related text is in one document, instead of needing to flip back and forth to understand what they need to do. I'm sensitive to the tradeoff involved: it means parts of the 262 spec will require expertise that 262 editors probably won't have. That's risky, but so is splitting the text. Not sure there's a perfect solution here. That said, I think (2) and (3) are also OK. Both would be better than the current state, so I'm happy to follow whatever y'all decide is best. I somewhat prefer (2) over (3) because if we need to split then it makes the most sense to me to have 262 deal with identifier algorithms while 402 deals with identifier data: how to build TZDB, which IDs are canonical, when to update TZDB, etc. Happy to iterate with you if none of the options above look appealing. |
915301d
to
62a8074
Compare
Hi @syg - Any thoughts on the three options I proposed above? Let me know if you'd like me to join the editor call if you want to talk it through in real time. Ideally we could resolve this fairly soon, because we were hoping to be able to ask for Stage 2 for proposal-canonical-tz at the next TC39 meeting in 3 weeks and we'd like to rebase that proposal's normative text on top of whatever we decide here in this PR. Also I just renamed HostTimeZoneIdentifier => SystemTimeZoneIdentifier and removed that AO from the host-defined section in the spec. |
@justingrant We would appreciate your dropping by the editor call to talk us through it. How about the next one on May 3rd at 2:30pm PT. |
Sounds great. I'll reach out in a DM to get an invite. Looking fwd to it. |
This commit simplifies and clarifies spec text related to time zone identifiers. Goals include making it easier to integrate Temporal into ECMA-262 soon, and simplifying both Temporal and ECMA-402 spec text by centralizing common logic in ECMA-262. If this commit is merged into ECMA-262, editorial PRs will follow for Temporal (draft: tc39/proposal-temporal#2573) and ECMA-402 (draft TBD). Changes: * Adds editorial spec text explaining how time zone identifiers work in ECMAScript, and pointing readers to 402 for implementations that use the IANA TimeZoneDatabase. * Adds definitions related to time zone identifiers. * Renames `DefaultTimeZone` to `SystemTimeZoneIdentifier` in order to more clearly match its intent. * Removes the need to override `SystemTimeZoneIdentifier` in ECMA-402. * Renames variables named _timeZone_ to _timeZoneIdentifier_ to avoid future ambiguity with `Temporal.TimeZone`. * Adds text that more clearly explains existing spec text allowing non-402 implementations to support non-UTC time zones. * Adds new abstract operation `AvailableNamedTimeZoneIdentifiers` which, along with the existing (renamed) `SystemTimeZoneIdentifier`, enables all ECMA-402 and Temporal operations related to time zone identifiers to be implemented on top of these AOs.
34459ee
to
73926a5
Compare
Whoo-hoo! Thanks editors and others for all your help getting this PR done! |
This commit builds on the new ECMA-262 text for time zone identifiers that was introduced in tc39/ecma262#3035. This commit adds Temporal-specific AOs and makes a handful of edits to ECMA-262 AOs for changes that didn't apply to %Date% but that will be required after Temporal is merged into ECMA-262. This commit also revises the ASCII-case-insensitive section to address feedback from ECMA-262 editors before this text was removed from tc39/ecma262#3035: * Remove the "sequence of code points" info because only Strings seemed to use these definitions. * Minor wordsmithing, e.g. "string value" => "String"
This commit builds on the new ECMA-262 text for time zone identifiers that was introduced in tc39/ecma262#3035. This commit adds Temporal-specific AOs and makes a handful of edits to ECMA-262 AOs for changes that didn't apply to %Date% but that will be required after Temporal is merged into ECMA-262. This commit also revises the ASCII-case-insensitive section to address feedback from ECMA-262 editors before this text was removed from tc39/ecma262#3035: * Remove the "sequence of code points" info because only Strings seemed to use these definitions. * Minor wordsmithing, e.g. "string value" => "String"
This commit builds on the new ECMA-262 text for time zone identifiers that was introduced in tc39/ecma262#3035. This commit adds Temporal-specific AOs and makes a handful of edits to ECMA-262 AOs for changes that didn't apply to %Date% but that will be required after Temporal is merged into ECMA-262. This commit also revises the ASCII-case-insensitive section to address feedback from ECMA-262 editors before this text was removed from tc39/ecma262#3035: * Remove the "sequence of code points" info because only Strings seemed to use these definitions. * Minor wordsmithing, e.g. "string value" => "String"
This commit builds on the new ECMA-262 text for time zone identifiers that was introduced in tc39/ecma262#3035. This commit adds Temporal-specific AOs and makes a handful of edits to ECMA-262 AOs for changes that didn't apply to %Date% but that will be required after Temporal is merged into ECMA-262. This commit also revises the ASCII-case-insensitive section to address feedback from ECMA-262 editors before this text was removed from tc39/ecma262#3035: * Remove the "sequence of code points" info because only Strings seemed to use these definitions. * Minor wordsmithing, e.g. "string value" => "String"
This commit builds on the new ECMA-262 text for time zone identifiers that was introduced in tc39/ecma262#3035. This commit adds Temporal-specific AOs and makes a handful of edits to ECMA-262 AOs for changes that didn't apply to %Date% but that will be required after Temporal is merged into ECMA-262. This commit also revises the ASCII-case-insensitive section to address feedback from ECMA-262 editors before this text was removed from tc39/ecma262#3035: * Remove the "sequence of code points" info because only Strings seemed to use these definitions. * Minor wordsmithing, e.g. "string value" => "String"
This commit builds on the new ECMA-262 text for time zone identifiers that was introduced in tc39/ecma262#3035. This commit adds Temporal-specific AOs and makes a handful of edits to ECMA-262 AOs for changes that didn't apply to %Date% but that will be required after Temporal is merged into ECMA-262. This commit also revises the ASCII-case-insensitive section to address feedback from ECMA-262 editors before this text was removed from tc39/ecma262#3035: * Remove the "sequence of code points" info because only Strings seemed to use these definitions. * Minor wordsmithing, e.g. "string value" => "String"
This commit builds on the new ECMA-262 text for time zone identifiers that was introduced in tc39/ecma262#3035. This commit adds Temporal-specific AOs and makes a handful of edits to ECMA-262 AOs for changes that didn't apply to %Date% but that will be required after Temporal is merged into ECMA-262. This commit also revises the ASCII-case-insensitive section to address feedback from ECMA-262 editors before this text was removed from tc39/ecma262#3035: * Remove the "sequence of code points" info because only Strings seemed to use these definitions. * Minor wordsmithing, e.g. "string value" => "String"
This commit builds on the new ECMA-262 text for time zone identifiers that was introduced in tc39/ecma262#3035. This commit adds Temporal-specific AOs and makes a handful of edits to ECMA-262 AOs for changes that didn't apply to %Date% but that will be required after Temporal is merged into ECMA-262. This commit also revises the ASCII-case-insensitive section to address feedback from ECMA-262 editors before this text was removed from tc39/ecma262#3035: * Remove the "sequence of code points" info because only Strings seemed to use these definitions. * Minor wordsmithing, e.g. "string value" => "String"
This commit builds on the new ECMA-262 text for time zone identifiers that was introduced in tc39/ecma262#3035. This commit adds Temporal-specific AOs and makes a handful of edits to ECMA-262 AOs for changes that didn't apply to %Date% but that will be required after Temporal is merged into ECMA-262. This commit also revises the ASCII-case-insensitive section to address feedback from ECMA-262 editors before this text was removed from tc39/ecma262#3035: * Remove the "sequence of code points" info because only Strings seemed to use these definitions. * Minor wordsmithing, e.g. "string value" => "String"
This commit builds on the new ECMA-262 text for time zone identifiers that was introduced in tc39/ecma262#3035. This commit adds Temporal-specific AOs and makes a handful of edits to ECMA-262 AOs for changes that didn't apply to %Date% but that will be required after Temporal is merged into ECMA-262. This commit also revises the ASCII-case-insensitive section to address feedback from ECMA-262 editors before this text was removed from tc39/ecma262#3035: * Remove the "sequence of code points" info because only Strings seemed to use these definitions. * Minor wordsmithing, e.g. "string value" => "String"
This commit builds on the new ECMA-262 text for time zone identifiers that was introduced in tc39/ecma262#3035. This commit adds Temporal-specific AOs and makes a handful of edits to ECMA-262 AOs for changes that didn't apply to %Date% but that will be required after Temporal is merged into ECMA-262. This commit also revises the ASCII-case-insensitive section to address feedback from ECMA-262 editors before this text was removed from tc39/ecma262#3035: * Remove the "sequence of code points" info because only Strings seemed to use these definitions. * Minor wordsmithing, e.g. "string value" => "String"
This commit builds on the new ECMA-262 text for time zone identifiers that was introduced in tc39/ecma262#3035. This commit adds Temporal-specific AOs and makes a handful of edits to ECMA-262 AOs for changes that didn't apply to %Date% but that will be required after Temporal is merged into ECMA-262. This commit also revises the ASCII-case-insensitive section to address feedback from ECMA-262 editors before this text was removed from tc39/ecma262#3035: * Remove the "sequence of code points" info because only Strings seemed to use these definitions. * Minor wordsmithing, e.g. "string value" => "String"
This commit builds on the new ECMA-262 text for time zone identifiers that was introduced in tc39/ecma262#3035. This commit adds Temporal-specific AOs and makes a handful of edits to ECMA-262 AOs for changes that didn't apply to %Date% but that will be required after Temporal is merged into ECMA-262. This commit also revises the ASCII-case-insensitive section to address feedback from ECMA-262 editors before this text was removed from tc39/ecma262#3035: * Remove the "sequence of code points" info because only Strings seemed to use these definitions. * Minor wordsmithing, e.g. "string value" => "String"
This commit builds on the new ECMA-262 text for time zone identifiers that was introduced in tc39/ecma262#3035. This commit adds Temporal-specific AOs and makes a handful of edits to ECMA-262 AOs for changes that didn't apply to %Date% but that will be required after Temporal is merged into ECMA-262. This commit also revises the ASCII-case-insensitive section to address feedback from ECMA-262 editors before this text was removed from tc39/ecma262#3035: * Remove the "sequence of code points" info because only Strings seemed to use these definitions. * Minor wordsmithing, e.g. "string value" => "String"
This commit builds on the new ECMA-262 text for time zone identifiers that was introduced in tc39/ecma262#3035. This commit adds Temporal-specific AOs and makes a handful of edits to ECMA-262 AOs for changes that didn't apply to %Date% but that will be required after Temporal is merged into ECMA-262. This commit also revises the ASCII-case-insensitive section to address feedback from ECMA-262 editors before this text was removed from tc39/ecma262#3035: * Remove the "sequence of code points" info because only Strings seemed to use these definitions. * Minor wordsmithing, e.g. "string value" => "String"
This commit builds on the new ECMA-262 text for time zone identifiers that was introduced in tc39/ecma262#3035. This commit adds Temporal-specific AOs and makes a handful of edits to ECMA-262 AOs for changes that didn't apply to %Date% but that will be required after Temporal is merged into ECMA-262. This commit also revises the ASCII-case-insensitive section to address feedback from ECMA-262 editors before this text was removed from tc39/ecma262#3035: * Remove the "sequence of code points" info because only Strings seemed to use these definitions. * Minor wordsmithing, e.g. "string value" => "String"
This commit builds on the new ECMA-262 text for time zone identifiers that was introduced in tc39/ecma262#3035. This commit adds Temporal-specific AOs and makes a handful of edits to ECMA-262 AOs for changes that didn't apply to %Date% but that will be required after Temporal is merged into ECMA-262. This commit also revises the ASCII-case-insensitive section to address feedback from ECMA-262 editors before this text was removed from tc39/ecma262#3035: * Remove the "sequence of code points" info because only Strings seemed to use these definitions. * Minor wordsmithing, e.g. "string value" => "String"
This commit builds on the new ECMA-262 text for time zone identifiers that was introduced in tc39/ecma262#3035. This commit adds Temporal-specific AOs and makes a handful of edits to ECMA-262 AOs for changes that didn't apply to %Date% but that will be required after Temporal is merged into ECMA-262. This commit also revises the ASCII-case-insensitive section to address feedback from ECMA-262 editors before this text was removed from tc39/ecma262#3035: * Remove the "sequence of code points" info because only Strings seemed to use these definitions. * Minor wordsmithing, e.g. "string value" => "String"
This commit builds on the new ECMA-262 text for time zone identifiers that was introduced in tc39/ecma262#3035. This commit adds Temporal-specific AOs and makes a handful of edits to ECMA-262 AOs for changes that didn't apply to %Date% but that will be required after Temporal is merged into ECMA-262. This commit also revises the ASCII-case-insensitive section to address feedback from ECMA-262 editors before this text was removed from tc39/ecma262#3035: * Remove the "sequence of code points" info because only Strings seemed to use these definitions. * Minor wordsmithing, e.g. "string value" => "String"
This commit builds on the new ECMA-262 text for time zone identifiers that was introduced in tc39/ecma262#3035. This commit adds Temporal-specific AOs and makes a handful of edits to ECMA-262 AOs for changes that didn't apply to %Date% but that will be required after Temporal is merged into ECMA-262. This commit also revises the ASCII-case-insensitive section to address feedback from ECMA-262 editors before this text was removed from tc39/ecma262#3035: * Remove the "sequence of code points" info because only Strings seemed to use these definitions. * Minor wordsmithing, e.g. "string value" => "String"
Removes an unnecessary line in SystemTimeZoneIdentifier. Originally this AO contained an assert that applied to named time zones but not offset time zones. Based on editors' feedback in tc39#3035, this assertion was replaced with different text, making the remaining If statement unnecessary. But I didn't notice this until after tc39#3035 was merged. This commit removes the now-unnecessary If statement.
Now that tc39/ecma262#3035 and tc39/proposal-temporal#2573 have been merged, this commit aligns the contextual text around this proposal's changes to the final merged text in those PRs. It also removes links to those PRs and replaces them with links to main. Note that this commit doesn't update any spec text changes that this proposal makes; only contextual text that surrounds this proposal's changes has been updated here.
Removes an unnecessary line in SystemTimeZoneIdentifier. Originally this AO contained an assert that applied to named time zones but not offset time zones. Based on editors' feedback in tc39#3035, this assertion was replaced with different text, making the remaining If statement unnecessary. But I didn't notice this until after tc39#3035 was merged. This commit removes the now-unnecessary If statement.
Removes an unnecessary line in SystemTimeZoneIdentifier. Originally this AO contained an assert that applied to named time zones but not offset time zones. Based on editors' feedback in tc39#3035, this assertion was replaced with different text, making the remaining If statement unnecessary. But I didn't notice this until after tc39#3035 was merged. This commit removes the now-unnecessary If statement.
This PR simplifies and clarifies spec text related to time zone identifiers, but is not intended to change any observable behavior nor implementer requirements.
Goals of this PR include to make it easier to later integrate Temporal into ECMA-262, to simplify both Temporal and ECMA-402 spec text by centralizing common logic in ECMA-262, and to make it less disruptive to make future normative changes to time zone identifier behavior.
If this PR is merged into ECMA-262, editorial PRs will follow for Temporal (draft: tc39/proposal-temporal#2573) and ECMA-402 (draft TBD) to leverage the updated spec text and AOs to simplify those specs.
Changes:
DefaultTimeZone
toSystemTimeZoneIdentifier
in order to more clearly match its intent.SystemTimeZoneIdentifier
in ECMA-402.Temporal.TimeZone
.AvailableNamedTimeZoneIdentifiers
which, along with the existing (renamed to)SystemTimeZoneIdentifier
, enables all ECMA-402 and Temporal operations related to time zone identifiers to be implemented on top of these AOs.EDIT: Other editorial changes that were originally in this PR have been moved into Temporal and will be merged along with Temporal later, either into 262 or 402.
cc @gibson042