Re: [PATCH v1 07/19] rebase -i: log the replay of root commits

On Mon, Aug 04, 2014 at 11:21:41PM +0200, Fabian Ruch wrote:

Thanks for laying out the differences in the user visible output. With
stock git we are seeing summaries for other commits, but not root
commits, _with the --verbose option_. It's the fault of my patch to show
the summary even in non-verbose mode. This is now fixed by wrapping the
relevant command in 'output', a shell function defined in git-rebase.sh
as follows:

Ah, OK. That makes a lot more sense, then.

quoted

output=$("$@" 2>&1 )
status=$?
test $status != 0 && printf "%s\n" "$output"
return $status

The problem that git-rebase--interactive has is that the redirection of
stdin to a variable (or a file) does not work reliably with commands
that invoke the log message editor, that is 'reword' and 'squash'
produce their output both in verbose and non-verbose mode. I wouldn't
know how to fix this but

1) invoking $GIT_EDITOR from git-rebase--interactive.sh, but to make
this right there should be a built-in command for shell scripts and an
interface for git-commit that prepare the editor contents like
git-commit does now, or

2) forcing $GIT_EDITOR and git-commit to print on distinct file
descriptors, which would involve temporarily wrapping the $GIT_EDITOR
call in a shell script that redirects stdin to some other file
descriptor similar to what t/test-lib.sh does, or

Hmm. In the test scripts, we send stdout and stderr for sub-commands to
fds 3 and 4 respectively. And then we either point those at /dev/null or
to >&1 and >&2, depending on whether $verbose mode was specified.

I don't think that will work here, though. You literally have one "git
commit" command to run, and you want its stderr/stdout to go somewhere
different than the $GIT_EDITOR it will invoke. Your (2) makes some sense
to me. Something like:

  GIT_EDITOR="$(shellquote "$(git var GIT_EDITOR)") >&3 2>&4" \
    git commit ... 3>&1 4>&2 >wherever 2>&1

Or we could just point it at /dev/tty, though I guess that may open
another can of worms (systems without /dev/tty, what happens when you do
not have a terminal, etc).

3) passing the --quiet option in non-verbose mode and omitting it in
verbose mode, which would cover the '$status != 0' above for if a
command fails, it should indicate its error status despite being asked
to be silent.

Yeah, that is probably the cleanest option if it works. I would just
worry that it is not as complete. It works for "git commit", but are
there are other commands wrapped in the verbose output that would want
the same treatment (that might not know about --quiet)? Your paragraph
below says it would not be that big a deal, so as long as we don't plan
to add anything in the future that could not handle the requirement,
that may be enough.

-Peff