{"thread":{"id":"26935","subject":"[PATCH] Share color list between graph and show-branch","startedAt":"2011-03-31T01:38:26Z","lastAt":"2011-04-05T07:29:16Z","messageCount":5,"participants":["Dan McGee","Junio C Hamano","Johan Herland"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"164745","messageId":"1301535506-1166-1-git-send-email-dpmcgee@gmail.com","threadId":"26935","inReplyTo":null,"subject":"[PATCH] Share color list between graph and show-branch","fromName":"Dan McGee","fromEmail":"dpmcgee@gmail.com","sentAt":"2011-03-31T01:38:26Z","receivedAt":"2011-03-31T01:38:26Z","isPatch":true,"sender":{"key":"dpmcgee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/265817?v=4"},"body":"This also adds the new colors to show-branch that were added a while\nback for graph output.\n\nSigned-off-by: Dan McGee <dpmcgee@gmail.com>\n---\n builtin/show-branch.c |   16 +++-------------\n color.c               |   19 +++++++++++++++++++\n color.h               |    4 ++++\n graph.c               |   21 ---------------------\n 4 files changed, 26 insertions(+), 34 deletions(-)\n\ndiff --git a/builtin/show-branch.c b/builtin/show-branch.c\nindex da69581..d00c0ac 100644\n--- a/builtin/show-branch.c\n+++ b/builtin/show-branch.c\n@@ -12,16 +12,6 @@ static const char* show_branch_usage[] = {\n };\n \n static int showbranch_use_color = -1;\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-};\n-\n-#define COLUMN_COLORS_MAX (ARRAY_SIZE(column_colors))\n \n static int default_num;\n static int default_alloc;\n@@ -37,7 +27,7 @@ static const char **default_arg;\n static const char *get_color_code(int idx)\n {\n \tif (showbranch_use_color)\n-\t\treturn column_colors[idx];\n+\t\treturn column_colors_ansi[idx % COLUMN_COLORS_ANSI_MAX];\n \treturn \"\";\n }\n \n@@ -892,7 +882,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \t\t\t\tfor (j = 0; j < i; j++)\n \t\t\t\t\tputchar(' ');\n \t\t\t\tprintf(\"%s%c%s [%s] \",\n-\t\t\t\t       get_color_code(i % COLUMN_COLORS_MAX),\n+\t\t\t\t       get_color_code(i),\n \t\t\t\t       is_head ? '*' : '!',\n \t\t\t\t       get_color_reset_code(), ref_name[i]);\n \t\t\t}\n@@ -954,7 +944,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \t\t\t\telse\n \t\t\t\t\tmark = '+';\n \t\t\t\tprintf(\"%s%c%s\",\n-\t\t\t\t       get_color_code(i % COLUMN_COLORS_MAX),\n+\t\t\t\t       get_color_code(i),\n \t\t\t\t       mark, get_color_reset_code());\n \t\t\t}\n \t\t\tputchar(' ');\ndiff --git a/color.c b/color.c\nindex 417cf8f..6631346 100644\n--- a/color.c\n+++ b/color.c\n@@ -3,6 +3,25 @@\n \n int git_use_color_default = 0;\n \n+/*\n+ * The list of available column colors.\n+ */\n+const char *column_colors_ansi[13] = {\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_RED,\n+\tGIT_COLOR_BOLD_GREEN,\n+\tGIT_COLOR_BOLD_YELLOW,\n+\tGIT_COLOR_BOLD_BLUE,\n+\tGIT_COLOR_BOLD_MAGENTA,\n+\tGIT_COLOR_BOLD_CYAN,\n+\tGIT_COLOR_RESET,\n+};\n+\n static int parse_color(const char *name, int len)\n {\n \tstatic const char * const color_names[] = {\ndiff --git a/color.h b/color.h\nindex c0528cf..a7da793 100644\n--- a/color.h\n+++ b/color.h\n@@ -53,6 +53,10 @@ struct strbuf;\n  */\n extern int git_use_color_default;\n \n+extern const char *column_colors_ansi[13];\n+\n+/* Ignore the RESET at the end when giving the size */\n+#define COLUMN_COLORS_ANSI_MAX (ARRAY_SIZE(column_colors_ansi) - 1)\n \n /*\n  * Use this instead of git_default_config if you need the value of color.ui.\ndiff --git a/graph.c b/graph.c\nindex ef2e24e..d1dd15e 100644\n--- a/graph.c\n+++ b/graph.c\n@@ -59,27 +59,6 @@ enum graph_state {\n \tGRAPH_COLLAPSING\n };\n \n-/*\n- * The list of available column colors.\n- */\n-static const char *column_colors_ansi[] = {\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_RED,\n-\tGIT_COLOR_BOLD_GREEN,\n-\tGIT_COLOR_BOLD_YELLOW,\n-\tGIT_COLOR_BOLD_BLUE,\n-\tGIT_COLOR_BOLD_MAGENTA,\n-\tGIT_COLOR_BOLD_CYAN,\n-\tGIT_COLOR_RESET,\n-};\n-\n-#define COLUMN_COLORS_ANSI_MAX (ARRAY_SIZE(column_colors_ansi) - 1)\n-\n static const char **column_colors;\n static unsigned short column_colors_max;\n \n-- \n1.7.4.2\n"},{"id":"165074","messageId":"7v7hbbcfoj.fsf@alter.siamese.dyndns.org","threadId":"26935","inReplyTo":"1301535506-1166-1-git-send-email-dpmcgee@gmail.com","subject":"Re: [PATCH] Share color list between graph and show-branch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-04-03T19:12:28Z","receivedAt":"2011-04-03T19:12:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dan McGee <dpmcgee@gmail.com> writes:\n\n> diff --git a/color.h b/color.h\n> index c0528cf..a7da793 100644\n> --- a/color.h\n> +++ b/color.h\n> @@ -53,6 +53,10 @@ struct strbuf;\n>   */\n>  extern int git_use_color_default;\n>  \n> +extern const char *column_colors_ansi[13];\n> +\n> +/* Ignore the RESET at the end when giving the size */\n> +#define COLUMN_COLORS_ANSI_MAX (ARRAY_SIZE(column_colors_ansi) - 1)\n\nSneaky.\n\nI first went \"Huh? -- this array-size macro cannot work\", expecting that\nthe array is not decleared with a fixed size in the header.\n\nIt may make sense to unify these two palettes whose slot assignment does\nnot have any meaning, but it feels that the above change totally goes\nagainst the spirit of using ARRAY_SIZE() macro, the point of which is to\nliberate programmers from having to count and adjust the size when adding\nthe contents to the array.\n\nWouldn't it make more sense to do something like\n\n    >>> in the header <<<\n    extern const char *custom_colors_ansi[];\n    extern const int CUSTOM_COLORS_ANSI_MAX;\n\n    >>> in the code <<<\n    const char *custom_colors_ansi[] = {\n            ... as before ...\n    };\n    /* Does not count the last element \"RESET\" */\n    const int CUSTOM_COLORS_ANSI_MAX = ARRAY_SIZE(custom_colors_ansi) - 1;\n\nto avoid mistakes?\n"},{"id":"165149","messageId":"BANLkTint1+c0h9DExydWeeafdgawEJPuMw@mail.gmail.com","threadId":"26935","inReplyTo":"7v7hbbcfoj.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Share color list between graph and show-branch","fromName":"Dan McGee","fromEmail":"dpmcgee@gmail.com","sentAt":"2011-04-05T00:32:05Z","receivedAt":"2011-04-05T00:32:05Z","isPatch":true,"sender":{"key":"dpmcgee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/265817?v=4"},"body":"On Sun, Apr 3, 2011 at 2:12 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Dan McGee <dpmcgee@gmail.com> writes:\n>\n>> diff --git a/color.h b/color.h\n>> index c0528cf..a7da793 100644\n>> --- a/color.h\n>> +++ b/color.h\n>> @@ -53,6 +53,10 @@ struct strbuf;\n>>   */\n>>  extern int git_use_color_default;\n>>\n>> +extern const char *column_colors_ansi[13];\n>> +\n>> +/* Ignore the RESET at the end when giving the size */\n>> +#define COLUMN_COLORS_ANSI_MAX (ARRAY_SIZE(column_colors_ansi) - 1)\n>\n> Sneaky.\n>\n> I first went \"Huh? -- this array-size macro cannot work\", expecting that\n> the array is not decleared with a fixed size in the header.\n>\n> It may make sense to unify these two palettes whose slot assignment does\n> not have any meaning, but it feels that the above change totally goes\n> against the spirit of using ARRAY_SIZE() macro, the point of which is to\n> liberate programmers from having to count and adjust the size when adding\n> the contents to the array.\n>\n> Wouldn't it make more sense to do something like\n>\n>    >>> in the header <<<\n>    extern const char *custom_colors_ansi[];\n>    extern const int CUSTOM_COLORS_ANSI_MAX;\n>\n>    >>> in the code <<<\n>    const char *custom_colors_ansi[] = {\n>            ... as before ...\n>    };\n>    /* Does not count the last element \"RESET\" */\n>    const int CUSTOM_COLORS_ANSI_MAX = ARRAY_SIZE(custom_colors_ansi) - 1;\n>\n> to avoid mistakes?\n\nDuh. This makes way more sense, I'll resend the patch with the\nnecessary changes; I couldn't think of an elegant way to do it at the\ntime.\n\nOn another note, we also have this whole crazy \"- 1\" bit and the RESET\nelement at the end, and yet I see nowhere that slot is actually used.\nIt looks like this was introduced by commit 1e3d4119d21df28.\n\n-Dan\n"},{"id":"165165","messageId":"1301982023-891-1-git-send-email-dpmcgee@gmail.com","threadId":"26935","inReplyTo":"BANLkTint1+c0h9DExydWeeafdgawEJPuMw@mail.gmail.com","subject":"[PATCH] Share color list between graph and show-branch","fromName":"Dan McGee","fromEmail":"dpmcgee@gmail.com","sentAt":"2011-04-05T05:40:23Z","receivedAt":"2011-04-05T05:40:23Z","isPatch":true,"sender":{"key":"dpmcgee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/265817?v=4"},"body":"This also adds the new colors to show-branch that were added a while\nback for graph output.\n\nSigned-off-by: Dan McGee <dpmcgee@gmail.com>\n---\n\nUpdated to not require a static length definition of the array anywhere.\n\n builtin/show-branch.c |   16 +++-------------\n color.c               |   22 ++++++++++++++++++++++\n color.h               |    3 +++\n graph.c               |   23 +----------------------\n 4 files changed, 29 insertions(+), 35 deletions(-)\n\ndiff --git a/builtin/show-branch.c b/builtin/show-branch.c\nindex da69581..1abcd9e 100644\n--- a/builtin/show-branch.c\n+++ b/builtin/show-branch.c\n@@ -12,16 +12,6 @@ static const char* show_branch_usage[] = {\n };\n \n static int showbranch_use_color = -1;\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-};\n-\n-#define COLUMN_COLORS_MAX (ARRAY_SIZE(column_colors))\n \n static int default_num;\n static int default_alloc;\n@@ -37,7 +27,7 @@ static const char **default_arg;\n static const char *get_color_code(int idx)\n {\n \tif (showbranch_use_color)\n-\t\treturn column_colors[idx];\n+\t\treturn column_colors_ansi[idx % column_colors_ansi_max];\n \treturn \"\";\n }\n \n@@ -892,7 +882,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \t\t\t\tfor (j = 0; j < i; j++)\n \t\t\t\t\tputchar(' ');\n \t\t\t\tprintf(\"%s%c%s [%s] \",\n-\t\t\t\t       get_color_code(i % COLUMN_COLORS_MAX),\n+\t\t\t\t       get_color_code(i),\n \t\t\t\t       is_head ? '*' : '!',\n \t\t\t\t       get_color_reset_code(), ref_name[i]);\n \t\t\t}\n@@ -954,7 +944,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \t\t\t\telse\n \t\t\t\t\tmark = '+';\n \t\t\t\tprintf(\"%s%c%s\",\n-\t\t\t\t       get_color_code(i % COLUMN_COLORS_MAX),\n+\t\t\t\t       get_color_code(i),\n \t\t\t\t       mark, get_color_reset_code());\n \t\t\t}\n \t\t\tputchar(' ');\ndiff --git a/color.c b/color.c\nindex 417cf8f..3db214c 100644\n--- a/color.c\n+++ b/color.c\n@@ -3,6 +3,28 @@\n \n int git_use_color_default = 0;\n \n+/*\n+ * The list of available column colors.\n+ */\n+const char *column_colors_ansi[] = {\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_RED,\n+\tGIT_COLOR_BOLD_GREEN,\n+\tGIT_COLOR_BOLD_YELLOW,\n+\tGIT_COLOR_BOLD_BLUE,\n+\tGIT_COLOR_BOLD_MAGENTA,\n+\tGIT_COLOR_BOLD_CYAN,\n+\tGIT_COLOR_RESET,\n+};\n+\n+/* Ignore the RESET at the end when giving the size */\n+const int column_colors_ansi_max = ARRAY_SIZE(column_colors_ansi) - 1;\n+\n static int parse_color(const char *name, int len)\n {\n \tstatic const char * const color_names[] = {\ndiff --git a/color.h b/color.h\nindex c0528cf..68a926a 100644\n--- a/color.h\n+++ b/color.h\n@@ -53,6 +53,9 @@ struct strbuf;\n  */\n extern int git_use_color_default;\n \n+/* A default list of colors to use for commit graphs and show-branch output */\n+extern const char *column_colors_ansi[];\n+extern const int column_colors_ansi_max;\n \n /*\n  * Use this instead of git_default_config if you need the value of color.ui.\ndiff --git a/graph.c b/graph.c\nindex ef2e24e..2f6893d 100644\n--- a/graph.c\n+++ b/graph.c\n@@ -59,27 +59,6 @@ enum graph_state {\n \tGRAPH_COLLAPSING\n };\n \n-/*\n- * The list of available column colors.\n- */\n-static const char *column_colors_ansi[] = {\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_RED,\n-\tGIT_COLOR_BOLD_GREEN,\n-\tGIT_COLOR_BOLD_YELLOW,\n-\tGIT_COLOR_BOLD_BLUE,\n-\tGIT_COLOR_BOLD_MAGENTA,\n-\tGIT_COLOR_BOLD_CYAN,\n-\tGIT_COLOR_RESET,\n-};\n-\n-#define COLUMN_COLORS_ANSI_MAX (ARRAY_SIZE(column_colors_ansi) - 1)\n-\n static const char **column_colors;\n static unsigned short column_colors_max;\n \n@@ -228,7 +207,7 @@ struct git_graph *graph_init(struct rev_info *opt)\n \n \tif (!column_colors)\n \t\tgraph_set_column_colors(column_colors_ansi,\n-\t\t\t\t\tCOLUMN_COLORS_ANSI_MAX);\n+\t\t\t\t\tcolumn_colors_ansi_max);\n \n \tgraph->commit = NULL;\n \tgraph->revs = opt;\n-- \n1.7.4.2\n"},{"id":"165170","messageId":"201104050929.17135.johan@herland.net","threadId":"26935","inReplyTo":"BANLkTint1+c0h9DExydWeeafdgawEJPuMw@mail.gmail.com","subject":"Re: [PATCH] Share color list between graph and show-branch","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2011-04-05T07:29:16Z","receivedAt":"2011-04-05T07:29:16Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Tuesday 05 April 2011, Dan McGee wrote:\n> On another note, we also have this whole crazy \"- 1\" bit and the RESET\n> element at the end, and yet I see nowhere that slot is actually used.\n> It looks like this was introduced by commit 1e3d4119d21df28.\n\nRead that commit again. You'll see that in graph.c:strbuf_write_column() it \nreplaces\n\n  strbuf_addstr(sb, GIT_COLOR_RESET);\n\nwith\n\n  strbuf_addstr(sb, column_get_color_code(column_colors_max));\n\nwhich resolves to the same thing. The reason for that extra indirection is \nto enable replacing the column_colors_ansi array with a different color \narray, to do graph coloring in non-ANSI contexts. Specifically, it was done \nto enable HTML/CSS coloring of graphs in CGit: \nhttp://hjemli.net/git/cgit/commit/?id=268b34af23cdcac87aed3300bfe6154cbc65753e\n\nIt should be obvious that if we replace the ANSI coloring scheme with some \nother coloring scheme, we also need to change the RESET entry (resetting a \nHTML \"color\" with the ANSI reset code is nonsense). Therefore I opted to \nmove the RESET code into the column_colors array, and make column_colors_max \nindicate both (a) the length of the column_colors array, and (b) the index \nof the RESET code in that same array. That's why we need the crazy \"- 1\" bit \nwhen defining COLUMN_COLORS_ANSI_MAX.\n\nBTW, this is documented graph.h:graph_set_column_colors() from the same \n1e3d4119d21df28 commit.\n\n\n...Johan\n\n-- \nJohan Herland, <johan@herland.net>\nwww.herland.net\n"}]}