[Translation] Introduce a way to configure the enabled locales #32433
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
yceruto
reviewed
Jul 8, 2019
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
Outdated
Show resolved
Hide resolved
yceruto
reviewed
Jul 8, 2019
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
Outdated
Show resolved
Hide resolved
yceruto
reviewed
Jul 8, 2019
Pierstoval
reviewed
Aug 8, 2019
Pierstoval
reviewed
Aug 8, 2019
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
Outdated
Show resolved
Hide resolved
c3a9485 to
06b572c
Compare
|
@javiereguiluz Can you rebase on current master? I'm wondering it we could use this new settings to automatically restrict |
06b572c to
26e2656
Compare
|
I've rebased and added a test, but please tell me any other test that you think it may be necessary. Thanks! |
|
I tweaked a bit the behavior of this option. It now works as follows: # config/packages/translation.yaml
framework:
translator:
# if you don't define this option or set it to null or [], all
# catalogs are generated (same behavior as current versions)
enabled_locales: null
enabled_locales: []
# if you define some values in this option, only those catalogs are generated
enabled_locales: ['es', 'en'] |
fabpot
requested changes
Dec 3, 2019
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
Outdated
Show resolved
Hide resolved
3a63bb2 to
7658434
Compare
nicolas-grekas
approved these changes
Feb 4, 2020
fabpot
approved these changes
Feb 4, 2020
|
Thank you @javiereguiluz. |
fabpot
added a commit
that referenced
this pull request
Feb 4, 2020
… locales (javiereguiluz) This PR was merged into the 5.1-dev branch. Discussion ---------- [Translation] Introduce a way to configure the enabled locales | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #31563 | License | MIT | Doc PR | - This implements the idea #31563 so we can decide if we want to add this or not. I tested it in the "Symfony Demo" app. Before: 107 catalogs created in cache/dev/translations/. After: 43 catalogs. But that's because the app is translated into lots of languages. In most cases, only 2 catalog files will be generated (vs 107 before). If this idea is approved, I'll add tests and docs. Thanks. Commits ------- 7658434 [Translation] Introduce a way to configure the enabled locales
fabpot
added a commit
that referenced
this pull request
Feb 4, 2020
…ales to build routes' default "_locale" requirement (nicolas-grekas) This PR was merged into the 5.1-dev branch. Discussion ---------- [FrameworkBundle] use framework.translator.enabled_locales to build routes' default "_locale" requirement | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - No need to configure the same requirements for `_locale` in all routes any more thanks to the `framework.translator.enabled_locales` config option introduced in #32433. Commits ------- 5eebd37 [FrameworkBundle] use framework.translator.enabled_locales to build routes' default "_locale" requirement
Merged
fabpot
added a commit
that referenced
this pull request
Jun 1, 2020
This PR was merged into the 5.1 branch. Discussion ---------- [FrameworkBundle] Fix enabled_locales behavior | Q | A | ------------- | --- | Branch? | 5.1 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - I was experimenting with enabled_locales on my application and I noticed the cache didn't actually change. It seems the generated service definition was invalid: the file `var/cache/dev/ContainerFEQLy1x/App_KernelDevDebugContainer.php` defined `getTranslator_DefaultService` by calling `new Translator` with 7 arguments instead of 6. It seems to be due to the fact that the DI extension does not replace the right argument. With the following fix applied the behavior works as expected. However, reading the comment of Javier in #32433, it seems he tested it against Demo and it worked with the previous code. I'm not sure why, @javiereguiluz I'd be interested in knowing if I'm missing something here :) . Commits ------- e2ce7f5 Fix enabled_locales behavior
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This implements the idea #31563 so we can decide if we want to add this or not. I tested it in the "Symfony Demo" app. Before: 107 catalogs created in cache/dev/translations/. After: 43 catalogs. But that's because the app is translated into lots of languages. In most cases, only 2 catalog files will be generated (vs 107 before).
If this idea is approved, I'll add tests and docs. Thanks.