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. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html