Opened 5 years ago

Closed 15 months ago

Last modified 7 months ago

#17375 closed Bug (fixed)

'makemessages' ignores plural from 'blocktrans'

Reported by: ahagenbruch Owned by: Sergey Kolosov
Component: Internationalization Version: master
Severity: Normal Keywords:
Cc: pigletto, 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 pigletto 5 years ago.
issue_17375.2.diff (15.6 KB) - added by pigletto 5 years ago.
patch_17375_updated.diff (13.9 KB) - added by pigletto 5 years ago.
Patch updated to current trunk

Download all attachments as: .zip

Change History (27)

comment:1 Changed 5 years ago by Claude Paroz

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 Changed 5 years ago by Aymeric Augustin

Triage Stage: UnreviewedAccepted

comment:3 Changed 5 years ago by pigletto

Needs tests: set
Owner: changed from nobody to pigletto

comment:4 Changed 5 years ago by pigletto

Needs tests: unset

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

Attached test for this specific case.

comment:5 Changed 5 years ago by pigletto

Status: newassigned

comment:6 Changed 5 years ago by Claude Paroz

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

Changed 5 years ago by pigletto

Attachment: issue_17375.diff added

comment:7 Changed 5 years ago by pigletto

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 Changed 5 years ago by pigletto

Cc: pigletto added

Changed 5 years ago by pigletto

Attachment: issue_17375.2.diff added

comment:9 Changed 5 years ago by pigletto

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

comment:10 Changed 5 years ago by Jakub Wiśniowski

Triage Stage: AcceptedReady for checkin

Looks and works OK.

comment:11 Changed 5 years ago by Jannis Leidel

This doesn't apply to trunk anymore.

comment:12 Changed 5 years ago by Aymeric Augustin

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

Indeed, patch no longer applies.

comment:13 Changed 5 years ago by pigletto

OK, I'm going to update it soon

Changed 5 years ago by pigletto

Attachment: patch_17375_updated.diff added

Patch updated to current trunk

comment:14 Changed 5 years ago by pigletto

Patch needs improvement: unset

Patch updated

comment:15 Changed 3 years ago by Claude Paroz

#22671 was a duplicate

comment:16 Changed 2 years ago by Tim Graham

Patch needs improvement: set

Patch no longer applies cleanly.

comment:17 Changed 18 months ago by Sergey Kolosov

Cc: m17.admin@… added
Owner: changed from pigletto to Sergey Kolosov

Working on it.

comment:18 Changed 16 months ago by Sergey Kolosov

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 Changed 16 months ago by Tim Graham

Patch needs improvement: set

Left some comments for improvement.

comment:20 Changed 15 months ago by Sergey Kolosov

Patch needs improvement: unset

comment:21 Changed 15 months ago by Tim Graham

Triage Stage: AcceptedReady for checkin

Claude, could you take a look?

comment:22 Changed 15 months ago by Claude Paroz

Looks like a solid patch. Thanks Sergey!

comment:23 Changed 15 months ago by Tim Graham <timograham@…>

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 Changed 7 months ago by Claude Paroz <claude@…>

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