Opened 5 years ago

Last modified 2 years ago

#14844 new Bug

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

Reported by: ramiro 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 5 years ago.
Patch as suggested by Claude, for 1.2.X
14844-1-wip.diff (7.0 KB) - added by ramiro 4 years ago.
work-in-progress patch implementing the proposed syntax

Download all attachments as: .zip

Change History (19)

comment:1 Changed 5 years ago by claudep

  • 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 5 years ago by ramiro

  • Summary changed from i18b blocktrans tag pluralization feature limited by gettext constraints and context to i18n 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 complicating English-speaking [Django core] developers normal workflow so to don't get the proposal rejected.

Last edited 4 years ago by ramiro (previous) (diff)

Changed 5 years ago by ramiro

Patch as suggested by Claude, for 1.2.X

comment:3 Changed 5 years ago by jezdez

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 follow-up: Changed 5 years ago by 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.

comment:5 in reply to: ↑ 4 Changed 5 years ago by jezdez

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 follow-up: Changed 5 years ago by 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? 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 5 years ago by claudep

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

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

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

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

Refs #5425, #14844

comment:11 Changed 5 years ago by lukeplant

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

Refs #5425, #14844

Backport of [14897] from trunk.

comment:12 Changed 5 years ago by ramiro

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

comment:13 Changed 5 years ago by ramiro

  • Triage Stage changed from Unreviewed to Accepted

comment:14 Changed 4 years ago by jaddison

  • milestone 1.3 deleted
  • Severity set to Normal
  • Type set to Bug

Changed 4 years ago by ramiro

work-in-progress patch implementing the proposed syntax

comment:15 Changed 4 years ago by ramiro

  • 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 3 years ago by anonymous

  • Cc 4glitch@… added

comment:17 Changed 2 years ago by ramiro

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