# [PATCH] Share color list between graph and show-branch 5 messages from 2011-03-31 to 2011-04-05. Participants: Dan McGee, Junio C Hamano, Johan Herland. Thread: https://gitlist.dev/t/26935 ## Dan McGee, 2011-03-31 01:38 Subject: [PATCH] Share color list between graph and show-branch Message-ID: <1301535506-1166-1-git-send-email-dpmcgee@gmail.com> URL: https://gitlist.dev/e/1301535506-1166-1-git-send-email-dpmcgee%40gmail.com ``` This also adds the new colors to show-branch that were added a while back for graph output. Signed-off-by: Dan McGee --- builtin/show-branch.c | 16 +++------------- color.c | 19 +++++++++++++++++++ color.h | 4 ++++ graph.c | 21 --------------------- 4 files changed, 26 insertions(+), 34 deletions(-) diff --git a/builtin/show-branch.c b/builtin/show-branch.c index da69581..d00c0ac 100644 --- a/builtin/show-branch.c +++ b/builtin/show-branch.c @@ -12,16 +12,6 @@ static const char* show_branch_usage[] = { }; static int showbranch_use_color = -1; -static char column_colors[][COLOR_MAXLEN] = { - GIT_COLOR_RED, - GIT_COLOR_GREEN, - GIT_COLOR_YELLOW, - GIT_COLOR_BLUE, - GIT_COLOR_MAGENTA, - GIT_COLOR_CYAN, -}; - -#define COLUMN_COLORS_MAX (ARRAY_SIZE(column_colors)) static int default_num; static int default_alloc; @@ -37,7 +27,7 @@ static const char **default_arg; static const char *get_color_code(int idx) { if (showbranch_use_color) - return column_colors[idx]; + return column_colors_ansi[idx % COLUMN_COLORS_ANSI_MAX]; return ""; } @@ -892,7 +882,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix) for (j = 0; j < i; j++) putchar(' '); printf("%s%c%s [%s] ", - get_color_code(i % COLUMN_COLORS_MAX), + get_color_code(i), is_head ? '*' : '!', get_color_reset_code(), ref_name[i]); } @@ -954,7 +944,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix) else mark = '+'; printf("%s%c%s", - get_color_code(i % COLUMN_COLORS_MAX), + get_color_code(i), mark, get_color_reset_code()); } putchar(' '); diff --git a/color.c b/color.c index 417cf8f..6631346 100644 --- a/color.c +++ b/color.c @@ -3,6 +3,25 @@ int git_use_color_default = 0; +/* + * The list of available column colors. + */ +const char *column_colors_ansi[13] = { + GIT_COLOR_RED, + GIT_COLOR_GREEN, + GIT_COLOR_YELLOW, + GIT_COLOR_BLUE, + GIT_COLOR_MAGENTA, + GIT_COLOR_CYAN, + GIT_COLOR_BOLD_RED, + GIT_COLOR_BOLD_GREEN, + GIT_COLOR_BOLD_YELLOW, + GIT_COLOR_BOLD_BLUE, + GIT_COLOR_BOLD_MAGENTA, + GIT_COLOR_BOLD_CYAN, + GIT_COLOR_RESET, +}; + static int parse_color(const char *name, int len) { static const char * const color_names[] = { diff --git a/color.h b/color.h index c0528cf..a7da793 100644 --- a/color.h +++ b/color.h @@ -53,6 +53,10 @@ struct strbuf; */ extern int git_use_color_default; +extern const char *column_colors_ansi[13]; + +/* Ignore the RESET at the end when giving the size */ +#define COLUMN_COLORS_ANSI_MAX (ARRAY_SIZE(column_colors_ansi) - 1) /* * Use this instead of git_default_config if you need the value of color.ui. diff --git a/graph.c b/graph.c index ef2e24e..d1dd15e 100644 --- a/graph.c +++ b/graph.c @@ -59,27 +59,6 @@ enum graph_state { GRAPH_COLLAPSING }; -/* - * The list of available column colors. - */ -static const char *column_colors_ansi[] = { - GIT_COLOR_RED, - GIT_COLOR_GREEN, - GIT_COLOR_YELLOW, - GIT_COLOR_BLUE, - GIT_COLOR_MAGENTA, - GIT_COLOR_CYAN, - GIT_COLOR_BOLD_RED, - GIT_COLOR_BOLD_GREEN, - GIT_COLOR_BOLD_YELLOW, - GIT_COLOR_BOLD_BLUE, - GIT_COLOR_BOLD_MAGENTA, - GIT_COLOR_BOLD_CYAN, - GIT_COLOR_RESET, -}; - -#define COLUMN_COLORS_ANSI_MAX (ARRAY_SIZE(column_colors_ansi) - 1) - static const char **column_colors; static unsigned short column_colors_max; -- 1.7.4.2 ``` ## Junio C Hamano, 2011-04-03 19:12 Subject: Re: [PATCH] Share color list between graph and show-branch Message-ID: <7v7hbbcfoj.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7v7hbbcfoj.fsf%40alter.siamese.dyndns.org In-Reply-To: <1301535506-1166-1-git-send-email-dpmcgee@gmail.com> ``` Dan McGee writes: > diff --git a/color.h b/color.h > index c0528cf..a7da793 100644 > --- a/color.h > +++ b/color.h > @@ -53,6 +53,10 @@ struct strbuf; > */ > extern int git_use_color_default; > > +extern const char *column_colors_ansi[13]; > + > +/* Ignore the RESET at the end when giving the size */ > +#define COLUMN_COLORS_ANSI_MAX (ARRAY_SIZE(column_colors_ansi) - 1) Sneaky. I first went "Huh? -- this array-size macro cannot work", expecting that the array is not decleared with a fixed size in the header. It may make sense to unify these two palettes whose slot assignment does not have any meaning, but it feels that the above change totally goes against the spirit of using ARRAY_SIZE() macro, the point of which is to liberate programmers from having to count and adjust the size when adding the contents to the array. Wouldn't it make more sense to do something like >>> in the header <<< extern const char *custom_colors_ansi[]; extern const int CUSTOM_COLORS_ANSI_MAX; >>> in the code <<< const char *custom_colors_ansi[] = { ... as before ... }; /* Does not count the last element "RESET" */ const int CUSTOM_COLORS_ANSI_MAX = ARRAY_SIZE(custom_colors_ansi) - 1; to avoid mistakes? ``` ## Dan McGee, 2011-04-05 00:32 Subject: Re: [PATCH] Share color list between graph and show-branch Message-ID: URL: https://gitlist.dev/e/BANLkTint1%2Bc0h9DExydWeeafdgawEJPuMw%40mail.gmail.com In-Reply-To: <7v7hbbcfoj.fsf@alter.siamese.dyndns.org> ``` On Sun, Apr 3, 2011 at 2:12 PM, Junio C Hamano wrote: > Dan McGee writes: > >> diff --git a/color.h b/color.h >> index c0528cf..a7da793 100644 >> --- a/color.h >> +++ b/color.h >> @@ -53,6 +53,10 @@ struct strbuf; >>   */ >>  extern int git_use_color_default; >> >> +extern const char *column_colors_ansi[13]; >> + >> +/* Ignore the RESET at the end when giving the size */ >> +#define COLUMN_COLORS_ANSI_MAX (ARRAY_SIZE(column_colors_ansi) - 1) > > Sneaky. > > I first went "Huh? -- this array-size macro cannot work", expecting that > the array is not decleared with a fixed size in the header. > > It may make sense to unify these two palettes whose slot assignment does > not have any meaning, but it feels that the above change totally goes > against the spirit of using ARRAY_SIZE() macro, the point of which is to > liberate programmers from having to count and adjust the size when adding > the contents to the array. > > Wouldn't it make more sense to do something like > >    >>> in the header <<< >    extern const char *custom_colors_ansi[]; >    extern const int CUSTOM_COLORS_ANSI_MAX; > >    >>> in the code <<< >    const char *custom_colors_ansi[] = { >            ... as before ... >    }; >    /* Does not count the last element "RESET" */ >    const int CUSTOM_COLORS_ANSI_MAX = ARRAY_SIZE(custom_colors_ansi) - 1; > > to avoid mistakes? Duh. This makes way more sense, I'll resend the patch with the necessary changes; I couldn't think of an elegant way to do it at the time. On another note, we also have this whole crazy "- 1" bit and the RESET element at the end, and yet I see nowhere that slot is actually used. It looks like this was introduced by commit 1e3d4119d21df28. -Dan ``` ## Dan McGee, 2011-04-05 05:40 Subject: [PATCH] Share color list between graph and show-branch Message-ID: <1301982023-891-1-git-send-email-dpmcgee@gmail.com> URL: https://gitlist.dev/e/1301982023-891-1-git-send-email-dpmcgee%40gmail.com In-Reply-To: ``` This also adds the new colors to show-branch that were added a while back for graph output. Signed-off-by: Dan McGee --- Updated to not require a static length definition of the array anywhere. builtin/show-branch.c | 16 +++------------- color.c | 22 ++++++++++++++++++++++ color.h | 3 +++ graph.c | 23 +---------------------- 4 files changed, 29 insertions(+), 35 deletions(-) diff --git a/builtin/show-branch.c b/builtin/show-branch.c index da69581..1abcd9e 100644 --- a/builtin/show-branch.c +++ b/builtin/show-branch.c @@ -12,16 +12,6 @@ static const char* show_branch_usage[] = { }; static int showbranch_use_color = -1; -static char column_colors[][COLOR_MAXLEN] = { - GIT_COLOR_RED, - GIT_COLOR_GREEN, - GIT_COLOR_YELLOW, - GIT_COLOR_BLUE, - GIT_COLOR_MAGENTA, - GIT_COLOR_CYAN, -}; - -#define COLUMN_COLORS_MAX (ARRAY_SIZE(column_colors)) static int default_num; static int default_alloc; @@ -37,7 +27,7 @@ static const char **default_arg; static const char *get_color_code(int idx) { if (showbranch_use_color) - return column_colors[idx]; + return column_colors_ansi[idx % column_colors_ansi_max]; return ""; } @@ -892,7 +882,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix) for (j = 0; j < i; j++) putchar(' '); printf("%s%c%s [%s] ", - get_color_code(i % COLUMN_COLORS_MAX), + get_color_code(i), is_head ? '*' : '!', get_color_reset_code(), ref_name[i]); } @@ -954,7 +944,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix) else mark = '+'; printf("%s%c%s", - get_color_code(i % COLUMN_COLORS_MAX), + get_color_code(i), mark, get_color_reset_code()); } putchar(' '); diff --git a/color.c b/color.c index 417cf8f..3db214c 100644 --- a/color.c +++ b/color.c @@ -3,6 +3,28 @@ int git_use_color_default = 0; +/* + * The list of available column colors. + */ +const char *column_colors_ansi[] = { + GIT_COLOR_RED, + GIT_COLOR_GREEN, + GIT_COLOR_YELLOW, + GIT_COLOR_BLUE, + GIT_COLOR_MAGENTA, + GIT_COLOR_CYAN, + GIT_COLOR_BOLD_RED, + GIT_COLOR_BOLD_GREEN, + GIT_COLOR_BOLD_YELLOW, + GIT_COLOR_BOLD_BLUE, + GIT_COLOR_BOLD_MAGENTA, + GIT_COLOR_BOLD_CYAN, + GIT_COLOR_RESET, +}; + +/* Ignore the RESET at the end when giving the size */ +const int column_colors_ansi_max = ARRAY_SIZE(column_colors_ansi) - 1; + static int parse_color(const char *name, int len) { static const char * const color_names[] = { diff --git a/color.h b/color.h index c0528cf..68a926a 100644 --- a/color.h +++ b/color.h @@ -53,6 +53,9 @@ struct strbuf; */ extern int git_use_color_default; +/* A default list of colors to use for commit graphs and show-branch output */ +extern const char *column_colors_ansi[]; +extern const int column_colors_ansi_max; /* * Use this instead of git_default_config if you need the value of color.ui. diff --git a/graph.c b/graph.c index ef2e24e..2f6893d 100644 --- a/graph.c +++ b/graph.c @@ -59,27 +59,6 @@ enum graph_state { GRAPH_COLLAPSING }; -/* - * The list of available column colors. - */ -static const char *column_colors_ansi[] = { - GIT_COLOR_RED, - GIT_COLOR_GREEN, - GIT_COLOR_YELLOW, - GIT_COLOR_BLUE, - GIT_COLOR_MAGENTA, - GIT_COLOR_CYAN, - GIT_COLOR_BOLD_RED, - GIT_COLOR_BOLD_GREEN, - GIT_COLOR_BOLD_YELLOW, - GIT_COLOR_BOLD_BLUE, - GIT_COLOR_BOLD_MAGENTA, - GIT_COLOR_BOLD_CYAN, - GIT_COLOR_RESET, -}; - -#define COLUMN_COLORS_ANSI_MAX (ARRAY_SIZE(column_colors_ansi) - 1) - static const char **column_colors; static unsigned short column_colors_max; @@ -228,7 +207,7 @@ struct git_graph *graph_init(struct rev_info *opt) if (!column_colors) graph_set_column_colors(column_colors_ansi, - COLUMN_COLORS_ANSI_MAX); + column_colors_ansi_max); graph->commit = NULL; graph->revs = opt; -- 1.7.4.2 ``` ## Johan Herland, 2011-04-05 07:29 Subject: Re: [PATCH] Share color list between graph and show-branch Message-ID: <201104050929.17135.johan@herland.net> URL: https://gitlist.dev/e/201104050929.17135.johan%40herland.net In-Reply-To: ``` On Tuesday 05 April 2011, Dan McGee wrote: > On another note, we also have this whole crazy "- 1" bit and the RESET > element at the end, and yet I see nowhere that slot is actually used. > It looks like this was introduced by commit 1e3d4119d21df28. Read that commit again. You'll see that in graph.c:strbuf_write_column() it replaces strbuf_addstr(sb, GIT_COLOR_RESET); with strbuf_addstr(sb, column_get_color_code(column_colors_max)); which resolves to the same thing. The reason for that extra indirection is to enable replacing the column_colors_ansi array with a different color array, to do graph coloring in non-ANSI contexts. Specifically, it was done to enable HTML/CSS coloring of graphs in CGit: http://hjemli.net/git/cgit/commit/?id=268b34af23cdcac87aed3300bfe6154cbc65753e It should be obvious that if we replace the ANSI coloring scheme with some other coloring scheme, we also need to change the RESET entry (resetting a HTML "color" with the ANSI reset code is nonsense). Therefore I opted to move the RESET code into the column_colors array, and make column_colors_max indicate both (a) the length of the column_colors array, and (b) the index of the RESET code in that same array. That's why we need the crazy "- 1" bit when defining COLUMN_COLORS_ANSI_MAX. BTW, this is documented graph.h:graph_set_column_colors() from the same 1e3d4119d21df28 commit. ...Johan -- Johan Herland, www.herland.net ```