Re: [PATCH v4] Add utf8_fprintf helper which returns correct columns

["Torsten Bögershausen" <tboegi@web.de>]

On 08.02.13 03:10, Jiang Xin wrote:
> Since command usages can be translated, they may not align well especially
> when they are translated to CJK. A wrapper utf8_fprintf can help to return
> the correct columns required.
> 
> 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 |  5 +++--
>  utf8.c          | 22 ++++++++++++++++++++++
>  utf8.h          |  1 +
>  3 files changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/parse-options.c b/parse-options.c
> index 67e98..a6ce9e 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -3,6 +3,7 @@
>  #include "cache.h"
>  #include "commit.h"
>  #include "color.h"
> +#include "utf8.h"
>  
>  static int parse_options_usage(struct parse_opt_ctx_t *ctx,
>  			       const char * const *usagestr,
> @@ -482,7 +483,7 @@ static int usage_argh(const struct option *opts, FILE *outfile)
>  			s = literal ? "[%s]" : "[<%s>]";
>  	else
>  		s = literal ? " %s" : " <%s>";
> -	return fprintf(outfile, s, opts->argh ? _(opts->argh) : _("..."));
> +	return utf8_fprintf(outfile, s, opts->argh ? _(opts->argh) : _("..."));
>  }
>  
>  #define USAGE_OPTS_WIDTH 24
> @@ -541,7 +542,7 @@ static int usage_with_options_internal(struct parse_opt_ctx_t *ctx,
>  		if (opts->long_name)
>  			pos += fprintf(outfile, "--%s", opts->long_name);
>  		if (opts->type == OPTION_NUMBER)
> -			pos += fprintf(outfile, "-NUM");
> +			pos += utf8_fprintf(outfile, _("-NUM"));
>  
>  		if ((opts->flags & PARSE_OPT_LITERAL_ARGHELP) ||
>  		    !(opts->flags & PARSE_OPT_NOARG))
> diff --git a/utf8.c b/utf8.c
> index a4ee6..05925 100644
> --- a/utf8.c
> +++ b/utf8.c
> @@ -430,6 +430,28 @@ int same_encoding(const char *src, const char *dst)
>  }
>  
>  /*
> + * Wrapper for fprintf and returns the total number of columns required
> + * for the printed string, assuming that the string is utf8.
> + */
> +int utf8_fprintf(FILE *stream, const char *format, ...)
> +{
> +	struct strbuf buf = STRBUF_INIT;
> +	va_list arg;
> +	int columns;
> +
> +	va_start (arg, format);
> +	strbuf_vaddf(&buf, format, arg);
> +	va_end (arg);
> +
> +	columns = fputs(buf.buf, stream);
> +	/* If no error occurs, returns columns really required with utf8_strwidth. */
> +	if (0 <= columns)
> +		columns = utf8_strwidth(buf.buf);
> +	strbuf_release(&buf);
> +	return columns;
> +}
> +

I don't think we handle the return code from fputs() correctly.

Please dee below for specifications on fprintf(),
something like the following could do:
 
int utf8_fprintf(FILE *stream, const char *format, ...)
{
	struct strbuf buf = STRBUF_INIT;
	va_list arg;
	int columns = 0;

	va_start (arg, format);
	strbuf_vaddf(&buf, format, arg);
	va_end (arg);

	if (EOF != fputs(buf.buf, stream))
		columns = utf8_strwidth(buf.buf);
	strbuf_release(&buf);
	return columns;
}

And as a side note: would fprintf_strwidth() be a better name?


Linux:
RETURN VALUE
       fputc(), putc() and  putchar()  return  the  character  written  as  an
       unsigned char cast to an int or EOF on error.

       puts()  and  fputs()  return a nonnegative number on success, or EOF on
       error.

Mac OS:
COMPATIBILITY
     fputs() now returns a non-negative number (as opposed to 0) on successful
     completion.  As a result, many tests (e.g., "fputs() == 0", "fputs() !=
     0") do not give the desired result.  Use "fputs() != EOF" or "fputs() ==
     EOF" to determine success or failure.

Posix:
RETURN VALUE
       Upon successful completion, fputs() shall return a non-negative number.
       Otherwise, it shall return EOF, set an error indicator for the  stream,
       and set errno to indicate the error.