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

Add 'calendar' option #54

Merged
merged 4 commits into from Oct 17, 2019

Conversation

@FrankYFTang
Copy link
Collaborator

commented Oct 3, 2019

Address #52
Also bump the stage to 3
and change "six types" to "nine types" (#53 )

spec.emu Outdated Show resolved Hide resolved
spec.emu Outdated Show resolved Hide resolved
spec.emu Outdated Show resolved Hide resolved
1. Let _calendar_ be ? GetOption(_options_, `"calendar"`, `"string"`, *undefined*, *undefined*).
1. If _calendar_ is not *undefined*, then
1. If _calendar_ does not match the `(3*8alphanum) *("-" (3*8alphanum))` sequence, throw a *RangeError* exception.
1. Set _opt_.[[ca]] to _calendar_.
1. Let _r_ be ResolveLocale(%DisplayNames%.[[AvailableLocales]], _requestedLocales_, _opt_, %DisplayNames%.[[RelevantExtensionKeys]]).

This comment has been minimized.

Copy link
@sffc

sffc Oct 3, 2019

Collaborator

It looks like this means the -u-ca- in the locale overrides the { calendar } option. This appears to be consistent with tc39/ecma402#175.

This comment has been minimized.

Copy link
@FrankYFTang

FrankYFTang Oct 4, 2019

Author Collaborator

no, this only pass the calendar option to the ResolveLocale. Inside ResolveLocale, it will consider that before consider the "-u-ca-"

@FrankYFTang

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 8, 2019

Other reviewers- Ping

@leobalter

This comment has been minimized.

Copy link
Member

commented Oct 17, 2019

It would be nice to have this one landing so it's easier to reflect on the tests.

@FrankYFTang

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 17, 2019

Since @leobalter approved, and no other reviewers express objection in the last 14 days, I will land this PR now. Please open new ticket if you find issues with it after I merge.

@FrankYFTang FrankYFTang merged commit e8b5ba5 into tc39:master Oct 17, 2019
@FrankYFTang FrankYFTang deleted the FrankYFTang:addCalendar branch Oct 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.