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

From: Keller, Jacob E <jacob.e.keller@intel.com>
Date: Apr 29, 2016, 22:48 UTC
Message-ID: <1461970113.731.3.camel@intel.com>
In-Reply-To: <CAGZ79kYx22oYobPxMkC03fGk-E9zaZZd2f+qafESkhcmFog7-w@mail.gmail.com>

On Fri, 2016-04-29 at 15:44 -0700, Stefan Beller wrote:

> > 
> > Currently it's an "opt in" knob, so this doesn't make sense to me.
> +static int diff_compaction_heuristic = 1;
>

Oops didn't know we'd made it default at some point. (all my versions had it disabled by default)

> It's rather an opt-out knob going by the current
> origin/jk/diff-compact-heuristic
>

Yea in that case, we could keep it.

Show 22 quoted lines

> 
> > 
> > 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).
> When writing my reply, I thought about people using Git from a binary
> distribution with little to no admin rights. They want to have an
> emergency
> knob to disable this thing, but cannot patch/recompile Git.
> 
> If you can patch and compile your version of Git, then reverting is
> easy, so
> in that case Junios patch looks good to me.
> 
> Thanks,
> Stefan

True. I think the chances that it needs such a thing are quite minor, and if an undocumented knob gets exposed it would have to become documented and maintained, so I'd prefer to avoid it. Given that the risk is pretty small I think that's ok.

Thanks, Jake