Re: [PATCHv3 4/4] am, rebase: teach quiet option
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:46:57
Stephen Boyd [off-list ref] writes:
git-rebase and git-am are talkative scripts. This option will quiet them and allow them to speak only when they fail or experience errors. Note that git-am with 3way will output errors when applying, even though the 3way will usually be successfull. We suppress these errors from git-apply because they are not "true" errors until the 3way has been attempted.
Thanks.
quoted hunk ↗ jump to hunk
@@ -498,11 +505,18 @@ do... case "$resolved" in '') - eval 'git apply '"$git_apply_opt"' --index "$dotest/patch"' + if test "$threeway" = t && test -n "$GIT_QUIET" + then + eval 'git apply '"$git_apply_opt" \ + ' --index "$dotest/patch" > /dev/null 2>&1' + else + eval 'git apply '"$git_apply_opt" \ + ' --index "$dotest/patch"' + fi apply_status=$?
Hmm, this long conditional body looks ugly, and I suspect it is harder to
maintain than necessary. Can we do something about it?
# When we are allowed to fall back to 3-way later, don't give
# false errors during the initial attempt.
squelch=
if test "$threeway" = t && test -n "$GIT_QUIET"
then
squelch='>/dev/null 2>&1 '
fi
eval "git apply $squelch$git_apply_opt"' --index "$dotest/patch"'
quoted hunk ↗ jump to hunk
@@ -72,11 +72,20 @@ continue_merge () { echo "directly, but instead do one of the following: " die "$RESOLVEMSG" fi - printf "Committed: %0${prec}d " $msgnum + if test -z "$GIT_QUIET" + then + printf "Committed: %0${prec}d " $msgnum + fi else - printf "Already applied: %0${prec}d " $msgnum + if test -z "$GIT_QUIET" + then + printf "Already applied: %0${prec}d " $msgnum + fi + fi + if test -z "$GIT_QUIET" + then + git rev-list --pretty=oneline -1 "$cmt" | sed -e 's/^[^ ]* //' fi - git rev-list --pretty=oneline -1 "$cmt" | sed -e 's/^[^ ]* //'
This is very good, because a straightforward translation is what we want in this particular patch. We however would want to update this using git show -s --oneline or something, perhaps even with a custom --format='...', in a follow-up patch. This "rev-list --pretty=oneline" piped to sed looks very much antiquated.
quoted hunk ↗ jump to hunk
@@ -445,15 +460,15 @@ then then # Lazily switch to the target branch if needed... test -z "$switch_to" || git checkout "$switch_to" - echo >&2 "Current branch $branch_name is up to date." + say >&2 "Current branch $branch_name is up to date." exit 0
Ah, I was blind. While sending non-error messages to stderr is justifiable, I do not think this one is, because all the other progress-y message in this program we reviewed so far go to stdout. I think we should drop >&2 here.
quoted hunk ↗ jump to hunk
@@ -471,7 +486,7 @@ fi # we just fast forwarded. if test "$mb" = "$branch" then - echo >&2 "Fast-forwarded $branch_name to $onto_name." + say >&2 "Fast-forwarded $branch_name to $onto_name."
Ditto.
There is one more thing to think about in git-am, which I do not think you
addressed. Consider this scenario.
(1) Tell am to run quietly, feeding a four-patch series.
$ git am -q -3 mbox
(2) The first patch applies cleanly; the second one does not apply,
even with -3, and leaves conflict (you did the right thing not to
squelch the message when this happens).
(3) You fix it up, and save the result from your editor, and tell it
to continue.
$ git am --resolved
Now, should the second invocation be also quiet, or talkative as default?
Note that the third and fourth patch are applied with -3 option in effect,
even though you did not say -3 when you restarted "am" with --resolved
(cf. ll.280-340 in git-am.sh).