Re: [RFC PATCH 1/3] git-rebase, sequencer: add a --quiet option for the interactive machinery
From: Johannes Schindelin <hidden>
Date: 2018-06-09 21:46:10
Hi Elijah, On Thu, 7 Jun 2018, Elijah Newren wrote:
While 'quiet' and 'interactive' may sound like antonyms, the interactive machinery actually has logic that implements several interactive_rebase=implied cases (--exec, --keep-empty, --rebase-merges) which won't pop up an editor. Further, we want to make the interactive machinery also take over for git-rebase--merge and become the default merge strategy, so it makes sense for these other cases to have a quiet option. git-rebase--interactive was already somewhat quieter than git-rebase--merge and git-rebase--am, possibly because cherry-pick has just traditionally been quieter. As such, we only drop a few informational messages -- "Rebasing (n/m)" and "Succesfully rebased..."
Makes sense. As long as it is coordinated with Alban and Pratik, as both of their GSoC projects are affected by this. In particular Pratik's project, I think, would actually *benefit* from your work, as it might even make it possible to turn all modes but --preserve-merges into pure builtin code, which would be awesome.
quoted hunk ↗ jump to hunk
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 06a7b79307..1f2401f702 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh@@ -139,8 +139,12 @@ mark_action_done () { if test "$last_count" != "$new_count" then last_count=$new_count - eval_gettext "Rebasing (\$new_count/\$total)"; printf "\r" - test -z "$verbose" || echo + if test -z "$GIT_QUIET" + then + eval_gettext "Rebasing (\$new_count/\$total)"; + printf "\r" + test -z "$verbose" || echo + fi fi }@@ -713,6 +717,7 @@ Commit or stash your changes, and then run "$hook" rebase < "$rewritten_list" true # we don't care if this hook failed fi && + test -z "$GIT_QUIET" && warn "$(eval_gettext "Successfully rebased and updated \$head_name.")"
In general, I tried the statements to return success at all times. That means that test -n "$GIT_QUIET" || would be better in this case.
quoted hunk ↗ jump to hunk
diff --git a/git-rebase.sh b/git-rebase.sh index 7d1612b31b..b639c0d4fe 100755 --- a/git-rebase.sh +++ b/git-rebase.sh@@ -136,7 +136,7 @@ write_basic_state () { echo "$head_name" > "$state_dir"/head-name && echo "$onto" > "$state_dir"/onto && echo "$orig_head" > "$state_dir"/orig-head && - echo "$GIT_QUIET" > "$state_dir"/quiet && + test t = "$GIT_QUIET" && : > "$state_dir"/quiet
Maybe it would be better to `echo t` into that file? That way, scripts that used the value in that file would continue to work. (But maybe there were no scripts that could use it, as only the interactive rebase allows scripting, and it did not handle that flag before?) The rest looks obviously good. Thank you! Dscho