Re: [PATCHv3 4/4] am, rebase: teach quiet option

[Stephen Boyd <bebarino@gmail.com>]

Junio C Hamano wrote:
> 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"'
>

Thanks. I thought there would be a nicer way but I didn't know it.

> 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.

Will do, gotta fixup some tests for that though...

> 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).

Thanks for bringing this up. I gave this some thought before I sent the
series out but I was secretly hoping nobody would care :-) I think it's
more correct and maybe even more consistent to keep the quiet option
enabled.

Seeing as this series is going for round 4 I'll make sure to include
this as well.