Re: [PATCH] document behavior of empty color name

From: Jeff King <peff@peff.net>
Date: Feb 3, 2017, 12:29 UTC
Message-ID: <20170203122859.753bexioxxkibfzb@sigill.intra.peff.net>
In-Reply-To: <20170203092430.GA10987@ash>

On Fri, Feb 03, 2017 at 04:24:30PM +0700, Duy Nguyen wrote:

Show 13 quoted lines

> > I don't think so. The default value is a git-config thing, but you would
> > want to be able to do the same thing in a config file. For example, to
> > disable coloring entirely for part of the diff, you could do:
> > 
> >   [color "diff"]
> >   meta = ""
> 
> OK but it makes log.graphColors add empty colors though. In t4202.39,
> we have " blue,invalid-color, cyan, red , ". With this patch the color
> after red would be no color. Without it, we get a complaint and the
> next color would be cycled back to blue. The test does not catch this
> because the test graph does not have enough fork points to get to red
> and back to blue.

Right, I think that's the correct behavior. The empty color name is a real color ("none"), and you can put it in your list just like any other color.

It's possible that somebody would like to use the sort of "hanging comma" behavior that people do with lists that might be added to later (e.g., for enums in C).

IMHO that would be best handled by having the list-parsing code drop trailing empty entries. But I don't think this special case is worth supporting, if only for the mental complexity it adds to the user.

-Peff