Re: [PATCH] Share color list between graph and show-branch

From: Johan Herland <johan@herland.net>
Date: Apr 5, 2011, 07:29 UTC
Message-ID: <201104050929.17135.johan@herland.net>
In-Reply-To: <BANLkTint1+c0h9DExydWeeafdgawEJPuMw@mail.gmail.com>

On Tuesday 05 April 2011, Dan McGee wrote:

> On another note, we also have this whole crazy "- 1" bit and the RESET
> element at the end, and yet I see nowhere that slot is actually used.
> It looks like this was introduced by commit 1e3d4119d21df28.

Read that commit again. You'll see that in graph.c:strbuf_write_column() it replaces

  strbuf_addstr(sb, GIT_COLOR_RESET);

with

  strbuf_addstr(sb, column_get_color_code(column_colors_max));

which resolves to the same thing. The reason for that extra indirection is to enable replacing the column_colors_ansi array with a different color array, to do graph coloring in non-ANSI contexts. Specifically, it was done to enable HTML/CSS coloring of graphs in CGit: http://hjemli.net/git/cgit/commit/?id=268b34af23cdcac87aed3300bfe6154cbc65753e

It should be obvious that if we replace the ANSI coloring scheme with some other coloring scheme, we also need to change the RESET entry (resetting a HTML "color" with the ANSI reset code is nonsense). Therefore I opted to move the RESET code into the column_colors array, and make column_colors_max indicate both (a) the length of the column_colors array, and (b) the index of the RESET code in that same array. That's why we need the crazy "- 1" bit when defining COLUMN_COLORS_ANSI_MAX.

BTW, this is documented graph.h:graph_set_column_colors() from the same 1e3d4119d21df28 commit.

...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net