[FrameworkBundle][Translation] Extract translation IDs from all of src #40229

natewiebe13 · 2021-02-17T21:25:28Z

Q	A
Bug fix?	no
New feature?	yes
Deprecations?	no
Tickets	Related to #39126 possibly #35082 as well
License	MIT
Doc PR	TBD

This PR allows extracting (bin/console translation:update) and debugging (bin/console debug:translation) translations using Translatable messages.

Currently we only check classes that include the TranslatorInterface, but this no longer covers all instances of this.

Current considerations:

Should this be treated as a bug fix or a new feature? On one hand, text extraction would no longer work if moving to TranslatableMessages (like we're doing) on the other, it wasn't intended to search all PHP files. As a bug fix would get this into Symfony faster, as a feature would mean having to wait until 5.3 is released.
Is there a better way to get the source directory that doesn't involve hardcoding /src?
Adding this in on a project with ~12k LOC in /src takes these operations from about 3s to 5s, but I feel like this is reasonable considering this command isn't likely called constantly. I can provide more accurate stats as requested.

yceruto · 2021-02-20T22:37:38Z

Should this be treated as a bug fix or a new feature?

I think this should be treated as a new feature, yes.

Is there a better way to get the source directory that doesn't involve hardcoding /src?

What about adding a new option in related commands to extract also from one specific directory or file?

For example, with such option we could add the src/ path to the current $viewPaths list:

bin/console translation:update en --extra-path=src

The option could accept an array of paths too. What do you think?

Pixelshaped · 2021-02-21T11:37:18Z

Have I missed something or TranslatableMessage are now extracted?
Went to see the PhpExtractor and found they were!
That's great: in our project we built a whole workaround to extract all keys that were not in classes where the TranslationInterface could be injected (say, enums, DTOs, etc). Using grep.
I'm absolutely seconding the fact that we should be able to provide all paths necessary to the extraction command!

natewiebe13 · 2021-02-21T17:29:43Z

The option could accept an array of paths too. What do you think?

Thinking about DX I think having src included by default is consistent with having things "just work". I know our team would always want to include src. I've seen other places hardcode this path, but just wanted to mention it in case there's actually a way to avoid that.

Regarding adding that as an option does solve my concern around making this work for teams that use non-standard directories, but I think adding that should be in a separate PR as they are different features. If it turns out that including src by default isn't wanted, I think adding the path as an option is a good fallback.

natewiebe13 · 2021-02-21T17:34:38Z

Have I missed something or TranslatableMessage are now extracted?
Went to see the PhpExtractor and found they were!

Support was added to PhpExtractor along with the feature. The missing part is that the commands that use them only include classes that inject TranslatorInterface. I haven't had a chance to test this out, but potentially as another workaround, you could determine which tag is being detected by the commands and add it through services.yaml?

natewiebe13 · 2021-02-22T22:09:18Z

... potentially as another workaround, you could determine which tag is being detected by the commands and add it through services.yaml?

I had a chance to dig into this and it doesn't appear that this is a potential workaround.

fabpot · 2021-02-25T07:40:52Z

src/Symfony/Bundle/FrameworkBundle/Command/TranslationDebugCommand.php

@@ -140,6 +140,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
            $transPaths[] = $this->defaultTransPath;
        }
        $viewsPaths = $this->viewsPaths;
+        $viewsPaths[] = $kernel->getProjectDir().'/src';


I would create a new array as src/ does not contain views, something like $codePaths?

@fabpot just to provide context, currently if you inject TranslatorInterface, those classes get added to $viewPaths even though it doesn't contain any templates.

Here's an example dump:

^ array:16 [ 0 => "public" 1 => "/var/www/html/vendor/symfony/twig-bridge/Resources/views/Email" 2 => "/var/www/html/vendor/symfony/twig-bridge/Resources/views/Form" 3 => "/var/www/html/vendor/symfony/framework-bundle/Command/TranslationDebugCommand.php" 4 => "/var/www/html/vendor/symfony/form/Extension/Core/Type/ColorType.php" 5 => "/var/www/html/vendor/symfony/form/Extension/Core/Type/TransformationFailureExtension.php" 6 => "/var/www/html/vendor/symfony/form/Extension/Validator/Type/FormTypeValidatorExtension.php" 7 => "/var/www/html/vendor/symfony/form/Extension/Validator/Type/UploadValidatorExtension.php" 8 => "/var/www/html/vendor/symfony/form/Extension/Csrf/Type/FormTypeCsrfExtension.php" 9 => "/var/www/html/vendor/symfony/validator/ValidatorBuilder.php" 10 => "/var/www/html/vendor/symfony/framework-bundle/CacheWarmer/TranslationsCacheWarmer.php" 11 => "/var/www/html/vendor/symfony/translation/DataCollector/TranslationDataCollector.php" 12 => "/var/www/html/src/Controller/SecurityController.php" 13 => "/var/www/html/vendor/symfony/form/Extension/Core/Type/FileType.php" 14 => "/var/www/html/vendor/symfony/twig-bridge/Extension/TranslationExtension.php" 15 => "/var/www/html/vendor/symfony/twig-bridge/Extension/FormExtension.php" ]

Can you confirm if we should keep it this way or if we should add $codePaths and then if we do add $codePaths would we just use it for adding the src folder or would we move some of these files into $codePaths instead?

I would say that the issue is the name $viewPaths. When the variable was first introduced, the PhpExtractor was targetting only PHP templates, not other usages of the translator

Vars have been renamed to codePaths

Pixelshaped · 2021-02-25T16:08:45Z

Have I missed something or TranslatableMessage are now extracted?
Went to see the PhpExtractor and found they were!

Support was added to PhpExtractor along with the feature. The missing part is that the commands that use them only include classes that inject TranslatorInterface. I haven't had a chance to test this out, but potentially as another workaround, you could determine which tag is being detected by the commands and add it through services.yaml?

As long as TranslatableMessage aren't picked up in any class they might be declared in, the extract command will be a bit underwhelming. But I'm not sure that's the case: I went to see PhpExtractor and it seems all php files are parsed and every sequence in the $sequences array is looked for. Sure for ->trans( to work you have to inject the TranslatorInterface somewhere, but new TranslatableMessage( does not need this requirement and seems to be picked up by default. This is cool!

fabpot · 2021-03-05T12:18:04Z

Thank you @natewiebe13.

…ebe13) This PR was squashed before being merged into the 5.3-dev branch. Discussion ---------- Update docs relating to translation extraction Update translation extraction docs based on changes from symfony/symfony#40229 Fixes #15061 Commits ------- 77089fa Update docs relating to translation extraction

This is a hack to scan froms for translation while waiting for symfony/symfony#40229 to be released. See symfony/symfony#35082 (comment)

carsonbot added Status: Needs Review Feature Translation labels Feb 17, 2021

carsonbot changed the title ~~[Translation] Extract translation IDs from all of src~~ Extract translation IDs from all of src Feb 17, 2021

derrabus added the FrameworkBundle label Feb 17, 2021

carsonbot changed the title ~~Extract translation IDs from all of src~~ [FrameworkBundle][Translation] Extract translation IDs from all of src Feb 17, 2021

This was referenced Feb 20, 2021

TranslatableMessage not extracted #39126

Closed

Form Class translation extraction #35082

Closed

nicolas-grekas added this to the 5.x milestone Feb 22, 2021

fabpot requested changes Feb 25, 2021

View reviewed changes

carsonbot added Status: Needs Work and removed Status: Needs Review labels Feb 25, 2021

natewiebe13 force-pushed the translation-extration-src branch from 860cc41 to f693ab3 Compare February 26, 2021 18:58

natewiebe13 requested a review from fabpot March 4, 2021 20:13

carsonbot added Status: Needs Review and removed Status: Needs Work labels Mar 4, 2021

fabpot approved these changes Mar 4, 2021

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Mar 4, 2021

[FrameworkBundle][Translation] Extract translation IDs from all of src

b02ae50

fabpot force-pushed the translation-extration-src branch from f693ab3 to b02ae50 Compare March 5, 2021 12:17

fabpot mentioned this pull request Mar 5, 2021

[FrameworkBundle][Translation] Extract translation IDs from all of src symfony/symfony-docs#15061

Closed

fabpot merged commit f8e2cff into symfony:5.x Mar 5, 2021

natewiebe13 deleted the translation-extration-src branch March 5, 2021 15:15

natewiebe13 mentioned this pull request Mar 5, 2021

Update docs relating to translation extraction symfony/symfony-docs#15063

Merged

n-peugnet added a commit to n-peugnet/antilope that referenced this pull request Apr 8, 2021

fix: add Form dir to twig path

61569e8

This is a hack to scan froms for translation while waiting for symfony/symfony#40229 to be released. See symfony/symfony#35082 (comment)

n-peugnet added a commit to n-peugnet/antilope that referenced this pull request Apr 9, 2021

fix: add Form dir to twig path

35c3954

This is a hack to scan froms for translation while waiting for symfony/symfony#40229 to be released. See symfony/symfony#35082 (comment)

fabpot mentioned this pull request Apr 18, 2021

Release v5.3.0-BETA1 #40853

Merged

E-M-P-I-R-E mentioned this pull request Mar 4, 2022

Translator commands scan source files multiple times [Translation] #45645

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FrameworkBundle][Translation] Extract translation IDs from all of src #40229

[FrameworkBundle][Translation] Extract translation IDs from all of src #40229

natewiebe13 commented Feb 17, 2021 •

edited

Loading

yceruto commented Feb 20, 2021

Pixelshaped commented Feb 21, 2021

natewiebe13 commented Feb 21, 2021 •

edited

Loading

natewiebe13 commented Feb 21, 2021

natewiebe13 commented Feb 22, 2021

fabpot Feb 25, 2021

natewiebe13 Feb 25, 2021

stof Feb 25, 2021

natewiebe13 Feb 26, 2021

Pixelshaped commented Feb 25, 2021

fabpot commented Mar 5, 2021

[FrameworkBundle][Translation] Extract translation IDs from all of src #40229

[FrameworkBundle][Translation] Extract translation IDs from all of src #40229

Conversation

natewiebe13 commented Feb 17, 2021 • edited Loading

yceruto commented Feb 20, 2021

Pixelshaped commented Feb 21, 2021

natewiebe13 commented Feb 21, 2021 • edited Loading

natewiebe13 commented Feb 21, 2021

natewiebe13 commented Feb 22, 2021

fabpot Feb 25, 2021

Choose a reason for hiding this comment

natewiebe13 Feb 25, 2021

Choose a reason for hiding this comment

stof Feb 25, 2021

Choose a reason for hiding this comment

natewiebe13 Feb 26, 2021

Choose a reason for hiding this comment

Pixelshaped commented Feb 25, 2021

fabpot commented Mar 5, 2021

natewiebe13 commented Feb 17, 2021 •

edited

Loading

natewiebe13 commented Feb 21, 2021 •

edited

Loading