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

Re: [PATCH 2/2] add--interactive: detect bogus diffFilter output

From
Junio C Hamano <gitster@pobox.com>
Date
Mar 5, 2018, 20:53 UTC
Message-ID
<xmqqsh9e8css.fsf@gitster-ct.c.googlers.com>
In-Reply-To
<20180303055849.GB15952@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 10 quoted lines
> It's important that the diff-filter only filter the
> individual lines, and that there remain a one-to-one mapping
> between the input and output lines. Otherwise, things like
> hunk-splitting will behave quite unexpectedly (e.g., you
> think you are splitting at one point, but it has a different
> effect in the text patch we apply).
>
> We can't detect all problematic cases, but we can at least
> catch the obvious case where we don't even have the correct
> number of lines.

Will queue. We could probably also make sure that each of the corresponding line pair begins with the same '-/ /+' letter, but we need to draw a line and stop somewhere, and I think the number of lines is probably a good enough place.

Show 44 quoted lines
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  git-add--interactive.perl  | 8 ++++++++
>  t/t3701-add-interactive.sh | 8 ++++++++
>  2 files changed, 16 insertions(+)
>
> diff --git a/git-add--interactive.perl b/git-add--interactive.perl
> index 964c3a7542..ff02008abe 100755
> --- a/git-add--interactive.perl
> +++ b/git-add--interactive.perl
> @@ -705,6 +705,14 @@ sub parse_diff {
>  	}
>  	my (@hunk) = { TEXT => [], DISPLAY => [], TYPE => 'header' };
>  
> +	if (@colored && @colored != @diff) {
> +		print STDERR
> +		  "fatal: mismatched output from interactive.diffFilter\n",
> +		  "hint: Your filter must maintain a one-to-one correspondence\n",
> +		  "hint: between its input and output lines.\n";
> +		exit 1;
> +	}
> +
>  	for (my $i = 0; $i < @diff; $i++) {
>  		if ($diff[$i] =~ /^@@ /) {
>  			push @hunk, { TEXT => [], DISPLAY => [],
> diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
> index 64fe56c3d5..9bb17f91b2 100755
> --- a/t/t3701-add-interactive.sh
> +++ b/t/t3701-add-interactive.sh
> @@ -404,6 +404,14 @@ test_expect_success TTY 'diffFilter filters diff' '
>  	grep foo:.*content output
>  '
>  
> +test_expect_success TTY 'detect bogus diffFilter output' '
> +	git reset --hard &&
> +
> +	echo content >test &&
> +	test_config interactive.diffFilter "echo too-short" &&
> +	printf y | test_must_fail test_terminal git add -p
> +'
> +
>  test_expect_success 'patch-mode via -i prompts for files' '
>  	git reset --hard &&
Previous: Jeff KingNext: Jeff King
Message 10 of 23 in “Bug report: "Use of uninitialized value $_ in print"”
  1. Sam KuperMar 2, 2018
  2. Jonathan NiederMar 2, 2018
  3. Jeff KingMar 2, 2018
  4. Junio C HamanoMar 2, 2018
  5. Jeff KingMar 2, 2018
  6. Junio C HamanoMar 2, 2018
  7. Jeff KingMar 3, 2018
  8. 1/2 t3701: add a test for interactive.diffFilterJeff King, Mar 3, 2018
  9. 2/2 add--interactive: detect bogus diffFilter outputJeff King, Mar 3, 2018
  10. Junio C HamanoMar 5, 2018
  11. Jeff KingMar 5, 2018
  12. Ævar Arnfjörð BjarmasonMar 5, 2018
  13. Junio C HamanoMar 5, 2018
  14. Ævar Arnfjörð BjarmasonMar 5, 2018
  15. Jonathan NiederMar 5, 2018
  16. Sam KuperMar 2, 2018
  17. Jeff KingMar 2, 2018
  18. Sam KuperMar 2, 2018
  19. Jeff KingMar 3, 2018
  20. Junio C HamanoMar 2, 2018
  21. Sam KuperMar 2, 2018
  22. Junio C HamanoMar 2, 2018
  23. Jeff KingMar 3, 2018

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.