Re: [PATCH v3 4/5] add -p: handle `diff-so-fancy`'s hunk headers better

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Dscho

On 29/08/2022 16:11, Johannes Schindelin via GitGitGadget wrote:
From: Johannes Schindelin <johannes.schindelin@xxxxxx>

The `diff-so-fancy` diff colorizer produces hunk headers that look
nothing like what the built-in `add -p` expects: there is no `@@ ... @@`
line range, and therefore the parser cannot determine where any extra
information starts (such as the function name that is often added to
those hunk header lines).

However, we can do better than simply swallowing the unparseable hunk
header. There is probably information the user wants to see, after all.
In the `diff-so-fancy` case, it shows something like `@ file:1 @`.

If the line range could not be found in the colored hunk header, let's
just show the complete hunk header.

Looking at the tests, I don't think we just show the complete hunk header, we show the offsets from the unfiltered diff as well. I think that is unfortunate as it kind of defeats the purpose of interactive.diffFilter which is to the the user see the diff how they want to (so long as it has the same number of lines)

Best Wishes

Phillip

Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
---
  add-patch.c                | 13 +++++++++++--
  t/t3701-add-interactive.sh |  4 ++++
  2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/add-patch.c b/add-patch.c
index 9d575d30ed0..0217cdd7c4a 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -363,8 +363,17 @@ static int parse_hunk_header(struct add_p_state *s, struct hunk *hunk)
  	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, showing nothing */
-		header->colored_extra_start = hunk->colored_start;
+		/*
+		 * We tried to parse the line range out of the colored hunk
+		 * header, so that we could show just the extra information
+		 * after the line range.
+		 *
+		 * At this point, we did not find that line range, but the hunk
+		 * header likely has information that the user might find
+		 * interesting. Let's just show the entire hunk header instead
+		 * in that case.
+		 */
+		header->colored_extra_start = line - s->colored.buf;
  	header->colored_extra_end = hunk->colored_start;
return 0;
diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index 49200b7df68..39e68b6d066 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -775,10 +775,14 @@ test_expect_success 'handle iffy colored hunk headers' '
  		add -p <n &&
  	force_color git -c interactive.diffFilter="sed \"s/\(.*@@\).*/\1FN/\"" \
  		add -p >output 2>&1 <n &&
+	force_color git -c interactive.diffFilter="sed \"s/\(.*@@\).*/file/\"" \
+		add -p >output-so-fancy 2>&1 <n &&
  	if test_have_prereq ADD_I_USE_BUILTIN
  	then
+		grep "@ file\$" output-so-fancy &&
  		grep "@ FN\$" output
  	else
+		grep "^file\$" output-so-fancy &&
  		grep "@FN\$" output
  	fi
  '



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux