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

i18n: support reusing the same placeholder for ICU #16159

Merged
merged 2 commits into from
Aug 23, 2024

Conversation

connorjclark
Copy link
Collaborator

@connorjclark connorjclark commented Aug 22, 2024

It can be useful to reuse the same ICU variable names in a i18n string. For example:

/**
   * @description Text block that compares a local metric value to real user experiences.
   * @example {LCP} PH1
   * @example {500 ms} PH2
   * @example {400 ms} PH3
   * @example {40%} PH4
   */
  needsImprovementPoorDetailedCompare:
      'Your local {PH1} {PH2} needs improvement and is rated the same as {PH4} of real-user {PH1} experiences. However, the field data 75th percentile {PH1} {PH3} is poor.',

source

The above currently ends up with this in our ctc file: "message": "Your local $ICU_0$ $ICU_1$ needs improvement and is rated the same as $ICU_3$ of real-user {PH1} experiences. However, the field data 75th percentile {PH1} $ICU_2$ is good." (this is wrong).

We already support replacing multiple instances of a ICU variable: https://github.com/GoogleChrome/lighthouse/blob/c79628af9bdaa537a2abd1b34da922e28b81bd98/core/test/scripts/i18n/bake-ctc-to-lhl-test.js#L43C50-L60

But our collect strings step does not currently ever produce a ICU message string that reuses the same variable. For direct ICU (like the above example), it would just leave the {PH1} literal as-is for subsequent instances. For the custom format one, it would create a new variable. This PR fixes both of those cases.

internal links:

https://localizer.google.com/query/13584722

@connorjclark connorjclark requested a review from a team as a code owner August 22, 2024 20:15
@connorjclark connorjclark requested review from adamraine and removed request for a team August 22, 2024 20:15
copybara-service bot pushed a commit to ChromeDevTools/devtools-frontend that referenced this pull request Aug 23, 2024
This patches in a change from upstream:
GoogleChrome/lighthouse#16159

See the above PR for details.

Bug: None
Change-Id: Id73070ec4c2b4ed5ff227d0a5d486416b67eb5a5
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/5807627
Commit-Queue: Simon Zünd <szuend@chromium.org>
Reviewed-by: Adam Raine <asraine@chromium.org>
Reviewed-by: Simon Zünd <szuend@chromium.org>
@connorjclark connorjclark merged commit 92de531 into main Aug 23, 2024
27 checks passed
@connorjclark connorjclark deleted the fix-collect-strings-repeats branch August 23, 2024 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants