Merge lp:~ursinha/launchpad/plural-forms-bug-156107 into lp:launchpad
- plural-forms-bug-156107
- Merge into devel
Status: | Merged |
---|---|
Merged at revision: | not available |
Proposed branch: | lp:~ursinha/launchpad/plural-forms-bug-156107 |
Merge into: | lp:launchpad |
Diff against target: | None lines |
To merge this branch: | bzr merge lp:~ursinha/launchpad/plural-forms-bug-156107 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Henning Eggers (community) | code | Approve | |
Ursula Junque (community) | Needs Resubmitting | ||
Review via email:
|
Commit message
Description of the change
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Ursula Junque (ursinha) wrote : | # |
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Henning Eggers (henninge) wrote : | # |
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Hi Ursula,
thank you for finally fixing this long-standing bug. This looks very
good although I do have a few comments. ;)
review needs-fixing
Cheers,
Henning
Am 10.09.2009 19:00, Ursula Junque schrieb:
> Ursula Junque has proposed merging lp:~ursinha/launchpad/plural-forms-bug-156107 into lp:launchpad/devel.
>
> Requested reviews:
> Canonical Launchpad Engineering (launchpad)
>
> = Summary =
>
> This branch contains the fix for bug 156107, that talks about how plural forms are displayed in the language index page.
>
>
> == Implementation details ==
> This fix has two parts.
> 1) Migrating plural forms related methods to translations/
> Some methods have been moved from utilities/
> change were updated as well.
>
> 2) Writing a method that would create examples of plural forms, so we could display them friendly in
> the language index page, instead of the C formula as it was before.
> I've added a property to LanguageView, where the list of dicts that my function returns is slightly
> modified. Then the template just displays the list, as you can see in any language index page in
> this branch.
>
>
> == Tests ==
> bin/test -vvt language-views
> bin/test -vvt xx-language
>
>
> == Demo and Q/A ==
> To check the changed pages, go to:
> - https:/
> - https:/
> - https:/
>
> It should display the new format in different cases.
>
>
> = Launchpad lint =
>
> Checking for conflicts. and issues in doctests and templates.
> Running jslint, xmllint, pyflakes, and pylint.
> Using normal rules.
>
> Linting changed files:
> lib/lp/
> lib/lp/
> lib/lp/
> lib/lp/
> lib/lp/
> lib/lp/
> lib/lp/
> lib/lp/
>
>
> === modified file 'lib/lp/
> --- lib/lp/
> +++ lib/lp/
> @@ -26,6 +26,7 @@
> GetitemNavigation, LaunchpadView, LaunchpadFormView,
> LaunchpadEditFo
> enabled_
> +from lp.translations
>
> from canonical.widgets import LabeledMultiChe
>
> @@ -156,6 +157,26 @@
> def getTopContribut
> return self.context.
>
> + @property
> + def friendly_
> + """Formats the plural forms' example list.
> +
> + It takes the list of examples for each plural form and transforms in a
> + comma separated list to be displayed.
> + """
>...
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Ursula Junque (ursinha) wrote : | # |
[snip]
> > === modified file 'lib/lp/
> > --- lib/lp/
> 06:48:47 +0000
> > +++ lib/lp/
> 14:04:22 +0000
> > @@ -9,9 +9,9 @@
> > >>> from canonical.
> >
> > >>> language_set = getUtility(
> > - >>> language = language_
> > + >>> portuguese = language_
> > >>> language_admin_view = create_view(
> > - ... language, '+admin', layer=Translati
> > + ... portuguese, '+admin', layer=Translati
> > >>> language_
> >
> > The language +admin view provides a label and a page_title for the page.
> > @@ -73,3 +73,17 @@
> > >>> print language_
> > http://
> >
> > +
> > +View Language
> > +------------------
> > +
> > + >>> serbian = language_
> > + >>> language_view = create_
> > + ... layer=Translati
> > +
> > +The friendlypluralforms function shows us a list of plural forms and a set
> of examples
> > +for each one, so one won't need to understand the plural formula expression
> to see how it works.
> > +
> > + >>> language_
> > + [{'examples': '1.', 'form': 0}, {'examples': '2, 3, 4, ...', 'form':
> 1}, {'examples': '5, 6, 7, 8, 9, 10, ...', 'form': 2}]
>
> So, I have a problem with this although I have seen that it is used
> elsewhere, too, even in this patch. Dictionary keys do not have a
> defined order so it might be pure coincidence that they are in
> alphabetical order here. But this could change with future versions of
> Python and the test would break. Maybe you could ask around if that is
> really OK or else come up with some code to check this.
I've workarounded this one by checking what's inside the dicts, is it necessary to show the structure dict/list or the content is fine?
[snip]
> > === modified file 'lib/lp/
> > --- lib/lp/
> 18:35:37 +0000
> > +++ lib/lp/
> 00:06:48 +0000
> > @@ -92,11 +92,13 @@
> > >>> pluralforms_portlet = find_portlet(
> forms')
> > >>> print pluralforms_portlet
> > <...
> > - There are 2 plural forms for Spanish.
> > - ...
> > - <pre>n != 1</pre>
> > - ...
> > -
> > + Spanish has 2 plural forms:
> > + ...
> > + Form 0 for 1.
> > + ...
> > + Form 1 for 2, 3, 4, 5, 6, 7, ...
> > + ...
> > +
>
> Do you know about extract_text? This would be a good place to use it.
> lib/canonical/
Done. Thanks for the tip on the extract_text method. :)
>
> > >>> translationteam
> > ... browser.contents, 'Translation teams')
[snip]
> > === added file 'lib/lp/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Henning Eggers (henninge) wrote : | # |
As I have seen your new incremental patch, you resolved all the issues we talked about. Wonderful!
Thanks for your great effort! ;-)
Henning
Preview Diff
1 | === modified file 'lib/lp/translations/browser/language.py' |
2 | --- lib/lp/translations/browser/language.py 2009-08-28 09:27:25 +0000 |
3 | +++ lib/lp/translations/browser/language.py 2009-09-10 13:31:25 +0000 |
4 | @@ -26,6 +26,7 @@ |
5 | GetitemNavigation, LaunchpadView, LaunchpadFormView, |
6 | LaunchpadEditFormView, action, canonical_url, ContextMenu, NavigationMenu, |
7 | enabled_with_permission, Link, custom_widget) |
8 | +from lp.translations.utilities.pluralforms import make_friendly_plural_forms |
9 | |
10 | from canonical.widgets import LabeledMultiCheckBoxWidget |
11 | |
12 | @@ -156,6 +157,26 @@ |
13 | def getTopContributors(self): |
14 | return self.context.translators[:20] |
15 | |
16 | + @property |
17 | + def friendly_plural_forms(self): |
18 | + """Formats the plural forms' example list. |
19 | + |
20 | + It takes the list of examples for each plural form and transforms in a |
21 | + comma separated list to be displayed. |
22 | + """ |
23 | + pluralforms_list = make_friendly_plural_forms( |
24 | + self.context.pluralexpression, self.context.pluralforms) |
25 | + |
26 | + for item in pluralforms_list: |
27 | + examples = ", ".join(map(str, item['examples'])) |
28 | + if len(item['examples']) != 1: |
29 | + examples += ", ..." |
30 | + else: |
31 | + examples += "." |
32 | + item['examples'] = examples |
33 | + |
34 | + return pluralforms_list |
35 | + |
36 | |
37 | class LanguageAdminView(LaunchpadEditFormView): |
38 | """Handle an admin form submission.""" |
39 | |
40 | === modified file 'lib/lp/translations/browser/tests/language-views.txt' |
41 | --- lib/lp/translations/browser/tests/language-views.txt 2009-08-28 06:48:47 +0000 |
42 | +++ lib/lp/translations/browser/tests/language-views.txt 2009-09-10 14:04:22 +0000 |
43 | @@ -9,9 +9,9 @@ |
44 | >>> from canonical.launchpad.layers import TranslationsLayer |
45 | |
46 | >>> language_set = getUtility(ILanguageSet) |
47 | - >>> language = language_set.getLanguageByCode('pt_BR') |
48 | + >>> portuguese = language_set.getLanguageByCode('pt_BR') |
49 | >>> language_admin_view = create_view( |
50 | - ... language, '+admin', layer=TranslationsLayer) |
51 | + ... portuguese, '+admin', layer=TranslationsLayer) |
52 | >>> language_admin_view.initialize() |
53 | |
54 | The language +admin view provides a label and a page_title for the page. |
55 | @@ -73,3 +73,17 @@ |
56 | >>> print language_add_view.next_url |
57 | http://translations.launchpad.dev/+languages/lp_US |
58 | |
59 | + |
60 | +View Language |
61 | +------------------ |
62 | + |
63 | + >>> serbian = language_set.getLanguageByCode('sr') |
64 | + >>> language_view = create_initialized_view(serbian, '+index', |
65 | + ... layer=TranslationsLayer) |
66 | + |
67 | +The friendlypluralforms function shows us a list of plural forms and a set of examples |
68 | +for each one, so one won't need to understand the plural formula expression to see how it works. |
69 | + |
70 | + >>> language_view.friendly_plural_forms |
71 | + [{'examples': '1.', 'form': 0}, {'examples': '2, 3, 4, ...', 'form': 1}, {'examples': '5, 6, 7, 8, 9, 10, ...', 'form': 2}] |
72 | + |
73 | |
74 | === modified file 'lib/lp/translations/scripts/fix_plural_forms.py' |
75 | --- lib/lp/translations/scripts/fix_plural_forms.py 2009-07-17 00:26:05 +0000 |
76 | +++ lib/lp/translations/scripts/fix_plural_forms.py 2009-09-08 17:18:04 +0000 |
77 | @@ -16,7 +16,8 @@ |
78 | from lp.translations.model.translationmessage import TranslationMessage |
79 | from lp.translations.interfaces.translations import TranslationConstants |
80 | from lp.translations.utilities.gettext_po_parser import ( |
81 | - POHeader, plural_form_mapper) |
82 | + POHeader) |
83 | +from lp.translations.utilities.pluralforms import plural_form_mapper |
84 | |
85 | |
86 | def get_mapping_for_pofile_plurals(pofile): |
87 | |
88 | === modified file 'lib/lp/translations/stories/standalone/xx-language.txt' |
89 | --- lib/lp/translations/stories/standalone/xx-language.txt 2009-08-28 18:35:37 +0000 |
90 | +++ lib/lp/translations/stories/standalone/xx-language.txt 2009-09-10 00:06:48 +0000 |
91 | @@ -92,11 +92,13 @@ |
92 | >>> pluralforms_portlet = find_portlet(browser.contents, 'Plural forms') |
93 | >>> print pluralforms_portlet |
94 | <... |
95 | - There are 2 plural forms for Spanish. |
96 | - ... |
97 | - <pre>n != 1</pre> |
98 | - ... |
99 | - |
100 | + Spanish has 2 plural forms: |
101 | + ... |
102 | + Form 0 for 1. |
103 | + ... |
104 | + Form 1 for 2, 3, 4, 5, 6, 7, ... |
105 | + ... |
106 | + |
107 | >>> translationteams_portlet = find_portlet( |
108 | ... browser.contents, 'Translation teams') |
109 | >>> print translationteams_portlet |
110 | |
111 | === modified file 'lib/lp/translations/templates/language-index.pt' |
112 | --- lib/lp/translations/templates/language-index.pt 2009-08-28 18:36:27 +0000 |
113 | +++ lib/lp/translations/templates/language-index.pt 2009-09-10 14:03:15 +0000 |
114 | @@ -32,15 +32,15 @@ |
115 | <h2>Plural forms</h2> |
116 | <tal:has_pluralforms condition="context/pluralexpression"> |
117 | <p> |
118 | - There are <tal:pluralforms replace="context/pluralforms">2</tal:pluralforms> |
119 | - plural forms for <tal:language replace="view/language_name">Español</tal:language>. |
120 | - </p> |
121 | - <p> |
122 | - The default plural form expression for <tal:language replace="view/language_name">Español</tal:language> is: |
123 | - </p> |
124 | - <pre tal:content="context/pluralexpression"> |
125 | - n != 1 |
126 | - </pre> |
127 | + <tal:language replace="view/language_name">Español</tal:language> has |
128 | + <tal:pluralforms replace="context/pluralforms">2</tal:pluralforms> plural forms: |
129 | + </p> |
130 | + <div tal:repeat="pluralform view/friendly_plural_forms"> |
131 | + <p> |
132 | + Form <tal:index replace="pluralform/form">0</tal:index> for |
133 | + <tal:examples replace="pluralform/examples">1, 2, 3...</tal:examples> |
134 | + </p> |
135 | + </div> |
136 | <p> |
137 | When translating into this language, you need to be able to |
138 | express plural forms effectively. The plural form expression tells |
139 | @@ -114,7 +114,6 @@ |
140 | </p> |
141 | </div> |
142 | </div> |
143 | - |
144 | <div class="yui-u"> |
145 | <div class="portlet"> |
146 | <h2>Countries</h2> |
147 | |
148 | === modified file 'lib/lp/translations/utilities/doc/gettext_po_parser.txt' |
149 | --- lib/lp/translations/utilities/doc/gettext_po_parser.txt 2009-07-02 17:16:50 +0000 |
150 | +++ lib/lp/translations/utilities/doc/gettext_po_parser.txt 2009-09-08 17:18:55 +0000 |
151 | @@ -435,7 +435,7 @@ |
152 | |
153 | >>> form_1 = 'n!=1' |
154 | >>> form_2 = 'n==1' |
155 | - >>> from lp.translations.utilities.gettext_po_parser import ( |
156 | + >>> from lp.translations.utilities.pluralforms import ( |
157 | ... plural_form_mapper) |
158 | >>> plural_form_mapper(form_1, form_2) |
159 | {0: 1, 1: 0, 2: 2, 3: 3, 4: 4, 5: 5} |
160 | |
161 | === modified file 'lib/lp/translations/utilities/gettext_po_parser.py' |
162 | --- lib/lp/translations/utilities/gettext_po_parser.py 2009-07-30 17:56:41 +0000 |
163 | +++ lib/lp/translations/utilities/gettext_po_parser.py 2009-09-08 17:17:07 +0000 |
164 | @@ -13,7 +13,6 @@ |
165 | __all__ = [ |
166 | 'POHeader', |
167 | 'POParser', |
168 | - 'plural_form_mapper', |
169 | ] |
170 | |
171 | import gettext |
172 | @@ -38,84 +37,9 @@ |
173 | TranslationFileData, |
174 | TranslationMessageData) |
175 | from canonical.launchpad.versioninfo import revno |
176 | - |
177 | - |
178 | -class BadPluralExpression(Exception): |
179 | - """Local "escape hatch" exception for unusable plural expressions.""" |
180 | - |
181 | - |
182 | -def make_plural_function(expression): |
183 | - """Create a lambda function for C-like plural expression.""" |
184 | - # Largest expressions we could find in practice were 113 characters |
185 | - # long. 500 is a reasonable value which is still 4 times more than |
186 | - # that, yet not incredibly long. |
187 | - if expression is None or len(expression) > 500: |
188 | - raise BadPluralExpression |
189 | - |
190 | - # Guard against '**' usage: it's not useful in evaluating |
191 | - # plural forms, yet can be used to introduce a DoS. |
192 | - if expression.find('**') != -1: |
193 | - raise BadPluralExpression |
194 | - |
195 | - # We allow digits, whitespace [ \t], parentheses, "n", and operators |
196 | - # as allowed by GNU gettext implementation as well. |
197 | - if not re.match('^[0-9 \t()n|&?:!=<>+%*/-]*$', expression): |
198 | - raise BadPluralExpression |
199 | - |
200 | - try: |
201 | - function = gettext.c2py(expression) |
202 | - except (ValueError, SyntaxError): |
203 | - raise BadPluralExpression |
204 | - |
205 | - return function |
206 | - |
207 | - |
208 | -def make_plurals_identity_map(): |
209 | - """Return a dict mapping each plural form number onto itself.""" |
210 | - return dict(enumerate(xrange(TranslationConstants.MAX_PLURAL_FORMS))) |
211 | - |
212 | - |
213 | -def plural_form_mapper(first_expression, second_expression): |
214 | - """Maps plural forms from one plural formula to the other. |
215 | - |
216 | - Returns a dict indexed by indices in the `first_formula` |
217 | - pointing to corresponding indices in the `second_formula`. |
218 | - """ |
219 | - identity_map = make_plurals_identity_map() |
220 | - try: |
221 | - first_func = make_plural_function(first_expression) |
222 | - second_func = make_plural_function(second_expression) |
223 | - except BadPluralExpression: |
224 | - return identity_map |
225 | - |
226 | - # Can we create a mapping from one expression to the other? |
227 | - mapping = {} |
228 | - for n in range(1000): |
229 | - try: |
230 | - first_form = first_func(n) |
231 | - second_form = second_func(n) |
232 | - except (ArithmeticError, TypeError): |
233 | - return identity_map |
234 | - |
235 | - # Is either result out of range? |
236 | - valid_forms = range(TranslationConstants.MAX_PLURAL_FORMS) |
237 | - if first_form not in valid_forms or second_form not in valid_forms: |
238 | - return identity_map |
239 | - |
240 | - if first_form in mapping: |
241 | - if mapping[first_form] != second_form: |
242 | - return identity_map |
243 | - else: |
244 | - mapping[first_form] = second_form |
245 | - |
246 | - # The mapping must be an isomorphism. |
247 | - if sorted(mapping.keys()) != sorted(mapping.values()): |
248 | - return identity_map |
249 | - |
250 | - # Fill in the remaining inputs from the identity map: |
251 | - result = identity_map.copy() |
252 | - result.update(mapping) |
253 | - return result |
254 | +from lp.translations.utilities.pluralforms import (make_plurals_identity_map, |
255 | + plural_form_mapper) |
256 | + |
257 | |
258 | |
259 | class POSyntaxWarning(Warning): |
260 | |
261 | === added file 'lib/lp/translations/utilities/pluralforms.py' |
262 | --- lib/lp/translations/utilities/pluralforms.py 1970-01-01 00:00:00 +0000 |
263 | +++ lib/lp/translations/utilities/pluralforms.py 2009-09-10 00:07:47 +0000 |
264 | @@ -0,0 +1,103 @@ |
265 | +# Copyright 2009 Canonical Ltd. This software is licensed under the |
266 | +# GNU Affero General Public License version 3 (see the file LICENSE). |
267 | + |
268 | +import re |
269 | +import gettext |
270 | +from lp.translations.interfaces.translations import ( |
271 | + TranslationConstants) |
272 | + |
273 | + |
274 | +class BadPluralExpression(Exception): |
275 | + """Local "escape hatch" exception for unusable plural expressions.""" |
276 | + |
277 | +def make_friendly_plural_forms(expression, pluralforms_count): |
278 | + """Return a dict's list of plural forms' examples from an expression.""" |
279 | + expression_ = make_plural_function(expression) |
280 | + forms = {} |
281 | + |
282 | + for n in range(1, 200): |
283 | + forms.setdefault(expression_(n), []) |
284 | + # Giving a limit of 6 examples per plural form. If all the forms were |
285 | + # contemplated, it stops. |
286 | + if len(forms[expression_(n)]) == 6 and len(forms) == pluralforms_count: |
287 | + break |
288 | + if len(forms[expression_(n)]) == 6: |
289 | + continue |
290 | + forms[expression_(n)].append(n) |
291 | + |
292 | + return [{'form':k, 'examples':v} for (k,v) in forms.iteritems()] |
293 | + |
294 | + |
295 | +def make_plural_function(expression): |
296 | + """Create a lambda function for C-like plural expression.""" |
297 | + # Largest expressions we could find in practice were 113 characters |
298 | + # long. 500 is a reasonable value which is still 4 times more than |
299 | + # that, yet not incredibly long. |
300 | + if expression is None or len(expression) > 500: |
301 | + raise BadPluralExpression |
302 | + |
303 | + # Guard against '**' usage: it's not useful in evaluating |
304 | + # plural forms, yet can be used to introduce a DoS. |
305 | + if expression.find('**') != -1: |
306 | + raise BadPluralExpression |
307 | + |
308 | + # We allow digits, whitespace [ \t], parentheses, "n", and operators |
309 | + # as allowed by GNU gettext implementation as well. |
310 | + if not re.match('^[0-9 \t()n|&?:!=<>+%*/-]*$', expression): |
311 | + raise BadPluralExpression |
312 | + |
313 | + try: |
314 | + function = gettext.c2py(expression) |
315 | + except (ValueError, SyntaxError): |
316 | + raise BadPluralExpression |
317 | + |
318 | + return function |
319 | + |
320 | + |
321 | +def make_plurals_identity_map(): |
322 | + """Return a dict mapping each plural form number onto itself.""" |
323 | + return dict(enumerate(xrange(TranslationConstants.MAX_PLURAL_FORMS))) |
324 | + |
325 | + |
326 | +def plural_form_mapper(first_expression, second_expression): |
327 | + """Maps plural forms from one plural formula to the other. |
328 | + |
329 | + Returns a dict indexed by indices in the `first_formula` |
330 | + pointing to corresponding indices in the `second_formula`. |
331 | + """ |
332 | + identity_map = make_plurals_identity_map() |
333 | + try: |
334 | + first_func = make_plural_function(first_expression) |
335 | + second_func = make_plural_function(second_expression) |
336 | + except BadPluralExpression: |
337 | + return identity_map |
338 | + |
339 | + # Can we create a mapping from one expression to the other? |
340 | + mapping = {} |
341 | + for n in range(1000): |
342 | + try: |
343 | + first_form = first_func(n) |
344 | + second_form = second_func(n) |
345 | + except (ArithmeticError, TypeError): |
346 | + return identity_map |
347 | + |
348 | + # Is either result out of range? |
349 | + valid_forms = range(TranslationConstants.MAX_PLURAL_FORMS) |
350 | + if first_form not in valid_forms or second_form not in valid_forms: |
351 | + return identity_map |
352 | + |
353 | + if first_form in mapping: |
354 | + if mapping[first_form] != second_form: |
355 | + return identity_map |
356 | + else: |
357 | + mapping[first_form] = second_form |
358 | + |
359 | + # The mapping must be an isomorphism. |
360 | + if sorted(mapping.keys()) != sorted(mapping.values()): |
361 | + return identity_map |
362 | + |
363 | + # Fill in the remaining inputs from the identity map: |
364 | + result = identity_map.copy() |
365 | + result.update(mapping) |
366 | + return result |
367 | + |
= Summary =
This branch contains the fix for bug 156107, that talks about how plural forms are displayed in the language index page.
== Implementation details == utilities/ pluralforms. py, newly created file. gettext_ po_parser. py to this one. Tests related to this
This fix has two parts.
1) Migrating plural forms related methods to translations/
Some methods have been moved from utilities/
change were updated as well.
2) Writing a method that would create examples of plural forms, so we could display them friendly in
the language index page, instead of the C formula as it was before.
I've added a property to LanguageView, where the list of dicts that my function returns is slightly
modified. Then the template just displays the list, as you can see in any language index page in
this branch.
== Tests ==
bin/test -vvt language-views
bin/test -vvt xx-language
== Demo and Q/A == /translations. launchpad. dev/+languages/ sr /translations. launchpad. dev/+languages/ ru /translations. launchpad. dev/+languages/ it
To check the changed pages, go to:
- https:/
- https:/
- https:/
It should display the new format in different cases.
= Launchpad lint =
Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.
Linting changed files: translations/ utilities/ doc/gettext_ po_parser. txt translations/ scripts/ fix_plural_ forms.py translations/ utilities/ pluralforms. py translations/ browser/ tests/language- views.txt translations/ browser/ language. py translations/ templates/ language- index.pt translations/ stories/ standalone/ xx-language. txt translations/ utilities/ gettext_ po_parser. py
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/