Re: [PATCH v5 12/16] merge-recursive: flush output buffer before printing error messages

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 27, 2016, 21:37 UTC
Message-ID: <xmqqh9bavk2x.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <349bdfacfffa11d06241655c9e0b62506e58758b.1469547160.git.johannes.schindelin@gmx.de>

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

Show 7 quoted lines

> The data structure passed to the recursive merge machinery has a feature
> where the caller can ask for the output to be buffered into a strbuf, by
> setting the field 'buffer_output'.
>
> Previously, we simply swallowed the buffered output when showing error
> messages. With this patch, we show the output first, and only then print
> the error message.

I didn't quite understand this paragraph until I realized that you meant "when showing die message". We died without flushing, losing accumulated output.

Show 6 quoted lines

> +static int err(struct merge_options *o, const char *err, ...)
> +{
> +	va_list params;
> +
> +	va_start(params, err);
> +	flush_output(o);

I would have written the above two swapped; va_start() logically is about what happens in the next four lines.

> +	strbuf_vaddf(&o->obuf, err, params);
> +	error("%s", o->obuf.buf);
> +	strbuf_reset(&o->obuf);

Sneaky ;-)

The remainder replaces error(...) with err(o, ...) and updates the callchain to pass the merge_options around, which looked good.

Thanks.