Johannes Schindelin [off-list ref] writes:
quoted
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.
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.
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".
Hi Junio,
On Mon, 1 Aug 2016, Junio C Hamano wrote:
Johannes Schindelin [off-list ref] writes:
quoted
quoted
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