Re: [Findutils-patches] [PATCH] Initial attempts to internationalise plural forms better

[James Youngman]

On Dec 20, 2007 3:46 AM, Eric Blake <address@hidden> wrote:
> >  #define textdomain(Domain)
> >  #define bindtextdomain(Package, Directory)
> > +#define ngettext(singular,plural,n) ((1==n) ? singular : plural)
>
> ...but you should get all three of these for free by including gnulib's
> gettext.h, properly no-op'd when not using NLS.

OK.   A long time ago (<=4.1.7), findutils used to use a captive
version of libintl, and so I guess now this can be removed.  I hadn't
realised.

> Why'd you change to %1$s notation?

Because...

> Translators are already permitted to add the %1$s notation in
> their translation if they access things in a different order, so you don't
> have to do it by default.

... I hadn't realised that.


> This comment applies throughout the bulk of your patch.

Perhaps I should revert those changes, then.   What do you think?

> > +    return xstrdup(_("unknown"
> > +                  /* TRANSLATORS: this is essentially an abbreviation
> > +                     for "unknown file system type" */));
>
> I'm not sure it works that way.  Generally, the TRANSLATORS comment has to
> come on the line before the _() usage.

Thanks.

>
> > +                  _("warning: you have specified the %1$s "
>
> Inconsistent in your use of WARNING: vs. warning:.

And the odd Warning: too, for good measure.   I propose to standardise
on one option.  Any preference?


> Oops.  database_file_size is size_t, which may be bigger than long.  You
> need to use the documented trick from the gettext manual:
> ngettetxt("string_a", "string_b",
>           (database_file_size > ULONG_MAX
>            ? (database_file_size % 1000000) + 1000000
>            : database_file_size));

I'm not sure I understand why.   Surely, the constant 1 in the macro
would just get promoted appropriately?

#define ngettext(singular,plural,n) ((1==n) ? singular : plural)

After all, this isn't a function call.


>
> > +  /* XXX: We would ideally use ngettext() here, but I don't know
> > +   *      how to use it to handle more than one possibly-plural thing/
> > +   */
> > +  printf(_("File names have a cumulative length of %1$s bytes.\n"
> > +        "Of those file names,\n"
> > +        "\n\t%2$s contain whitespace, "
> > +        "\n\t%3$s contain newline characters, "
> > +        "\n\tand %4$s contain characters with the high bit set.\n"),
>
> Why not ask on bug-gnu-gettext for advice?

Actually I did, the XXX is mildly out of date, sorry.   The response
is that this is not so bad as-is, but the only other thing I could do
is break the sentence up into parts for separate message-translation,
providing an explanatory comment.


> > @@ -1859,10 +1864,12 @@ dolocate (int argc, char **argv, int secure_db_fd)
> >                 /* For example:
> >                    warning: database `fred' is more than 8 days old (actual 
> > age is 10 days)*/
> >                 error (0, 0,
> > -                      _("warning: database %s is more than %d %s old 
> > (actual age is %.1f %s)"),
> > +                      _("warning: database %1$s is more than %2$d %3$s old 
> > (actual age is %4$.1f %5$s)"),
> >                        quotearg_n_style(0,  locale_quoting_style, e),
> > -                      warn_number_units,              _(warn_name_units),
> > -                      (age/(double)SECONDS_PER_UNIT), _(warn_name_units));
> > +                      warn_number_units,
> > +                      _(warn_name_units),
>
> Does this work correctly?  xgettext is unable to extract a string here;

I confess, I didn't test it an any language other than English.    But:

LANG=fr_FR ./locate kdfljsdlfkj
./locate: warning: database `/usr/local/var/locatedb' is more than 8
jours old (actual age is 17,0 jours)

Ouch.


> are all possible values of warn_name_units previously marked with N_()?

Yes, but I think we need better pluralisation support there (i.e. days
versus day).


> > -             _("Invalid escape sequence %s in input delimiter 
> > specification; character values must not exceed %lx."),
> > +             _("Invalid escape sequence %1$s in input delimiter 
> > specification; "
> > +               "character values must not exceed %2$lx."),
> >               s, (unsigned long)UCHAR_MAX);
>
> Not changed by your patch, but (unsigned long)UCHAR_MAX is redundant;
> gnulib guarantees that UCHAR_MAX has the correct type.  I'd kill the cast.

Leaving the type as unsigned char, or so I assume.   That gets
promoted to unsigned in order to be passed into a variadic function
call.    So I'd need to change the format from %lx to %x too, yes?

James.