Johannes Sixt <j.sixt@xxxxxxxxxxxxx> writes:
> 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html