Re: weird diff output?

From: Stefan Beller <sbeller@google.com>
Date: Apr 14, 2016, 21:05 UTC
Message-ID: <CAGZ79ka8pgPNZKaVWnsa_S07esxkN9nJfhcMZvCfd5U6MtsrYQ@mail.gmail.com>
In-Reply-To: <20160414183405.GE22068@sigill.intra.peff.net>

On Thu, Apr 14, 2016 at 11:34 AM, Jeff King <peff@peff.net> wrote:

Show 17 quoted lines

> On Thu, Apr 14, 2016 at 06:56:39AM -0700, Davide Libenzi wrote:
>
>> That was a zillions of years ago :) , but from a quick look at email
>> thread, if you want to do it within xdiff, xdi_change_compact would be
>> the place.  The issue is knowing in which situations one diff look
>> better than another, and embedding an if-tis-do-tat logic deep into
>> the core diff machinery.  In theory one could implement the same thing
>> higher up, working with the unified diff text format, where maybe a
>> user can provide its own diff post-process hook script.  In any case,
>> that still leaves open the issue on what to shift in the diff chunks,
>> and in which cases. Which is likely going to be language/format
>> dependent. IMHO, it gets nasty pretty quickly.
>
> Thanks, that's helpful. Stefan already came up with a heuristic that I
> implemented as a post-processing script in perl. It _seems_ to work
> pretty well in practice across multiple languages, so our next step was
> to implement it in an actual usable and efficient way. :)

To reiterate the heuristic for Davide (so you can avoid reading the whole thread):

    If there are diff chunks, which can be shifted around, shift it such that
    the last empty line is below the chunk and the rest above.

Example: (indented, shiftable part marked with Xs)

        diff --git a/test.c b/test.c
        index 2d7f343..2a14d36 100644
        --- a/test.c
        +++ b/test.c
        @@ -8,6 +8,14 @@ void A()
         }

         /**
        + * This is text.
        + */
        +void B()
        +{
        +  text text
X1      +}
X2      +
X3      +/**
          * This does 'foo foo'.
          */
         void C()

The last empty line is X2, so that's where we wrap: (X2 is the last line of the diff)

        diff --git a/test.c b/test.c
        index 2d7f343..2a14d36 100644
        --- a/test.c
        +++ b/test.c
        @@ -8,6 +8,14 @@ void A()
         }

X3      +/**
        + * This is text.
        + */
        +void B()
        +{
        +  text text
X1      +}
X2      +
         /**
          * This does 'foo foo'.
          */
         void C()

Show 5 quoted lines

>
> Looking over the code, I agree that xdl_change_compact() is the place we
> would want to put it. We'd probably tie it to a command-line option and
> let people play around with it, and then consider making it the default
> if there's widespread approval.

I just stumbled upon http://blog.scoutapp.com/articles/2016/04/12/3-git-productivity-hacks which advertises git config --global pager.diff "diff-so-fancy | less --tabs=4 -RFX"

Would you consider your perl script good enough to put that instead of diff-so-fancy?

>
> -Peff