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

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 18, 2017, 17:35 UTC
Message-ID: <xmqq60epfy27.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CANYiYbEcMrriaor9OT4c2qtfh9Ja5NJ9KBSxa3XhPAuoN0t42A@mail.gmail.com>

Jiang Xin <worldhello.net@gmail.com> writes:

Show 23 quoted lines

>> 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.