git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Duy Nguyen <pclouds@gmail.com>
To: Jeff King <peff@peff.net>
Cc: Git Mailing List <git@vger.kernel.org>
Subject: Re: [PATCH] log: support 256 colors with --graph=256colors
Date: Thu, 22 Dec 2016 16:48:48 +0700	[thread overview]
Message-ID: <CACsJy8CnS1=_vA5xhbZ94Qyh7ySC5FvaALu1vhQwt_YJya4wHA@mail.gmail.com> (raw)
In-Reply-To: <20161220165754.hkmnsxiwbcgn6uin@sigill.intra.peff.net>

On Tue, Dec 20, 2016 at 11:57 PM, Jeff King <peff@peff.net> wrote:
> On Tue, Dec 20, 2016 at 07:39:29PM +0700, Nguyễn Thái Ngọc Duy wrote:
>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>> ---
>>  I got mad after tracing two consecutive red history lines in `git log
>>  --graph --oneline` back to their merge points, far far away. Yeah
>>  probably should fire up tig, or gitk or something.
>>
>>  This may sound like a good thing to add, but I don't know how good it
>>  is compared to the good old 16 color palette, yet as I haven't tried it
>>  for long since it's just written.
>
> Hmm. At some point the colors become too close together to be easily
> distinguishable. In your code you have:
>
>> +     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 step by 16, over a set of 215 colors. That seems to give only 13
> colors, versus the original 16. :)
>
> I know that is a simplification. If you wrap around, then you get your
> 13 colors, and then another 13 colors that aren't _quite_ the same, and
> so on, until you've used all 256. I'm just not sure if the 1st and 14th
> color would be visually different enough for it to matter (I admit I
> didn't do any experiments, though).

Yep. If the jump sequence is a random one, we're less likely to run
into this. But I think Junio's "run git-log in 2 terminals with the
same coloring" convinces me randomization here is not the best thing.

The best solution would be select colors per text line, so we can pick
different colors. But I think that's a lot of computation (and
probably an NP problem too). The second best option is have a good,
predefined color palette. We don't need a red of all shades, we need
something that look distinct enough from the rest. I googled for this
first and failed. But I think I could approach it a different way:
collect colors that have names. That reduces the number of colors so
we can go back to "step 1 at a time" and still don't run into two
similar colors often.

>> ---graph::
>> +--graph[=<options>]::
>>       Draw a text-based graphical representation of the commit history
>>       on the left hand side of the output.  This may cause extra lines
>>       to be printed in between commits, in order for the graph history
>
> I wonder if we would ever want another use for "--graph=foo"

I do. See the screenshot in [1] from the original mail. I have to
stare at --graph so often lately that it might get my attention before
other things.

> I guess any such thing could fall under the name of "graph options", and we'd
> end up with "--graph=256colors,unicode" or something like that.

Exactly.

> I do suspect people would want a config option for this, though. I.e.,
> you'd want to enable it all the time if you have a terminal which can
> handle 256 colors, not just for a particular invocation.

Yeah. That also means we need the ability to override/negate config
options, perhaps something like --graph=-256colors.
-- 
Duy

  reply	other threads:[~2016-12-22  9:49 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 [this message]
2016-12-22 19:06     ` Junio C Hamano
2016-12-25  2:36       ` Duy Nguyen
2016-12-20 17:26 ` Junio C Hamano
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='CACsJy8CnS1=_vA5xhbZ94Qyh7ySC5FvaALu1vhQwt_YJya4wHA@mail.gmail.com' \
    --to=pclouds@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    /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).