Re: [PATCH] diff compaction heuristic: favor shortest neighboring blank lines

From: Jeff King <peff@peff.net>
Date: Jun 17, 2016, 15:36 UTC
Message-ID: <20160617153637.GA9314@sigill.intra.peff.net>
In-Reply-To: <20160616174620.1011-1-sbeller@google.com>

On Thu, Jun 16, 2016 at 10:46:20AM -0700, Stefan Beller wrote:

Show 28 quoted lines

> The compaction heuristic for diffs was deemed quite good, but in 5580b27
> we have an example demonstrates a common case, that fails with the existing
> heuristic. That is why we disabled the heuristic in the v2.9.0 release.
> 
> With this patch a diff looks like:
> 
>          def bar
>                  do_bar_stuff()
> 
>                  common_ending()
>          end
> 
> +        def bal
> +                do_bal_stuff()
> +
> +                common_ending()
> +        end
> +
>          def baz
>                  do_baz_stuff()
> 
>                  common_ending()
>          end
> 
> whereas before we had:
> 
>   WIP (TODO: ask peff to provide an example that actually triggers, I seem to be
>        unable to write one, though I thought the above was one)

Did you want something that triggers the "bad" case with the existing compaction heuristic?

I gave one in:

  http://article.gmane.org/gmane.comp.version-control.git/296947

I think the difference is that in my example, the diff (before compaction) has the blank line at the top (because we are adding a new entry at the bottom), whereas in yours, the blank line is at the bottom.

This patch does make my "bad" case look better. Unfortunately, it re-breaks another case:

	cat >file.rb <<\EOF
	##
	# This is the foo function.
	def foo
	  foo stuff
	end

	##
	# This is the bar function.
	def bar
	  bar stuff
	end

	##
	# This is the baz function.
	def baz
	  baz stuff
	end
	EOF

	git init
	git add file.rb
	sed -i 7,12d file.rb
	git diff

That goes back to the original, pre-compaction-heuristic diff:

	diff --git a/file.rb b/file.rb
	index 0b31fb6..ee25d63 100644
	--- a/file.rb
	+++ b/file.rb
	@@ -5,12 +5,6 @@ def foo
	 end
	 
	 ##
	-# This is the bar function.
	-def bar
	-  bar stuff
	-end
	-
	-##
	 # This is the baz function.
	 def baz
	   baz stuff

-Peff