Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.The current version of localization server for Drupal 6 (running on localize.drupal.org) needs to go to great length to try and protect itself from broken .po files imported via deployed modules (modules actually running under the site). Drupal so nicely imports all translation files when enabling modules, but if any of those have a broken plural formula, it breaks plurals after the import. So I think we should try protecting the plural forms just like the existing strings when the KEEP option is used.
Since the automated import uses the KEEP mode when importing, this should protect a properly set up plural form.
The only issue here is that since most people would experience their first .po import via an automated import, they would not get any plural form set up. So I've added a clause to override the KEEP option for the plural formula if there was no plural formula set up earlier. Actually it only checks one of the fields use to store the plural setup, but that should be fine IMHO.
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | keep-plurals-D6.patch | 2 KB | Gábor Hojtsy |
| #8 | plural-untouch.patch | 5.9 KB | Gábor Hojtsy |
| #6 | 566390_keep-plurals.patch | 3.34 KB | Jose Reyero |
| keep-plurals.patch | 3.08 KB | Gábor Hojtsy | |
Comments
Comment #1
Gábor HojtsyTo test:
- import a .po file
- check that the plural formula from the .po file was saved in the DB (languages table)
- mess with the plural formula in the info file (change numbers, equations, do not break the syntax, because it not going to import)
- import it again
- verify that your plural formula became broken
- apply the patch
- mess with your plural formula in a different way (but still do not mess with the syntax, so it gets parsed properly)
- check your protection checkbox while importing
- verify that it did not indeed mess up your existing plural form
- profit :)
Comment #2
seutje CreditAttribution: seutje commentedTested:
Existing behavior:
(n!=1)(n!=1)plural form(n!=0)in the po file(n!=0)After applying patch:
(n!=1)(n!=1)plural form(n!=0)in the po file(n!=1)plural form is still intact and wasn't changedfrom what I understand that's how it should be so the patch works fine and profit was achieved :)
Comment #3
Gábor HojtsyDo you feel it is good to go then?
Comment #4
seutje CreditAttribution: seutje commentedI do, but shouldn't some1 else review it as well?
Comment #5
Gábor HojtsyAs explained in #654864: Plural-Forms generated in bad way => unable to properly edit .po files this should be considered a bug. I'd love if someone could be confident it is RTBC (I am :), and mark it as such. Thank you!
Comment #6
Jose Reyero CreditAttribution: Jose Reyero commentedThe patch doesn't apply anymore (functions moved around) but when fixed that it works as promised.
(Haven't been able to test the module install though, something seems broken with the new admin UI)
Updated patch.
Comment #7
plachCleaning-up the "language system" issue queue as per http://cyrve.com/criticals.
Comment #8
Gábor HojtsyAdded test coverage to ensure it works as expected and not rely on manual testers :)
- tests now check at first whether number of plurals was saved
- overwrite tester .po getter modified to use plural formula with 3 plurals
- tests added to check that if we asked to not overwrite, plurals are not overwritten either
- tests added to check that if we asked to overwrite, plurals are then overwritten
Locale import tests pass for me with these changes, and they cover the functionality of the patch, so if if all passes, I'm hopeful this is RTBC.
This is really a very small patch and almost all of the non-test changes are indentation differences.
Comment #9
seutje CreditAttribution: seutje commentedawesomeness, works exactly as expected.
good to go?
Comment #10
Gábor HojtsyYay! :)
Comment #11
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks.
Comment #12
Gábor HojtsyLet's backport to Drupal 6 too. This is a plague many people caught.
Comment #13
Gábor HojtsyBackport to D6. Testing welcome. Note that we do not change the labels on forms due to this not being a change to justify string breaks.
Comment #14
seutje CreditAttribution: seutje commentedWorks exactly as expected, a bit unfortunate that we can't change the strings at this point, which leads to the fact that there is no mention anywhere of when plural forms are updated and when they aren't... perhaps we need to add a line to http://drupal.org/node/21145 or something
but this is by no means a stopper for this fix (imo)
Comment #15
Gábor Hojtsy@seutje: well, this was a bug at the start because people expected that plural updates will work the same as string updates, so I think while it might not be as clear, it should be understandable from the import modes.
Comment #16
Gábor HojtsyGreat, committed then.
Comment #18
ajayg CreditAttribution: ajayg commentedNot sure if this bug still exists or this is a sideffect.
1-Download and clean install drupal 6.17 module.
2. Enable locale module
3. Add a language (any language)
4. Export template to create ".pot" file.
5. Try to import the template in the new language
You get an error
"The translation file drupal.pot contains an error: the plural formula could not be parsed"
Why I need to do this? Was trying to add a translation for a new language and wanted to start from scratch with existing language (english) and gradually translate the string as I can.
Comment #19
ajayg CreditAttribution: ajayg commentedComment #20
Gábor HojtsyWhen you start from scratch, you need to specify the plural formula in your .po file or Drupal will not know what formula to use and will not import your file. Once the plural formula is set, the above fix was about trying to protect it from people importing files like you did which previously killed off a properly set formula. You need to at least import a proper formula once with a .po file. This has nothing to do with the above issue. Look for this error message on drupal.org and you'll find answers.