Re: [PATCH] PRItime: wrap PRItime for better l10n compatibility

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jul 19, 2017, 13:25 UTC
Message-ID: <alpine.DEB.2.21.1.1707191456010.4193@virtualbox>
In-Reply-To: <CANYiYbFROuyXso2ZKuJWDp4cSwpBu=bNAbC-yZtEyDwkbUcAhQ@mail.gmail.com>

Hi Jian (or is it Xin?),

On Wed, 19 Jul 2017, Jiang Xin wrote:

Show 37 quoted lines

> 2017-07-19 1:35 GMT+08:00 Junio C Hamano <gitster@pobox.com>:
> > Jiang Xin <worldhello.net@gmail.com> writes:
> >
> >>> 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.
> >
> > I think the question can better be answered if we know how gettext
> > tools special case PRIuMAX.  One thing that may be problematic is
> > that timestamp can later become a signed type and use of one level
> > of redirection in the current code via PRItime and via timestamp_t
> > is a good way to keep such a transition much easier.  Reverting it
> > to use PRIuMAX would make such a transition much harder.
> 
> Gettext handles macros such as PRIuMAX in commit 8b45c5df1 ("Add
> support for ISO C 99 <inttypes.h> format string directive macros.",
> 2002-07-23 12:33:13 +0000).

Wow. This is ugly.

If I understand correctly, then this will not even work correctly for PRIuMAX on Windows: I highly doubt that the gettext library will interpret %I64u in the format string correctly to be what %<PRIuMAX> in the po file refers to.

But there may be hope. Since the character sequence "PRItime" is highly unlikely to occur in Git's source code in any context other than the format to print/parse timestamp_t, it should be possible to automate a the string replacement

	git ls-files -z \*.[ch] |
	xargs -0r sed -i 's/PRItime/PRIuMAX/g'

(assuming, of course, that you use GNU sed, not BSD sed, for which the `-i` needs to read `-i ''` instead) as part of the update?

For all the reasons Junio mentioned, I, too, would be reluctant to change the source code to cull all of the PRItime mentions, as it is pleasing from a semantic point of view that we know what the heck we are talking about here.

BTW *thank you so much* for your Herculean effort to keep going with the translation.

Ciao, Dscho