Re: [PATCH v2 04/23] rebase -i: hide interactive command messages in verbose mode
Hi Thomas, Thomas Rast writes: Fabian Ruch baf...@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): 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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 04/23] rebase -i: hide interactive command messages in verbose mode
Fabian Ruch baf...@gmail.com writes: Hi Thomas, Thomas Rast writes: Fabian Ruch baf...@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'. [...] 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. Ah, makes sense. Thanks for the explanations! -- Thomas Rast t...@thomasrast.ch -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 04/23] rebase -i: hide interactive command messages in verbose mode
Fabian Ruch baf...@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. @@ -873,9 +873,8 @@ test_expect_success 'running git rebase -i --exec git show HEAD' ' ( FAKE_LINES=1 exec_git_show_HEAD 2 exec_git_show_HEAD export FAKE_LINES - git rebase -i HEAD~2 expect + git rebase -i HEAD~2 expected ) - sed -e 1,9d expect expected test_cmp expected actual ' Getting rid of these magic removals is a very nice change, thank you. -- Thomas Rast t...@thomasrast.ch -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 04/23] 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. Temporarily redirect the editor output to a third file
descriptor in order to ship it around the capture stream. Wrap the
respective git-commit command lines in `output`.
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.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh| 9 +
git-rebase.sh | 12 ++--
t/lib-rebase.sh | 8
t/t3404-rebase-interactive.sh | 18 ++
t/t3406-rebase-message.sh | 18 ++
5 files changed, 43 insertions(+), 22 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 89ef5e2..5dfdf13 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -504,7 +504,7 @@ do_next () {
mark_action_done
do_pick $sha1 $rest
- git commit --allow-empty --amend --no-post-rewrite
${gpg_sign_opt:+$gpg_sign_opt} || {
+ output git commit --allow-empty --amend --no-post-rewrite
${gpg_sign_opt:+$gpg_sign_opt} || {
warn Could not amend commit after successfully picking
$sha1... $rest
warn This is most likely due to an empty commit
message, or the pre-commit hook
warn failed. If the pre-commit hook failed, you may
need to resolve the issue before
@@ -558,14 +558,14 @@ do_next () {
# This is the final command of this squash/fixup group
if test -f $fixup_msg
then
- do_with_author git commit --allow-empty-message
--allow-empty \
+ do_with_author output git commit
--allow-empty-message --allow-empty \
--amend --no-verify -F $fixup_msg \
${gpg_sign_opt:+$gpg_sign_opt} ||
die_failed_squash $sha1 $rest
else
cp $squash_msg $GIT_DIR/SQUASH_MSG || exit
rm -f $GIT_DIR/MERGE_MSG
- do_with_author git commit --allow-empty --amend
--no-verify -F $GIT_DIR/SQUASH_MSG -e \
+ do_with_author output git commit --allow-empty
--amend --no-verify -F $GIT_DIR/SQUASH_MSG -e \
${gpg_sign_opt:+$gpg_sign_opt} ||
die_failed_squash $sha1 $rest
fi
@@ -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
diff --git a/git-rebase.sh b/git-rebase.sh
index 55da9db..f90541e 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -131,9 +131,17 @@ write_basic_state () {
output () {
case $verbose in
'')
- output=$($@ 21 )
+ 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\
+ $@ 31 $state_dir/output 21
+ )
status=$?
- test $status != 0 printf %s\n $output
+ test $status != 0 cat $state_dir/output
return $status
;;
*)
diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
index 6bd2522..0cd1193 100644
--- a/t/lib-rebase.sh
+++ b/t/lib-rebase.sh
@@ -41,8 +41,8 @@ set_fake_editor () {
test -z $FAKE_LINES exit
grep -v '^#' $1 $1.tmp
rm -f $1
- echo 'rebase -i script before editing:'
- cat $1.tmp
+ echo 'rebase -i script before editing:' 2
+ cat $1.tmp 2
action=pick
