Re: [PATCH/RFC] checkout: print something when checking out paths
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Nov 20, 2018, 02:53 UTC
- Message-ID
- <xmqqpnv0bgsd.fsf@gitster-ct.c.googlers.com>
- In-Reply-To
- <CACsJy8B6wKGg2Jsopct-0dYNhKJGf9RdnrnTqBOt4kxy6LzxMw@mail.gmail.com>
Duy Nguyen <pclouds@gmail.com> writes:
Show 8 quoted lines
>> > I see this at the same level as "Switched to branch 'foo'" which is >> > also protected by opts->quiet. If we start hiding messages based on >> > tty it should be done for other messages in update_refs_for_switch() >> > too, I guess? > > No let's drop this for now. I promise to come back with something > better (at least it still sounds better in my mind). If that idea does > not work out, we can come back and see if we can improve this.
Let's leave it in 'pu'.
I do agree that this is similar to existing messages that talk about checkout out a branch to work on etc., and I think giving feedback when checkout paths out _is_ a good thing to do for interactive users.
If we were to squelch such "notice" output for non-interactive use, we should do so to the "notice" messages for checking out a branch, as well, and also to other subcommands that report what they did with these "notice" output. And that is a separate topic.
The primary reason why I was annoyed was because "make test" (I think I have DEFAULT_TEST_TARGET=prove, if it matters) output was littered with these "checked out N paths", even though I am not asking for verbose output.
It could be that the real cause of that is perhaps because we have too many "git checkout" that is outside test_expect_* block, in which case we should fix these tests to perform the checkout inside a test_expect_success block for test set-up, and my annoyance was only shooting at the messenger.
For example, the attached patch illustrates the right problem but addresses it in a wrong way. This checkout_files() helper does too many things outside (and before) the test_expect_success block it has (other helpers like commit_chk_wrnNNO share the same problem), and making "git checkout" noisy will reveal that as a problem by leaking the noisy output directly to the tester. But the real fix is to enclose the set-up step inside a test_expect_success block, which is not done by the following illustration patch, which instead just squelches the message.
t/t0027-auto-crlf.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh index beb5927f77..3587e454f1 100755 --- a/t/t0027-auto-crlf.sh +++ b/t/t0027-auto-crlf.sh @@ -293,9 +293,9 @@ checkout_files () { do rm crlf_false_attr__$f.txt && if test -z "$ceol"; then - git checkout crlf_false_attr__$f.txt + git checkout -- crlf_false_attr__$f.txt else - git -c core.eol=$ceol checkout crlf_false_attr__$f.txt + git -c core.eol=$ceol checkout -- crlf_false_attr__$f.txt fi done