Re: [PATCH v1 06/25] contrib: remove 'diffall'

2014-05-09 Thread Tim Henigan
On Thu, May 8, 2014 at 5:58 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
 No activity since 2010, no tests.

 Cc: Tim Henigan tim.heni...@gmail.com
 Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
 ---
  contrib/diffall/README  |  31 --
  contrib/diffall/git-diffall | 257 
 
  2 files changed, 288 deletions(-)
  delete mode 100644 contrib/diffall/README
  delete mode 100755 contrib/diffall/git-diffall

I see no problem with removing this script from contrib.  However, the
commit message
should mention that git-difftool learned all the features of
git-diffall when the '--dir-diff'
option was added in v1.7.11 (ca. June 2012).

Also, the script was first added to contrib in Feb. 2012, so no
activity since 2010 is
incorrect.  I'm not sure this information is useful in the commit
message at all.
--
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 v1 06/25] contrib: remove 'diffall'

2014-05-09 Thread Tim Henigan
Hi Jonathan,

On Fri, May 9, 2014 at 11:50 AM, Jonathan Nieder jrnie...@gmail.com wrote:
 Hi Tim,

 Tim Henigan wrote:
 On Thu, May 8, 2014 at 5:58 PM, Felipe Contreras
 felipe.contre...@gmail.com wrote:

  contrib/diffall/README  |  31 --
  contrib/diffall/git-diffall | 257 
 
  2 files changed, 288 deletions(-)
  delete mode 100644 contrib/diffall/README
  delete mode 100755 contrib/diffall/git-diffall

 I see no problem with removing this script from contrib.  However, the 
 commit message
 should mention that git-difftool learned all the features of git-diffall 
 when the '--dir-diff'
 option was added in v1.7.11 (ca. June 2012).

 A few questions:

  * Do you still use git-diffall?  Since it hasn't been a maintenance
burden, I wouldn't mind keeping it if it still has users.

I have not used diffall since a few months after the difftool '--dir-diff'
option was released.  Once difftool learned those features, the
diffall script became obsolete.

For people using older git (i.e. before v1.7.11), it may still be useful.
For them, the original out-of-tree repo remains available on github [1].

[1]: https://github.com/thenigan/git-diffall


  * Any thoughts about how to help people who have been using it to
migrate to difftool?  Would a note in the release notes to look
into the --dir-diff option to difftool be enough, or are there
more specific pointers that could be useful?

Pointing to 'difftool --dir-diff' should be enough.

The only change in behavior is that when a working tree file is part
of the diff and is modified during the diff, 'difftool --dir-diff' automatically
keeps the modifications.  The 'diffall' script required the user to use
the '--copy-back' option to do the same.

All other options are exactly the same.


 Once those questions are dealt with, this seems like a nice small
 cleanup.  Thanks for the quick feedback.

If it would be helpful, I can send a patch that replaces the contents
of 'contrib/diffall' with a README that explains the above and points
to the github repo for people using versions of git prior to v1.7.11.
This would be similar to what was done for 'contrib/vim'.

However, I like the idea of simply removing the directory from contrib
and pointing to 'difftool --dir-diff' in the release notes.
--
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 v2 04/17] contrib: remove 'diffall'

2014-05-09 Thread Tim Henigan
On Fri, May 9, 2014 at 12:11 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
 There is no more need for this tool since the --dir-dirr option was
 introduced.

s/--dir-dirr/--dir-diff/
--
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 v1 06/25] contrib: remove 'diffall'

2014-05-09 Thread Tim Henigan
Hi Jonathan,

On Fri, May 9, 2014 at 1:12 PM, Jonathan Nieder jrnie...@gmail.com wrote:
 Hi,

 Tim Henigan wrote:

 However, I like the idea of simply removing the directory from contrib
 and pointing to 'difftool --dir-diff' in the release notes.

 Thanks.  I believe Junio uses commit messages as reference when
 writing release notes so hopefully this should be enough to make that
 happen.

The updated commit message looks good to me.  Thanks.

-Tim
--
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 1/5] difftool: Simplify print_tool_help()

2012-07-24 Thread Tim Henigan
On Sun, Jul 22, 2012 at 11:42 PM, David Aguilar dav...@gmail.com wrote:
 Eliminate a global variable and File::Find usage by building upon
 basename() and glob() instead.

glob was used in an early revision of the patch that led to bf73fc2
(difftool: print list of valid tools with '--tool-help') as well [1].
However, if the path to git or the path under 'mergetools' includes
spaces, glob fails.  To work around the problem, File::Find was used
instead [2].

Does this implementation handle that case?  I'm sorry, but I haven't
had time to apply and test myself.

[1]: http://thread.gmane.org/gmane.comp.version-control.git/193233/focus=193925
[2]: http://thread.gmane.org/gmane.comp.version-control.git/194158
--
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 v3 4/5] difftool: Use symlinks when diffing against the worktree

2012-07-24 Thread Tim Henigan
I'm sorry I am so late to see and comment on this...I am just getting
caught up after a few busy weeks due to $dayjob and vacation.


On Mon, Jul 23, 2012 at 2:05 AM, David Aguilar dav...@gmail.com wrote:

 diff --git a/git-difftool.perl b/git-difftool.perl
 index 2ae344c..a5b371f 100755
 --- a/git-difftool.perl
 +++ b/git-difftool.perl

 @@ -271,6 +276,7 @@ sub main
 gui = undef,
 help = undef,
 prompt = undef,
 +   symlinks = $^O ne 'MSWin32'  $^O ne 'msys',

Should this test for cygwin as well?


 @@ -342,13 +350,18 @@ sub dir_diff

 # If the diff including working copy files and those
 # files were modified during the diff, then the changes
 -   # should be copied back to the working tree
 -   for my $file (@working_tree) {
 -   if (-e $b/$file  compare($b/$file, $workdir/$file)) {
 +   # should be copied back to the working tree.
 +   # Do not copy back files when symlinks are used and the
 +   # external tool did not replace the original link with a file.
 +   for my $file (@worktree) {
 +   next if $symlinks  -l $b/$file;
 +   if (-f $b/$file  compare($b/$file, $workdir/$file)) {

compare returns '-1' if an error is encountered while reading a file.
In this (unlikely) case, should it still overwrite the working copy
file?  I think the answer is 'yes', but thought it was worth
mentioning.
--
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