Thread (1 message) 1 message, 1 author, 2016-06-15

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:48:23

Mark Lodato [off-list ref] writes:
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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help