Re: [PATCH 8/9 v13] difftool: teach difftool to handle directory diffs

From: Tim Henigan <tim.henigan@gmail.com>
Date: Apr 18, 2012, 13:13 UTC
Message-ID: <CAFouetjbHewYzQXZr33xGKgwk0k7D8R0XfoP7k2qAV6Nq_d+Ow@mail.gmail.com>
In-Reply-To: <CAJDDKr6djdBvUbV6qZZu75iR2UbFHt8_D0+V+K_C+-Dgx8BfVA@mail.gmail.com>

On Tue, Apr 17, 2012 at 11:23 PM, David Aguilar <davvid@gmail.com> wrote:

Show 31 quoted lines

> On Tue, Apr 17, 2012 at 6:25 AM, Tim Henigan <tim.henigan@gmail.com> wrote:
>> On Sun, Apr 15, 2012 at 9:01 PM, David Aguilar <davvid@gmail.com> wrote:
>>> On Sun, Apr 15, 2012 at 3:20 PM, David Aguilar <davvid@gmail.com> wrote:
>>>> On Fri, Apr 13, 2012 at 9:36 AM, Tim Henigan <tim.henigan@gmail.com> wrote:
>
> I think I narrowed it down.  I have this in my ~/.gitconfig
> --
> [diff]
>    renames = copy
> --
> That means we should be able to reproduce this by doing:
>
>    $ git difftool --dir-diff -M -C baf5aaa33383af656a34b7ba9039e9eb3c9e678c
>
> That ends up calling `git diff --raw -M -C
> baf5aaa33383af656a34b7ba9039e9eb3c9e678c`, whose output contains:
>
> ...[snip]...
> :000000 100755 0000000... 05824fa... A  t/lib-gpg.sh
> :100644 100644 83855fa... 83855fa... R100       t/t7004/pubring.gpg
>  t/lib-gpg/pubring.gpg
> :100644 100644 8fed133... 8fed133... R100       t/t7004/random_seed
>  t/lib-gpg/random_seed
> :100644 100644 d831cd9... d831cd9... R100       t/t7004/secring.gpg
>  t/lib-gpg/secring.gpg
> :100644 100644 abace96... abace96... R100       t/t7004/trustdb.gpg
>  t/lib-gpg/trustdb.gpg
> :100644 100644 3f24384... f7dc078... M  t/lib-httpd.sh
> ...[snip]...
>
> I suspect the R100 lines are the ones that are throwing it off.

It is indeed the lines that show the renames/copies that cause the error. Adding the '-C' or '-M' option to 'git diff --raw' changes the diff output for copied/modified files from:

    :<mode> <mode> <SHA1> <SHA1> <status> <file>
to
    :<mode> <mode> <SHA1> <SHA1> <status> <file before> <file after>

The difftool code assumes that the diff output will be of the first form and fails when given the second form.

So now we must decide how to handle deal with this use case. It seems there are two options:

1) Append '--no-renames' to the end of the 'git diff --raw' argument
list.  This will override any '-C' or '-M' settings.  This is a simple
solution, but it loses some information about copies and renames.

2) Add new logic to parse copies and renames.  Your earlier email
advocated this approach, but I am concerned that the implementation
will include some tough choices.

So, I would like to simply amend the current patch to include solution
1.  Solution 2 can be considered in the future.

Does this sound reasonable?