Re: [PATCH] diff: prefer indent heuristic over compaction heuristic
- From
Jacob Keller <jacob.keller@gmail.com>
- Date
- Dec 22, 2016, 21:41 UTC
- Message-ID
- <CA+P7+xrkp-qiUVmfeLUcaMP-RSDbH4u3vCjVoQN8=mhz25Cd3A@mail.gmail.com>
- In-Reply-To
- <xmqqinqbfz2r.fsf@gitster.mtv.corp.google.com>
On Thu, Dec 22, 2016 at 1:12 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 16 quoted lines
> Jacob Keller <jacob.keller@gmail.com> writes: > >> I don't think we have too many config options that interact in this >> way, so I understand that "last writing of a particular configuration" >> makes sense, but interactions between configs is something that would >> have never occurred to me. I'll send a patch to drop the compaction >> heuristic since I think we're all 100% in agreement that it is >> superseded by the new configuration (as no case has been shown where >> the new one is worse than compaction, and most show it to be better). > > If I recall correctly, we agreed that we'll drop the implementation > of compaction, but use the name --compaction-heuristics to trigger > the new and improved "indent heuristics": > > <20161101205916.d74n6lhgp2hexpzr@sigill.intra.peff.net> >
That sounds familiar.
Show 69 quoted lines
> So let's do this.
>
> -- >8 --
> Subject: [PATCH] diff: retire the original experimental "compaction" heuristics
>
> This retires the experimental "compaction" heuristics but with a
> twist. It removes the mention of "indent" heuristics, which was a
> competing experiment, from everywhere, guts the core logic of the
> original "compaction" heuristics out and replaces it with the logic
> used by the "indent" heuristics.
>
> The externally visible effect of this change is that people who have
> been experimenting by setting diff.compactionHeuristic configuration
> or giving the command line option --compaction-heuristic will start
> getting the behaviour based on the improved heuristics that used to
> be called "indent" heuristics.
>
> Suggested-by: Jeff King <peff@peff.net>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> Documentation/diff-config.txt | 6 ++---
> Documentation/diff-heuristic-options.txt | 2 --
> builtin/blame.c | 3 +--
> diff.c | 25 ++++-----------------
> git-add--interactive.perl | 5 +----
> t/t4061-diff-indent.sh | 38 ++++++++++++++++----------------
> xdiff/xdiff.h | 1 -
> xdiff/xdiffi.c | 37 +++----------------------------
> 8 files changed, 30 insertions(+), 87 deletions(-)
>
> diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt
> index 58f4bd6afa..39fff3aef9 100644
> --- a/Documentation/diff-config.txt
> +++ b/Documentation/diff-config.txt
> @@ -171,11 +171,9 @@ diff.tool::
>
> include::mergetools-diff.txt[]
>
> -diff.indentHeuristic::
> diff.compactionHeuristic::
> - Set one of these options to `true` to enable one of two
> - experimental heuristics that shift diff hunk boundaries to
> - make patches easier to read.
> + Set this option to `true` to enable experimental heuristics
> + that shift diff hunk boundaries to make patches easier to read.
>
> diff.algorithm::
> Choose a diff algorithm. The variants are as follows:
> diff --git a/Documentation/diff-heuristic-options.txt b/Documentation/diff-heuristic-options.txt
> index 36cb549df9..3cb024aa22 100644
> --- a/Documentation/diff-heuristic-options.txt
> +++ b/Documentation/diff-heuristic-options.txt
> @@ -1,5 +1,3 @@
> ---indent-heuristic::
> ---no-indent-heuristic::
> --compaction-heuristic::
> --no-compaction-heuristic::
> These are to help debugging and tuning experimental heuristics
> diff --git a/builtin/blame.c b/builtin/blame.c
> index 4ddfadb71f..395d4011fb 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -2596,7 +2596,6 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
> * and are only included here to get included in the "-h"
> * output:
> */
> - { OPTION_LOWLEVEL_CALLBACK, 0, "indent-heuristic", NULL, NULL, N_("Use an experimental indent-based heuristic to improve diffs"), PARSE_OPT_NOARG, parse_opt_unknown_cb },
> { OPTION_LOWLEVEL_CALLBACK, 0, "compaction-heuristic", NULL, NULL, N_("Use an experimental blank-line-based heuristic to improve diffs"), PARSE_OPT_NOARG, parse_opt_unknown_cb },
>The unchanged context line should have its description re-worded to something like "Use an experimental heuristic to improve diffs" as it no longer uses only blank lines.
Everything else looked correct.
Thanks, Jake