From: Junio C Hamano <hidden> Date: 2016-11-01 20:56:41
Jeff King [off-list ref] writes:
On Mon, Oct 31, 2016 at 02:49:42PM -0700, Junio C Hamano wrote:
quoted
* Output from "git diff" can be made easier to read by selecting
which lines are common and which lines are added/deleted
intelligently when the lines before and after the changed section
are the same. A command line option is added to help with the
experiment to find a good heuristics.
I'm not sure we ever resolved all the discussion around options and
defaults here.
As of -rc0, we have both --indent-heuristic and --compaction-heuristic
(along with matching config), and they are mutually exclusive.
In [1], Stefan suggested just replacing the compaction heuristic
entirely with the new one (and you seemed to agree). If we're going to
do that, it makes sense to do so before the release, so that we don't
get stuck supporting --indent-heuristic forever.
You meant "compaction" in the last part? I think it is probably a
good idea.
I'd vote for just removing compaction-heuristic while keeping the
indent-heuristic with experimental label and knobs and keeping it
off by default for a while.
Thanks for bringing it up.
From: Jeff King <hidden> Date: 2016-11-01 20:59:24
On Tue, Nov 01, 2016 at 01:56:34PM -0700, Junio C Hamano wrote:
quoted
As of -rc0, we have both --indent-heuristic and --compaction-heuristic
(along with matching config), and they are mutually exclusive.
In [1], Stefan suggested just replacing the compaction heuristic
entirely with the new one (and you seemed to agree). If we're going to
do that, it makes sense to do so before the release, so that we don't
get stuck supporting --indent-heuristic forever.
You meant "compaction" in the last part? I think it is probably a
good idea.
I thought the plan mentioned in the mail I linked was to keep the indent
heuristic, but simply _call_ it the compaction heuristic. IOW, to swap
out the implementation under the hood for something we know is better.
We've already released a version with --compaction-heuristic, so we are
stuck keeping it forever either way.
I'd vote for just removing compaction-heuristic while keeping the
indent-heuristic with experimental label and knobs and keeping it
off by default for a while.
So the matching variant of that plan would be to drop the internals of
compaction-heuristic, swap in the new heuristic instead, and then drop
all of the --indent-heuristic. It remains off by default, but we may
flip that in a later release.
-Peff
From: Stefan Beller <hidden> Date: 2016-11-01 21:11:05
On Tue, Nov 1, 2016 at 1:59 PM, Jeff King [off-list ref] wrote:
On Tue, Nov 01, 2016 at 01:56:34PM -0700, Junio C Hamano wrote:
quoted
quoted
As of -rc0, we have both --indent-heuristic and --compaction-heuristic
(along with matching config), and they are mutually exclusive.
In [1], Stefan suggested just replacing the compaction heuristic
entirely with the new one (and you seemed to agree). If we're going to
do that, it makes sense to do so before the release, so that we don't
get stuck supporting --indent-heuristic forever.
You meant "compaction" in the last part? I think it is probably a
good idea.
I thought the plan mentioned in the mail I linked was to keep the indent
heuristic, but simply _call_ it the compaction heuristic. IOW, to swap
out the implementation under the hood for something we know is better.
AFAICT Michaels design is better in every aspect than what I did initially,
so it supersedes the work I did there. I would support the swap in names.
We've already released a version with --compaction-heuristic, so we are
stuck keeping it forever either way.
IIRC the release notes specifically noted this flag to be experimental and
may be removed in future versions.
When not doing the swap of the implementation, but rather remove the
experimental feature of compaction-heuristic and introducing a *new*
experimental --indent-heuristic, this may drive the point across that
these names are actually experimental.
From: Jacob Keller <hidden> Date: 2016-11-02 01:22:12
On Tue, Nov 1, 2016 at 2:10 PM, Stefan Beller [off-list ref] wrote:
On Tue, Nov 1, 2016 at 1:59 PM, Jeff King [off-list ref] wrote:
quoted
On Tue, Nov 01, 2016 at 01:56:34PM -0700, Junio C Hamano wrote:
quoted
quoted
As of -rc0, we have both --indent-heuristic and --compaction-heuristic
(along with matching config), and they are mutually exclusive.
In [1], Stefan suggested just replacing the compaction heuristic
entirely with the new one (and you seemed to agree). If we're going to
do that, it makes sense to do so before the release, so that we don't
get stuck supporting --indent-heuristic forever.
You meant "compaction" in the last part? I think it is probably a
good idea.
I thought the plan mentioned in the mail I linked was to keep the indent
heuristic, but simply _call_ it the compaction heuristic. IOW, to swap
out the implementation under the hood for something we know is better.
AFAICT Michaels design is better in every aspect than what I did initially,
so it supersedes the work I did there. I would support the swap in names.
Agreed, it's much better than the original idea, and results in better
diffs in every single case we could find.
quoted
We've already released a version with --compaction-heuristic, so we are
stuck keeping it forever either way.
IIRC the release notes specifically noted this flag to be experimental and
may be removed in future versions.
I agree, I think that we specifically spelled out that this might go
away, and so I don't think we're stuck supporting it forever. We don't
even really need a deprecation time frame either.
When not doing the swap of the implementation, but rather remove the
experimental feature of compaction-heuristic and introducing a *new*
experimental --indent-heuristic, this may drive the point across that
these names are actually experimental.
I think we should swap names as "compaction heuristic" is more generic.
Thanks,
Jake