Skip to content

[Translation] %parameters% are not replaced when file is first dumped as +intl-icuand then read #35328

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
ruudk opened this issue Jan 13, 2020 · 11 comments
Closed

[Translation] %parameters% are not replaced when file is first dumped as +intl-icuand then read #35328

ruudk opened this issue Jan 13, 2020 · 11 comments
Labels
Bug Status: Needs Review Translation

Comments

@ruudk
Copy link
Contributor

ruudk commented Jan 13, 2020

Symfony version(s) affected: 4.4

Description
We have a FileDumper that converts messages.en.yml to messages.en.php files. Since 4.4 this file became message+intl-icu.en.php with the same content.

Upon reading these +incl-icu.php files, the %placeholder% are no longer replaced with the correct value.

Spoke with @yceruto in #35264 (comment) and he suggested this is a different issue.

How to reproduce
See commits on https://github.com/ruudk/symfony-translation-bug-poc/commits/master

Possible Solution
The FileDumper should not just add +intl-icu to files.
@ruudk ruudk mentioned this issue Jan 13, 2020
Closed
@ruudk ruudk changed the title [Translation] Parameters are not replaced when file is first dumped as +intl-icuand then read [Translation] %parameters% are not replaced when file is first dumped as +intl-icuand then read Jan 13, 2020
@yceruto
Copy link
Member

yceruto commented Jan 14, 2020

Hi @ruudk, I'll check your reproducer at some point this week. Thank you for opening this issue.

@nicwortel
Copy link

nicwortel commented Jan 15, 2020 •
edited
Loading

@yceruto I came here through #35264 because we are experiencing a similar issue. I wonder if this issue is really a bug though. The ICU message format is a different format than Symfony's one. Where Symfony's format uses percentage signs for placeholders (%placeholder%), ICU uses brackets ({placeholder}). So I guess it's expected that %placeholder% doesn't work in a +intl-icu file.

Having said that, I do believe that #34797 that was released with Symfony 4.4.2 and 5.0.2 is a breaking change for use cases where translation files are created that don't exist yet. In our situation, we use https://github.com/php-translation/symfony-bundle to download translations from Loco and dump them to (gitignored) XLIFF files during our build process. Using symfony/translation v4.4.1, the files dumped are messages.en.xlf, messages.nl.xlf, etc. After updating to symfony/translation v4.4.2, the dumped file names are suddenly changed to messages+intl-icu.en.xlf, messages+intl-icu.nl.xlf, etc.

So the assumption in #34797 is that the translation files already exist and the decision between intl-icu or the "old" format is based entirely on the existance of those files. Whenever that is not the case (for example when using https://github.com/php-translation/symfony-bundle to download translations from an external source, adding a new language, etc.), this breaks with the previous behaviour in v4.4.1.

I'm not sure if this issue is the right place for what I just wrote, let me know if I should move it to #35264 or should create a new issue.

@yceruto yceruto mentioned this issue Jan 15, 2020
Merged
@yceruto
Copy link
Member

yceruto commented Jan 15, 2020

@nicwortel I can confirm what you said. Let's revert #34797 in the meantime. see #35351

@yceruto
Copy link
Member

yceruto commented Jan 15, 2020

The ICU message format is a different format than Symfony's one. Where Symfony's format uses percentage signs for placeholders (%placeholder%), ICU uses brackets ({placeholder}). So I guess it's expected that %placeholder% doesn't work in a +intl-icu file.

Let's close because we can do nothing here. Thank you @ruudk and @nicwortel.

@yceruto yceruto closed this as completed Jan 15, 2020
@XWB
Copy link
Contributor

XWB commented Jan 20, 2020

I'm having exactly the same issue as @nicwortel

nicolas-grekas added a commit that referenced this issue Jan 21, 2020
@nicolas-grekas
bug #35351 Revert #34797 "Fixed translations file dumper behavior" and …
039feed 
…fix #34713 (yceruto)

This PR was squashed before being merged into the 4.4 branch.

Discussion
----------

Revert #34797 "Fixed translations file dumper behavior" and fix #34713

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #35264
| License       | MIT
| Doc PR        | -

Revert #34797

See also #35328

It's very likely that the new way will be completely different from this one that is being reverted. That's why I'm reverting rather than fixing it.

Commits
-------

9ca8720 Fixed #34713 Move new messages to intl domain when possible
56e79fe Revert "Fixed translations file dumper behavior"
symfony-splitter pushed a commit to symfony/framework-bundle that referenced this issue Jan 21, 2020
@nicolas-grekas
bug #35351 Revert #34797 "Fixed translations file dumper behavior" an…
4278493 
…d fix #34713 (yceruto)

This PR was squashed before being merged into the 4.4 branch.

Discussion
----------

Revert #34797 "Fixed translations file dumper behavior" and fix #34713

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #35264
| License       | MIT
| Doc PR        | -

Revert symfony/symfony#34797

See also symfony/symfony#35328

It's very likely that the new way will be completely different from this one that is being reverted. That's why I'm reverting rather than fixing it.

Commits
-------

9ca872054b Fixed #34713 Move new messages to intl domain when possible
56e79fefa1 Revert "Fixed translations file dumper behavior"
@odolbeau odolbeau mentioned this issue Jan 28, 2020
Closed
@Guite Guite mentioned this issue Jan 28, 2020
Closed
@michaelKaefer
Copy link
Contributor

michaelKaefer commented Mar 10, 2020 •
edited
Loading

I tried and searched for more than 1.5 hours to find this.. It was really a pain not knowing why it did not work. At least an error would be cool. @ruudk Can you reopen this issue?

@ruudk
Copy link
Contributor Author

ruudk commented Mar 10, 2020

Why? This has been fixed with another release.

@michaelKaefer
Copy link
Contributor

@ruudk Ah, I didn't know. How could I have seen this on my own?

@ruudk
Copy link
Contributor Author

ruudk commented Mar 10, 2020

No worries. This PR reverted the broken behaviour: #35351

@yceruto yceruto mentioned this issue May 18, 2020
Closed
@mhitza mhitza mentioned this issue Jan 25, 2023
Closed
@xoniq
Copy link

xoniq commented Jul 7, 2023

As of 2023 this is still an issue. On a new project.

@Nek-
Copy link
Contributor

Nek- commented Apr 12, 2024 •
edited
Loading

Oh my. I'm having a headache for some minutes/hours. Removing +intl-icu suffix after the filename was fixing the issue. 😬

  • Symfony 7.0
  • New project setup with symfony cli
  • Translation files generated with the Symfony commands

However, this looks more like an issue of the documentation that is highlighting %var% instead of {var} as suggested here: https://symfony.com/doc/current/reference/formats/message_format.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Status: Needs Review Translation
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
8 participants
@ruudk @Nek- @XWB @nicwortel @yceruto @xoniq @carsonbot @michaelKaefer