[FrameworkBundle][Translation][TwigBridge] Add option to extract blank messages #45772
Conversation
carsonbot
changed the title
|
Psalm fail seems unrelated as I see the same error in other PRs. AppVeyor timeout. |
|
Pinging @Nyholm, because you agreed in my previous PR that this is a problem worth solving 🙂 . |
|
Hey! I think @natewiebe13 has recently worked with this code. Maybe they can help review this? Cheers! Carsonbot |
src/Symfony/Component/Translation/Extractor/BlankableExtractorInterface.php
Outdated
Show resolved
Hide resolved
|
@gndk great work, thank you! Could you please rebase your branch and address the 2 quick comments? Thanks 👍 |
src/Symfony/Bundle/FrameworkBundle/Tests/Command/TranslationUpdateCommandTest.php
Outdated
Show resolved
Hide resolved
| if (!method_exists($this->extractor, 'blank') && true === $input->getOption('blank')) { | ||
| $errorIo->error('The --blank option is not supported by this extractor. It is available in symfony/translation 6.1 and symfony/twig-bridge 6.1.'); | ||
|
|
||
| return 1; | ||
| } | ||
|
|
There was a problem hiding this comment.
I added the error message up here because other checks like this also live here. And I don't need to pass the OutputInterface down to the method. Seems cleaner this way.
Not sure about the wording of the message. Anything you want to change or add?
…k messages Extract new-found messages as blank strings instead of a prefixed copy of the translation key when using the blank option for the translation:extract command. This allows for a smoother workflow when using third party translation providers, because empty strings are considered “untranslated”, while an exact copy of the translation key is considered “translated”.
There was a problem hiding this comment.
The proposed method is a setter. This makes the extractor services mutable. We'd really better avoid that.
Does it really make sense to have a command line option for this? Isn't setting messages as empty a project-wide setting? If yes, then we'd better go with a config option and a constructor argument.
|
@nicolas-grekas is right, we should avoid mutable services, I missed it. 👍 for config option. |
|
Ok, I will turn it into a config option instead 👍 . Don't have time this week, so will be done after the easter weekend. |
|
Friendly ping @gndk |
Currently, new messages are extracted as a prefixed copy of the key (
"foo" -> "__foo") when using thetranslation:extractcommand.With his PR I propose a
--blankoption that would extract new messages as blank strings ("foo" -> "").This allows for smoother workflows with translation providers like Loco, Lokalise or CrowdIn, because by some of them the prefixed copy is considered translated, while blank strings are considered untranslated.
The result is that you currently have 100% translation progress immediately after
translation:push, even if none of the translation keys are actually translated. This makes working with hundreds or thousands of translation keys or general collaboration through these tools unnecessarily hard.For example lets say you have 1000 translation keys in your application and want to add a new locale. You extract and push them to the provider. Then you or some translator starts work on them. They won't finish all keys in one session, but instead maybe finish 100 of them and continue work the next day. However, because all the messages already have the "translated" status, its hard to know where to continue. You have to look at every single message again and determine if it is really translated, or if the strings are just technically different.
It is very easy to miss untranslated messages if all of them have a green checkmark, regardless of the real translation status.
The benefit of pushing blank strings as translation messages is that it allows you to easily filter for untranslated messages as a "todo list", instead of checking each message manually for its translation status.