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

2013-05-26 Thread Kenichi Saita
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 nito...@gmail.com
---
 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

--
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] difftool --dir-diff: copy back all files matching the working tree

2013-05-26 Thread John Keeping
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 nito...@gmail.com

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.

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