Hi,
Jeff King writes:
> On Tue, Jul 29, 2014 at 01:18:07AM +0200, Fabian Ruch wrote:
>> The command line used to recreate root commits specifies the option
>> `-q` which suppresses the commit summary message. However,
>> git-rebase--interactive tends to tell the user about the commits it
>> creates in the final history, if she wishes (cf. command line option
>> `--verbose`). The code parts handling non-root commits and squash
>> commits all output commit summary messages. Do not make the replay of
>> root commits an exception. Remove the option to make the report of
>> the rebased history complete.
>>
>> It is OK that the commit summary is still suppressed when git-commit
>> is used to initialize the authorship of the sentinel commit because
>> this additional commit is an implementation detail hidden from the
>> final history. The removed `-q` option was probably introduced as a
>> copy-and-paste error stemming from that part of the root commit
>> handling code.
>
> I'm confused. This implies that we should be seeing summaries for other
> commits, but not root commits, and this patch is bring them into
> harmony. But if I have a repo like this:
>
> git init -q repo &&
> cd repo &&
> for i in one two; do
> echo $i >file &&
> git add file &&
> git commit -q -m $i
> done
>
> then using stock git gives me this:
>
> $ GIT_EDITOR=true git rebase -i --root 2>&1 | perl -pe 's/\r/\\r\n/g'
> Rebasing (1/2)\r
> Rebasing (2/2)\r
> Successfully rebased and updated refs/heads/master.
>
> but with your patch, I get:
>
> $ GIT_EDITOR=true git.compile rebase -i --root 2>&1 | perl -pe 's/\r/\\r\n/g'
> Rebasing (1/2)\r
> [detached HEAD 60834b3] one
> Date: Fri Aug 1 20:00:05 2014 -0400
> 1 file changed, 1 insertion(+)
> create mode 100644 file
> Rebasing (2/2)\r
> Successfully rebased and updated refs/heads/master.
>
> Am I misunderstanding the purpose of the patch?
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:
> 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
> cat >"$state_dir"/editor.sh <<EOF
> #!/bin/sh
> $(git var GIT_EDITOR) \$* >&3
> EOF
> chmod +x "$state_dir"/editor.sh
> (
> export GIT_EDITOR="$state_dir"/editor.sh
> "$@" 3>&1 >"$state_dir"/output 2>&1
> )
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.
Options 2) and 3) seem attainable within this patch series and 3) sounds
like the cleanest option but I'm uncertain if I'm missing something
here. The only command line that is wrapped in 'output' and that doesn't
support a --quiet option seems to be a 'warn' line which could simply be
skipped in non-verbose mode. (Johannes Schindelin is cc'd as the
original author of git-rebase--interactive.sh and 'output' in particular).
Fabian
--
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