On Fri, 2016-04-29 at 15:35 -0700, Stefan Beller wrote: > On Fri, Apr 29, 2016 at 3:18 PM, Junio C Hamano <gits...@pobox.com> > wrote: > > > > Jacob Keller <jacob.kel...@gmail.com> writes: > > > > > > > > On Fri, Apr 29, 2016 at 1:29 PM, Junio C Hamano <gits...@pobox.co > > > m> wrote: > > > > > > > > Jeff King <p...@peff.net> writes: > > > > > > > > > > > > > > ... Having the two directly next to each other reads > > > > > better to me. This is a pretty unusual diff, though, in that > > > > > it did > > > > > change the surrounding whitespace (and if you look further in > > > > > the diff, > > > > > the identical change is made elsewhere _without_ touching the > > > > > whitespace). So this is kind of an anomaly. And IMHO the > > > > > weirdness here > > > > > is outweighed by the vast number of improvements elsewhere. > > > > So... is everybody happy with the result and now we can drop > > > > the > > > > tweaking knob added to help experimentation before merging the > > > > result to 'master'? > > > > > > > > I am pretty happy with the end result myself. > > > I am very happy with it. I haven't had any issues, and I think > > > we'll > > > find better traction by enabling it at this point and seeing > > > when/if > > > someone complains. > > > > > > I think for most it won't be noticed and for those that do it > > > will > > > likely be positive. > > I am doing this only to prepare in case we have a concensus, > > i.e. this is not to declare that I do not care what other people > > say. Here is a patch to remove the experimentation knob. > > > > Let's say we keep this patch out of tree for now and keep the topic > > in 'next' so that people can further play with it for several more > > weeks, and then apply this on top and merge the result to 'master' > > early in the next cycle. > > > > -- >8 -- > > diff: enable "compaction heuristics" and lose experimentation knob > > > > It seems that the new "find a good hunk boundary by locating a > > blank > > line" heuristics gives much more pleasant result without much > > noticeable downsides. Let's make it the new algorithm for real, > > without the opt-out knob we added while experimenting with it. > I would remove the opt-out knob much later in the game, i.e. > > 1) make a patch that removes the documentation only > before the next release (i.e. before 2.9) > 2) make a patch to remove the actual (unlabeled) knobs, > merge into master before 2.10 (i.e. just after the 2.9 > release) > > Then we get the most of the community to test it with the 2.9 release > and still have an emergency knob in case some major headaches > show up. After one release cycle we'll be much more confident > about its usage and its short comings and do not need the > emergency turn off. If the community doesn't like it for some reason > we can document it and debate the default setting? > > I agree we want the knob gone eventually. > Making it an undocumented feature is as good as that from > a users point of view? >
Currently it's an "opt in" knob, so this doesn't make sense to me. If we remove the entire knob as is, we can always (fairly easily) add it back. I would keep the code inside xdiff as a knob, but set it to enable default so that the user config has no knob at the top level but the xdiff machinery does (this making a "disable" be relatively small patch). Thanks, Jake
