Re: What's cooking in git.git (Mar 2010, #01; Wed, 03)

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 5, 2010, 03:23 UTC
Message-ID: <7v7hprh2ot.fsf@alter.siamese.dyndns.org>
In-Reply-To: <ca433831003041730w7ccbc953kad3b600e7b112e0e@mail.gmail.com>

Mark Lodato <lodatom@gmail.com> writes:

Show 10 quoted lines

> The disagreement is whether --name-only output should be colored or
> not.  In the patch, it is not, which I argue makes more sense.  When
> --name-only is given, the only thing output is filenames.  Having them
> all be the same color adds no information, and I personally find it
> annoying to see one big block of the same color. GNU grep does color
> the filenames with --name-only.  Michael Witten argues that this makes
> the output consistent: whenever it's a filename, it's colored. [1]  He
> also thinks that matching GNU grep's behavior is important.  He didn't
> convince me and I didn't convince him, so it would be nice to have
> more opinions on this.

I don't have a very strong preference, but I would say painting filenames in --name-only output the same way would make more sense than not doing so, as it is obviously consistent if we paint the name of the file exactly the same way whenever we write it at the leftmost column as the hit label, no matter what options are in effect, e.g. -c, -l, or nothing.

As to the coloring of <foo> in "Binary file <foo> matches", I don't think it matters very much which way you choose. That string is an oddball to begin with---it isn't even prefixed with the filename like normal "hit" is:

    $ git grep Q t/test4*.png t/Makefile
    Binary file t/test4012.png matches
    t/Makefile:SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
    t/Makefile:     @echo "*** $@ ***"; GIT_CONFIG=.git/config ...
    t/Makefile:     '$(SHELL_PATH_SQ)' ./aggregate-results.sh test-results/t*-*

and I think it is deliberately made an oddball, i.e. it shouldn't be like this:

    $ git grep Q t/test4*.png t/Makefile
    t/test4012.png: Binary file t/test4012.png matches
    t/Makefile:SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
    t/Makefile:     @echo "*** $@ ***"; GIT_CONFIG=.git/config ...
    t/Makefile:     '$(SHELL_PATH_SQ)' ./aggregate-results.sh test-results/t*-*

because if you did so, you cannot tell if t/test4012.png is a binary file, or it has that matched string anymore (well you can---the string doesn't have Q, but I think you know what I mean).

That makes me think that it is not even violating consistency if we treat the <foo> in "Binary file <foo> matches" differently from the usual filename label at the leftmost column. We do not have to be consistent there, as the whole point of the line being an oddball is because it fundamentally wants to be shown differently.

On the other hand, painting <foo> in the same "filename" color may make it easier to spot for color-loving people.

IOW, you can argue both ways, and both argument equally makes sense. That is why I don't think it matters very much.

And in such a case, it is typically safer to follow existing practices if there are any. If GNU paints it, we should. If GNU doesn't, we probably shouldn't.