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

Normative: Allow Collator to get collation from option #459

Merged
merged 5 commits into from
Jan 5, 2021

Conversation

FrankYFTang
Copy link
Contributor

Collator already have "co" in [[RelevantExtensionKeys]] internal slot and therefore the usage by using "-u-co-$collationkey" is already supported in ECMA402.

This PR will also make our extension / option handling in ECMA402 consistent.

I check other object and I believe this is the only inconsistent handling between u extension and option now.

Close #380

Collator already have "co" in  [[RelevantExtensionKeys]] internal slot and therefore the  usage by using "-u-co-$collationkey" is already supported in ECMA402.

Close #380
spec/collator.html Outdated Show resolved Hide resolved
spec/collator.html Outdated Show resolved Hide resolved
To make it the same as the order already in Table 3: Resolved Options of Collator Instances
Copy link
Contributor Author

@FrankYFTang FrankYFTang left a comment

Choose a reason for hiding this comment

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

PTAL

@sffc sffc added this to Priority Issues in ECMA-402 Meeting Topics Jun 17, 2020
@sffc
Copy link
Contributor

sffc commented Jun 17, 2020

Looks good! Let's try to get consensus at the next 402 meeting.

@leobalter leobalter changed the title Allow Collator to get collation from option Normative: Allow Collator to get collation from option Jun 18, 2020
@leobalter leobalter added needs tests Small Smaller change solvable in a Pull Request labels Jun 18, 2020
Comment on lines 36 to 39
1. Let _collation_ be ? GetOption(_options_, `"collation"`, `"string"`, *undefined*, *undefined*).
1. If _collation_ is not *undefined*, then
1. If _collation_ does not match the Unicode Locale Identifier `type` nonterminal, throw a *RangeError* exception.
1. Set _opt_.[[co]] to _collation_.

This comment was marked as resolved.

@@ -14,6 +14,10 @@ <h1>InitializeCollator ( _collator_, _locales_, _options_ )</h1>
The abstract operation InitializeCollator accepts the arguments _collator_ (which must be an object), _locales_, and _options_. It initializes _collator_ as a *Collator* object. The following steps are taken:
</p>

<p>
The following algorithm refers to the `type` nonterminal from <a href="http://www.unicode.org/reports/tr35/#Unicode_locale_identifier">UTS 35's Unicode Locale Identifier grammar</a>.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: "https" instead of "http".

@FrankYFTang FrankYFTang mentioned this pull request Jun 20, 2020
@FrankYFTang FrankYFTang moved this from Priority Issues to Previously Discussed in ECMA-402 Meeting Topics Jul 9, 2020
@sffc
Copy link
Contributor

sffc commented Jul 20, 2020

TC39 gave consensus for this change on 2020-07-20.

@FrankYFTang
Copy link
Contributor Author

@FrankYFTang
Copy link
Contributor Author

I still have "You’re not authorized to merge this pull request." next to the grey "Merge pull request" button.

@FrankYFTang
Copy link
Contributor Author

FYI- landed into v8 and should be available in Chrome 86

pull bot pushed a commit to Alan-love/v8 that referenced this pull request Jul 22, 2020
Per change in tc39/ecma402#459

Bug: v8:10732
Change-Id: I2ef21e8b450cbf9c61f987c61f3ba7d6959db81a
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2309149
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Frank Tang <ftang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#68976}
@sffc sffc added has consensus Has consensus from TC39-TG2 has consensus (TG1) Has consensus from TC39-TG1 labels Aug 13, 2020
@Constellation
Copy link
Collaborator

FYI, JSC enabled it on Trunk https://trac.webkit.org/changeset/267102/webkit

@FrankYFTang
Copy link
Contributor Author

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Nov 24, 2020
… r=yulia

Implements the changes from the "has consensus" PR <tc39/ecma402#459>.

Drive-by change:
- Enable a named groups RegExp test which wasn't enabled when bug 1362154 was implemented.

Differential Revision: https://phabricator.services.mozilla.com/D95734
sidvishnoi pushed a commit to sidvishnoi/gecko-webmonetization that referenced this pull request Nov 27, 2020
… r=yulia

Implements the changes from the "has consensus" PR <tc39/ecma402#459>.

Drive-by change:
- Enable a named groups RegExp test which wasn't enabled when bug 1362154 was implemented.

Differential Revision: https://phabricator.services.mozilla.com/D95734
ryanhaddad pushed a commit to WebKit/WebKit that referenced this pull request Dec 22, 2020
https://bugs.webkit.org/show_bug.cgi?id=216529

Reviewed by Ross Kirsling.

JSTests:

* stress/intl-collator-co-extension.js:
(shouldThrow):

Source/JavaScriptCore:

This patch adds "collation" option to Intl.Collator. We are already getting consensus[1], and will be integrated into the spec.
Previously, passing "collation" is only available through "-u-co-" unicode extension in the passed locale. The proposal exposes
collation option as an option to Intl.Collator so that we can set it easily.
"collation" is used only when "usage" is "sort". "search" usage will filter out collation options since "search" itself is one of
the "collation" option.

[1]: tc39/ecma402#459

* runtime/IntlCollator.cpp:
(JSC::IntlCollator::sortLocaleData):
(JSC::IntlCollator::initializeCollator):

Canonical link: https://commits.webkit.org/229383@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@267102 268f45cc-cd09-0410-ab3c-d52691b4dbfc
@leobalter leobalter merged commit a57ae15 into master Jan 5, 2021
@leobalter leobalter deleted the FrankYFTang-patch-4 branch January 5, 2021 01:51
@ryzokuken
Copy link
Member

@FrankYFTang @sffc https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/Collator/Collator#parameters already lists "collation" as an accepted option for Intl.Collator. Does this PR need any further MDN documentation?

@FrankYFTang
Copy link
Contributor Author

It seems they list the possible values there. Not sure what is missing. Please at least add "zhuyin" as possible value

<keyword>
    <key name="co" description="Collation type key" alias="collation">
        <type name="big5han" description="Pinyin ordering for Latin, big5 charset ordering for CJK characters (used in Chinese)"/>
        <type name="compat" description="A previous version of the ordering, for compatibility" since="26"/>
        <type name="dict" description="Dictionary style ordering (such as in Sinhala)" alias="dictionary"/>
        <type name="direct" description="Binary code point order (used in Hindi)" deprecated="true"/>
        <type name="ducet" description="The default Unicode collation element table order" since="2.0.1"/>
        <type name="emoji" description="Recommended ordering for emoji characters" since="27"/>
        <type name="eor" description="European ordering rules" since="24"/>
        <type name="gb2312" description="Pinyin ordering for Latin, gb2312han charset ordering for CJK characters (used in Chinese)" alias="gb2312han"/>
        <type name="phonebk" description="Phonebook style ordering (such as in German)" alias="phonebook"/>
        <type name="phonetic" description="Phonetic ordering (sorting based on pronunciation)"/>
        <type name="pinyin" description="Pinyin ordering for Latin and for CJK characters (used in Chinese)"/>
        <type name="reformed" description="Reformed ordering (such as in Swedish)"/>
        <type name="search" description="Special collation type for string search" since="1.9"/>
        <type name="searchjl" description="Special collation type for Korean initial consonant search" since="2.0.1"/>
        <type name="standard" description="Default ordering for each language"/>
        <type name="stroke" description="Pinyin ordering for Latin, stroke order for CJK characters (used in Chinese)"/>
        <type name="trad" description="Traditional style ordering (such as in Spanish)" alias="traditional"/>
        <type name="unihan" description="Pinyin ordering for Latin, Unihan radical-stroke ordering for CJK characters (used in Chinese)"/>
        <type name="zhuyin" description="Pinyin ordering for Latin, zhuyin order for Bopomofo and CJK characters (used in Chinese)" since="22"/>
    </key>

I think we should add all those missed from above (for example "zhuyin") except "search" and "standard" that is clearly stated not allowed in ECMA402

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has consensus (TG1) Has consensus from TC39-TG1 has consensus Has consensus from TC39-TG2 has tests Small Smaller change solvable in a Pull Request
Projects
ECMA-402 Meeting Topics
Previously Discussed
Development

Successfully merging this pull request may close these issues.

Add "collation" option to Intl.Collator
9 participants