Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2248563003:
Support language tag extensions with multiple subtags for a key (Closed)
|
Created:
4 years, 4 months ago by jungshik at Google Modified:
4 years, 4 months ago Reviewers:
Dan Ehrenberg CC:
v8-reviews_googlegroups.com Base URL:
https://chromium.googlesource.com/v8/v8.git@master Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
DescriptionSupport language tag extensions with multiple subtags for a key
Language tags with Unicode extensions can have multiple subtags
for a key (e.g. -ca-ismalic-civil has 'islamic-civi' for 'ca').
BUG=v8:4749
TEST=intl/date-format/calendar-with-multiple-type-subtags.js
Committed: https://crrev.com/339f08d2e9650e05d63c6a756b8302c60ee665b5
Cr-Commit-Position: refs/heads/master@{#38692}
Patch Set 1 #Patch Set 2 : add test #Patch Set 3 : add a real test for nu in presence of multiple type subtags for ca #
Messages
Total messages: 23 (15 generated)
The CQ bit was checked by jshin@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from
==========
Support language tag extensions with a multiword value for a key
Language tags with Unicode extensions can have a 'multiword value'
for a key (e.g. -ca-ismalic-civil has 'islamic-civi' for 'ca').
BUG=4749
TEST=
> Intl.DateTimeFormat("am-u-ca-ethiopic-amete-alem").resolvedOptions().calendar
ethioaa
> Intl.DateTimeFormat("ar-u-ca-islamic-civil").resolvedOptions().calendar
islamic-civil
==========
to
==========
Support language tag extensions with a multiple subtags for a key
Language tags with Unicode extensions can have multiple subtags
for a key (e.g. -ca-ismalic-civil has 'islamic-civi' for 'ca').
BUG=v8:4749
TEST=intl/date-format/calendar-with-multiple-type-subtags.js
==========
jshin@chromium.org changed reviewers: + littledan@chromium.org
Dan, can you take a look? Thanks
Description was changed from ========== Support language tag extensions with a multiple subtags for a key Language tags with Unicode extensions can have multiple subtags for a key (e.g. -ca-ismalic-civil has 'islamic-civi' for 'ca'). BUG=v8:4749 TEST=intl/date-format/calendar-with-multiple-type-subtags.js ========== to ========== Support language tag extensions with multiple subtags for a key Language tags with Unicode extensions can have multiple subtags for a key (e.g. -ca-ismalic-civil has 'islamic-civi' for 'ca'). BUG=v8:4749 TEST=intl/date-format/calendar-with-multiple-type-subtags.js ==========
The CQ bit was checked by jshin@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The state machine for parsing multiple BCP47 tags looks a little tricky. Do you think you could add a test case for multiple key-value pairs? (or are there no meaningful examples of that with what ECMA402 exposes?) Otherwise lgtm, good to see this fix.
Thank you for taking a look.
On 2016/08/17 05:36:10, Dan Ehrenberg wrote:
> The state machine for parsing multiple BCP47 tags looks a little tricky. Do
you
> think you could add a test case for multiple key-value pairs? (or are there no
> meaningful examples of that with what ECMA402 exposes?) Otherwise lgtm, good
to
> see this fix.
I have one in the test:
options =
Intl.DateTimeFormat("ar-u-ca-islamic-civil-nu-arab").resolvedOptions();
assertEquals(options.calendar, "islamic-civil");
assertEquals(options.numberingSystem, "arab");
Or, did you have something different in mind?
On 2016/08/17 at 09:44:34, jshin wrote:
> Thank you for taking a look.
>
> On 2016/08/17 05:36:10, Dan Ehrenberg wrote:
> > The state machine for parsing multiple BCP47 tags looks a little tricky. Do
you
> > think you could add a test case for multiple key-value pairs? (or are there
no
> > meaningful examples of that with what ECMA402 exposes?) Otherwise lgtm, good
to
> > see this fix.
>
> I have one in the test:
>
> options =
> Intl.DateTimeFormat("ar-u-ca-islamic-civil-nu-arab").resolvedOptions();
> assertEquals(options.calendar, "islamic-civil");
> assertEquals(options.numberingSystem, "arab");
>
> Or, did you have something different in mind?
I don't see a test that the nu tag is parsed correctly.
On 2016/08/17 13:48:46, Dan Ehrenberg wrote:
> On 2016/08/17 at 09:44:34, jshin wrote:
> > Thank you for taking a look.
> >
> > On 2016/08/17 05:36:10, Dan Ehrenberg wrote:
> > > The state machine for parsing multiple BCP47 tags looks a little tricky.
Do
> you
> > > think you could add a test case for multiple key-value pairs? (or are
there
> no
> > > meaningful examples of that with what ECMA402 exposes?) Otherwise lgtm,
good
> to
> > > see this fix.
> >
> > I have one in the test:
> >
> > options =
> > Intl.DateTimeFormat("ar-u-ca-islamic-civil-nu-arab").resolvedOptions();
> > assertEquals(options.calendar, "islamic-civil");
> > assertEquals(options.numberingSystem, "arab");
> >
> > Or, did you have something different in mind?
>
> I don't see a test that the nu tag is parsed correctly.
Ok. 'ar' has the default 'nu' set to 'arab' so that my test is not effective.
Thanks for pointing that out. I added 'nu-latn' to 'ar-....'.
The CQ bit was checked by jshin@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from littledan@chromium.org Link to the patchset: https://codereview.chromium.org/2248563003/#ps40001 (title: "add a real test for nu in presence of multiple type subtags for ca")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Support language tag extensions with multiple subtags for a key Language tags with Unicode extensions can have multiple subtags for a key (e.g. -ca-ismalic-civil has 'islamic-civi' for 'ca'). BUG=v8:4749 TEST=intl/date-format/calendar-with-multiple-type-subtags.js ========== to ========== Support language tag extensions with multiple subtags for a key Language tags with Unicode extensions can have multiple subtags for a key (e.g. -ca-ismalic-civil has 'islamic-civi' for 'ca'). BUG=v8:4749 TEST=intl/date-format/calendar-with-multiple-type-subtags.js ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Support language tag extensions with multiple subtags for a key Language tags with Unicode extensions can have multiple subtags for a key (e.g. -ca-ismalic-civil has 'islamic-civi' for 'ca'). BUG=v8:4749 TEST=intl/date-format/calendar-with-multiple-type-subtags.js ========== to ========== Support language tag extensions with multiple subtags for a key Language tags with Unicode extensions can have multiple subtags for a key (e.g. -ca-ismalic-civil has 'islamic-civi' for 'ca'). BUG=v8:4749 TEST=intl/date-format/calendar-with-multiple-type-subtags.js Committed: https://crrev.com/339f08d2e9650e05d63c6a756b8302c60ee665b5 Cr-Commit-Position: refs/heads/master@{#38692} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/339f08d2e9650e05d63c6a756b8302c60ee665b5 Cr-Commit-Position: refs/heads/master@{#38692} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
