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

3 messages, 2 authors, 2016-06-15 · open the first message on its own page

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:56:03

Jiang Xin [off-list ref] writes:

quoted hunk

Signed-off-by: Jiang Xin <redacted>
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 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.

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

From: Jiang Xin <hidden>
Date: 2016-06-15 22:56:04

2013/2/6 Junio C Hamano [off-list ref]:

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.

I agree, a helper named 'utf8_fprintf' in utf8.c is better.
I will send a patch latter.


-- 
Jiang Xin

[PATCH v3] Add utf8_fprintf helper which returns correct columns

From: Jiang Xin <hidden>
Date: 2016-06-15 22:56:04

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 <redacted>
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 parse-options.c |  5 +++--
 utf8.c          | 20 ++++++++++++++++++++
 utf8.h          |  1 +
 3 files changed, 24 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..52dbd 100644
--- a/utf8.c
+++ b/utf8.c

@@ -430,6 +430,26 @@ 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);
+
+	fputs(buf.buf, stream);
+	columns = utf8_strwidth(buf.buf);
+	strbuf_release(&buf);
+	return columns;
+}
+
+/*
  * Given a buffer and its encoding, return it re-encoded
  * with iconv.  If the conversion fails, returns NULL.
  */

diff --git a/utf8.h b/utf8.h
index a2142..501b2 100644
--- a/utf8.h
+++ b/utf8.h

@@ -8,6 +8,7 @@ int utf8_strwidth(const char *string);
 int is_utf8(const char *text);
 int is_encoding_utf8(const char *name);
 int same_encoding(const char *, const char *);
+int utf8_fprintf(FILE *, const char *, ...);
 
 void strbuf_add_wrapped_text(struct strbuf *buf,
 		const char *text, int indent, int indent2, int width);

-- 
1.8.1.1.370.g47b6ee8.dirty

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help