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
1=== modified file 'lib/lp/translations/model/potmsgset.py'
2--- lib/lp/translations/model/potmsgset.py 2010-09-08 02:40:38 +0000
3+++ lib/lp/translations/model/potmsgset.py 2010-09-27 08:14:43 +0000
4@@ -489,15 +489,6 @@
5 date_updated = current.date_reviewed
6 return (date_updated is not None and date_updated > timestamp)
7
8- def _list_of_msgids(self):
9- """Return a list of [singular_text, plural_text] if the message
10- is using plural forms, or just [singular_text] if it's not.
11- """
12- original_texts = [self.singular_text]
13- if self.plural_text is not None:
14- original_texts.append(self.plural_text)
15- return original_texts
16-
17 def _sanitizeTranslations(self, translations, pluralforms):
18 """Sanitize `translations` using self.applySanityFixes.
19
20@@ -528,14 +519,12 @@
21 # By default all translations are correct.
22 validation_status = TranslationValidationStatus.OK
23
24- # Cache the list of singular_text and plural_text
25- original_texts = self._list_of_msgids()
26-
27 # Validate the translation we got from the translation form
28 # to know if gettext is unhappy with the input.
29 try:
30 validate_translation(
31- original_texts, translations, self.flags)
32+ self.singular_text, self.plural_text,
33+ translations, self.flags)
34 except GettextValidationError:
35 if ignore_errors:
36 # The translations are stored anyway, but we set them as
37@@ -543,13 +532,7 @@
38 validation_status = TranslationValidationStatus.UNKNOWNERROR
39 else:
40 # Check to know if there is any translation.
41- has_translations = False
42- for key in translations.keys():
43- if translations[key] is not None:
44- has_translations = True
45- break
46-
47- if has_translations:
48+ if any(translations.values()):
49 # Partial translations cannot be stored, the
50 # exception is raised again and handled outside
51 # this method.
52
53=== modified file 'lib/lp/translations/scripts/gettext_check_messages.py'
54--- lib/lp/translations/scripts/gettext_check_messages.py 2010-09-06 10:40:54 +0000
55+++ lib/lp/translations/scripts/gettext_check_messages.py 2010-09-27 08:14:43 +0000
56@@ -1,4 +1,4 @@
57-# Copyright 2009 Canonical Ltd. This software is licensed under the
58+# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
59 # GNU Affero General Public License version 3 (see the file LICENSE).
60
61 __metaclass__ = type
62@@ -10,7 +10,6 @@
63 timedelta,
64 )
65
66-from storm.locals import Store
67 from zope.component import getUtility
68 from zope.security.proxy import removeSecurityProxy
69
70@@ -95,11 +94,13 @@
71 :return: Error message string if there is an error, or None otherwise.
72 """
73 potmsgset = translationmessage.potmsgset
74- msgids = potmsgset._list_of_msgids()
75- msgstrs = translationmessage.translations
76+ # validate_translation takes a dict but translations are a list.
77+ msgstrs = dict(enumerate(translationmessage.translations))
78
79 try:
80- validate_translation(msgids, msgstrs, potmsgset.flags)
81+ validate_translation(
82+ potmsgset.singular_text, potmsgset.plural_text,
83+ msgstrs, potmsgset.flags)
84 except GettextValidationError, error:
85 self._error_count += 1
86 return unicode(error)
87
88=== modified file 'lib/lp/translations/utilities/tests/test_validate.py'
89--- lib/lp/translations/utilities/tests/test_validate.py 2010-08-23 08:41:03 +0000
90+++ lib/lp/translations/utilities/tests/test_validate.py 2010-09-27 08:14:43 +0000
91@@ -17,46 +17,60 @@
92
93 def test_validate_translation_c_format(self):
94 # Correct c-format translations will be validated.
95- english = ["English %s number %d"]
96+ english = "English %s number %d"
97 flags = ["c-format"]
98 translations = {0: "Translation %s number %d"}
99 # This should not raise GettextValidationError.
100- validate_translation(english, translations, flags)
101+ validate_translation(english, None, translations, flags)
102
103 def test_validate_translation_c_format_fail(self):
104 # Mismatched format specifiers will not be validated.
105- english = ["English %s number %d"]
106+ english = "English %s number %d"
107 flags = ["c-format"]
108 translations = {0: "Translation %d"}
109 self.assertRaises(
110 GettextValidationError,
111- validate_translation, english, translations, flags)
112+ validate_translation, english, None, translations, flags)
113+
114+ def test_validate_translation_no_flag(self):
115+ # Mismatched format specifiers don't matter if no format has been
116+ # specified.
117+ english = "English %s number %d"
118+ flags = []
119+ translations = {0: "Translation number %d"}
120+ # This should not raise GettextValidationError.
121+ validate_translation(english, None, translations, flags)
122
123 def test_validate_translation_c_format_plural(self):
124 # Correct c-format translations will be validated on plurals.
125- english = ["English %s number %d", "English plural %s number %d"]
126+ english_singular = "English %s number %d"
127+ english_plural = "English plural %s number %d"
128 flags = ["c-format"]
129 translations = {
130 0: "Translation singular %s number %d",
131 1: "Translation plural %s number %d",
132 }
133 # This should not raise GettextValidationError.
134- validate_translation(english, translations, flags)
135+ validate_translation(
136+ english_singular, english_plural, translations, flags)
137
138 def test_validate_translation_c_format_plural_no_singular_format(self):
139 # As a special case, the singular does not need format specifiers.
140- english = ["English %s number %d", "English plural %s number %d"]
141+ english_singular = "English %s number %d"
142+ english_plural = "English plural %s number %d"
143 flags = ["c-format"]
144 translations = {
145 0: "Translation singular",
146 1: "Translation plural %s number %d",
147 }
148 # This should not raise GettextValidationError.
149- validate_translation(english, translations, flags)
150+ validate_translation(
151+ english_singular, english_plural, translations, flags)
152
153 def test_validate_translation_c_format_plural_fail(self):
154 # Not matching format specifiers will not be validated.
155- english = ["English %s number %d", "English plural %s number %d"]
156+ english_singular = "English %s number %d"
157+ english_plural = "English plural %s number %d"
158 flags = ["c-format"]
159 translations = {
160 0: "Translation singular %d",
161@@ -64,7 +78,8 @@
162 }
163 self.assertRaises(
164 GettextValidationError,
165- validate_translation, english, translations, flags)
166+ validate_translation, english_singular, english_plural,
167+ translations, flags)
168
169
170 def test_suite():
171
172=== modified file 'lib/lp/translations/utilities/validate.py'
173--- lib/lp/translations/utilities/validate.py 2010-03-26 19:37:58 +0000
174+++ lib/lp/translations/utilities/validate.py 2010-09-27 08:14:43 +0000
175@@ -15,27 +15,29 @@
176 """Gettext validation failed."""
177
178
179-def validate_translation(original, translation, flags):
180+def validate_translation(original_singular, original_plural,
181+ translations, flags):
182 """Check with gettext if a translation is correct or not.
183
184 If the translation has a problem, raise `GettextValidationError`.
185
186- :param msgids: A list of one or two msgids, depending on whether the
187- message has a plural.
188- :param translation: A dictionary of translations, indexed with the plural
189+ :param original_singular: The English msgid.
190+ :param original_plural: The English plural msgid, if the message has a
191+ plural or None otherwise.
192+ :param translations: A dictionary of translations, indexed with the plural
193 form number.
194 :param flags: This message's flags as a list of strings.
195 """
196 msg = gettextpo.PoMessage()
197- msg.set_msgid(original[0])
198+ msg.set_msgid(original_singular)
199
200- if len(original) > 1:
201+ if original_plural is not None:
202 # It has plural forms.
203- msg.set_msgid_plural(original[1])
204- for form in range(len(translation)):
205- msg.set_msgstr_plural(form, translation[form])
206- elif len(translation) > 0:
207- msg.set_msgstr(translation[0])
208+ msg.set_msgid_plural(original_plural)
209+ for form, translation in translations.iteritems():
210+ msg.set_msgstr_plural(form, translation)
211+ elif 0 in translations:
212+ msg.set_msgstr(translations[0])
213 else:
214 pass
215
216@@ -48,4 +50,3 @@
217 except gettextpo.error, e:
218 # Wrap gettextpo.error in GettextValidationError.
219 raise GettextValidationError(unicode(e))
220-

Subscribers

People subscribed via source and target branches