[PATCH 0/5] i18n-ize relative dates, git-help and parseopt

[PATCH 0/5] i18n-ize relative dates, git-help and parseopt

[Nguyễn Thái Ngọc Duy]

Except "git log --date=relative", the focus of this series is "git
help" and "git blah -h" (only "git help -h" for the latter because
other commands need a lot of N_() additions). I think I got everything
from "git help [options]" fully translatable.

Nguyễn Thái Ngọc Duy (5):
  i18n: keep the last \n even when text is poisoned
  i18n: mark relative dates for translation
  i18n: parseopt: lookup help and argument translations when showing
    usage
  i18n: help: mark parseopt strings for translation
  i18n: help: mark strings for translation

 Makefile            |    2 +-
 builtin/help.c      |   58 +++++++++++++++++++++++++-------------------------
 date.c              |   46 +++++++++++++++++++++++++---------------
 generate-cmdlist.sh |    2 +-
 gettext.c           |   10 ++++++++
 gettext.h           |    6 +++-
 git.c               |    2 +-
 help.c              |   34 ++++++++++++++++++-----------
 parse-options.c     |   15 ++++++-------
 9 files changed, 103 insertions(+), 72 deletions(-)

-- 
1.7.3.1.256.g2539c.dirty

[PATCH 1/5] i18n: keep the last \n even when text is poisoned

[Nguyễn Thái Ngọc Duy]

This helps maintain the layout somewhat when translatable text is
poisoned.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 gettext.c |   10 ++++++++++
 gettext.h |    6 ++++--
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/gettext.c b/gettext.c
index f75bca7..60bc2ad 100644
--- a/gettext.c
+++ b/gettext.c
@@ -24,6 +24,16 @@ int use_gettext_poison(void)
 		poison_requested = getenv("GIT_GETTEXT_POISON") ? 1 : 0;
 	return poison_requested;
 }
+
+const char *poison_text(const char *msgid)
+{
+	int len = strlen(msgid);
+	if (len && msgid[len-1] == '\n')
+		return "# GETTEXT POISON #\n";
+	else
+		return "# GETTEXT POISON #";
+}
+
 #endif
 
 #ifndef NO_GETTEXT
diff --git a/gettext.h b/gettext.h
index 57ba8bb..2313c84 100644
--- a/gettext.h
+++ b/gettext.h
@@ -38,20 +38,22 @@ static inline void git_setup_gettext(void)
 
 #ifdef GETTEXT_POISON
 extern int use_gettext_poison(void);
+extern const char *poison_text(const char *msgid);
 #else
 #define use_gettext_poison() 0
+#define poison_text(x) NULL
 #endif
 
 static inline FORMAT_PRESERVING(1) const char *_(const char *msgid)
 {
-	return use_gettext_poison() ? "# GETTEXT POISON #" : gettext(msgid);
+	return use_gettext_poison() ? poison_text(msgid) : gettext(msgid);
 }
 
 static inline FORMAT_PRESERVING(1) FORMAT_PRESERVING(2)
 const char *Q_(const char *msgid, const char *plu, unsigned long n)
 {
 	if (use_gettext_poison())
-		return "# GETTEXT POISON #";
+		return poison_text(msgid);
 	return ngettext(msgid, plu, n);
 }
 
-- 
1.7.3.1.256.g2539c.dirty

Re: [PATCH 1/5] i18n: keep the last \n even when text is poisoned

[Jonathan Nieder]

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

> --- a/gettext.c
> +++ b/gettext.c
> @@ -24,6 +24,16 @@ int use_gettext_poison(void)
>  		poison_requested = getenv("GIT_GETTEXT_POISON") ? 1 : 0;
>  	return poison_requested;
>  }
> +
> +const char *poison_text(const char *msgid)
> +{
> +	int len = strlen(msgid);
> +	if (len && msgid[len-1] == '\n')
> +		return "# GETTEXT POISON #\n";
> +	else
> +		return "# GETTEXT POISON #";

I realize this was not the motivation behind the above patch, but if
the translation of some message has to end with a newline for git to
function correctly, would we consider that a bug?

I am of two minds on that:

 - on one hand, translators tend to be trustworthy, reasonable folks
 - on the other hand, anything we can do to make the translation
   process less fussy seems like time well spent

The latter wins out for me, so I would prefer not to have this patch
so the test suite can detect important newlines that should be not be
part of the translatable string.

Just my two cents,
Jonathan

Re: [PATCH 1/5] i18n: keep the last \n even when text is poisoned

[Nguyen Thai Ngoc Duy]

2012/3/9 Jonathan Nieder <jrnieder@gmail.com>:
> I realize this was not the motivation behind the above patch, but if
> the translation of some message has to end with a newline for git to
> function correctly, would we consider that a bug?
>
> I am of two minds on that:
>
>  - on one hand, translators tend to be trustworthy, reasonable folks

We trust msgfmt and it does check \n in msgstr if there is in msgid.

>  - on the other hand, anything we can do to make the translation
>   process less fussy seems like time well spent

That's at the cost of more code to split \n out of the strings, and
there are 113 of them (some not published yet).

> The latter wins out for me, so I would prefer not to have this patch
> so the test suite can detect important newlines that should be not be
> part of the translatable string.

It's the former for me, but that's probably because I'm also a
translator :) Anyway for tricky cases where msgfmt fails to detect,
there's the role of l10n coordinators, who should review translation
and catch these errors.

> Just my two cents,
> Jonathan
-- 
Duy

Re: [PATCH 1/5] i18n: keep the last \n even when text is poisoned

[Nguyen Thai Ngoc Duy]

On Fri, Mar 9, 2012 at 8:11 AM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
> It's the former for me, but that's probably because I'm also a
> translator :) Anyway for tricky cases where msgfmt fails to detect,
> there's the role of l10n coordinators, who should review translation
> and catch these errors.

On second thought, most of call sites are [fs]printf or strbuf_addf,
and we have to update the call sites anyway to mark for translation.
If we introduce [fs]printf_ln, changes should be minimum. So now I'm
on your side too.
-- 
Duy

Re: [PATCH 1/5] i18n: keep the last \n even when text is poisoned

[Ævar Arnfjörð Bjarmason]

2012/3/8 Jonathan Nieder <jrnieder@gmail.com>:
> The latter wins out for me, so I would prefer not to have this patch
> so the test suite can detect important newlines that should be not be
> part of the translatable string.

FWIW that's why I kept it this way. I can't recall the specific cases
but I smoked out some issues with the poison feature that I wouldn't
have smoked out if it had a \n at the end.

Arguably we should make it even less user friendly and have it
randomly output 10-50 bytes of binary garbage to catch more errors.

[PATCH 2/5] i18n: mark relative dates for translation

[Nguyễn Thái Ngọc Duy]

English dates get correct plural/singular form as a side effect.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 date.c |   46 +++++++++++++++++++++++++++++-----------------
 1 files changed, 29 insertions(+), 17 deletions(-)

diff --git a/date.c b/date.c
index a5055ca..063f344 100644
--- a/date.c
+++ b/date.c
@@ -93,38 +93,46 @@ const char *show_date_relative(unsigned long time, int tz,
 {
 	unsigned long diff;
 	if (now->tv_sec < time)
-		return "in the future";
+		return _("in the future");
 	diff = now->tv_sec - time;
 	if (diff < 90) {
-		snprintf(timebuf, timebuf_size, "%lu seconds ago", diff);
+		snprintf(timebuf, timebuf_size,
+			 Q_("%lu second ago", "%lu seconds ago", diff), diff);
 		return timebuf;
 	}
 	/* Turn it into minutes */
 	diff = (diff + 30) / 60;
 	if (diff < 90) {
-		snprintf(timebuf, timebuf_size, "%lu minutes ago", diff);
+		snprintf(timebuf, timebuf_size,
+			 Q_("%lu minute ago", "%lu minutes ago", diff), diff);
 		return timebuf;
 	}
 	/* Turn it into hours */
 	diff = (diff + 30) / 60;
 	if (diff < 36) {
-		snprintf(timebuf, timebuf_size, "%lu hours ago", diff);
+		snprintf(timebuf, timebuf_size,
+			 Q_("%lu hour ago", "%lu hours ago", diff), diff);
 		return timebuf;
 	}
 	/* We deal with number of days from here on */
 	diff = (diff + 12) / 24;
 	if (diff < 14) {
-		snprintf(timebuf, timebuf_size, "%lu days ago", diff);
+		snprintf(timebuf, timebuf_size,
+			 Q_("%lu day ago", "%lu days ago", diff), diff);
 		return timebuf;
 	}
 	/* Say weeks for the past 10 weeks or so */
 	if (diff < 70) {
-		snprintf(timebuf, timebuf_size, "%lu weeks ago", (diff + 3) / 7);
+		snprintf(timebuf, timebuf_size,
+			 Q_("%lu week ago", "%lu weeks ago", (diff + 3) / 7),
+			 (diff + 3) / 7);
 		return timebuf;
 	}
 	/* Say months for the past 12 months or so */
 	if (diff < 365) {
-		snprintf(timebuf, timebuf_size, "%lu months ago", (diff + 15) / 30);
+		snprintf(timebuf, timebuf_size,
+			 Q_("%lu month ago", "%lu months ago", (diff + 15) / 30),
+			 (diff + 15) / 30);
 		return timebuf;
 	}
 	/* Give years and months for 5 years or so */
@@ -132,19 +140,23 @@ const char *show_date_relative(unsigned long time, int tz,
 		unsigned long totalmonths = (diff * 12 * 2 + 365) / (365 * 2);
 		unsigned long years = totalmonths / 12;
 		unsigned long months = totalmonths % 12;
-		int n;
-		n = snprintf(timebuf, timebuf_size, "%lu year%s",
-				years, (years > 1 ? "s" : ""));
-		if (months)
-			snprintf(timebuf + n, timebuf_size - n,
-					", %lu month%s ago",
-					months, (months > 1 ? "s" : ""));
-		else
-			snprintf(timebuf + n, timebuf_size - n, " ago");
+		if (months) {
+			struct strbuf sb = STRBUF_INIT;
+			strbuf_addf(&sb, Q_("%lu year", "%lu years", years), years);
+			/* TRANSLATORS: "%s" is "<n> years" */
+			snprintf(timebuf, timebuf_size,
+				 Q_("%s, %lu month ago", "%s, %lu months ago", months),
+				 sb.buf, months);
+			strbuf_release(&sb);
+		} else
+			snprintf(timebuf, timebuf_size,
+				 Q_("%lu year ago", "%lu years ago", years), years);
 		return timebuf;
 	}
 	/* Otherwise, just years. Centuries is probably overkill. */
-	snprintf(timebuf, timebuf_size, "%lu years ago", (diff + 183) / 365);
+	snprintf(timebuf, timebuf_size,
+		 Q_("%lu year ago", "%lu years ago", (diff + 183) / 365),
+		 (diff + 183) / 365);
 	return timebuf;
 }
 
-- 
1.7.3.1.256.g2539c.dirty

Re: [PATCH 2/5] i18n: mark relative dates for translation

[Jonathan Nieder]

Hi,

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

> English dates get correct plural/singular form as a side effect.
[...]
> +++ b/date.c
> @@ -93,38 +93,46 @@ const char *show_date_relative(unsigned long time, int tz,
[...]
>  	if (diff < 90) {
> -		snprintf(timebuf, timebuf_size, "%lu seconds ago", diff);
> +		snprintf(timebuf, timebuf_size,
> +			 Q_("%lu second ago", "%lu seconds ago", diff), diff);
>  		return timebuf;

This leaves me vaguely nervous --- sure, no language is going to use
an expression for "<n> years" that is more than 200 bytes long, but if
one does, it would get truncated.

Would something like the following (untested) on top make sense?

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
My other worry is that

> +	struct strbuf sb = STRBUF_INIT;
> +	strbuf_addf(&sb, Q_("%lu year", "%lu years", years), years);
> +	/* TRANSLATORS: "%s" is "<n> years" */
> +	snprintf(timebuf, timebuf_size,
> +	         Q_("%s, %lu month ago", "%s, %lu months ago", months),
> +	         sb.buf, months);
> +	strbuf_release(&sb);

seems excessively complicated.  How do translations normally deal with
cases like this of strings with multiple numbers in them?

 cache.h     |    5 ++--
 date.c      |   73 +++++++++++++++++++++++++++++++----------------------------
 test-date.c |    7 +++---
 3 files changed, 45 insertions(+), 40 deletions(-)

diff --git i/cache.h w/cache.h
index e5e1aa4e..ca704bdd 100644
--- i/cache.h
+++ w/cache.h
@@ -906,10 +906,9 @@ enum date_mode {
 };
 
 const char *show_date(unsigned long time, int timezone, enum date_mode mode);
-const char *show_date_relative(unsigned long time, int tz,
+void show_date_relative(unsigned long time, int tz,
 			       const struct timeval *now,
-			       char *timebuf,
-			       size_t timebuf_size);
+			       struct strbuf *timebuf);
 int parse_date(const char *date, char *buf, int bufsize);
 int parse_date_basic(const char *date, unsigned long *timestamp, int *offset);
 void datestamp(char *buf, int bufsize);
diff --git i/date.c w/date.c
index 063f3449..dfe906dd 100644
--- i/date.c
+++ w/date.c
@@ -86,54 +86,55 @@ static int local_tzoffset(unsigned long time)
 	return offset * eastwest;
 }
 
-const char *show_date_relative(unsigned long time, int tz,
+void show_date_relative(unsigned long time, int tz,
 			       const struct timeval *now,
-			       char *timebuf,
-			       size_t timebuf_size)
+			       struct strbuf *timebuf)
 {
 	unsigned long diff;
-	if (now->tv_sec < time)
-		return _("in the future");
+	if (now->tv_sec < time) {
+		strbuf_addstr(timebuf, _("in the future"));
+		return;
+	}
 	diff = now->tv_sec - time;
 	if (diff < 90) {
-		snprintf(timebuf, timebuf_size,
+		strbuf_addf(timebuf,
 			 Q_("%lu second ago", "%lu seconds ago", diff), diff);
-		return timebuf;
+		return;
 	}
 	/* Turn it into minutes */
 	diff = (diff + 30) / 60;
 	if (diff < 90) {
-		snprintf(timebuf, timebuf_size,
+		strbuf_addf(timebuf,
 			 Q_("%lu minute ago", "%lu minutes ago", diff), diff);
-		return timebuf;
+		return;
 	}
 	/* Turn it into hours */
 	diff = (diff + 30) / 60;
 	if (diff < 36) {
-		snprintf(timebuf, timebuf_size,
+		strbuf_addf(timebuf,
 			 Q_("%lu hour ago", "%lu hours ago", diff), diff);
-		return timebuf;
+		return;
 	}
 	/* We deal with number of days from here on */
 	diff = (diff + 12) / 24;
 	if (diff < 14) {
-		snprintf(timebuf, timebuf_size,
+		strbuf_addf(timebuf,
 			 Q_("%lu day ago", "%lu days ago", diff), diff);
-		return timebuf;
+		return;
 	}
 	/* Say weeks for the past 10 weeks or so */
 	if (diff < 70) {
-		snprintf(timebuf, timebuf_size,
+		strbuf_addf(timebuf,
 			 Q_("%lu week ago", "%lu weeks ago", (diff + 3) / 7),
 			 (diff + 3) / 7);
-		return timebuf;
+		return;
 	}
 	/* Say months for the past 12 months or so */
 	if (diff < 365) {
-		snprintf(timebuf, timebuf_size,
+		strbuf_addf(timebuf,
 			 Q_("%lu month ago", "%lu months ago", (diff + 15) / 30),
 			 (diff + 15) / 30);
-		return timebuf;
+		return;
 	}
 	/* Give years and months for 5 years or so */
 	if (diff < 1825) {
@@ -141,40 +142,42 @@ const char *show_date_relative(unsigned long time, int tz,
 		unsigned long years = totalmonths / 12;
 		unsigned long months = totalmonths % 12;
 		if (months) {
 			struct strbuf sb = STRBUF_INIT;
 			strbuf_addf(&sb, Q_("%lu year", "%lu years", years), years);
 			/* TRANSLATORS: "%s" is "<n> years" */
-			snprintf(timebuf, timebuf_size,
+			strbuf_addf(timebuf,
 				 Q_("%s, %lu month ago", "%s, %lu months ago", months),
 				 sb.buf, months);
 			strbuf_release(&sb);
 		} else
-			snprintf(timebuf, timebuf_size,
+			strbuf_addf(timebuf,
 				 Q_("%lu year ago", "%lu years ago", years), years);
-		return timebuf;
+		return;
 	}
 	/* Otherwise, just years. Centuries is probably overkill. */
-	snprintf(timebuf, timebuf_size,
+	strbuf_addf(timebuf,
 		 Q_("%lu year ago", "%lu years ago", (diff + 183) / 365),
 		 (diff + 183) / 365);
-	return timebuf;
 }
 
 const char *show_date(unsigned long time, int tz, enum date_mode mode)
 {
 	struct tm *tm;
-	static char timebuf[200];
+	static struct strbuf timebuf = STRBUF_INIT;
 
 	if (mode == DATE_RAW) {
-		snprintf(timebuf, sizeof(timebuf), "%lu %+05d", time, tz);
-		return timebuf;
+		strbuf_reset(&timebuf);
+		strbuf_addf(&timebuf, "%lu %+05d", time, tz);
+		return timebuf.buf;
 	}
 
 	if (mode == DATE_RELATIVE) {
 		struct timeval now;
+
+		strbuf_reset(&timebuf);
 		gettimeofday(&now, NULL);
-		return show_date_relative(time, tz, &now,
-					  timebuf, sizeof(timebuf));
+		show_date_relative(time, tz, &now, &timebuf);
+		return timebuf.buf;
 	}
 
 	if (mode == DATE_LOCAL)
@@ -183,23 +186,25 @@ const char *show_date(unsigned long time, int tz, enum date_mode mode)
 	tm = time_to_tm(time, tz);
 	if (!tm)
 		return NULL;
+
+	strbuf_reset(&timebuf);
 	if (mode == DATE_SHORT)
-		sprintf(timebuf, "%04d-%02d-%02d", tm->tm_year + 1900,
+		strbuf_addf(&timebuf, "%04d-%02d-%02d", tm->tm_year + 1900,
 				tm->tm_mon + 1, tm->tm_mday);
 	else if (mode == DATE_ISO8601)
-		sprintf(timebuf, "%04d-%02d-%02d %02d:%02d:%02d %+05d",
+		strbuf_addf(&timebuf, "%04d-%02d-%02d %02d:%02d:%02d %+05d",
 				tm->tm_year + 1900,
 				tm->tm_mon + 1,
 				tm->tm_mday,
 				tm->tm_hour, tm->tm_min, tm->tm_sec,
 				tz);
 	else if (mode == DATE_RFC2822)
-		sprintf(timebuf, "%.3s, %d %.3s %d %02d:%02d:%02d %+05d",
+		strbuf_addf(&timebuf, "%.3s, %d %.3s %d %02d:%02d:%02d %+05d",
 			weekday_names[tm->tm_wday], tm->tm_mday,
 			month_names[tm->tm_mon], tm->tm_year + 1900,
 			tm->tm_hour, tm->tm_min, tm->tm_sec, tz);
 	else
-		sprintf(timebuf, "%.3s %.3s %d %02d:%02d:%02d %d%c%+05d",
+		strbuf_addf(&timebuf, "%.3s %.3s %d %02d:%02d:%02d %d%c%+05d",
 				weekday_names[tm->tm_wday],
 				month_names[tm->tm_mon],
 				tm->tm_mday,
@@ -207,7 +212,7 @@ const char *show_date(unsigned long time, int tz, enum date_mode mode)
 				tm->tm_year + 1900,
 				(mode == DATE_LOCAL) ? 0 : ' ',
 				tz);
-	return timebuf;
+	return timebuf.buf;
 }
 
 /*
diff --git i/test-date.c w/test-date.c
index 6bcd5b03..10afaabb 100644
--- i/test-date.c
+++ w/test-date.c
@@ -7,13 +7,14 @@ static const char *usage_msg = "\n"
 
 static void show_dates(char **argv, struct timeval *now)
 {
-	char buf[128];
+	struct strbuf buf = STRBUF_INIT;
 
 	for (; *argv; argv++) {
 		time_t t = atoi(*argv);
-		show_date_relative(t, 0, now, buf, sizeof(buf));
-		printf("%s -> %s\n", *argv, buf);
+		show_date_relative(t, 0, now, &buf);
+		printf("%s -> %s\n", *argv, buf.buf);
 	}
+	strbuf_release(&buf);
 }
 
 static void parse_dates(char **argv, struct timeval *now)

Re: [PATCH 2/5] i18n: mark relative dates for translation

[Johannes Sixt]

Am 15.03.2012 19:51, schrieb Jonathan Nieder:
> @@ -141,40 +142,42 @@ const char *show_date_relative(unsigned long time, int tz,
>  		unsigned long years = totalmonths / 12;
>  		unsigned long months = totalmonths % 12;
>  		if (months) {
>  			struct strbuf sb = STRBUF_INIT;
>  			strbuf_addf(&sb, Q_("%lu year", "%lu years", years), years);
>  			/* TRANSLATORS: "%s" is "<n> years" */
> -			snprintf(timebuf, timebuf_size,
> +			strbuf_addf(timebuf,

OT: You have 6 lines of context here, unlike all other hunks, which have
only 3 lines. Something weird or simple to explain?

-- Hannes

Re: [PATCH 2/5] i18n: mark relative dates for translation

[Jonathan Nieder]

Johannes Sixt wrote:
> Am 15.03.2012 19:51, schrieb Jonathan Nieder:

>> @@ -141,40 +142,42 @@ const char *show_date_relative(unsigned long time, int tz,
>>  		unsigned long years = totalmonths / 12;
>>  		unsigned long months = totalmonths % 12;
>>  		if (months) {
>>  			struct strbuf sb = STRBUF_INIT;
>>  			strbuf_addf(&sb, Q_("%lu year", "%lu years", years), years);
>>  			/* TRANSLATORS: "%s" is "<n> years" */
>> -			snprintf(timebuf, timebuf_size,
>> +			strbuf_addf(timebuf,
>
> OT: You have 6 lines of context here, unlike all other hunks, which have
> only 3 lines. Something weird or simple to explain?

Just manual patch mangling. ;-)

Re: [PATCH 2/5] i18n: mark relative dates for translation

[Nguyen Thai Ngoc Duy]

On Fri, Mar 16, 2012 at 1:51 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Hi,
>
> Nguyễn Thái Ngọc Duy wrote:
>
>> English dates get correct plural/singular form as a side effect.
> [...]
>> +++ b/date.c
>> @@ -93,38 +93,46 @@ const char *show_date_relative(unsigned long time, int tz,
> [...]
>>       if (diff < 90) {
>> -             snprintf(timebuf, timebuf_size, "%lu seconds ago", diff);
>> +             snprintf(timebuf, timebuf_size,
>> +                      Q_("%lu second ago", "%lu seconds ago", diff), diff);
>>               return timebuf;
>
> This leaves me vaguely nervous --- sure, no language is going to use
> an expression for "<n> years" that is more than 200 bytes long, but if
> one does, it would get truncated.
>
> Would something like the following (untested) on top make sense?

It does. I will reuse your patch next time.

> My other worry is that
>
>> +     struct strbuf sb = STRBUF_INIT;
>> +     strbuf_addf(&sb, Q_("%lu year", "%lu years", years), years);
>> +     /* TRANSLATORS: "%s" is "<n> years" */
>> +     snprintf(timebuf, timebuf_size,
>> +              Q_("%s, %lu month ago", "%s, %lu months ago", months),
>> +              sb.buf, months);
>> +     strbuf_release(&sb);
>
> seems excessively complicated.  How do translations normally deal with
> cases like this of strings with multiple numbers in them?

I don't recall any similar cases. A search for 'ago"' in all gnome
translations I have only shows "<one number> ago", or "%d blah ago,
<absolute time>". The closet is probably strftime, where translators
are free to reorder date and time items, something like this

/* Translators: the first %s is the number of months, the second the
number of years */
sprintf(.., "%s, %s ago", month_string, year_string);

Or we can just round it up and show only one number.
-- 
Duy

[PATCH 3/5] i18n: parseopt: lookup help and argument translations when showing usage

[Nguyễn Thái Ngọc Duy]

"struct option" and the help usage array needs N_() marking on help
and argument text for this to work.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 parse-options.c |   15 +++++++--------
 1 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index 850cfa7..6247c20 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -490,7 +490,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 fprintf(outfile, s, opts->argh ? gettext(opts->argh) : "...");
 }
 
 #define USAGE_OPTS_WIDTH 24
@@ -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", *usagestr++);
+	fprintf(outfile, _("usage: %s\n"), gettext(*usagestr++));
 	while (*usagestr && **usagestr)
-		fprintf(outfile, "   or: %s\n", *usagestr++);
+		fprintf(outfile, _("   or: %s\n"), gettext(*usagestr++));
 	while (*usagestr) {
-		fprintf(outfile, "%s%s\n",
-				**usagestr ? "    " : "",
-				*usagestr);
+		fprintf(outfile, "%s%s\n", **usagestr ? "    " : "",
+			gettext(*usagestr));
 		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", gettext(opts->help));
 			continue;
 		}
 		if (!full && (opts->flags & PARSE_OPT_HIDDEN))
@@ -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, "", gettext(opts->help));
 	}
 	fputc('\n', outfile);
 
-- 
1.7.3.1.256.g2539c.dirty

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

[Jonathan Nieder]

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)
>  			s = literal ? "[%s]" : "[<%s>]";
>  	else
>  		s = literal ? " %s" : " <%s>";
> -	return fprintf(outfile, s, opts->argh ? opts->argh : "...");
> +	return fprintf(outfile, s, opts->argh ? gettext(opts->argh) : "...");

It is ok to use _() with an argument that is not a string literal, and
in fact it is needed if the argument is to be poisoned.

Maybe it would be worth resending the patch to make the gettext()
identifier not available in NO_GETTEXT builds, so this kind of thing
is easier to catch.

Thanks and hope that helps,
Jonathan

[PATCH 4/5] i18n: help: mark parseopt strings for translation

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 A demonstration of the previous patch.

 builtin/help.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/builtin/help.c b/builtin/help.c
index 61ff798..22756bd 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -32,17 +32,17 @@ enum help_format {
 static int show_all = 0;
 static enum help_format help_format = HELP_FORMAT_NONE;
 static struct option builtin_help_options[] = {
-	OPT_BOOLEAN('a', "all", &show_all, "print all available commands"),
-	OPT_SET_INT('m', "man", &help_format, "show man page", HELP_FORMAT_MAN),
-	OPT_SET_INT('w', "web", &help_format, "show manual in web browser",
+	OPT_BOOLEAN('a', "all", &show_all, N_("print all available commands")),
+	OPT_SET_INT('m', "man", &help_format, N_("show man page"), HELP_FORMAT_MAN),
+	OPT_SET_INT('w', "web", &help_format, N_("show manual in web browser"),
 			HELP_FORMAT_WEB),
-	OPT_SET_INT('i', "info", &help_format, "show info page",
+	OPT_SET_INT('i', "info", &help_format, N_("show info page"),
 			HELP_FORMAT_INFO),
 	OPT_END(),
 };
 
 static const char * const builtin_help_usage[] = {
-	"git help [--all] [--man|--web|--info] [command]",
+	N_("git help [--all] [--man|--web|--info] [command]"),
 	NULL
 };
 
-- 
1.7.3.1.256.g2539c.dirty

[PATCH 5/5] i18n: help: mark strings for translation

[Nguyễn Thái Ngọc Duy]

This patch also marks most common commands' synopsis for translation
so that "git help" gives a friendly listing.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Makefile            |    2 +-
 builtin/help.c      |   48 ++++++++++++++++++++++++------------------------
 generate-cmdlist.sh |    2 +-
 git.c               |    2 +-
 help.c              |   34 +++++++++++++++++++++-------------
 5 files changed, 48 insertions(+), 40 deletions(-)

diff --git a/Makefile b/Makefile
index be1957a..34cf971 100644
--- a/Makefile
+++ b/Makefile
@@ -2282,7 +2282,7 @@ XGETTEXT_FLAGS_C = $(XGETTEXT_FLAGS) --language=C \
 	--keyword=_ --keyword=N_ --keyword="Q_:1,2"
 XGETTEXT_FLAGS_SH = $(XGETTEXT_FLAGS) --language=Shell
 XGETTEXT_FLAGS_PERL = $(XGETTEXT_FLAGS) --keyword=__ --language=Perl
-LOCALIZED_C := $(C_OBJ:o=c)
+LOCALIZED_C := $(C_OBJ:o=c) common-cmds.h
 LOCALIZED_SH := $(SCRIPT_SH)
 LOCALIZED_PERL := $(SCRIPT_PERL)
 
diff --git a/builtin/help.c b/builtin/help.c
index 22756bd..3d0f8de 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -54,7 +54,7 @@ static enum help_format parse_help_format(const char *format)
 		return HELP_FORMAT_INFO;
 	if (!strcmp(format, "web") || !strcmp(format, "html"))
 		return HELP_FORMAT_WEB;
-	die("unrecognized help format '%s'", format);
+	die(_("unrecognized help format '%s'"), format);
 }
 
 static const char *get_man_viewer_info(const char *name)
@@ -82,7 +82,7 @@ static int check_emacsclient_version(void)
 	ec_process.err = -1;
 	ec_process.stdout_to_stderr = 1;
 	if (start_command(&ec_process))
-		return error("Failed to start emacsclient.");
+		return error(_("Failed to start emacsclient."));
 
 	strbuf_read(&buffer, ec_process.err, 20);
 	close(ec_process.err);
@@ -95,7 +95,7 @@ static int check_emacsclient_version(void)
 
 	if (prefixcmp(buffer.buf, "emacsclient")) {
 		strbuf_release(&buffer);
-		return error("Failed to parse emacsclient version.");
+		return error(_("Failed to parse emacsclient version."));
 	}
 
 	strbuf_remove(&buffer, 0, strlen("emacsclient"));
@@ -103,7 +103,7 @@ static int check_emacsclient_version(void)
 
 	if (version < 22) {
 		strbuf_release(&buffer);
-		return error("emacsclient version '%d' too old (< 22).",
+		return error(_("emacsclient version '%d' too old (< 22)."),
 			version);
 	}
 
@@ -121,7 +121,7 @@ static void exec_woman_emacs(const char *path, const char *page)
 			path = "emacsclient";
 		strbuf_addf(&man_page, "(woman \"%s\")", page);
 		execlp(path, "emacsclient", "-e", man_page.buf, (char *)NULL);
-		warning("failed to exec '%s': %s", path, strerror(errno));
+		warning(_("failed to exec '%s': %s"), path, strerror(errno));
 	}
 }
 
@@ -149,7 +149,7 @@ static void exec_man_konqueror(const char *path, const char *page)
 			path = "kfmclient";
 		strbuf_addf(&man_page, "man:%s(1)", page);
 		execlp(path, filename, "newTab", man_page.buf, (char *)NULL);
-		warning("failed to exec '%s': %s", path, strerror(errno));
+		warning(_("failed to exec '%s': %s"), path, strerror(errno));
 	}
 }
 
@@ -158,7 +158,7 @@ static void exec_man_man(const char *path, const char *page)
 	if (!path)
 		path = "man";
 	execlp(path, "man", page, (char *)NULL);
-	warning("failed to exec '%s': %s", path, strerror(errno));
+	warning(_("failed to exec '%s': %s"), path, strerror(errno));
 }
 
 static void exec_man_cmd(const char *cmd, const char *page)
@@ -166,7 +166,7 @@ static void exec_man_cmd(const char *cmd, const char *page)
 	struct strbuf shell_cmd = STRBUF_INIT;
 	strbuf_addf(&shell_cmd, "%s %s", cmd, page);
 	execl("/bin/sh", "sh", "-c", shell_cmd.buf, (char *)NULL);
-	warning("failed to exec '%s': %s", cmd, strerror(errno));
+	warning(_("failed to exec '%s': %s"), cmd, strerror(errno));
 }
 
 static void add_man_viewer(const char *name)
@@ -206,8 +206,8 @@ static int add_man_viewer_path(const char *name,
 	if (supported_man_viewer(name, len))
 		do_add_man_viewer_info(name, len, value);
 	else
-		warning("'%s': path for unsupported man viewer.\n"
-			"Please consider using 'man.<tool>.cmd' instead.",
+		warning(_("'%s': path for unsupported man viewer.\n"
+			  "Please consider using 'man.<tool>.cmd' instead."),
 			name);
 
 	return 0;
@@ -218,8 +218,8 @@ static int add_man_viewer_cmd(const char *name,
 			      const char *value)
 {
 	if (supported_man_viewer(name, len))
-		warning("'%s': cmd for supported man viewer.\n"
-			"Please consider using 'man.<tool>.path' instead.",
+		warning(_("'%s': cmd for supported man viewer.\n"
+			  "Please consider using 'man.<tool>.path' instead."),
 			name);
 	else
 		do_add_man_viewer_info(name, len, value);
@@ -280,11 +280,11 @@ void list_common_cmds_help(void)
 			longest = strlen(common_cmds[i].name);
 	}
 
-	puts("The most commonly used git commands are:");
+	puts(_("The most commonly used git commands are:"));
 	for (i = 0; i < ARRAY_SIZE(common_cmds); i++) {
 		printf("   %s   ", common_cmds[i].name);
 		mput_char(' ', longest - strlen(common_cmds[i].name));
-		puts(common_cmds[i].help);
+		puts(gettext(common_cmds[i].help));
 	}
 }
 
@@ -348,7 +348,7 @@ static void exec_viewer(const char *name, const char *page)
 	else if (info)
 		exec_man_cmd(info, page);
 	else
-		warning("'%s': unknown man viewer.", name);
+		warning(_("'%s': unknown man viewer."), name);
 }
 
 static void show_man_page(const char *git_cmd)
@@ -365,7 +365,7 @@ static void show_man_page(const char *git_cmd)
 	if (fallback)
 		exec_viewer(fallback, page);
 	exec_viewer("man", page);
-	die("no man viewer handled the request");
+	die(_("no man viewer handled the request"));
 }
 
 static void show_info_page(const char *git_cmd)
@@ -373,7 +373,7 @@ static void show_info_page(const char *git_cmd)
 	const char *page = cmd_to_page(git_cmd);
 	setenv("INFOPATH", system_path(GIT_INFO_PATH), 1);
 	execlp("info", "info", "gitman", page, (char *)NULL);
-	die("no info viewer handled the request");
+	die(_("no info viewer handled the request"));
 }
 
 static void get_html_page_path(struct strbuf *page_path, const char *page)
@@ -384,7 +384,7 @@ static void get_html_page_path(struct strbuf *page_path, const char *page)
 	/* Check that we have a git documentation directory. */
 	if (stat(mkpath("%s/git.html", html_path), &st)
 	    || !S_ISREG(st.st_mode))
-		die("'%s': not a documentation directory.", html_path);
+		die(_("'%s': not a documentation directory."), html_path);
 
 	strbuf_init(page_path, 0);
 	strbuf_addf(page_path, "%s/%s.html", html_path, page);
@@ -424,16 +424,16 @@ int cmd_help(int argc, const char **argv, const char *prefix)
 	parsed_help_format = help_format;
 
 	if (show_all) {
-		printf("usage: %s\n\n", git_usage_string);
-		list_commands("git commands", &main_cmds, &other_cmds);
-		printf("%s\n", git_more_info_string);
+		printf(_("usage: %s\n\n"), gettext(git_usage_string));
+		list_commands(_("git commands"), &main_cmds, &other_cmds);
+		printf("%s\n", gettext(git_more_info_string));
 		return 0;
 	}
 
 	if (!argv[0]) {
-		printf("usage: %s\n\n", git_usage_string);
+		printf(_("usage: %s\n\n"), gettext(git_usage_string));
 		list_common_cmds_help();
-		printf("\n%s\n", git_more_info_string);
+		printf("\n%s\n", gettext(git_more_info_string));
 		return 0;
 	}
 
@@ -445,7 +445,7 @@ int cmd_help(int argc, const char **argv, const char *prefix)
 
 	alias = alias_lookup(argv[0]);
 	if (alias && !is_git_command(argv[0])) {
-		printf("`git %s' is aliased to `%s'\n", argv[0], alias);
+		printf(_("`git %s' is aliased to `%s'\n"), argv[0], alias);
 		return 0;
 	}
 
diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh
index 1093ef4..9a4c9b9 100755
--- a/generate-cmdlist.sh
+++ b/generate-cmdlist.sh
@@ -16,7 +16,7 @@ do
      /^NAME/,/git-'"$cmd"'/H
      ${
 	    x
-	    s/.*git-'"$cmd"' - \(.*\)/  {"'"$cmd"'", "\1"},/
+	    s/.*git-'"$cmd"' - \(.*\)/  {"'"$cmd"'", N_("\1")},/
 	    p
      }' "Documentation/git-$cmd.txt"
 done
diff --git a/git.c b/git.c
index 3805616..4486deb 100644
--- a/git.c
+++ b/git.c
@@ -13,7 +13,7 @@ const char git_usage_string[] =
 	"           <command> [<args>]";
 
 const char git_more_info_string[] =
-	"See 'git help <command>' for more information on a specific command.";
+	N_("See 'git help <command>' for more information on a specific command.");
 
 static struct startup_info git_startup_info;
 static int use_pager = -1;
diff --git a/help.c b/help.c
index 14eefc9..69927d8 100644
--- a/help.c
+++ b/help.c
@@ -217,8 +217,9 @@ void list_commands(const char *title, struct cmdnames *main_cmds,
 
 	if (main_cmds->cnt) {
 		const char *exec_path = git_exec_path();
-		printf("available %s in '%s'\n", title, exec_path);
-		printf("----------------");
+		printf(_("available %s in '%s'\n"), title, exec_path);
+		/* TRANSLATIONS: this must align with "available %s in '%s'\n"*/
+		printf(_("----------------"));
 		mput_char('-', strlen(title) + strlen(exec_path));
 		putchar('\n');
 		pretty_print_string_list(main_cmds, longest);
@@ -226,8 +227,12 @@ void list_commands(const char *title, struct cmdnames *main_cmds,
 	}
 
 	if (other_cmds->cnt) {
-		printf("%s available from elsewhere on your $PATH\n", title);
-		printf("---------------------------------------");
+		printf(_("%s available from elsewhere on your $PATH\n"), title);
+		/* TRANSLATIONS:
+		 * this must align with
+		 * "%s available from elsewhere on your $PATH\n"
+		 */
+		printf(_("---------------------------------------"));
 		mput_char('-', strlen(title));
 		putchar('\n');
 		pretty_print_string_list(other_cmds, longest);
@@ -341,7 +346,7 @@ const char *help_unknown_cmd(const char *cmd)
 	      sizeof(*main_cmds.names), levenshtein_compare);
 
 	if (!main_cmds.cnt)
-		die ("Uh oh. Your system reports no Git commands at all.");
+		die(_("Uh oh. Your system reports no Git commands at all."));
 
 	/* skip and count prefix matches */
 	for (n = 0; n < main_cmds.cnt && !main_cmds.names[n]->len; n++)
@@ -362,23 +367,26 @@ const char *help_unknown_cmd(const char *cmd)
 		const char *assumed = main_cmds.names[0]->name;
 		main_cmds.names[0] = NULL;
 		clean_cmdnames(&main_cmds);
-		fprintf(stderr, "WARNING: You called a Git command named '%s', "
-			"which does not exist.\n"
-			"Continuing under the assumption that you meant '%s'\n",
+		fprintf(stderr,
+			_("WARNING: You called a Git command named '%s', "
+			  "which does not exist.\n"
+			  "Continuing under the assumption that you meant '%s'\n"),
 			cmd, assumed);
 		if (autocorrect > 0) {
-			fprintf(stderr, "in %0.1f seconds automatically...\n",
+			fprintf(stderr, _("in %0.1f seconds automatically...\n"),
 				(float)autocorrect/10.0);
 			poll(NULL, 0, autocorrect * 100);
 		}
 		return assumed;
 	}
 
-	fprintf(stderr, "git: '%s' is not a git command. See 'git --help'.\n", cmd);
+	fprintf(stderr, _("git: '%s' is not a git command. See 'git --help'.\n"), cmd);
 
 	if (SIMILAR_ENOUGH(best_similarity)) {
-		fprintf(stderr, "\nDid you mean %s?\n",
-			n < 2 ? "this": "one of these");
+		fprintf(stderr,
+			Q_("\nDid you mean this?\n",
+			   "\nDid you mean one of these?\n",
+			   n));
 
 		for (i = 0; i < n; i++)
 			fprintf(stderr, "\t%s\n", main_cmds.names[i]->name);
@@ -389,6 +397,6 @@ const char *help_unknown_cmd(const char *cmd)
 
 int cmd_version(int argc, const char **argv, const char *prefix)
 {
-	printf("git version %s\n", git_version_string);
+	printf(_("git version %s\n"), git_version_string);
 	return 0;
 }
-- 
1.7.3.1.256.g2539c.dirty