netbox-community / netbox

The premier source of truth powering network automation. Open source under Apache 2. Public demo: https://demo.netbox.dev
http://netboxlabs.com/oss/netbox/
Apache License 2.0
15.42k stars 2.51k forks source link

Use ngettext for strings with count #16660

Open afranke opened 3 weeks ago

afranke commented 3 weeks ago

Proposed Changes

netbox/dcim/views.py has _("Disconnected {count} {type}") where count is as far as I can tell an arbitrary number and type a pre-baked plural. This cannot be properly translated in some languages, because plurals don’t work the same way as they do in English.

You need to:

  1. use ngettext for the count part, so plural forms can be handled properly.
  2. use one full string per type, because that’s the only way nouns can be declined as necessary.

This would result in something along the lines of

…
ngettext("Disconnected 1 device", "Disconnected {count} devices", count)
…
ngettext("Disconnected 1 client", "Disconnected {count} clients", count)
…

with something like a switch-case to match a given type with a given ngettext call.

Sorry for using client and device instead of actual types, I couldn’t easily find the type list.

Justification

You shouldn’t make up your own plural handling. There are tools for that, and good reasons to use them.

See #16659 for a similar but different situation.

afranke commented 3 weeks ago

I only mentioned one string because it’s the one I could find, but there may be other ones that are affected by this issue.

afranke commented 3 weeks ago

And of course right after I post that comment, I find more:

I may still be missing some more.

jeremystretch commented 3 weeks ago

@afranke thanks for bringing this to our attention. Would you be willing to contribute a PR to make the proposed changes?

afranke commented 3 weeks ago

I’m sorry but I won’t be able to. I can review a PR to check if it’s doing the right thing from an i18n perspective though.

Julio-Oliveira-Encora commented 2 weeks ago

Could you please assign it to me?

arthanson commented 5 days ago

@afranke In this case In netbox/dcim/views.py if we used the line:

                messages.success(request, _(ngettext(
                    "Disconnected 1 {type}",
                    "Disconnected {count} {type_plural}",
                    count,
                )).format(
                    count=count,
                    type=self.queryset.model._meta.verbose_name,
                    type_plural=self.queryset.model._meta.verbose_name_plural
                ))

Would that work for pluralization as you mentioned for Polish? We just have the singular and pluralized model name.

afranke commented 4 days ago

No, it wouldn’t work. See point 2 in the issue description.