From: Tim Henigan <tim.henigan@gmail.com>
Date: Wed, 18 Apr 2012 13:13:02 GMT
Subject: Re: [PATCH 8/9 v13] difftool: teach difftool to handle directory diffs
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:
> 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?