{"thread":{"id":"43389","subject":"Re: [PATCH] Colourise git-branch output","startedAt":"2006-12-11T22:10:08Z","lastAt":"2006-12-12T07:58:09Z","messageCount":9,"participants":["Sean","Linus Torvalds","Junio C Hamano","Andy Parkins"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"295242","messageId":"200612112210.08327.andyparkins@gmail.com","threadId":"43389","inReplyTo":null,"subject":"[PATCH] Colourise git-branch output","fromName":"Andy Parkins","fromEmail":"andyparkins@gmail.com","sentAt":"2006-12-11T22:10:08Z","receivedAt":"2006-12-11T22:10:08Z","isPatch":true,"sender":{"key":"andyparkins@gmail.com","avatar":null},"body":"I wanted to have a visual indication of which branches are local and which are\nremote in git-branch -a output; however Junio was concerned that someone might\nbe using the output in a script. This patch addresses the problem by colouring\nthe git-branch output - which in \"auto\" mode won't be activated.\n\nI've based it off the colouring code for builtin-diff.c; which means there is a\nbranch.color configuration variable that needs setting to something before the\ncolor will appear.\n\nThis patch chooses green for local, red for remote and bold green for current.\n\nAs yet, there is no support for changing the colors using the config file; but\nit wouldn't be hard to add.\n\nSigned-off-by: Andy Parkins \n---\nI know Junio isn't keen on adding colour all over the place in git; but I\nthink it's appropriate here. When running in \"git-branch -a\" mode (or even\n\"git-branch -r\" mode for that matter), because the \"refs/remotes\" prefix is\nstripped from the branch name, there is no way to distinguish between local\nand remote branches. Colouring them makes it easy to see which is which,\nbut more importantly doesn't break any scripts because the colour would be\nautomatically disabled.\n\n builtin-branch.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++----\n 1 files changed, 59 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin-branch.c b/builtin-branch.c\nindex 3d5cb0e..1c1fa8f 100644\n--- a/builtin-branch.c\n+++ b/builtin-branch.c\n@@ -5,6 +5,7 @@\n * Based on git-branch.sh by Junio C Hamano.\n */\n \n+#include \"color.h\"\n #include \"cache.h\"\n #include \"refs.h\"\n #include \"commit.h\"\n@@ -17,6 +18,38 @@ static const char builtin_branch_usage[] =\n static const char *head;\n static unsigned char head_sha1[20];\n \n+static int branch_use_color;\n+static char branch_colors[][COLOR_MAXLEN] = {\n+\t\"\\033[m\",\t/* reset */\n+\t\"\",\t\t/* PLAIN (normal) */\n+\t\"\\033[31m\",\t/* REMOTE (red) */\n+\t\"\\033[32m\",\t/* LOCAL (green) */\n+\t\"\\033[1;32m\",\t/* CURRENT (boldgreen) */\n+};\n+enum color_branch {\n+\tCOLOR_BRANCH_RESET = 0,\n+\tCOLOR_BRANCH_PLAIN = 1,\n+\tCOLOR_BRANCH_REMOTE = 2,\n+\tCOLOR_BRANCH_LOCAL = 3,\n+\tCOLOR_BRANCH_CURRENT = 4,\n+};\n+\n+int git_branch_config(const char *var, const char *value)\n+{\n+\tif (!strcmp(var, \"branch.color\")) {\n+\t\tbranch_use_color = git_config_colorbool(var, value);\n+\t\treturn 0;\n+\t}\n+\treturn git_default_config(var, value);\n+}\n+\n+const char *branch_get_color(enum color_branch ix)\n+{\n+\tif (branch_use_color)\n+\t\treturn branch_colors[ix];\n+\treturn \"\";\n+}\n+\n static int in_merge_bases(const unsigned char *sha1,\n \t\t\t struct commit *rev1,\n \t\t\t struct commit *rev2)\n@@ -183,6 +216,7 @@ static void print_ref_list(int kinds, int verbose, int abbrev)\n \tint i;\n \tchar c;\n \tstruct ref_list ref_list;\n+\tint color;\n \n \tmemset(&ref_list, 0, sizeof(ref_list));\n \tref_list.kinds = kinds;\n@@ -191,18 +225,38 @@ static void print_ref_list(int kinds, int verbose, int abbrev)\n \tqsort(ref_list.list, ref_list.index, sizeof(struct ref_item), ref_cmp);\n \n \tfor (i = 0; i < ref_list.index; i++) {\n+\t\tswitch( ref_list.list[i].kind ) {\n+\t\t\tcase REF_LOCAL_BRANCH:\n+\t\t\t\tcolor = COLOR_BRANCH_LOCAL;\n+\t\t\t\tbreak;\n+\t\t\tcase REF_REMOTE_BRANCH:\n+\t\t\t\tcolor = COLOR_BRANCH_REMOTE;\n+\t\t\t\tbreak;\n+\t\t\tdefault:\n+\t\t\t\tcolor = COLOR_BRANCH_PLAIN;\n+\t\t\t\tbreak;\n+\t\t}\n+\n \t\tc = ' ';\n \t\tif (ref_list.list[i].kind == REF_LOCAL_BRANCH &&\n-\t\t\t\t!strcmp(ref_list.list[i].name, head))\n+\t\t\t\t!strcmp(ref_list.list[i].name, head)) {\n \t\t\tc = '*';\n+\t\t\tcolor = COLOR_BRANCH_CURRENT;\n+\t\t}\n \n \t\tif (verbose) {\n-\t\t\tprintf(\"%c %-*s\", c, ref_list.maxwidth,\n-\t\t\t ref_list.list[i].name);\n+\t\t\tprintf(\"%c %s%-*s%s\", c,\n+\t\t\t\t\tbranch_get_color(color),\n+\t\t\t\t\tref_list.maxwidth,\n+\t\t\t\t\tref_list.list[i].name,\n+\t\t\t\t\tbranch_get_color(COLOR_BRANCH_RESET));\n \t\t\tprint_ref_info(ref_list.list[i].sha1, abbrev);\n \t\t}\n \t\telse\n-\t\t\tprintf(\"%c %s\\n\", c, ref_list.list[i].name);\n+\t\t\tprintf(\"%c %s%s%s\\n\", c,\n+\t\t\t\t\tbranch_get_color(color),\n+\t\t\t\t\tref_list.list[i].name,\n+\t\t\t\t\tbranch_get_color(COLOR_BRANCH_RESET));\n \t}\n \n \tfree_ref_list(&ref_list);\n@@ -253,7 +307,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \tint kinds = REF_LOCAL_BRANCH;\n \tint i;\n \n-\tgit_config(git_default_config);\n+\tgit_config(git_branch_config);\n \n \tfor (i = 1; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n-- \n1.4.4.1.geeee8\n"},{"id":"297445","messageId":"7vbqma1251.fsf@assigned-by-dhcp.cox.net","threadId":"43389","inReplyTo":"200612112210.08327.andyparkins@gmail.com","subject":"Re: [PATCH] Colourise git-branch output","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-12-11T22:47:06Z","receivedAt":"2006-12-11T22:47:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andy Parkins writes:\n\n> I wanted to have a visual indication of which branches are\n> local and which are remote in git-branch -a output; however\n> Junio was concerned that someone might be using the output in\n> a script. This patch addresses the problem by colouring the\n> git-branch output - which in \"auto\" mode won't be activated.\n\nHmm.\n\n\"git branch --color\" does not seem to work, so is this config\nonly?\n\n> As yet, there is no support for changing the colors using the\n> config file; but it wouldn't be hard to add.\n\nI am not sure about that. Would it be something like this?\n\n\t[branch.color]\n \tremote = purple\n local = yellow\n\nI wonder what would happen when you have a branch called\n\"color\". Would the above cause \"git-fetch\" to fetch from\n\"purple\" remote by default?\n"},{"id":"294588","messageId":"BAYC1-PASMTP114CF00792AB16B95C1597AED70@CEZ.ICE","threadId":"43389","inReplyTo":"200612112210.08327.andyparkins@gmail.com","subject":"Re: [PATCH] Colourise git-branch output","fromName":"Sean","fromEmail":"seanlkml@sympatico.ca","sentAt":"2006-12-12T00:38:57Z","receivedAt":"2006-12-12T00:38:57Z","isPatch":true,"sender":{"key":"seanlkml@sympatico.ca","avatar":"https://gravatar.com/avatar/f92923f54fc08c401fc59b71829d4b89e9b8087fbba45ff87c82e6a83aee02ae?d=mp&s=160"},"body":"On Mon, 11 Dec 2006 22:10:08 +0000\nAndy Parkins wrote:\n\n> +int git_branch_config(const char *var, const char *value)\n> +{\n> +\tif (!strcmp(var, \"branch.color\")) {\n> +\t\tbranch_use_color = git_config_colorbool(var, value);\n> +\t\treturn 0;\n> +\t}\n> +\treturn git_default_config(var, value);\n> +}\n\nAs Junio already highlighted, the \"branch.*\" namespace is for actual\nbranch names. This config option should go into \"color.branch\" or some\nother spot.\n\n"},{"id":"295162","messageId":"7v7iwx2a0z.fsf@assigned-by-dhcp.cox.net","threadId":"43389","inReplyTo":"BAYC1-PASMTP114CF00792AB16B95C1597AED70@CEZ.ICE","subject":"Re: [PATCH] Colourise git-branch output","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-12-12T01:11:24Z","receivedAt":"2006-12-12T01:11:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sean writes:\n\n> On Mon, 11 Dec 2006 22:10:08 +0000\n> Andy Parkins wrote:\n>\n>> +int git_branch_config(const char *var, const char *value)\n>> +{\n>> +\tif (!strcmp(var, \"branch.color\")) {\n>> +\t\tbranch_use_color = git_config_colorbool(var, value);\n>> +\t\treturn 0;\n>> +\t}\n>> +\treturn git_default_config(var, value);\n>> +}\n>\n> As Junio already highlighted, the \"branch.*\" namespace is for actual\n> branch names. This config option should go into \"color.branch\" or some\n> other spot.\n\nI didn't. And \"branch.color = auto\" is actually fine.\n\nThe problematic case is \"branch.color.remote = purple\".\n\n\n"},{"id":"296949","messageId":"BAYC1-PASMTP092A9E80667F373BF69AA6AED70@CEZ.ICE","threadId":"43389","inReplyTo":"7v7iwx2a0z.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Colourise git-branch output","fromName":"Sean","fromEmail":"seanlkml@sympatico.ca","sentAt":"2006-12-12T01:24:46Z","receivedAt":"2006-12-12T01:24:46Z","isPatch":true,"sender":{"key":"seanlkml@sympatico.ca","avatar":"https://gravatar.com/avatar/f92923f54fc08c401fc59b71829d4b89e9b8087fbba45ff87c82e6a83aee02ae?d=mp&s=160"},"body":"On Mon, 11 Dec 2006 17:11:24 -0800\nJunio C Hamano wrote:\n\n\n> > As Junio already highlighted, the \"branch.*\" namespace is for actual\n> > branch names. This config option should go into \"color.branch\" or some\n> > other spot.\n> \n> I didn't. And \"branch.color = auto\" is actually fine.\n> \n> The problematic case is \"branch.color.remote = purple\".\n\nTechnically it is workable.. but why even start down the road of having\nanything but branch names after a \"branch.\"? There has to be a better\nspot for this variable, and it makes it more future proof, as you\nhighlighted.\n\n"},{"id":"295490","messageId":"Pine.LNX.4.64.0612111936240.6452@woody.osdl.org","threadId":"43389","inReplyTo":"BAYC1-PASMTP092A9E80667F373BF69AA6AED70@CEZ.ICE","subject":"Re: [PATCH] Colourise git-branch output","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-12-12T03:39:58Z","receivedAt":"2006-12-12T03:39:58Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Mon, 11 Dec 2006, Sean wrote:\n> \n> Technically it is workable.. but why even start down the road of having\n> anything but branch names after a \"branch.\"? There has to be a better\n> spot for this variable, and it makes it more future proof, as you\n> highlighted.\n\nI do agree with Sean, both for the stability reason, but perhaps even more \nbecause I personally think it would just be better to have a separate \n\"[color]\" subsection.\n\nIn fact, I'd almost prefer to see\n\n\t[color]\n\t\tdiff = auto\n\nover\n\n\t[diff]\n\t\tcolor = auto\n\nexactly because once we have different things that take colorization \narguments, it's just nicer to have them all together (and we already have \n\"status\", and now we're getting \"branch\" too.\n\n"},{"id":"298717","messageId":"7vejr5zoqe.fsf@assigned-by-dhcp.cox.net","threadId":"43389","inReplyTo":"Pine.LNX.4.64.0612111936240.6452@woody.osdl.org","subject":"Re: [PATCH] Colourise git-branch output","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-12-12T05:07:21Z","receivedAt":"2006-12-12T05:07:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> On Mon, 11 Dec 2006, Sean wrote:\n>> \n>> Technically it is workable.. but why even start down the road of having\n>> anything but branch names after a \"branch.\"? There has to be a better\n>> spot for this variable, and it makes it more future proof, as you\n>> highlighted.\n>\n> I do agree with Sean, both for the stability reason, but perhaps even more \n> because I personally think it would just be better to have a separate \n> \"[color]\" subsection.\n>\n> In fact, I'd almost prefer to see\n>\n> \t[color]\n> \t\tdiff = auto\n>\n> over\n>\n> \t[diff]\n> \t\tcolor = auto\n>\n> exactly because once we have different things that take colorization \n> arguments, it's just nicer to have them all together (and we already have \n> \"status\", and now we're getting \"branch\" too.\n\nI tend to agree. We probably should start deprecating\ndiff.color and diff.color. and swap them around,\nlike this:\n\n\t[color]\n \tdiff = auto\n branch = auto\n # it begs \"* = auto\" entry perhaps...\n\t[color.diff]\n \told = red\n new = green\n\t[color.branch]\n \tremote = purple\n\n"},{"id":"295042","messageId":"Pine.LNX.4.64.0612112135360.6452@woody.osdl.org","threadId":"43389","inReplyTo":"7vejr5zoqe.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Colourise git-branch output","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-12-12T05:41:57Z","receivedAt":"2006-12-12T05:41:57Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Mon, 11 Dec 2006, Junio C Hamano wrote:\n> \n> \t[color]\n> \tdiff = auto\n> branch = auto\n> # it begs \"* = auto\" entry perhaps...\n> \t[color.diff]\n> \told = red\n> new = green\n> \t[color.branch]\n> \tremote = purple\n\nI wish you'd learn to use the proper syntax ;)\n\nIt would be\n\n\t[color \"diff\"]\n\t\told = red\n\t\tnew = green\n\nand what's nice about it is that you can also do\n\n\t[color \"diff\"]\n\t\tauto\n\t\told = red\n\t\tnew = green\n\nand the config file rules for booleans are such that a config variable \nwithout the \"= val\" part parses the same as \"= true\", so you can now do\n\n\tgit repo-config --bool color.diff.auto\n\nand it will say \"true\".\n\nNow, I just think that would be a nice syntax.\n\nOf course, for legacy reasons, and for people who rather than keeping the \n_color_ information together want to keep the _diff_ information together, \nwe can/probably-should support both color.diff.* and diff.color.* formats.\n\n"},{"id":"295852","messageId":"7vfybly29a.fsf@assigned-by-dhcp.cox.net","threadId":"43389","inReplyTo":"Pine.LNX.4.64.0612112135360.6452@woody.osdl.org","subject":"Re: [PATCH] Colourise git-branch output","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-12-12T07:58:09Z","receivedAt":"2006-12-12T07:58:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> I wish you'd learn to use the proper syntax ;)\n\nOk, ok, I'm an old timer. But [color.diff] syntax is also\naccepted and I have it in my ~/.gitconfig:\n\n\t$ cat ~/.gitconfig\n\t[diff.color]\n \told = red reverse\n\t$ git repo-config --global diff.color.old\n red reverse\n $ exit\n\n> ... what's nice about it is that you can also do\n>\n> \t[color \"diff\"]\n> \t\tauto\n> \t\told = red\n> \t\tnew = green\n>\n> and the config file rules for booleans are such that a config variable \n> without the \"= val\" part parses the same as \"= true\", so you can now do\n>\n> \tgit repo-config --bool color.diff.auto\n>\n> and it will say \"true\".\n\nI find that your version of \"auto\" above to be utterly\nconfusing.\n\nThe existing configuration variable diff.color as an extended\nboolean takes true, false or \"auto\".\n\nI do agree this is a nice way to say that I pretend to be color\nblind to git repo-config:\n\n\t$ git repo-config --global --bool color.diff false\n $ cat ~/.gitconfig\n [color]\n \tdiff = false\n\nand it is also nice to be able to say my preference is \"auto\"\nwith (you can have --bool with this example and still use\n\"auto\", by the way, when setting):\n\n\t$ git repo-config --global color.diff auto\n $ cat ~/.gitconfig\n [color]\n \tdiff = auto\n\nI also agree it is nice that the \"boolean magic\" kicks in when\nwe hand-craft it:\n\n\t$ cat ~/.gitconfig\n [color]\n \tdiff\n\t$ git repo-config --global --bool color.diff\n true\n\nBut your handcrafted\n\n\t[color \"diff\"]\n \tauto\n\nsimply feels a very confusing syntax. How would you even\nexpress my preference is always-color (that is, \"true\")?\n\nIf we are changing color.diff to color.diff.usecolor which is\nbool + auto, then I would understand it.\n\n\t[color \"diff\"]\n \tusecolor\n # usecolor = auto\n # usecolor = no\n\nbut that would not let you query with --bool (\"auto\" would\nbarf).\n\nI guess we should at least extend --bool so that scripts can\neasily query extended bools; if the value looks like a bool (or\nthere is no value), it would return it normalized to \"true\" or\n\"false\" so they do not have to say:\n\n\tcase `repo-config --bool the.variable` in\n true | yes | on | 1)\n \techo ah you mean yes ;;\n\tesac\n\nbut if the value is not either boolean true or false, then\nreturn the string as is (such as \"auto\"). I am inclined to just\nextend --bool itself to do so (which means we would lose the\nerror checking form the script for truly boolean fields) rather\nthan adding a new --bool-or-string option.\n\nIn any case,\n\n\t[color]\n \tdiff\n # diff = auto\n # diff = no\n\nwould work already if we only talk about built-in accesses. We\nwould need to extend repo-config --bool to make it easier for\nscripts to work with this extended bool, but that is a minor\ndetail.\n\nSo it seems to me that the only reason to have\ncolor.diff.usecolor is so that we can have \"diff\" related color\nsettings in one place. Which is fine.\n\nYou could do the same thing with your \"auto\"\n\n\t[color \"diff\"]\n \tauto\n\t\t# auto = always\n # auto = no\n\nbut what that means is that color.diff.auto _is_ the extended\nboolean that tells us to:\n\n\t* use color on terminal when set to true\n * use color unconditionally when set to a non-bool 'always'\n * never use color when set to false\n\nwhich sounds rather, eh, unusual.\n\n\n\t\n"}]}