From: Junio C Hamano Date: Tue, 24 Jul 2012 15:57:10 GMT Subject: Re: [PATCH v3 4/5] difftool: Use symlinks when diffing against the worktree Message-ID: <7v7gttgnjd.fsf@alter.siamese.dyndns.org> In-Reply-To: Tim Henigan writes: > 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 wrote: >> >> 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? > > >> @@ -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. It probably is safer to report the error, not touch anything and let the user take an appropriate action.