[Translation] Allow default parameters #53425

Jean-Beru · 2024-01-04T16:06:07Z

Q	A
Branch?	7.3
Bug fix?	no
New feature?	yes
Deprecations?	no
Issues	Fix #48182
License	MIT

This PRs adds a GlobalsTranslator which decorates the Translator when global parameters are given.

Usage:

# config/framework.yaml
framework:
    # ...
    translator:
        # ...
        globals:
            '{version}': '1.2.3'
            '%%application_name%%': 'My app' # "%" must be escaped if it is not a DI parameter

# messages.en.yaml
app:
  version: 'Application version: {version}'
  name: 'Application name: %application_name%'

# twig template
{{ 'app.version'|trans }} # Displays "Application version: 1.2.3"
{{ 'app.version'|trans({ '{version}': '2.3.4' }) }} # Displays "Application version: 2.3.4"
{{ 'app.name'|trans }} # Displays "Application name: My app"
{{ 'app.name'|trans({ '{application_name}': 'My new app' }) }} # Displays "Application name: My new app"

Profiler view:

Jean-Beru · 2024-01-04T16:13:04Z

Friendly ping @94noni 😉

94noni

nice :)
just a small comment regarding if we can add this feature on the default translator or not
obviously in our app we defined a decorator, but here it is core/new feature so I dont see why a new class is needed (or did I miss something?)

(and your example in PR desc part "# twig template" seems wrong for default using global)

wrong

{{ 'app.version'|trans }} # Displays "1.2.3"
{{ 'app.version'|trans({ '{version}': '2.3.4' }) }} # Displays "2.3.4"

correct

{{ 'app.version'|trans }} # Displays "Application version: 1.2.3"
{{ 'app.version'|trans({ '{version}': '2.3.4' }) }} # Displays "Application version: 2.3.4"

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php

94noni

lgtm, lets wait other reviews :)

src/Symfony/Component/Translation/GlobalsTranslator.php

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

src/Symfony/Component/Translation/DefaultParametersTranslator.php

src/Symfony/Component/Translation/Tests/DefaultParametersTranslatorTest.php

Jean-Beru · 2024-01-12T22:04:35Z

Could it be considered as a BC break since we can use a public method from the Translator service (from implemented interfaces or not) but not present in this decorator?

luxferre · 2024-03-02T11:22:16Z

Can a global parameter value reference translation key?

Jean-Beru · 2024-03-02T19:53:12Z

Can a global parameter value reference translation key?

For now, in this PR, a global parameter can be used with a fixed value, a parameter or an environment variable.

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd

welcoMattic

Minor comments, otherwise it's ✅ for me!
@Jean-Beru could you squash your commits please?

src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/translation.html.twig

src/Symfony/Component/Translation/Translator.php

Jean-Beru · 2025-01-28T10:58:34Z

Minor comments, otherwise it's ✅ for me! @Jean-Beru could you squash your commits please?

Thanks @welcoMattic !

PR is fixed, squashed and rebased.

src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php

src/Symfony/Component/Translation/Translator.php

src/Symfony/Component/Translation/Tests/TranslatorTest.php

nicolas-grekas

Here is my review as a patch.

diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
index 44e9816e18..eba4ee4534 100644
--- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
+++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
@@ -180,6 +180,7 @@ use Symfony\Component\Translation\Command\XliffLintCommand as BaseXliffLintComma
 use Symfony\Component\Translation\Extractor\PhpAstExtractor;
 use Symfony\Component\Translation\LocaleSwitcher;
 use Symfony\Component\Translation\PseudoLocalizationTranslator;
+use Symfony\Component\Translation\TranslatableMessage;
 use Symfony\Component\Translation\Translator;
 use Symfony\Component\TypeInfo\Type;
 use Symfony\Component\TypeInfo\TypeResolver\PhpDocAwareReflectionTypeResolver;
@@ -1612,11 +1613,7 @@ class FrameworkExtension extends Extension
         }
 
         foreach ($config['globals'] as $name => $global) {
-            if (isset($global['value'])) {
-                $translator->addMethodCall('addGlobalParameter', [$name, $global['value']]);
-            } else {
-                $translator->addMethodCall('addGlobalTranslatableParameter', [$name, $global['message'], $global['parameters'] ?? [], $global['domain'] ?? null]);
-            }
+            $translator->addMethodCall('addGlobalParameter', [$name, $global['value'] ?? new Definition(TranslatableMessage::class, [$global['message'], $global['parameters'] ?? [], $global['domain'] ?? null])]);
         }
 
         if ($config['pseudo_localization']['enabled']) {
diff --git a/src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/translation.html.twig b/src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/translation.html.twig
index 5babc0d893..18319e4993 100644
--- a/src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/translation.html.twig
+++ b/src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/translation.html.twig
@@ -172,18 +172,9 @@
                 <tr>
                     <td class="font-normal text-small nowrap"></td>
                     <td class="font-normal text-small nowrap">{{ id }}</td>
-                    <td class="font-normal text-small nowrap">{{ value }}</td>
+                    <td class="font-normal text-small nowrap">{{ profiler_dump(value) }}</td>
                 </tr>
             {% endfor %}
-            {% for locale, values in collector.globalTranslatedParameters %}
-                {% for id, value in values %}
-                    <tr>
-                        <td class="font-normal text-small nowrap">{{ locale }}</td>
-                        <td class="font-normal text-small nowrap">{{ id }}</td>
-                        <td class="font-normal text-small nowrap">{{ value }}</td>
-                    </tr>
-                {% endfor %}
-            {% endfor %}
             </tbody>
         </table>
     {% endif %}
diff --git a/src/Symfony/Component/Translation/CHANGELOG.md b/src/Symfony/Component/Translation/CHANGELOG.md
index 64502a89fd..365c5cf1cd 100644
--- a/src/Symfony/Component/Translation/CHANGELOG.md
+++ b/src/Symfony/Component/Translation/CHANGELOG.md
@@ -4,7 +4,7 @@ CHANGELOG
 7.3
 ---
 
- * Add `addGlobalParameter` and `addGlobalTranslatableParameter` to allow defining global translation parameters
+ * Add `Translator::addGlobalParameter()` to allow defining global translation parameters
 
 7.2
 ---
diff --git a/src/Symfony/Component/Translation/DataCollector/TranslationDataCollector.php b/src/Symfony/Component/Translation/DataCollector/TranslationDataCollector.php
index bb7dc6b399..e2e597a705 100644
--- a/src/Symfony/Component/Translation/DataCollector/TranslationDataCollector.php
+++ b/src/Symfony/Component/Translation/DataCollector/TranslationDataCollector.php
@@ -45,7 +45,6 @@ class TranslationDataCollector extends DataCollector implements LateDataCollecto
         $this->data['locale'] = $this->translator->getLocale();
         $this->data['fallback_locales'] = $this->translator->getFallbackLocales();
         $this->data['global_parameters'] = $this->translator->getGlobalParameters();
-        $this->data['global_translated_parameters'] = $this->translator->getGlobalTranslatedParameters();
     }
 
     public function reset(): void
@@ -94,14 +93,6 @@ class TranslationDataCollector extends DataCollector implements LateDataCollecto
         return $this->data['global_parameters'] ?? [];
     }
 
-    /**
-     * @internal
-     */
-    public function getGlobalTranslatedParameters(): Data|array
-    {
-        return $this->data['global_translated_parameters'] ?? [];
-    }
-
     public function getName(): string
     {
         return 'translation';
diff --git a/src/Symfony/Component/Translation/Translator.php b/src/Symfony/Component/Translation/Translator.php
index 6994cb9b6f..d57ec9e2c9 100644
--- a/src/Symfony/Component/Translation/Translator.php
+++ b/src/Symfony/Component/Translation/Translator.php
@@ -61,15 +61,10 @@ class Translator implements TranslatorInterface, TranslatorBagInterface, LocaleA
     private bool $hasIntlFormatter;
 
     /**
-     * @var array<string, string|int|float>
+     * @var array<string, string|int|float|TranslatableInterface>
      */
     private array $globalParameters = [];
 
-    /**
-     * @var array<string, TranslatableInterface>
-     */
-    private array $globalTranslatableParameters = [];
-
     /**
      * @var array<string, string|int|float>
      */
@@ -170,16 +165,11 @@ class Translator implements TranslatorInterface, TranslatorBagInterface, LocaleA
         return $this->fallbackLocales;
     }
 
-    public function addGlobalParameter(string $id, string|int|float $value): void
+    public function addGlobalParameter(string $id, string|int|float|TranslatableInterface $value): void
     {
         $this->globalParameters[$id] = $value;
     }
 
-    public function addGlobalTranslatableParameter(string $id, string $message, array $parameters = [], ?string $domain = null): void
-    {
-        $this->globalTranslatableParameters[$id] = new TranslatableMessage($message, $parameters, $domain);
-    }
-
     /**
      * @internal
      */
@@ -188,14 +178,6 @@ class Translator implements TranslatorInterface, TranslatorBagInterface, LocaleA
         return $this->globalParameters;
     }
 
-    /**
-     * @internal
-     */
-    public function getGlobalTranslatedParameters(): array
-    {
-        return $this->globalTranslatedParameters;
-    }
-
     public function trans(?string $id, array $parameters = [], ?string $domain = null, ?string $locale = null): string
     {
         if (null === $id || '' === $id) {
@@ -215,17 +197,24 @@ class Translator implements TranslatorInterface, TranslatorBagInterface, LocaleA
             }
         }
 
-        if ($this->globalTranslatableParameters && !isset($this->globalTranslatedParameters[$locale])) {
-            $this->globalTranslatedParameters[$locale] = []; // Avoid infinite loops
-            $this->globalTranslatedParameters[$locale] = array_map(fn ($parameter) => $parameter->trans($this, $locale), $this->globalTranslatableParameters);
+        foreach ($parameters as $key => $value) {
+            if ($value instanceof TranslatableInterface) {
+                $parameters[$key] = $value->trans($this, $locale);
+            }
         }
-        if ($this->globalParameters) {
-            $parameters += $this->globalParameters;
+
+        if (null === $globalParameters =& $this->globalTranslatedParameters[$locale]) {
+            $globalParameters = $this->globalParameters;
+            foreach ($globalParameters as $key => $value) {
+                if ($value instanceof TranslatableInterface) {
+                    $globalParameters[$key] = $value->trans($this, $locale);
+                }
+            }
         }
-        if (isset($this->globalTranslatedParameters[$locale])) {
-            $parameters += $this->globalTranslatedParameters[$locale];
+
+        if ($globalParameters) {
+            $parameters += $globalParameters;
         }
-        $parameters = array_map(fn ($parameter) => $parameter instanceof TranslatableInterface ? $parameter->trans($this, $locale) : $parameter, $parameters);
 
         $len = \strlen(MessageCatalogue::INTL_DOMAIN_SUFFIX);
         if ($this->hasIntlFormatter

The TL;DR is that I don't think that we need nor should have two methods for this.
Also, the PR is missing some cross-components checks I think (fwb / profiler are using feats that might not be available depending on the version of translator that's installed.)

Jean-Beru · 2025-03-05T17:21:28Z

Here is my review as a patch.
The TL;DR is that I don't think that we need nor should have two methods for this. Also, the PR is missing some cross-components checks I think (fwb / profiler are using feats that might not be available depending on the version of translator that's installed.)

Thanks for the review ❤️

the PR is missing some cross-components checks

I updated the profiler part view but what about the bundle? Should we use InstalledVersions::satisfies to check component version, use a dedicated interface or just check the method existence?

stof · 2025-03-05T17:25:02Z

The cleanest solution for FrameworkBundle is to bump the min supported version of symfony/translation (note that the conflict rule is the most important one here, as require-dev only impacts our CI)

Jean-Beru · 2025-03-06T06:20:45Z

The cleanest solution for FrameworkBundle is to bump the min supported version of symfony/translation (note that the conflict rule is the most important one here, as require-dev only impacts our CI)

Thanks for the tip!

src/Symfony/Component/Translation/Tests/TranslatorCacheTest.php

src/Symfony/Component/Translation/Translator.php

src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/translation.html.twig

src/Symfony/Component/Translation/Translator.php

src/Symfony/Component/Translation/DataCollectorTranslator.php

src/Symfony/Component/Translation/Translator.php

fabpot · 2025-03-10T22:51:56Z

Thank you @Jean-Beru.

…ation (94noni) This PR was merged into the 7.3 branch. Discussion ---------- [Translation] document global variables feature configuration document symfony/symfony#53425 close #20754 > [!NOTE] > I am very unsure of the php config via object config (not inline php array) > all example have been taken from the merged PR Commits ------- 97de6eb [Translator] document global variables feature configuration

Jean-Beru requested a review from welcoMattic as a code owner January 4, 2024 16:06

carsonbot added Status: Needs Review Feature Translation labels Jan 4, 2024

carsonbot added this to the 7.1 milestone Jan 4, 2024

Jean-Beru force-pushed the feat/add-translation-global-variables branch 3 times, most recently from 5f6aaca to 62cb6a3 Compare January 4, 2024 16:10

94noni reviewed Jan 4, 2024

View reviewed changes

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php Outdated Show resolved Hide resolved

94noni reviewed Jan 5, 2024

View reviewed changes

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php Outdated Show resolved Hide resolved

94noni approved these changes Jan 5, 2024

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Jan 5, 2024

nicolas-grekas reviewed Jan 5, 2024

View reviewed changes

src/Symfony/Component/Translation/GlobalsTranslator.php Outdated Show resolved Hide resolved

Jean-Beru force-pushed the feat/add-translation-global-variables branch 2 times, most recently from aab1d74 to c449989 Compare January 5, 2024 21:23

maxbeckers approved these changes Jan 8, 2024

View reviewed changes

nicolas-grekas changed the title ~~[Translation] Allow global parameters~~ [Translation] Allow default parameters Jan 8, 2024

nicolas-grekas reviewed Jan 8, 2024

View reviewed changes

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php Outdated Show resolved Hide resolved

nicolas-grekas reviewed Jan 8, 2024

View reviewed changes

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php Outdated Show resolved Hide resolved

nicolas-grekas reviewed Jan 8, 2024

View reviewed changes

src/Symfony/Component/Translation/DefaultParametersTranslator.php Outdated Show resolved Hide resolved

welcoMattic reviewed Jan 9, 2024

View reviewed changes

src/Symfony/Component/Translation/Tests/DefaultParametersTranslatorTest.php Outdated Show resolved Hide resolved

Jean-Beru force-pushed the feat/add-translation-global-variables branch 2 times, most recently from 575ce2b to 9410fa4 Compare April 16, 2024 15:02

stof requested changes Apr 16, 2024

View reviewed changes

carsonbot removed the Status: Reviewed label Apr 16, 2024

GromNaN removed the Translation label Jan 22, 2025

carsonbot changed the title ~~[Translation] Allow default parameters~~ Allow default parameters Jan 22, 2025

GromNaN added the Translation label Jan 22, 2025

carsonbot changed the title ~~Allow default parameters~~ [Translation] Allow default parameters Jan 22, 2025

welcoMattic reviewed Jan 27, 2025

View reviewed changes

Jean-Beru force-pushed the feat/add-translation-global-variables branch 2 times, most recently from 64f38ad to 7ca354b Compare January 28, 2025 10:55

Jean-Beru force-pushed the feat/add-translation-global-variables branch from 7ca354b to 9d201f3 Compare January 28, 2025 11:41

stof reviewed Jan 28, 2025

View reviewed changes

Jean-Beru force-pushed the feat/add-translation-global-variables branch from 1013418 to a59e38a Compare February 5, 2025 10:01

nicolas-grekas reviewed Feb 12, 2025

View reviewed changes

Jean-Beru force-pushed the feat/add-translation-global-variables branch from a59e38a to daea7d8 Compare March 5, 2025 17:09

xabbuh reviewed Mar 6, 2025

View reviewed changes

src/Symfony/Component/Translation/Tests/TranslatorCacheTest.php Outdated Show resolved Hide resolved

xabbuh reviewed Mar 6, 2025

View reviewed changes

src/Symfony/Component/Translation/Translator.php Show resolved Hide resolved

xabbuh reviewed Mar 6, 2025

View reviewed changes

src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/translation.html.twig Outdated Show resolved Hide resolved

xabbuh reviewed Mar 6, 2025

View reviewed changes

src/Symfony/Component/Translation/Translator.php Show resolved Hide resolved

nicolas-grekas reviewed Mar 6, 2025

View reviewed changes

src/Symfony/Component/Translation/DataCollectorTranslator.php Show resolved Hide resolved

nicolas-grekas reviewed Mar 6, 2025

View reviewed changes

src/Symfony/Component/Translation/Translator.php Outdated Show resolved Hide resolved

fabpot approved these changes Mar 8, 2025

View reviewed changes

[Translation] Allow default parameters

652c658

fabpot force-pushed the feat/add-translation-global-variables branch from 121ce11 to 652c658 Compare March 10, 2025 22:51

fabpot merged commit 36092eb into symfony:7.3 Mar 10, 2025
7 of 8 checks passed

fabpot mentioned this pull request Mar 10, 2025

[Translation] Allow default parameters symfony/symfony-docs#20754

Closed

94noni mentioned this pull request Apr 7, 2025

[Translation] document global variables feature configuration symfony/symfony-docs#20862

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Translation] Allow default parameters #53425

[Translation] Allow default parameters #53425

Jean-Beru commented Jan 4, 2024 •

edited

Loading

Jean-Beru commented Jan 4, 2024

94noni left a comment •

edited

Loading

94noni left a comment

Jean-Beru commented Jan 12, 2024

luxferre commented Mar 2, 2024

Jean-Beru commented Mar 2, 2024

welcoMattic left a comment •

edited

Loading

Jean-Beru commented Jan 28, 2025

nicolas-grekas left a comment

Jean-Beru commented Mar 5, 2025

stof commented Mar 5, 2025

Jean-Beru commented Mar 6, 2025

fabpot commented Mar 10, 2025

[Translation] Allow default parameters #53425

[Translation] Allow default parameters #53425

Conversation

Jean-Beru commented Jan 4, 2024 • edited Loading

Jean-Beru commented Jan 4, 2024

94noni left a comment • edited Loading

Choose a reason for hiding this comment

wrong

correct

94noni left a comment

Choose a reason for hiding this comment

Jean-Beru commented Jan 12, 2024

luxferre commented Mar 2, 2024

Jean-Beru commented Mar 2, 2024

welcoMattic left a comment • edited Loading

Choose a reason for hiding this comment

Jean-Beru commented Jan 28, 2025

nicolas-grekas left a comment

Choose a reason for hiding this comment

Jean-Beru commented Mar 5, 2025

stof commented Mar 5, 2025

Jean-Beru commented Mar 6, 2025

fabpot commented Mar 10, 2025

Jean-Beru commented Jan 4, 2024 •

edited

Loading

94noni left a comment •

edited

Loading

welcoMattic left a comment •

edited

Loading