Skip to content

Update i18n locale data extraction script #21626

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.

Sign up for GitHub

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

Jump to bottom
Closed
ocombe wants to merge 2 commits into from
Closed

Update i18n locale data extraction script #21626

ocombe wants to merge 2 commits into from

Conversation

@ocombe
Copy link
Contributor

@ocombe ocombe commented Jan 18, 2018 •
edited
Loading

PR Type

What kind of change does this PR introduce?

[x] Bugfix

What is the current behavior?

We have multiple issues with the extraction script for CLDR data:

  • in node 8 they changed the inspect function from util to print missing values from arrays (util: change sparse arrays inspection format nodejs/node#11576) resulting in corrupted data files
  • plural functions are also corrupted because of ``` characters that were not present before for some reason (new version of uglify maybe?)
  • we need to extract the plural functions from data arrays for TS 2.6
  • some invisible characters (such as the left to right/right to left UTF characters \u200e & \u200f) were stripped away by the inspect method

Issue Number: #21608

What is the new behavior?

I've replaced the inspect function by a modified JSON5.stringify method which fixed all of the issues, and I've extracted the plural function as requested. You can find the new LTR/RTL characters in languages such as arabic (ar.ts) or hebrew (he.ts)
I pushed two different commits since I did two different updates.

Does this PR introduce a breaking change?

[x] No

@ocombe ocombe added action: review The PR is still awaiting reviews from at least one requested reviewer area: i18n Issues related to localization and internationalization target: patch This PR is targeted for the next patch release labels Jan 18, 2018
@ocombe ocombe requested a review from vicb January 18, 2018 13:17
@ocombe ocombe force-pushed the fix/#21608-update-extract-script branch 2 times, most recently from 47d6bf1 to dfe26ed Compare January 18, 2018 13:37
@angular angular deleted a comment from mary-poppins Jan 18, 2018
@mary-poppins
Copy link

mary-poppins commented Jan 18, 2018

You can preview dfe26ed at https://pr21626-dfe26ed.ngbuilds.io/.

@angular angular deleted a comment from mary-poppins Jan 18, 2018
@ocombe ocombe force-pushed the fix/#21608-update-extract-script branch from dfe26ed to 97312d8 Compare January 18, 2018 15:39
@mary-poppins
Copy link

mary-poppins commented Jan 18, 2018

You can preview 97312d8 at https://pr21626-97312d8.ngbuilds.io/.

@vicb vicb added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Jan 18, 2018
@vicb
Copy link
Contributor

vicb commented Jan 18, 2018

While the TS 2.6 commit (second one) is not strictly required for the patch branch, it's better to keep generated files in sync between master and patch.

@gregbown gregbown mentioned this pull request Jan 19, 2018
Closed
@mhevery
Copy link
Contributor

mhevery commented Jan 19, 2018

running g3sync presubmit http://test/OCL:182567375:BASE:182567631:1516391751302:a8e6c969

@mhevery mhevery closed this in 135a282 Jan 19, 2018
mhevery pushed a commit that referenced this pull request Jan 19, 2018
@ocombe @mhevery
fix(common): extract plural function from i18n locale data files for …
97b18b2 
…TS 2.6 (#21626)

Fixes #21608

PR Close #21626
mhevery pushed a commit to mhevery/angular that referenced this pull request Jan 25, 2018
@ocombe @mhevery
fix(common): don't remove special characters when extracting CLDR data (
a62c186 
angular#21626)

PR Close angular#21626
mhevery pushed a commit to mhevery/angular that referenced this pull request Jan 25, 2018
@ocombe @mhevery
fix(common): extract plural function from i18n locale data files for …
71f9eaa 
…TS 2.6 (angular#21626)

Fixes angular#21608

PR Close angular#21626
@ocombe ocombe mentioned this pull request Jan 29, 2018
Closed
@angular-automatic-lock-bot
Copy link

angular-automatic-lock-bot bot commented Sep 13, 2019

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@vicb vicb vicb approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

action: merge The PR is ready for merge by the caretaker area: i18n Issues related to localization and internationalization cla: yes target: patch This PR is targeted for the next patch release

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@ocombe @mary-poppins @vicb @mhevery @googlebot