[PATCH] Colourise git-branch output

[PATCH] Colourise git-branch output

[Andy Parkins]

I wanted to have a visual indication of which branches are local and which are
remote in git-branch -a output; however Junio was concerned that someone might
be using the output in a script.  This patch addresses the problem by colouring
the git-branch output - which in "auto" mode won't be activated.

I've based it off the colouring code for builtin-diff.c; which means there is a
branch.color configuration variable that needs setting to something before the
color will appear.

This patch chooses green for local, red for remote and bold green for current.

As yet, there is no support for changing the colors using the config file; but
it wouldn't be hard to add.

Signed-off-by: Andy Parkins <andyparkins@gmail.com>
---
 builtin-branch.c |   57 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 54 insertions(+), 3 deletions(-)

diff --git a/builtin-branch.c b/builtin-branch.c
index 6dd33ee..de7f81e 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -5,6 +5,7 @@
  * Based on git-branch.sh by Junio C Hamano.
  */
 
+#include "color.h"
 #include "cache.h"
 #include "refs.h"
 #include "commit.h"
@@ -17,6 +18,38 @@ static const char builtin_branch_usage[]
 static const char *head;
 static unsigned char head_sha1[20];
 
+static int branch_use_color;
+static char branch_colors[][COLOR_MAXLEN] = {
+	"\033[m",	/* reset */
+	"",		/* PLAIN (normal) */
+	"\033[31m",	/* REMOTE (red) */
+	"\033[32m",	/* LOCAL (green) */
+	"\033[1;32m",	/* CURRENT (boldgreen) */
+};
+enum color_branch {
+	COLOR_BRANCH_RESET = 0,
+	COLOR_BRANCH_PLAIN = 1,
+	COLOR_BRANCH_REMOTE = 2,
+	COLOR_BRANCH_LOCAL = 3,
+	COLOR_BRANCH_CURRENT = 4,
+};
+
+int git_branch_config(const char *var, const char *value)
+{
+	if (!strcmp(var, "branch.color")) {
+		branch_use_color = git_config_colorbool(var, value);
+		return 0;
+	}
+	return git_default_config(var, value);
+}
+
+const char *branch_get_color(enum color_branch ix)
+{
+	if (branch_use_color)
+		return branch_colors[ix];
+	return "";
+}
+
 static int in_merge_bases(const unsigned char *sha1,
 			  struct commit *rev1,
 			  struct commit *rev2)
@@ -157,6 +190,7 @@ static void print_ref_list( int type_wan
 	int i;
 	char c;
 	struct ref_list ref_list;
+	int color;
 
 	memset( &ref_list, 0, sizeof( ref_list ) );
 	ref_list.type_wanted = type_wanted;
@@ -165,11 +199,28 @@ static void print_ref_list( int type_wan
 	qsort(ref_list.list, ref_list.index, sizeof(struct ref_item), ref_cmp);
 
 	for (i = 0; i < ref_list.index; i++) {
+		switch( ref_list.list[i].type ) {
+			case REF_LOCAL_BRANCH:
+				color = COLOR_BRANCH_LOCAL;
+				break;
+			case REF_REMOTE_BRANCH:
+				color = COLOR_BRANCH_REMOTE;
+				break;
+			default:
+				color = COLOR_BRANCH_PLAIN;
+				break;
+		}
+
 		c = ' ';
-		if (!strcmp(ref_list.list[i].name, head))
+		if (!strcmp(ref_list.list[i].name, head)) {
 			c = '*';
+			color = COLOR_BRANCH_CURRENT;
+		}
 
-		printf("%c %s\n", c, ref_list.list[i].name);
+		printf("%c %s%s%s\n", c,
+				branch_get_color(color),
+				ref_list.list[i].name,
+				branch_get_color(COLOR_BRANCH_RESET));
 	}
 
 	tidy_ref_list( &ref_list );
@@ -219,7 +270,7 @@ int cmd_branch(int argc, const char **ar
 	int i;
 	int type_wanted = REF_LOCAL_BRANCH;
 
-	git_config(git_default_config);
+	git_config(git_branch_config);
 
 	for (i = 1; i < argc; i++) {
 		const char *arg = argv[i];
-- 
1.4.3.2

Re: [PATCH] Colourise git-branch output

[Junio C Hamano]

Andy Parkins <andyparkins@gmail.com> writes:

> I wanted to have a visual indication of which branches are
> local and which are remote in git-branch -a output; however
> Junio was concerned that someone might be using the output in
> a script.  This patch addresses the problem by colouring the
> git-branch output - which in "auto" mode won't be activated.

Yuck.  We are getting more and more color happy.  As long as
this stays optional I'm Ok with it; we'll see if people find it
useful soon enough.

> @@ -165,11 +199,28 @@ static void print_ref_list( int type_wan
>  	qsort(ref_list.list, ref_list.index, sizeof(struct ref_item), ref_cmp);
>  
>  	for (i = 0; i < ref_list.index; i++) {
> +		switch( ref_list.list[i].type ) {
> +			case REF_LOCAL_BRANCH:
> +				color = COLOR_BRANCH_LOCAL;
> +				break;

Style.  SP between "switch" and open parenthesis, no SP after
that open parenthesis.  We tend to align "switch", "case", and
"default" on the same column.

Re: [PATCH] Colourise git-branch output

[Alex Riesen]

Junio C Hamano, Fri, Nov 03, 2006 20:25:59 +0100:
> > I wanted to have a visual indication of which branches are
> > local and which are remote in git-branch -a output; however
> > Junio was concerned that someone might be using the output in
> > a script.  This patch addresses the problem by colouring the
> > git-branch output - which in "auto" mode won't be activated.
> 
> Yuck.  We are getting more and more color happy.  As long as
> this stays optional I'm Ok with it; we'll see if people find it
> useful soon enough.
> 

As long as the output stays stable one can always colorize it with
an external program, using pseudo terminals, if needed. Wont work
for the other platform though. As usual.

[PATCH] Colourise git-branch output

[Andy Parkins]

I wanted to have a visual indication of which branches are local and which are
remote in git-branch -a output; however Junio was concerned that someone might
be using the output in a script.  This patch addresses the problem by colouring
the git-branch output - which in "auto" mode won't be activated.

I've based it off the colouring code for builtin-diff.c; which means there is a
branch.color configuration variable that needs setting to something before the
color will appear.

This patch chooses green for local, red for remote and bold green for current.

As yet, there is no support for changing the colors using the config file; but
it wouldn't be hard to add.

Signed-off-by: Andy Parkins <andyparkins@gmail.com>
---
I know Junio isn't keen on adding colour all over the place in git; but I
think it's appropriate here.  When running in "git-branch -a" mode (or even
"git-branch -r" mode for that matter), because the "refs/remotes" prefix is
stripped from the branch name, there is no way to distinguish between local
and remote branches.  Colouring them makes it easy to see which is which,
but more importantly doesn't break any scripts because the colour would be
automatically disabled.

 builtin-branch.c |   64 +++++++++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 59 insertions(+), 5 deletions(-)

diff --git a/builtin-branch.c b/builtin-branch.c
index 3d5cb0e..1c1fa8f 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -5,6 +5,7 @@
  * Based on git-branch.sh by Junio C Hamano.
  */
 
+#include "color.h"
 #include "cache.h"
 #include "refs.h"
 #include "commit.h"
@@ -17,6 +18,38 @@ static const char builtin_branch_usage[] =
 static const char *head;
 static unsigned char head_sha1[20];
 
+static int branch_use_color;
+static char branch_colors[][COLOR_MAXLEN] = {
+	"\033[m",	/* reset */
+	"",		/* PLAIN (normal) */
+	"\033[31m",	/* REMOTE (red) */
+	"\033[32m",	/* LOCAL (green) */
+	"\033[1;32m",	/* CURRENT (boldgreen) */
+};
+enum color_branch {
+	COLOR_BRANCH_RESET = 0,
+	COLOR_BRANCH_PLAIN = 1,
+	COLOR_BRANCH_REMOTE = 2,
+	COLOR_BRANCH_LOCAL = 3,
+	COLOR_BRANCH_CURRENT = 4,
+};
+
+int git_branch_config(const char *var, const char *value)
+{
+	if (!strcmp(var, "branch.color")) {
+		branch_use_color = git_config_colorbool(var, value);
+		return 0;
+	}
+	return git_default_config(var, value);
+}
+
+const char *branch_get_color(enum color_branch ix)
+{
+	if (branch_use_color)
+		return branch_colors[ix];
+	return "";
+}
+
 static int in_merge_bases(const unsigned char *sha1,
 			  struct commit *rev1,
 			  struct commit *rev2)
@@ -183,6 +216,7 @@ static void print_ref_list(int kinds, int verbose, int abbrev)
 	int i;
 	char c;
 	struct ref_list ref_list;
+	int color;
 
 	memset(&ref_list, 0, sizeof(ref_list));
 	ref_list.kinds = kinds;
@@ -191,18 +225,38 @@ static void print_ref_list(int kinds, int verbose, int abbrev)
 	qsort(ref_list.list, ref_list.index, sizeof(struct ref_item), ref_cmp);
 
 	for (i = 0; i < ref_list.index; i++) {
+		switch( ref_list.list[i].kind ) {
+			case REF_LOCAL_BRANCH:
+				color = COLOR_BRANCH_LOCAL;
+				break;
+			case REF_REMOTE_BRANCH:
+				color = COLOR_BRANCH_REMOTE;
+				break;
+			default:
+				color = COLOR_BRANCH_PLAIN;
+				break;
+		}
+
 		c = ' ';
 		if (ref_list.list[i].kind == REF_LOCAL_BRANCH &&
-				!strcmp(ref_list.list[i].name, head))
+				!strcmp(ref_list.list[i].name, head)) {
 			c = '*';
+			color = COLOR_BRANCH_CURRENT;
+		}
 
 		if (verbose) {
-			printf("%c %-*s", c, ref_list.maxwidth,
-			       ref_list.list[i].name);
+			printf("%c %s%-*s%s", c,
+					branch_get_color(color),
+					ref_list.maxwidth,
+					ref_list.list[i].name,
+					branch_get_color(COLOR_BRANCH_RESET));
 			print_ref_info(ref_list.list[i].sha1, abbrev);
 		}
 		else
-			printf("%c %s\n", c, ref_list.list[i].name);
+			printf("%c %s%s%s\n", c,
+					branch_get_color(color),
+					ref_list.list[i].name,
+					branch_get_color(COLOR_BRANCH_RESET));
 	}
 
 	free_ref_list(&ref_list);
@@ -253,7 +307,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	int kinds = REF_LOCAL_BRANCH;
 	int i;
 
-	git_config(git_default_config);
+	git_config(git_branch_config);
 
 	for (i = 1; i < argc; i++) {
 		const char *arg = argv[i];
-- 
1.4.4.1.geeee8

Re: [PATCH] Colourise git-branch output

[Junio C Hamano]

Andy Parkins <andyparkins@gmail.com> writes:

> I wanted to have a visual indication of which branches are
> local and which are remote in git-branch -a output; however
> Junio was concerned that someone might be using the output in
> a script.  This patch addresses the problem by colouring the
> git-branch output - which in "auto" mode won't be activated.

Hmm.

"git branch --color" does not seem to work, so is this config
only?

> As yet, there is no support for changing the colors using the
> config file; but it wouldn't be hard to add.

I am not sure about that.  Would it be something like this?

	[branch.color]
        	remote = purple
                local = yellow

I wonder what would happen when you have a branch called
"color".  Would the above cause "git-fetch" to fetch from
"purple" remote by default?

Re: [PATCH] Colourise git-branch output

[Sean]

On Mon, 11 Dec 2006 22:10:08 +0000
Andy Parkins <andyparkins@gmail.com> wrote:

> +int git_branch_config(const char *var, const char *value)
> +{
> +	if (!strcmp(var, "branch.color")) {
> +		branch_use_color = git_config_colorbool(var, value);
> +		return 0;
> +	}
> +	return git_default_config(var, value);
> +}

As Junio already highlighted, the "branch.*" namespace is for actual
branch names.  This config option should go into "color.branch" or some
other spot.

Re: [PATCH] Colourise git-branch output

[Junio C Hamano]

Sean <seanlkml@sympatico.ca> writes:

> On Mon, 11 Dec 2006 22:10:08 +0000
> Andy Parkins <andyparkins@gmail.com> wrote:
>
>> +int git_branch_config(const char *var, const char *value)
>> +{
>> +	if (!strcmp(var, "branch.color")) {
>> +		branch_use_color = git_config_colorbool(var, value);
>> +		return 0;
>> +	}
>> +	return git_default_config(var, value);
>> +}
>
> As Junio already highlighted, the "branch.*" namespace is for actual
> branch names.  This config option should go into "color.branch" or some
> other spot.

I didn't.  And "branch.color = auto" is actually fine.

The problematic case is "branch.color.remote = purple".

Re: [PATCH] Colourise git-branch output

[Sean]

On Mon, 11 Dec 2006 17:11:24 -0800
Junio C Hamano <junkio@cox.net> wrote:


> > As Junio already highlighted, the "branch.*" namespace is for actual
> > branch names.  This config option should go into "color.branch" or some
> > other spot.
> 
> I didn't.  And "branch.color = auto" is actually fine.
> 
> The problematic case is "branch.color.remote = purple".

Technically it is workable.. but why even start down the road of having
anything but branch names after a "branch."?   There has to be a better
spot for this variable, and it makes it more future proof, as you
highlighted.

Re: [PATCH] Colourise git-branch output

[Linus Torvalds]

On Mon, 11 Dec 2006, Sean wrote:
> 
> Technically it is workable.. but why even start down the road of having
> anything but branch names after a "branch."?   There has to be a better
> spot for this variable, and it makes it more future proof, as you
> highlighted.

I do agree with Sean, both for the stability reason, but perhaps even more 
because I personally think it would just be better to have a separate 
"[color]" subsection.

In fact, I'd almost prefer to see

	[color]
		diff = auto

over

	[diff]
		color = auto

exactly because once we have different things that take colorization 
arguments, it's just nicer to have them all together (and we already have 
"status", and now we're getting "branch" too.

Re: [PATCH] Colourise git-branch output

[Junio C Hamano]

Linus Torvalds <torvalds@osdl.org> writes:

> On Mon, 11 Dec 2006, Sean wrote:
>> 
>> Technically it is workable.. but why even start down the road of having
>> anything but branch names after a "branch."?   There has to be a better
>> spot for this variable, and it makes it more future proof, as you
>> highlighted.
>
> I do agree with Sean, both for the stability reason, but perhaps even more 
> because I personally think it would just be better to have a separate 
> "[color]" subsection.
>
> In fact, I'd almost prefer to see
>
> 	[color]
> 		diff = auto
>
> over
>
> 	[diff]
> 		color = auto
>
> exactly because once we have different things that take colorization 
> arguments, it's just nicer to have them all together (and we already have 
> "status", and now we're getting "branch" too.

I tend to agree.  We probably should start deprecating
diff.color and diff.color.<stuff> and swap them around,
like this:

	[color]
        	diff = auto
                branch = auto
                # it begs "* = auto" entry perhaps...
	[color.diff]
        	old = red
                new = green
	[color.branch]
        	remote = purple

Re: [PATCH] Colourise git-branch output

[Linus Torvalds]

On Mon, 11 Dec 2006, Junio C Hamano wrote:
> 
> 	[color]
>         	diff = auto
>                 branch = auto
>                 # it begs "* = auto" entry perhaps...
> 	[color.diff]
>         	old = red
>                 new = green
> 	[color.branch]
>         	remote = purple

I wish you'd learn to use the proper syntax ;)

It would be

	[color "diff"]
		old = red
		new = green

and what's nice about it is that you can also do

	[color "diff"]
		auto
		old = red
		new = green

and the config file rules for booleans are such that a config variable 
without the "= val" part parses the same as "= true", so you can now do

	git repo-config --bool color.diff.auto

and it will say "true".

Now, I just think that would be a nice syntax.

Of course, for legacy reasons, and for people who rather than keeping the 
_color_ information together want to keep the _diff_ information together, 
we can/probably-should support both color.diff.* and diff.color.* formats.

[PATCH] Colourise git-branch output

[Andy Parkins]

I wanted to have a visual indication of which branches are local and
which are remote in git-branch -a output; however Junio was concerned
that someone might be using the output in a script.  This patch
addresses the problem by colouring the git-branch output - which in
"auto" mode won't be activated.

I've based it off the colouring code for builtin-diff.c; which means
there is a branch color configuration variable that needs setting to
something before the color will appear.

The colour parameter is "color.branch" rather than "branch.color" to
avoid clashing with the default namespace for default branch merge
definitions.

This patch chooses green for local, red for remote and bold green for
current.

Signed-off-by: Andy Parkins <andyparkins@gmail.com>
---
 builtin-branch.c |   92 +++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 87 insertions(+), 5 deletions(-)

diff --git a/builtin-branch.c b/builtin-branch.c
index 3d5cb0e..7c87b8d 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -5,6 +5,7 @@
  * Based on git-branch.sh by Junio C Hamano.
  */
 
+#include "color.h"
 #include "cache.h"
 #include "refs.h"
 #include "commit.h"
@@ -17,6 +18,58 @@ static const char builtin_branch_usage[] =
 static const char *head;
 static unsigned char head_sha1[20];
 
+static int branch_use_color;
+static char branch_colors[][COLOR_MAXLEN] = {
+	"\033[m",	/* reset */
+	"",		/* PLAIN (normal) */
+	"\033[31m",	/* REMOTE (red) */
+	"\033[32m",	/* LOCAL (green) */
+	"\033[1;32m",	/* CURRENT (boldgreen) */
+};
+enum color_branch {
+	COLOR_BRANCH_RESET = 0,
+	COLOR_BRANCH_PLAIN = 1,
+	COLOR_BRANCH_REMOTE = 2,
+	COLOR_BRANCH_LOCAL = 3,
+	COLOR_BRANCH_CURRENT = 4,
+};
+
+static int parse_branch_color_slot(const char *var, int ofs)
+{
+	if (!strcasecmp(var+ofs, "plain"))
+		return COLOR_BRANCH_PLAIN;
+	if (!strcasecmp(var+ofs, "reset"))
+		return COLOR_BRANCH_RESET;
+	if (!strcasecmp(var+ofs, "remote"))
+		return COLOR_BRANCH_REMOTE;
+	if (!strcasecmp(var+ofs, "local"))
+		return COLOR_BRANCH_LOCAL;
+	if (!strcasecmp(var+ofs, "current"))
+		return COLOR_BRANCH_CURRENT;
+	die("bad config variable '%s'", var);
+}
+
+int git_branch_config(const char *var, const char *value)
+{
+	if (!strcmp(var, "color.branch")) {
+		branch_use_color = git_config_colorbool(var, value);
+		return 0;
+	}
+	if (!strncmp(var, "color.branch.", 13)) {
+		int slot = parse_branch_color_slot(var, 13);
+		color_parse(value, var, branch_colors[slot]);
+		return 0;
+	}
+	return git_default_config(var, value);
+}
+
+const char *branch_get_color(enum color_branch ix)
+{
+	if (branch_use_color)
+		return branch_colors[ix];
+	return "";
+}
+
 static int in_merge_bases(const unsigned char *sha1,
 			  struct commit *rev1,
 			  struct commit *rev2)
@@ -183,6 +236,7 @@ static void print_ref_list(int kinds, int verbose, int abbrev)
 	int i;
 	char c;
 	struct ref_list ref_list;
+	int color;
 
 	memset(&ref_list, 0, sizeof(ref_list));
 	ref_list.kinds = kinds;
@@ -191,18 +245,38 @@ static void print_ref_list(int kinds, int verbose, int abbrev)
 	qsort(ref_list.list, ref_list.index, sizeof(struct ref_item), ref_cmp);
 
 	for (i = 0; i < ref_list.index; i++) {
+		switch( ref_list.list[i].kind ) {
+			case REF_LOCAL_BRANCH:
+				color = COLOR_BRANCH_LOCAL;
+				break;
+			case REF_REMOTE_BRANCH:
+				color = COLOR_BRANCH_REMOTE;
+				break;
+			default:
+				color = COLOR_BRANCH_PLAIN;
+				break;
+		}
+
 		c = ' ';
 		if (ref_list.list[i].kind == REF_LOCAL_BRANCH &&
-				!strcmp(ref_list.list[i].name, head))
+				!strcmp(ref_list.list[i].name, head)) {
 			c = '*';
+			color = COLOR_BRANCH_CURRENT;
+		}
 
 		if (verbose) {
-			printf("%c %-*s", c, ref_list.maxwidth,
-			       ref_list.list[i].name);
+			printf("%c %s%-*s%s", c,
+					branch_get_color(color),
+					ref_list.maxwidth,
+					ref_list.list[i].name,
+					branch_get_color(COLOR_BRANCH_RESET));
 			print_ref_info(ref_list.list[i].sha1, abbrev);
 		}
 		else
-			printf("%c %s\n", c, ref_list.list[i].name);
+			printf("%c %s%s%s\n", c,
+					branch_get_color(color),
+					ref_list.list[i].name,
+					branch_get_color(COLOR_BRANCH_RESET));
 	}
 
 	free_ref_list(&ref_list);
@@ -253,7 +327,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	int kinds = REF_LOCAL_BRANCH;
 	int i;
 
-	git_config(git_default_config);
+	git_config(git_branch_config);
 
 	for (i = 1; i < argc; i++) {
 		const char *arg = argv[i];
@@ -297,6 +371,14 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 			verbose = 1;
 			continue;
 		}
+		if (!strcmp(arg, "--color")) {
+			branch_use_color = 1;
+			continue;
+		}
+		if (!strcmp(arg, "--no-color")) {
+			branch_use_color = 0;
+			continue;
+		}
 		usage(builtin_branch_usage);
 	}
 
-- 
1.4.4.1.geeee8

Re: [PATCH] Colourise git-branch output

[Junio C Hamano]

Linus Torvalds <torvalds@osdl.org> writes:

> I wish you'd learn to use the proper syntax ;)

Ok, ok, I'm an old timer.  But [color.diff] syntax is also
accepted and I have it in my ~/.gitconfig:

	$ cat ~/.gitconfig
	[diff.color]
        	old = red reverse
	$ git repo-config --global diff.color.old
        red reverse
        $ exit

> ... what's nice about it is that you can also do
>
> 	[color "diff"]
> 		auto
> 		old = red
> 		new = green
>
> and the config file rules for booleans are such that a config variable 
> without the "= val" part parses the same as "= true", so you can now do
>
> 	git repo-config --bool color.diff.auto
>
> and it will say "true".

I find that your version of "auto" above to be utterly
confusing.

The existing configuration variable diff.color as an extended
boolean takes true, false or "auto".

I do agree this is a nice way to say that I pretend to be color
blind to git repo-config:

	$ git repo-config --global --bool color.diff false
        $ cat ~/.gitconfig
        [color]
        	diff = false

and it is also nice to be able to say my preference is "auto"
with (you can have --bool with this example and still use
"auto", by the way, when setting):

	$ git repo-config --global color.diff auto
        $ cat ~/.gitconfig
        [color]
        	diff = auto

I also agree it is nice that the "boolean magic" kicks in when
we hand-craft it:

	$ cat ~/.gitconfig
        [color]
        	diff
	$ git repo-config --global --bool color.diff
        true

But your handcrafted

	[color "diff"]
        	auto

simply feels a very confusing syntax.  How would you even
express my preference is always-color (that is, "true")?

If we are changing color.diff to color.diff.usecolor which is
bool + auto, then I would understand it.

	[color "diff"]
        	usecolor
                # usecolor = auto
                # usecolor = no

but that would not let you query with --bool ("auto" would
barf).

I guess we should at least extend --bool so that scripts can
easily query extended bools; if the value looks like a bool (or
there is no value), it would return it normalized to "true" or
"false" so they do not have to say:

	case `repo-config --bool the.variable` in
        true | yes | on | 1)
        	echo ah you mean yes ;;
	esac

but if the value is not either boolean true or false, then
return the string as is (such as "auto").  I am inclined to just
extend --bool itself to do so (which means we would lose the
error checking form the script for truly boolean fields) rather
than adding a new --bool-or-string option.

In any case,

	[color]
        	diff
                # diff = auto
                # diff = no

would work already if we only talk about built-in accesses.  We
would need to extend repo-config --bool to make it easier for
scripts to work with this extended bool, but that is a minor
detail.

So it seems to me that the only reason to have
color.diff.usecolor is so that we can have "diff" related color
settings in one place.  Which is fine.

You could do the same thing with your "auto"

	[color "diff"]
        	auto
		# auto = always
                # auto = no

but what that means is that color.diff.auto _is_ the extended
boolean that tells us to:

	* use color on terminal when set to true
        * use color unconditionally when set to a non-bool 'always'
        * never use color when set to false

which sounds rather, eh, unusual.


	

Re: [PATCH] Colourise git-branch output

[Johannes Sixt]

Andy Parkins wrote:
> This patch chooses green for local, red for remote and bold green for
> current.

Sorry for chiming in so late, but red and green are usually poor choices
since red-green color-blindness is surprisingly frequent...

Maybe its sufficient to have just the remote branches (dark-)red, and
the rest in the default color, with the current branch bold?

-- Hannes

Re: [PATCH] Colourise git-branch output

[Junio C Hamano]

Johannes Sixt <J.Sixt@eudaptics.com> writes:

> Andy Parkins wrote:
>> This patch chooses green for local, red for remote and bold green for
>> current.
>
> Sorry for chiming in so late, but red and green are usually poor choices
> since red-green color-blindness is surprisingly frequent...
>
> Maybe its sufficient to have just the remote branches (dark-)red, and
> the rest in the default color, with the current branch bold?

Even without red-green blindness issue, I think that makes
sense.  colored-diff uses green/red for added/deleted but that
is shown against the context in plain.  A sane thing to do for
branch listing would be to show the usual case (i.e. local) in
plain and show the remote ones differently.

Something like this on top of Andy's?

If we keep '*' prefix for the current one, I do not see a reason
to show it in a different color from other local branches, by
the way, but I did not go that far in this patch.

---
diff --git a/builtin-branch.c b/builtin-branch.c
index 7c87b8d..d1c243d 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -23,8 +23,8 @@ static char branch_colors[][COLOR_MAXLEN] = {
 	"\033[m",	/* reset */
 	"",		/* PLAIN (normal) */
 	"\033[31m",	/* REMOTE (red) */
-	"\033[32m",	/* LOCAL (green) */
-	"\033[1;32m",	/* CURRENT (boldgreen) */
+	"",		/* LOCAL (normal) */
+	"\033[32m",	/* CURRENT (green) */
 };
 enum color_branch {
 	COLOR_BRANCH_RESET = 0,

Re: [PATCH] Colourise git-branch output

[Andy Parkins]

On Tuesday 2006 December 12 10:10, Junio C Hamano wrote:

> Even without red-green blindness issue, I think that makes
> sense.  colored-diff uses green/red for added/deleted but that

I certainly don't have any objection.  For me, the use of colour is not to 
make things look pretty it's to give visual queues.

> is shown against the context in plain.  A sane thing to do for
> branch listing would be to show the usual case (i.e. local) in
> plain and show the remote ones differently.

The only reason I picked red and green was to indicate "can be checked out" 
and "cannot be checked out".  However, when git eventually allows arbitrary 
commits to be checked out that green/red, can/can't distinction will be 
meaningless anyway.

> +	"",		/* LOCAL (normal) */
> +	"\033[32m",	/* CURRENT (green) */

In keeping with the "don't use green" idea - can I suggest just bold normal 
for the CURRENT?  That way there is the most minimal use of colour for the 
default git-branch output, but still retaining a visual indicator.

Andy
-- 
Dr Andy Parkins, M Eng (hons), MIEE

Re: [PATCH] Colourise git-branch output

[Junio C Hamano]

Andy Parkins <andyparkins@gmail.com> writes:

> The colour parameter is "color.branch" rather than "branch.color" to
> avoid clashing with the default namespace for default branch merge
> definitions.

Very nice.

>  		c = ' ';
>  		if (ref_list.list[i].kind == REF_LOCAL_BRANCH &&
> -				!strcmp(ref_list.list[i].name, head))
> +				!strcmp(ref_list.list[i].name, head)) {
>  			c = '*';
> +			color = COLOR_BRANCH_CURRENT;
> +		}
>  
>  		if (verbose) {
> -			printf("%c %-*s", c, ref_list.maxwidth,
> -			       ref_list.list[i].name);
> +			printf("%c %s%-*s%s", c,
> +					branch_get_color(color),
> +					ref_list.maxwidth,
> +					ref_list.list[i].name,
> +					branch_get_color(COLOR_BRANCH_RESET));
>  			print_ref_info(ref_list.list[i].sha1, abbrev);
>  		}
>  		else
> -			printf("%c %s\n", c, ref_list.list[i].name);
> +			printf("%c %s%s%s\n", c,
> +					branch_get_color(color),
> +					ref_list.list[i].name,
> +					branch_get_color(COLOR_BRANCH_RESET));
>  	}

Now this makes me wonder if under output coloring we would still
want the two-space indent and '*' prefix.

Re: [PATCH] Colourise git-branch output

[Junio C Hamano]

Andy Parkins <andyparkins@gmail.com> writes:

>> +	"",		/* LOCAL (normal) */
>> +	"\033[32m",	/* CURRENT (green) */
>
> In keeping with the "don't use green" idea - can I suggest just bold normal 
> for the CURRENT?  That way there is the most minimal use of colour for the 
> default git-branch output, but still retaining a visual indicator.

I do not have strong preference either way.

The CURRENT is highlighted with '*' so I think we certainly can
lose green and even go vanilla.  I only tried to avoid bold
because no built-in default coloring currently use it, and in
the past I've worked with terminals that do not have enough
contrast between normal and bold and for some people that might
become an issue.