Test failure with trac.wiki.tests.functional.TestWikiDelete

[Ryan J Ollos]

Failure seen on ​AppVeyor and ​Travis CI.

======================================================================
FAIL: runTest (trac.wiki.tests.functional.TestWikiDelete)
Delete a wiki page.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/build/edgewall/trac/trac/wiki/tests/functional.py", line 76, in runTest
    r'version=2">all 2 versions</a>\s+of the page' % name)
  File "/home/travis/build/edgewall/trac/trac/tests/functional/better_twill.py", line 224, in better_find
    (to_unicode(e), filename))
TwillAssertionError: no match to 'Removing\s+<a href="/wiki/BagpipeTongue\?action=history&amp;version=2">all 2 versions</a>\s+of the page' at file:///home/travis/build/edgewall/trac/testenv/trac/log/TestWikiDelete.html
----------------------------------------------------------------------
Ran 222 tests in 677.196s
FAILED (failures=1)
make: *** [functional-test] Error 1

[Jun Omae]

ngettext() is used in wiki_delete.html like this:

 73           # set num_versions
 74           <a href="${history_href}">${
 75             ngettext("%(num)d version", "%(num)d versions", num=num_versions)
 76           }</a>
 77           # endset
...
131           #   set num_versions
132           <a href="${history_href}">${
133             ngettext("%(num)d version", "all %(num)d versions", num=num_versions)
134             }</a>
135           #   endset

extract_messages command extracts "%(num)d version" and "%(num)d versions" from the above but "all %(num)d versions" is not extracted.

#: trac/wiki/templates/wiki_delete.html:75
#: trac/wiki/templates/wiki_delete.html:133
#, python-format
msgid "%(num)d version"
msgid_plural "%(num)d versions"
msgstr[0] ""
msgstr[1] ""

[Jun Omae]

extract_messages command's behavior is right. msgid_plural is used only when messages catalog is not found for active language. msgid is used as a key and must be unique without msgid_plural. Therefore, usage of ngettext in wiki_delete.html is wrong.

trac/wiki/templates/wiki_delete.html

@@ -130 +130 @@
           # else:
           #   set num_versions
           <a href="${history_href}">${
-            ngettext("%(num)d version", "all %(num)d versions", num=num_versions)
+            ngettext("%(num)d version", "%(num)d versions", num=num_versions)
             }</a>
           #   endset
           #   trans num_versions, created = first_modified, modified = last_modified
 
-          Removing ${num_versions} of the page, which was created
+          Removing all ${num_versions} of the page, which was created
           ${created} and last modified ${modified}.
 
           #   endtrans

trac/wiki/tests/functional.py

@@ -72 +72 @@
         tc.formvalue('delete', 'action', 'delete')
         tc.submit('delete_page')
         tc.find("Are you sure you want to completely delete this page?")
-        tc.find(r'Removing\s+<a href="/wiki/%s\?action=history&amp;'
-                r'version=2">all 2 versions</a>\s+of the page' % name)
+        tc.find(r'Removing all\s+<a href="/wiki/%s\?action=history&amp;'
+                r'version=2">2 versions</a>\s+of the page' % name)
         tc.notfind("The following attachments will also be deleted:")
         tc.submit('delete', 'delete-confirm')
         tc.find("The page %s has been deleted." % name)

[Jun Omae]

Committed in [16076].

[Ryan J Ollos]

At least for English, it seems that the plural form will always be used because it's inside a conditional where num_versions != 1: trunk/trac/wiki/templates/wiki_delete.html@16076:120,130#L115​.

[Jun Omae]

If compiled catalog is used, 2nd argument of ngettext is ignored.

$ grep -F '%(num)d version' trac/locale/messages.pot trac/locale/en_US/LC_MESSAGES/messages.po
trac/locale/messages.pot:msgid "%(num)d version"
trac/locale/messages.pot:msgid_plural "%(num)d versions"
trac/locale/en_US/LC_MESSAGES/messages.po:msgid "%(num)d version"
trac/locale/en_US/LC_MESSAGES/messages.po:msgid_plural "%(num)d versions"
$ make compile-en_US
python  setup.py  compile_catalog -l en_US  compile_catalog_js -l en_US  compile_catalog_tracini -l en_US \
            generate_messages_js -l en_US
running compile_catalog
compiling catalog trac/locale/en_US/LC_MESSAGES/messages.po to trac/locale/en_US/LC_MESSAGES/messages.mo
running compile_catalog_js
compiling catalog trac/locale/en_US/LC_MESSAGES/messages-js.po to trac/locale/en_US/LC_MESSAGES/messages-js.mo
running compile_catalog_tracini
compiling catalog trac/locale/en_US/LC_MESSAGES/tracini.po to trac/locale/en_US/LC_MESSAGES/tracini.mo
running generate_messages_js
generating messages javascript 'trac/locale/en_US/LC_MESSAGES/messages-js.mo' to 'trac/htdocs/js/messages/en_US.js'

$ make start-python
Python 2.7.6 (default, Oct 26 2016, 20:30:19)
[GCC 4.8.4] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> from trac.util.translation import make_activable, ngettext, translations
>>> make_activable(lambda: 'en_US')
>>> ngettext('%(num)d version', 'all %(num)d versions', 1)
u'1 version'
>>> ngettext('%(num)d version', 'all %(num)d versions', 2)
u'2 versions'   # <== not 'all %(num)d versions'
>>> translations.active.files
['trac/locale/en_US/LC_MESSAGES/messages.mo']

$ make start-python
Python 2.7.6 (default, Oct 26 2016, 20:30:19)
[GCC 4.8.4] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> from trac.util.translation import make_activable, ngettext, translations
>>> make_activable(lambda: 'en_US')
>>> ngettext('%(num)d version', 'all %(num)d versions', 1)
u'1 version'
>>> ngettext('%(num)d version', 'all %(num)d versions', 2)
u'all 2 versions'
>>> translations.active.files
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: NullTranslationsBabel instance has no attribute 'files'

[Ryan J Ollos]

Okay, I think I might understand the reason now. I had wondered why the following wasn't used:

trac/wiki/templates/wiki_delete.html

@@ -129 +129 @@
           #   endtrans
           # else:
           #   set num_versions
-          <a href="${history_href}">${
-            ngettext("%(num)d version", "%(num)d versions", num=num_versions)
-            }</a>
+          <a href="${history_href}">${_("%(num)d versions", num=num_versions)}</a>
           #   endset
           #   trans num_versions, created = first_modified, modified = last_modified

But on making the change I see that it would result in an extra entry in the catalog:

@@ -7596,28 +7596,33 @@ msgid ""
 "%(created)s."
 msgstr ""

-#: trac/wiki/templates/wiki_delete.html:136
+#: trac/wiki/templates/wiki_delete.html:132
+#, python-format
+msgid "%(num)d versions"
+msgstr ""
+
+#: trac/wiki/templates/wiki_delete.html:134
 #, python-format
 msgid ""
 "Removing all %(num_versions)s of the page, which was created %(created)s "
 "and last modified %(modified)s."
 msgstr ""

[Jun Omae]

I've considered the same idea but plural expression is more complex than English (nplural=2), Japanese (nplural=1), etc. I consider we should use ngettext() to be able to translate as plural.

E.g. and see also ​https://github.com/python-babel/babel/blob/v2.4.0/babel/messages/plurals.py#L22.

    # Russian
    'ru': (3, '(n%10==1 && n%100!=11 ? 0 : n%10>=2 && n%10<=4 && (n%100<10 || n%100>=20) ? 1 : 2)'),

[Ryan J Ollos]

Thanks, I will keep in mind for cases in which plural is used.