8000 [FrameworkBundle] Update translation commands to work with default paths by yceruto · Pull Request #25065 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[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

Merged
merged 1 commit into from
Nov 23, 2017

Conversation

yceruto
Copy link
Member
@yceruto yceruto commented Nov 21, 2017
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

  • Add some tests.

@@ -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>
Copy link
Member Author
@yceruto yceruto Nov 21, 2017

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?

Copy link
Member

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

@nicolas-grekas nicolas-grekas added this to the 3.4 milestone Nov 21, 2017
@yceruto yceruto force-pushed the translation_commands branch from 3a76018 to 7f292a3 Compare November 21, 2017 13:16
@nicolas-grekas
Copy link
Member

@yceruto do you think you'll be able to have this PR ready before 4.0.0? Would be awesome :)

@yceruto
Copy link
Member Author
yceruto commented Nov 22, 2017

Sure, today! :)

@yceruto yceruto force-pushed the translation_commands branch from 7661cd6 to dc72866 Compare November 22, 2017 18:43
@yceruto
Copy link
Member Author
yceruto commented Nov 22, 2017

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:

  • %kernel.root_dir%/Resources/translations/
  • %kernel.root_dir%/Resources/views/
  • %translator.default_path%/
  • %twig.default_path%/

Bundle paths:

  • <BundlePath>/Resources/translations/
  • <BundlePath>/Resources/views/
  • %kernel.root_dir%/Resources/<Bundle>/translations/
  • %kernel.root_dir%/Resources/<Bundle>/views/
  • %translator.default_path%/<Bundle>/
  • %twig.default_path%/<Bundle>/

Ready for reviews!

Copy link
Member
@chalasr chalasr left a 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
Copy link
Member

Choose a reason for hiding this comment

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

string|null

Copy link
Member

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
Copy link
Member

Choose a reason for hiding this comment

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

string|null

@fabpot
Copy link
Member
fabpot commented Nov 23, 2017

Thank you @yceruto.

@fabpot fabpot merged commit dc72866 into symfony:3.4 Nov 23, 2017
fabpot added a commit that referenced this pull request Nov 23, 2017
… 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
@yceruto yceruto deleted the translation_commands branch November 23, 2017 15:53
This was referenced Nov 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants
0