Launchpad itself

Merge lp:~jtv/launchpad/henninge_validate_translation-cleanup into lp:~launchpad/launchpad/recife

  1. henninge_validate_translation-cleanup
  2. Merge into recife
Proposed by Jeroen T. Vermeulen
Status: Rejected
Rejected by: Henning Eggers
Proposed branch: lp:~jtv/launchpad/henninge_validate_translation-cleanup
Merge into: lp:~launchpad/launchpad/recife
Diff against target: 220 lines (+47/-47)
4 files modified
lib/lp/translations/model/potmsgset.py (+3/-20)
lib/lp/translations/scripts/gettext_check_messages.py (+6/-5)
lib/lp/translations/utilities/tests/test_validate.py (+25/-10)
lib/lp/translations/utilities/validate.py (+13/-12)
To merge this branch: bzr merge lp:~jtv/launchpad/henninge_validate_translation-cleanup
Reviewer Review Type Date Requested Status
Graham Binns (community) code Approve
Review via email: mp+36683@code.launchpad.net

Commit message

Henning's validate_translation cleanup.

Description of the change

= Henning's validate_translation cleanup =

For the Recife feature branch. I extracted this from Henning's ongoing feature work in his absence, since it'll take a lot of diff out of his main branch.

What it does is make the validate_translation helper conform to our agreed standard way of passing translation strings around: as a dict mapping plural-form numbers to translation strings. We used a mixture of these dicts and None-augmented lists in the past, and that is also what validate_translation accepted.

En passant this also eliminates another helper. Unlike msgstrs, msgids have a fixed set of plural forms: "normal" messages have only a singular and an optional plural. There's no need to generalize that into a list.

To test this, best run all the Translations tests:
{{{
./bin/test -vvc lp.translations
}}}

No lint.

Jeroen

To post a comment you must log in.
Revision history for this message
Martin Pool (mbp) wrote :

 it seems plausible to me but i've never looked at this before

Revision history for this message
Graham Binns (gmb) :
review: Approve (code)
Revision history for this message
Henning Eggers (henninge) wrote :

This particular branch contains more revisions than intended and has therefore been uncommitted from the recife branch. I created a new branch and applied the changes from the diff and committed that to the recife branch.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/translations/model/potmsgset.py'
--- lib/lp/translations/model/potmsgset.py 2010-09-08 02:40:38 +0000
+++ lib/lp/translations/model/potmsgset.py 2010-09-27 08:14:43 +0000
@@ -489,15 +489,6 @@
489 date_updated = current.date_reviewed489 date_updated = current.date_reviewed
490 return (date_updated is not None and date_updated > timestamp)490 return (date_updated is not None and date_updated > timestamp)
491491
492 def _list_of_msgids(self):
493 """Return a list of [singular_text, plural_text] if the message
494 is using plural forms, or just [singular_text] if it's not.
495 """
496 original_texts = [self.singular_text]
497 if self.plural_text is not None:
498 original_texts.append(self.plural_text)
499 return original_texts
500
501 def _sanitizeTranslations(self, translations, pluralforms):492 def _sanitizeTranslations(self, translations, pluralforms):
502 """Sanitize `translations` using self.applySanityFixes.493 """Sanitize `translations` using self.applySanityFixes.
503494
@@ -528,14 +519,12 @@
528 # By default all translations are correct.519 # By default all translations are correct.
529 validation_status = TranslationValidationStatus.OK520 validation_status = TranslationValidationStatus.OK
530521
531 # Cache the list of singular_text and plural_text
532 original_texts = self._list_of_msgids()
533
534 # Validate the translation we got from the translation form522 # Validate the translation we got from the translation form
535 # to know if gettext is unhappy with the input.523 # to know if gettext is unhappy with the input.
536 try:524 try:
537 validate_translation(525 validate_translation(
538 original_texts, translations, self.flags)526 self.singular_text, self.plural_text,
527 translations, self.flags)
539 except GettextValidationError:528 except GettextValidationError:
540 if ignore_errors:529 if ignore_errors:
541 # The translations are stored anyway, but we set them as530 # The translations are stored anyway, but we set them as
@@ -543,13 +532,7 @@
543 validation_status = TranslationValidationStatus.UNKNOWNERROR532 validation_status = TranslationValidationStatus.UNKNOWNERROR
544 else:533 else:
545 # Check to know if there is any translation.534 # Check to know if there is any translation.
546 has_translations = False535 if any(translations.values()):
547 for key in translations.keys():
548 if translations[key] is not None:
549 has_translations = True
550 break
551
552 if has_translations:
553 # Partial translations cannot be stored, the536 # Partial translations cannot be stored, the
554 # exception is raised again and handled outside537 # exception is raised again and handled outside
555 # this method.538 # this method.
556539
=== modified file 'lib/lp/translations/scripts/gettext_check_messages.py'
--- lib/lp/translations/scripts/gettext_check_messages.py 2010-09-06 10:40:54 +0000
+++ lib/lp/translations/scripts/gettext_check_messages.py 2010-09-27 08:14:43 +0000
@@ -1,4 +1,4 @@
1# Copyright 2009 Canonical Ltd. This software is licensed under the1# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4__metaclass__ = type4__metaclass__ = type
@@ -10,7 +10,6 @@
10 timedelta,10 timedelta,
11 )11 )
1212
13from storm.locals import Store
14from zope.component import getUtility13from zope.component import getUtility
15from zope.security.proxy import removeSecurityProxy14from zope.security.proxy import removeSecurityProxy
1615
@@ -95,11 +94,13 @@
95 :return: Error message string if there is an error, or None otherwise.94 :return: Error message string if there is an error, or None otherwise.
96 """95 """
97 potmsgset = translationmessage.potmsgset96 potmsgset = translationmessage.potmsgset
98 msgids = potmsgset._list_of_msgids()97 # validate_translation takes a dict but translations are a list.
99 msgstrs = translationmessage.translations98 msgstrs = dict(enumerate(translationmessage.translations))
10099
101 try:100 try:
102 validate_translation(msgids, msgstrs, potmsgset.flags)101 validate_translation(
102 potmsgset.singular_text, potmsgset.plural_text,
103 msgstrs, potmsgset.flags)
103 except GettextValidationError, error:104 except GettextValidationError, error:
104 self._error_count += 1105 self._error_count += 1
105 return unicode(error)106 return unicode(error)
106107
=== modified file 'lib/lp/translations/utilities/tests/test_validate.py'
--- lib/lp/translations/utilities/tests/test_validate.py 2010-08-23 08:41:03 +0000
+++ lib/lp/translations/utilities/tests/test_validate.py 2010-09-27 08:14:43 +0000
@@ -17,46 +17,60 @@
1717
18 def test_validate_translation_c_format(self):18 def test_validate_translation_c_format(self):
19 # Correct c-format translations will be validated.19 # Correct c-format translations will be validated.
20 english = ["English %s number %d"]20 english = "English %s number %d"
21 flags = ["c-format"]21 flags = ["c-format"]
22 translations = {0: "Translation %s number %d"}22 translations = {0: "Translation %s number %d"}
23 # This should not raise GettextValidationError.23 # This should not raise GettextValidationError.
24 validate_translation(english, translations, flags)24 validate_translation(english, None, translations, flags)
2525
26 def test_validate_translation_c_format_fail(self):26 def test_validate_translation_c_format_fail(self):
27 # Mismatched format specifiers will not be validated.27 # Mismatched format specifiers will not be validated.
28 english = ["English %s number %d"]28 english = "English %s number %d"
29 flags = ["c-format"]29 flags = ["c-format"]
30 translations = {0: "Translation %d"}30 translations = {0: "Translation %d"}
31 self.assertRaises(31 self.assertRaises(
32 GettextValidationError,32 GettextValidationError,
33 validate_translation, english, translations, flags)33 validate_translation, english, None, translations, flags)
34
35 def test_validate_translation_no_flag(self):
36 # Mismatched format specifiers don't matter if no format has been
37 # specified.
38 english = "English %s number %d"
39 flags = []
40 translations = {0: "Translation number %d"}
41 # This should not raise GettextValidationError.
42 validate_translation(english, None, translations, flags)
3443
35 def test_validate_translation_c_format_plural(self):44 def test_validate_translation_c_format_plural(self):
36 # Correct c-format translations will be validated on plurals.45 # Correct c-format translations will be validated on plurals.
37 english = ["English %s number %d", "English plural %s number %d"]46 english_singular = "English %s number %d"
47 english_plural = "English plural %s number %d"
38 flags = ["c-format"]48 flags = ["c-format"]
39 translations = {49 translations = {
40 0: "Translation singular %s number %d",50 0: "Translation singular %s number %d",
41 1: "Translation plural %s number %d",51 1: "Translation plural %s number %d",
42 }52 }
43 # This should not raise GettextValidationError.53 # This should not raise GettextValidationError.
44 validate_translation(english, translations, flags)54 validate_translation(
55 english_singular, english_plural, translations, flags)
4556
46 def test_validate_translation_c_format_plural_no_singular_format(self):57 def test_validate_translation_c_format_plural_no_singular_format(self):
47 # As a special case, the singular does not need format specifiers.58 # As a special case, the singular does not need format specifiers.
48 english = ["English %s number %d", "English plural %s number %d"]59 english_singular = "English %s number %d"
60 english_plural = "English plural %s number %d"
49 flags = ["c-format"]61 flags = ["c-format"]
50 translations = {62 translations = {
51 0: "Translation singular",63 0: "Translation singular",
52 1: "Translation plural %s number %d",64 1: "Translation plural %s number %d",
53 }65 }
54 # This should not raise GettextValidationError.66 # This should not raise GettextValidationError.
55 validate_translation(english, translations, flags)67 validate_translation(
68 english_singular, english_plural, translations, flags)
5669
57 def test_validate_translation_c_format_plural_fail(self):70 def test_validate_translation_c_format_plural_fail(self):
58 # Not matching format specifiers will not be validated.71 # Not matching format specifiers will not be validated.
59 english = ["English %s number %d", "English plural %s number %d"]72 english_singular = "English %s number %d"
73 english_plural = "English plural %s number %d"
60 flags = ["c-format"]74 flags = ["c-format"]
61 translations = {75 translations = {
62 0: "Translation singular %d",76 0: "Translation singular %d",
@@ -64,7 +78,8 @@
64 }78 }
65 self.assertRaises(79 self.assertRaises(
66 GettextValidationError,80 GettextValidationError,
67 validate_translation, english, translations, flags)81 validate_translation, english_singular, english_plural,
82 translations, flags)
6883
6984
70def test_suite():85def test_suite():
7186
=== modified file 'lib/lp/translations/utilities/validate.py'
--- lib/lp/translations/utilities/validate.py 2010-03-26 19:37:58 +0000
+++ lib/lp/translations/utilities/validate.py 2010-09-27 08:14:43 +0000
@@ -15,27 +15,29 @@
15 """Gettext validation failed."""15 """Gettext validation failed."""
1616
1717
18def validate_translation(original, translation, flags):18def validate_translation(original_singular, original_plural,
19 translations, flags):
19 """Check with gettext if a translation is correct or not.20 """Check with gettext if a translation is correct or not.
2021
21 If the translation has a problem, raise `GettextValidationError`.22 If the translation has a problem, raise `GettextValidationError`.
2223
23 :param msgids: A list of one or two msgids, depending on whether the24 :param original_singular: The English msgid.
24 message has a plural.25 :param original_plural: The English plural msgid, if the message has a
25 :param translation: A dictionary of translations, indexed with the plural26 plural or None otherwise.
27 :param translations: A dictionary of translations, indexed with the plural
26 form number.28 form number.
27 :param flags: This message's flags as a list of strings.29 :param flags: This message's flags as a list of strings.
28 """30 """
29 msg = gettextpo.PoMessage()31 msg = gettextpo.PoMessage()
30 msg.set_msgid(original[0])32 msg.set_msgid(original_singular)
3133
32 if len(original) > 1:34 if original_plural is not None:
33 # It has plural forms.35 # It has plural forms.
34 msg.set_msgid_plural(original[1])36 msg.set_msgid_plural(original_plural)
35 for form in range(len(translation)):37 for form, translation in translations.iteritems():
36 msg.set_msgstr_plural(form, translation[form])38 msg.set_msgstr_plural(form, translation)
37 elif len(translation) > 0:39 elif 0 in translations:
38 msg.set_msgstr(translation[0])40 msg.set_msgstr(translations[0])
39 else:41 else:
40 pass42 pass
4143
@@ -48,4 +50,3 @@
48 except gettextpo.error, e:50 except gettextpo.error, e:
49 # Wrap gettextpo.error in GettextValidationError.51 # Wrap gettextpo.error in GettextValidationError.
50 raise GettextValidationError(unicode(e))52 raise GettextValidationError(unicode(e))
51

Subscribers

People subscribed via source and target branches