merged branch jfsimon/issue-5940 (PR #7304)

This PR was squashed before being merged into the 2.1 branch (closes #7304). Commits ------- 5bc30bb [Translation] added xliff loader/dumper with resname support Discussion ---------- [Translation] added xliff loader/dumper with resname support | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #5940 --------------------------------------------------------------------------- by jfsimon at 2013-03-08T15:49:18Z @Tobion what do you think of `XliffFileWithResnameDumper` and `XliffFileWithResnameLoader`? --------------------------------------------------------------------------- by robinduval at 2013-03-08T15:49:58Z 💯 great idea ! --------------------------------------------------------------------------- by lizjulien at 2013-03-08T15:51:45Z 🌟 better. --------------------------------------------------------------------------- by saro0h at 2013-03-08T15:52:50Z 👍 Can't be worse than the previous one --------------------------------------------------------------------------- by dxb at 2013-03-08T16:00:32Z +1 --------------------------------------------------------------------------- by Tobion at 2013-03-08T16:07:59Z @jfsimon yes the name make sense. It is not possible to add this feature without BC break? And if not, why not simply add an option/enable method to the existing classes. Having new classes for this seems awkward to me. --------------------------------------------------------------------------- by jfsimon at 2013-03-08T16:17:44Z @Tobion I totaly agree with you, but this has been discussed in #5940. Does the presence of the `resname` attribute in the `trans-unit` tags represent a BC break? Would opening another PR with this solution be a good idea? --------------------------------------------------------------------------- by stof at 2013-03-08T17:01:00Z Your XliffFileWithResnameLoader is able to load all files supported by XliffFileLoader so IMO you don't need to add a new class. --------------------------------------------------------------------------- by jfsimon at 2013-03-08T18:21:32Z @stof would the `resname` attribute addition in the dumped XLIFF be considered as a BC break? --------------------------------------------------------------------------- by jfsimon at 2013-03-09T13:16:32Z @stof done. @mvrhov done.
2013-03-12 12:04:38 +01:00 · 2013-03-12 12:04:38 +01:00 · d77b97cb07
commit d77b97cb07
parent e508ecc729 5bc30bb98a
5 changed files with 64 additions and 17 deletions
--- a/src/Symfony/Component/Translation/Dumper/XliffFileDumper.php
+++ b/src/Symfony/Component/Translation/Dumper/XliffFileDumper.php
@ -27,24 +27,30 @@ class XliffFileDumper extends FileDumper
    {
        $dom = new \DOMDocument('1.0', 'utf-8');
        $dom->formatOutput = true;
+
        $xliff = $dom->appendChild($dom->createElement('xliff'));
        $xliff->setAttribute('version', '1.2');
        $xliff->setAttribute('xmlns', 'urn:oasis:names:tc:xliff:document:1.2');
+
        $xliffFile = $xliff->appendChild($dom->createElement('file'));
        $xliffFile->setAttribute('source-language', $messages->getLocale());
        $xliffFile->setAttribute('datatype', 'plaintext');
        $xliffFile->setAttribute('original', 'file.ext');
+
        $xliffBody = $xliffFile->appendChild($dom->createElement('body'));
-        $id = 1;
        foreach ($messages->all($domain) as $source => $target) {
-            $trans = $dom->createElement('trans-unit');
-            $trans->setAttribute('id', $id);
-            $s = $trans->appendChild($dom->createElement('source'));
+            $translation = $dom->createElement('trans-unit');
+
+            $translation->setAttribute('id', md5($source));
+            $translation->setAttribute('resname', $source);
+
+            $s = $translation->appendChild($dom->createElement('source'));
            $s->appendChild($dom->createTextNode($source));
-            $t = $trans->appendChild($dom->createElement('target'));
+
+            $t = $translation->appendChild($dom->createElement('target'));
            $t->appendChild($dom->createTextNode($target));
-            $xliffBody->appendChild($trans);
-            $id++;
+
+            $xliffBody->appendChild($translation);
        }

        return $dom->saveXML();
--- a/src/Symfony/Component/Translation/Loader/XliffFileLoader.php
+++ b/src/Symfony/Component/Translation/Loader/XliffFileLoader.php
@ -11,6 +11,7 @@

 namespace Symfony\Component\Translation\Loader;

+use Symfony\Component\DependencyInjection\SimpleXMLElement;
 use Symfony\Component\Translation\MessageCatalogue;
 use Symfony\Component\Config\Resource\FileResource;

@ -39,10 +40,14 @@ class XliffFileLoader implements LoaderInterface

        $catalogue = new MessageCatalogue($locale);
        foreach ($xml->xpath('//xliff:trans-unit') as $translation) {
-            if (!isset($translation->source) || !isset($translation->target)) {
+            $attributes = $translation->attributes();
+
+            if (!(isset($attributes['resname']) || isset($translation->source)) || !isset($translation->target)) {
                continue;
            }
-            $catalogue->set((string) $translation->source, (string) $translation->target, $domain);
+
+            $source = isset($attributes['resname']) && $attributes['resname'] ? $attributes['resname'] : $translation->source;
+            $catalogue->set((string) $source, (string) $translation->target, $domain);
        }
        $catalogue->addResource(new FileResource($resource));

@ -54,6 +59,8 @@ class XliffFileLoader implements LoaderInterface
     *
     * @param string $file
     *
+     * @throws \RuntimeException
+     *
     * @return \SimpleXMLElement
     */
    private function parseFile($file)
@ -109,6 +116,8 @@ class XliffFileLoader implements LoaderInterface
    /**
     * Returns the XML errors of the internal XML parser
     *
+     * @param boolean $internalErrors
+     *
     * @return array An array of errors
     */
    private function getXmlErrors($internalErrors)
--- a/src/Symfony/Component/Translation/Tests/Loader/XliffFileLoaderTest.php
+++ b/src/Symfony/Component/Translation/Tests/Loader/XliffFileLoaderTest.php
@ -25,7 +25,7 @@ class XliffFileLoaderTest extends \PHPUnit_Framework_TestCase

    public function testLoad()
    {
-        $loader = new XliffFileLoader();
+        $loader = $this->createLoader();
        $resource = __DIR__.'/../fixtures/resources.xlf';
        $catalogue = $loader->load($resource, 'en', 'domain1');

@ -33,9 +33,17 @@ class XliffFileLoaderTest extends \PHPUnit_Framework_TestCase
        $this->assertEquals(array(new FileResource($resource)), $catalogue->getResources());
    }

+    public function testLoadWithResname()
+    {
+        $loader = $this->createLoader();
+        $catalogue = $loader->load(__DIR__.'/../fixtures/resname.xlf', 'en', 'domain1');
+
+        $this->assertEquals(array('foo' => 'bar', 'bar' => 'baz', 'baz' => 'foo'), $catalogue->all('domain1'));
+    }
+
    public function testIncompleteResource()
    {
-        $loader = new XliffFileLoader();
+        $loader = $this->createLoader();
        $catalogue = $loader->load(__DIR__.'/../fixtures/resources.xlf', 'en', 'domain1');

        $this->assertEquals(array('foo' => 'bar', 'key' => '', 'test' => 'with'), $catalogue->all('domain1'));
@ -47,7 +55,7 @@ class XliffFileLoaderTest extends \PHPUnit_Framework_TestCase
     */
    public function testLoadInvalidResource()
    {
-        $loader = new XliffFileLoader();
+        $loader = $this->createLoader();
        $catalogue = $loader->load(__DIR__.'/../fixtures/resources.php', 'en', 'domain1');
    }

@ -56,7 +64,7 @@ class XliffFileLoaderTest extends \PHPUnit_Framework_TestCase
     */
    public function testLoadResourceDoesNotValidate()
    {
-        $loader = new XliffFileLoader();
+        $loader = $this->createLoader();
        $catalogue = $loader->load(__DIR__.'/../fixtures/non-valid.xlf', 'en', 'domain1');
    }

@ -65,7 +73,7 @@ class XliffFileLoaderTest extends \PHPUnit_Framework_TestCase
     */
    public function testLoadThrowsAnExceptionIfFileNotLocal()
    {
-        $loader = new XliffFileLoader();
+        $loader = $this->createLoader();
        $resource = 'http://example.com/resources.xlf';
        $loader->load($resource, 'en', 'domain1');
    }
@ -76,7 +84,12 @@ class XliffFileLoaderTest extends \PHPUnit_Framework_TestCase
     */
    public function testDocTypeIsNotAllowed()
    {
-        $loader = new XliffFileLoader();
+        $loader = $this->createLoader();
        $loader->load(__DIR__.'/../fixtures/withdoctype.xlf', 'en', 'domain1');
    }
+
+    protected function createLoader()
+    {
+        return new XliffFileLoader();
+    }
 }
--- a/src/Symfony/Component/Translation/Tests/fixtures/resname.xlf
+++ b/src/Symfony/Component/Translation/Tests/fixtures/resname.xlf
@ -0,0 +1,19 @@
+<?xml version="1.0" encoding="utf-8"?>
+<xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" version="1.2">
+  <file source-language="en" datatype="plaintext" original="file.ext">
+    <body>
+      <trans-unit id="1" resname="foo">
+        <source></source>
+        <target>bar</target>
+      </trans-unit>
+      <trans-unit id="2" resname="bar">
+        <source>bar source</source>
+        <target>baz</target>
+      </trans-unit>
+      <trans-unit id="3">
+        <source>baz</source>
+        <target>foo</target>
+      </trans-unit>
+    </body>
+  </file>
+</xliff>
--- a/src/Symfony/Component/Translation/Tests/fixtures/resources-clean.xlf
+++ b/src/Symfony/Component/Translation/Tests/fixtures/resources-clean.xlf
@ -2,11 +2,11 @@
 <xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" version="1.2">
  <file source-language="en" datatype="plaintext" original="file.ext">
    <body>
-      <trans-unit id="1">
+      <trans-unit id="acbd18db4cc2f85cedef654fccc4a4d8" resname="foo">
        <source>foo</source>
        <target>bar</target>
      </trans-unit>
-      <trans-unit id="2">
+      <trans-unit id="3c6e0b8a9c15224a8228b9a98ca1531d" resname="key">
        <source>key</source>
        <target></target>
      </trans-unit>