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

Fix #2770: Implement Intl.getCanonicalLocales #2818

Merged
merged 5 commits into from
May 12, 2017

Conversation

dilijev
Copy link
Contributor

@dilijev dilijev commented Apr 8, 2017

@dilijev
Copy link
Contributor Author

dilijev commented Apr 10, 2017

@dotnet-bot test Ubuntu static_ubuntu_linux_release please

@dilijev
Copy link
Contributor Author

dilijev commented Apr 17, 2017

Ping @bterlson @kunalspathak @tcare

assert.areEqual(Intl.getCanonicalLocales({ '0': 'en-us' }), [], "Objects which might look like arrays are fine, but treated as 0 length.");
assert.areEqual(Intl.getCanonicalLocales({ 'a': 'b' }), [], "Arbitrary Objects are fine, treated as 0-length arrays.");

// TODO test this
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we leaving this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah sorry about the TODOs. I wanted to get this out for review. I'll add more tests to cover the gaps in the next iteration.

assert.areEqual(Intl.getCanonicalLocales('xx-zzz'), ['xx-zzz'], ""); // TODO double-check canonicalization routine, v8 makes this 'zzz'
assert.areEqual(Intl.getCanonicalLocales('xx-abcd-zz'), ['xx-Abcd-ZZ'],
"xx-Abcd-ZZ: [unsupported language xx] using [unsupported script Abcd] as used in [unsupported locale ZZ]");
// TODO -u- extension keys
Copy link
Contributor

Choose a reason for hiding this comment

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

+1

@@ -1,4 +1,4 @@
//-------------------------------------------------------------------------------------------------------
//-------------------------------------------------------------------------------------------------------
Copy link
Contributor

Choose a reason for hiding this comment

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

There should be awards for bug fixes this severe.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

lol. Like I said, drive by cleanup. :P

@@ -51,7 +51,7 @@ var tests = [
}
catch (e) {
if (!(e instanceof RangeError)) {
fail("Incorrect exception was thrown.");
assert.fail("Incorrect exception was thrown.");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

/fyi @veler

@dilijev dilijev changed the base branch from master to release/2.0 May 11, 2017 22:53
@dilijev dilijev force-pushed the intl-getCanonicalLocales branch 3 times, most recently from e4a6dc6 to ca56a2b Compare May 11, 2017 23:08
@dilijev
Copy link
Contributor Author

dilijev commented May 11, 2017

Note: I'm porting this PR to release/2.0 since it seems relevant there.
This involves porting #2726 from master to release/2.0 so I included that in this PR as well.

@dilijev
Copy link
Contributor Author

dilijev commented May 11, 2017

Fixing CI in #2962

@dilijev
Copy link
Contributor Author

dilijev commented May 12, 2017

Nevermind -- after talking offline with @digitalinfinity this is not needed in release/2.0. Retargeting back to master.

@dilijev dilijev changed the base branch from release/2.0 to master May 12, 2017 00:38
@dilijev dilijev force-pushed the intl-getCanonicalLocales branch 3 times, most recently from a379e5b to 578717d Compare May 12, 2017 02:51
* NumberFormatOptions.js: Move test which is the same everywhere to the platform-common test.
* DateTimeFormatOptions.js: Fix fail -> assert.fail.
* Remove explicit -Intl from tests. They are tagged Intl already and if the tests are running then Intl will be on by default in that configuration.
@chakrabot chakrabot merged commit fe530a8 into chakra-core:master May 12, 2017
@dilijev dilijev deleted the intl-getCanonicalLocales branch May 12, 2017 21:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants