Re: [PATCH i18n 03/11] i18n: parseopt: lookup help and argument translations when showing usage

[Jonathan Nieder <jrnieder@gmail.com>]

Hi again,

Nguyễn Thái Ngọc Duy wrote:

> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -490,7 +490,7 @@ static int usage_argh(const struct option *opts, FILE *outfile)

The interesting parts in this patch are strings not marked for
translation:

>  			s = literal ? "[%s]" : "[<%s>]";
>  	else
>  		s = literal ? " %s" : " <%s>";

That means the usage message will have one of the formats

	--foo[=<bar>]
	-f[<bar>]
	--foo <bar>
	-f <bar>

in all languages.  Makes sense.

> -	return fprintf(outfile, s, opts->argh ? opts->argh : "...");
> +	return fprintf(outfile, s, opts->argh ? _(opts->argh) : "...");

"bar" becomes "..." in all languages when the caller was too lazy to
fill it in.  I wonder if we should not just require argh to be
non-NULL for options that can take an argument and catch mistakes in
parse_options_check().

> @@ -508,13 +508,12 @@ static int usage_with_options_internal(struct parse_opt_ctx_t *ctx,
>  	if (!err && ctx && ctx->flags & PARSE_OPT_SHELL_EVAL)
>  		fprintf(outfile, "cat <<\\EOF\n");
>  
> -	fprintf(outfile, "usage: %s\n",
> +	fprintf_ln(outfile, _("usage: %s"),

It's too bad this doesn't share code with usage.c. :)  The prompt will
be translated in some contexts and not in others, which seems fine.

> -                                   *usagestr++);
> +                                   _(*usagestr++));

Maybe this change belongs in a separate patch that would mark the
usage strings with N_ at the same time. (*)

>  	while (*usagestr && **usagestr)
> -		fprintf(outfile, "   or: %s\n", *usagestr++);
> +		fprintf_ln(outfile, _("   or: %s"), _(*usagestr++));

Maybe worth a translators note to explain how these line up.

>  	while (*usagestr) {
> -		fprintf(outfile, "%s%s\n",
> -				**usagestr ? "    " : "",
> -				*usagestr);
> +		fprintf(outfile, "%s%s\n", **usagestr ? "    " : "",
> +			_(*usagestr));

Mph, the space is going to look wrong in other languages.

>  		usagestr++;
>  	}
>  
> @@ -528,7 +527,7 @@ static int usage_with_options_internal(struct parse_opt_ctx_t *ctx,
>  		if (opts->type == OPTION_GROUP) {
>  			fputc('\n', outfile);
>  			if (*opts->help)
> -				fprintf(outfile, "%s\n", opts->help);
> +				fprintf(outfile, "%s\n", _(opts->help));
[...]
> @@ -558,7 +557,7 @@ static int usage_with_options_internal(struct parse_opt_ctx_t *ctx,
>  			fputc('\n', outfile);
>  			pad = USAGE_OPTS_WIDTH;
>  		}
> -		fprintf(outfile, "%*s%s\n", pad + USAGE_GAP, "", opts->help);
> +		fprintf(outfile, "%*s%s\n", pad + USAGE_GAP, "", _(opts->help));
[...]
> --- a/parse-options.h
> +++ b/parse-options.h
> @@ -66,12 +66,14 @@ typedef int parse_opt_ll_cb(struct parse_opt_ctx_t *ctx,
>   *
>   * `argh`::
>   *   token to explain the kind of argument this option wants. Keep it
> - *   homogeneous across the repository.
> + *   homogeneous across the repository. Should be wrapped by N_() for
> + *   translation.
>   *
>   * `help`::
>   *   the short help associated to what the option does.
>   *   Must never be NULL (except for OPTION_END).
>   *   OPTION_GROUP uses this pointer to store the group header.
> + *   Should be wrapped by N_() for translation.
[...]
> @@ -158,7 +160,8 @@ struct option {
>  #define OPT_BOOLEAN OPT_COUNTUP
>  
>  /* parse_options() will filter out the processed options and leave the
> - * non-option arguments in argv[].
> + * non-option arguments in argv[]. usagestr strings should be marked
> + * for translation with N_().

Also might be worth splitting into a separate patch that adjusts
callers to use N_ at the same time.  Is there some easy way to catch
strings not in the po template that are passed to gettext() using a
variable (at runtime)?

Thanks for some food for thought.
Jonathan