[2.1] Extracting translation messages from templates #1283
Closed
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
|
Can you extract the actual twig crawling, writing and such logic into a service object of its own? I have applications where i want to extend or change this mechanism of finding templates somewhere to translate. |
|
Also the command should be |
| $output->writeln(sprintf('Generating "<info>%s</info>" translation files for "<info>%s</info>"', $input->getArgument('locale'), $foundBundle->getName())); | ||
|
|
||
| // create catalogue | ||
| $catalogue = new \Symfony\Component\Translation\MessageCatalogue($input->getArgument('locale')); |
There was a problem hiding this comment.
you should add a use statement to follow the CS.
|
@stof no doubts you are a very dedicated code reader ;) I will push your suggestions in a few |
|
Instead of isolating this command to twig, you could just add twig as another |
|
@xaav I suggested to move it to TwigBundle because only Twig is supported. |
| * | ||
| * @var array | ||
| */ | ||
| private $supportedLoaders = array('yml', 'xliff', 'php'); |
There was a problem hiding this comment.
This is wrong. You should automatically detect supported loaders from the DIC, not have them hardcoded.
There was a problem hiding this comment.
You are right, I am just waiting for @stof here https://github.com/symfony/symfony/pull/1283/files#L2R41
There was a problem hiding this comment.
Just grab all the supported loaders in the __construct() function, then store the metadata in the $this->supportedLoaders array.
| /** | ||
| * Interface implemented by all extractors | ||
| */ | ||
| interface ExtractorInterface { |
There was a problem hiding this comment.
the curly brace should be on its own line
|
@stof so most of the classes should be moved to the translation component. I'am currently doing it. |
8000
|
@michelsalib Only classes that depends only on the Translation component |
|
Right, I'll push it in 5 minutes ;) |
|
Idea: Instead of creating a separate ExtractorInterface, we could just use the LoaderInterface and move the Finder code to the command. I've been gone last week, so I'll take another look at the PhpExtractor and see if I can improve it. |
|
I won't move the Finder code to the command. The things are very decoupled now, and I would like to keep it this way. |
added 3 commits
July 2, 2011 15:42
Conflicts: src/Symfony/Bundle/TwigBundle/Resources/config/twig.xml
Conflicts: src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/TranslatorPass.php src/Symfony/Bundle/TwigBundle/Resources/config/twig.xml src/Symfony/Bundle/TwigBundle/TwigBundle.php
|
First, thank you all for your hard work on this feature. As this is a quite large patch, I would rather split it up in more manageable chunks. First, I think we can work on the |
|
Sure, I should be able to do it by Monday evening. |
|
Hi, I did as requested here : #2045. |
fabpot
added a commit
that referenced
this pull request
Aug 29, 2011
Commits ------- 6278fcb -- add dumpers for translation component Discussion ---------- [2.1] Add dumpers for translation catalogs As seen here #1283, I push just the translation catalogs dumpers. It involved renaming and some essentially. I also included a Pot dumper/loader that have been provided here : michelsalib/BCCExtraToolsBundle#12. --------------------------------------------------------------------------- by fabpot at 2011/08/28 11:12:30 -0700 Can you add the license header? and the main phpdoc block for each class? --------------------------------------------------------------------------- by michelsalib at 2011/08/29 02:32:50 -0700 Done ! --------------------------------------------------------------------------- by fabpot at 2011/08/29 03:17:43 -0700 Last, but not the least, can you add some unit tests and squash all your commits? Thanks a lot. --------------------------------------------------------------------------- by michelsalib at 2011/08/29 03:21:43 -0700 How do I squash it, should I make a new PR ? --------------------------------------------------------------------------- by fabpot at 2011/08/29 03:25:09 -0700 No need to make a new PR, you can just force the push with `-f`. --------------------------------------------------------------------------- by fabpot at 2011/08/29 03:33:59 -0700 Also, in the tests, it would great if you can add some that do a load/dump/load, just to be sure that everything work fine. --------------------------------------------------------------------------- by michelsalib at 2011/08/29 03:39:41 -0700 What is the need of such a test, considering that the current tests that I am writing are using the same fixtures ? --------------------------------------------------------------------------- by fabpot at 2011/08/29 03:47:50 -0700 The goal is to ensure that the load/dump calls are idempotent. --------------------------------------------------------------------------- by michelsalib at 2011/08/29 03:56:12 -0700 Ye. Actually the load is using referenc files from the fixtures directory. My new tests will be using the dumper with the same files. Isn't that enough ? I try to stay DRY on this one. --------------------------------------------------------------------------- by michelsalib at 2011/08/29 05:09:52 -0700 I just add unit tests and squash the commits. I still need to be conviced about the load/dump/load unit tests. But if I did not made a point, I'll do as requested on next answer.
|
closing this PR as the second part has now been submitted as #2051 |
fabpot
added a commit
that referenced
this pull request
Sep 9, 2011
Commits ------- ef322f6 -- add command that extracts translation messages from templates Discussion ---------- [2.1] Extracting translation messages from templates As seen here #1283 and here #2045, I push the command that extract translation from templates. There are still a lot of new things here, but it seems more manageable. --------------------------------------------------------------------------- by stof at 2011/09/04 02:04:40 -0700 @michelsalib Could you try to refactor the code to make it more flexible by moving the creating of the file to the dumpers to support other outputs (database...) ? --------------------------------------------------------------------------- by michelsalib at 2011/09/04 02:35:50 -0700 You are right, I shall do it tonight. --------------------------------------------------------------------------- by michelsalib at 2011/09/04 11:53:35 -0700 I just pushed a refactoring that should allow more flexibility. Dumpers are now responsive for writing the files. This way it is now possible to implement the DumperInterface that dump to a database and add it to the TranslationWriter. I updated the tests accordingly. --------------------------------------------------------------------------- by fabpot at 2011/09/05 23:27:27 -0700 To be consistent with other dumpers in the framework, the dumpers extending `FileDumper` should use `FileDumper` as a suffix like in `YmlFileDumper`. --------------------------------------------------------------------------- by fabpot at 2011/09/05 23:41:12 -0700 A general note on PHPDoc: The first line of a phpdoc ends with a dot and starts with a present verb like in `Extracts translation messages from template files.` --------------------------------------------------------------------------- by michelsalib at 2011/09/06 01:23:31 -0700 I fixed most of the remarks. I just need to go through the phpdoc (in a few minutes). --------------------------------------------------------------------------- by stloyd at 2011/09/06 01:28:55 -0700 @michelsalib you should use `git rebase` (see [docs](http://symfony.com/doc/current/contributing/code/patches.html#id1)) instead of `git merge`. --------------------------------------------------------------------------- by michelsalib at 2011/09/06 01:31:06 -0700 @stloyd sorry. I will rebase (squash) everything when I am finished. --------------------------------------------------------------------------- by kaiwa at 2011/09/06 01:31:18 -0700 Hey, it might be a little bit late, but may i ask a not code-related question? Is it correct that the `--force` option only means to write the output to a file instead of writing it to stdout? If so, 1. is it semantically correct? I mean... i'm not a native english speaker, but from unix programs i'm used to interpret a `--force` option as something like "overwrite", "ignore errors" or "suppress warnings". An option which is used in case of trouble most time. Feels confusing to me to be forced ;-) to use the `--force` option for simply writing to files. 2. does it makes sense to have a default behaviour instead of requiring the user to give either `--force` or `--dump-messages`? In which cases does the user wants to dump the messages to the console? Is it only for debugging / to review the messages? --------------------------------------------------------------------------- by michelsalib at 2011/09/06 01:33:58 -0700 @kaiwa Your concerns seems perfectly right. Initially I just wanted to mimic the `doctrine:schema:update` command. But it can be changed. @fabpot what do you think ? --------------------------------------------------------------------------- by michelsalib at 2011/09/06 02:01:22 -0700 @stloyd I tried to do a `git rebase` and I am quite lost. I think I messed something when merge instead of rebase. How can I clean/squash my PR properly ? --------------------------------------------------------------------------- by stloyd at 2011/09/06 02:11:29 -0700 @michelsalib for now just work as it is ;-) When I back from work I will try to help you to rebase it properly. --------------------------------------------------------------------------- by michelsalib at 2011/09/06 02:12:17 -0700 @stloyd Thank you ! --------------------------------------------------------------------------- by stloyd at 2011/09/06 12:39:18 -0700 @michelsalib I was trying to rebase your code and revert it this _bad_ commit (merge), but without success. IMO best way would be making _brand new_ branch based on symfony/master, and the `git cherry-pick` ([hint](http://ariejan.net/2010/06/10/cherry-picking-specific-commits-from-another-branch)) commits you need (almost all in this PR but without this one with merge), then you will need to apply again _by hand_ changes from commit michelsalib@fce24c7 (this clean up you have done there). I'm sorry I can't give you more help... --------------------------------------------------------------------------- by michelsalib at 2011/09/07 09:41:36 -0700 @stloyd I finally succeed to fix this PR. Thanks to your help! I must admit the whole thing with `cherry-pick` command was quite epic. @fabpot Don't be sorry, I am about to fix the PHPDoc. I shall squash right after. --------------------------------------------------------------------------- by michelsalib at 2011/09/07 10:07:20 -0700 I just squashed and did a last polish of the code. You might want to read it again before merge. --------------------------------------------------------------------------- by fabpot at 2011/09/07 11:40:48 -0700 ok, code looks really good now. I think the only missing thing is some more unit tests (for the extractors for instance). --------------------------------------------------------------------------- by michelsalib at 2011/09/07 12:18:59 -0700 Thanks, I'll look into it tomorrow. --------------------------------------------------------------------------- by michelsalib at 2011/09/08 15:13:10 -0700 Hi, I just added unit tests for both extractors (php and yaml). Concerning the yaml extractor, the test is quite tricky because I need to mock the twig environment while returning a twig tree that contain a trans block and a trans filter. But it work fine with as little side effects as possible. Also I am not sure that the test should be in the TwigBundle. IMO, the PR seems ready. --------------------------------------------------------------------------- by stof at 2011/09/08 15:34:41 -0700 As the extractor is in bridge, the tests should be in the folder containing the tests for the bridge in tests/ --------------------------------------------------------------------------- by michelsalib at 2011/09/08 15:41:45 -0700 Thanks @stof, it is now fixed. --------------------------------------------------------------------------- by michelsalib at 2011/09/09 00:48:47 -0700 thanks @stloyd, it is fixed now. --------------------------------------------------------------------------- by michelsalib at 2011/09/09 01:25:24 -0700 Fixed again ;)
|
Hi all, When i use commande like: msgstr is autocomplete with msgid, bu i would like the target is empty, for new entries. How should i do? |
This was referenced Dec 28, 2013
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.
As seen in #1259, I push myself the translation:update command. I tried to address most of the remarks. I also moved the command to the Twig Bundle as long as it extracts translation strings from twig templates.
For the moment it does not address extraction from php template or cross bundle translations.
Thanks to @xaav for his help on this PR.
translation:update command examples
To extract messages from your bundle and display in the console:
translation:update --dump-messages fr MyBundleYou can save them to the
MyBundle\Resources\translationsdirectory:translation:update --force fr MyBundleIn another language:
translation:update --force en MyBundleSpecify the output format with the
--output-formatoption (eitheryml,xliff,phporpot):translation:update --output-format="xliff" --force en MyBundleChange the prefix used for newly added messages with the
--prefixoption:translation:update --output-format="xliff" --force --prefix='myprefix' en MyBundle