Re: [PATCH v2 13/18] color: provide inverted colors, too

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

 



On Sun, May 06, 2018 at 12:03:50AM +0200, Johannes Schindelin wrote:

> > There's a "reverse" attribute (which we already parse and support) that
> > can do this without having to repeat the colors. AFAIK it's well
> > supported everywhere, but I could be wrong.
> 
> How would I use that here, though? I need to get the thing via
> diff_get_color_opt() which takes a parameter of type `enum color_diff`.
> There is no way I can specify `reverse` here, can I?

My thinking was that the code would know that coloring the initial "+"
should combine color.diff.new, along with a new tbdiff-specific config
option. So the C equivalent of something like this:

  new=$(git config --get-color color.diff.new green)
  tbdiff=$(git config --get-color color.tbdiff.new reverse)
  reset=$(git config --get-color color.diff.reset reset)

  echo "${new}${tbdiff}+${reset}${new}+actual diff content${reset}"

Then if you set color.diff.new to blue, you'll get a reverse-blue "+"
without having to configure anything else.

You can still override the tbdiff coloring with a totally unrelated
color, since it comes after ${new} (so you could set it to purple or
something if you wanted, though obviously a background or attribute from
${new} can still leak through if you have one set). The only downside in
such a case is that the color sequence is slightly longer ("green, no
blue!").

You could also have tbdiff.new and tbdiff.old to allow setting them
independently (but they'd both default to "reverse").

> > I wonder if that would make configuring this slightly more pleasant,
> > since it saves the user having to define "oldinv" whenever they change
> > "old".
> 
> I am all for making the configuration more pleasant. So I hope I can make
> use of the `reverse` thing here, without having to introduce a new enum
> value.

I think the new enum (and matching config) has some value in case people
want to override it.  But if you don't want to, diff_get_color() is
really just checking want_color() as a convenience. You could do that,
too:

  const char *reverse = want_color(opt->use_color) ? GIT_COLOR_REVERSE : "";

You'd have to introduce GIT_COLOR_REVERSE. I don't think we have a
constant for it yet, but it's \x[7m.

-Peff



[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