threads / patch / 43389

patchRe: [PATCH] Colourise git-branch output

Subject: Re: [PATCH] Colourise git-branch output

## tl;dr

9 messages between Dec 11, 2006 and Dec 12, 2006. Diffs are folded; open one to read it.

replies: 8people: 4as markdown or json

Andy Parkins· Dec 11, 2006, 22:10 UTC · lore

[PATCH] Colourise git-branch output

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(-)
Show changes to builtin-branch.c +59 −5
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
Junio C Hamano· Dec 11, 2006, 22:47 UTC · re: Andy Parkins · lore
Andy Parkins <andyparkins@gmail.com> writes:
Show 5 quoted lines
> 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?

Sean· Dec 12, 2006, 00:38 UTC · re: Andy Parkins · lore

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

Show 8 quoted lines
> +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.

Junio C Hamano· Dec 12, 2006, 01:11 UTC · re: Sean · lore
Sean <seanlkml@sympatico.ca> writes:
Show 15 quoted lines
> 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".
Sean· Dec 12, 2006, 01:24 UTC · re: Junio C Hamano · lore

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

Show 7 quoted lines
> > 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.

Linus Torvalds· Dec 12, 2006, 03:39 UTC · re: Sean · lore
On Mon, 11 Dec 2006, Sean wrote:
Show 5 quoted lines
> 
> 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.

Junio C Hamano· Dec 12, 2006, 05:07 UTC · re: Linus Torvalds · lore
Linus Torvalds <torvalds@osdl.org> writes:
Show 24 quoted lines
> 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
Linus Torvalds· Dec 12, 2006, 05:41 UTC · re: Junio C Hamano · lore
On Mon, 11 Dec 2006, Junio C Hamano wrote:
Show 10 quoted lines
> 
> 	[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.

Junio C Hamano· Dec 12, 2006, 07:58 UTC · re: Linus Torvalds · lore
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
Show 13 quoted lines
> ... 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.
	

← back to recent threads