Johannes Schindelin <johannes.schindelin@xxxxxx> writes:
> Since 66a155b (Enable output buffering in merge-recursive., 2007-01-14),
> we already accumulate the output in a buffer. The idea was to avoid
> interfering with the progress output that goes to stderr, which is
> unbuffered, when we write to stdout, which is buffered.
>
> We extend that buffering to allow the caller to handle the output
> (possibly suppressing it). This will help us when extending the
> sequencer to do rebase -i's brunt work: it does not want the picks to
> print anything by default but instead determine itself whether to print
> the output or not.
>
> Note that we also redirect the error messages into the output buffer
> when the caller asked not to flush the output buffer, for two reasons:
> 1) to retain the correct output order, and 2) to allow the caller to
> suppress *all* output.
I am not yet sure if it makes sense to mix both the regular output
and an error into the same buffer for the callers to process (your
"reason 1)" above), and this looks like a wrong way to allow a
caller that wants no output (your "reason 2)" above). A caller that
wants to massage the output would want to know which ones are errors
and which ones are not, I would imagine, and setting a knob to make
both output() and err() a no-op would be a more suitable way to give
a caller a total silence.
At least I cannot yet judge if this is a good change, only from the
material presented here. That does not mean I have a concrete
reason to say this is a bad change, either. Only from the material
presented here, I cannot judge that, either.
> Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
> ---
> merge-recursive.c | 17 +++++++++++++----
> merge-recursive.h | 2 +-
> 2 files changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/merge-recursive.c b/merge-recursive.c
> index 723b8d0..311cfa4 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -25,7 +25,7 @@
>
> static void flush_output(struct merge_options *o)
> {
> - if (o->obuf.len) {
> + if (o->buffer_output < 2 && o->obuf.len) {
> fputs(o->obuf.buf, stdout);
> strbuf_reset(&o->obuf);
> }
> @@ -36,10 +36,19 @@ static int err(struct merge_options *o, const char *err, ...)
> va_list params;
>
> va_start(params, err);
> - flush_output(o);
> + if (o->buffer_output < 2)
> + flush_output(o);
> + else {
> + strbuf_complete(&o->obuf, '\n');
> + strbuf_addstr(&o->obuf, "error: ");
> + }
> strbuf_vaddf(&o->obuf, err, params);
> - error("%s", o->obuf.buf);
> - strbuf_reset(&o->obuf);
> + if (o->buffer_output > 1)
> + strbuf_addch(&o->obuf, '\n');
> + else {
> + error("%s", o->obuf.buf);
> + strbuf_reset(&o->obuf);
> + }
> va_end(params);
>
> return -1;
> diff --git a/merge-recursive.h b/merge-recursive.h
> index d415724..340704c 100644
> --- a/merge-recursive.h
> +++ b/merge-recursive.h
> @@ -13,7 +13,7 @@ struct merge_options {
> MERGE_RECURSIVE_THEIRS
> } recursive_variant;
> const char *subtree_shift;
> - unsigned buffer_output : 1;
> + unsigned buffer_output : 2; /* 1: output at end, 2: keep buffered */
> unsigned renormalize : 1;
Once a field ceases to be a boolean, it is OK not to squish it into
a bitfield like this for a struct that we will have only a very
small number of instances of. Treating it just like "verbosity",
which occupies a whole int even though it can only get up to 5 or
so, would be more appropriate.
> long xdl_opts;
> int verbosity;
--
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