-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[FrameworkBundle] Update translation commands to work with default paths #25065
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@@ -78,6 +78,8 @@ | |||
<argument type="service" id="translator" /> | |||
<argument type="service" id="translation.reader" /> | |||
<argument type="service" id="translation.extractor" /> | |||
<argument>%translator.default_path%</argument> | |||
<argument on-invalid="null">%twig.default_path%</argument> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
on-invalid="null"
strategy over container parameter is not supported or (surely) I'm missing something? Should I do it into FrameworkExtension
then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, it should be done in the DI extension then
3a76018
to
7f292a3
Compare
@yceruto do you think you'll be able to have this PR ready before 4.0.0? Would be awesome :) |
Sure, today! :) |
060141b
to
7661cd6
Compare
7661cd6
to
dc72866
Compare
Tests added and all green! Also I've tested this PR in a real project and work like a charm ;) These are all paths where translations and templates are debugged and dumped: Root paths:
Bundle paths:
Ready for reviews! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With minor comments
|
||
/** | ||
* @param TranslatorInterface $translator | ||
* @param TranslationReaderInterface $reader | ||
* @param ExtractorInterface $extractor | ||
* @param string $defaultTransPath | ||
* @param string $defaultViewsPath |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
string|null
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've removed the block as it does not help in any way.
|
||
/** | ||
* @param TranslationWriterInterface $writer | ||
* @param TranslationReaderInterface $reader | ||
* @param ExtractorInterface $extractor | ||
* @param string $defaultLocale | ||
* @param string $defaultTransPath | ||
* @param string $defaultViewsPath |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
string|null
Sorry, something went wrong.
All reactions
Thank you @yceruto. |
All reactions
Sorry, something went wrong.
… default paths (yceruto) This PR was merged into the 3.4 branch. Discussion ---------- [FrameworkBundle] Update translation commands to work with default paths | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #25062 | License | MIT | Doc PR | symfony/symfony-docs#8634 This should make translation commands (debug & update) work with `translator.default_path` and `twig.default_path` directories (introduced here in 3.4) and their overridden paths if available. Would be great to include also the custom paths mapping by the user, either `translator.paths` as `twig.paths`, but I'm not sure about the right way and probably it should be implemented on another branch. TODO - [x] Add some tests. Commits ------- dc72866 Update translation commands to work with default paths
fabpot
nicolas-grekas
chalasr
Successfully merging this pull request may close these issues.
This should make translation commands (debug & update) work with
translator.default_path
andtwig.default_path
directories (introduced here in 3.4) and their overridden paths if available.Would be great to include also the custom paths mapping by the user, either
translator.paths
astwig.paths
, but I'm not sure about the right way and probably it should be implemented on another branch.TODO