From: Junio C Hamano <gitster@pobox.com>
To: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] log: support 256 colors with --graph=256colors
Date: Tue, 20 Dec 2016 09:26:03 -0800 [thread overview]
Message-ID: <xmqqh95yldg4.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20161220123929.15329-1-pclouds@gmail.com> ("Nguyễn Thái Ngọc Duy"'s message of "Tue, 20 Dec 2016 19:39:29 +0700")
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> diff --git a/graph.c b/graph.c
> index d4e8519..75375a1 100644
> --- a/graph.c
> +++ b/graph.c
> @@ -78,6 +78,7 @@ static void graph_show_line_prefix(const struct diff_options *diffopt)
>
> static const char **column_colors;
> static unsigned short column_colors_max;
> +static int column_colors_step;
>
> void graph_set_column_colors(const char **colors, unsigned short colors_max)
> {
> @@ -234,10 +235,24 @@ void graph_setup_line_prefix(struct diff_options *diffopt)
> }
>
>
> -struct git_graph *graph_init(struct rev_info *opt)
> +struct git_graph *graph_init_with_options(struct rev_info *opt, const char *arg)
> {
> struct git_graph *graph = xmalloc(sizeof(struct git_graph));
>
> + if (arg && !strcmp(arg, "256colors")) {
> + int i, start = 17, stop = 232;
> + column_colors_max = stop - start;
> + column_colors =
> + xmalloc((column_colors_max + 1) * sizeof(*column_colors));
> + for (i = start; i < stop; i++) {
> + struct strbuf sb = STRBUF_INIT;
> + strbuf_addf(&sb, "\033[38;5;%dm", i);
> + column_colors[i - start] = strbuf_detach(&sb, NULL);
> + }
> + column_colors[column_colors_max] = xstrdup(GIT_COLOR_RESET);
> + /* ignore the closet 16 colors on either side for the next line */
> + column_colors_step = 16;
> + }
So you pre-fill a table of colors with 232-17=215 slots. Is the
idea that it is a co-prime with column_colors_step which is set to
16 so that going over the table with wraparound will cover all its
elements?
> @@ -382,6 +397,20 @@ static unsigned short graph_get_current_column_color(const struct git_graph *gra
> */
> static void graph_increment_column_color(struct git_graph *graph)
> {
> + if (column_colors_step) {
> + static int random_initialized;
> + int v;
> +
> + if (!random_initialized) {
> + srand((unsigned int)getpid());
> + random_initialized = 1;
> + }
> + v = rand() % (column_colors_max - column_colors_step * 2);
> + graph->default_column_color += column_colors_step + v;
> + graph->default_column_color %= column_colors_max;
> + return;
> + }
> +
> graph->default_column_color = (graph->default_column_color + 1) %
> column_colors_max;
> }
This is too ugly to live as-is for two reasons.
- Do you really need rand()? Doesn't this frustrate somebody who
runs the same "git log" in two terminals in order to view an
overly tall graph, expecting both commands that were started with
the same set of arguments to paint the same line in the same
color?
- Even if you needed rand(), you should be able to factor out
computation of V so that the function does not need an early
return that hints totally different processing for two codepaths.
static void graph_increment_column_color(struct git_graph *g)
{
int next;
if (! 256-color in use) {
next = 1;
} else {
do whatever to compute your v;
next = v + column_colors_step;
}
g->default_column_color =
(g->default_column_color + v) / column_colors_max;
}
Or something like that, perhaps?
next prev parent reply other threads:[~2016-12-20 17:26 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-12-20 12:39 [PATCH] log: support 256 colors with --graph=256colors Nguyễn Thái Ngọc Duy
2016-12-20 16:57 ` Jeff King
2016-12-22 9:48 ` Duy Nguyen
2016-12-22 19:06 ` Junio C Hamano
2016-12-25 2:36 ` Duy Nguyen
2016-12-20 17:26 ` Junio C Hamano [this message]
2016-12-22 9:38 ` Duy Nguyen
2016-12-24 11:38 ` [PATCH v2] log --graph: customize the graph lines with config log.graphColors Nguyễn Thái Ngọc Duy
2016-12-25 23:02 ` Junio C Hamano
2017-01-08 10:13 ` [PATCH v3] " Nguyễn Thái Ngọc Duy
2017-01-09 3:05 ` Junio C Hamano
2017-01-09 5:30 ` Jeff King
2017-01-09 10:30 ` Duy Nguyen
2017-01-09 14:23 ` Junio C Hamano
2017-01-09 5:34 ` Jeff King
2017-01-09 10:10 ` Duy Nguyen
2017-01-09 10:32 ` [PATCH v4] " Nguyễn Thái Ngọc Duy
2017-01-09 17:29 ` Junio C Hamano
2017-01-12 12:20 ` Duy Nguyen
2017-01-19 11:41 ` [PATCH v5 0/3] nd/log-graph-configurable-colors Nguyễn Thái Ngọc Duy
2017-01-19 11:41 ` [PATCH v5 1/3] color.c: fix color_parse_mem() with value_len == 0 Nguyễn Thái Ngọc Duy
2017-01-19 16:38 ` Jeff King
2017-01-28 4:07 ` Jeff King
2017-01-19 11:41 ` [PATCH v5 2/3] color.c: trim leading spaces in color_parse_mem() Nguyễn Thái Ngọc Duy
2017-01-19 16:41 ` Jeff King
2017-01-19 18:22 ` Junio C Hamano
2017-01-19 11:41 ` [PATCH v5 3/3] log --graph: customize the graph lines with config log.graphColors Nguyễn Thái Ngọc Duy
2017-01-19 16:51 ` Jeff King
2017-01-19 18:27 ` Junio C Hamano
2017-01-19 19:34 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqh95yldg4.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=pclouds@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).