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

Re: [PATCH 2/2] xdiff: implement empty line chunk heuristic

From
Keller, Jacob E <jacob.e.keller@intel.com>
Date
Apr 29, 2016, 22:39 UTC
Message-ID
<1461969582.731.1.camel@intel.com>
In-Reply-To
<CAGZ79kZu=keNaCbt4T=CzH3i9qr+BxXw6AiWR-q1Cs4U80Jzng@mail.gmail.com>
On Fri, 2016-04-29 at 15:35 -0700, Stefan Beller wrote:
Show 72 quoted lines
> On Fri, Apr 29, 2016 at 3:18 PM, Junio C Hamano <gitster@pobox.com>
> wrote:
> > 
> > Jacob Keller <jacob.keller@gmail.com> writes:
> > 
> > > 
> > > On Fri, Apr 29, 2016 at 1:29 PM, Junio C Hamano <gitster@pobox.co
> > > m> wrote:
> > > > 
> > > > Jeff King <peff@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

Previous: Stefan BellerNext: Stefan Beller
Message 12 of 19 in “[PATCHv5 0/2] xdiff: implement empty line chunk heuristic”
  1. Stefan BellerApr 19, 2016
  2. 1/2 xdiff: add recs_match helper functionStefan Beller, Apr 19, 2016
  3. 2/2 xdiff: implement empty line chunk heuristicStefan Beller, Apr 19, 2016
  4. Junio C HamanoApr 19, 2016
  5. Jeff KingApr 20, 2016
  6. Jeff KingApr 20, 2016
  7. Stefan BellerApr 20, 2016
  8. Junio C HamanoApr 29, 2016
  9. Jacob KellerApr 29, 2016
  10. Junio C HamanoApr 29, 2016
  11. Stefan BellerApr 29, 2016
  12. Keller, Jacob EApr 29, 2016
  13. Stefan BellerApr 29, 2016
  14. Keller, Jacob EApr 29, 2016
  15. Junio C HamanoMay 2, 2016
  16. Stefan BellerMay 2, 2016
  17. Jeff KingMay 2, 2016
  18. Jacob KellerMay 3, 2016
  19. Jeff KingApr 30, 2016

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.