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

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

From
PWPhillip Wood <phillip.wood123@gmail.com>
Date
Sep 1, 2022, 16:03 UTC
Message-ID
<a560ecb1-2049-7844-758f-4bd44b61eea7@gmail.com>
In-Reply-To
<93d0e3b4d2a3dec5588c6e41d69313385356aa8f.1662046939.git.gitgitgadget@gmail.com>
Hi Dscho
On 01/09/2022 16:42, Johannes Schindelin via GitGitGadget wrote:
Show 22 quoted lines
> 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>
> ---
Show 13 quoted lines
> @@ -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;

Having an extra branch for this that returns early makes it easy to see what is being printed when we cannot parse the hunk header.

Show 16 quoted lines
> 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

We check that the hunk header line from filtered diff was reproduced verbatim

This all looks good to me
Best Wishes
Phillip
Previous: Johannes Schindelin via GitGitGadgetNext: Phillip Wood
Message 65 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.