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

[Jonathan Nieder <jrnieder@gmail.com>]

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)