-
Notifications
You must be signed in to change notification settings - Fork 475
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
Remove locales with explicit u-hc Unicode extension #3958
Conversation
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.
Given:
// no locale has "h24" as a default. see https://github.com/tc39/ecma402/pull/758#issue-1622377292
This comment / code path no longer seems relevant:
// however, "h24" can be set via locale extension.
OK. @ben-allen explained this to me and I think I understand it. With that understanding (which I may still have gotten wrong), Linus's comment makes sense to me. Maybe the loop body should look something like this? let hcDefault = new Intl.DateTimeFormat(locale, { hour: "numeric" }).resolvedOptions().hourCycle;
let hcHour12 = new Intl.DateTimeFormat(locale, { hour: "numeric", hour12: true }).resolvedOptions().hourCycle;
assert(hcHour12 === "h11" || hcHour12 === "h12", "Expected hourCycle for hour12: true to be in [\"h11\", \"h12\"]");
if (hcDefault === "h11" || hcDefault === "h12") {
assert.sameValue(hcHour12, hcDefault);
}
// no locale has "h24" as a default. see https://github.com/tc39/ecma402/pull/758#issue-1622377292
let hcNotHour12 = new Intl.DateTimeFormat(locale, { hour: "numeric", hour12: false }).resolvedOptions().hourCycle;
assert.sameValue(hcNotHour12, "h23", "Expected hourCycle for hour12: false to be \"h23\""); |
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.
(see previous comment, which I neglected to send as a review comment)
I've simply changed the test to match what I've originally proposed in tc39/ecma402#758 (comment). |
The tests are passing an explicit `hour12` option, which disables any `hourCycle` option and any `u-hc` Unicode extension. Therefore passing input locales with `u-hc` is invalid. ```js let locale = "en-u-hc-h24"; let hcDefault = new Intl.DateTimeFormat(locale, { hour: "numeric" }).resolvedOptions().hourCycle; assert.sameValue(hcDefault, "h24", "hour-cycle through Unicode extension"); let hc24 = new Intl.DateTimeFormat(locale, { hour: "numeric", hour12: false }).resolvedOptions().hourCycle; // Incorrect assertion, because |hc24| uses |hour12|, which disables any // hour-cycle option and the hour-cycle is determined from the locale "en". // That means |hc24| will be "h23". assert.sameValue(hc24, hcDefault); ```
OK, fair enough, that seems fine for now. |
The tests are passing an explicit
hour12
option, which disables anyhourCycle
option and anyu-hc
Unicode extension. Therefore passing input locales withu-hc
is invalid.