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

2012-07-24 Thread Junio C Hamano
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.
--
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 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  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-23 Thread Junio C Hamano
David Aguilar  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 
> ---
> 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


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

2012-07-22 Thread David Aguilar
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 
---
Handles the case where an editor unlinks the original symlink,
replacing it with a file.

 Documentation/git-difftool.txt |8 
 git-difftool.perl  |   33 +++--
 2 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt
index 31fc2e3..313d54e 100644
--- a/Documentation/git-difftool.txt
+++ b/Documentation/git-difftool.txt
@@ -66,6 +66,14 @@ of the diff post-image.  `$MERGED` is the name of the file 
which is
 being compared. `$BASE` is provided for compatibility
 with custom merge tool commands and has the same value as `$MERGED`.
 
+--symlinks::
+--no-symlinks::
+   'git difftool''s default behavior is create symlinks to the
+   working tree when run in `--dir-diff` mode.
++
+   Specifying `--no-symlinks` instructs 'git difftool' to create
+   copies instead.  `--no-symlinks` is the default on Windows.
+
 --tool-help::
Print a list of diff tools that may be used with `--tool`.
 
diff --git a/git-difftool.perl b/git-difftool.perl
index 2ae344c..a5b371f 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -92,7 +92,7 @@ sub print_tool_help
 
 sub setup_dir_diff
 {
-   my ($repo, $workdir) = @_;
+   my ($repo, $workdir, $symlinks) = @_;
 
# Run the diff; exit immediately if no diff found
# 'Repository' and 'WorkingCopy' must be explicitly set to insure that
@@ -209,8 +209,13 @@ sub setup_dir_diff
unless (-d "$rdir/$dir") {
mkpath("$rdir/$dir") or die $!;
}
-   copy("$workdir/$file", "$rdir/$file") or die $!;
-   chmod(stat("$workdir/$file")->mode, "$rdir/$file") or die $!;
+   if ($symlinks) {
+   symlink("$workdir/$file", "$rdir/$file") or die $!;
+   } else {
+   copy("$workdir/$file", "$rdir/$file") or die $!;
+   my $mode = stat("$workdir/$file")->mode;
+   chmod($mode, "$rdir/$file") or die $!;
+   }
}
 
# Changes to submodules require special treatment. This loop writes a
@@ -271,6 +276,7 @@ sub main
gui => undef,
help => undef,
prompt => undef,
+   symlinks => $^O ne 'MSWin32' && $^O ne 'msys',
tool_help => undef,
);
GetOptions('g|gui!' => \$opts{gui},
@@ -278,6 +284,8 @@ sub main
'h' => \$opts{help},
'prompt!' => \$opts{prompt},
'y' => sub { $opts{prompt} = 0; },
+   'symlinks' => \$opts{symlinks},
+   'no-symlinks' => sub { $opts{symlinks} = 0; },
't|tool:s' => \$opts{difftool_cmd},
'tool-help' => \$opts{tool_help},
'x|extcmd:s' => \$opts{extcmd});
@@ -316,7 +324,7 @@ sub main
# will invoke a separate instance of 'git-difftool--helper' for
# each file that changed.
if (defined($opts{dirdiff})) {
-   dir_diff($opts{extcmd});
+   dir_diff($opts{extcmd}, $opts{symlinks});
} else {
file_diff($opts{prompt});
}
@@ -324,13 +332,13 @@ sub main
 
 sub dir_diff
 {
-   my ($extcmd) = @_;
+   my ($extcmd, $symlinks) = @_;
 
my $rc;
my $repo = Git->repository();
 
my $workdir = find_worktree($repo);
-   my ($a, $b, @working_tree) = setup_dir_diff($repo, $workdir);
+   my ($a, $b, @worktree) = setup_dir_diff($repo, $workdir, $symlinks);
if (defined($extcmd)) {
$rc = system($extcmd, $a, $b);
} else {
@@ -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")) {
copy("$b/$file", "$workdir/$file") or die $!;
-   chmod(stat("$b/$file")->mode, "$workdir/$file") or die 
$!;
+   my $mode = stat("$b/$file")->mode;
+   chmod($mode, "$workdir/$fil