Re: [PATCH v2 04/11] add -i: use `reset_color` consistently

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Dscho

On 13/11/2020 13:53, Johannes Schindelin wrote:
Hi Phillip,

On Fri, 13 Nov 2020, Phillip Wood wrote:

Hi Dscho

On 11/11/2020 12:28, Johannes Schindelin via GitGitGadget wrote:
From: Johannes Schindelin <johannes.schindelin@xxxxxx>

We already maintain a list of colors in the `add_i_state`, therefore we
should use them.

Playing devil's advocate for a moment - why do we have a reset entry in that
list? The next patch makes sure it cannot be customized which is a good thing
so why not drop the `reset` member from `add_i_state` entirely? The perl
version needed to get the reset string from `git config` and store it
somewhere but in the C version we have a perfectly good constant we can use
instead.

Right.

On the other hand, does it hurt to keep that field for consistency with
the rest of the coloring stuff?

It creates the illusion that `reset` is similar to the others but it isn't as we don't want it to be customizable.

It would probably cost more to make the
change you suggested than it would benefit us.

At the moment there is one use of `s->s.color_reset` and two of `GIT_COLOR_RESET` in add-patch.c

Best Wishes

Phillip

Ciao,
Dscho




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux