-
Notifications
You must be signed in to change notification settings - Fork 107
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: Added support for sentence break suppressions to Intl.Segmenter #783
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.
LGTM
<emu-note> | ||
Intl.Segmenter does not have any relevant extension keys. | ||
</emu-note> |
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.
There are actually others: lb, lw, dx. See https://www.unicode.org/reports/tr35/#UnicodeLineBreakStyleIdentifier
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.
lb and lw are only for line break, which Intl.Segmenter doesn't support.
dx is not widely used or implemented, and I've raised questions about its utility.
Co-authored-by: Shane F. Carr <[email protected]>
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.
spec changes LGTM, none of the three mentioned extension keys are supported in Intl
yet and we shouldn't introduce more than one in a single PR so it looks good to go from my end. Let's see how people feel about this.
Needs feedback from @Constellation. Also, @FrankYFTang pointed out that changes are needed in Appendix A/B. |
Sorry, I think we need additional changes that I forget to mention in TG2. A. We should also change B. The internal slot of Locale should also include "ss" C. We should also add a getter in section 14.3 which return either "none" (default) or "standard") |
9a97754
to
a873ee4
Compare
…e break suppressions data to Annex A
a873ee4
to
4e83568
Compare
Updated to incorporate required changes to Intl.Locale & include a note in Annex A. Unclear on needed changes to Annex B. |
@@ -52,6 +52,8 @@ <h1>Intl.Locale ( _tag_ [ , _options_ ] )</h1> | |||
1. If _numberingSystem_ is not *undefined*, then | |||
1. If _numberingSystem_ does not match the Unicode Locale Identifier `type` nonterminal, throw a *RangeError* exception. | |||
1. Set _opt_.[[nu]] to _numberingSystem_. | |||
1. Let _sentenceBreakSuppressions_ be ? GetOption(_options_, *"sentenceBreakSuppressions"*, ~string~, «*"none"*, *"standard"* », *"none"*). |
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.
For Intl.Locale constructor, the default value for "sentenceBreakSuppressions" should be undefined. (Comparing to other options, such as "calendar", "collation", "hourCycle", "caseFirst", "numeric", "numberingSystem")
Note: @dminor would like to wait until ICU4X supports this feature before merging this PR. |
The PR is not mergeable any more in its current form, so I'll close it, and we can make a new one once the ICU4X issue is resolved. |
fix #580
Added support for u-ss to Intl.Segmenter