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

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

Previous: Junio C HamanoNext: Junio C Hamano
Message 5 of 17 in “diff: prefer indent heuristic over compaction heuristic”
  1. diff: prefer indent heuristic over compaction heuristicJacob Keller, Dec 17, 2016
  2. Junio C HamanoDec 17, 2016
  3. Jacob KellerDec 17, 2016
  4. Junio C HamanoDec 22, 2016
  5. Jacob KellerDec 22, 2016
  6. Junio C HamanoDec 22, 2016
  7. Jeff KingDec 23, 2016
  8. Jacob KellerDec 23, 2016
  9. Jeff KingDec 23, 2016
  10. Junio C HamanoDec 23, 2016
  11. Junio C HamanoDec 23, 2016
  12. Jeff KingDec 23, 2016
  13. Jacob KellerDec 23, 2016
  14. Michael HaggertyDec 24, 2016
  15. Stefan BellerApr 27, 2017
  16. Jeff KingApr 28, 2017
  17. Martin LiškaApr 28, 2017

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.