Re: [PATCH] document behavior of empty color name

On Thu, Feb 02, 2017 at 01:42:44PM +0100, Jeff King wrote:

On Thu, Feb 02, 2017 at 04:16:15PM +0700, Duy Nguyen wrote:

quoted

quoted

I hadn't heard anything back,

Sorry I was accidentally busy during Luna new year holiday.

No problem. That sounds much more fun than working on Git. :)

quoted

quoted

-	if (!len)
-		return -1;
+	if (!len) {
+		dst[0] = '\0';
+		return 0;
+	}
 
 	if (!strncasecmp(ptr, "reset", len)) {
 		xsnprintf(dst, end - dst, GIT_COLOR_RESET);

I wonder if it makes more sense to do this in builtin/config.c. The
"default value" business is strictly git-config command's. The parsing
function does not need to know. Maybe something like this?

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.

I suppose this is ok after your documentation patch ("that's by
design!"). Just wanted to point it out in case I miss something.
--
Duy