On Thu, Apr 14, 2016 at 2:05 PM, Stefan Beller <sbeller@xxxxxxxxxx> wrote:
> On Thu, Apr 14, 2016 at 11:34 AM, Jeff King <peff@xxxxxxxx> wrote:
>> 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()
>
>
>>
>> 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?
>
Interesting. I'll play around with that for a bit and see how fast it appears.
Thanks,
Jake
>>
>> -Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html