Skip to content

fix(compiler): allow numbers as ICU select cases - #18095

Closed
ocombe wants to merge 2 commits into
angular:masterfrom
ocombe:fix-icu-numbers
Closed

fix(compiler): allow numbers as ICU select cases#18095
ocombe wants to merge 2 commits into
angular:masterfrom
ocombe:fix-icu-numbers

Conversation

@ocombe

@ocombe ocombe commented Jul 13, 2017

Copy link
Copy Markdown
Contributor

PR Type

What kind of change does this PR introduce?

[x] Bugfix

What is the current behavior?

We cannot use number values for cases of select ICUs even if the number is a string value: {gender, select, 1 {male} f {female}} fails with error message Invalid ICU message. Missing '}'.

Issue Number: #17799

What is the new behavior?

We allow numbers in select ICUs for cases

Does this PR introduce a breaking change?

[x] No

@ocombe ocombe added area: i18n Issues related to localization and internationalization action: review The PR is still awaiting reviews from at least one requested reviewer labels Jul 13, 2017
@ocombe
ocombe requested a review from vicb July 13, 2017 11:59
vicb
vicb reviewed Jul 18, 2017
View reviewed changes
Comment thread packages/compiler/test/i18n/integration_common.ts Outdated

@vicb vicb Jul 18, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: change any to other

vicb
vicb reviewed Jul 18, 2017
View reviewed changes
Comment thread packages/compiler/test/ml_parser/html_parser_spec.ts Outdated

@vicb vicb Jul 18, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: "not error" -> that's not what you test (no .not.toThrow())

"should support ..."

@vicb vicb added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Jul 18, 2017
@ocombe
ocombe force-pushed the fix-icu-numbers branch from 38c44af to 26193c8 Compare July 19, 2017 08:33
@angular angular deleted a comment from mary-poppins Jul 19, 2017
@mary-poppins

mary-poppins commented Jul 19, 2017

Copy link
Copy Markdown

You can preview 26193c8 at https://pr18095-26193c8.ngbuilds.io/.

@vicb vicb added action: merge The PR is ready for merge by the caretaker and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Jul 19, 2017
@ocombe
ocombe force-pushed the fix-icu-numbers branch from 26193c8 to 3119a08 Compare July 19, 2017 16:49
@googlebot

googlebot commented Jul 20, 2017

Copy link
Copy Markdown

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

@mary-poppins

mary-poppins commented Jul 20, 2017

Copy link
Copy Markdown

You can preview 660f33a at https://pr18095-660f33a.ngbuilds.io/.

@mhevery mhevery closed this in a3a5429 Jul 20, 2017
@ocombe
ocombe deleted the fix-icu-numbers branch July 21, 2017 12:13
mhevery pushed a commit that referenced this pull request Jul 26, 2017
@ocombe @mhevery
fix(compiler): allow numbers for ICU message cases in lexer (#18095)
a8ac77b 
Closes #18095
Fixes #17799
asnowwolf pushed a commit to asnowwolf/angular that referenced this pull request Aug 11, 2017
@ocombe Zhicheng Wang
fix(compiler): allow numbers for ICU message cases in lexer (angular#…
8f6d51b 
…18095)

Closes angular#18095
Fixes angular#17799
juleskremer pushed a commit to juleskremer/angular that referenced this pull request Aug 26, 2017
@ocombe @juleskremer
fix(compiler): allow numbers for ICU message cases in lexer (angular#…
cc01e90 
…18095)

Closes angular#18095
Fixes angular#17799
juleskremer pushed a commit to juleskremer/angular that referenced this pull request Aug 28, 2017
@ocombe @juleskremer
fix(compiler): allow numbers for ICU message cases in lexer (angular#…
0b6c969 
…18095)

Closes angular#18095
Fixes angular#17799
@angular-automatic-lock-bot

angular-automatic-lock-bot Bot commented Sep 12, 2019

Copy link
Copy Markdown

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 12, 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: no

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

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