Thread (10 messages) flat view 10 messages, 3 authors, 2016-06-15

Re: [PATCH 2/2] difftool --dir-diff: symlink all files matching the working tree

From: David Aguilar <hidden>
Date: 2016-06-15 22:56:23

Possibly related (same subject, not in this thread)

On Wed, Mar 13, 2013 at 1:33 PM, John Keeping [off-list ref] wrote:
quoted hunk ↗ jump to hunk
Some users like to edit files in their diff tool when using "git
difftool --dir-diff --symlink" to compare against the working tree but
difftool currently only created symlinks when a file contains unstaged
changes.

Change this behaviour so that symlinks are created whenever the
right-hand side of the comparison has the same SHA1 as the file in the
working tree.

Note that textconv filters are handled in the same way as by git-diff
and if a clean filter is not the inverse of its smudge filter we already
get a null SHA1 from "diff --raw" and will symlink the file without
going through the new hash-object based check.

Reported-by: Matt McClure <redacted>
Signed-off-by: John Keeping <redacted>
---
 Documentation/git-difftool.txt |  4 +++-
 git-difftool.perl              | 21 ++++++++++++++++++---
 t/t7800-difftool.sh            | 14 ++++++++++++++
 3 files changed, 35 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt
index e575fea..8361e6e 100644
--- a/Documentation/git-difftool.txt
+++ b/Documentation/git-difftool.txt
@@ -72,7 +72,9 @@ 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.
+       working tree when run in `--dir-diff` mode and the right-hand
+       side of the comparison yields the same content as the file in
+       the working tree.
 +
 Specifying `--no-symlinks` instructs 'git difftool' to create copies
 instead.  `--no-symlinks` is the default on Windows.
diff --git a/git-difftool.perl b/git-difftool.perl
index 0a90de4..5f093ae 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -83,6 +83,21 @@ sub exit_cleanup
        exit($status | ($status >> 8));
 }

+sub use_wt_file
+{
+       my ($repo, $workdir, $file, $sha1, $symlinks) = @_;
+       my $null_sha1 = '0' x 40;
+
+       if ($sha1 eq $null_sha1) {
+               return 1;
+       } elsif (not $symlinks) {
+               return 0;
+       }
+
+       my $wt_sha1 = $repo->command_oneline('hash-object', "$workdir/$file");
+       return $sha1 eq $wt_sha1;
+}
+
 sub setup_dir_diff
 {
        my ($repo, $workdir, $symlinks) = @_;
@@ -159,10 +174,10 @@ EOF
                }

                if ($rmode ne $null_mode) {
-                       if ($rsha1 ne $null_sha1) {
-                               $rindex .= "$rmode $rsha1\t$dst_path\0";
-                       } else {
+                       if (use_wt_file($repo, $workdir, $dst_path, $rsha1, $symlinks)) {
                                push(@working_tree, $dst_path);
+                       } else {
+                               $rindex .= "$rmode $rsha1\t$dst_path\0";
                        }
                }
        }
diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index eb1d3f8..8102ce1 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -370,6 +370,20 @@ test_expect_success PERL 'difftool --dir-diff' '
        echo "$diff" | stdin_contains file
 '

+write_script .git/CHECK_SYMLINKS <<\EOF &&
Tiny nit.  Is there any downside to leaving this file
at the root instead of inside the .git dir?
+#!/bin/sh
+test -L "$2/file" &&
+test -L "$2/file2" &&
+test -L "$2/sub/sub"
+echo $?
+EOF
+
+test_expect_success PERL,SYMLINKS 'difftool --dir-diff --symlink without unstaged changes' '
+       result=$(git difftool --dir-diff --symlink \
+               --extcmd "./.git/CHECK_SYMLINKS" branch HEAD) &&
+       test "$result" = 0
+'
+
How about something like this?

+       echo 0 >expect &&
+       git difftool --dir-diff --symlink \
+               --extcmd ./CHECK_SYMLINKS branch HEAD >actual &&
+       test_cmp expect actual

(sans gmail whitespace damage) so that we can keep it chained with &&.
Ah.. it seems your branch is based on master, perhaps?

There's stuff cooking in next for difftool's tests.
I'm not sure if this patch is based on top of them.
Can you rebase the tests so that the chaining is done like it is in 'next'?
-- 
David
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help