[Translation] added extract messages from controllers #26738
Conversation
| @@ -127,6 +127,7 @@ protected function execute(InputInterface $input, OutputInterface $output) | ||
| if ($this->defaultViewsPath) { | ||
| $viewsPaths[] = $this->defaultViewsPath; | ||
| } | ||
| $controllersPaths = array($kernel->getRootDir().'/Controller'); |
There was a problem hiding this comment.
What if my controllers are somewhere else?
There was a problem hiding this comment.
@iltar you're right. I added a parameter to constructor (as for the views). But I'm not sure how to test it. Can you help me?
There was a problem hiding this comment.
What about making the parameter an array of paths? That would cover multiple locations. You can test it by writing fixtures for your test, writing controllers with translations.
There was a problem hiding this comment.
An array of paths sounds great to me, but for the tests I wonder how to set path(s) when I call the command on CLI (bin/console translation:update)?
When I try your suggestion, it returns null for $this->router.
I'm not sure it's OK to have a behaviour for templates (parameter in constructor) and another behaviour for controllers (find auto).
There was a problem hiding this comment.
There's a small issue with the current implementation:
- It assumes a certain directory
- I can only add 1 extra directory
It's hard to find all controllers (but not impossible).
In a command, you could do something like this:
public function execute(InputInterface $input, OutputInterface $output)
{
$rows = [];
foreach ($this->router->getRouteCollection() as $route) {
if (!$route->hasDefault('_controller')) {
continue;
}
$request = new Request([], [], ['_controller' => $route->getDefault('_controller')]);
$controller = $this->resolver->getController($request);
}
}The other option would be to pass a list of resolved controllers or a list of directories (less accurate).
|
You'll have to inject the router |
|
Do you think it's a good option to have 2 different behaviours for templates and controllers? |
|
If I inject RouterInterface, I will break BC (constructor signature will require a new parameter). |
|
You can make it optional, or not typehint it and resolve the actual type (+deprecation warning) in the constructor |
|
I injected RouterInterface and scan for controllers. |
src/Symfony/Bundle/FrameworkBundle/Command/TranslationUpdateCommand.php
Outdated
Show resolved
Hide resolved
src/Symfony/Bundle/FrameworkBundle/Command/TranslationUpdateCommand.php
Outdated
Show resolved
Hide resolved
src/Symfony/Bundle/FrameworkBundle/Command/TranslationUpdateCommand.php
Outdated
Show resolved
Hide resolved
src/Symfony/Bundle/FrameworkBundle/Command/TranslationUpdateCommand.php
Outdated
Show resolved
Hide resolved
src/Symfony/Bundle/FrameworkBundle/Tests/Command/TranslationUpdateCommandTest.php
Outdated
Show resolved
Hide resolved
FIX revert the CS change here to avoid making merging more complex with older branches FIX one line instructions
|
Instead of hardcoding something for controllers only, what about allowing to configure a list of paths in a "translation.something" configuration option and make the command look there? That would work for controllers but also for anything else, wdyt? Another idea could be to detect all services that have the translator injected into them and consider them all as targets for translation extraction. |
I prefer your 2nd idea, but I've no clue how do that. I don't think that "grep" every PHP classes is a right way? |
|
See #29121 for alternative solution implementing the 2nd idea. |
|
Closed in favor of #29121. |
Added a loop on controllers to extract messages.