Opened 12 years ago

Closed 9 years ago

Last modified 8 years ago

#17375 closed Bug (fixed)

'makemessages' ignores plural from 'blocktrans'

Reported by: ahagenbruch Owned by: Sergey Kolosov
Component: Internationalization Version: dev
Severity: Normal Keywords:
Cc: Maciej Wiśniowski, m17.admin@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I have two files with equal strings to translate, in the first file
with trans and in the second file with blocktrans and a plural form:

a.html:
{% trans 'My string' %}
b.html:
{% blocktrans count counter=mylist|length %}My string{% plural %}My strings{% endblocktrans %}

and when I run django-admin.py makemessages -l de I get

django.po
#: templates/a.html:108
#: templates/b.html:3
msgid "My string"
msgstr ""

and not as you'd expect

django.po
#: templates/a.html:108
#: templates/b.html:3
msgid "My string"
msgid_plural "My strings"
msgstr[0] ""
msgstr[1] ""

I can reproduce this for similar structures in other files.
blocktrans strings that don't have trans equivalents in other
files correctly produce entries with plural forms in the .po file.

Attachments (3)

issue_17375.diff (15.1 KB ) - added by Maciej Wiśniowski 12 years ago.
issue_17375.2.diff (15.6 KB ) - added by Maciej Wiśniowski 12 years ago.
patch_17375_updated.diff (13.9 KB ) - added by Maciej Wiśniowski 12 years ago.
Patch updated to current trunk

Download all attachments as: .zip

Change History (27)

comment:1 by Claude Paroz, 12 years ago

Seems to trigger this bug: https://savannah.gnu.org/bugs/index.php?35027

One possible workaround in makemessages could be to pass a list of files to xgettext instead of concatenating messages for each individual file, as xgettext seems to properly handle the merge.

comment:2 by Aymeric Augustin, 12 years ago

Triage Stage: UnreviewedAccepted

comment:3 by Maciej Wiśniowski, 12 years ago

Needs tests: set
Owner: changed from nobody to Maciej Wiśniowski

comment:4 by Maciej Wiśniowski, 12 years ago

Needs tests: unset

This issue doesn't exist anymore in current trunk (1.4).

Attached test for this specific case.

comment:5 by Maciej Wiśniowski, 12 years ago

Status: newassigned

comment:6 by Claude Paroz, 12 years ago

The only reason it is working in your patch is that the msgid of the problematic string in test.html has a msgctxt line.

by Maciej Wiśniowski, 12 years ago

Attachment: issue_17375.diff added

comment:7 by Maciej Wiśniowski, 12 years ago

Ok, right, it fails for blocktrans that doesn't use 'context'. Uploaded patch solves issue as suggested by using xgettext call with multiple files at once.

One thing to modify there yet is to make use of -f, --files-from in cmd for xgettext, instead of passing great amount of parameters.

comment:8 by Maciej Wiśniowski, 12 years ago

Cc: Maciej Wiśniowski added

by Maciej Wiśniowski, 12 years ago

Attachment: issue_17375.2.diff added

comment:9 by Maciej Wiśniowski, 12 years ago

Uploaded issue_17375.2.diff that uses --files-from parameter for xgettext. Now patch seems to be complete for me.

comment:10 by Jakub Wiśniowski, 12 years ago

Triage Stage: AcceptedReady for checkin

Looks and works OK.

comment:11 by Jannis Leidel, 12 years ago

This doesn't apply to trunk anymore.

comment:12 by Aymeric Augustin, 12 years ago

Has patch: set
Patch needs improvement: set
Triage Stage: Ready for checkinAccepted

Indeed, patch no longer applies.

comment:13 by Maciej Wiśniowski, 12 years ago

OK, I'm going to update it soon

by Maciej Wiśniowski, 12 years ago

Attachment: patch_17375_updated.diff added

Patch updated to current trunk

comment:14 by Maciej Wiśniowski, 12 years ago

Patch needs improvement: unset

Patch updated

comment:15 by Claude Paroz, 10 years ago

#22671 was a duplicate

comment:16 by Tim Graham, 10 years ago

Patch needs improvement: set

Patch no longer applies cleanly.

comment:17 by Sergey Kolosov, 9 years ago

Cc: m17.admin@… added
Owner: changed from Maciej Wiśniowski to Sergey Kolosov

Working on it.

comment:18 by Sergey Kolosov, 9 years ago

Patch needs improvement: unset

Created a PR with another implementation based on the original idea from Claude:
https://github.com/django/django/pull/5187

comment:19 by Tim Graham, 9 years ago

Patch needs improvement: set

Left some comments for improvement.

comment:20 by Sergey Kolosov, 9 years ago

Patch needs improvement: unset

comment:21 by Tim Graham, 9 years ago

Triage Stage: AcceptedReady for checkin

Claude, could you take a look?

comment:22 by Claude Paroz, 9 years ago

Looks like a solid patch. Thanks Sergey!

comment:23 by Tim Graham <timograham@…>, 9 years ago

Resolution: fixed
Status: assignedclosed

In e7588233:

Fixed #17375 -- Changed makemessages to use xgettext with --files-from

Changed the way makemessages invokes xgettext from one call per
translatable file to one call per locale directory (using --files-from).
This allows to avoid https://savannah.gnu.org/bugs/index.php?35027 and,
as a positive side effect, speeds up localization build.

comment:24 by Claude Paroz <claude@…>, 8 years ago

In 185f90c:

Adapted _assertPoLocComment for multi-file source lines in po files

Refs #17375.

Note: See TracTickets for help on using tickets.
Back to Top