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

updated Intl.DateTimeFormat.prototype.resolvedOptions() tests to reflect recent normative change #3885

Merged
merged 3 commits into from
Oct 2, 2023

Conversation

ben-allen
Copy link
Contributor

@ben-allen ben-allen commented Jul 26, 2023

Address #3812
Fix #3876

tc39/ecma402#758 changes the hourCycle logic in Intl.DateTimeFormat.InitializeDateAndTime() to no longer erroneously assume that h24 is the 24 hour clock expected in locales using h11 or h12 as defaults.

7353744 and 15792da update hourCycle-default.js to reflect the new behaviour.

note: I don't believe any engine currently passes all of the tests in the new hourCycle-default.js. v8 and node fail because they associate h12 with h24, as was required by the old algorithm, while jsc and spidermonkey fail because they ignore the -u-hc-h24 extension when hour12 is set to false, as below:

// sm and jsc
new Intl.DateTimeFormat('en-u-hc-h24', {hour: "numeric", hour12: false}).resolvedOptions().hourCycle;
=> "h23"

commit 2648144 updates hourCycle-dateStyle.js with explanatory comments. Essentially, all this test does is verify that hour12 and hourCycle are set to undefined when dateStyle but not timeStyle is set, but the comments and metadata in the previous version appear to say something different.

@ben-allen ben-allen requested a review from a team as a code owner July 26, 2023 16:18
@ben-allen ben-allen marked this pull request as draft July 26, 2023 16:41
@ben-allen ben-allen force-pushed the pr-758 branch 3 times, most recently from 3f447aa to 15792da Compare July 27, 2023 01:04
@ben-allen ben-allen marked this pull request as ready for review July 27, 2023 01:06
@Ms2ger Ms2ger added the awaiting consensus This needs committee consensus before it can be eligible to be merged. label Aug 25, 2023
Copy link
Contributor

@Ms2ger Ms2ger left a comment

Choose a reason for hiding this comment

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

Ready when the spec is.

@ptomato
Copy link
Contributor

ptomato commented Oct 2, 2023

The normative change reached consensus in the TC39 meeting of 2023-09. Given Ms2ger's approval I think we can merge this. I squashed the updates from addressing code review comments into the original topical commits.

@ptomato ptomato merged commit 5e6f25b into tc39:main Oct 2, 2023
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting consensus This needs committee consensus before it can be eligible to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Need test for ECMA402 #758 Normative: Change the hourCycle default logic
3 participants