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

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

 



Mark Lodato <lodatom@xxxxxxxxx> 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.


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

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