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

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

quoted

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").

quoted

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