Re: [PATCH v2 04/23] rebase -i: hide interactive command messages in verbose mode

From: Fabian Ruch <bafain@gmail.com>
Date: Aug 11, 2014, 08:26 UTC
Message-ID: <53E87E47.9060007@gmail.com>
In-Reply-To: <87y4uyajd3.fsf@thomasrast.ch>

Hi Thomas,

Thomas Rast writes:

Show 21 quoted lines

> Fabian Ruch <bafain@gmail.com> writes:
>> @@ -923,6 +923,8 @@ EOF
>>  	;;
>>  esac
>>  
>> +mkdir -p "$state_dir" || die "Could not create temporary $state_dir"
>> +
>>  git var GIT_COMMITTER_IDENT >/dev/null ||
>>  	die "You need to set your committer info first"
>>  
>> @@ -938,7 +940,6 @@ then
>>  fi
>>  
>>  orig_head=$(git rev-parse --verify HEAD) || die "No HEAD?"
>> -mkdir -p "$state_dir" || die "Could not create temporary $state_dir"
>>  
>>  : > "$state_dir"/interactive || die "Could not mark as interactive"
>>  write_basic_state
> 
> Why this change?  I can't figure out how it relates to the output
> change.

Creating the state directory a few steps earlier into 'git_rebase__interactive' is necessary because the changed definition of 'output' needs it for 'editor.sh'. This change was triggered by a failing test case that used the <branch> argument with git-rebase. The 'git checkout <branch>', which is executed if 'switch_to' is set to <branch>, is wrapped into an 'output' line and 'output' failed because it wasn't able to create 'editor.sh'.

The state directory (of git-rebase--interactive!) is now created directly after the case expression that handles --continue, --skip and --edit-todo. They all assume the existence of the state directory and either jump into 'do_rest' or 'exit' immediately, that is creating the directory earlier would make the options handling code somewhat incorrect and would not change anything for the start sequence of git-rebase--interactive.

The patch message now reads as follows (with the reference to 7725cb5 in the second paragraph and the complete third paragraph added):

Show 30 quoted lines

> rebase -i: hide interactive command messages in verbose mode
> 
> git-rebase--interactive prints summary messages of the commits it
> creates in the final history only if the `--verbose` option is
> specified by the user and suppresses them otherwise. This behaviour
> is implemented by wrapping git-commit calls in a shell function named
> `output` which redirects stderr to stdout, captures stdout in a shell
> variable and ignores its contents unless the command exits with an
> error status.
> 
> The command lines used to implement the to-do list commands `reword`
> and `squash` print diagnostic messages even in non-verbose mode. The
> reason for this inconsistency is that both commands launch the log
> message editor which usually requires a working terminal attached to
> stdin. Wrapping the `reword` and `squash` command lines in `output`
> would seemingly freeze the terminal (see commit 7725cb5, "rebase -i:
> fix reword when using a terminal editor"). Temporarily redirect the
> editor output to a third file descriptor in order to ship it around
> the capture stream. Wrap the remaining git-commit command lines in
> the new `output`.
> 
> In order to temporarily redirect the editor output, the new
> definition of `output` creates a script in the state directory to be
> used as `GIT_EDITOR`. Make sure the state directory exists before
> `output` is called for the first time.
> 
> fake_editor prints the to-do list before and after applying the
> `FAKE_LINES` rewrite rules to it. Redirect this debug output to
> stderr so that it does not interfere with the git-rebase status
> output. Add test.

   Fabian