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

[Eric Blake]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

According to James Youngman on 7/21/2007 5:49 AM:
> +++ b/find/find.c
> @@ -64,6 +64,7 @@
>  # define _(Text) Text

^^^ This is your own responsibility...

>  #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.

> @@ -487,7 +488,8 @@ wd_sanity_check(const char *thing_to_stat,
>         {
>           fstype = filesystem_type(newinfo, current_dir);
>           error (1, 0,
> -                _("%s%s changed during execution of %s (old device number 
> %ld, new device number %ld, file system type is %s) [ref %ld]"),
> +                _("%1$s%2$s changed during execution of %3$s "
> +                  "(old device number %4$ld, new device number %5$ld, file 
> system type is %6$s) [ref %7$ld]"),

Why'd you change to %1$s notation?  Since you used all of the arguments
exactly once, in order, it wasn't really necessary; also, it makes the
resulting binary slightly larger and potentially a bit slower in the C
locale.  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.  This comment applies throughout the bulk of
your patch.

> @@ -1121,12 +1124,14 @@ issue_loop_warning(const char *name, const char 
> *pathname, int level)
>         * to /a/b/c.
>         */
>        error(0, 0,
> -         _("Filesystem loop detected; %s has the same device number and 
> inode as a directory which is %d %s."),
> +         ngettext(
> +                  "Filesystem loop detected; %1$s has the same device number 
> and inode as "
> +                  "a directory which is %2$d level higher in the file system 
> hierarchy",
> +                  "Filesystem loop detected; %1$s has the same device number 
> and inode as "
> +                  "a directory which is %2$d levels higher in the file 
> system hierarchy",
> +                  (long)distance),

Good move for pluralization, but still questionable use of %1$s.

> +  else
> +    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.

> +                  _("warning: you have specified the %1$s "

Inconsistent in your use of WARNING: vs. warning:.

>  print_stats(int argc, size_t database_file_size)
>  {
> -  char hbuf[LONGEST_HUMAN_READABLE + 1];
> +  char hbuf1[LONGEST_HUMAN_READABLE + 1];
> +  char hbuf2[LONGEST_HUMAN_READABLE + 1];
> +  char hbuf3[LONGEST_HUMAN_READABLE + 1];
> +  char hbuf4[LONGEST_HUMAN_READABLE + 1];
>    
> -  printf(_("Locate database size: %s bytes\n"),
> +  printf(ngettext("Locate database size: %s byte\n",
> +               "Locate database size: %s bytes\n",
> +               database_file_size),

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));

> +  /* 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?

> @@ -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;
are all possible values of warn_name_units previously marked with N_()?

> -             _("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.

- --
Don't work too hard, make some time for fun as well!

Eric Blake             address@hidden
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.5 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFHaeWP84KuGfSFAYARAvCyAKC9aPROkCR3K1+3jHNNjRPv17B2mQCggPi9
LDj9tid3yfKjsmGHm7w0q6E=
=I04b
-----END PGP SIGNATURE-----