From: Junio C Hamano <gitster@pobox.com>
Date: Mon, 17 Jul 2017 17:10:24 GMT
Subject: Re: [PATCH] PRItime: wrap PRItime for better l10n compatibility
Message-ID: <xmqq7ez7htvj.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <3ccfa2fb49d471f807d77d9a280e4b7cfe56faea.1500304209.git.worldhello.net@gmail.com>

Jiang Xin <worldhello.net@gmail.com> 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@free.fr>
> Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
> ...
> @@ -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.