Re: [PATCH v2 2/2] i18n: mark OPTION_NUMBER (-NUM) for translation

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 5, 2013, 17:07 UTC
Message-ID: <7vd2weu1sq.fsf@alter.siamese.dyndns.org>
In-Reply-To: <8d6d4d869ea58e0a26b3bb6377fc102728948997.1360080194.git.worldhello.net@gmail.com>

Jiang Xin <worldhello.net@gmail.com> writes:

Show 29 quoted lines

> Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  parse-options.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/parse-options.c b/parse-options.c
> index cd029f..be916 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -497,6 +497,8 @@ static int usage_with_options_internal(struct parse_opt_ctx_t *ctx,
>  				       const struct option *opts, int full, int err)
>  {
>  	FILE *outfile = err ? stderr : stdout;
> +	const char *opt_num_buff = _("-NUM");
> +	int opt_num_size = utf8_strwidth(opt_num_buff);
>  
>  	if (!usagestr)
>  		return PARSE_OPT_HELP;
> @@ -544,8 +546,10 @@ static int usage_with_options_internal(struct parse_opt_ctx_t *ctx,
>  			pos += fprintf(outfile, ", ");
>  		if (opts->long_name)
>  			pos += fprintf(outfile, "--%s", opts->long_name);
> -		if (opts->type == OPTION_NUMBER)
> -			pos += fprintf(outfile, "-NUM");
> +		if (opts->type == OPTION_NUMBER) {
> +			fputs(opt_num_buff, outfile);
> +			pos += opt_num_size;
> +		}

I somehow suspect that this is going in a direction that makes this piece of code much less maintainable.

Look at the entire function and see how many places you do fprintf on strings that are marked with _(). short_name and long_name are not likely to be translated, but everything else is, especially multiple places that show _(opts->help) neither of these patches touch.

I wonder if it makes more sense to add a helper function that returns the number of column positions (not bytes) with a signature similar to fprintf() and use that throughout the function instead.