Opened 6 years ago

Last modified 4 years ago

#14844 new Bug

i18n blocktrans tag pluralization feature limited by gettext constraints and shared local tag context

Reported by: Ramiro Morales Owned by: nobody
Component: Internationalization Version: master
Severity: Normal Keywords:
Cc: 4glitch@… Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

In r13998 a pluralization bug in the footer of paginated change list views for the admin app was fixed by switching to rely in the blocktrans i18n tag using:

{% blocktrans with cl.opts.verbose_name as verbose_name and cl.opts.verbose_name_plural as verbose_name_plural count cl.result_count as count %}
{{ count }} {{ verbose_name }}{% plural %}{{ count }} {{ verbose_name_plural }}
{% endblocktrans %}

Once makemessages is run to update a language's PO file, it contains this fuzzy entry:

#: contrib/admin/templates/admin/pagination.html:9
#, fuzzy, python-format
msgid "%(count)s %(verbose_name)s"
msgid_plural "%(count)s %(verbose_name_plural)s"
msgstr[0] "..."
msgstr[1] "..."

After updating the translation, removing the fuzzy marker and when trying to generate a MO file, this somewhat crytic errors is generated for that literal:

django.po:812: a format specification for argument 'verbose_name', as in 'msgstr[0]', doesn't exist in 'msgid'

Our documentation warns about this at the bottom of the blocktrans description, pointing to the ungettext() notes (http://docs.djangoproject.com/en/1.2/topics/i18n/internationalization/#pluralization-var-notes) because GNU gettext doesn't allow the variable placeholders used to be different.

But if we change the template construct to use :

{% blocktrans with cl.opts.verbose_name as vname and cl.opts.verbose_name_plural as vname count cl.result_count as count %}
{{ count }} {{ vname }}
{% plural %}
{{ count }} {{ vname }}
{% endblocktrans %}

and repeat the PO update & compile cycle then the admin always shows the pluralized model name (its verbose_name_plural). This is because the template context is unique and the last value bound to the same extra context var is the one that gets used.

This reveals a limitation of blocktrans: The GNU gettext constraint is playing against our need to have correct pluralized names for generic literals like this where the entities at play can be different objects.

This problem can be seen in the admin change list view for the auth User model when using e.g. 'es_AR' (and 'es') and 'fr' locales (they have two plural forms and the translations of 'user' and 'users' differ in these locales, 'de' doesn't.)

This error also affects the 1.2.X branch.

I think one way to solve this would be providing the blocktrans tag the ability to designate a context var that can take different values depending of if it's being used either in the singular or (fallback) plural branches. I will attach a patch later that implements a name subtag similar to the count one and auxiliar to it, with syntax name <filter-or-var-expression-for-the-singular-case> or <filter-or-var-expression-for-the-plural-case> as <new-context-var>. E.g.

{% blocktrans count cl.result_count as count name cl.opts.verbose_name or cl.opts.verbose_name_plural as vname %}
{{ count }} {{ vname }}
{% plural %}
{{ count }} {{ vname }}
{% endblocktrans %}

So blocktrans approaches more the capabilities provided by ungettext() to Python code.

A last note maybe worth adding to the docs as part of this: The plural section of blocktrans and the plural parameter of ungettext() are fallbacks in the case no translation is found in the catalog, the correct locale translations from the catalog according to the (possibly more than two) plural rules have greater priority.

Attachments (2)

14844-1.2.X.diff (1009 bytes) - added by Ramiro Morales 6 years ago.
Patch as suggested by Claude, for 1.2.X
14844-1-wip.diff (7.0 KB) - added by Ramiro Morales 5 years ago.
work-in-progress patch implementing the proposed syntax

Download all attachments as: .zip

Change History (19)

comment:1 Changed 6 years ago by Claude Paroz

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset

As discussed on IRC, my opinion is to avoid using verbose_name_plural at all:

{% blocktrans with cl.opts.verbose_name as verbose_name count cl.result_count as count %}
{{ count }} "{{ verbose_name }}" object{% plural %}{{ count }} "{{ verbose_name }}" objects
{% endblocktrans %}

Mixing proper ngettext plural with limited only-one plural approach is essentially broken.

comment:2 Changed 6 years ago by Ramiro Morales

Summary: i18b blocktrans tag pluralization feature limited by gettext constraints and contexti18n blocktrans tag pluralization feature limited by gettext constraints and unique local tag context

On IRC Jannis pointed to the work being done for #7817, #9456: https://github.com/SmileyChris/django/compare/master...7817-include-with.
The syntax changes for blocktrans to be implemented mean we need to find another syntax if the proposal is accepted as a solution for this ticket.

I'd be inclined to implement the solution proposed by Claude for both 1.2.X and trunk now so to unbreak the i18n workflow (PO -> MO conversion is broken ATM) and target a longer term solution for trunk once the template syntax overhaul dust settles.

That longer term solution might involve to end the full reliance on verbose_name_plural when pluralizing model names in Python code and templates, for the benefit of locales with more than two plural forms. I suspect the challenge would be to achieve that without interfering with or complication English speaking Django core developers normal workflow so to don't get the proposal rejected.

Version 0, edited 6 years ago by Ramiro Morales (next)

Changed 6 years ago by Ramiro Morales

Attachment: 14844-1.2.X.diff added

Patch as suggested by Claude, for 1.2.X

comment:3 Changed 6 years ago by Jannis Leidel

As a long term fix syntax I'd like to propose:

{% blocktrans with cl.opts.verbose_name as vname count cl.result_count as count %}
{{ count }} {{ vname }}
{% plural with cl.opts.verbose_name_plural as vname %}
{{ count }} {{ vname }}
{% endblocktrans %}

comment:4 Changed 6 years ago by Claude Paroz

Unless verbose_name_plural itself is fixed (but how can it be fixed if the plural count is not known beforehand?), the solution proposed by jezdez is unfortunately still wrong for many languages.

comment:5 in reply to:  4 Changed 6 years ago by Jannis Leidel

Replying to claudep:

Unless verbose_name_plural itself is fixed (but how can it be fixed if the plural count is not known beforehand?), the solution proposed by jezdez is unfortunately still wrong for many languages.

Yeah, assuming the fix is possible.

comment:6 Changed 6 years ago by Luke Plant

I think the solution proposed by Claude sounds about right. The problem is that it causes a regression - for the case of English, verbose_name_plural will be ignored. AFAICS, [13998] never fixed the bug it was supposed to fix - can someone confirm that? If so, my apologies, and I will go ahead and revert it in order to get the i18n workflow fixed.

Then we need to agree on a plan for verbose_name_plural. The annoying thing is that if you remove it, you have to enable the i18n framework just to get plurals in the admin for English. A significant amount of time and effort had gone in to make the i18n framework optional, and I don't think we should break that, since there will be many deployments that use only English. One option is to use verbose_name_plural only if i18n is disabled, and document this clearly.

comment:7 Changed 6 years ago by Claude Paroz

This is indeed a design decision. Do we want to make Django a good i18n citizen or not? What you call a regression ([3 "Book" objects] instead of [3 books]) is part of the necessary compromises we need to make in order to be i18n friendly.

I'm not against keeping verbose_name_plural, either for non-i18n apps or for undefined plurals. But as soon as you have defined plurals with real numbers, you have to present whole sentences to translators. Any constructed sentence will surely be broken at least for some languages.

comment:8 in reply to:  6 Changed 6 years ago by Ramiro Morales

Replying to lukeplant:

I think the solution proposed by Claude sounds about right. The problem is that it causes a regression - for the case of English, verbose_name_plural will be ignored. AFAICS, [13998] never fixed the bug it was supposed to fix - can someone confirm that?

Correct, it didn't achieve what was intended, and prevents translators from creating a compiled (MO) catalogs after updating the PO files if they have updated to r13998 or newer.

comment:9 Changed 6 years ago by Jannis Leidel

So if I understand this issue correctly, we should at least fix what has been done in r13998 since we are very close to a beta release. I think the patch by Claude from 12/6 would work for trunk, too.

For any other long term fix of verbose_name_plural we have go back to the white board for 1.4. If anyone has other good idea, please chime in this week.

comment:10 Changed 6 years ago by Luke Plant

(In [14897]) Reverted [13998] because it never worked.

Refs #5425, #14844

comment:11 Changed 6 years ago by Luke Plant

(In [14898]) [1.2.X] Reverted [13998] because it never worked.

Refs #5425, #14844

Backport of [14897] from trunk.

comment:12 Changed 6 years ago by Ramiro Morales

The ticket tracking the verbose_name_plural issue re: i18n for plural-forms > 2 is #11688.

comment:13 Changed 6 years ago by Ramiro Morales

Triage Stage: UnreviewedAccepted

comment:14 Changed 5 years ago by James Addison

milestone: 1.3
Severity: Normal
Type: Bug

Changed 5 years ago by Ramiro Morales

Attachment: 14844-1-wip.diff added

work-in-progress patch implementing the proposed syntax

comment:15 Changed 5 years ago by Ramiro Morales

Easy pickings: unset
Has patch: set
Needs documentation: set
Patch needs improvement: set
UI/UX: unset

I've attached a path with an initial implementation of the syntax proposed by Jannis in comment:3 for review.

It depends on the patch for #11688. Also, it still needs changes to the blocktrans documentation.

comment:16 Changed 4 years ago by anonymous

Cc: 4glitch@… added

comment:17 Changed 4 years ago by Ramiro Morales

Summary: i18n blocktrans tag pluralization feature limited by gettext constraints and unique local tag contexti18n blocktrans tag pluralization feature limited by gettext constraints and shared local tag context
Note: See TracTickets for help on using tickets.
Back to Top