Edgewall Software
Modify

Opened 7 years ago

Closed 7 years ago

Last modified 4 years ago

#12850 closed defect (fixed)

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

Reported by: Ryan J Ollos Owned by: Jun Omae
Priority: normal Milestone: 1.3.2
Component: wiki system Version:
Severity: normal Keywords:
Cc: Branch:
Release Notes:
API Changes:
Internal Changes:

Fix wrong msgid_plural in wiki_delete.html.

Description

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

Attachments (0)

Change History (9)

comment:1 by Jun Omae, 7 years ago

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] ""

comment:2 by Jun Omae, 7 years ago

Owner: set to Jun Omae
Status: newassigned

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

    diff --git a/trac/wiki/templates/wiki_delete.html b/trac/wiki/templates/wiki_delete.html
    index d9b68553c..172e87830 100644
    a b  
    130130          # else:
    131131          #   set num_versions
    132132          <a href="${history_href}">${
    133             ngettext("%(num)d version", "all %(num)d versions", num=num_versions)
     133            ngettext("%(num)d version", "%(num)d versions", num=num_versions)
    134134            }</a>
    135135          #   endset
    136136          #   trans num_versions, created = first_modified, modified = last_modified
    137137
    138           Removing ${num_versions} of the page, which was created
     138          Removing all ${num_versions} of the page, which was created
    139139          ${created} and last modified ${modified}.
    140140
    141141          #   endtrans
  • trac/wiki/tests/functional.py

    diff --git a/trac/wiki/tests/functional.py b/trac/wiki/tests/functional.py
    index 292099581..a46c5d769 100755
    a b class TestWikiDelete(FunctionalTwillTestCaseSetup):  
    7272        tc.formvalue('delete', 'action', 'delete')
    7373        tc.submit('delete_page')
    7474        tc.find("Are you sure you want to completely delete this page?")
    75         tc.find(r'Removing\s+<a href="/wiki/%s\?action=history&amp;'
    76                 r'version=2">all 2 versions</a>\s+of the page' % name)
     75        tc.find(r'Removing all\s+<a href="/wiki/%s\?action=history&amp;'
     76                r'version=2">2 versions</a>\s+of the page' % name)
    7777        tc.notfind("The following attachments will also be deleted:")
    7878        tc.submit('delete', 'delete-confirm')
    7979        tc.find("The page %s has been deleted." % name)
Last edited 7 years ago by Jun Omae (previous) (diff)

comment:3 by Jun Omae, 7 years ago

Release Notes: modified (diff)
Resolution: fixed
Status: assignedclosed

Committed in [16076].

comment:4 by Ryan J Ollos, 7 years ago

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.

comment:5 by Jun Omae, 7 years ago

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'

comment:6 by Ryan J Ollos, 7 years ago

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

  • trac/wiki/templates/wiki_delete.html

    $git diff trac/wiki/templates/wiki_delete.html
    diff --git a/trac/wiki/templates/wiki_delete.html b/trac/wiki/templates/wiki_delete.html
    index 172e87830..872a027a0 100644
    a b  
    129129          #   endtrans
    130130          # else:
    131131          #   set num_versions
    132           <a href="${history_href}">${
    133             ngettext("%(num)d version", "%(num)d versions", num=num_versions)
    134             }</a>
     132          <a href="${history_href}">${_("%(num)d versions", num=num_versions)}</a>
    135133          #   endset
    136134          #   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 ""

comment:7 by Jun Omae, 7 years ago

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)'),

comment:8 by Ryan J Ollos, 7 years ago

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

comment:9 by Ryan J Ollos, 4 years ago

Internal Changes: modified (diff)
Release Notes: modified (diff)

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Jun Omae.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Jun Omae to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.