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

Re: [RFC PATCH, WAS: "weird diff output?" 2/2] xdiff: implement empty line chunk heuristic

From
Stefan Beller <sbeller@google.com>
Date
Apr 15, 2016, 19:57 UTC
Message-ID
<CAGZ79ka7h25=rHun_hPv1qjqeghXt1UwUU3Q6xT0aj4+OW87fg@mail.gmail.com>
In-Reply-To
<20160415165141.4712-3-jacob.e.keller@intel.com>
On Fri, Apr 15, 2016 at 9:51 AM, Jacob Keller <jacob.e.keller@intel.com> wrote:
Show 69 quoted lines
> From: Stefan Beller <sbeller@google.com>
>
> In order to produce the smallest possible diff and combine several diff
> hunks together, we implement a heuristic from GNU Diff which moves diff
> hunks forward as far as possible when we find common context above and
> below a diff hunk. This sometimes produces less readable diffs when
> writing C, Shell, or other programming languages, ie:
>
> ...
>  /*
> + *
> + *
> + */
> +
> +/*
> ...
>
> instead of the more readable equivalent of
>
> ...
> +/*
> + *
> + *
> + */
> +
>  /*
> ...
>
> Implement the following heuristic to (optionally) produce the desired
> output.
>
>   If there are diff chunks which can be shifted around, shift each hunk
>   such that the last common empty line is below the chunk with the rest
>   of the context above.
>
> This heuristic appears to resolve the above example and several other
> common issues without producing significantly weird results. However, as
> with any heuristic it is not really known whether this will always be
> more optimal. Thus, leave the heuristic disabled by default.
>
> Add an XDIFF flag to enable this heuristic only conditionally. Add
> a diff command line option and diff configuration option to allow users
> to enable this option when desired.
>
> TODO:
> * Add tests
> * Add better/more documentation explaining the heuristic, possibly with
>   examples(?)
> * better name(?)
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
>  Documentation/diff-config.txt  |  6 ++++++
>  Documentation/diff-options.txt |  6 ++++++
>  diff.c                         | 11 +++++++++++
>  xdiff/xdiff.h                  |  2 ++
>  xdiff/xdiffi.c                 | 26 ++++++++++++++++++++++++++
>  5 files changed, 51 insertions(+)
>
> diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt
> index edba56522bce..cebf82702d2a 100644
> --- a/Documentation/diff-config.txt
> +++ b/Documentation/diff-config.txt
> @@ -170,6 +170,12 @@ diff.tool::
>
>  include::mergetools-diff.txt[]
>
> +diff.emptyLineHeuristic::

I was looking at the TODO here and thought about the name: It should not encode the `emptyLine` into the config option as it is only one of many heuristics.

It should be something like `diff.heuristic=lastEmptyLine` The we could add firstEmptyLine, aggressiveUp, aggressiveDown, breakAtShortestLineLength or other styles as well later on.

I do not quite understand the difference between diff.algorithm and the newly proposed heuristic as the heuristic is part of the algorithm? So I guess we'd need to have some documentation saying how these differ. (fundamental algorithm vs last minute style fixup?)

Previous: Jacob KellerNext: Junio C Hamano
Message 6 of 22 in “implement better chunk heuristics”
  1. 0/2 implement better chunk heuristicsJacob Keller, Apr 15, 2016
  2. 1/2 xdiff: add xdl_hash_and_recmatch helper functionJacob Keller, Apr 15, 2016
  3. Junio C HamanoApr 15, 2016
  4. Jacob KellerApr 15, 2016
  5. 2/2 xdiff: implement empty line chunk heuristicJacob Keller, Apr 15, 2016
  6. Stefan BellerApr 15, 2016
  7. Junio C HamanoApr 15, 2016
  8. Jacob KellerApr 15, 2016
  9. Junio C HamanoApr 15, 2016
  10. Jacob KellerApr 15, 2016
  11. Jacob KellerApr 15, 2016
  12. Junio C HamanoApr 15, 2016
  13. Jacob KellerApr 15, 2016
  14. Jacob KellerApr 15, 2016
  15. Stefan BellerApr 15, 2016
  16. Stefan BellerApr 15, 2016
  17. Junio C HamanoApr 15, 2016
  18. Stefan BellerApr 15, 2016
  19. Junio C HamanoApr 15, 2016
  20. Jacob KellerApr 15, 2016
  21. Jacob KellerApr 15, 2016
  22. Jacob KellerApr 15, 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.