{"thread":{"id":"18370","subject":"[RFC] Colorization of log --graph","startedAt":"2009-03-18T10:05:12Z","lastAt":"2009-03-31T12:09:57Z","messageCount":24,"participants":["Allan Caffee","Johannes Schindelin","Eric Raible","Santi Béjar","Markus Heidelberg","Nanako Shiraishi","Jeff King","Junio C Hamano","Johannes Sixt"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"108362","messageId":"20090318100512.GA7932@linux.vnet","threadId":"18370","inReplyTo":null,"subject":"[RFC] Colorization of log --graph","fromName":"Allan Caffee","fromEmail":"allan.caffee@gmail.com","sentAt":"2009-03-18T10:05:12Z","receivedAt":"2009-03-18T10:05:12Z","isPatch":false,"sender":{"key":"allan.caffee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/114759?v=4"},"body":"I know that _some_ people arn't particularly fond of colors, but I was\nwondering how difficult it would be to colorize the edges on the\n--graph drawn by the log command? It can be a little tricky trying to\nfollow them with a relatively complex history. I was thinking something\nlike gitk already does. Is anybody else interested in seeing this?\n\n~Allan\n"},{"id":"108367","messageId":"alpine.DEB.1.00.0903181228420.10279@pacific.mpi-cbg.de","threadId":"18370","inReplyTo":"20090318100512.GA7932@linux.vnet","subject":"Re: [RFC] Colorization of log --graph","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-03-18T11:44:10Z","receivedAt":"2009-03-18T11:44:10Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 18 Mar 2009, Allan Caffee wrote:\n\n> I know that _some_ people arn't particularly fond of colors, but I was \n> wondering how difficult it would be to colorize the edges on the --graph \n> drawn by the log command? It can be a little tricky trying to follow \n> them with a relatively complex history. I was thinking something like \n> gitk already does.\n\nThat's a good idea! (And it is mentioned as a TODO in graph.c...)\n\n> Is anybody else interested in seeing this?\n\nCount me in. Are you interested in implementing this?\n\nIf so:\n\n- you need to #include \"color.h\" in graph.c\n\n- you need to insert a color identifier into struct column (there is an \n XXX comment at the correct location)\n\n- you need to find a way to determine colors for the branches\n\n- you need to put the handling into the function \n graph_output_pre_commit_line() in graph.c (and probably \n graph_output_commit_char(), graph_output_post_merge_line(), \n graph_output_collapsing_line(), graph_padding_line(), and\n graph_output_padding_line(), too)\n\n- it would make sense IMHO to introduce a new function that takes a \n pointer to an strbuf, a pointer to a struct column and a char (or maybe \n a string) that adds the appropriately colorized char (or string) to the \n strbuf\n\n- use the global variable diff_use_color to determine if the output should \n be colorized at all\n\n- probably you need to make an array of available colors or some such \n (which might be good to put into color.[ch])\n\nCiao,\nDscho\n"},{"id":"108385","messageId":"loom.20090318T164728-444@post.gmane.org","threadId":"18370","inReplyTo":"20090318100512.GA7932@linux.vnet","subject":"Re: [RFC] Colorization of log --graph","fromName":"Eric Raible","fromEmail":"raible+git@gmail.com","sentAt":"2009-03-18T16:52:42Z","receivedAt":"2009-03-18T16:52:42Z","isPatch":false,"sender":{"key":"raible+git@gmail.com","avatar":null},"body":"Allan Caffee gmail.com> writes:\n\n> \n> I know that _some_ people arn't particularly fond of colors, but I was\n> wondering how difficult it would be to colorize the edges on the\n> --graph drawn by the log command? It can be a little tricky trying to\n> follow them with a relatively complex history. I was thinking something\n> like gitk already does. Is anybody else interested in seeing this?\n> \n> ~Allan\n\nThis may be clueless (I suspect that it is) but I have never understood\nthe meaning of the different line colors in gitk. They seems arbitrary to me.\n\nI get that the current HEAD is represented as a yellow dot, but that's it.\n(As an aside, it might be nice if merges had a different color dot than\nnormal commits).\n\nCan anyone clue me in?\n\n- Eric\n"},{"id":"108386","messageId":"adf1fd3d0903181004k2554ae90uc101aad64947be7@mail.gmail.com","threadId":"18370","inReplyTo":"loom.20090318T164728-444@post.gmane.org","subject":"Re: [RFC] Colorization of log --graph","fromName":"Santi Béjar","fromEmail":"santi@agolina.net","sentAt":"2009-03-18T17:04:09Z","receivedAt":"2009-03-18T17:04:09Z","isPatch":false,"sender":{"key":"santi@agolina.net","avatar":null},"body":"2009/3/18 Eric Raible :\n> This may be clueless (I suspect that it is) but I have never understood\n> the meaning of the different line colors in gitk. They seems arbitrary to me.\n>\n> I get that the current HEAD is represented as a yellow dot, but that's it.\n> (As an aside, it might be nice if merges had a different color dot than\n> normal commits).\n>\n> Can anyone clue me in?\n\nGitk paints lines of development (lineal history without merges nor\nforks) with the same color.\n\nHTH,\nSanti\n"},{"id":"108387","messageId":"279b37b20903181029q7a526168y360874a48783d1dc@mail.gmail.com","threadId":"18370","inReplyTo":"adf1fd3d0903181004k2554ae90uc101aad64947be7@mail.gmail.com","subject":"Re: [RFC] Colorization of log --graph","fromName":"Eric Raible","fromEmail":"raible@gmail.com","sentAt":"2009-03-18T17:29:08Z","receivedAt":"2009-03-18T17:29:08Z","isPatch":false,"sender":{"key":"raible@gmail.com","avatar":null},"body":"On Wed, Mar 18, 2009 at 10:04 AM, Santi Béjar wrote:\n> Gitk paints lines of development (lineal history without merges nor\n> forks) with the same color.\n>\n> HTH,\n> Santi\n\nThanks for the quick reply. I suppose I realized that but it just\ndoesn't seem that profound.\nDon't get me wrong - I like gitk and still prefer it to any of the alternatives.\nBut its of color seems more flashy than useful to me.\n\nPerhaps I'd be happier if the color of the nth parent of a merge\n(selectable, first by default)\nemerged from the merge? I dunno.\n"},{"id":"108549","messageId":"b2e43f8f0903190959if539048r19e972899bd2132d@mail.gmail.com","threadId":"18370","inReplyTo":"alpine.DEB.1.00.0903181228420.10279@pacific.mpi-cbg.de","subject":"Re: [RFC] Colorization of log --graph","fromName":"Allan Caffee","fromEmail":"allan.caffee@gmail.com","sentAt":"2009-03-19T16:59:23Z","receivedAt":"2009-03-19T16:59:23Z","isPatch":false,"sender":{"key":"allan.caffee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/114759?v=4"},"body":"Hello and thanks for the speedy reply!\n\nOn Wed, Mar 18, 2009 at 7:44 AM, Johannes Schindelin\n wrote:\n> Hi,\n>\n> On Wed, 18 Mar 2009, Allan Caffee wrote:\n>\n> > I know that _some_ people arn't particularly fond of colors, but I was\n> > wondering how difficult it would be to colorize the edges on the --graph\n> > drawn by the log command? It can be a little tricky trying to follow\n> > them with a relatively complex history. I was thinking something like\n> > gitk already does.\n>\n> That's a good idea! (And it is mentioned as a TODO in graph.c...)\n\nThat's me, always thinking outside the box. ;-)\n\n> > Is anybody else interested in seeing this?\n>\n> Count me in. Are you interested in implementing this?\n\nI'll give it a go. Been a while since I've done anything of substance\nin pure C so it should be a nice refresher. :)\n\n> If so:\n>\n> - you need to #include \"color.h\" in graph.c\n>\n> - you need to insert a color identifier into struct column (there is an\n> XXX comment at the correct location)\n\nBy color identifier I assume you mean the ANSI escape sequence, right?\nI didn't see a type for representing colors in color.{c,h} other than\nthe int it seems to use internally.\n\n> - you need to find a way to determine colors for the branches\n\nOkay, so if we were to make this similiar to how gitk works it would involve:\nIf the previous commit was a merge:\n\tfor (i = 0; i < graph->num_columns; i++)\n\t\tgraph->columns[i]->color = get_next_column_color();\nelse\n\tget_current_column_color();\n\nI was thinking of storing the current color by adding a\ndefault_column_color attribute to git_graph that serves as an index into\ncolumn_colors. column_colors being the array of available colors.\n\n> - you need to put the handling into the function\n> graph_output_pre_commit_line() in graph.c (and probably\n> graph_output_commit_char(), graph_output_post_merge_line(),\n> graph_output_collapsing_line(), graph_padding_line(), and\n> graph_output_padding_line(), too)\n>\n> - it would make sense IMHO to introduce a new function that takes a\n> pointer to an strbuf, a pointer to a struct column and a char (or maybe\n> a string) that adds the appropriately colorized char (or string) to the\n> strbuf\n\nThat makes sense. Then we can just update the functions you mentioned\nabove to use this.\n\n> - use the global variable diff_use_color to determine if the output should\n> be colorized at all\n\nThe function for adding a column to an strbuf would offer a convenient\nplace to put the condition.\n\n> - probably you need to make an array of available colors or some such\n> (which might be good to put into color.[ch])\n\nThis would be the color_codes array I mentioned but it seems like it\nmight belong in graph.c. There's something similiar in diff.c and it\nseems like this is more related to graphing then to colors in general.\nAlthough I do think it makes sense to #define some of the more common\nANSI codes there so that they don't have to be duplicated. grep shows 6\noccurrences of '\\033[31m', the code for red foreground.\n\nI'll begin working on a patch. Comments/questions?\n\n~Allan\n"},{"id":"108555","messageId":"alpine.DEB.1.00.0903191831590.6357@intel-tinevez-2-302","threadId":"18370","inReplyTo":"b2e43f8f0903190959if539048r19e972899bd2132d@mail.gmail.com","subject":"Re: [RFC] Colorization of log --graph","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-03-19T17:41:24Z","receivedAt":"2009-03-19T17:41:24Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 19 Mar 2009, Allan Caffee wrote:\n\n> Hello and thanks for the speedy reply!\n\nHeh, Git is known for raw speed ;-)\n\n> On Wed, Mar 18, 2009 at 7:44 AM, Johannes Schindelin\n> wrote:\n>\n> > On Wed, 18 Mar 2009, Allan Caffee wrote:\n> >\n> > > Is anybody else interested in seeing this?\n> >\n> > Count me in. Are you interested in implementing this?\n> \n> I'll give it a go. Been a while since I've done anything of substance \n> in pure C so it should be a nice refresher. :)\n\nGreat!\n\n> > If so:\n> >\n> > - you need to #include \"color.h\" in graph.c\n> >\n> > - you need to insert a color identifier into struct column (there is an\n> > XXX comment at the correct location)\n> \n> By color identifier I assume you mean the ANSI escape sequence, right? I \n> didn't see a type for representing colors in color.{c,h} other than the \n> int it seems to use internally.\n\nI'd actually add an enum color_names, or something like that.\n\n> > - you need to find a way to determine colors for the branches\n> \n> Okay, so if we were to make this similiar to how gitk works it would \n> involve: If the previous commit was a merge:\n> \tfor (i = 0; i < graph->num_columns; i++)\n> \t\tgraph->columns[i]->color = get_next_column_color();\n> else\n> \tget_current_column_color();\n> \n> I was thinking of storing the current color by adding a \n> default_column_color attribute to git_graph that serves as an index into \n> column_colors. column_colors being the array of available colors.\n\nYep, I agree. That index could be of type \"enum color_names\" if you \nintroduce the latter...\n\n> > - you need to put the handling into the function \n> > graph_output_pre_commit_line() in graph.c (and probably \n> > graph_output_commit_char(), graph_output_post_merge_line(), \n> > graph_output_collapsing_line(), graph_padding_line(), and \n> > graph_output_padding_line(), too)\n> >\n> > - it would make sense IMHO to introduce a new function that takes a \n> > pointer to an strbuf, a pointer to a struct column and a char (or \n> > maybe a string) that adds the appropriately colorized char (or \n> > string) to the strbuf\n> \n> That makes sense. Then we can just update the functions you mentioned\n> above to use this.\n\nRight.\n\n> > - use the global variable diff_use_color to determine if the output \n> > should be colorized at all\n> \n> The function for adding a column to an strbuf would offer a convenient \n> place to put the condition.\n\nYes!\n\n> > - probably you need to make an array of available colors or some such \n> > (which might be good to put into color.[ch])\n> \n> This would be the color_codes array I mentioned but it seems like it\n> might belong in graph.c. There's something similiar in diff.c and it\n> seems like this is more related to graphing then to colors in general.\n> Although I do think it makes sense to #define some of the more common\n> ANSI codes there so that they don't have to be duplicated. grep shows 6\n> occurrences of '\\033[31m', the code for red foreground.\n\nI'd actually like to see it in color.[ch], so that other code paths can \nuse it, too.\n\nI'd start like this:\n\n\tenum color_name {\n\t\tCOLOR_RESET,\n\t\tCOLOR_RED,\n\t\tCOLOR_GREEN,\n\t\tCOLOR_YELLOW,\n\t\tCOLOR_BLUE,\n\t\tCOLOR_MAGENTA,\n\t\tCOLOR_CYAN,\n\t\tCOLOR_WHITE\n\t};\n\nMaybe the best thing would then be to add a function\n\n\tvoid strbuf_add_color(struct strbuf *buf, enum color_name name) {\n\t\tif (name == COLOR_RESET)\n\t\t\tstrbuf_addf(buf, \"\\033[m\");\n\t\telse\n\t\t\tstrbuf_addf(buf, \"\\033[%dm\", 31 + name - COLOR_RED);\n\t}\n\nCiao,\nDscho\n"},{"id":"108567","messageId":"200903192032.42117.markus.heidelberg@web.de","threadId":"18370","inReplyTo":"279b37b20903181029q7a526168y360874a48783d1dc@mail.gmail.com","subject":"Re: [RFC] Colorization of log --graph","fromName":"Markus Heidelberg","fromEmail":"markus.heidelberg@web.de","sentAt":"2009-03-19T19:32:41Z","receivedAt":"2009-03-19T19:32:41Z","isPatch":false,"sender":{"key":"markus.heidelberg@web.de","avatar":"https://avatars.githubusercontent.com/u/6334512?v=4"},"body":"Eric Raible, 18.03.2009:\n> On Wed, Mar 18, 2009 at 10:04 AM, Santi Béjar wrote:\n> > Gitk paints lines of development (lineal history without merges nor\n> > forks) with the same color.\n> >\n> > HTH,\n> > Santi\n> \n> Thanks for the quick reply. I suppose I realized that but it just\n> doesn't seem that profound.\n> Don't get me wrong - I like gitk and still prefer it to any of the alternatives.\n> But its of color seems more flashy than useful to me.\n\nIf scrolling through a history with many branches (so many parallel\nlines) like git.git, colors help you to follow a particular line.\n\nI don't find it easy to follow a line in a PCB, where you normally don't\nhave colors :)\n\nMarkus\n"},{"id":"108571","messageId":"279b37b20903191252h4c1541a1m8e391e01114d2ce6@mail.gmail.com","threadId":"18370","inReplyTo":"200903192032.42117.markus.heidelberg@web.de","subject":"Re: [RFC] Colorization of log --graph","fromName":"Eric Raible","fromEmail":"raible@gmail.com","sentAt":"2009-03-19T19:52:56Z","receivedAt":"2009-03-19T19:52:56Z","isPatch":false,"sender":{"key":"raible@gmail.com","avatar":null},"body":"2009/3/19 Markus Heidelberg :\n>\n> If scrolling through a history with many branches (so many parallel\n> lines) like git.git, colors help you to follow a particular line.\n>\n> I don't find it easy to follow a line in a PCB, where you normally don't\n> have colors :)\n>\n> Markus\n\nI find it easier to simply click on the line and then click the parent\nin the lower window. Which I'm only mentioning b.c. others might\nnot realize that it's possible.\n\nThe meta question is: anyone know of any gitk/git gui documentation\naside from Junio's excellent \"Fun with msysgit 1.6.1 preview\"\n(http://gitster.livejournal.com/24080.html)?\n\nAnything additional would be useful in my quest to get $dayjob to switch to git.\n\n- Eric\n"},{"id":"108573","messageId":"200903192104.16710.markus.heidelberg@web.de","threadId":"18370","inReplyTo":"279b37b20903191252h4c1541a1m8e391e01114d2ce6@mail.gmail.com","subject":"Re: [RFC] Colorization of log --graph","fromName":"Markus Heidelberg","fromEmail":"markus.heidelberg@web.de","sentAt":"2009-03-19T20:04:16Z","receivedAt":"2009-03-19T20:04:16Z","isPatch":false,"sender":{"key":"markus.heidelberg@web.de","avatar":"https://avatars.githubusercontent.com/u/6334512?v=4"},"body":"Eric Raible, 19.03.2009:\n> 2009/3/19 Markus Heidelberg :\n> >\n> > If scrolling through a history with many branches (so many parallel\n> > lines) like git.git, colors help you to follow a particular line.\n> >\n> > I don't find it easy to follow a line in a PCB, where you normally don't\n> > have colors :)\n> >\n> > Markus\n> \n> I find it easier to simply click on the line and then click the parent\n> in the lower window. Which I'm only mentioning b.c. others might\n> not realize that it's possible.\n\nOh, right. I know about it, but since I don't use gitk often, I didn't\nthink of it.\n\nMarkus\n"},{"id":"108594","messageId":"20090320064813.6117@nanako3.lavabit.com","threadId":"18370","inReplyTo":"alpine.DEB.1.00.0903191831590.6357@intel-tinevez-2-302","subject":"Re: [RFC] Colorization of log --graph","fromName":"Nanako Shiraishi","fromEmail":"nanako3@lavabit.com","sentAt":"2009-03-19T21:48:13Z","receivedAt":"2009-03-19T21:48:13Z","isPatch":false,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"Quoting Johannes Schindelin :\n\n> I'd start like this:\n>\n> \tenum color_name {\n> \t\tCOLOR_RESET,\n> \t\tCOLOR_RED,\n> \t\tCOLOR_GREEN,\n> \t\tCOLOR_YELLOW,\n> \t\tCOLOR_BLUE,\n> \t\tCOLOR_MAGENTA,\n> \t\tCOLOR_CYAN,\n> \t\tCOLOR_WHITE\n> \t};\n\nLooking for \"COLOR_RED\" in the archive gives:\n\n http://article.gmane.org/gmane.comp.version-control.git/109676\n\n-- \nNanako Shiraishi\nhttp://ivory.ap.teacup.com/nanako3/\n"},{"id":"108747","messageId":"b2e43f8f0903201213o396de6c0sb52149ed1d889d1@mail.gmail.com","threadId":"18370","inReplyTo":"20090320064813.6117@nanako3.lavabit.com","subject":"Re: [RFC] Colorization of log --graph","fromName":"Allan Caffee","fromEmail":"allan.caffee@gmail.com","sentAt":"2009-03-20T19:13:53Z","receivedAt":"2009-03-20T19:13:53Z","isPatch":false,"sender":{"key":"allan.caffee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/114759?v=4"},"body":"On Thu, Mar 19, 2009 at 5:48 PM, Nanako Shiraishi wrote:\n> Quoting Johannes Schindelin :\n>\n>> I'd start like this:\n>>\n>>       enum color_name {\n>>               COLOR_RESET,\n>>               COLOR_RED,\n>>               COLOR_GREEN,\n>>               COLOR_YELLOW,\n>>               COLOR_BLUE,\n>>               COLOR_MAGENTA,\n>>               COLOR_CYAN,\n>>               COLOR_WHITE\n>>       };\n>\n> Looking for \"COLOR_RED\" in the archive gives:\n>\n>  http://article.gmane.org/gmane.comp.version-control.git/109676\n>\n\nDuly noted. Perhaps those #defines should be relocated to color.h? If\nwe still wanted to provide a color_name type we could use\nGIT_COLOR_NAME_RESET et al. That would give us something like:\n\n#define GIT_COLOR_NORMAL\t\"\"\n#define GIT_COLOR_RESET\t\t\"\\033[m\"\n#define GIT_COLOR_BOLD\t\t\"\\033[1m\"\n#define GIT_COLOR_RED\t\t\"\\033[31m\"\n#define GIT_COLOR_GREEN\t\t\"\\033[32m\"\n#define GIT_COLOR_YELLOW\t\"\\033[33m\"\n#define GIT_COLOR_BLUE\t\t\"\\033[34m\"\n#define GIT_COLOR_CYAN\t\t\"\\033[36m\"\n#define GIT_COLOR_BG_RED\t\"\\033[41m\"\n\nenum color_name {\n\tGIT_COLOR_NAME_NORMAL\n\tGIT_COLOR_NAME_RESET,\n\tGIT_COLOR_NAME_RED,\n\tGIT_COLOR_NAME_GREEN,\n\tGIT_COLOR_NAME_YELLOW,\n\tGIT_COLOR_NAME_BLUE,\n\tGIT_COLOR_NAME_MAGENTA,\n\tGIT_COLOR_NAME_CYAN,\n\tGIT_COLOR_NAME_WHITE\n\tGIT_COLOR_NAME_BG_RED\n};\n\n/*\n * Map names to ANSI escape sequences. Consider putting this in color.c\n * and providing color_name_get_ansi_code(enum color_name).\n */\nconst char* git_color_codes[] {\n\tGIT_COLOR_RESET,\n\tGIT_COLOR_BOLD,\n\tGIT_COLOR_RED,\n\tGIT_COLOR_GREEN,\n\tGIT_COLOR_YELLOW,\n\tGIT_COLOR_BLUE,\n\tGIT_COLOR_CYAN,\n\tGIT_COLOR_BG_RED,\n};\n\nThat conveniently offers clients access to both the raw escape codes and\na clear type for storing/handling colors.\n\n~Allan\n"},{"id":"108754","messageId":"20090320195806.GC26934@coredump.intra.peff.net","threadId":"18370","inReplyTo":"b2e43f8f0903201213o396de6c0sb52149ed1d889d1@mail.gmail.com","subject":"Re: [RFC] Colorization of log --graph","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-03-20T19:58:06Z","receivedAt":"2009-03-20T19:58:06Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Mar 20, 2009 at 03:13:53PM -0400, Allan Caffee wrote:\n\n> /*\n> * Map names to ANSI escape sequences. Consider putting this in color.c\n> * and providing color_name_get_ansi_code(enum color_name).\n> */\n> const char* git_color_codes[] {\n> \tGIT_COLOR_RESET,\n> \tGIT_COLOR_BOLD,\n> \tGIT_COLOR_RED,\n> \tGIT_COLOR_GREEN,\n> \tGIT_COLOR_YELLOW,\n> \tGIT_COLOR_BLUE,\n> \tGIT_COLOR_CYAN,\n> \tGIT_COLOR_BG_RED,\n> };\n> \n> That conveniently offers clients access to both the raw escape codes and\n> a clear type for storing/handling colors.\n\nI want to point out one thing: an enum or a list like this is actually a\nsubset of the useful color codes that git can represent. Actual\nconfigured colors can have attributes, foreground, and background\ncolors. So they need to be stored in a character array.\n\nAdding an enum for GIT_COLOR_RED and using it throughout the code can be\nhelpful for simple cases, but it doesn't give you an easy way of saying\n\"red background, blue foreground\". Maybe that is enough for git internal\nusage, since we tend not to use backgrounds or attributes for defaults.\nBut maybe it makes more sense to do this as:\n\n const char *ansi_color(enum color fg, enum color bg, enum attribute attr);\n\nand return a pointer to a static array representing the color (and even\ncycle through a list the way sha1_to_hex or git_path does). And you\ncould even use it to simplify and share code with the config color\nparsing in color.c.\n\n-Peff\n"},{"id":"108757","messageId":"7vbprwgdgc.fsf@gitster.siamese.dyndns.org","threadId":"18370","inReplyTo":"b2e43f8f0903201213o396de6c0sb52149ed1d889d1@mail.gmail.com","subject":"Re: [RFC] Colorization of log --graph","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-20T20:13:23Z","receivedAt":"2009-03-20T20:13:23Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Allan Caffee writes:\n\n> On Thu, Mar 19, 2009 at 5:48 PM, Nanako Shiraishi wrote:\n>> Quoting Johannes Schindelin :\n>>\n>>> I'd start like this:\n>>>\n>>>       enum color_name {\n>>>               COLOR_RESET,\n>>>               COLOR_RED,\n>>>               COLOR_GREEN,\n>>>               COLOR_YELLOW,\n>>>               COLOR_BLUE,\n>>>               COLOR_MAGENTA,\n>>>               COLOR_CYAN,\n>>>               COLOR_WHITE\n>>>       };\n>>\n>> Looking for \"COLOR_RED\" in the archive gives:\n>>\n>>  http://article.gmane.org/gmane.comp.version-control.git/109676\n>>\n>\n> Duly noted. Perhaps those #defines should be relocated to color.h?\n\nHeh, I did not even realize the above 109676 was referring to what I wrote\nsometime ago.\n\n> If we still wanted to provide a color_name type we could use\n> GIT_COLOR_NAME_RESET et al. That would give us something like:\n>\n> #define GIT_COLOR_NORMAL\t\"\"\n> #define GIT_COLOR_RESET\t\t\"\\033[m\"\n> #define GIT_COLOR_BOLD\t\t\"\\033[1m\"\n> #define GIT_COLOR_RED\t\t\"\\033[31m\"\n> #define GIT_COLOR_GREEN\t\t\"\\033[32m\"\n> #define GIT_COLOR_YELLOW\t\"\\033[33m\"\n> #define GIT_COLOR_BLUE\t\t\"\\033[34m\"\n> #define GIT_COLOR_CYAN\t\t\"\\033[36m\"\n> #define GIT_COLOR_BG_RED\t\"\\033[41m\"\n>\n> enum color_name {\n> \tGIT_COLOR_NAME_NORMAL\n> \tGIT_COLOR_NAME_RESET,\n> \tGIT_COLOR_NAME_RED,\n> \tGIT_COLOR_NAME_GREEN,\n> \tGIT_COLOR_NAME_YELLOW,\n> \tGIT_COLOR_NAME_BLUE,\n> \tGIT_COLOR_NAME_MAGENTA,\n> \tGIT_COLOR_NAME_CYAN,\n> \tGIT_COLOR_NAME_WHITE\n> \tGIT_COLOR_NAME_BG_RED\n> };\n>\n> /*\n> * Map names to ANSI escape sequences. Consider putting this in color.c\n> * and providing color_name_get_ansi_code(enum color_name).\n> */\n> const char* git_color_codes[] {\n> \tGIT_COLOR_RESET,\n> \tGIT_COLOR_BOLD,\n> \tGIT_COLOR_RED,\n> \tGIT_COLOR_GREEN,\n> \tGIT_COLOR_YELLOW,\n> \tGIT_COLOR_BLUE,\n> \tGIT_COLOR_CYAN,\n> \tGIT_COLOR_BG_RED,\n> };\n>\n> That conveniently offers clients access to both the raw escape codes and\n> a clear type for storing/handling colors.\n\nIs git_color_codes[GIT_COLOR_NAME_FOO] supposed to give you GIT_COLOR_FOO?\n\nAre you consolidating various pieces of physical color definition to one\nplace? That sounds sensible.\n\nThe corrent code does:\n\ndiff.c::\n\tuser says \"meta\" is \"purple\"\n -> parse_diff_color_slot() says \"meta\" is slot 2\n -> git_diff_basic_config() asks color_parse() to place the ANSI\n representation of the \"purple\" in slot 2\n\t-> code uses diff_get_color() to grab \"meta\" color from the slot\n and sends it to the terminal\n\nbuiltin-branch.c duplicates the exact same logic with a separate tables\nand a set of slots. builtin-grep.c cheats and does not give the end user\nany customizability, which needs to be fixed.\n\nThe \"slots\" are defined in terms of what the color is used for, the\nmeaning, e.g. \"a line from the file before the change (DIFF_FILE_OLD)\"; we\ncannot avoid having application specific set of slots, but the parsing\nshould be able to share the code.\n\nOnce the slot number is known, we ask color_parse() to put the final\nphysical string (suitable for the terminal's consumption) to fill the\nslot. But for that, I do not think git_color_codes[] nor GIT_COLOR_FOO\nneed to be exposed to the applications (i.e. \"diff\", \"branch\", \"grep\").\nIt is an implementation detail that color_parse() always uses ANSI escape\nsequences right now, but we could encapsulate that in color.c and later\nperhaps start looking up from the terminfo database, for example.\n\nBut that leaves the question of initialization. I think it would give a\nbetter abstraction if we changed the type of values stored in a\ncolor-table like diff.c::diff_colors[] from physical string sent to the\nterminal to a color name (your enum color_name). Then the application\ncode can initialize their own color-table for each application-specific\nslots with GIT_COLOR_NAME_RED, let the configuration mechanism to\ncustomize it for the user. The codepath that currently assume the color\ntable contains strings that can be sent to the terminal need to be\nmodified to ask color_code_to_terminal_string(GIT_COLOR_NAME_YELLOW) or\nsomething. Which means:\n\n(1) Physical color representation should be known only to color.c. I.e.\n\n\t#define GIT_COLOR_BOLD \"\\033[1m\"\n\n does not belong to color.h (public header for application consumption)\n nor diff.c (application);\n\n(2) Logical color name and the ways to convert it for terminal consumption\n belongs to color.h. I.e.\n\n\tenum color_name {\n \tGIT_COLOR_NAME_YELLOW,\n ...\n\t}\n\n should go to color.h;\n\n color_fprintf() should be changed to take \"enum color_name color\"\n instead of \"const char *color\";\n\n We would need strbuf variant for callers that prepare the string in\n core before giving it to fprintf().\n\n(3) \"static const char *git_color_codes[]\" would be an implementation\n detail of the current \"ANSI-only\" one, hidden inside color.c, for\n color_fprintf() and its strbuf cousin to look at.\n"},{"id":"109902","messageId":"20090330141322.GA6221@linux.vnet","threadId":"18370","inReplyTo":"20090321175726.GA6677@linux.vnet","subject":"[RFC/PATCH] graph API: Added logic for colored edges.","fromName":"Allan Caffee","fromEmail":"allan.caffee@gmail.com","sentAt":"2009-03-30T14:13:23Z","receivedAt":"2009-03-30T14:13:23Z","isPatch":true,"sender":{"key":"allan.caffee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/114759?v=4"},"body":"Modified the graph drawing logic to colorize edges based on parent-child\nrelationships similiarly to gitk.\n\nSigned-off-by: Allan Caffee \n---\n\nI havn't gotten the chance to do any of the color clean up that's been\ndiscussed on this thread. I'll try to throw something together in a seperate\npatch series.\n\nAlso this patch isn't respecting the --no-color option which I imagine means\nthat diff_use_color_default isn't the right variable to be checking. Johannes\nmentioned using diff_use_color but the only instance I see is a parameter to\ndiff_get_color. What am I missing?\n\n~Allan\n\n color.h | 1 +\n graph.c | 167 ++++++++++++++++++++++++++++++++++++++++++++++++++++++---------\n 2 files changed, 144 insertions(+), 24 deletions(-)\n\ndiff --git a/color.h b/color.h\nindex 6846be1..18abeb7 100644\n--- a/color.h\n+++ b/color.h\n@@ -11,6 +11,7 @@\n #define GIT_COLOR_GREEN\t\t\"\\033[32m\"\n #define GIT_COLOR_YELLOW\t\"\\033[33m\"\n #define GIT_COLOR_BLUE\t\t\"\\033[34m\"\n+#define GIT_COLOR_MAGENTA\t\"\\033[35m\"\n #define GIT_COLOR_CYAN\t\t\"\\033[36m\"\n #define GIT_COLOR_BG_RED\t\"\\033[41m\"\n \ndiff --git a/graph.c b/graph.c\nindex 162a516..2929c8b 100644\n--- a/graph.c\n+++ b/graph.c\n@@ -1,9 +1,11 @@\n #include \"cache.h\"\n #include \"commit.h\"\n+#include \"color.h\"\n #include \"graph.h\"\n #include \"diff.h\"\n #include \"revision.h\"\n \n+extern int diff_use_color_default;\n /* Internal API */\n \n /*\n@@ -72,11 +74,22 @@ struct column {\n \t */\n \tstruct commit *commit;\n \t/*\n-\t * XXX: Once we add support for colors, struct column could also\n-\t * contain the color of its branch line.\n+\t * The color to (optionally) print this column in.\n \t */\n+\tchar *color;\n };\n \n+static void strbuf_write_column(struct strbuf *sb, const struct column *c,\n+\t\tconst char *s);\n+\n+static char* get_current_column_color (const struct git_graph* graph);\n+\n+/*\n+ * Update the default column color and return the new value.\n+ */\n+static char* get_next_column_color(struct git_graph* graph);\n+\n+\n enum graph_state {\n \tGRAPH_PADDING,\n \tGRAPH_SKIP,\n@@ -86,6 +99,24 @@ enum graph_state {\n \tGRAPH_COLLAPSING\n };\n \n+/*\n+ * The list of available column colors.\n+ */\n+static char column_colors[][COLOR_MAXLEN] = {\n+\tGIT_COLOR_RED,\n+\tGIT_COLOR_GREEN,\n+\tGIT_COLOR_YELLOW,\n+\tGIT_COLOR_BLUE,\n+\tGIT_COLOR_MAGENTA,\n+\tGIT_COLOR_CYAN,\n+\tGIT_COLOR_BOLD GIT_COLOR_RED,\n+\tGIT_COLOR_BOLD GIT_COLOR_GREEN,\n+\tGIT_COLOR_BOLD GIT_COLOR_YELLOW,\n+\tGIT_COLOR_BOLD GIT_COLOR_BLUE,\n+\tGIT_COLOR_BOLD GIT_COLOR_MAGENTA,\n+\tGIT_COLOR_BOLD GIT_COLOR_CYAN,\n+};\n+\n struct git_graph {\n \t/*\n \t * The commit currently being processed\n@@ -185,6 +216,11 @@ struct git_graph {\n \t * temporary array each time we have to output a collapsing line.\n \t */\n \tint *new_mapping;\n+\t/*\n+\t * The current default column color being used. This is\n+\t * stored as an index into the array column_colors.\n+\t */\n+\tshort default_column_color;\n };\n \n struct git_graph *graph_init(struct rev_info *opt)\n@@ -201,6 +237,7 @@ struct git_graph *graph_init(struct rev_info *opt)\n \tgraph->num_columns = 0;\n \tgraph->num_new_columns = 0;\n \tgraph->mapping_size = 0;\n+\tgraph->default_column_color = 0;\n \n \t/*\n \t * Allocate a reasonably large default number of columns\n@@ -317,6 +354,14 @@ static void graph_insert_into_new_columns(struct git_graph *graph,\n \t\t\t\t\t int *mapping_index)\n {\n \tint i;\n+\tchar *color = get_current_column_color(graph);\n+\n+\tfor (i = 0; i < graph->num_columns; i++) {\n+\t\tif (graph->columns[i].commit == commit) {\n+\t\t\tcolor = graph->columns[i].color;\n+\t\t\tbreak;\n+\t\t}\n+\t}\n \n \t/*\n \t * If the commit is already in the new_columns list, we don't need to\n@@ -334,6 +379,8 @@ static void graph_insert_into_new_columns(struct git_graph *graph,\n \t * This commit isn't already in new_columns. Add it.\n \t */\n \tgraph->new_columns[graph->num_new_columns].commit = commit;\n+/* fprintf(stderr,\"adding the %scommit%s\\n\", color, GIT_COLOR_RESET); */\n+\tgraph->new_columns[graph->num_new_columns].color = color;\n \tgraph->mapping[*mapping_index] = graph->num_new_columns;\n \t*mapping_index += 2;\n \tgraph->num_new_columns++;\n@@ -445,6 +492,12 @@ static void graph_update_columns(struct git_graph *graph)\n \t\t\tfor (parent = first_interesting_parent(graph);\n \t\t\t parent;\n \t\t\t parent = next_interesting_parent(graph, parent)) {\n+\t\t\t\t/*\n+\t\t\t\t * If this is a merge increment the current\n+\t\t\t\t * color.\n+\t\t\t\t */\n+\t\t\t\tif (graph->num_parents > 1)\n+\t\t\t\t\tget_next_column_color(graph);\n \t\t\t\tgraph_insert_into_new_columns(graph,\n \t\t\t\t\t\t\t parent->item,\n \t\t\t\t\t\t\t &mapping_idx);\n@@ -596,7 +649,7 @@ static void graph_output_padding_line(struct git_graph *graph,\n \t * Output a padding row, that leaves all branch lines unchanged\n \t */\n \tfor (i = 0; i < graph->num_new_columns; i++) {\n-\t\tstrbuf_addstr(sb, \"| \");\n+\t\tstrbuf_write_column(sb, &graph->new_columns[i], \"| \");\n \t}\n \n \tgraph_pad_horizontally(graph, sb);\n@@ -649,7 +702,10 @@ static void graph_output_pre_commit_line(struct git_graph *graph,\n \t\tstruct column *col = &graph->columns[i];\n \t\tif (col->commit == graph->commit) {\n \t\t\tseen_this = 1;\n-\t\t\tstrbuf_addf(sb, \"| %*s\", graph->expansion_row, \"\");\n+\t\t\tstruct strbuf tmp = STRBUF_INIT;\n+\t\t\tstrbuf_addf(&tmp, \"| %*s\", graph->expansion_row, \"\");\n+\t\t\tstrbuf_write_column(sb, col, tmp.buf);\n+\t\t\tstrbuf_release(&tmp);\n \t\t} else if (seen_this && (graph->expansion_row == 0)) {\n \t\t\t/*\n \t\t\t * This is the first line of the pre-commit output.\n@@ -662,13 +718,13 @@ static void graph_output_pre_commit_line(struct git_graph *graph,\n \t\t\t */\n \t\t\tif (graph->prev_state == GRAPH_POST_MERGE &&\n \t\t\t graph->prev_commit_index < i)\n-\t\t\t\tstrbuf_addstr(sb, \"\\\\ \");\n+\t\t\t\tstrbuf_write_column(sb, col, \"\\\\ \");\n \t\t\telse\n-\t\t\t\tstrbuf_addstr(sb, \"| \");\n+\t\t\t\tstrbuf_write_column(sb, col, \"| \");\n \t\t} else if (seen_this && (graph->expansion_row > 0)) {\n-\t\t\tstrbuf_addstr(sb, \"\\\\ \");\n+\t\t\tstrbuf_write_column(sb, col, \"\\\\ \");\n \t\t} else {\n-\t\t\tstrbuf_addstr(sb, \"| \");\n+\t\t\tstrbuf_write_column(sb, col, \"| \");\n \t\t}\n \t}\n \n@@ -728,6 +784,7 @@ static void graph_output_commit_line(struct git_graph *graph, struct strbuf *sb)\n \t */\n \tseen_this = 0;\n \tfor (i = 0; i <= graph->num_columns; i++) {\n+\t\tstruct column *col = &graph->columns[i];\n \t\tstruct commit *col_commit;\n \t\tif (i == graph->num_columns) {\n \t\t\tif (seen_this)\n@@ -751,7 +808,7 @@ static void graph_output_commit_line(struct git_graph *graph, struct strbuf *sb)\n \t\t\t\tstrbuf_addstr(sb, \". \");\n \t\t\t}\n \t\t} else if (seen_this && (graph->num_parents > 2)) {\n-\t\t\tstrbuf_addstr(sb, \"\\\\ \");\n+\t\t\tstrbuf_write_column(sb, col, \"\\\\ \");\n \t\t} else if (seen_this && (graph->num_parents == 2)) {\n \t\t\t/*\n \t\t\t * This is a 2-way merge commit.\n@@ -768,11 +825,11 @@ static void graph_output_commit_line(struct git_graph *graph, struct strbuf *sb)\n \t\t\t */\n \t\t\tif (graph->prev_state == GRAPH_POST_MERGE &&\n \t\t\t graph->prev_commit_index < i)\n-\t\t\t\tstrbuf_addstr(sb, \"\\\\ \");\n+\t\t\t\tstrbuf_write_column(sb, col, \"\\\\ \");\n \t\t\telse\n-\t\t\t\tstrbuf_addstr(sb, \"| \");\n+\t\t\t\tstrbuf_write_column(sb, col, \"| \");\n \t\t} else {\n-\t\t\tstrbuf_addstr(sb, \"| \");\n+\t\t\tstrbuf_write_column(sb, col, \"| \");\n \t\t}\n \t}\n \n@@ -789,6 +846,17 @@ static void graph_output_commit_line(struct git_graph *graph, struct strbuf *sb)\n \t\tgraph_update_state(graph, GRAPH_COLLAPSING);\n }\n \n+inline struct column* find_new_column_by_commit(struct git_graph *graph,\n+\t\t\t\t\t\tstruct commit *commit)\n+{\n+\tint i;\n+\tfor (i = 0; i < graph->num_new_columns; i++) {\n+\t\tif (graph->new_columns[i].commit == commit)\n+\t\t\treturn &graph->new_columns[i];\n+\t}\n+\treturn 0;\n+}\n+\n static void graph_output_post_merge_line(struct git_graph *graph, struct strbuf *sb)\n {\n \tint seen_this = 0;\n@@ -798,24 +866,43 @@ static void graph_output_post_merge_line(struct git_graph *graph, struct strbuf\n \t * Output the post-merge row\n \t */\n \tfor (i = 0; i <= graph->num_columns; i++) {\n+\t\tstruct column *col = &graph->columns[i];\n \t\tstruct commit *col_commit;\n \t\tif (i == graph->num_columns) {\n \t\t\tif (seen_this)\n \t\t\t\tbreak;\n \t\t\tcol_commit = graph->commit;\n \t\t} else {\n-\t\t\tcol_commit = graph->columns[i].commit;\n+\t\t\tcol_commit = col->commit;\n \t\t}\n \n \t\tif (col_commit == graph->commit) {\n+\t\t\t/*\n+\t\t\t * Since the current commit is a merge find\n+\t\t\t * the columns for the parent commits in\n+\t\t\t * new_columns and use those to format the\n+\t\t\t * edges.\n+\t\t\t */\n+\t\t\tstruct commit_list *parents = NULL;\n+\t\t\tstruct column *par_column;\n \t\t\tseen_this = 1;\n-\t\t\tstrbuf_addch(sb, '|');\n-\t\t\tfor (j = 0; j < graph->num_parents - 1; j++)\n-\t\t\t\tstrbuf_addstr(sb, \"\\\\ \");\n+\t\t\tparents = first_interesting_parent(graph);\n+\t\t\tassert(parents);\n+\t\t\tpar_column = find_new_column_by_commit(graph,parents->item);\n+\t\t\tassert(par_column);\n+\n+\t\t\tstrbuf_write_column(sb, par_column, \"|\");\n+\t\t\tfor (j = 0; j < graph->num_parents - 1; j++) {\n+\t\t\t\tparents = next_interesting_parent(graph, parents);\n+\t\t\t\tassert(parents);\n+\t\t\t\tpar_column = find_new_column_by_commit(graph,parents->item);\n+\t\t\t\tassert(par_column);\n+\t\t\t\tstrbuf_write_column(sb, par_column, \"\\\\ \");\n+\t\t\t}\n \t\t} else if (seen_this) {\n-\t\t\tstrbuf_addstr(sb, \"\\\\ \");\n+\t\t\tstrbuf_write_column(sb, col, \"\\\\ \");\n \t\t} else {\n-\t\t\tstrbuf_addstr(sb, \"| \");\n+\t\t\tstrbuf_write_column(sb, col, \"| \");\n \t\t}\n \t}\n \n@@ -834,6 +921,8 @@ static void graph_output_collapsing_line(struct git_graph *graph, struct strbuf\n {\n \tint i;\n \tint *tmp_mapping;\n+\tstatic int collapsing_columns[255];\n+\tint collapsing_seen_so_far = 0;\n \n \t/*\n \t * Clear out the new_mapping array\n@@ -912,9 +1001,11 @@ static void graph_output_collapsing_line(struct git_graph *graph, struct strbuf\n \t\tif (target < 0)\n \t\t\tstrbuf_addch(sb, ' ');\n \t\telse if (target * 2 == i)\n-\t\t\tstrbuf_addch(sb, '|');\n-\t\telse\n-\t\t\tstrbuf_addch(sb, '/');\n+\t\t\tstrbuf_write_column(sb, &graph->new_columns[target], \"|\");\n+\t\telse {\n+\t\t\tstrbuf_write_column(sb, &graph->new_columns[target], \"/\");\n+\n+\t\t}\n \t}\n \n \tgraph_pad_horizontally(graph, sb);\n@@ -979,9 +1070,10 @@ static void graph_padding_line(struct git_graph *graph, struct strbuf *sb)\n \t * children that we have already processed.)\n \t */\n \tfor (i = 0; i < graph->num_columns; i++) {\n-\t\tstruct commit *col_commit = graph->columns[i].commit;\n+\t\tstruct column *col = &graph->columns[i];\n+\t\tstruct commit *col_commit = col->commit;\n \t\tif (col_commit == graph->commit) {\n-\t\t\tstrbuf_addch(sb, '|');\n+\t\t\tstrbuf_write_column(sb, col, \"|\");\n \n \t\t\tif (graph->num_parents < 3)\n \t\t\t\tstrbuf_addch(sb, ' ');\n@@ -991,7 +1083,7 @@ static void graph_padding_line(struct git_graph *graph, struct strbuf *sb)\n \t\t\t\t\tstrbuf_addch(sb, ' ');\n \t\t\t}\n \t\t} else {\n-\t\t\tstrbuf_addstr(sb, \"| \");\n+\t\t\tstrbuf_write_column(sb, col, \"| \");\n \t\t}\n \t}\n \n@@ -1154,3 +1246,30 @@ void graph_show_commit_msg(struct git_graph *graph,\n \t\t\tputchar('\\n');\n \t}\n }\n+\n+static void strbuf_write_column(struct strbuf *sb, const struct column *c,\n+\t\tconst char *s)\n+{\n+\t/*\n+\t * TODO: I get the creeping suspicion that this isn't the\n+\t * right flag to be checking since --no-color doesn't turn\n+\t * this off.\n+\t */\n+\tif (diff_use_color_default)\n+\t\tstrbuf_addstr(sb, c->color);\n+\tstrbuf_addstr(sb, s);\n+\tif (diff_use_color_default)\n+\t\tstrbuf_addstr(sb, GIT_COLOR_RESET);\n+}\n+\n+static char* get_current_column_color (const struct git_graph* graph)\n+{\n+\treturn column_colors[graph->default_column_color];\n+}\n+\n+static char* get_next_column_color(struct git_graph* graph)\n+{\n+\tgraph->default_column_color = (graph->default_column_color + 1) %\n+\t\tARRAY_SIZE(column_colors);\n+\treturn (get_current_column_color(graph));\n+}\n-- \n1.5.4.3\n"},{"id":"109916","messageId":"7ee8d1c4ca806ce964356a1fe78efac19d56c29b.1238428115u.git.johannes.schindelin@gmx.de","threadId":"18370","inReplyTo":"cover.1238428115u.git.johannes.schindelin@gmx.de","subject":"[PATCH 1/2] graph.c: avoid compile warnings","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-03-30T15:49:36Z","receivedAt":"2009-03-30T15:49:36Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Signed-off-by: Johannes Schindelin \n---\n\n\tI'd actually like to see this and the next patch squashed in.\n\n graph.c | 4 +---\n 1 files changed, 1 insertions(+), 3 deletions(-)\n\ndiff --git a/graph.c b/graph.c\nindex 2929c8b..5e2f224 100644\n--- a/graph.c\n+++ b/graph.c\n@@ -701,8 +701,8 @@ static void graph_output_pre_commit_line(struct git_graph *graph,\n \tfor (i = 0; i < graph->num_columns; i++) {\n \t\tstruct column *col = &graph->columns[i];\n \t\tif (col->commit == graph->commit) {\n-\t\t\tseen_this = 1;\n \t\t\tstruct strbuf tmp = STRBUF_INIT;\n+\t\t\tseen_this = 1;\n \t\t\tstrbuf_addf(&tmp, \"| %*s\", graph->expansion_row, \"\");\n \t\t\tstrbuf_write_column(sb, col, tmp.buf);\n \t\t\tstrbuf_release(&tmp);\n@@ -921,8 +921,6 @@ static void graph_output_collapsing_line(struct git_graph *graph, struct strbuf\n {\n \tint i;\n \tint *tmp_mapping;\n-\tstatic int collapsing_columns[255];\n-\tint collapsing_seen_so_far = 0;\n \n \t/*\n \t * Clear out the new_mapping array\n-- \n1.6.2.1.493.g67cf3\n"},{"id":"109917","messageId":"0d85995e0a6b497080e7b02d6468e14a210dbbae.1238428115u.git.johannes.schindelin@gmx.de","threadId":"18370","inReplyTo":"cover.1238428115u.git.johannes.schindelin@gmx.de","subject":"[PATCH 2/2] --graph: respect --no-color","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-03-30T15:49:50Z","receivedAt":"2009-03-30T15:49:50Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Signed-off-by: Johannes Schindelin \n---\n graph.c | 7 ++++---\n 1 files changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/graph.c b/graph.c\nindex 5e2f224..6a8622c 100644\n--- a/graph.c\n+++ b/graph.c\n@@ -5,7 +5,6 @@\n #include \"diff.h\"\n #include \"revision.h\"\n \n-extern int diff_use_color_default;\n /* Internal API */\n \n /*\n@@ -1253,15 +1252,17 @@ static void strbuf_write_column(struct strbuf *sb, const struct column *c,\n \t * right flag to be checking since --no-color doesn't turn\n \t * this off.\n \t */\n-\tif (diff_use_color_default)\n+\tif (c->color)\n \t\tstrbuf_addstr(sb, c->color);\n \tstrbuf_addstr(sb, s);\n-\tif (diff_use_color_default)\n+\tif (c->color)\n \t\tstrbuf_addstr(sb, GIT_COLOR_RESET);\n }\n \n static char* get_current_column_color (const struct git_graph* graph)\n {\n+\tif (!DIFF_OPT_TST(&graph->revs->diffopt, COLOR_DIFF))\n+\t\treturn NULL;\n \treturn column_colors[graph->default_column_color];\n }\n \n-- \n1.6.2.1.493.g67cf3\n"},{"id":"109918","messageId":"7vd4bzf1e5.fsf@gitster.siamese.dyndns.org","threadId":"18370","inReplyTo":"7ee8d1c4ca806ce964356a1fe78efac19d56c29b.1238428115u.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH 1/2] graph.c: avoid compile warnings","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-30T15:58:42Z","receivedAt":"2009-03-30T15:58:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Signed-off-by: Johannes Schindelin \n> ---\n>\n> \tI'd actually like to see this and the next patch squashed in.\n>\n> graph.c | 4 +---\n> 1 files changed, 1 insertions(+), 3 deletions(-)\n>\n> diff --git a/graph.c b/graph.c\n> index 2929c8b..5e2f224 100644\n> --- a/graph.c\n> +++ b/graph.c\n> @@ -701,8 +701,8 @@ static void graph_output_pre_commit_line(struct git_graph *graph,\n> \tfor (i = 0; i < graph->num_columns; i++) {\n> \t\tstruct column *col = &graph->columns[i];\n> \t\tif (col->commit == graph->commit) {\n> -\t\t\tseen_this = 1;\n> \t\t\tstruct strbuf tmp = STRBUF_INIT;\n> +\t\t\tseen_this = 1;\n\nWhich codebase are you working on top of?\n\n> \t\t\tstrbuf_addf(&tmp, \"| %*s\", graph->expansion_row, \"\");\n> \t\t\tstrbuf_write_column(sb, col, tmp.buf);\n> \t\t\tstrbuf_release(&tmp);\n> @@ -921,8 +921,6 @@ static void graph_output_collapsing_line(struct git_graph *graph, struct strbuf\n> {\n> \tint i;\n> \tint *tmp_mapping;\n> -\tstatic int collapsing_columns[255];\n> -\tint collapsing_seen_so_far = 0;\n> \n> \t/*\n> \t * Clear out the new_mapping array\n> -- \n> 1.6.2.1.493.g67cf3\n"},{"id":"109921","messageId":"alpine.DEB.1.00.0903301749590.7534@intel-tinevez-2-302","threadId":"18370","inReplyTo":"20090330141322.GA6221@linux.vnet","subject":"Re: [RFC/PATCH] graph API: Added logic for colored edges.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-03-30T16:04:27Z","receivedAt":"2009-03-30T16:04:27Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 30 Mar 2009, Allan Caffee wrote:\n\n> Modified the graph drawing logic to colorize edges based on parent-child \n> relationships similiarly to gitk.\n> \n> Signed-off-by: Allan Caffee \n\nNice!\n\n> I havn't gotten the chance to do any of the color clean up that's been \n> discussed on this thread. I'll try to throw something together in a \n> seperate patch series.\n> \n> Also this patch isn't respecting the --no-color option which I imagine \n> means that diff_use_color_default isn't the right variable to be \n> checking. Johannes mentioned using diff_use_color but the only instance \n> I see is a parameter to diff_get_color. What am I missing?\n\nThe patch I sent you should work...\n\n> diff --git a/graph.c b/graph.c\n> index 162a516..2929c8b 100644\n> --- a/graph.c\n> +++ b/graph.c\n> @@ -72,11 +74,22 @@ struct column {\n> \t */\n> \tstruct commit *commit;\n> \t/*\n> -\t * XXX: Once we add support for colors, struct column could also\n> -\t * contain the color of its branch line.\n> +\t * The color to (optionally) print this column in.\n> \t */\n> +\tchar *color;\n> };\n> \n> +static void strbuf_write_column(struct strbuf *sb, const struct column *c,\n> +\t\tconst char *s);\n> +\n> +static char* get_current_column_color (const struct git_graph* graph);\n> +\n> +/*\n> + * Update the default column color and return the new value.\n> + */\n> +static char* get_next_column_color(struct git_graph* graph);\n> +\n> +\n\nPlease just insert the definitions here, instead of using a forward \ndeclaration.\n\n> @@ -86,6 +99,24 @@ enum graph_state {\n> \tGRAPH_COLLAPSING\n> };\n> \n> +/*\n> + * The list of available column colors.\n> + */\n> +static char column_colors[][COLOR_MAXLEN] = {\n> +\tGIT_COLOR_RED,\n> +\tGIT_COLOR_GREEN,\n> +\tGIT_COLOR_YELLOW,\n> +\tGIT_COLOR_BLUE,\n> +\tGIT_COLOR_MAGENTA,\n> +\tGIT_COLOR_CYAN,\n> +\tGIT_COLOR_BOLD GIT_COLOR_RED,\n> +\tGIT_COLOR_BOLD GIT_COLOR_GREEN,\n> +\tGIT_COLOR_BOLD GIT_COLOR_YELLOW,\n> +\tGIT_COLOR_BOLD GIT_COLOR_BLUE,\n> +\tGIT_COLOR_BOLD GIT_COLOR_MAGENTA,\n> +\tGIT_COLOR_BOLD GIT_COLOR_CYAN,\n> +};\n> +\n> struct git_graph {\n> \t/*\n> \t * The commit currently being processed\n\nI imagine that this is a good start. Whether to make a patch that moves \nthis into color.[ch] before or after this patch is up to Junio, I guess \n(even if I would prefer it to be done before, so that it gets done).\n\n> @@ -317,6 +354,14 @@ static void graph_insert_into_new_columns(struct git_graph *graph,\n> \t\t\t\t\t int *mapping_index)\n> {\n> \tint i;\n> +\tchar *color = get_current_column_color(graph);\n> +\n> +\tfor (i = 0; i < graph->num_columns; i++) {\n> +\t\tif (graph->columns[i].commit == commit) {\n> +\t\t\tcolor = graph->columns[i].color;\n> +\t\t\tbreak;\n> +\t\t}\n> +\t}\n\nI imagine that this would be better done using a struct decorate mapping \ncommits to the color strings.\n\nAlso, I'd only call get_current_column_color() if there was no color \nassigned to the commit (instead of calling it all the time).\n\nIt might not be a performance bottleneck here, but I guess it is better \nnot to get used to that pattern anyway.\n\n> @@ -334,6 +379,8 @@ static void graph_insert_into_new_columns(struct git_graph *graph,\n> \t * This commit isn't already in new_columns. Add it.\n> \t */\n> \tgraph->new_columns[graph->num_new_columns].commit = commit;\n> +/* fprintf(stderr,\"adding the %scommit%s\\n\", color, GIT_COLOR_RESET); */\n\nPlease remove this line.\n\n> @@ -649,7 +702,10 @@ static void graph_output_pre_commit_line(struct git_graph *graph,\n> \t\tstruct column *col = &graph->columns[i];\n> \t\tif (col->commit == graph->commit) {\n> \t\t\tseen_this = 1;\n> -\t\t\tstrbuf_addf(sb, \"| %*s\", graph->expansion_row, \"\");\n> +\t\t\tstruct strbuf tmp = STRBUF_INIT;\n> +\t\t\tstrbuf_addf(&tmp, \"| %*s\", graph->expansion_row, \"\");\n> +\t\t\tstrbuf_write_column(sb, col, tmp.buf);\n> +\t\t\tstrbuf_release(&tmp);\n\nMaybe it would be better to add functions\n\nconst char *column_color(struct column *c)\n{\n\treturn c->color ? c->color : \"\";\n}\n\nconst char *column_color_reset(struct column *c)\n{\n\treturn c->color ? GIT_COLOR_RESET : \"\";\n}\n\n?\n\nSorry, I have to stop the review here, ran out of time...\n\nIf nobody beats me to it, I will continue here later.\n\nThanks!\nDscho\n"},{"id":"109923","messageId":"7v1vsff0n9.fsf@gitster.siamese.dyndns.org","threadId":"18370","inReplyTo":"7vd4bzf1e5.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 1/2] graph.c: avoid compile warnings","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-30T16:14:50Z","receivedAt":"2009-03-30T16:14:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n>> diff --git a/graph.c b/graph.c\n>> index 2929c8b..5e2f224 100644\n>> --- a/graph.c\n>> +++ b/graph.c\n>> @@ -701,8 +701,8 @@ static void graph_output_pre_commit_line(struct git_graph *graph,\n>> \tfor (i = 0; i < graph->num_columns; i++) {\n>> \t\tstruct column *col = &graph->columns[i];\n>> \t\tif (col->commit == graph->commit) {\n>> -\t\t\tseen_this = 1;\n>> \t\t\tstruct strbuf tmp = STRBUF_INIT;\n>> +\t\t\tseen_this = 1;\n>\n> Which codebase are you working on top of?\n\nNevermind. I didn't realize it was \"here are to help whipping your series\ninto shape\" meant for Allan.\n"},{"id":"109995","messageId":"alpine.DEB.1.00.0903311210000.10279@pacific.mpi-cbg.de","threadId":"18370","inReplyTo":"20090330141322.GA6221@linux.vnet","subject":"Re: [RFC/PATCH] graph API: Added logic for colored edges.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-03-31T10:13:20Z","receivedAt":"2009-03-31T10:13:20Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 30 Mar 2009, Allan Caffee wrote:\n\n> +static void strbuf_write_column(struct strbuf *sb, const struct column *c,\n> +\t\tconst char *s)\n> +{\n> +\t/*\n> +\t * TODO: I get the creeping suspicion that this isn't the\n> +\t * right flag to be checking since --no-color doesn't turn\n> +\t * this off.\n> +\t */\n\nHeh, of course I forgot to remove this with my to-be-squashed-in patch...\n\n> +\tif (diff_use_color_default)\n> +\t\tstrbuf_addstr(sb, c->color);\n> +\tstrbuf_addstr(sb, s);\n> +\tif (diff_use_color_default)\n> +\t\tstrbuf_addstr(sb, GIT_COLOR_RESET);\n> +}\n\nHow about this function instead?\n\nstatic void strbuf_add_column(struct strbuf *sb,\n\tconst struct column *column, const char *fmt, ...)\n{\n va_list ap;\n\n va_start(ap, fmt);\n\tif (column->color)\n\t\tstrbuf_addstr(sb, column->color);\n strbuf_vaddf(sb, fmt, ap);\n\tif (column->color)\n\t\tstrbuf_addstr(sb, GIT_COLOR_RESET);\n va_end(ap);\n}\n\nHmm?\n\nCiao,\nDscho\n"},{"id":"109996","messageId":"alpine.DEB.1.00.0903311213260.10279@pacific.mpi-cbg.de","threadId":"18370","inReplyTo":"20090330141322.GA6221@linux.vnet","subject":"Re: [RFC/PATCH] graph API: Added logic for colored edges.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-03-31T10:21:22Z","receivedAt":"2009-03-31T10:21:22Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 30 Mar 2009, Allan Caffee wrote:\n\n> @@ -445,6 +492,12 @@ static void graph_update_columns(struct git_graph *graph)\n> \t\t\tfor (parent = first_interesting_parent(graph);\n> \t\t\t parent;\n> \t\t\t parent = next_interesting_parent(graph, parent)) {\n> +\t\t\t\t/*\n> +\t\t\t\t * If this is a merge increment the current\n> +\t\t\t\t * color.\n> +\t\t\t\t */\n> +\t\t\t\tif (graph->num_parents > 1)\n> +\t\t\t\t\tget_next_column_color(graph);\n> \t\t\t\tgraph_insert_into_new_columns(graph,\n> \t\t\t\t\t\t\t parent->item,\n> \t\t\t\t\t\t\t &mapping_idx);\n\nHmm. I would have expected the color to be an argument to \ngraph_insert_into_new_columns()...\n\nOh, and please forget about my stupid babbling about using struct \ndecoration for colors: the column already knows commit and color, so if \nyou need the color in a functino in addition to the commit, you should \npass either the column struct instead, or the commit and the color as \nindividual parameters.\n\nThis concludes my review ;-)\n\nThanks,\nDscho\n"},{"id":"109997","messageId":"49D1EFF2.303@viscovery.net","threadId":"18370","inReplyTo":"alpine.DEB.1.00.0903311210000.10279@pacific.mpi-cbg.de","subject":"Re: [RFC/PATCH] graph API: Added logic for colored edges.","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-03-31T10:26:58Z","receivedAt":"2009-03-31T10:26:58Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Johannes Schindelin schrieb:\n> How about this function instead?\n> \n> static void strbuf_add_column(struct strbuf *sb,\n> \tconst struct column *column, const char *fmt, ...)\n> {\n> va_list ap;\n> \n> va_start(ap, fmt);\n> \tif (column->color)\n> \t\tstrbuf_addstr(sb, column->color);\n> strbuf_vaddf(sb, fmt, ap);\n> \tif (column->color)\n> \t\tstrbuf_addstr(sb, GIT_COLOR_RESET);\n> va_end(ap);\n> }\n> \n> Hmm?\n\nExcept the strbuf_vaddf() is only in your private repository ;)\n\n-- Hannes\n"},{"id":"110004","messageId":"alpine.DEB.1.00.0903311409320.7052@intel-tinevez-2-302","threadId":"18370","inReplyTo":"49D1EFF2.303@viscovery.net","subject":"Re: [RFC/PATCH] graph API: Added logic for colored edges.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-03-31T12:09:57Z","receivedAt":"2009-03-31T12:09:57Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 31 Mar 2009, Johannes Sixt wrote:\n\n> Johannes Schindelin schrieb:\n> > How about this function instead?\n> > \n> > static void strbuf_add_column(struct strbuf *sb,\n> > \tconst struct column *column, const char *fmt, ...)\n> > {\n> > va_list ap;\n> > \n> > va_start(ap, fmt);\n> > \tif (column->color)\n> > \t\tstrbuf_addstr(sb, column->color);\n> > strbuf_vaddf(sb, fmt, ap);\n> > \tif (column->color)\n> > \t\tstrbuf_addstr(sb, GIT_COLOR_RESET);\n> > va_end(ap);\n> > }\n> > \n> > Hmm?\n> \n> Except the strbuf_vaddf() is only in your private repository ;)\n\nLOL & schenkelklopf!\n\nYou're absolutely correct... I'm sorry,\nDscho\n"}]}