2017-07-18 1:10 GMT+08:00 Junio C Hamano <gitster@xxxxxxxxx>: > Jiang Xin <worldhello.net@xxxxxxxxx> writes: > >> Commit cb71f8bdb5 ("PRItime: introduce a new "printf format" for >> timestamps", 2017-04-21) does not play well with i18n framework. The >> static string concatenation cannot be correctly interpreted by >> gettext utilities, such as xgettext. >> >> Wrap PRItime in format_raw_time() function, so that we can extract >> correct l10n messages into "po/git.pot". >> >> Reported-by: Jean-Noël Avila <jn.avila@xxxxxxx> >> Signed-off-by: Jiang Xin <worldhello.net@xxxxxxxxx> >> ... >> @@ -191,6 +200,15 @@ struct date_mode *date_mode_from_type(enum date_mode_type type) >> return &mode; >> } >> >> +const char *format_raw_time(timestamp_t time) >> +{ >> + static struct strbuf time_buf = STRBUF_INIT; >> + >> + strbuf_reset(&time_buf); >> + strbuf_addf(&time_buf, "%"PRItime, time); >> + return time_buf.buf; >> +} > > Hmm. > > Two potential issues are: > > - After this patch, there still are quite a many > > printf("time is %"PRItime" ...\n", timestamp) > > so the burden on the programmers having to remember when it is > required to use format_raw_time() becomes unclear, and makes the > change/churn larger when an existing message needs to be marked > for translation. > > - The static struct strbuf here is a cheap way to avoid leaks, but > at the same time it is unfriendly to threaded code. We could > instead do: > > void append_PRItime(struct strbuf *buf, timestamp_t time); > > to fix that trivially, but the damage to the caller obviously is > much larger going this way. > I wonder if we can replace the original %lu for timestamp with PRIuMAX instead. PRIuMAX works fine with gettext utils. -- Jiang Xin