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

From: Tim Henigan <tim.henigan@gmail.com>
Date: Apr 10, 2012, 15:01 UTC
Message-ID: <CAFouetiBMK+WWU9yOLaO5SLyUO1K_S-YW0LTt5tE5UTcMM8K=w@mail.gmail.com>
In-Reply-To: <CAJDDKr76eMiA4rOHQhar3aToVThDfbc8Ki5tr2PGU_UpDMAVeA@mail.gmail.com>

Hi David,

Thank you for the review. I am working on the changes you suggested, but want to discuss one of them:

On Mon, Apr 9, 2012 at 8:14 AM, David Aguilar <davvid@gmail.com> wrote:

Show 9 quoted lines

> On Wed, Apr 4, 2012 at 12:21 PM, Tim Henigan <tim.henigan@gmail.com> wrote:
>> +
>> +       # If the diff including working copy files and those
>> +       # files were modified during the diff, then the changes
>> +       # should be copied back to the working tree
>> +       my $repo = Git->repository();
>> +       my $workdir = $repo->repo_path() . "/..";
>
> Does this work when $GIT_WORK_TREE / core.worktree are defined?

I had not considered this case and the existing code will certainly ignore alternate work tree locations. To fix the problem, I plan to add something like this:

+sub find_worktree
+{
+       my $worktree;
+       my $env_worktree = $ENV{GIT_WORK_TREE};
+       my $core_worktree = Git::config('core.worktree');
+
+       if (length($env_worktree) > 0) {
+               $worktree = $env_worktree;
+       } elsif (length($core_worktree) > 0) {
+               $worktree = $core_worktree;
+       } else {
+               my $repo = Git->repository();
+               $worktree = $repo->repo_path() . "/..";
+       }
+
+       return $worktree;
+}

Does this look like a reasonable solution? I don't typically use '$GIT_WORK_TREE' or 'core.worktree'. The above function works for me, but I would appreciate some feedback before I send the next revision of the patch (v12!).

Thanks, Tim