Hi Junio,
On Mon, 1 Aug 2016, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes:
>
> >> I actually now see how this would work well for "reason 2)". If a
> >> caller wants to run the function and wants to pretend as if it did
> >> not run anything when it failed, for example, using this to spool all
> >> output and error to a strbuf and discard it when the function returns
> >> an error, and emit the spooled output to standard output and standard
> >> error in the order the lines were collected when the function returns
> >> a success, would be a good way to do so.
> >
> > That is actually the exact opposite of the intended usage: when any
> > `pick` in an interactive rebase succeeds, its output is discarded so
> > as not to bother the user. We show the complete output only when it
> > fails.
>
> Oh, it makes sense, too, to show the output only when there is an error.
Thanks ;-)
> But in that case, there would be both messages meant for the
> standard output and also meant for the standard error, and we need
> some way to make sure they go to the right channel.
Not necessarily. Let's have a look at our existing code in
git-rebase.sh:
output () {
case "$verbose" in
'')
output=$("$@" 2>&1 )
status=$?
test $status != 0 && printf "%s\n" "$output"
return $status
;;
*)
"$@"
;;
esac
}
This incredibly well-named function (</sarcasm>, my fault: dfa49f3 (Shut "git
rebase -i" up when no --verbose was given, 2007-07-23)) accumulates all
output, both stdout and stderr, and shows it only in case of an error.
Crucially, *all* output goes to stdout. No distinction is being made
between stdout and stderr.
This is the existing behavior of rebase -i.
> I however do not think an array of <bool, const char *> is the only
> way to achieve that. We can get away by a single strbuf that
> accumulates all output() that we have seen so far, i.e. "we only
> accumulate output() and ignore flush() as long as what we see are
> only from output()" mode.
>
> Then the err() routine operating under this new mode can show what
> has been accumulated to the standard output (because with this tweak
> I am outlining here, by definition, the strbuf will only keep the
> output() material and not err() things), show the err() message, and
> switch back to the normal "we accumulate output() and honor flush()"
> mode. Of course, when we are doing multiple rounds, the mode must
> be reset to "accumulate output and ignore flush" mode at the
> beginning of each rouhd.
>
> That would give us "silence if there is no error, but if we are
> showing error, show them to the standard error, while giving
> non-error message to the standard output".
It all makes sense what you say. In case you want to preserve the channel
in some future modification.
However, I am right now most concerned about keeping existing behavior as
faithfully as possible (with the exception of execution speed, which I
want to improve dramatically).
As such, it would be a serious mistake to implement that mode and use it
in the rebase--helper: it would very likely cause regressions in existing
scripts, probably even my own.
So I do understand your concern, and I agree that it would make for a fine
design, in a different context than this patch series.
Ciao,
Dscho
--
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