Re: [PATCH v2] difftool --dir-diff: always use identical working tree file

On Tue, May 28, 2013 at 11:06:13AM -0700, Junio C Hamano wrote:

Kenichi Saita [off-list ref] writes:

quoted

When deciding whether or not we should link a working tree file into
the temporary right-hand directory for a directory diff, we
currently behave differently in the --symlink and --no-symlink
cases.  If using symlinks any identical files are linked across but
with --no-symlink only files that contain unstaged changes are
copied back into the working tree.

I may have missed an earlier discussion, but I do not follow the
last sentence.  The former part (i.e. symlinks) talks about what is
done to populate the temporary tree (i.e. everything is linked), but
the latter part (i.e. not symlinks) only talks about what is copied
back, i.e. it is not a contrast between the behaviour of symlink vs
no-symlink case wrt how the temporary tree is populated.

Confused...

Yeah, the commit message is still quite focused on the end effect of
copying files back.  But that's not what's being changed here.

In my suggested commit message I tried to make it clear that we're
changing when we decide to copy a file across to the temporary tree.
This has the beneficial (side-)effect of changing the set of files we
consider for copying back into the working tree after the diff tool has
been run.

quoted

Change this so that identical files are copied back as well.  This
is beneficial because it widens the set of circumstances in which we
copy changes made by the user back into the working tree.

Ah, OK, you meant that the set of files we keep in @working_tree
array for later copying back are different between the two.

quoted

Signed-off-by: Kenichi Saita <redacted>
---
 git-difftool.perl   |    9 ++-------
 t/t7800-difftool.sh |   19 +++++++++++++++++++
 2 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/git-difftool.perl b/git-difftool.perl
index 8a75205..e57d3d1 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl

@@ -85,13 +85,9 @@ sub exit_cleanup
 
 sub use_wt_file
 {
-	my ($repo, $workdir, $file, $sha1, $symlinks) = @_;
+	my ($repo, $workdir, $file, $sha1) = @_;
 	my $null_sha1 = '0' x 40;
 
-	if ($sha1 ne $null_sha1 and not $symlinks) {
-		return 0;
-	}
-
 	if (! -e "$workdir/$file") {
 		# If the file doesn't exist in the working tree, we cannot
 		# use it.

@@ -213,8 +209,7 @@ EOF
 
 		if ($rmode ne $null_mode) {
 			my ($use, $wt_sha1) = use_wt_file($repo, $workdir,
-							  $dst_path, $rsha1,
-							  $symlinks);
+							  $dst_path, $rsha1);
 			if ($use) {
 				push @working_tree, $dst_path;
 				$wtindex .= "$rmode $wt_sha1\t$dst_path\0";

diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index d46f041..2418528 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh

@@ -385,6 +385,25 @@ test_expect_success PERL,SYMLINKS 'difftool --dir-diff --symlink without unstage
 	test_cmp actual expect
 '
 
+write_script modify-right-file <<\EOF
+echo "new content" >"$2/file"
+EOF
+
+run_dir_diff_test 'difftool --dir-diff syncs worktree with unstaged change' '
+	test_when_finished git reset --hard &&
+	echo "orig content" >file &&
+	git difftool -d $symlinks --extcmd "$(pwd)/modify-right-file" branch &&
+	echo "new content" >expect &&
+	test_cmp expect file
+'
+
+run_dir_diff_test 'difftool --dir-diff syncs worktree without unstaged change' '
+	test_when_finished git reset --hard &&
+	git difftool -d $symlinks --extcmd "$(pwd)/modify-right-file" branch &&
+	echo "new content" >expect &&
+	test_cmp expect file
+'
+
 write_script modify-file <<\EOF
 echo "new content" >file
 EOF

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