Opened 4 years ago

Closed 14 hours ago

#17375 closed Bug (fixed)

'makemessages' ignores plural from 'blocktrans'

Reported by: ahagenbruch Owned by: sergeykolosov
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 4 years ago.
issue_17375.2.diff (15.6 KB) - added by pigletto 4 years ago.
patch_17375_updated.diff (13.9 KB) - added by pigletto 3 years ago.
Patch updated to current trunk

Download all attachments as: .zip

Change History (26)

comment:1 Changed 4 years ago by claudep

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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 4 years ago by aaugustin

  • Triage Stage changed from Unreviewed to Accepted

comment:3 Changed 4 years ago by pigletto

  • Needs tests set
  • Owner changed from nobody to pigletto

comment:4 Changed 4 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 4 years ago by pigletto

  • Status changed from new to assigned

comment:6 Changed 4 years ago by claudep

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

comment:7 Changed 4 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 4 years ago by pigletto

  • Cc pigletto added

Changed 4 years ago by pigletto

comment:9 Changed 4 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 4 years ago by ext

  • Triage Stage changed from Accepted to Ready for checkin

Looks and works OK.

comment:11 Changed 4 years ago by jezdez

This doesn't apply to trunk anymore.

comment:12 Changed 4 years ago by aaugustin

  • Has patch set
  • Patch needs improvement set
  • Triage Stage changed from Ready for checkin to Accepted

Indeed, patch no longer applies.

comment:13 Changed 3 years ago by pigletto

OK, I'm going to update it soon

Changed 3 years ago by pigletto

Patch updated to current trunk

comment:14 Changed 3 years ago by pigletto

  • Patch needs improvement unset

Patch updated

comment:15 Changed 16 months ago by claudep

#22671 was a duplicate

comment:16 Changed 14 months ago by timo

  • Patch needs improvement set

Patch no longer applies cleanly.

comment:17 Changed 3 months ago by sergeykolosov

  • Cc m17.admin@… added
  • Owner changed from pigletto to sergeykolosov

Working on it.

comment:18 Changed 4 days ago by sergeykolosov

  • 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 2 days ago by timgraham

  • Patch needs improvement set

Left some comments for improvement.

comment:20 Changed 41 hours ago by sergeykolosov

  • Patch needs improvement unset

comment:21 Changed 41 hours ago by timgraham

  • Triage Stage changed from Accepted to Ready for checkin

Claude, could you take a look?

comment:22 Changed 27 hours ago by claudep

Looks like a solid patch. Thanks Sergey!

comment:23 Changed 14 hours ago by Tim Graham <timograham@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

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.

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