Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[FrameworkBundle] [Translation] Update XLIFF source tags with new translation:update-xliff-sources command#53630
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
base:7.4
Are you sure you want to change the base?
Conversation
5219f97 to708ab39CompareCopying the default locale in the source tags of files for other locales should probably be done in a separate command than |
I created a dedicated command in the Translation component |
6095355 toefcfc4dCompareefcfc4d to496b27aCompare
welcoMattic left a comment• edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thanks@squrious for this great work! I have 2 small comments about wording.
Otherwise your PR looks good to me 👍
Can you rebase your branch please?
| $defaultLocaleCatalogue = $translatorBag->getCatalogue($this->defaultLocale); | ||
| if (!$defaultLocaleCatalogue instanceof MetadataAwareInterface) { | ||
| $io->error(sprintf('The default locale catalogue must implement "%s" to be used in this tool.', MetadataAwareInterface::class)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
| $io->error(sprintf('The default locale catalogue must implement "%s" to be used in thistool.', MetadataAwareInterface::class)); | |
| $io->error(sprintf('The default locale catalogue must implement "%s" to be used inbythiscommand.', MetadataAwareInterface::class)); |
| $currentCatalogue = $translatorBag->getCatalogue($locale); | ||
| if (!$currentCatalogue instanceof MessageCatalogue) { | ||
| $io->warning(sprintf('The catalogue for locale "%s" must be an instance of "%s" to be used in this tool.', $locale, MessageCatalogue::class)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
| $io->warning(sprintf('The catalogue for locale "%s" must be an instance of "%s" to be usedin thistool.',$locale, MessageCatalogue::class)); | |
| $io->warning(sprintf('The catalogue for locale "%s" must be an instance of "%s" to be usedby thiscommand.',$locale, MessageCatalogue::class)); |
| new InputOption('locales', null, InputOption::VALUE_REQUIRED | InputOption::VALUE_IS_ARRAY, 'Specify the locale to update.', $this->enabledLocales), | ||
| ]) | ||
| ->setHelp(<<<EOF | ||
| The <info>%command.name%</info> command updates the source tags in XLIFF files with the default locale translation if available. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
| The <info>%command.name%</info> command updates the source tags inXLIFF files with thedefaultlocaletranslation if available. | |
| The <info>%command.name%</info> command updates the source tags inXLIFF files with thedefault translation localeif available. |
?
| $format = $input->getOption('format'); | ||
| if (!\array_key_exists($format, self::FORMATS)) { | ||
| $io->error(sprintf('Unknown format "%s". Available formats are: %s.', $format, implode(', ', array_map(fn ($f) => '"'.$f.'"', array_keys(self::FORMATS))))); | ||
| return self::INVALID; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
shouldn't we move this toinitialize method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Not sure about that, I can't return any code from theinitialize method to indicate the option is invalid, so I would have to throw an exception instead.
I used the same logic as inTranslationUpdateCommand.
| * file that was distributed with this source code. | ||
| */ | ||
| namespace Command; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
namespace is wrong
ff77984 tof2c0cf4CompareHello, I updated the PR according to your comments (not sure about the use of Also moved the changelog entry in the translation component to 7.2. Low deps tests fail but I guess it's expected as the PR covers both framework bundle and translation component. |
f2c0cf4 to5e27384Compare
To fix the low deps tests, you need to bump the required version of the component in the bundle. |
Uh oh!
There was an error while loading.Please reload this page.
Context
Currently, XLIFF files generated with the
translation:extractcommand use the translation key to create the<source>tag:But usually this
<source>tag contains the default locale text to be translated (XLIFF 1.2,XLIFF 2.0).Also, the
<source>tag is already used inXliffFileLoaderto populate the catalogue metadata (but only for v1).Proposed solution
This PR adds three features:
sourcemetadata (if available) to create thesourcetag inXliffFileDumpersourcetag to populate thesourcemetadata inXliffFileLoaderfor XLIFF 2.0translation:update-xliff-sourcescommand to update thesourcetag in XLIFF filesResult
Given the following entries in XLIFF files:
Running
Will produce updates the files with:
The command has options to specify locales, domains, and paths: