Re: [PATCHv2 0/3] Make diff plumbing commands respect the indentHeuristic.

From: Jeff King <peff@peff.net>
Date: Apr 29, 2017, 13:14 UTC
Message-ID: <20170429131439.ohgren3i7xr4tjex@sigill.intra.peff.net>
In-Reply-To: <20170429124052.yhgwofbbd5pkd24p@sigill.intra.peff.net>

On Sat, Apr 29, 2017 at 08:40:52AM -0400, Jeff King wrote:

Show 16 quoted lines

> On Fri, Apr 28, 2017 at 06:33:12PM -0400, Marc Branchaud wrote:
> 
> > v2: Fixed up the commit messages and added tests.
> > 
> > Marc Branchaud (2):
> >   diff: make the indent heuristic part of diff's basic configuration
> >   diff: have the diff-* builtins configure diff before initializing
> >     revisions
> > 
> > Stefan Beller (1):
> >   diff: enable indent heuristic by default
> 
> Thanks, these look fine to me. I'd like to get an ACK from Michael, in
> case he had some other reason for omitting them from git_diff_ui_config
> (from my recollection, it's probably just a mix of conservatism and
> following what the compaction heuristic had done).

Sorry, I spoke too soon. The third one needs a few test adjustments squashed in to pass the tests.

The fixes below for t4061 are pretty obvious, but the one in t4051 is anything but. What happens is that we have a function:

  int dummy();
  {
     some body
  }

and modify it to look like:

  int dummy();
  int dummy();
  {
     some body
  }

and we expect that this counts as making a change to the function for the purposes of -W. But with the indent heuristic, instead of saying "we added an extra line after the first dummy()", we say "we added an extra dummy() before the function". Which is perfectly reasonable.

I don't think duplicating the line is important to the test, as opposed to making any random change. But we can't just change a line in the body itself, because the point of the test is making sure -W shows the whole function. And the function is short enough that a change in the body would show the whole thing via context anyway. So I opted to make the change at the same spot, but make the diff less ambiguous.

diff --git a/t/t4051-diff-function-context.sh b/t/t4051-diff-function-context.sh
index 6154acb45..5f46c0341 100755
--- a/t/t4051-diff-function-context.sh
+++ b/t/t4051-diff-function-context.sh
@@ -72,7 +72,7 @@ test_expect_success 'setup' '
 
 	# overlap function context of 1st change and -u context of 2nd change
 	grep -v "delete me from hello" <"$dir/hello.c" >file.c &&
-	sed 2p <"$dir/dummy.c" >>file.c &&
+	sed "2aextra line" <"$dir/dummy.c" >>file.c &&
 	commit_and_tag changed_hello_dummy file.c &&
 
 	git checkout initial &&
diff --git a/t/t4061-diff-indent.sh b/t/t4061-diff-indent.sh
index 13d3dc96a..56d7d7760 100755
--- a/t/t4061-diff-indent.sh
+++ b/t/t4061-diff-indent.sh
@@ -153,7 +153,7 @@ test_expect_success 'prepare' '
 '
 
 test_expect_success 'diff: ugly spaces' '
-	git diff old new -- spaces.txt >out &&
+	git diff --no-indent-heuristic old new -- spaces.txt >out &&
 	compare_diff spaces-expect out
 '
 
@@ -183,7 +183,7 @@ test_expect_success 'diff: --indent-heuristic with --histogram' '
 '
 
 test_expect_success 'diff: ugly functions' '
-	git diff old new -- functions.c >out &&
+	git diff --no-indent-heuristic old new -- functions.c >out &&
 	compare_diff functions-expect out
 '
 
@@ -193,7 +193,7 @@ test_expect_success 'diff: nice functions with --indent-heuristic' '
 '
 
 test_expect_success 'blame: ugly spaces' '
-	git blame old..new -- spaces.txt >out-blame &&
+	git blame --no-indent-heuristic old..new -- spaces.txt >out-blame &&
 	compare_blame spaces-expect out-blame
 '