improved diff tool

improved diff tool

[Piers Titus van der Torren]

Dear git people,
I've created a diff algorithm that focuses on creating readable diffs,
see https://github.com/pierstitus/klondiff

The git integration as an external diff command works quite well,
though it would be nice to integrate it deeper in git, and also in
git-gui and gitk. Any way to use ext-diff from the gui tools?

Is there interest to incorporate this algorithm in the main git
codebase? And if so, any hints on how to proceed?

Piers

Re: improved diff tool

[Johannes Schindelin]

Hi Piers,

On Thu, 30 Aug 2018, Piers Titus van der Torren wrote:

> I've created a diff algorithm that focuses on creating readable diffs,
> see https://github.com/pierstitus/klondiff

Looks intriguing.

> The git integration as an external diff command works quite well,
> though it would be nice to integrate it deeper in git, and also in
> git-gui and gitk. Any way to use ext-diff from the gui tools?

Git GUI and gitk are both Tcl/Tk programs, and will need quite a bit of
work to accommodate for your diff mode.

To put things into perspective: the `--color-words` mode is not integrated
into Git GUI nor gitk, and it has been around for a while...

> Is there interest to incorporate this algorithm in the main git
> codebase? And if so, any hints on how to proceed?

The best advice I have is to look at the `--color-words` mode. It comes
with its own "consume" function that accumulates lines from the diff, then
outputs them in a different way than the regular colored diff. Your mode
would want to do it very similarly.

This is the accumulating part:

	https://github.com/git/git/blob/v2.19.0-rc1/diff.c#L1886

and this is the display part:

	https://github.com/git/git/blob/v2.19.0-rc1/diff.c#L2013

Basically, I would suggest to do a `git grep color.words` to find the
places where the `--color-words` mode is special-cased, and add new
special-casing for your mode. Which, BTW, I would suggest to find a
catchier name for ;-)

I have not looked closely at your implementation, but I could imagine that
you might want to have at least part of your algorithm step in at a much
lower level: If you can use the patience diff algorithm itself for
pre-processing and initial diff generation, that's great, just force
XDF_PATIENCE_DIFF; Otherwise you will have to implement an alternative,
similar to https://github.com/git/git/blob/v2.19.0-rc1/xdiff/xpatience.c.

Ciao,
Johannes

Re: improved diff tool

[Stefan Beller]

On Thu, Aug 30, 2018 at 4:33 AM Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
>
> Hi Piers,
>
> On Thu, 30 Aug 2018, Piers Titus van der Torren wrote:
>
> > I've created a diff algorithm that focuses on creating readable diffs,
> > see https://github.com/pierstitus/klondiff
>
> Looks intriguing.

Yes it does.

The diff of c07c0923 looks quite interesting as it shows what was
intended to happen (indented a lot of code as it was wrapped).

Playing around with that commit and some recently added switches
  --color-moved --color-moved-ws=allow-indentation-change which is
supposed to solve a similar problem did not yield as good results.

> > The git integration as an external diff command works quite well,
> > though it would be nice to integrate it deeper in git, and also in
> > git-gui and gitk. Any way to use ext-diff from the gui tools?
>
> Git GUI and gitk are both Tcl/Tk programs, and will need quite a bit of
> work to accommodate for your diff mode.

It would be awesome to have gitk/git gui working with more diff options.

>
> To put things into perspective: the `--color-words` mode is not integrated
> into Git GUI nor gitk, and it has been around for a while...

How is it different from the "Color words" mode that sits in the drop down
menu that defaults to "Line diff" ?
https://imgur.com/a/qvo4oOC

(Oh, I realize I had to draw the window wide to see it as it hides easily :/)

> > Is there interest to incorporate this algorithm in the main git
> > codebase? And if so, any hints on how to proceed?

It depends on the the layering,
look at xdiff/xdiffi.c xdl_do_diff() which has

  if (XDF_DIFF_ALG(xpp->flags) == XDF_PATIENCE_DIFF)
    return xdl_do_patience_diff(mf1, mf2, xpp, xe);

  if (XDF_DIFF_ALG(xpp->flags) == XDF_HISTOGRAM_DIFF)
    return xdl_do_histogram_diff(mf1, mf2, xpp, xe);

as that produces the xdiff/ is for producing the diffs.

All the coloring is at a later step, see diff.c (it's a big file),
builtin_diff(), which uses fn_out_consume() as a callback from
the xdiff/ stuff and assembles a git diff around the raw diff.

diff.c is responsible for everything after the raw diff, including
coloring, or getting the format of patches right (such as file names
before a diff, matching up (renamed) files for diffing and such)

> The best advice I have is to look at the `--color-words` mode. It comes
> with its own "consume" function that accumulates lines from the diff, then
> outputs them in a different way than the regular colored diff. Your mode
> would want to do it very similarly.
>
> This is the accumulating part:
>
>         https://github.com/git/git/blob/v2.19.0-rc1/diff.c#L1886
>
> and this is the display part:
>
>         https://github.com/git/git/blob/v2.19.0-rc1/diff.c#L2013
>
> Basically, I would suggest to do a `git grep color.words` to find the
> places where the `--color-words` mode is special-cased, and add new
> special-casing for your mode. Which, BTW, I would suggest to find a
> catchier name for ;-)

AFAICT this is more than just a coloring scheme as it both produces
a different low level diff, which would need code in the xdiff parts
as well as colors, that is in diff.c.

Thanks,
Stefan

Re: improved diff tool

[Christian Couder]

On Thu, Aug 30, 2018 at 7:22 PM, Stefan Beller <sbeller@google.com> wrote:
>
> AFAICT this is more than just a coloring scheme as it both produces
> a different low level diff, which would need code in the xdiff parts
> as well as colors, that is in diff.c.

About the low level diff, Michael Haggerty's tools for experimenting
with diff "slider" heuristics might also help:

https://github.com/mhagger/diff-slider-tools

Re: improved diff tool

[Ævar Arnfjörð Bjarmason]

On Thu, Aug 30 2018, Piers Titus van der Torren wrote:

> Is there interest to incorporate this algorithm in the main git
> codebase? And if so, any hints on how to proceed?

This looks very nice, it would be great to have it in git. I think it's
more useful to focus on getting it into the git C codebase, the user
base of gitk/git-gui is tiny by comparison.

Aside from what others have mentioned, maybe this commit helps to get an
idea of how to do stuff like this: 3cde4e02ee ("diff: retire
"compaction" heuristics", 2016-12-23)

I.e. it's one of our previous diff algorithms being ripped out. Grepping
for the various --diff-algorithm=* options in "man git-diff" in the
source/history should also give good ideas of where to start hooking in.

I see from your name / some brief spying on you via Google that you
might be in the Amsterdam area. I'm not very familiar with the diff part
of the codebase, but if you'd like I'd be happy to meet in person
sometime to help you get started on various other stuff in git.git if
you'd like.