Re: [PATCH/RFC] checkout: print something when checking out paths

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Duy Nguyen <pclouds@xxxxxxxxx> writes:

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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux