Skip to content

[Translator][Loader] added XLIFF 2.0 support. #15717

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 5 commits into from
Sep 16, 2015
Merged

Conversation

aitboudad
Copy link
Contributor

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Fixed tickets #11853, #12853
Tests pass? yes
License MIT

// simple_xml will always return utf-8 encoded values
$target = $this->utf8ToCharset((string) (isset($translation->target) ? $translation->target : $source), $encoding);

$catalogue->set((string) $source, $target, $domain);
Copy link
Member

Choose a reason for hiding this comment

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

support for notes and target attributes should be added for XLiff 2 for the equivalent features

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok I'll look into it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I only added support for target attributes
but for notes there's no way to do a mapping because aren't assigned to specific segments, but to whole units.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@stof what do you think to take account of notes only when a unit contains one segment and ignore other cases ?

<unit id="1">
    <notes>
        <note priority="1" appliesTo="target">foo</note>
    </notes>
    <segment>
        <source>foo</source>
        <target>foo</target>
    </segment>
</unit>

use Symfony\Component\Config\Resource\FileResource;
use Symfony\Component\Translation\Loader\XliffFileLoader;
Copy link
Contributor

Choose a reason for hiding this comment

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

Symfony use the ordered use statements?

Otherwise i would revert this line

Copy link
Member

Choose a reason for hiding this comment

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

Indeed, we do not re-sort existing use statements to avoid merging issues

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 thank you!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

reverted

@stof
Copy link
Member

stof commented Sep 8, 2015

Support for Xliff2 should also be added in the dumper

@stof
Copy link
Member

stof commented Sep 8, 2015

should we have support for <cp hex="HHHH"> for chars which are not allowed in XML itself ?

@aitboudad
Copy link
Contributor Author

@stof we can add support for <cp hex="HHHH"> later in another PR.

Ping @symfony/deciders

@fabpot
Copy link
Member

fabpot commented Sep 15, 2015

👍

@aitboudad
Copy link
Contributor Author

Thank you @xphere.

@aitboudad aitboudad merged commit 0c24d55 into symfony:2.8 Sep 16, 2015
aitboudad added a commit that referenced this pull request Sep 16, 2015
… aitboudad)

This PR was merged into the 2.8 branch.

Discussion
----------

[Translator][Loader] added XLIFF 2.0 support.

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Fixed tickets  | #11853, #12853
| Tests pass?   | yes
| License       | MIT

Commits
-------

0c24d55 [Translation][Dumper] added XLIFF 2.0 support.
7af4fc7 [XLIFF 2.0] added support for target attributes.
ace6042 apply some fixes.
ce540ae update changelog.
ff5d6a3 [Translation][Loader] added XLIFF 2.0 support.
@aitboudad aitboudad deleted the pr_12853 branch September 16, 2015 14:52
@fabpot fabpot mentioned this pull request Nov 16, 2015
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.

6 participants