8000 [FrameworkBundle][Translation] Add support for Translator paths, Twig paths and Translator aware services paths in commands by yceruto · Pull Request #29121 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[FrameworkBundle][Translation] Add support for Translator paths, Twig paths and Translator aware services paths in commands #29121

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Feb 13, 2019
Merged

[FrameworkBundle][Translation] Add support for Translator paths, Twig paths and Translator aware services paths in commands #29121

merged 1 commit into from
Feb 13, 2019

Conversation

yceruto
Copy link
Member
@yceruto yceruto commented Nov 7, 2018 •
edited
Loading
Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #29085, #29633, #17739
License MIT
Doc PR TODO

Add custom (also common) Twig and Translation paths to the translation commands:

  • Custom directories configured in twig.paths.
  • Custom directories configured in translator.paths
  • The Resources/translations/ directory of Validation component (if installed).
  • The Resources/translations/ directory of Form component (if installed).
  • The Resources/translations/ directory of Security Core component (if installed).

nicolas-grekas
nicolas-grekas reviewed Nov 7, 2018
View reviewed changes
Copy link
Member
@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for starting this :)

@yceruto yceruto force-pushed the deprecating_bundle_argument branch 4 times, most recently from 6077362 to 533d4d4 Compare November 7, 2018 17:06
@yceruto yceruto mentioned this pull request Nov 7, 2018
Merged
@yceruto
Copy link
Member Author
yceruto commented Nov 7, 2018 •
edited
Loading

Update: Extracting translation messages from services (e.g. controllers and other PHP files) that have the translator injected into them by using the following service injection methods: (properties, bindings, construct arguments and method calls).

BUT they're other methods that I can't reach (e.g. the service locator related to: services subscriber and controller service arguments) I need help to accomplish it, maybe @nicolas-grekas ?

@yceruto yceruto force-pushed the deprecating_bundle_argument branch from ceb615f to 46fc19b Compare November 8, 2018 00:59
fabpot added a commit that referenced this pull request Nov 8, 2018
@fabpot
bug #29128 [FrameworkBundle] Cleaning translation commands and fix a …
6ba1803 
…bug for --all option (yceruto)

This PR was squashed before being merged into the 4.2-dev branch (closes #29128).

Discussion
----------

[FrameworkBundle] Cleaning translation commands and fix a bug for --all option

| Q             | A
| ------------- | ---
| Branch?       | master (4.2)
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #29121 (comment)
| License       | MIT
| Doc PR        | -

Commits
-------

dfc7dcc Also fix a bug for --all option
3cbefd8 Cleaning translation commands
@yceruto yceruto mentioned this pull request Nov 8, 2018
Closed
@nicolas-grekas nicolas-grekas added this to the next milestone Nov 8, 2018
nicolas-grekas
nicolas-grekas reviewed Nov 8, 2018
View reviewed changes
Copy link
Member
@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should maybe split the translator propagation in another pass.
I'd suggest to look at the HotPathPass also: we could looks at services that has the translation injected transitively. And that would give you a hint about following locators btw (HotPathPass doesn't follow them but here we would - see instanceof ArgumentInterface + its getValue method).

@yceruto yceruto force-pushed the deprecating_bundle_argument branch 2 times, most recently from 8004646 to 9fe9c2d Compare November 16, 2018 18:34
@yceruto
Copy link
Member Author
yceruto commented Nov 16, 2018

Status: Needs work

@yceruto yceruto force-pushed the deprecating_bundle_argument branch 9 times, most recently from 5d56766 to 555a562 Compare November 19, 2018 15:24
@yceruto yceruto force-pushed the deprecating_bundle_argument branch from 6a12280 to e57b804 Compare January 24, 2019 22:30
@yceruto
Copy link
Member Author
yceruto commented Jan 24, 2019

Yay! funcional tests passed!

@nicolas-grekas this is ready on my side, how does it look for you?

@yceruto yceruto force-pushed the deprecating_bundle_argument branch 2 times, most recently from 7b2afe6 to b37ed12 Compare January 27, 2019 13:27
@yceruto yceruto mentioned this pull request Jan 31, 2019
Closed
@yceruto
Copy link
Member Author
yceruto commented Jan 31, 2019

Friendly ping.

@yceruto
Copy link
Member Author
yceruto commented Feb 6, 2019

Maybe I should split this PR into two for an easier review? I mean, move the last point that contains a little more complexity:

  • All PHP files from services "translator aware" via constructor argument, public property, method calls, service subscriber, controller argument)

@xabbuh
Copy link
Member
xabbuh commented Feb 8, 2019

@yceruto I think that could be helpful

@yceruto yceruto force-pushed the deprecating_bundle_argument branch from b37ed12 to 0383072 Compare February 8, 2019 16:13
@yceruto
Copy link
Member Author
yceruto commented Feb 8, 2019 •
edited
Loading

@xabbuh working on it, thanks for your reply.

Status: Needs work

@yceruto yceruto force-pushed the deprecating_bundle_argument branch 2 times, most recently from 598b3d4 to 31d7a09 Compare February 8, 2019 17:25
@yceruto
Copy link
Member Author
yceruto commented Feb 8, 2019 •
edited
Loading

Update: I've divided the complexity of this PR as I mentioned earlier, see the updated description.

Status: Needs review

@yceruto yceruto mentioned this pull request Feb 8, 2019
Merged
@yceruto
Copy link
Member Author
yceruto commented Feb 8, 2019

Done #30120.

@fabpot
Copy link
Member
fabpot commented Feb 13, 2019

Thank you @yceruto.

@fabpot fabpot merged commit 31d7a09 into symfony:master Feb 13, 2019
fabpot added a commit that referenced this pull request Feb 13, 2019
@fabpot
feature #29121 [FrameworkBundle][Translation] Add support for Transla…
03dfdc4 
…tor paths, Twig paths and Translator aware services paths in commands (yceruto)

This PR was merged into the 4.3-dev branch.

Discussion
----------

[FrameworkBundle][Translation] Add support for Translator paths, Twig paths and Translator aware services paths in commands

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #29085, #29633, #17739
| License       | MIT
| Doc PR        | TODO

Add custom (also common) Twig and Translation paths to the translation commands:
 * Custom directories configured in `twig.paths`.
 * Custom directories configured in `translator.paths`
 * The `Resources/translations/` directory of `Validation` component (if installed).
 * The `Resources/translations/` directory of `Form` component (if installed).
 * The `Resources/translations/` directory of Security Core component (if installed).

Commits
-------

31d7a09 Add support for translator paths and twig paths in translation commands
@yceruto yceruto deleted the deprecating_bundle_argument branch February 13, 2019 13:08
@nicolas-grekas nicolas-grekas modified the milestones: next, 4.3 Apr 30, 2019
@fabpot fabpot mentioned this pull request May 9, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicolas-grekas nicolas-grekas nicolas-grekas left review comments

@fabpot fabpot fabpot approved these changes

@damienalexandre damienalexandre damienalexandre approved these changes

Assignees
No one assigned
Labels
Feature FrameworkBundle Status: Reviewed Translation
Projects
None yet
Milestone
4.3
Development

Successfully merging this pull request may close these issues.
6 participants
@yceruto @nicolas-grekas @xabbuh @fabpot @damienalexandre @carsonbot
0