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

[John Keeping]

On Wed, Mar 13, 2013 at 08:41:29PM -0700, David Aguilar wrote:
 On Wed, Mar 13, 2013 at 1:33 PM, John Keeping j...@keeping.me.uk wrote:
  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?

I followed what some of the other uses of write_script (in other tests)
did.  I think putting it under .git is slightly better because it won't
show up as untracked in the repository but that shouldn't matter here,
so I'm happy to change it in a re-roll.

  +#!/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 .

I hadn't considered using test_cmp, if we go that way I wonder if we can
do slightly better for future debugging.  Something like this perhaps?

+write_script .git/CHECK_SYMLINKS \EOF 
+for f in file file2 sub/sub
+do
+   echo $f
+   readlink $2/$f
+done actual
+EOF
+
+test_expect_success PERL,SYMLINKS 'difftool --dir-diff --symlink without 
unstaged changes' '
+   cat EOF expect 
+file
+$(pwd)/file
+file2
+$(pwd)/file2
+sub/sub
+$(pwd)/sub/sub
+EOF
+   git difftool --dir-diff --symlink \
+   --extcmd ./.git/CHECK_SYMLINKS branch HEAD 
+   test_cmp actual expect
+'

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

Yes it is based on master.  The cleanup on next looks good, I'll base
the re-roll on that.


John
--
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 2/2] difftool --dir-diff: symlink all files matching the working tree

[Junio C Hamano]

John Keeping j...@keeping.me.uk writes:

 +write_script .git/CHECK_SYMLINKS \EOF 
 +#!/bin/sh
 +test -L $2/file 
 +test -L $2/file2 
 +test -L $2/sub/sub
 +echo $?
 +EOF

Please drop #!/bin/sh from the above; it is misleading and
pointless.

After all, you are using write_script to avoid having to know
where the user's shell is.
--
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