Skip to content
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

Jump to bottom

[Translation] Fix TranslationPullCommand with ICU translations #44085

Merged
merged 1 commit into from Dec 25, 2021
Merged

[Translation] Fix TranslationPullCommand with ICU translations #44085

merged 1 commit into from Dec 25, 2021
+93 −9

Conversation

@Kocal
Copy link
Contributor

@Kocal Kocal commented Nov 16, 2021 •
edited

Q A
Branch? 5.3
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #...
License MIT
Doc PR symfony/symfony-docs#...

Today I've faced an issue when trying to pull (ICU and non-ICU) translations from Loco:

  • I have a non-ICU translation say_hello tagged as messages
  • and a ICU translation say_hello_icu tagged as messages+intl-icu

When running bin/console translation:pull loco --domains messages --domains messages+intl-icu, it only creates a single file messages.fr.xlf that contains the two translations, so say_hello_icu will never be interpreted as an ICU-translation:

<?xml version="1.0" encoding="utf-8"?>
<xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" version="1.2">
  <file source-language="fr" target-language="fr" datatype="plaintext" original="file.ext">
    <header>
      <tool tool-id="symfony" tool-name="Symfony"/>
    </header>
    <body>
      <trans-unit id="xQEX2ok" resname="say_hello_icu">
        <source>say_hello_icu</source>
        <target>Bonjour {name} !</target>
      </trans-unit>
      <trans-unit id="1IHotcu" resname="say_hello">
        <source>say_hello</source>
        <target>Bonjour %name% !</target>
      </trans-unit>
    </body>
  </file>
</xliff>

With this fix, it creates two files as expected:

  • messages.fr.xlf:
<?xml version="1.0" encoding="utf-8"?>
<xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" version="1.2">
  <file source-language="fr" target-language="fr" datatype="plaintext" original="file.ext">
    <header>
      <tool tool-id="symfony" tool-name="Symfony"/>
    </header>
    <body>
      <trans-unit id="1IHotcu" resname="say_hello">
        <source>say_hello</source>
        <target>Bonjour %name% !</target>
      </trans-unit>
    </body>
  </file>
</xliff>
  • messages+intl-icu.fr.xlf:
<?xml version="1.0" encoding="utf-8"?>
<xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" version="1.2">
  <file source-language="fr" target-language="fr" datatype="plaintext" original="file.ext">
    <header>
      <tool tool-id="symfony" tool-name="Symfony"/>
    </header>
    <body>
      <trans-unit id="xQEX2ok" resname="say_hello_icu">
        <source>say_hello_icu</source>
        <target>Bonjour {name} !</target>
      </trans-unit>
    </body>
  </file>
</xliff>

I've also added more tests to LocoProvider and TranslationPullCommand, ensuring it still handle ICU translations in the future.

Ping @welcoMattic
@carsonbot carsonbot added this to the 5.3 milestone Nov 16, 2021
@Kocal
Copy link
Contributor Author

@Kocal Kocal commented Nov 16, 2021

Failures are unrelated.

@rpkamp rpkamp mentioned this pull request Nov 16, 2021
Closed
@carsonbot
Copy link

@carsonbot carsonbot commented Nov 17, 2021

Hey!

I think @Guite has recently worked with this code. Maybe they can help review this?

Cheers!

Carsonbot

@Kocal Kocal force-pushed the fix/translation-pull-icu branch 2 times, most recently from 4acad64 to fea0d19 Dec 3, 2021
@Kocal Kocal force-pushed the fix/translation-pull-icu branch from fea0d19 to 1a77293 Dec 10, 2021
@Kocal
Copy link
Contributor Author

@Kocal Kocal commented Dec 10, 2021

AppVeyor failures are not related.

@Kocal Kocal force-pushed the fix/translation-pull-icu branch from 1a77293 to 05b3b2d Dec 17, 2021
@Kocal
Copy link
Contributor Author

@Kocal Kocal commented Dec 17, 2021

Status: needs review

@nicolas-grekas nicolas-grekas force-pushed the fix/translation-pull-icu branch from 162d7d4 to 1802488 Dec 25, 2021
@nicolas-grekas
Copy link
Member

@nicolas-grekas nicolas-grekas commented Dec 25, 2021

Thank you @Kocal.

@nicolas-grekas nicolas-grekas merged commit 758c07f into symfony:5.3 Dec 25, 2021
10 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicolas-grekas nicolas-grekas

@stof stof

@welcoMattic welcoMattic

Assignees
No one assigned
Labels
Bug Status: Reviewed Translation
Projects
None yet
Milestone
5.3
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
@Kocal @carsonbot @nicolas-grekas @stof @welcoMattic