threads / patch / 29880

patch, 5 partsi18n-ize relative dates, git-help and parseopt

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

## tl;dr

15 messages between Mar 8, 2012 and Mar 16, 2012. Diffs are folded; open one to read it.

replies: 14people: 4as markdown or json

Nguyễn Thái Ngọc Duy· Mar 8, 2012, 09:16 UTC · lore

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
Nguyễn Thái Ngọc Duy· Mar 8, 2012, 09:16 UTC · re: Nguyễn Thái Ngọc Duy · lore

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

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(-)
Show changes to 2 files +14 −2

gettext.c, gettext.h

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
Jonathan Nieder· Mar 8, 2012, 22:01 UTC · re: Nguyễn Thái Ngọc Duy · lore

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

Nguyễn Thái Ngọc Duy wrote:
Show 14 quoted lines
> --- 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

Nguyen Thai Ngoc Duy· Mar 9, 2012, 01:11 UTC · re: Jonathan Nieder · lore

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

2012/3/9 Jonathan Nieder <jrnieder@gmail.com>:
Show 7 quoted lines
> 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
Nguyen Thai Ngoc Duy· Mar 9, 2012, 02:33 UTC · re: Nguyen Thai Ngoc Duy · lore

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

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
Ævar Arnfjörð Bjarmason· Mar 9, 2012, 11:24 UTC · re: Jonathan Nieder · lore

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

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.

Nguyễn Thái Ngọc Duy· Mar 8, 2012, 09:16 UTC · re: Nguyễn Thái Ngọc Duy · lore

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

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(-)
Show changes to date.c +29 −17
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
Jonathan Nieder· Mar 15, 2012, 18:51 UTC · re: Nguyễn Thái Ngọc Duy · lore

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

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,
[...]
Show 5 quoted lines
>  	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
Show 7 quoted lines
> +	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(-)
Show changes to diff +43 −38
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)
Johannes Sixt· Mar 15, 2012, 19:16 UTC · re: Jonathan Nieder · lore

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

Am 15.03.2012 19:51, schrieb Jonathan Nieder:
Show 9 quoted lines
> @@ -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
Jonathan Nieder· Mar 15, 2012, 19:18 UTC · re: Johannes Sixt · lore

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

Johannes Sixt wrote:
> Am 15.03.2012 19:51, schrieb Jonathan Nieder:
Show 12 quoted lines
>> @@ -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. ;-)
Nguyen Thai Ngoc Duy· Mar 16, 2012, 11:47 UTC · re: Jonathan Nieder · lore

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

On Fri, Mar 16, 2012 at 1:51 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
Show 20 quoted lines
> 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.
Show 12 quoted lines
> 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
Nguyễn Thái Ngọc Duy· Mar 8, 2012, 09:16 UTC · re: Nguyễn Thái Ngọc Duy · lore

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

"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(-)
Show changes to parse-options.c +7 −8
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
Jonathan Nieder· Mar 8, 2012, 22:07 UTC · re: Nguyễn Thái Ngọc Duy · lore

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

Nguyễn Thái Ngọc Duy wrote:
Show 8 quoted lines
> --- 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

Nguyễn Thái Ngọc Duy· Mar 8, 2012, 09:16 UTC · re: Nguyễn Thái Ngọc Duy · lore

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

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(-)
Show changes to builtin/help.c +5 −5
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
Nguyễn Thái Ngọc Duy· Mar 8, 2012, 09:16 UTC · re: Nguyễn Thái Ngọc Duy · lore

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

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(-)
Show changes to 5 files +48 −40

Makefile, builtin/help.c, generate-cmdlist.sh, git.c, help.c

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

← back to recent threads