NotificationsYou must be signed in to change notification settings
Fork9.7k
Star30.8k

[FrameworkBundle] Add default translations path option and convention#24860

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

Jump to bottom

Merged

fabpot merged 1 commit intosymfony:3.4fromyceruto:translations_default_path

Nov 11, 2017

Merged

[FrameworkBundle] Add default translations path option and convention#24860

fabpot merged 1 commit intosymfony:3.4fromyceruto:translations_default_path

Nov 11, 2017

Conversation

Copy link

Member

yceruto commentedNov 7, 2017•
edited
Loading

Q	A
Branch?	3.4
Bug fix?	no
New feature?	no
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	-
License	MIT
Doc PR	ToDo

Similar to Twig default path, this proposal adds adefault_path option undertranslator config:

framework:translator:default_path:'%kernel.project_dir%/config/translations'

adding this default path to the discovered translations dirs. Thus, overriding bundle translations is possible by using this new convention:config/translations/<BundleName>/messages.en.xlf.

Also a new container parameter%translator.default_path% is defined by external purpose (similar to#24840)

Note: The current convention%kernel.root_dir%/Resources/translations path has priority over the new one.

TODO:

Add more tests about the new path
Update changelog

carsonbot added Status: Needs Review FrameworkBundle Feature labels

Nov 7, 2017

Copy link

MemberAuthor

yceruto commentedNov 7, 2017

Having this in 3.4/4.0 could be awesome but I guess it doesn't qualify, right?

Copy link

MemberAuthor

yceruto commentedNov 10, 2017•
edited
Loading

Any opinion on this? I'd like to finish it :)

Copy link

Member

fabpot commentedNov 10, 2017

It does qualify. LGTM.

Copy link

Member

fabpot commentedNov 10, 2017

Wouldn't it be "better" to move them toconfig/translations? Like for serialization and validation configs?

yceruto changed the base branch frommaster to3.4

November 10, 2017 20:06

yceruto force-pushed thetranslations_default_path branch fromdee7d07 to01dbc55Compare

November 10, 2017 20:13

Add default translations path option and convention

1a8b1b4

yceruto force-pushed thetranslations_default_path branch from01dbc55 to1a8b1b4Compare

November 10, 2017 20:14

Copy link

MemberAuthor

yceruto commentedNov 11, 2017

Great! ready :)

Copy link

Member

fabpot commentedNov 11, 2017

Thank you@yceruto.

fabpot merged commit1a8b1b4 intosymfony:3.4

Nov 11, 2017

fabpot added a commit that referenced this pull request

Nov 11, 2017

feature#24860[FrameworkBundle] Add default translations path option…

b0ce1c1

… and convention (yceruto)This PR was merged into the 3.4 branch.Discussion----------[FrameworkBundle] Add default translations path option and convention| Q             | A| ------------- | ---| Branch?       | 3.4| Bug fix?      | no| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets | -| License       | MIT| Doc PR        | ToDoSimilar to Twig default path, this proposal adds a `default_path` option under `translator` config:```yamlframework:    translator:        default_path: '%kernel.project_dir%/config/translations'```adding this default path to the discovered translations dirs. Thus, overriding bundle translations is possible by using this new convention: `config/translations/<BundleName>/messages.en.xlf`.Also a new container parameter `%translator.default_path%` is defined by external purpose (similar to#24840)Note: The current convention `%kernel.root_dir%/Resources/translations` path has priority over the new one.TODO:- [x] Add more tests about the new path- [x] Update changelogCommits-------1a8b1b4 Add default translations path option and convention

Copy link

Contributor

ogizanagi commentedNov 11, 2017•
edited
Loading

Do we want to reflect this change in theexisting translation recipe or create a new one for 3.4 (so we can even remove theframework.translator.paths line)?