Added option to keep initial translated messages

[clytemnestra]

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes/no
Fixed tickets
License	Apache2

Description

I've remade this PR on branch master, without whitespace changes.

The idea was to do this, so I added an option --keeptm which will keep your already defined translation messages.

Todos

• [ ] Tests
• [ ] Documentation
• [ ] Changelog

[gnat42]

Looks interesting. One small nitpick that I left in the comments about the variable name. I'd be ready to merge this feature except it is missing tests and it would also be nice to have a little documentation added about the switch.

Thanks for the PR.

[clytemnestra]

Should I change everywhere from plural to singular? Since it's everywhere as plural, not only those 2 situations you showed me. Or you want only those 2 cases?

I'll be adding docs soon, as for tests I don't know exactly where to write them and how to run them.

[gnat42]

Yeah all the function names should be adjusted accordingly once the variable name is changed.

[gnat42]

As for tests, there are some already in the Tests directory, I would suggest there should be a Tests/Translation/UpdaterTest.php file created that runs one or two tests with the switch on and off, ensuring that the output is as expected. You may have to create a fixture file someplace there to check against.

[clytemnestra]

Well if you have a project that you haven't ran the command on just yet, and you run it you will get

'button.delete' ='button.delete'

instead of

'button.delete' ='Delete'

All your messages will be replaced with their keys. My method however, it first translates the key and sets it as the message.

[clytemnestra]

Hm, I don't seem to find the cookbook docs on the website? They should be added, they contain useful info.

Or you need access from jms?

[clytemnestra]

I wanted to run the current tests before adding my own to see if it works. I'm running phpunit Tests and getting:

PHPUnit 3.7.21 by Sebastian Bergmann.

Configuration read from E:\test\JMSTranslationBundle\phpunit.xml.dist

...............................................................  63 / 153 ( 41%)
...................................................F..........F 126 / 153 ( 82%)
...........................

Time: 0 seconds, Memory: 26.25Mb

There were 2 failures:

1) JMS\TranslationBundle\Tests\Translation\Extractor\FileExtractorTest::testExtractWithSimpleTestFixtures
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
             0 => JMS\TranslationBundle\Model\FileSource Object (
-                'path' => 'E:\test\JMSTranslationBundle\Tests\Translation\Extractor/Fixture/SimpleTest/Controller/DefaultController.php'
+                'path' => 'E:\test\JMSTranslationBundle\Tests\Translation\Extractor/Fixture/SimpleTest\Controller\DefaultController.php'

@@ @@
             0 => JMS\TranslationBundle\Model\FileSource Object (
-                'path' => 'E:\test\JMSTranslationBundle\Tests\Translation\Extractor/Fixture/SimpleTest/GlobalNamespace.php'
+                'path' => 'E:\test\JMSTranslationBundle\Tests\Translation\Extractor/Fixture/SimpleTest\GlobalNamespace.php'

@@ @@
             0 => JMS\TranslationBundle\Model\FileSource Object (
-                'path' => 'E:\test\JMSTranslationBundle\Tests\Translation\Extractor/Fixture/SimpleTest/Resources/views/php_template.html.php'
+                'path' => 'E:\test\JMSTranslationBundle\Tests\Translation\Extractor/Fixture/SimpleTest\Resources\views\php_template.html.php'

@@ @@
             0 => JMS\TranslationBundle\Model\FileSource Object (
-                'path' => 'E:\test\JMSTranslationBundle\Tests\Translation\Extractor/Fixture/SimpleTest/Resources/views/php_template.html.php'
+                'path' => 'E:\test\JMSTranslationBundle\Tests\Translation\Extractor/Fixture/SimpleTest\Resources\views\php_template.html.php'

@@ @@
             0 => JMS\TranslationBundle\Model\FileSource Object (
-                'path' => 'E:\test\JMSTranslationBundle\Tests\Translation\Extractor/Fixture/SimpleTest/Resources/views/php_template.html.php'
+                'path' => 'E:\test\JMSTranslationBundle\Tests\Translation\Extractor/Fixture/SimpleTest\Resources\views\php_template.html.php'

@@ @@
             0 => JMS\TranslationBundle\Model\FileSource Object (
-                'path' => 'E:\test\JMSTranslationBundle\Tests\Translation\Extractor/Fixture/SimpleTest/Resources/views/php_template.html.php'
+                'path' => 'E:\test\JMSTranslationBundle\Tests\Translation\Extractor/Fixture/SimpleTest\Resources\views\php_template.html.php'

@@ @@
             0 => JMS\TranslationBundle\Model\FileSource Object (
-                'path' => 'E:\test\JMSTranslationBundle\Tests\Translation\Extractor/Fixture/SimpleTest/Resources/views/twig_template.html.twig'
+                'path' => 'E:\test\JMSTranslationBundle\Tests\Translation\Extractor/Fixture/SimpleTest\Resources\views\twig_template.html.twig'

@@ @@
             0 => JMS\TranslationBundle\Model\FileSource Object (
-                'path' => 'E:\test\JMSTranslationBundle\Tests\Translation\Extractor/Fixture/SimpleTest/Resources/views/twig_template.html.twig'
+                'path' => 'E:\test\JMSTranslationBundle\Tests\Translation\Extractor/Fixture/SimpleTest\Resources\views\twig_template.html.twig'

@@ @@
             0 => JMS\TranslationBundle\Model\FileSource Object (
-                'path' => 'E:\test\JMSTranslationBundle\Tests\Translation\Extractor/Fixture/SimpleTest/Resources/views/twig_template.html.twig'
+                'path' => 'E:\test\JMSTranslationBundle\Tests\Translation\Extractor/Fixture/SimpleTest\Resources\views\twig_template.html.twig'

@@ @@
             0 => JMS\TranslationBundle\Model\FileSource Object (
-                'path' => 'E:\test\JMSTranslationBundle\Tests\Translation\Extractor/Fixture/SimpleTest/Resources/views/twig_template.html.twig'
+                'path' => 'E:\test\JMSTranslationBundle\Tests\Translation\Extractor/Fixture/SimpleTest\Resources\views\twig_template.html.twig'
                 'line' => 7
                 'column' => null
             )
         )
     )
 )

E:\test\JMSTranslationBundle\Tests\Translation\Extractor\FileExtractorTest.php:91

2) JMS\TranslationBundle\Tests\Translation\Extractor\File\TranslationContainerExtractorTest::testExtractFormModel
Failed asserting that two objects are equal.
--- Expected
+++ Actual
@@ @@
                         0 => JMS\TranslationBundle\Model\FileSource Object (
-                            'path' => 'E:\test\JMSTranslationBundle\Tests\Translation\Extractor\File/Fixture/MyFormModel.php'
+                            'path' => 'E:\test\JMSTranslationBundle\Tests\Translation\Extractor\File\Fixture\MyFormModel.php'

@@ @@
                         0 => JMS\TranslationBundle\Model\FileSource Object (
-                            'path' => 'E:\test\JMSTranslationBundle\Tests\Translation\Extractor\File/Fixture/MyFormModel.php'
+                            'path' => 'E:\test\JMSTranslationBundle\Tests\Translation\Extractor\File\Fixture\MyFormModel.php'
                             'line' => 13
                             'column' => null
                         )
                     )
                 )
             )
         )
     )
 )

E:\test\JMSTranslationBundle\Tests\Translation\Extractor\File\TranslationContainerExtractorTest.php:45

Could it be because I'm on Windows? I'd like to have these work before trying to add my own. Any idea on what should I do?

[gnat42]

Hey @Nyholm should we get a PR that uses DIRECTORY_SEPARATOR so that tests function on windows when dealing with the paths?

[clytemnestra]

I think I found that DIRECTORY_SEPARATOR might be a solution for this problem too, but thought of it to be troublesome for me to try and do it properly. I'm going to write tests for these as soon as I'll get on a linux machine.

[Nyholm]

I have heard that newer versions of Windows (maybe Windows server 2013) handles this properly.

@clytemnestra, what OS are you running?

[clytemnestra]

That was on Win10 with XAMPP.

[Nyholm]

I have review this but I do not understand the problem. Or, I can reproduce it.

I have a project where I do not use this bundle. The project has a lot of translations in /app/Resources/translations

1. I installed the dev-master version of this bundle
2. Adding the config from the cookbook (see below)
3. I run php app/console translation:extract sv --config=app
4. My messages.sv.xlf was modfied with some extra translations (like 5) and all trans-units had at least one jms:reference-file tag. All translations where still in place.

My config:

jms_translation:
    locales: ['en', 'sv'] # List of locales supported by your application
    source_language: 'en' # The language that the sources is written in
    configs:
        app:
            dirs: ["%kernel.root_dir%", "%kernel.root_dir%/../src"]
            output_dir: "%kernel.root_dir%/Resources/translations"
            ignored_domains: [routes]
            excluded_names: ["*TestCase.php", "*Test.php"]
            excluded_dirs: [cache, data, logs, Tests]

Can you give me a way to reproduce this error where all translations is gone when using the bundle on an existing project?

[clytemnestra]

That's what I also did on a project that had translations not in app/Resources, but in bundles. The output was sent in app/Resources as xliff, but the keys were put as the translation value. The page wasn't chaging unless I deleted cache, even in dev mode.

I'll try tomorrow to test again and see if it still happens, maybe it was an issue with an old version.

[clytemnestra]

Yeah, same issue.

jms_translation:
    configs:
        app:
            dirs: [%kernel.root_dir%, %kernel.root_dir%/../src]
            output_dir: %kernel.root_dir%/Resources/translations
            ignored_domains: [routes]
            excluded_names: ["*TestCase.php", "*Test.php"]
            excluded_dirs: [cache, data, logs]
            extractors: [alias_of_the_extractor]

 translation:extract en --dir=./src/ --output-dir=./app/Resources/translations

And after deleting the cache I still get keys instead of messages.

Source:

fos_user:
    username:
        already_used: The username is already used
        blank: Please enter a username
        short: "[-Inf,Inf]The username is too short"
        long: "[-Inf,Inf]The username is too long"
    email:
        already_used: The email is already used
        blank: Please enter an email
        short: "[-Inf,Inf]The email is too short"
        long: "[-Inf,Inf]The email is too long"
        invalid: The email is not valid
    password:
        blank: Please enter a password
        short: "[-Inf,Inf]The password is too short"
        mismatch: The entered passwords don't match
    new_password:
        blank: Please enter a new password
        short: "[-Inf,Inf]The new password is too short"
    current_password:
        invalid: The entered password is invalid
    group:
        blank: Please enter a name
        short: "[-Inf,Inf]The name is too short"
        long: "[-Inf,Inf]The name is too long"

private_message:
    title:
        blank: Please enter a title
    content:
        blank: Please enter a message
    receiver:
        blank: Please enter a user name to send the message to
        invalid: The entered user is invalid
        himself: You can't send a message to yourself

review:
    comment:
        blank: Please enter a review
    rate:
        blank: Please enter a rate

user:
    firstname:
        blank: Please enter a firstname
    lastname:
        blank: Please enter a lastname

medic:
    age:
        diffAge: The difference between Age and Real Age must be at most 10 years.
    spokenLanguage:
        blank: Please select at least a spoken language

features:
    title:
        blank: Please enter a title
    details:
        blank: Please enter a comment

ticket:
    title:
        blank: Please enter a title
    message:
        blank: Please enter a message

Result:

<?xml version="1.0" encoding="utf-8"?>
<xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:jms="urn:jms:translation" version="1.2">
  <file date="2016-07-08T13:11:35Z" source-language="en" target-language="en" datatype="plaintext" original="not.available">
    <header>
      <tool tool-id="JMSTranslationBundle" tool-name="JMSTranslationBundle" tool-version="1.1.0-DEV"/>
      <note>The source node in most cases contains the sample message as written by the developer. If it looks like a dot-delimitted string such as "form.label.firstname", then the developer has not provided a default message.</note>
    </header>
    <body>
      <trans-unit id="a29d6d5aa5131805e60ceac186b93963b5647b68" resname="The password fields must match.">
        <source>The password fields must match.</source>
        <target state="new">The password fields must match.</target>
        <jms:reference-file line="102">./src\MedAppBundle\Form\FirmProfileType.php</jms:reference-file>
        <jms:reference-file line="90">./src\MedAppBundle\Form\MedicProfileType.php</jms:reference-file>
        <jms:reference-file line="66">./src\MedAppBundle\Form\ProfileType.php</jms:reference-file>
      </trans-unit>
      <trans-unit id="9f4795878a06937ae3795f529c16e1d17665430b" resname="features.details.blank">
        <source>features.details.blank</source>
        <target state="new">features.details.blank</target>
      </trans-unit>
      <trans-unit id="81c89c8d083b8df4e9f7cc9c84f4d98d3db8eb4b" resname="features.title.blank">
        <source>features.title.blank</source>
        <target state="new">features.title.blank</target>
      </trans-unit>
      <trans-unit id="f48ccf84018671218aac090abf90770212497b70" resname="fos_user.email.already_used">
        <source>fos_user.email.already_used</source>
        <target state="new">fos_user.email.already_used</target>
      </trans-unit>
      <trans-unit id="d0e28536be754aebc7ef73cdc6cf0d729311ce1d" resname="fos_user.email.blank">
        <source>fos_user.email.blank</source>
        <target state="new">fos_user.email.blank</target>
      </trans-unit>
      <trans-unit id="17a097b4a496b3378a290ea4f8a05bc2a7879142" resname="fos_user.email.invalid">
        <source>fos_user.email.invalid</source>
        <target state="new">fos_user.email.invalid</target>
      </trans-unit>
      <trans-unit id="9760d0f9c28fee7f91e5cad9e282921b13cd2f0b" resname="fos_user.email.long">
        <source>fos_user.email.long</source>
        <target state="new">fos_user.email.long</target>
      </trans-unit>
      <trans-unit id="1a1b926521b2b3010b91de1787f9265b3d6a97ae" resname="fos_user.email.short">
        <source>fos_user.email.short</source>
        <target state="new">fos_user.email.short</target>
      </trans-unit>
      <trans-unit id="a5a6b14a2360fe5c9876c4f2746ce5eace79b37d" resname="fos_user.group.blank">
        <source>fos_user.group.blank</source>
        <target state="new">fos_user.group.blank</target>
      </trans-unit>
      <trans-unit id="a85da9132715da5c672fe03d4cbb3f9ecbcfb115" resname="fos_user.group.long">
        <source>fos_user.group.long</source>
        <target state="new">fos_user.group.long</target>
      </trans-unit>
      <trans-unit id="ca13a192c80234ef7ffc377e3a7f2926c9d6561b" resname="fos_user.group.short">
        <source>fos_user.group.short</source>
        <target state="new">fos_user.group.short</target>
      </trans-unit>
      <trans-unit id="1df43a6e24af03c39d20b2f1f5cbefdef7b30416" resname="fos_user.password.blank">
        <source>fos_user.password.blank</source>
        <target state="new">fos_user.password.blank</target>
      </trans-unit>
      <trans-unit id="4b3130df261ac66147653997d2d83952a330faac" resname="fos_user.password.short">
        <source>fos_user.password.short</source>
        <target state="new">fos_user.password.short</target>
      </trans-unit>
      <trans-unit id="77c8ec07f205976bbed1a3ce90fdc458891bb81d" resname="fos_user.username.already_used">
        <source>fos_user.username.already_used</source>
        <target state="new">fos_user.username.already_used</target>
      </trans-unit>
      <trans-unit id="312a20b0239a4236f4b08e4e0ed35153a03667d0" resname="fos_user.username.blank">
        <source>fos_user.username.blank</source>
        <target state="new">fos_user.username.blank</target>
      </trans-unit>
      <trans-unit id="56ef686ed0b86d02dd4077e0796a4f1e61377552" resname="fos_user.username.long">
        <source>fos_user.username.long</source>
        <target state="new">fos_user.username.long</target>
      </trans-unit>
      <trans-unit id="920e8d37f8a7c923ece7d123d55668ac522d612d" resname="fos_user.username.short">
        <source>fos_user.username.short</source>
        <target state="new">fos_user.username.short</target>
      </trans-unit>
      <trans-unit id="5a5f76ab44184c48b8e6ad3fe4d7f647b26a5779" resname="medic.age.diffAge">
        <source>medic.age.diffAge</source>
        <target state="new">medic.age.diffAge</target>
      </trans-unit>
      <trans-unit id="be2a5bd31059aae54366296cbbb34a9a87b68050" resname="medic.spokenLanguage.blank">
        <source>medic.spokenLanguage.blank</source>
        <target state="new">medic.spokenLanguage.blank</target>
      </trans-unit>
      <trans-unit id="2c0b778d469245a389459a2ae6d1fd78eb8aa813" resname="private_message.content.blank">
        <source>private_message.content.blank</source>
        <target state="new">private_message.content.blank</target>
      </trans-unit>
      <trans-unit id="5c0f73e8bbeb76e0347ddac6f776fb4c249c9191" resname="private_message.receiver.blank">
        <source>private_message.receiver.blank</source>
        <target state="new">private_message.receiver.blank</target>
      </trans-unit>
      <trans-unit id="d56e004b458c53120cd5bc3cb7d31df1a756909c" resname="private_message.receiver.himself">
        <source>private_message.receiver.himself</source>
        <target state="new">private_message.receiver.himself</target>
      </trans-unit>
      <trans-unit id="2a1f15d1731346877a7b36fd2c2f7b2b768fb20b" resname="private_message.receiver.invalid">
        <source>private_message.receiver.invalid</source>
        <target state="new">private_message.receiver.invalid</target>
        <jms:reference-file line="36">./src\PrivateMessageBundle\Form\MessageType.php</jms:reference-file>
      </trans-unit>
      <trans-unit id="1d61219fbbac9d4695d79fb80614217e678e9c9d" resname="private_message.title.blank">
        <source>private_message.title.blank</source>
        <target state="new">private_message.title.blank</target>
      </trans-unit>
      <trans-unit id="4029930caaef577f89eaf9a59cf282275f18a5a0" resname="review.comment.blank">
        <source>review.comment.blank</source>
        <target state="new">review.comment.blank</target>
      </trans-unit>
      <trans-unit id="147f42c29fd69d3b2558b0b1a376c406d7c96177" resname="review.rate.blank">
        <source>review.rate.blank</source>
        <target state="new">review.rate.blank</target>
      </trans-unit>
      <trans-unit id="752ac7512c8370bd6c426479ce60aba5c26c7cd5" resname="ticket.message.blank">
        <source>ticket.message.blank</source>
        <target state="new">ticket.message.blank</target>
      </trans-unit>
      <trans-unit id="a07a71af04119324b3623048b911a85fb59d3ae0" resname="ticket.title.blank">
        <source>ticket.title.blank</source>
        <target state="new">ticket.title.blank</target>
      </trans-unit>
      <trans-unit id="fcc92be641521c9821befc14582cb776ddfd9d10" resname="user.firstname.blank">
        <source>user.firstname.blank</source>
        <target state="new">user.firstname.blank</target>
      </trans-unit>
      <trans-unit id="f2ecd22264a9810977e548e4106fbf6ce4cc9397" resname="user.lastname.blank">
        <source>user.lastname.blank</source>
        <target state="new">user.lastname.blank</target>
      </trans-unit>
    </body>
  </file>
</xliff>

[Nyholm]

What if you add output format to your config?

jms_translation:
    configs:
        app:
            dirs: [%kernel.root_dir%, %kernel.root_dir%/../src]
            output_dir: %kernel.root_dir%/Resources/translations
            ignored_domains: [routes]
            excluded_names: ["*TestCase.php", "*Test.php"]
            excluded_dirs: [cache, data, logs]
            extractors: [alias_of_the_extractor]
            output_format: "yaml"

[clytemnestra]

Same result. Just got .xlf instead of .xliff

[Nyholm]

What if you use the "keep"?

 translation:extract en --keep --dir=./src/ --output-dir=./app/Resources/translations

[clytemnestra]

I thought --keep is for keeping translation keys in the already generated files that have been removed from the project, so they don't get removed from the file, too. I'll try to run it tomorrow and see what it does, but doubt it'll solve the problem.

[clytemnestra]

Yup, won't work.

The thing is nowhere is the translator service used to translate the keys, they are just extracted and inserted in the file as is. I've applied a translation to the keys before they are added in the file, that's all.

[murhafh]

It is strange, this also works for me, whenever I run the extract command the old translations stay in place, only the new strings are added to the file, but no translations are removed.

[htm52]

Hi,

I'm really interested in this feature, will this PR be merged anytime soon ?