Re: Difficulty with parsing colorized diff output

(For the Git project itself, we long ago realized that putting raw color
codes into the source is a big pain when working with diffs, and we
instead use tools like t/test-lib-functions.sh's test_decode_color).

And also we hid the colors behind #defines and such.

quoted

* Context lines do not begin with reset code, but do end with a reset
code. It would be preferable in my opinion if they had both (like
every other line), or none at all.

Context lines do have both. It's just that the default color for context
lines is empty. ;)

The content itself can contain color codes.

Instead of unconditionally resetting each line, we could parse each
content line to determine if we actually have to reset the colors.

But yes, I think it might be reasonable to recognize when an opening
color is empty, and turn the closing reset into a noop in that case (or
I guess you are also advocating the opposite: turn an empty color into a
noop \x1b[m or similar).

I think most of the coloring, including context lines, is coming from
diff.c:emit_diff_symbol_from_struct(). Instead of unconditionally
calling:

  context = diff_get_color_opt(o, DIFF_CONTEXT);
  reset = diff_get_color_opt(o, DIFF_RESET);

I suspect we could have a helper like this:

  static const char *diff_get_reset(const char *color)
  {
        if (!color || !*color)
                return "";
        return diff_colors[DIFF_RESET];
  }
  ...
  context = diff_get_color_opt(o, DIFF_CONTEXT);
  reset = diff_get_reset(context);

Another easier way to do so would be to drop
the line

    needs_reset = 1; /* 'line' may contain color codes. */

in diff.c::emit_line_0
I run the test suite and it passes (I thought we had a test
enforcing we'd reset any user provided coloring).

quoted

* Added lines have excess codes after the plus sign. The entire prefix
is, `\x1b[32m+\x1b[m\x1b[32m` translating to GREEN PLUS RESET GREEN.
Emitting codes after the plus sign makes the parsing more complex and
idiosyncratic.

Then we have broken code in diff.c::emit_line_ws_markup
in the last case ("else {") which first emits the sign via
emit_line_0 and then the rest via ws_check_emit.
It sounds like we'd want to replace `reset` in the call
of emit_line_0 with
  set == set_sign ? NULL : reset
and set in the call to ws_check_emit with
  set == set_sign ? NULL : set

Manually looking at some diff output  of said diff we'd get
  <RED>- emit_line_0(o, set_sign ? set_sign : set, NULL, !!set_sign,
reset,<RESET>
  <GREEN>+<RESET> <GREEN>emit_line_0(o, set_sign ? set_sign : set,
NULL, !!set_sign, set == set_sign ? NULL : reset,<RESET>

and the issue is that we do not color the beginning white space
of the emission from ws_check_emit but maybe we should.

Another idea would be to allow Git to output its output
as if it was run through test_decode_color, slightly related:
https://public-inbox.org/git/20180804015317.182683-8-sbeller@google.com/
i.e. we'd markup the output instead of coloring it.