Johannes Schindelin [off-list ref] writes:
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.
+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.
On Wed, Jul 27, 2016 at 2:37 PM, Junio C Hamano [off-list ref] wrote:
quoted
+ strbuf_vaddf(&o->obuf, err, params);
+ error("%s", o->obuf.buf);
+ strbuf_reset(&o->obuf);
Sneaky ;-)
Just to avoid confusion, I am _fine_ with this "we happen to have
a strbuf that we know to be empty at this point, so let's reuse it
and clean after ourselves before returning".
I just found it somewhere between clever and ugly, and "sneaky"
was the first word that came to my mind.
Hi Junio,
On Wed, 27 Jul 2016, Junio C Hamano wrote:
Johannes Schindelin [off-list ref] writes:
quoted
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.
I rephrased it, using your explanation.
quoted
+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.
For some reason, I thought that `va_start()` must be the first statement
of the function. Fixed.
quoted
+ strbuf_vaddf(&o->obuf, err, params);
+ error("%s", o->obuf.buf);
+ strbuf_reset(&o->obuf);
Sneaky ;-)
Thanks ;-)
Dscho