Code

Opened 7 years ago

Closed 3 years ago

#5425 closed (duplicate)

Incorrect plurals in admin pagination template

Reported by: Petr Marhoun <petr.marhoun@…> Owned by: nobody
Component: contrib.admin Version:
Severity: Keywords: admin, internationalization
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description


Attachments (3)

admin-pagination.diff (970 bytes) - added by Petr Marhoun <petr.marhoun@…> 7 years ago.
00-admin-pagination.diff (949 bytes) - added by Petr Marhoun <petr.marhoun@…> 6 years ago.
new version - after autoescape merge
fix5425.patch (982 bytes) - added by mk 4 years ago.

Download all attachments as: .zip

Change History (22)

Changed 7 years ago by Petr Marhoun <petr.marhoun@…>

comment:1 Changed 7 years ago by Petr Marhoun <petr.marhoun@…>

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

See http://groups.google.com/group/django-developers/browse_thread/thread/2d8a06071553eafe/1c6587f10052814a#1c6587f10052814a

My language is really strange. Imagine that you have this grammar:

  • 1 contact,
  • 2 contacts, 3 contacts, 4 contacts
  • 5 contactz, 6 contactz, ...

And expression "5 contacts" is an error. OK, everybody knows what it
means - but for perfectionist it is not good enough.

So I propose to enable "translate" this expression in admin. It would
be no change for English and for other languages. But I could use this
translation:

  • msgid "%(count)s %(verbose_name)s"
  • msgstr "%(verbose_name)s: %(count)s"

comment:2 Changed 7 years ago by Densetsu no Ero-sennin <densetsu.no.ero.sennin@…>

This won't work for languages with more than one plural form. For example, in Russian "1 object, 2 objects, 10 objects" becomes "1 объект, 2 объекта, 10 объектов". I think, the only way to handle it correctly is to use ungettext, possibly via {% blocktrans %} ... {% plural %} ... {% endblocktrans %} template tag.

comment:3 Changed 7 years ago by Petr Marhoun <petr.marhoun@…>

My language (Czech) is similar - it has also two plural forms. Usually I use ungettext - but it is not possible in this situation what is the reason for this ticket.

For pagination template I have to use Meta class from model which has only two relevant attributes - verbose_name and verbose_name_plural. Therefore I can't use ungettext. So I use trick - I know that "2 объектов" (I supposed it is used now) is a mistake, so I "translate" it as "Oбъектов: 2" which is ok (maybe, I don't speak Russian - but it is ok in my language).

There is another solution: Verbose name could be defined as function with ungettext. I tried it and it worked - but it seems to be too complex for this problem. If Django developers tell it is the preferred variant, I will create a patch - it is no problem for me.

comment:4 Changed 7 years ago by Densetsu no Ero-sennin <densetsu.no.ero.sennin@…>

What if we use something like ungettext(u'%%i %s' % verbose_name, u'%%i %s' % verbose_name_plural, n) % n? The only problem is how to mark these dynamically created strings for translation. Maybe make-messages.py could take care of this.

comment:5 Changed 7 years ago by Simon G. <dev@…>

  • Triage Stage changed from Unreviewed to Design decision needed

comment:6 Changed 7 years ago by ubernostrum

  • Resolution set to duplicate
  • Status changed from new to closed

Since there are about a million different proposals for handling pagination in templates in various places, I'd like to consolidate it all into #3169 for now (and, more realistically, in discussion on django-developers).

comment:7 Changed 7 years ago by Petr Marhoun <petr.marhoun@…>

  • Keywords internationalization added; templates, pagination removed
  • Resolution duplicate deleted
  • Status changed from closed to reopened
  • Summary changed from [newforms-admin] - more flexible pagination template to [newforms-admin] - incorrect plurals in admin pagination template

I don't understand what exactly you propose. I suppose I have this model:

class Object(models.Model):
    # ...
    class Meta:
        verbose_name = _('object')
        verbose_name_plural = _('object')

If the current code would be in python, it would be something as:

if n == 1:
    text = '%i %s' % (count, verbose_name)
else:
    text = '%i %s' % (count, verbose_name_plural)

The result is:

  • 1 object; 2 objects; 5 objects - ok in English
  • 1 objekt; 2 objekty; 10 objekty - not ok in Czech
  • 1 объект; 2 объекта; 10 объекта - probably not ok in Russian

I propose this little change:

if n == 1:
    text = _('%{count}i %{verbose_name}s') % {'count': count, 'verbose_name': verbose_name}
else:
    text = _('%{count}i %{verbose_name_plural}s') % {'count': count, 'verbose_name_plural': verbose_name_plural}

After translation the result would be:

  • 1 object; 2 objects; 5 objects - ok in English
  • Objekt: 1; Objekty: 2; Objekty: 10 - quite ok in Czech
  • Oбъект: 1; Oбъекта: 2; Oбъекта: 10 - maybe quite ok in Russian

I tried another solution:

class Object(models.Model):
    # ...
    class Meta:
        verbose_name = lambda n: ungettext('object', 'objects', n) % {'n' : n}

Then I changed django.db.models.options.Options to check if verbose_name was function. It worked and it was backward-compatible but I think that this problem doesn't deserve such complex solution.

So I try to imagine which patch you propose. Should make-messages.py be smarted? I think it is possible - expressions in models after verbose_name would be considered as pluralized messages. Options class and template would count with it. But is it worth? Is my solution really not good enough for Russian?

comment:8 Changed 7 years ago by Petr Marhoun <petr.marhoun@…>

I reopened the ticket because it is not about pagination - it is about internationalization in a template. It just happened that the name of the template was "pagination.html".

I see I chose bad summary and bad keywords. So I changed it. Now it shouldn't be misguided.

Changed 6 years ago by Petr Marhoun <petr.marhoun@…>

new version - after autoescape merge

comment:9 Changed 6 years ago by Karen Tracey <kmtracey@…>

  • Keywords nfa-someday added

Old admin looks to be no different from newforms-admin for this issue, so this should not block merge.

comment:10 Changed 6 years ago by mtredinnick

I don't like this solution. It's too wordy (admin is only a single example of where you might need pluralisation support in this sort of situation). We need a version of blocktrans that can handle plural specifications somehow, I suspect.

I have a memory that this has been discussed on django-developer in the past and we got a bit stuck on the syntax. I'd rather persist with trying to get that right, though. Somebody who has ideas should start a discussion on django-dev to see if we can get some consensus.

comment:11 Changed 4 years ago by jezdez

  • Keywords nfa-someday removed
  • Summary changed from [newforms-admin] - incorrect plurals in admin pagination template to Incorrect plurals in admin pagination template
  • Triage Stage changed from Design decision needed to Accepted
  • Version newforms-admin deleted

comment:12 Changed 4 years ago by adamnelson

  • Patch needs improvement set

comment:13 Changed 4 years ago by mk

  • Patch needs improvement unset
  • Triage Stage changed from Accepted to Ready for checkin

We have a blocktrans tag supporting pluralization now. Trivial one-line patch attached, test suite still passes.

Changed 4 years ago by mk

comment:14 Changed 4 years ago by lukeplant

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

(In [13998]) Fixed #5425 - Incorrect plurals in admin pagination template.

Thanks to Petr Marhoun for the report, and mk for the patch.

comment:15 Changed 4 years ago by lukeplant

(In [13999]) [1.2.X] Fixed #5425 - Incorrect plurals in admin pagination template.

Thanks to Petr Marhoun for the report, and mk for the patch.

Backport of [13998] from trunk

comment:16 Changed 3 years ago by lukeplant

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

Refs #5425, #14844

comment:17 Changed 3 years ago by lukeplant

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

Refs #5425, #14844

Backport of [14897] from trunk.

comment:18 Changed 3 years ago by lukeplant

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:19 Changed 3 years ago by lukeplant

  • Resolution set to duplicate
  • Status changed from reopened to closed
  • Triage Stage changed from Ready for checkin to Accepted

Closing this as a duplicate of #14844, because that has more discussion about how to correctly fix this.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.