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

2012-07-24 Thread Tim Henigan 
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 dav...@gmail.com 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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2012-07-24 Thread Junio C Hamano 
Tim Henigan tim.heni...@gmail.com 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 dav...@gmail.com 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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2012-07-23 Thread Junio C Hamano 
David Aguilar dav...@gmail.com writes:

 Teach difftool's --dir-diff mode to use symlinks to represent
 files from the working copy, and make it the default behavior
 for the non-Windows platforms.

 Using symlinks is simpler and safer since we do not need to
 worry about copying files back into the worktree.
 The old behavior is still available as --no-symlinks.

 Signed-off-by: David Aguilar dav...@gmail.com
 ---
 Handles the case where an editor unlinks the original symlink,
 replacing it with a file.

Thanks; will replace.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html