Re: [PATCH] log: support 256 colors with --graph=256colors

From: Jeff King <peff@peff.net>
Date: Dec 20, 2016, 16:57 UTC
Message-ID: <20161220165754.hkmnsxiwbcgn6uin@sigill.intra.peff.net>
In-Reply-To: <20161220123929.15329-1-pclouds@gmail.com>

On Tue, Dec 20, 2016 at 07:39:29PM +0700, Nguyễn Thái Ngọc Duy wrote:

Show 9 quoted lines

> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  I got mad after tracing two consecutive red history lines in `git log
>  --graph --oneline` back to their merge points, far far away. Yeah
>  probably should fire up tig, or gitk or something.
> 
>  This may sound like a good thing to add, but I don't know how good it
>  is compared to the good old 16 color palette, yet as I haven't tried it
>  for long since it's just written.

Hmm. At some point the colors become too close together to be easily distinguishable. In your code you have:

Show 14 quoted lines

> +	if (arg && !strcmp(arg, "256colors")) {
> +		int i, start = 17, stop = 232;
> +		column_colors_max = stop - start;
> +		column_colors =
> +			xmalloc((column_colors_max + 1) * sizeof(*column_colors));
> +		for (i = start; i < stop; i++) {
> +			struct strbuf sb = STRBUF_INIT;
> +			strbuf_addf(&sb, "\033[38;5;%dm", i);
> +			column_colors[i - start] = strbuf_detach(&sb, NULL);
> +		}
> +		column_colors[column_colors_max] = xstrdup(GIT_COLOR_RESET);
> +		/* ignore the closet 16 colors on either side for the next line */
> +		column_colors_step = 16;
> +	}

So you step by 16, over a set of 215 colors. That seems to give only 13 colors, versus the original 16. :)

I know that is a simplification. If you wrap around, then you get your 13 colors, and then another 13 colors that aren't _quite_ the same, and so on, until you've used all 256. I'm just not sure if the 1st and 14th color would be visually different enough for it to matter (I admit I didn't do any experiments, though).

Show 5 quoted lines

> ---graph::
> +--graph[=<options>]::
>  	Draw a text-based graphical representation of the commit history
>  	on the left hand side of the output.  This may cause extra lines
>  	to be printed in between commits, in order for the graph history

I wonder if we would ever want another use for "--graph=foo". I guess any such thing could fall under the name of "graph options", and we'd end up with "--graph=256colors,unicode" or something like that.

I do suspect people would want a config option for this, though. I.e., you'd want to enable it all the time if you have a terminal which can handle 256 colors, not just for a particular invocation.

-Peff