[PATCH] difftool --dir-diff: copy back all files matching the working tree

Subsystems: the rest

DORMANTno replies

3 messages, 2 authors, 2016-06-15 · open the first message on its own page

[PATCH] difftool --dir-diff: copy back all files matching the working tree

From: Kenichi Saita <hidden>
Date: 2016-06-15 22:57:27

After running the user's diff tool, "git difftool --dir-dif --no-symlink"
currently copied back a temporary file to working tree only when a file
contains unstaged changes in the working tree.

Change this behavior so that temporary files are copied back to working
tree whenever the right-hand side of the comparison has the same SHA1
as the file in the working tree.

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

-- 
1.7.1

Re: [PATCH] difftool --dir-diff: copy back all files matching the working tree

From: John Keeping <hidden>
Date: 2016-06-15 22:57:27

On Mon, May 27, 2013 at 12:00:46AM +0900, Kenichi Saita wrote:

After running the user's diff tool, "git difftool --dir-dif --no-symlink"
currently copied back a temporary file to working tree only when a file
contains unstaged changes in the working tree.

Change this behavior so that temporary files are copied back to working
tree whenever the right-hand side of the comparison has the same SHA1
as the file in the working tree.

Signed-off-by: Kenichi Saita <redacted>

This change looks sensible to me, but I found the commit message quite
confusing.

The code being changed here is to do with choosing whether to copy the
working tree file to the temporary directory (or symlink it) and hence
only indirectly related to whether it will be copied back.  It might be
clearer to phrase it like this:

    difftool --dir-diff: always use identical working tree file

    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.

    Change this so that identical files are copied across 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.

quoted hunk

---
 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

-- 
1.7.1

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

From: Kenichi Saita <hidden>
Date: 2016-06-15 22:57:27

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.

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.

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

-- 
1.7.1

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help