Not showing localized plural strings

[Milo Casagrande]

Binary package hint: fast-user-switch-applet

If I choose from the fusa menu in the upper right corner one of the actions, the dialog that appears in the middle of the screen always shows the singular form, while I should see most of the time the plural form. I'm attaching a screenshot of the Italian translation: the number is 51 and it should say "secondi" not "secondo".

The translation of the strings is correct, they are strings 117 to 123 in Launchpad template.

I tested it with the English locale (en_US) and it is working as expected.

I don't know if it's a consequences of this, but also the title of the dialog and the button are not shown in their localized form.

The package version is 2.24.0-0ubuntu9
Ubuntu version is Jaunty.

[Milo Casagrande (milo)]

Looking at the code, looks like the ngettext function for handling plural forms is not working as expected. Probably it is related to the fact that the translations are not handled directly inside the ngettext function, but postponed with the macro N_() and referenced inside the ngettext function.

A consequences of this is also the fact the the POT file resulting from Launchpad does not have plural forms in it. You can also see this here:

https://translations.edge.launchpad.net/ubuntu/jaunty/+source/fast-user-switch-applet/+pots/fast-user-switch-applet/it/122/+translate

There is no plural form in that string, singular and plural are handled separately in two different strings (they should be handled in one string). This is also a problem for languages with more than 2 plural forms.

[Christoph Korn (c-korn)]

I tried to fix that by removing the N_ before the strings.
But that did not work.

Still only the singular localized string is shown.

[Milo Casagrande (milo)]

Il giorno sab, 21/03/2009 alle 17.38 +0000, Christoph Korn ha scritto:
> I tried to fix that by removing the N_ before the strings.
> But that did not work.
>
> Still only the singular localized string is shown.

Hmmm... probably I didn't explain myself in the best way with my last
message.

>From my POV, the problem is not with the use of the N_() macro itself,
but how it is used in this situation.

I'll start with the case of the not localized title and buttons, because
probably it's easier.

I'll take some snippets from the code:

gtk_label_set_text(GTK_LABEL(dialog->title),title_strings[dialog->action]);

In this case, the title_strings[dialog->action] elements are translated
here:

/*LOGOUT_DIALOG_LOGOUT,*/ N_("Log Out"),
/*LOGOUT_DIALOG_RESTART,*/ N_("Restart"),
/*LOGOUT_DIALOG_SHUTDOWN,*/ N_("Shut Down")

If I'm not totally wrong, when using N_() to postpone actual
translation, I need to use _() (or gettext()) around the "caller". So,
it should be:

gtk_label_set_text(GTK_LABEL(dialog->title),_(title_strings[dialog->action]));

And so on for the other elements (BTW, there is also another problem
here, but I'll talk about it at the end of the mail [1]).

The problem seems more complex with the handling of plural forms, like
here:

gchar * timeouttxt =
g_strdup_printf(ngettext(singular_strings[dialog->action],
plural_strings[dialog->action], dialog->timeout), dialog->timeout);

I don't know if using _() around the "caller" here will work out fine. I
don't know how gettext, when extracting the strings, will map all the
cases that are in the structure that defines the translatable strings.

In my mind, I would rather take case by case with a switch case using
one ngettext call for each plural I need, but it's a big code change to
do.

Sorry if I don't present a patch, but my C coding skills date back 4
years ago and have never had the chance to code in C again (even if this
would be a good chance), I will probably do more harm than good right
now.

Hope that my POV could be of some use anyway.

Cheers.

[1] Here we are using the same string as a title and as a button label:
there could be languages that translate those elements in a different
way. Using a context here would be great.

--
Milo Casagrande <email address hidden>

[Christoph Korn (c-korn)]

I read the examples here:
http://live.gnome.org/TranslationProject/DevGuidelines/Plurals

g_printf (ngettext ("You tried to move %d folder at once. You can't move more than 256 folders at once.",
                    "You tried to move %d folders at once. You can't move more than 256 folders at once.",
                    nbr_of_folders),
          nbr_of_folders);
The string are not enclosed with _() here.
So I supposed that my approach to remove the N_() had to work.

[Milo Casagrande (milo)]

Il giorno sab, 21/03/2009 alle 19.18 +0000, Christoph Korn ha scritto:
> I read the examples here:
> http://live.gnome.org/TranslationProject/DevGuidelines/Plurals
>
> g_printf (ngettext ("You tried to move %d folder at once. You can't move more than 256 folders at once.",
> "You tried to move %d folders at once. You can't move more than 256 folders at once.",
> nbr_of_folders),
> nbr_of_folders);
> The string are not enclosed with _() here.
> So I supposed that my approach to remove the N_() had to work.

Yes, with that example yes, but because in that example they have the
strings directly inside the ngettext function. With the fusa applet, the
strings are not directly inside the function, but are outside and you
need the N_() macro because you can't use the strings directly, but you
have to call gettext() or _() when you are going to use them.

--
Milo Casagrande <email address hidden>

[Christoph Korn (c-korn)]

Well, I still do not understand why _() is required when the string is in an constant array instead of
passed as a constant to the ngettext() function.
Nevertheless, the attached patch really makes the plural strings show when they should.
Means: plural n!=1. singular n==1.

[Christoph Korn (c-korn)]

Subscribing ubuntu-main-sponsors to review the patch.

@ Milo Casagrande: Thank you for your help!

[Milo Casagrande (milo)]

Il giorno sab, 21/03/2009 alle 20.44 +0000, Christoph Korn ha scritto:
> Subscribing ubuntu-main-sponsors to review the patch.
>
> @ Milo Casagrande: Thank you for your help!

You're more than welcome! And I have to thank you for your dedicated
time!

I would like to (try to) cast some more light on why you need the _() on
the ngettext function when you're using the N_() macro.

The N_() macro only _marks_ a string as translatable, it's different
from the _() that _handle_ the string. The N_() macro defers the actual
translation of the string, so if you want to really have the string
translated at runtime, you also have to call the handling function _().

Sorry if I sound boring! (but I'm rather keen on all these i18n aspects)

Cheers.

--
Milo Casagrande <email address hidden>

[Christoph Korn (c-korn)]

Ok.

I have built packages that have the patch applied.
Can you also test them please?

https://launchpad.net/~getdeb.packages/+archive/ppa

And do you know how we get the patch reviewed fast and published in jaunty?

[Milo Casagrande (milo)]

Il giorno sab, 21/03/2009 alle 23.51 +0000, Christoph Korn ha scritto:
> I have built packages that have the patch applied.
> Can you also test them please?
>
> https://launchpad.net/~getdeb.packages/+archive/ppa

I'll test it ASAP, thank you.

> And do you know how we get the patch reviewed fast and published in
> jaunty?

Actually... no. Could be a good idea writing to devel@ and
devel-discuss@ mailing list (I'm subscribed to both) and ping somebody
there so that we can raise the problem.

I'll be away till 3pm (Italy time) if you need any other test done.

--
Milo Casagrande <email address hidden>

[Christoph Korn (c-korn)]

I mailed to the devel-discuss and devel mailing list.

[Milo Casagrande (milo)]

Il giorno sab, 21/03/2009 alle 23.51 +0000, Christoph Korn ha scritto:
> Ok.
>
> I have built packages that have the patch applied.
> Can you also test them please?
>
> https://launchpad.net/~getdeb.packages/+archive/ppa

Looks like it's working.

The only problem I see, but probably it's on my side, is that if I run
'intltool-update --pot' to create a new pot file, I don't get the msgid
and msgid_plural.

--
Milo Casagrande <email address hidden>

[Milo Casagrande (milo)]

Apart from the really bad (and probably wrong too) C code, do you think that something like the patch attached could be made?

Testing it with intltool I get the msgid and msgid_plural for all the strings with the right plural form for my language (in this way languages with 5 or more plural forms are safe).

[Christoph Korn (c-korn)]

There were some errors in the code.
I attached the fixed code.

The package with the patch applied can be found in the PPA.

However I get some untranslated strings.

msgid "Unless you cancel, you will be logged out in %d second."
msgid_plural "Unless you cancel, you will be logged out in %d seconds."
msgstr[0] "Sie werden in %d Sekunde automatisch abgemeldet."
msgstr[1] "Sie werden in %d Sekunden automatisch abgemeldet."

Did I translate the right strings?
(I don't mean if the translation is correct but if the right strings are
translated?)

I translated msgstr[0] in singular form and msgstr[1] in plural form.

[Christoph Korn (c-korn)]

Just ignore what I said.
I forgot to delete the "fuzzy" keyword.

Everything works fine.

[Milo Casagrande (milo)]

Tested the PPA package and it works.

Thank you!

[Gabor Kelemen (kelemeng)]

About this part:

+static const gchar * button_strings[LOGOUT_DIALOG_ACTION_CNT] = {
+ /* LOGOUT_DIALOG_LOGOUT, */ NC_("button", "Log Out"),
+ /* LOGOUT_DIALOG_RESTART, */ NC_("button", "Restart"),
+ /* LOGOUT_DIALOG_SHUTDOWN, */ NC_("button", "Shut Down")

Why do not have the buttons accelerators here, just any normal button has?

[Gabor Kelemen (kelemeng)]

See also: http://library.gnome.org/devel/hig-book/stable/input-keyboard.html.en

[Ted Gould (ted)]

On Sun, 2009-03-29 at 23:31 +0000, Gabor Kelemen wrote:
> About this part:
>
> +static const gchar * button_strings[LOGOUT_DIALOG_ACTION_CNT] = {
> + /* LOGOUT_DIALOG_LOGOUT, */ NC_("button", "Log Out"),
> + /* LOGOUT_DIALOG_RESTART, */ NC_("button", "Restart"),
> + /* LOGOUT_DIALOG_SHUTDOWN, */ NC_("button", "Shut Down")
>
> Why do not have the buttons accelerators here, just any normal button
> has?

I guess, I'm not too concerned about it as the button has a default
action for return. I can't imagine most people would use a specific key
over the very large "Return" on the side of their keyboard :)

[Launchpad Janitor (janitor)]

This bug was fixed in the package fast-user-switch-applet - 2.24.0-0ubuntu10

---------------
fast-user-switch-applet (2.24.0-0ubuntu10) jaunty; urgency=low

  * 84_session_management.patch:
    * Patch from Christoph Korn to fix the translations for the titles
      of the dialogs and the plural strings. (LP: #345587 and LP: #345344)
    * Patch from Christoph Korn to add a configuration option for whether
      the confirmation dialogs are shown. (LP: #345480)
    * Clean up the confirmation configuration patch to make the labels
      change dynamically as the GConf key is changed.
    * Switch key on the configuration patch to make it so that it defaults
      to false, which is the same as no key. This makes it so that the
      default upgrade case is to have the confirmation dialogs.
    * Change the logout dialogs to be standard dialogs instead of special
      ones and shorten the text in them. Also make them appear on all desktops
      and in the window switcher list incase they get lost.
  * 90_status_management.patch: Update to add invisible entry and icons
    for that entry. Patch from Walter Somerville. (LP: #294731) Fixed
    patch to make the invisible item above the logout item.
  * 88_status_icons.patch: Add icons for invisible status (above). Contributed
    by Ken Wimer.
  * 92_autotools.patch: Update for changes above.
  * Closes meta-bug for upload request: (LP: #347697)

 -- Ted Gould <email address hidden> Mon, 23 Mar 2009 21:37:32 -0500