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

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 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.

 BTW anyone interested in bringing this type [1] of --graph to git? I
 tried the unicode box characters, but the vertical lines do not join
 with diagonal ones, making the graph a bit ugly (even though it's
 still better than ascii version)

 [1] https://github.com/magit/magit/issues/495#issuecomment-17480757

 Documentation/rev-list-options.txt |  5 ++++-
 graph.c                            | 31 ++++++++++++++++++++++++++++++-
 graph.h                            |  8 +++++++-
 revision.c                         |  4 ++++
 4 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index 5da7cf5..0a0c2f3 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt

@@ -825,7 +825,7 @@ you would get an output like this:
 	-xxxxxxx... 1st on a
 -----------------------------------------------------------------------
 
---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

@@ -836,6 +836,9 @@ This enables parent rewriting, see 'History Simplification' below.
 +
 This implies the `--topo-order` option by default, but the
 `--date-order` option may also be specified.
++
+The only supported option is `256colors` which allows more than 16
+colors for drawing the commit history.
 
 --show-linear-break[=<barrier>]::
 	When --graph is not used, all history branches are flattened

diff --git a/graph.c b/graph.c
index d4e8519..75375a1 100644
--- a/graph.c
+++ b/graph.c

@@ -78,6 +78,7 @@ static void graph_show_line_prefix(const struct diff_options *diffopt)
 
 static const char **column_colors;
 static unsigned short column_colors_max;
+static int column_colors_step;
 
 void graph_set_column_colors(const char **colors, unsigned short colors_max)
 {

@@ -234,10 +235,24 @@ void graph_setup_line_prefix(struct diff_options *diffopt)
 }
 
 
-struct git_graph *graph_init(struct rev_info *opt)
+struct git_graph *graph_init_with_options(struct rev_info *opt, const char *arg)
 {
 	struct git_graph *graph = xmalloc(sizeof(struct git_graph));
 
+	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;
+	}
 	if (!column_colors)
 		graph_set_column_colors(column_colors_ansi,
 					column_colors_ansi_max);

@@ -382,6 +397,20 @@ static unsigned short graph_get_current_column_color(const struct git_graph *gra
  */
 static void graph_increment_column_color(struct git_graph *graph)
 {
+	if (column_colors_step) {
+		static int random_initialized;
+		int v;
+
+		if (!random_initialized) {
+			srand((unsigned int)getpid());
+			random_initialized = 1;
+		}
+		v = rand() % (column_colors_max - column_colors_step * 2);
+		graph->default_column_color += column_colors_step + v;
+		graph->default_column_color %= column_colors_max;
+		return;
+	}
+
 	graph->default_column_color = (graph->default_column_color + 1) %
 		column_colors_max;
 }

diff --git a/graph.h b/graph.h
index af62339..8069aa4 100644
--- a/graph.h
+++ b/graph.h

@@ -40,7 +40,13 @@ void graph_set_column_colors(const char **colors, unsigned short colors_max);
 /*
  * Create a new struct git_graph.
  */
-struct git_graph *graph_init(struct rev_info *opt);
+struct git_graph *graph_init_with_options(struct rev_info *opt, const char *arg);
+
+static inline struct git_graph *graph_init(struct rev_info *opt)
+{
+	return graph_init_with_options(opt, NULL);
+}
+
 
 /*
  * Update a git_graph with a new commit.

diff --git a/revision.c b/revision.c
index b37dbec..07bea54 100644
--- a/revision.c
+++ b/revision.c

@@ -1933,6 +1933,10 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		revs->topo_order = 1;
 		revs->rewrite_parents = 1;
 		revs->graph = graph_init(revs);
+	} else if (skip_prefix(arg, "--graph=", &arg)) {
+		revs->topo_order = 1;
+		revs->rewrite_parents = 1;
+		revs->graph = graph_init_with_options(revs, arg);
 	} else if (!strcmp(arg, "--root")) {
 		revs->show_root_diff = 1;
 	} else if (!strcmp(arg, "--no-commit-id")) {

-- 
2.8.2.524.g6ff3d78

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

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 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:

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

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

On Tue, Dec 20, 2016 at 11:57 PM, Jeff King [off-list ref] wrote:

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

quoted

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 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:

quoted

+     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).

Yep. If the jump sequence is a random one, we're less likely to run
into this. But I think Junio's "run git-log in 2 terminals with the
same coloring" convinces me randomization here is not the best thing.

The best solution would be select colors per text line, so we can pick
different colors. But I think that's a lot of computation (and
probably an NP problem too). The second best option is have a good,
predefined color palette. We don't need a red of all shades, we need
something that look distinct enough from the rest. I googled for this
first and failed. But I think I could approach it a different way:
collect colors that have names. That reduces the number of colors so
we can go back to "step 1 at a time" and still don't run into two
similar colors often.

quoted

---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 do. See the screenshot in [1] from the original mail. I have to
stare at --graph so often lately that it might get my attention before
other things.

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.

Exactly.

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.

Yeah. That also means we need the ability to override/negate config
options, perhaps something like --graph=-256colors.
-- 
Duy

If you have a 256 colors terminal (or one with true color support), then
the predefined 12 colors seem limited. On the other hand, you don't want
to draw graph lines with every single color in this mode because the two
colors could look extremely similar. This option allows you to hand pick
the colors you want.

Even with standard terminal, if your background color is neither black
or white, then the graph line may match your background and become
hidden. You can exclude your background color (or simply the colors you
hate) with this.

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 Sounds like the good first step should be something like this instead
 of jumping straight to generating a new color palette automatically.

 It's not hard to create a script that generate this config value
 based on some jump calculation, if you don't want to manually picking
 colors.

 Documentation/config.txt |  4 ++++
 graph.c                  | 36 ++++++++++++++++++++++++++++++++++--
 2 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index d51182a..4f26c2a 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt

@@ -2033,6 +2033,10 @@ log.follow::
 	i.e. it cannot be used to follow multiple files and does not work well
 	on non-linear history.
 
+log.graphColors::
+	A list of colors, separated by commas, that can be used to draw
+	history lines in `git log --graph`.
+
 log.showRoot::
 	If true, the initial commit will be shown as a big creation event.
 	This is equivalent to a diff against an empty tree.

diff --git a/graph.c b/graph.c
index d4e8519..9c58fd1 100644
--- a/graph.c
+++ b/graph.c

@@ -79,6 +79,39 @@ static void graph_show_line_prefix(const struct diff_options *diffopt)
 static const char **column_colors;
 static unsigned short column_colors_max;
 
+static void set_column_colors_by_config(void)
+{
+	static char **colors;
+	static int colors_max, colors_alloc;
+	char *string = NULL;
+	const char *end, *start;
+
+	if (git_config_get_string("log.graphcolors", &string)) {
+		graph_set_column_colors(column_colors_ansi,
+					column_colors_ansi_max);
+		return;
+	}
+
+	start = string;
+	end = string + strlen(string);
+	while (start < end) {
+		const char *comma = strchrnul(start, ',');
+		char color[COLOR_MAXLEN];
+
+		if (!color_parse_mem(start, comma - start, color)) {
+			ALLOC_GROW(colors, colors_max + 1, colors_alloc);
+			colors[colors_max++] = xstrdup(color);
+		} else
+			warning(_("ignore invalid color '%.*s'"),
+				(int)(comma - start), start);
+		start = comma + 1;
+	}
+	free(string);
+	ALLOC_GROW(colors, colors_max + 1, colors_alloc);
+	colors[colors_max] = xstrdup(GIT_COLOR_RESET);
+	graph_set_column_colors((const char **)colors, colors_max);
+}
+
 void graph_set_column_colors(const char **colors, unsigned short colors_max)
 {
 	column_colors = colors;

@@ -239,8 +272,7 @@ struct git_graph *graph_init(struct rev_info *opt)
 	struct git_graph *graph = xmalloc(sizeof(struct git_graph));
 
 	if (!column_colors)
-		graph_set_column_colors(column_colors_ansi,
-					column_colors_ansi_max);
+		set_column_colors_by_config();
 
 	graph->commit = NULL;
 	graph->revs = opt;

-- 
2.8.2.524.g6ff3d78