All of lore.kernel.org
 help / color / mirror / Atom feed
From: Duy Nguyen <pclouds@gmail.com>
To: Jiang Xin <worldhello.net@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>, Git List <git@vger.kernel.org>
Subject: Re: [PATCH] Get correct column with for options in command usage
Date: Tue, 5 Feb 2013 19:15:52 +0700	[thread overview]
Message-ID: <20130205121552.GA16601@lanh> (raw)
In-Reply-To: <0b035132df6de2cac56ac59d66b04f30e90ad760.1360049671.git.worldhello.net@gmail.com>

On Tue, Feb 05, 2013 at 03:40:32PM +0800, Jiang Xin wrote:
> Command usage would not align well if command options are translated,
> especially to CJK. Call utf8_strwidth in function usage_argh, so that
> the caller will get correct column width.

Yeah, I just noticed a misalignment in Vietnamese translation (just
two spaces to the left, hard to notice unless paying attention).

>  static int usage_argh(const struct option *opts, FILE *outfile)
>  {
> -	const char *s;
> +	const char *s, *p;
>  	int literal = (opts->flags & PARSE_OPT_LITERAL_ARGHELP) || !opts->argh;
>  	if (opts->flags & PARSE_OPT_OPTARG)
>  		if (opts->long_name)
> @@ -482,7 +482,9 @@ 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) : _("..."));
> +	p = opts->argh ? _(opts->argh) : _("...");
> +	fprintf(outfile, s, p);
> +	return utf8_strwidth(p) + strlen(s) - 2;
>  }

First of all, #include "utf8.h" is required for utf8_strwidth().

The "strlen(s) - 2" is quite sensitive to how "s" is written. How
about this? A bit longer but clearer. Your version is OK too with a
comment explaining "strlen(s) - 2".

-- 8< --
diff --git a/parse-options.c b/parse-options.c
index 67e98a6..0f803bd 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -474,6 +474,8 @@ int parse_options(int argc, const char **argv, const char *prefix,
 static int usage_argh(const struct option *opts, FILE *outfile)
 {
 	const char *s;
+	struct strbuf sb = STRBUF_INIT;
+	int columns;
 	int literal = (opts->flags & PARSE_OPT_LITERAL_ARGHELP) || !opts->argh;
 	if (opts->flags & PARSE_OPT_OPTARG)
 		if (opts->long_name)
@@ -482,7 +484,11 @@ 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) : _("..."));
+	strbuf_addf(&sb, s, opts->argh ? _(opts->argh) : _("..."));
+	fprintf(outfile, sb.buf);
+	columns = utf8_strwidth(sb.buf);
+	strbuf_release(&sb);
+	return columns;
 }
 
 #define USAGE_OPTS_WIDTH 24
-- 8< --

While looking at parse-options.c, I notice "-NUM" is not marked for
translation. I think you might want to mark it so that you can
translate "NUM" to a similar abbreviation in a local language. A
similar patch like this is required so we get columns for "-NUM"
translation instead of the number of bytes.
--
Duy

  reply	other threads:[~2013-02-05 12:15 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-05  7:40 [PATCH] Get correct column with for options in command usage Jiang Xin
2013-02-05 12:15 ` Duy Nguyen [this message]
2013-02-05 12:18   ` Duy Nguyen
2013-02-05 16:09     ` Junio C Hamano
2013-02-05 16:16   ` [PATCH v2 1/2] " Jiang Xin
2013-02-05 16:16   ` [PATCH v2 2/2] i18n: mark OPTION_NUMBER (-NUM) for translation Jiang Xin
2013-02-05 17:07     ` Junio C Hamano
2013-02-06  0:15       ` Jiang Xin
2013-02-06  2:44         ` Junio C Hamano
2013-02-06  3:02           ` Jiang Xin
2013-02-06  4:35             ` Junio C Hamano
2013-02-06 10:45               ` Duy Nguyen
2013-02-06 15:47                 ` Junio C Hamano
2013-02-08  2:10                   ` [PATCH v4] Add utf8_fprintf helper which returns correct columns Jiang Xin
2013-02-08  6:03                     ` Torsten Bögershausen
2013-02-08  6:13                       ` Torsten Bögershausen
2013-02-08  7:20                       ` Jiang Xin
2013-02-08 16:19                         ` Torsten Bögershausen
2013-02-09  6:31                           ` [PATCH v5] Add utf8_fprintf helper that " Jiang Xin
2013-02-06  1:16       ` [PATCH v3] Add utf8_fprintf helper which " Jiang Xin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130205121552.GA16601@lanh \
    --to=pclouds@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=worldhello.net@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.