Re: Re* [BUG-ish] diff compaction heuristic false positive

[Jeff King <peff@peff.net>]

On Fri, Jun 10, 2016 at 11:13:10AM -0700, Junio C Hamano wrote:

> Jacob Keller <jacob.keller@gmail.com> writes:
> 
> > I think we could use the indentation trick and it might help in this
> > case. I agree, let's disable this for this cycle and experiment in the
> > next one. Good catch, Peff.
> >
> > As others have said you will always be able to produce counter
> > examples, that's the nature of heuristics. The idea is to see if we
> > can come up with something simple that mostly improves the output,
> > even if sometimes it might have a negative impact on the outputs. But
> > I think we should avoid changing behavior unless it's mostly an
> > improvement.
> 
> OK, let's do this then for the upcoming release for now.  I am
> tempted to flip it back on after the release in 'next', so that
> developers would be exposed to the heuristic by default, though.

Thanks for the patch, and I agree flipping it back on in "next" is a
good idea.

It may be that I am making a fuss over nothing. As you say, we always
knew that it might have false positives. Mostly I was just surprised how
frequent they were in this example (I also wondered why the same pattern
did not trigger in the C code we looked at).

> -- >8 --
> Subject: [PATCH] diff: disable compaction heuristic for now

Looks good.

We probably want a patch to the release notes to note that it's not on
by default. And we may want to advertise the experimental knob so
that people actually try it (otherwise we won't get feedback from the
masses).

-Peff