Re: [RFC/PATCH] i18n of multi-line messages

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 22, 2011, 18:08 UTC
Message-ID: <7v39cceay0.fsf@alter.siamese.dyndns.org>
In-Reply-To: <4EF2E824.7020509@viscovery.net>

Johannes Sixt <j.sixt@viscovery.net> writes:

Show 20 quoted lines

> Am 12/22/2011 8:38, schrieb Junio C Hamano:
>> +static void v_format(const char *prefix, const char *fmt, va_list params,
>> +		     emit_fn emit, void *cb_data)
>> +{
>> +	struct strbuf buf = STRBUF_INIT;
>> +	struct strbuf line = STRBUF_INIT;
>> +	const char *cp, *np;
>> +
>> +	strbuf_vaddf(&buf, fmt, params);
> ...
>>  void vreportf(const char *prefix, const char *err, va_list params)
>>  {
>> -	char msg[4096];
>> -	vsnprintf(msg, sizeof(msg), err, params);
>> -	fprintf(stderr, "%s%s\n", prefix, msg);
>> +	v_format(prefix, err, params, emit_report, NULL);
>>  }
>
> Using strbuf (or xmalloc for that matter) from a function that can be
> called from die() is a big no-no. You should keep the fixed-sized buffer.

I _think_ I liked the simplicity of having the logic to

 - format localized message to a multi-line buffer; and
 - split the contents of that buffer into lines and add prefix in an
   i18n friendly way

in vreportf(), but there are many problems in this approach, it seems. We may need to limit the message length for error()/die() codepath, but we do not want to be limited in others, definitely not from advise().

Also some calls to error() are meant to produce plumbing error message and should never be localized. The approach to localize the prefix in vreportf() will not work for this reason.

I think we should start from the original "advise-only" way. In the longer term (if somebody cares about it deeply), things can be fixed up and the mechanism can then be unified in the following order:

 (1) figure out a way to allow error() and die() tell if they are called
     to produce a plumbing message that should not be translated (multiple
     approaches are possible, ranging from adding error_plumb() function
     to marking the message format string specially);

 (2) update the existing error()/die() calls that are used to produce
     plumbing message and mark them as such, using the mechanism decided
     in (1);

 (3) Take the v_format/vreport code from my patch I am discarding with
     this message, enhance them to turn the "prefix" i18n part
     consitional, and use that to reimplement the mechanism (1).

But that is not for this year.