Re: [PATCH v2] log --graph: customize the graph lines with config log.graphColors

Nguyễn Thái Ngọc Duy  [off-list ref] writes:

 Sounds like the good first step should be something like this instead
 of jumping straight to generating a new color palette automatically.

I like this not merely as a good first step but potentially a good
endgame.

I doubt you want these to be static (and allow multiple instances of
the same configuration variable to accumulate).  As you defined the
value to be comma-separated colors, users would want the usual "last
one wins" rule to override previous settings, no?

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

On the other hand, if you do want cumulative semantics, then you'd
need to free the previous set you held in the statics here, I would
think.

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

"ignore invalid color"?  Who is giving the command to ignore to whom?

"by" in the function name somehow sounds funny, at least to me.

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>
---
 Compared to v2:

 * set_column_colors_by_config() is renamed to set_column_colors()
 * no memory leak if the function is called the second time
 * proper space trimming since color_parse_mem expects so
 * fixed the warning message, giving some context
 * at least one test to exercise the code
 * I'm not going with the cumulative behavior because I think that's
   just harder to manage colors, and we would need a way to remove
   colors from the config too.

 Documentation/config.txt |  4 ++++
 graph.c                  | 46 ++++++++++++++++++++++++++++++++++++++++++++--
 t/t4202-log.sh           | 22 ++++++++++++++++++++++
 3 files changed, 70 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 0bcb679..33a007b 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt

@@ -2003,6 +2003,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 dd17201..048f5cb 100644
--- a/graph.c
+++ b/graph.c

@@ -62,6 +62,49 @@ enum graph_state {
 static const char **column_colors;
 static unsigned short column_colors_max;
 
+static void set_column_colors(void)
+{
+	static char **colors;
+	static int colors_max, colors_alloc;
+	char *string = NULL;
+	const char *end, *start;
+	int i;
+
+	for (i = 0; i < colors_max; i++)
+		free(colors[i]);
+	if (colors)
+		free(colors[colors_max]);
+	colors_max = 0;
+
+	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];
+
+		while (start < comma && isspace(*start))
+			start++;
+
+		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' in log.graphColors"),
+				(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;

@@ -208,8 +251,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();
 
 	graph->commit = NULL;
 	graph->revs = opt;

diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index e2db47c..afe0715 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh

@@ -313,6 +313,28 @@ test_expect_success 'log --graph with merge' '
 	test_cmp expect actual
 '
 
+cat > expect.colors <<\EOF
+*   Merge branch 'side'
+<BLUE>|<RESET><CYAN>\<RESET>
+<BLUE>|<RESET> * side-2
+<BLUE>|<RESET> * side-1
+* <CYAN>|<RESET> Second
+* <CYAN>|<RESET> sixth
+* <CYAN>|<RESET> fifth
+* <CYAN>|<RESET> fourth
+<CYAN>|<RESET><CYAN>/<RESET>
+* third
+* second
+* initial
+EOF
+
+test_expect_success 'log --graph with merge with log.graphColors' '
+	test_config log.graphColors " blue , cyan , red " &&
+	git log --color=always --graph --date-order --pretty=tformat:%s |
+		test_decode_color | sed "s/ *\$//" >actual &&
+	test_cmp expect.colors actual
+'
+
 test_expect_success 'log --raw --graph -m with merge' '
 	git log --raw --graph --oneline -m master | head -n 500 >actual &&
 	grep "initial" actual

-- 
2.8.2.524.g6ff3d78

On Sun, Jan 08, 2017 at 05:13:33PM +0700, Nguyễn Thái Ngọc Duy wrote:

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.

I like this approach much more than the 256-color option.

 * I'm not going with the cumulative behavior because I think that's
   just harder to manage colors, and we would need a way to remove
   colors from the config too.

Yeah, figuring out the list semantics would be a pain. This makes it
hard to exclude a single color, but I think it's more likely somebody
would want to replace the whole set with something that works well
against their background.

+test_expect_success 'log --graph with merge with log.graphColors' '
+	test_config log.graphColors " blue , cyan , red " &&

This funny syntax isn't required, right? It should work with the more
natural:

    test_config log.graphColors "blue, cyan, red"

-Peff

On Mon, Jan 9, 2017 at 12:34 PM, Jeff King [off-list ref] wrote:

quoted

+test_expect_success 'log --graph with merge with log.graphColors' '
+     test_config log.graphColors " blue , cyan , red " &&

This funny syntax isn't required, right? It should work with the more
natural:

    test_config log.graphColors "blue, cyan, red"

Yeah spaces are optional. I just wanted to put extra spaces there
because I hit a bug with them.
-- 
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>
---
 v4:

 * rename the function again (and it reads better)
 * use argv_array
 * fix a bug with two consecutive commas (after spaces are trimmed)

 Documentation/config.txt |  4 ++++
 graph.c                  | 43 +++++++++++++++++++++++++++++++++++++++++--
 t/t4202-log.sh           | 22 ++++++++++++++++++++++
 3 files changed, 67 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 0bcb679..33a007b 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt

@@ -2003,6 +2003,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 dd17201..83b1d42 100644
--- a/graph.c
+++ b/graph.c

@@ -4,6 +4,7 @@
 #include "graph.h"
 #include "diff.h"
 #include "revision.h"
+#include "argv-array.h"
 
 /* Internal API */
 

@@ -62,6 +63,45 @@ enum graph_state {
 static const char **column_colors;
 static unsigned short column_colors_max;
 
+static void read_graph_colors_config(void)
+{
+	static struct argv_array colors = ARGV_ARRAY_INIT;
+	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;
+	}
+
+	argv_array_clear(&colors);
+	start = string;
+	end = string + strlen(string);
+	while (start < end) {
+		const char *comma = strchrnul(start, ',');
+		char color[COLOR_MAXLEN];
+
+		while (start < comma && isspace(*start))
+			start++;
+		if (start == comma) {
+			start = comma + 1;
+			continue;
+		}
+
+		if (!color_parse_mem(start, comma - start, color))
+			argv_array_push(&colors, color);
+		else
+			warning(_("ignore invalid color '%.*s' in log.graphColors"),
+				(int)(comma - start), start);
+		start = comma + 1;
+	}
+	free(string);
+	argv_array_push(&colors, GIT_COLOR_RESET);
+	/* graph_set_column_colors takes a max-index, not a count */
+	graph_set_column_colors(colors.argv, colors.argc - 1);
+}
+
 void graph_set_column_colors(const char **colors, unsigned short colors_max)
 {
 	column_colors = colors;

@@ -208,8 +248,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);
+		read_graph_colors_config();
 
 	graph->commit = NULL;
 	graph->revs = opt;

diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index e2db47c..0aeabed 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh

@@ -313,6 +313,28 @@ test_expect_success 'log --graph with merge' '
 	test_cmp expect actual
 '
 
+cat > expect.colors <<\EOF
+*   Merge branch 'side'
+<BLUE>|<RESET><CYAN>\<RESET>
+<BLUE>|<RESET> * side-2
+<BLUE>|<RESET> * side-1
+* <CYAN>|<RESET> Second
+* <CYAN>|<RESET> sixth
+* <CYAN>|<RESET> fifth
+* <CYAN>|<RESET> fourth
+<CYAN>|<RESET><CYAN>/<RESET>
+* third
+* second
+* initial
+EOF
+
+test_expect_success 'log --graph with merge with log.graphColors' '
+	test_config log.graphColors ",, blue,invalid-color, cyan, red  , " &&
+	git log --color=always --graph --date-order --pretty=tformat:%s |
+		test_decode_color | sed "s/ *\$//" >actual &&
+	test_cmp expect.colors actual
+'
+
 test_expect_success 'log --raw --graph -m with merge' '
 	git log --raw --graph --oneline -m master | head -n 500 >actual &&
 	grep "initial" actual

-- 
2.8.2.524.g6ff3d78

v5 moves space trimming to color_parse_mem() from read_graph_colors_config,
which is renamed to parse_graph... because the config reading is moved
back to graph_init.

I think it looks better, but we may be pushing the limits of
argv_array's abuse.

Nguyễn Thái Ngọc Duy (3):
  color.c: fix color_parse_mem() with value_len == 0
  color.c: trim leading spaces in color_parse_mem()
  log --graph: customize the graph lines with config log.graphColors

 Documentation/config.txt |  4 ++++
 color.c                  | 10 +++++++++-
 graph.c                  | 42 +++++++++++++++++++++++++++++++++++++++---
 t/t4202-log.sh           | 22 ++++++++++++++++++++++
 4 files changed, 74 insertions(+), 4 deletions(-)

-- 
2.8.2.524.g6ff3d78

In this code we want to match the word "reset". If len is zero,
strncasecmp() will return zero and we incorrectly assume it's "reset" as
a result.

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 color.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/color.c b/color.c
index 81c2676..a9eadd1 100644
--- a/color.c
+++ b/color.c

@@ -207,6 +207,9 @@ int color_parse_mem(const char *value, int value_len, char *dst)
 	struct color fg = { COLOR_UNSPECIFIED };
 	struct color bg = { COLOR_UNSPECIFIED };
 
+	if (!len)
+		return -1;
+
 	if (!strncasecmp(value, "reset", len)) {
 		xsnprintf(dst, end - dst, GIT_COLOR_RESET);
 		return 0;

-- 
2.8.2.524.g6ff3d78

Normally color_parse_mem() is called from config parser which trims the
leading spaces already. The new caller in the next patch won't. Let's be
tidy and trim leading spaces too (we already trim trailing spaces before
comma).

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 color.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/color.c b/color.c
index a9eadd1..7bb4a96 100644
--- a/color.c
+++ b/color.c

@@ -207,10 +207,15 @@ int color_parse_mem(const char *value, int value_len, char *dst)
 	struct color fg = { COLOR_UNSPECIFIED };
 	struct color bg = { COLOR_UNSPECIFIED };
 
+	while (len > 0 && isspace(*ptr)) {
+		ptr++;
+		len--;
+	}
+
 	if (!len)
 		return -1;
 
-	if (!strncasecmp(value, "reset", len)) {
+	if (!strncasecmp(ptr, "reset", len)) {
 		xsnprintf(dst, end - dst, GIT_COLOR_RESET);
 		return 0;
 	}

-- 
2.8.2.524.g6ff3d78

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>
---
 Documentation/config.txt |  4 ++++
 graph.c                  | 42 +++++++++++++++++++++++++++++++++++++++---
 t/t4202-log.sh           | 22 ++++++++++++++++++++++
 3 files changed, 65 insertions(+), 3 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 0bcb679..33a007b 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt

@@ -2003,6 +2003,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 dd17201..822d40a 100644
--- a/graph.c
+++ b/graph.c

@@ -4,6 +4,7 @@
 #include "graph.h"
 #include "diff.h"
 #include "revision.h"
+#include "argv-array.h"
 
 /* Internal API */
 

@@ -62,6 +63,26 @@ enum graph_state {
 static const char **column_colors;
 static unsigned short column_colors_max;
 
+static void parse_graph_colors_config(struct argv_array *colors, const char *string)
+{
+	const char *end, *start;
+
+	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))
+			argv_array_push(colors, color);
+		else
+			warning(_("ignore invalid color '%.*s' in log.graphColors"),
+				(int)(comma - start), start);
+		start = comma + 1;
+	}
+	argv_array_push(colors, GIT_COLOR_RESET);
+}
+
 void graph_set_column_colors(const char **colors, unsigned short colors_max)
 {
 	column_colors = colors;

@@ -207,9 +228,24 @@ 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);
+	if (!column_colors) {
+		struct argv_array ansi_colors = {
+			column_colors_ansi,
+			column_colors_ansi_max + 1
+		};
+		struct argv_array *colors = &ansi_colors;
+		char *string;
+
+		if (!git_config_get_string("log.graphcolors", &string)) {
+			static struct argv_array custom_colors = ARGV_ARRAY_INIT;
+			argv_array_clear(&custom_colors);
+			parse_graph_colors_config(&custom_colors, string);
+			free(string);
+			colors = &custom_colors;
+		}
+		/* graph_set_column_colors takes a max-index, not a count */
+		graph_set_column_colors(colors->argv, colors->argc - 1);
+	}
 
 	graph->commit = NULL;
 	graph->revs = opt;

diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index e2db47c..0aeabed 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh

@@ -313,6 +313,28 @@ test_expect_success 'log --graph with merge' '
 	test_cmp expect actual
 '
 
+cat > expect.colors <<\EOF
+*   Merge branch 'side'
+<BLUE>|<RESET><CYAN>\<RESET>
+<BLUE>|<RESET> * side-2
+<BLUE>|<RESET> * side-1
+* <CYAN>|<RESET> Second
+* <CYAN>|<RESET> sixth
+* <CYAN>|<RESET> fifth
+* <CYAN>|<RESET> fourth
+<CYAN>|<RESET><CYAN>/<RESET>
+* third
+* second
+* initial
+EOF
+
+test_expect_success 'log --graph with merge with log.graphColors' '
+	test_config log.graphColors ",, blue,invalid-color, cyan, red  , " &&
+	git log --color=always --graph --date-order --pretty=tformat:%s |
+		test_decode_color | sed "s/ *\$//" >actual &&
+	test_cmp expect.colors actual
+'
+
 test_expect_success 'log --raw --graph -m with merge' '
 	git log --raw --graph --oneline -m master | head -n 500 >actual &&
 	grep "initial" actual

-- 
2.8.2.524.g6ff3d78

On Thu, Jan 19, 2017 at 06:41:21PM +0700, Nguyễn Thái Ngọc Duy wrote:

In this code we want to match the word "reset". If len is zero,
strncasecmp() will return zero and we incorrectly assume it's "reset" as
a result.

This is probably a good idea. This _is_ user-visible, so it's possible
somebody was using empty config as a synonym for "reset". But since it
was never documented, I feel like relying on that is somewhat crazy.

-Peff

On Thu, Jan 19, 2017 at 06:41:22PM +0700, Nguyễn Thái Ngọc Duy wrote:

Normally color_parse_mem() is called from config parser which trims the
leading spaces already. The new caller in the next patch won't. Let's be
tidy and trim leading spaces too (we already trim trailing spaces before
comma).

What comma? I don't think that exists until the next patch. :)

I think just trimming from the front is OK, though, because
color_parse_mem() trims trailing whitespace after a word. So either you
have a word and we will trim after it, or you do not (in which case
this will trim everything and hit the !len case you added).

So maybe a better commit message is just:

  Normally color_parse_mem() is called from config parser which trims
  the leading spaces already. The new caller in the next patch won't.
  Let's be tidy and trim leading spaces too (we already trim trailing
  spaces after a word).

-Peff

On Thu, Jan 19, 2017 at 06:41:23PM +0700, Nguyễn Thái Ngọc Duy wrote:

+static void parse_graph_colors_config(struct argv_array *colors, const char *string)
+{
+	const char *end, *start;
+
+	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))
+			argv_array_push(colors, color);
+		else
+			warning(_("ignore invalid color '%.*s' in log.graphColors"),
+				(int)(comma - start), start);
+		start = comma + 1;
+	}
+	argv_array_push(colors, GIT_COLOR_RESET);
+}

This looks good.

Hrm. At first I thought this would cause memory corrution, because your
argv_array_clear() would try to free() the non-heap array you've stuffed
inside. But you only clear the custom_colors array which actually is
dynamically allocated. This outer one is just here to give uniform
access:

+		struct argv_array *colors = &ansi_colors;
+		char *string;
+
+		if (!git_config_get_string("log.graphcolors", &string)) {
+			static struct argv_array custom_colors = ARGV_ARRAY_INIT;
+			argv_array_clear(&custom_colors);
+			parse_graph_colors_config(&custom_colors, string);
+			free(string);
+			colors = &custom_colors;
+		}
+		/* graph_set_column_colors takes a max-index, not a count */
+		graph_set_column_colors(colors->argv, colors->argc - 1);
+	}

Since there's only one line that cares about the result of "colors",
maybe it would be less confusing to do:

  if (!git_config_get-string("log.graphcolors", &string)) {
	... parse, etc ...
	graph_set_column_colors(colors.argv, colors.argc - 1);
  } else {
	graph_set_column_colors(column_colors_ansi,
	                        column_colors_ansi_max);
  }

-Peff

On Thu, Jan 19, 2017 at 11:38:41AM -0500, Jeff King wrote:

On Thu, Jan 19, 2017 at 06:41:21PM +0700, Nguyễn Thái Ngọc Duy wrote:

quoted

In this code we want to match the word "reset". If len is zero,
strncasecmp() will return zero and we incorrectly assume it's "reset" as
a result.

This is probably a good idea. This _is_ user-visible, so it's possible
somebody was using empty config as a synonym for "reset". But since it
was never documented, I feel like relying on that is somewhat crazy.

Hrm. This seems to break the add--interactive script if you do not have
color.diff.plain set:

  $ GIT_TRACE=1 git add -p
  ...
  22:58:12.568990 [pid=11401] git.c:387  trace: built-in: git 'config' '--get-color' 'color.diff.plain' ''
  fatal: unable to parse default color value
  config --get-color color.diff.plain : command returned error: 128

As you can see, the default value the empty string, which is now an
error.

The default in the C code for that value is GIT_COLOR_NORMAL, which
really is the empty string. So I think the old code was buggy to choose
"reset", but the new one is worse because it fails entirely. :)

We probably want something like this instead:

diff --git a/color.c b/color.c
index 190b2da96..dee61557e 100644
--- a/color.c
+++ b/color.c

@@ -212,8 +212,10 @@ int color_parse_mem(const char *value, int value_len, char *dst)
 		len--;
 	}
 
-	if (!len)
-		return -1;
+	if (!len) {
+		dst[0] = '\0';
+		return 0;
+	}
 
 	if (!strncasecmp(ptr, "reset", len)) {
 		xsnprintf(dst, end - dst, GIT_COLOR_RESET);

The breakage is in 'next' (it looks like it went out a few days ago; I'm
surprised I didn't notice until now).

-Peff