git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v5 2/3] add -p: gracefully handle unparseable hunk headers in colored diffs

From
Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>
Date
Sep 1, 2022, 15:42 UTC
Message-ID
<93d0e3b4d2a3dec5588c6e41d69313385356aa8f.1662046939.git.gitgitgadget@gmail.com>
In-Reply-To
<pull.1336.v5.git.1662046939.gitgitgadget@gmail.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>

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>
---
 add-patch.c                | 25 +++++++++++++++++--------
 t/t3701-add-interactive.sh | 10 ++++++++++
 2 files changed, 27 insertions(+), 8 deletions(-)
diff --git a/add-patch.c b/add-patch.c
index 34f3807ff32..a6bd150de51 100644
--- 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 8a594700f7b..a94e7c53c8a 100755
--- 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 'handle very large filtered diff' '
 	git reset --hard &&
 	# The specific number here is not important, but it must
-- 
gitgitgadget
Previous: Junio C HamanoNext: Phillip Wood
Message 64 of 67 in “built-in add -p: support diff-so-fancy better”
  1. 0/3 built-in add -p: support diff-so-fancy betterJohannes Schindelin via GitGitGadget, Aug 23, 2022
  2. 3/3 add -p: handle `diff-so-fancy`'s hunk headers betterJohannes Schindelin via GitGitGadget, Aug 23, 2022
  3. 1/3 t3701: redefine what is "bogus" output of a diff filterJohannes Schindelin via GitGitGadget, Aug 23, 2022
  4. 2/3 add -p: gracefully ignore unparseable hunk headers in colored diffsJohannes Schindelin via GitGitGadget, Aug 23, 2022
  5. Philippe BlainAug 24, 2022
  6. Johannes SchindelinAug 24, 2022
  7. Philippe BlainAug 24, 2022
  8. Philippe BlainAug 24, 2022
  9. Junio C HamanoAug 24, 2022
  10. Johannes SchindelinAug 24, 2022
  11. Junio C HamanoAug 24, 2022
  12. 0/4 built-in add -p: support diff-so-fancy betterJohannes Schindelin via GitGitGadget, Aug 24, 2022
  13. 1/4 t3701: redefine what is "bogus" output of a diff filterJohannes Schindelin via GitGitGadget, Aug 24, 2022
  14. 2/4 add -p: gracefully ignore unparseable hunk headers in colored diffsJohannes Schindelin via GitGitGadget, Aug 24, 2022
  15. Junio C HamanoAug 29, 2022
  16. 3/4 add -p: handle `diff-so-fancy`'s hunk headers betterJohannes Schindelin via GitGitGadget, Aug 24, 2022
  17. Junio C HamanoAug 29, 2022
  18. Johannes SchindelinAug 29, 2022
  19. Junio C HamanoAug 29, 2022
  20. Johannes SchindelinAug 30, 2022
  21. 4/4 add -p: ignore dirty submodulesJohannes Schindelin via GitGitGadget, Aug 24, 2022
  22. Junio C HamanoAug 24, 2022
  23. Philippe BlainAug 25, 2022
  24. Johannes SchindelinAug 26, 2022
  25. Philippe BlainAug 26, 2022
  26. 0/5 built-in add -p: support diff-so-fancy betterJohannes Schindelin via GitGitGadget, Aug 29, 2022
  27. 1/5 t3701: redefine what is "bogus" output of a diff filterJohannes Schindelin via GitGitGadget, Aug 29, 2022
  28. Phillip WoodAug 30, 2022
  29. Junio C HamanoAug 30, 2022
  30. Phillip WoodAug 31, 2022
  31. Jeff KingAug 31, 2022
  32. Jeff KingAug 31, 2022
  33. Johannes SchindelinAug 31, 2022
  34. 2/5 add -p: gracefully ignore unparseable hunk headers in colored diffsJohannes Schindelin via GitGitGadget, Aug 29, 2022
  35. 4/5 add -p: handle `diff-so-fancy`'s hunk headers betterJohannes Schindelin via GitGitGadget, Aug 29, 2022
  36. Phillip WoodAug 30, 2022
  37. 3/5 add -p: insert space in colored hunk header as neededJohannes Schindelin via GitGitGadget, Aug 29, 2022
  38. 5/5 add -p: ignore dirty submodulesJohannes Schindelin via GitGitGadget, Aug 29, 2022
  39. Phillip WoodAug 30, 2022
  40. Johannes SchindelinAug 31, 2022
  41. Junio C HamanoAug 31, 2022
  42. Johannes SchindelinAug 31, 2022
  43. Junio C HamanoAug 29, 2022
  44. Johannes SchindelinAug 30, 2022
  45. Phillip WoodAug 30, 2022
  46. Johannes SchindelinAug 31, 2022
  47. 0/3 built-in add -p: support diff-so-fancy betterJohannes Schindelin via GitGitGadget, Aug 31, 2022
  48. 2/3 add -p: gracefully handle unparseable hunk headers in colored diffsJohannes Schindelin via GitGitGadget, Aug 31, 2022
  49. Phillip WoodSep 1, 2022
  50. Johannes SchindelinSep 1, 2022
  51. 3/3 add -p: ignore dirty submodulesJohannes Schindelin via GitGitGadget, Aug 31, 2022
  52. Jeff KingSep 1, 2022
  53. Jeff KingSep 1, 2022
  54. Junio C HamanoSep 1, 2022
  55. Johannes SchindelinSep 2, 2022
  56. 1/3 add -p: detect more mismatches between plain vs colored diffsJohannes Schindelin via GitGitGadget, Aug 31, 2022
  57. Phillip WoodSep 1, 2022
  58. Phillip WoodSep 1, 2022
  59. Junio C HamanoSep 1, 2022
  60. 0/3 built-in add -p: support diff-so-fancy betterJohannes Schindelin via GitGitGadget, Sep 1, 2022
  61. 1/3 add -p: detect more mismatches between plain vs colored diffsJohannes Schindelin via GitGitGadget, Sep 1, 2022
  62. 3/3 add -p: ignore dirty submodulesJohannes Schindelin via GitGitGadget, Sep 1, 2022
  63. Junio C HamanoSep 1, 2022
  64. 2/3 add -p: gracefully handle unparseable hunk headers in colored diffsJohannes Schindelin via GitGitGadget, Sep 1, 2022
  65. Phillip WoodSep 1, 2022
  66. Phillip WoodSep 1, 2022
  67. Junio C HamanoSep 1, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.