Re: [PATCH v3 4/5] difftool: Use symlinks when diffing against the worktree

From: Tim Henigan <tim.henigan@gmail.com>
Date: Jul 24, 2012, 13:35 UTC
Message-ID: <CAFouethz=7Ed-=AsnC2Fh+-hZcpjzmSLB9aCWd4tBw8GvH1K-w@mail.gmail.com>
In-Reply-To: <1343023530-31463-1-git-send-email-davvid@gmail.com>

I'm sorry I am so late to see and comment on this...I am just getting caught up after a few busy weeks due to $dayjob and vacation.

On Mon, Jul 23, 2012 at 2:05 AM, David Aguilar <davvid@gmail.com> wrote:

Show 11 quoted lines

>
> diff --git a/git-difftool.perl b/git-difftool.perl
> index 2ae344c..a5b371f 100755
> --- a/git-difftool.perl
> +++ b/git-difftool.perl
>
> @@ -271,6 +276,7 @@ sub main
>                 gui => undef,
>                 help => undef,
>                 prompt => undef,
> +               symlinks => $^O ne 'MSWin32' && $^O ne 'msys',

Should this test for cygwin as well?

Show 13 quoted lines

> @@ -342,13 +350,18 @@ sub dir_diff
>
>         # 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
> -       for my $file (@working_tree) {
> -               if (-e "$b/$file" && compare("$b/$file", "$workdir/$file")) {
> +       # should be copied back to the working tree.
> +       # Do not copy back files when symlinks are used and the
> +       # external tool did not replace the original link with a file.
> +       for my $file (@worktree) {
> +               next if $symlinks && -l "$b/$file";
> +               if (-f "$b/$file" && compare("$b/$file", "$workdir/$file")) {

compare returns '-1' if an error is encountered while reading a file. In this (unlikely) case, should it still overwrite the working copy file? I think the answer is 'yes', but thought it was worth mentioning.