add -p: gracefully handle unparseable hunk headers in colored diffs

In
https://lore.kernel.org/git/ecf6f5be-22ca-299f-a8f1-bda38e5ca246@gmail.com,
Phillipe Blain reported that the built-in `git add -p` command fails
when asked to use [`diff-so-fancy`][diff-so-fancy] to colorize the diff.

The reason is that this tool produces colored diffs with a hunk header
that does not contain any parseable `@@ ... @@` line range information,
and therefore we cannot detect any part in that header that comes after
the line range.

As proposed by Phillip Wood, let's take that for a clear indicator that
we should show the hunk headers verbatim. This is what the Perl version
of the interactive `add` command did, too.

[diff-so-fancy]: https://github.com/so-fancy/diff-so-fancy

Reported-by: Philippe Blain <levraiphilippeblain@gmail.com>
Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/add-patch.c b/add-patch.c
index 34f3807ff32b1a38ba135906c4d352f06bc15c2f..a6bd150de5112bfbbc318f51ee280e4f67be2c85 100644 (file)
--- a/add-patch.c
+++ b/add-patch.c
@@ -238,6 +238,7 @@ struct hunk_header {
         * include the newline.
         */
        size_t extra_start, extra_end, colored_extra_start, colored_extra_end;
+       unsigned suppress_colored_line_range:1;
 };
 
 struct hunk {
@@ -358,15 +359,14 @@ static int parse_hunk_header(struct add_p_state *s, struct hunk *hunk)
        if (!eol)
                eol = s->colored.buf + s->colored.len;
        p = memmem(line, eol - line, "@@ -", 4);
-       if (!p)
-               return error(_("could not parse colored hunk header '%.*s'"),
-                            (int)(eol - line), line);
-       p = memmem(p + 4, eol - p - 4, " @@", 3);
-       if (!p)
-               return error(_("could not parse colored hunk header '%.*s'"),
-                            (int)(eol - line), line);
+       if (p && (p = memmem(p + 4, eol - p - 4, " @@", 3))) {
+               header->colored_extra_start = p + 3 - s->colored.buf;
+       } else {
+               /* could not parse colored hunk header, leave as-is */
+               header->colored_extra_start = hunk->colored_start;
+               header->suppress_colored_line_range = 1;
+       }
        hunk->colored_start = eol - s->colored.buf + (*eol == '\n');
-       header->colored_extra_start = p + 3 - s->colored.buf;
        header->colored_extra_end = hunk->colored_start;
 
        return 0;
@@ -659,6 +659,15 @@ static void render_hunk(struct add_p_state *s, struct hunk *hunk,
                if (!colored) {
                        p = s->plain.buf + header->extra_start;
                        len = header->extra_end - header->extra_start;
+               } else if (header->suppress_colored_line_range) {
+                       strbuf_add(out,
+                                  s->colored.buf + header->colored_extra_start,
+                                  header->colored_extra_end -
+                                  header->colored_extra_start);
+
+                       strbuf_add(out, s->colored.buf + hunk->colored_start,
+                                  hunk->colored_end - hunk->colored_start);
+                       return;
                } else {
                        strbuf_addstr(out, s->s.fraginfo_color);
                        p = s->colored.buf + header->colored_extra_start;

diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index 6b1137e7c9c0823c1be3c52fd0d49d34343c3157..447fc3380afabb70722be003a0dfc9883724e903 100755 (executable)
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -767,6 +767,16 @@ test_expect_success 'detect bogus diffFilter output' '
        grep "mismatched output" output
 '
 
+test_expect_success 'handle iffy colored hunk headers' '
+       git reset --hard &&
+
+       echo content >test &&
+       printf n >n &&
+       force_color git -c interactive.diffFilter="sed s/.*@@.*/XX/" \
+               add -p >output 2>&1 <n &&
+       grep "^XX$" output
+'
+
 test_expect_success 'diff.algorithm is passed to `git diff-files`' '
        git reset --hard &&