Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Translation] Allow default parameters#53425

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

Jean-Beru
Copy link
Contributor

@Jean-BeruJean-Beru commentedJan 4, 2024
edited
Loading

QA
Branch?7.3
Bug fix?no
New feature?yes
Deprecations?no
IssuesFix#48182
LicenseMIT

This PRs adds aGlobalsTranslator which decorates theTranslator when global parameters are given.

Usage:

# config/framework.yamlframework:# ...translator:# ...globals:'{version}':'1.2.3''%%application_name%%':'My app'# "%" must be escaped if it is not a DI parameter
# messages.en.yamlapp: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:
image

welcoMattic, 94noni, David-Moisan, norkunas, albertmueller, alamirault, and Jibbarth reacted with thumbs up emoji
@carsonbotcarsonbot added this to the7.1 milestoneJan 4, 2024
@Jean-BeruJean-Beruforce-pushed thefeat/add-translation-global-variables branch 3 times, most recently from5f6aaca to62cb6a3CompareJanuary 4, 2024 16:10
@Jean-Beru
Copy link
ContributorAuthor

Friendly ping@94noni 😉

94noni reacted with rocket emoji

Copy link
Contributor

@94noni94noni left a comment
edited
Loading

Choose a reason for hiding this comment

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

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"

Jean-Beru and welcoMattic reacted with thumbs up emoji
Copy link
Contributor

@94noni94noni left a comment

Choose a reason for hiding this comment

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

lgtm, lets wait other reviews :)

@Jean-BeruJean-Beruforce-pushed thefeat/add-translation-global-variables branch 2 times, most recently fromaab1d74 toc449989CompareJanuary 5, 2024 21:23
@nicolas-grekasnicolas-grekas changed the title[Translation] Allow global parameters[Translation] Allow default parametersJan 8, 2024
@Jean-Beru
Copy link
ContributorAuthor

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

@luxferre
Copy link

Can a global parameter value reference translation key?

@Jean-Beru
Copy link
ContributorAuthor

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.

luxferre reacted with thumbs up emoji

@Jean-BeruJean-Beruforce-pushed thefeat/add-translation-global-variables branch 2 times, most recently from575ce2b to9410fa4CompareApril 16, 2024 15:02
@carsonbotcarsonbot changed the titleAllow default parameters[Translation] Allow default parametersJan 22, 2025
Copy link
Member

@welcoMatticwelcoMattic left a comment
edited
Loading

Choose a reason for hiding this comment

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

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

@Jean-BeruJean-Beruforce-pushed thefeat/add-translation-global-variables branch 2 times, most recently from64f38ad to7ca354bCompareJanuary 28, 2025 10:55
@Jean-Beru
Copy link
ContributorAuthor

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

Thanks@welcoMattic !

PR is fixed, squashed and rebased.

@Jean-BeruJean-Beruforce-pushed thefeat/add-translation-global-variables branch from7ca354b to9d201f3CompareJanuary 28, 2025 11:41
@Jean-BeruJean-Beruforce-pushed thefeat/add-translation-global-variables branch from1013418 toa59e38aCompareFebruary 5, 2025 10:01
Copy link
Member

@nicolas-grekasnicolas-grekas left a comment

Choose a reason for hiding this comment

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

Here is my review as a patch.
diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.phpindex 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.twigindex 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></td>                     <td>{{ id }}</td>-                    <td>{{ value }}</td>+                    <td>{{ profiler_dump(value) }}</td>                 </tr>             {% endfor %}-            {% for locale, values in collector.globalTranslatedParameters %}-                {% for id, value in values %}-                    <tr>-                        <td>{{ locale }}</td>-                        <td>{{ id }}</td>-                        <td>{{ value }}</td>-                    </tr>-                {% endfor %}-            {% endfor %}             </tbody>         </table>     {% endif %}diff --git a/src/Symfony/Component/Translation/CHANGELOG.md b/src/Symfony/Component/Translation/CHANGELOG.mdindex 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.phpindex 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.phpindex 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-BeruJean-Beruforce-pushed thefeat/add-translation-global-variables branch froma59e38a todaea7d8CompareMarch 5, 2025 17:09
@Jean-Beru
Copy link
ContributorAuthor

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 useInstalledVersions::satisfies to check component version, use a dedicated interface or just check the method existence?

@stof
Copy link
Member

stof commentedMar 5, 2025

The cleanest solution for FrameworkBundle is to bump the min supported version ofsymfony/translation (note that theconflict rule is the most important one here, asrequire-dev only impacts our CI)

@Jean-Beru
Copy link
ContributorAuthor

The cleanest solution for FrameworkBundle is to bump the min supported version ofsymfony/translation (note that theconflict rule is the most important one here, asrequire-dev only impacts our CI)

Thanks for the tip!

@fabpotfabpotforce-pushed thefeat/add-translation-global-variables branch from121ce11 to652c658CompareMarch 10, 2025 22:51
@fabpot
Copy link
Member

Thank you@Jean-Beru.

@fabpotfabpot merged commit36092eb intosymfony:7.3Mar 10, 2025
7 of 8 checks passed
javiereguiluz added a commit to symfony/symfony-docs that referenced this pull requestApr 8, 2025
…ation (94noni)This PR was merged into the 7.3 branch.Discussion----------[Translation] document global variables feature configurationdocumentsymfony/symfony#53425close#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 PRCommits-------97de6eb [Translator] document global variables feature configuration
@Jean-BeruJean-Beru deleted the feat/add-translation-global-variables branchApril 24, 2025 07:56
@fabpotfabpot mentioned this pull requestMay 2, 2025
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas left review comments

@stofstofstof requested changes

@welcoMatticwelcoMatticwelcoMattic left review comments

@xabbuhxabbuhxabbuh left review comments

@fabpotfabpotfabpot approved these changes

@94noni94noni94noni approved these changes

@maxbeckersmaxbeckersmaxbeckers approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
7.3
Development

Successfully merging this pull request may close these issues.

[Translation] Default parameters
11 participants
@Jean-Beru@luxferre@94noni@stof@fabpot@nicolas-grekas@welcoMattic@xabbuh@maxbeckers@GromNaN@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp