When exporting po file templates, with the default option (Source text only, no translation) I've found these issues:
1. We get duplicated strings.
To get this, you just need to add two languages and add a translation of the same string for each.
Looking into the query, it seems either there's one DISTINCT missing or one join not needed (we wouln't need to join locales_target here).
2. Then, reloading the page, we get multiple warnings like these:
Warning: addcslashes() expects parameter 1 to be string, array given in Drupal\Component\Gettext\PoItem->formatString() (line 269 of core/lib/Drupal/Component/Gettext/PoItem.php).
I suspect this last one is related to the singular / plural handling in PoItem class.
Comment | File | Size | Author |
---|---|---|---|
#20 | 1789090-20.patch | 2.25 KB | vijaycs85 |
#20 | 1789090-diff-18-20.txt | 653 bytes | vijaycs85 |
Comments
Comment #1
Gábor HojtsyAdding D8MI tags.
Comment #2
Jose Reyero CreditAttribution: Jose Reyero commentedThe 'duplicated strings' issue will be fixed by #1785086: Introduce a generic API for interface translation strings
Comment #3
Jose Reyero CreditAttribution: Jose Reyero commentedSo, duplicated strings fixed, now there's only the export warnings.
Comment #4
Sutharsan CreditAttribution: Sutharsan commentedUpdating title to match the issue scope.
Comment #5
Outi CreditAttribution: Outi commentedJust to notice that the problem Jose Reyero describes still occurs – in the end of the exportation of a .po file, I get this message:
Warning: addcslashes() expects parameter 1 to be string, array given in Drupal\Component\Gettext\PoItem->formatString() (line 269 of core/lib/Drupal/Component/Gettext/PoItem.php).
Comment #6
Gábor HojtsyOh, sounds like an issue with plural exports? can you look in the exported .po file if there are msgid and msgid_plural items that are empty? Or suspicious in some way?
Comment #7
Outi CreditAttribution: Outi commentedWe checked with Gábor Hojtsy the msgid and msgid_plural items and they seemed to be normal.
I wanted to add that I had this problem in these conditions:
- I installed the site in French
- on admin/config/regional/translate I translated some strings from English to French
- on admin/config/regional/translate/export I exported these translations on a .po file
- I go the error message
The problem doesn't occur when
- I install the site in English
- I don't translate anything and on admin/config/regional/translate/export I have this message: "No language available. The export will only contain source strings."
- I click anyway on the "export" link and get the drupal.pot file
- I don't have the error message
Comment #8
droplet CreditAttribution: droplet commentedAlso, symfony required php_fileinfo ext to output to files. Is there somewhere we can/should document it ?
Comment #11
Jose Reyero CreditAttribution: Jose Reyero at Axel Springer España commentedI've found these warnings again when exporting translations (Spanish). This happened using a custom script though, so not sure where this fixes the core issue too.
Apparently this was happening with plural strings that had just one of them translated (instead of the two) -- We have two plural forms in Spanish, just like in English.
When setting the values with PoItem::setFromArray(), it was checking whether there were more than one translations to set it as plural or not. The patch changes this to check whether there are more than one source strings --instead of translations.
In the resulting po trings that were not properly exported before resulting in
+ Warning
Are now exported as:
One line patch, easy to review !! :-)
Comment #13
Jose Reyero CreditAttribution: Jose Reyero at Axel Springer España commentedRe-rolled patch with the right root folder so testbot can apply it.
Comment #14
Gábor HojtsyA test would be great for this one. Also the issue summary has one more issue explained. Is that still in the scope here?
Comment #15
Jose Reyero CreditAttribution: Jose Reyero at Axel Springer España commentedAbout the first issue (1. Duplicated strings) it should be already fixed, see #2
About the second one (2. Warnings when exporting pot), not exactly the same than "when exporting translations", but this patch fixes, it is the same issue AFAIK. With the patch applied, warnings are gone.
So I think this one is almost RTBC, it just needs that tests -no time for them myself atm.
Comment #16
droplet CreditAttribution: droplet commentedDoesn't it same to my patch #8 ?
Comment #17
Jose Reyero CreditAttribution: Jose Reyero at Axel Springer España commented@droplet,
Right, I think it is the same and I see you've also added some improved tests. Though it seems those tests don't check for this specific issue.
So if you could update the patch for it to apply cleanly, and maybe also add a test for this one, that would be great. (I think just adding some string that doesn't have both plural forms translated would do).
Comment #18
vijaycs85re-roll of #8
Comment #19
Jose Reyero CreditAttribution: Jose Reyero at Axel Springer España commentedLatest patch looks good to me, just found a typo, otherwise it's RTBC
+ * A .po file fragment with a translated string.
Comment #20
vijaycs85Updating...
Comment #21
Gábor HojtsyYay, let's get this in then! Thanks all!
Comment #22
Gábor HojtsyComment #24
Gábor HojtsyFail on migratetermnodetest.
Comment #26
alexpottJust applying the test and running it does not result in fail so whatever bug we're fixing here is not tested yet by the test.
Comment #27
Jose Reyero CreditAttribution: Jose Reyero at Axel Springer España commentedAbout the test, I think what we need to test is what happens when you only add one of the two plural translations, then click on save, then export the po.
Comment #28
alexpottFYI for reasons I don't understand we've managed to replicate this bug in #2557113: Make t() return a TranslationWrapper object to remove reliance on a static, unpredictable safe list
Comment #29
rodrigoaguileraThis has been fixed at #2557113: Make t() return a TranslationWrapper object to remove reliance on a static, unpredictable safe list and I also can't reproduce the duplicated strings nor the warnings.
I don't know if the test makes sense if we cannot make it fail.
Comment #30
Gábor HojtsyYeah the PoItem change is there. Closing as duplicate instead then. It is unfortunate that credit from this issue did not make it to https://www.drupal.org/commitlog/commit/2/e7a7de9f8cf29bb5d4593eccd44659... :/