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(ivy): i18n - start generated placeholder name at `PH` #32493

Closed

Conversation

@cexbrayat
Copy link
Contributor

commented Sep 5, 2019

On top of #32594

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.io application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

Currently the expressions used in a template string are automatically named PH_1, PH_2, etc. Whereas interpolations used in i18n templates generates placeholders automatically named INTERPOLATION, INTERPOLATION_1, etc.

What is the new behavior?

This commit aligns the behaviors by starting the generated placeholder names for expressions at PH, then PH_1, etc.

It also documents this behavior in the documentation of $localize as it was not mentioned before.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

This was discussed with @petebacondarwin

@cexbrayat cexbrayat requested a review from IgorMinar as a code owner Sep 5, 2019
@googlebot

This comment has been minimized.

Copy link

commented Sep 5, 2019

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added the cla: no label Sep 5, 2019
@cexbrayat cexbrayat force-pushed the cexbrayat:fix/i18n-placeholder-name branch from 6caa61a to 10495c7 Sep 5, 2019
@mhevery mhevery added the comp: core label Sep 5, 2019
@ngbot ngbot bot added this to the needsTriage milestone Sep 5, 2019
Copy link
Member

left a comment

Due to #32509 could you also rename the placeholders to be uppercase? I.E. PH_... rather than ph_....

@cexbrayat

This comment has been minimized.

Copy link
Contributor Author

commented Sep 9, 2019

@petebacondarwin Sure, will do 👍

@cexbrayat cexbrayat force-pushed the cexbrayat:fix/i18n-placeholder-name branch from 10495c7 to 99194d1 Sep 10, 2019
@cexbrayat cexbrayat requested a review from angular/dev-infra-framework as a code owner Sep 10, 2019
@cexbrayat cexbrayat changed the title fix(ivy): i18n - start generated placeholder name at `ph` fix(ivy): i18n - start generated placeholder name at `PH` Sep 10, 2019
@petebacondarwin petebacondarwin self-assigned this Sep 13, 2019
@petebacondarwin petebacondarwin force-pushed the cexbrayat:fix/i18n-placeholder-name branch from 99194d1 to c22cdff Sep 13, 2019
@cexbrayat cexbrayat requested review from angular/fw-compiler as code owners Sep 13, 2019
@cexbrayat cexbrayat force-pushed the cexbrayat:fix/i18n-placeholder-name branch from c22cdff to e27c6c7 Sep 17, 2019
@googlebot

This comment has been minimized.

Copy link

commented Sep 17, 2019

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: yes and removed cla: no labels Sep 17, 2019
@cexbrayat cexbrayat force-pushed the cexbrayat:fix/i18n-placeholder-name branch from e27c6c7 to 09baf5e Sep 17, 2019
Currently the expressions used in a template string are automatically named
`PH_1`, `PH_2`, etc. Whereas interpolations used in i18n templates generate
placeholders automatically named `INTERPOLATION`, `INTERPOLATION_1`, etc.

This commit aligns the behaviors by starting the generated placeholder
names for expressions at `PH`, then `PH_1`, etc.

It also documents this behavior in the documentation of `$localize` as
it was not mentioned before.
@cexbrayat cexbrayat force-pushed the cexbrayat:fix/i18n-placeholder-name branch from 09baf5e to f7fbc99 Sep 17, 2019
@petebacondarwin petebacondarwin removed request for angular/fw-compiler and IgorMinar Sep 17, 2019
@AndrewKushnir

This comment has been minimized.

Copy link
Contributor

commented Sep 17, 2019

arnehoek added a commit to arnehoek/angular that referenced this pull request Sep 26, 2019
)

Currently the expressions used in a template string are automatically named
`PH_1`, `PH_2`, etc. Whereas interpolations used in i18n templates generate
placeholders automatically named `INTERPOLATION`, `INTERPOLATION_1`, etc.

This commit aligns the behaviors by starting the generated placeholder
names for expressions at `PH`, then `PH_1`, etc.

It also documents this behavior in the documentation of `$localize` as
it was not mentioned before.

PR Close angular#32493
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.