* [PATCH 1/9 v2] difftool: parse options using Getopt::Long
2012-03-18 1:55 ` [PATCH 0/9 v2] " Tim Henigan
@ 2012-03-18 1:55 ` Tim Henigan
2012-03-18 1:55 ` [PATCH 2/9 v2] difftool: exit(0) when usage is printed Tim Henigan
` (8 subsequent siblings)
9 siblings, 0 replies; 26+ messages in thread
From: Tim Henigan @ 2012-03-18 1:55 UTC (permalink / raw)
To: gitster, davvid, git; +Cc: Tim Henigan
Replace custom option/argument parser with standard Getopt::Long
module. This shortens the code and makes it easier to understand.
Signed-off-by: Tim Henigan <tim.henigan@gmail.com>
---
git-difftool.perl | 109 +++++++++++++++++++++--------------------------------
1 file changed, 44 insertions(+), 65 deletions(-)
diff --git a/git-difftool.perl b/git-difftool.perl
index 09b65f1..e365727 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -15,11 +15,8 @@ use strict;
use warnings;
use Cwd qw(abs_path);
use File::Basename qw(dirname);
-
-require Git;
-
-my $DIR = abs_path(dirname($0));
-
+use Getopt::Long qw(:config pass_through);
+use Git;
sub usage
{
@@ -33,6 +30,7 @@ USAGE
sub setup_environment
{
+ my $DIR = abs_path(dirname($0));
$ENV{PATH} = "$DIR:$ENV{PATH}";
$ENV{GIT_PAGER} = '';
$ENV{GIT_EXTERNAL_DIFF} = 'git-difftool--helper';
@@ -47,75 +45,56 @@ sub exe
return $exe;
}
-sub generate_command
-{
- my @command = (exe('git'), 'diff');
- my $skip_next = 0;
- my $idx = -1;
- my $prompt = '';
- for my $arg (@ARGV) {
- $idx++;
- if ($skip_next) {
- $skip_next = 0;
- next;
- }
- if ($arg eq '-t' || $arg eq '--tool') {
- usage() if $#ARGV <= $idx;
- $ENV{GIT_DIFF_TOOL} = $ARGV[$idx + 1];
- $skip_next = 1;
- next;
- }
- if ($arg =~ /^--tool=/) {
- $ENV{GIT_DIFF_TOOL} = substr($arg, 7);
- next;
- }
- if ($arg eq '-x' || $arg eq '--extcmd') {
- usage() if $#ARGV <= $idx;
- $ENV{GIT_DIFFTOOL_EXTCMD} = $ARGV[$idx + 1];
- $skip_next = 1;
- next;
- }
- if ($arg =~ /^--extcmd=/) {
- $ENV{GIT_DIFFTOOL_EXTCMD} = substr($arg, 9);
- next;
- }
- if ($arg eq '-g' || $arg eq '--gui') {
- eval {
- my $tool = Git::command_oneline('config',
- 'diff.guitool');
- if (length($tool)) {
- $ENV{GIT_DIFF_TOOL} = $tool;
- }
- };
- next;
- }
- if ($arg eq '-y' || $arg eq '--no-prompt') {
- $prompt = 'no';
- next;
- }
- if ($arg eq '--prompt') {
- $prompt = 'yes';
- next;
- }
- if ($arg eq '-h') {
- usage();
- }
- push @command, $arg;
+# parse command-line options. all unrecognized options and arguments
+# are passed through to the 'git diff' command.
+my ($difftool_cmd, $extcmd, $gui, $help, $no_prompt, $prompt);
+GetOptions('g|gui' => \$gui,
+ 'h' => \$help,
+ 'prompt' => \$prompt,
+ 't|tool:s' => \$difftool_cmd,
+ 'x|extcmd:s' => \$extcmd,
+ 'y|no-prompt' => \$no_prompt);
+
+if (defined($help)) {
+ usage();
+}
+if (defined($difftool_cmd)) {
+ if (length($difftool_cmd) > 0) {
+ $ENV{GIT_DIFF_TOOL} = $difftool_cmd;
+ } else {
+ print "No <tool> given for --tool=<tool>\n";
+ usage();
}
- if ($prompt eq 'yes') {
- $ENV{GIT_DIFFTOOL_PROMPT} = 'true';
- } elsif ($prompt eq 'no') {
- $ENV{GIT_DIFFTOOL_NO_PROMPT} = 'true';
+}
+if (defined($extcmd)) {
+ if (length($extcmd) > 0) {
+ $ENV{GIT_DIFFTOOL_EXTCMD} = $extcmd;
+ } else {
+ print "No <cmd> given for --extcmd=<cmd>\n";
+ usage();
+ }
+}
+if (defined($gui)) {
+ my $guitool = "";
+ $guitool = Git::config('diff.guitool');
+ if (length($guitool) > 0) {
+ $ENV{GIT_DIFF_TOOL} = $guitool;
}
- return @command
+}
+if (defined($prompt)) {
+ $ENV{GIT_DIFFTOOL_PROMPT} = 'true';
+}
+elsif (defined($no_prompt)) {
+ $ENV{GIT_DIFFTOOL_NO_PROMPT} = 'true';
}
setup_environment();
+my @command = (exe('git'), 'diff', @ARGV);
# ActiveState Perl for Win32 does not implement POSIX semantics of
# exec* system call. It just spawns the given executable and finishes
# the starting program, exiting with code 0.
# system will at least catch the errors returned by git diff,
# allowing the caller of git difftool better handling of failures.
-my $rc = system(generate_command());
+my $rc = system(@command);
exit($rc | ($rc >> 8));
--
1.7.9.1.290.gbd444
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 2/9 v2] difftool: exit(0) when usage is printed
2012-03-18 1:55 ` [PATCH 0/9 v2] " Tim Henigan
2012-03-18 1:55 ` [PATCH 1/9 v2] difftool: parse options using Getopt::Long Tim Henigan
@ 2012-03-18 1:55 ` Tim Henigan
2012-03-18 1:55 ` [PATCH 3/9 v2] difftool: remove explicit change of PATH Tim Henigan
` (7 subsequent siblings)
9 siblings, 0 replies; 26+ messages in thread
From: Tim Henigan @ 2012-03-18 1:55 UTC (permalink / raw)
To: gitster, davvid, git; +Cc: Tim Henigan
Prior to this commit, the script exited with an error whenever the
usage string was printed, regardless of the reason it was done. In
cases where usage was printed due to a user request (e.g. '-h'
option), the script should exit without error (exit 0).
This commit adds an argument to the usage function that allows the
exit code to be specified when the function is called.
Signed-off-by: Tim Henigan <tim.henigan@gmail.com>
---
git-difftool.perl | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/git-difftool.perl b/git-difftool.perl
index e365727..81ecf34 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -20,12 +20,13 @@ use Git;
sub usage
{
+ my $exitcode = shift;
print << 'USAGE';
usage: git difftool [-t|--tool=<tool>] [-x|--extcmd=<cmd>]
[-y|--no-prompt] [-g|--gui]
['git diff' options]
USAGE
- exit 1;
+ exit($exitcode);
}
sub setup_environment
@@ -56,14 +57,14 @@ GetOptions('g|gui' => \$gui,
'y|no-prompt' => \$no_prompt);
if (defined($help)) {
- usage();
+ usage(0);
}
if (defined($difftool_cmd)) {
if (length($difftool_cmd) > 0) {
$ENV{GIT_DIFF_TOOL} = $difftool_cmd;
} else {
print "No <tool> given for --tool=<tool>\n";
- usage();
+ usage(1);
}
}
if (defined($extcmd)) {
@@ -71,7 +72,7 @@ if (defined($extcmd)) {
$ENV{GIT_DIFFTOOL_EXTCMD} = $extcmd;
} else {
print "No <cmd> given for --extcmd=<cmd>\n";
- usage();
+ usage(1);
}
}
if (defined($gui)) {
--
1.7.9.1.290.gbd444
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 3/9 v2] difftool: remove explicit change of PATH
2012-03-18 1:55 ` [PATCH 0/9 v2] " Tim Henigan
2012-03-18 1:55 ` [PATCH 1/9 v2] difftool: parse options using Getopt::Long Tim Henigan
2012-03-18 1:55 ` [PATCH 2/9 v2] difftool: exit(0) when usage is printed Tim Henigan
@ 2012-03-18 1:55 ` Tim Henigan
2012-03-18 1:55 ` [PATCH 4/9 v2] difftool: stop appending '.exe' to git Tim Henigan
` (6 subsequent siblings)
9 siblings, 0 replies; 26+ messages in thread
From: Tim Henigan @ 2012-03-18 1:55 UTC (permalink / raw)
To: gitster, davvid, git; +Cc: Tim Henigan
Adding the script directory to PATH is not needed. The script is
located at '$(git --exec-path)', which is already on the PATH.
Signed-off-by: Tim Henigan <tim.henigan@gmail.com>
---
git-difftool.perl | 4 ----
1 file changed, 4 deletions(-)
diff --git a/git-difftool.perl b/git-difftool.perl
index 81ecf34..53fcd7e 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -13,8 +13,6 @@
use 5.008;
use strict;
use warnings;
-use Cwd qw(abs_path);
-use File::Basename qw(dirname);
use Getopt::Long qw(:config pass_through);
use Git;
@@ -31,8 +29,6 @@ USAGE
sub setup_environment
{
- my $DIR = abs_path(dirname($0));
- $ENV{PATH} = "$DIR:$ENV{PATH}";
$ENV{GIT_PAGER} = '';
$ENV{GIT_EXTERNAL_DIFF} = 'git-difftool--helper';
}
--
1.7.9.1.290.gbd444
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 4/9 v2] difftool: stop appending '.exe' to git
2012-03-18 1:55 ` [PATCH 0/9 v2] " Tim Henigan
` (2 preceding siblings ...)
2012-03-18 1:55 ` [PATCH 3/9 v2] difftool: remove explicit change of PATH Tim Henigan
@ 2012-03-18 1:55 ` Tim Henigan
2012-03-18 1:55 ` [PATCH 5/9 v2] difftool: eliminate setup_environment function Tim Henigan
` (5 subsequent siblings)
9 siblings, 0 replies; 26+ messages in thread
From: Tim Henigan @ 2012-03-18 1:55 UTC (permalink / raw)
To: gitster, davvid, git; +Cc: Tim Henigan
The system call to Git works the same whether or not ".exe" is
appended to "git". The extra code is not necessary.
Signed-off-by: Tim Henigan <tim.henigan@gmail.com>
---
git-difftool.perl | 11 +----------
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/git-difftool.perl b/git-difftool.perl
index 53fcd7e..99ff587 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -33,15 +33,6 @@ sub setup_environment
$ENV{GIT_EXTERNAL_DIFF} = 'git-difftool--helper';
}
-sub exe
-{
- my $exe = shift;
- if ($^O eq 'MSWin32' || $^O eq 'msys') {
- return "$exe.exe";
- }
- return $exe;
-}
-
# parse command-line options. all unrecognized options and arguments
# are passed through to the 'git diff' command.
my ($difftool_cmd, $extcmd, $gui, $help, $no_prompt, $prompt);
@@ -86,7 +77,7 @@ elsif (defined($no_prompt)) {
}
setup_environment();
-my @command = (exe('git'), 'diff', @ARGV);
+my @command = ('git', 'diff', @ARGV);
# ActiveState Perl for Win32 does not implement POSIX semantics of
# exec* system call. It just spawns the given executable and finishes
--
1.7.9.1.290.gbd444
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 5/9 v2] difftool: eliminate setup_environment function
2012-03-18 1:55 ` [PATCH 0/9 v2] " Tim Henigan
` (3 preceding siblings ...)
2012-03-18 1:55 ` [PATCH 4/9 v2] difftool: stop appending '.exe' to git Tim Henigan
@ 2012-03-18 1:55 ` Tim Henigan
2012-03-18 1:55 ` [PATCH 6/9 v2] difftool: replace system call with Git::command_noisy Tim Henigan
` (4 subsequent siblings)
9 siblings, 0 replies; 26+ messages in thread
From: Tim Henigan @ 2012-03-18 1:55 UTC (permalink / raw)
To: gitster, davvid, git; +Cc: Tim Henigan
Removing this function shortens the code and makes it easier to read.
Now all environment variables are set as part of procedural operation.
Signed-off-by: Tim Henigan <tim.henigan@gmail.com>
---
git-difftool.perl | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/git-difftool.perl b/git-difftool.perl
index 99ff587..9495f14 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -27,12 +27,6 @@ USAGE
exit($exitcode);
}
-sub setup_environment
-{
- $ENV{GIT_PAGER} = '';
- $ENV{GIT_EXTERNAL_DIFF} = 'git-difftool--helper';
-}
-
# parse command-line options. all unrecognized options and arguments
# are passed through to the 'git diff' command.
my ($difftool_cmd, $extcmd, $gui, $help, $no_prompt, $prompt);
@@ -76,7 +70,8 @@ elsif (defined($no_prompt)) {
$ENV{GIT_DIFFTOOL_NO_PROMPT} = 'true';
}
-setup_environment();
+$ENV{GIT_PAGER} = '';
+$ENV{GIT_EXTERNAL_DIFF} = 'git-difftool--helper';
my @command = ('git', 'diff', @ARGV);
# ActiveState Perl for Win32 does not implement POSIX semantics of
--
1.7.9.1.290.gbd444
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 6/9 v2] difftool: replace system call with Git::command_noisy
2012-03-18 1:55 ` [PATCH 0/9 v2] " Tim Henigan
` (4 preceding siblings ...)
2012-03-18 1:55 ` [PATCH 5/9 v2] difftool: eliminate setup_environment function Tim Henigan
@ 2012-03-18 1:55 ` Tim Henigan
2012-03-18 1:55 ` [PATCH 7/9 v2] difftool: teach difftool to handle directory diffs Tim Henigan
` (3 subsequent siblings)
9 siblings, 0 replies; 26+ messages in thread
From: Tim Henigan @ 2012-03-18 1:55 UTC (permalink / raw)
To: gitster, davvid, git; +Cc: Tim Henigan
The Git.pm module includes functions intended to standardize working
with Git repositories in Perl scripts. This commit teaches difftool
to use Git::command_noisy rather than a system call to run the diff
command.
Signed-off-by: Tim Henigan <tim.henigan@gmail.com>
---
git-difftool.perl | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/git-difftool.perl b/git-difftool.perl
index 9495f14..8498089 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -72,12 +72,4 @@ elsif (defined($no_prompt)) {
$ENV{GIT_PAGER} = '';
$ENV{GIT_EXTERNAL_DIFF} = 'git-difftool--helper';
-my @command = ('git', 'diff', @ARGV);
-
-# ActiveState Perl for Win32 does not implement POSIX semantics of
-# exec* system call. It just spawns the given executable and finishes
-# the starting program, exiting with code 0.
-# system will at least catch the errors returned by git diff,
-# allowing the caller of git difftool better handling of failures.
-my $rc = system(@command);
-exit($rc | ($rc >> 8));
+git_cmd_try { Git::command_noisy(('diff', @ARGV)) } 'exit code %d';
--
1.7.9.1.290.gbd444
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 7/9 v2] difftool: teach difftool to handle directory diffs
2012-03-18 1:55 ` [PATCH 0/9 v2] " Tim Henigan
` (5 preceding siblings ...)
2012-03-18 1:55 ` [PATCH 6/9 v2] difftool: replace system call with Git::command_noisy Tim Henigan
@ 2012-03-18 1:55 ` Tim Henigan
2012-03-18 1:55 ` [PATCH 8/9 v2] difftool: teach dir-diff to copy modified files back to working tree Tim Henigan
` (2 subsequent siblings)
9 siblings, 0 replies; 26+ messages in thread
From: Tim Henigan @ 2012-03-18 1:55 UTC (permalink / raw)
To: gitster, davvid, git; +Cc: Tim Henigan
Prior to this commit, the difftool utility could only open files one
at a time. The new '--dir-diff' option copies all the modified files
to a temporary location and runs a directory diff on them.
Signed-off-by: Tim Henigan <tim.henigan@gmail.com>
---
Changes in v2:
- Location of /tmp is now determined via the File::Temp::tempdir. In
v1, the location was hard-coded to '/tmp'.
- Lexical filehandles are now used for open().
- Modern, 3 argument, form of open() is now used.
- Arguments to the 'system' call for $extcmd are now given in list form.
- Replaced 'system' call to 'git difftool--helper' with Git::command_noisy
Documentation/git-difftool.txt | 6 ++
git-difftool--helper.sh | 20 ++++--
git-difftool.perl | 144 ++++++++++++++++++++++++++++++++++++----
3 files changed, 152 insertions(+), 18 deletions(-)
diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt
index fe38f66..aba5e76 100644
--- a/Documentation/git-difftool.txt
+++ b/Documentation/git-difftool.txt
@@ -19,6 +19,12 @@ linkgit:git-diff[1].
OPTIONS
-------
+-d::
+--dir-diff::
+ Copy the modified files to a temporary location and perform
+ a directory diff on them. This mode never prompts before
+ launching the diff tool.
+
-y::
--no-prompt::
Do not prompt before launching a diff tool.
diff --git a/git-difftool--helper.sh b/git-difftool--helper.sh
index e6558d1..bd0556f 100755
--- a/git-difftool--helper.sh
+++ b/git-difftool--helper.sh
@@ -6,6 +6,7 @@
# Copyright (c) 2009, 2010 David Aguilar
TOOL_MODE=diff
+SUBDIRECTORY_OK=1
. git-mergetool--lib
# difftool.prompt controls the default prompt/no-prompt behavior
@@ -73,9 +74,16 @@ then
fi
fi
-# Launch the merge tool on each path provided by 'git diff'
-while test $# -gt 6
-do
- launch_merge_tool "$1" "$2" "$5"
- shift 7
-done
+if test -n "$GIT_DIFFTOOL_DIRDIFF"
+then
+ LOCAL="$1"
+ REMOTE="$2"
+ run_merge_tool "$merge_tool" false
+else
+ # Launch the merge tool on each path provided by 'git diff'
+ while test $# -gt 6
+ do
+ launch_merge_tool "$1" "$2" "$5"
+ shift 7
+ done
+fi
diff --git a/git-difftool.perl b/git-difftool.perl
index 8498089..1f7b8f2 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -1,18 +1,24 @@
#!/usr/bin/env perl
# Copyright (c) 2009, 2010 David Aguilar
+# Copyright (c) 2012 Tim Henigan
#
# This is a wrapper around the GIT_EXTERNAL_DIFF-compatible
# git-difftool--helper script.
#
# This script exports GIT_EXTERNAL_DIFF and GIT_PAGER for use by git.
-# GIT_DIFFTOOL_NO_PROMPT, GIT_DIFFTOOL_PROMPT, and GIT_DIFF_TOOL
-# are exported for use by git-difftool--helper.
+# GIT_DIFFTOOL_NO_PROMPT, GIT_DIFFTOOL_PROMPT, GIT_DIFFTOOL_DIRDIFF,
+# and GIT_DIFF_TOOL are exported for use by git-difftool--helper.
#
# Any arguments that are unknown to this script are forwarded to 'git diff'.
use 5.008;
use strict;
use warnings;
+use File::Basename qw(dirname);
+use File::Copy;
+use File::stat;
+use File::Path qw(mkpath);
+use File::Temp qw(tempdir);
use Getopt::Long qw(:config pass_through);
use Git;
@@ -22,15 +28,109 @@ sub usage
print << 'USAGE';
usage: git difftool [-t|--tool=<tool>] [-x|--extcmd=<cmd>]
[-y|--no-prompt] [-g|--gui]
+ [-d|--dir-diff]
['git diff' options]
USAGE
exit($exitcode);
}
+sub setup_dir_diff
+{
+ # Run the diff; exit immediately if no diff found
+ my $repo = Git->repository();
+ my $diffrtn = $repo->command_oneline(['diff', '--raw', '--no-abbrev', '-z', @ARGV]);
+ exit(0) if (length($diffrtn) == 0);
+
+ # Setup temp directories
+ my $tmpdir = tempdir('git-diffall.XXXXX', CLEANUP => 1, TMPDIR => 1);
+ my $ldir = "$tmpdir/left";
+ my $rdir = "$tmpdir/right";
+ mkpath($ldir) or die $!;
+ mkpath($rdir) or die $!;
+
+ # Build index info for left and right sides of the diff
+ my $submodule_mode = "160000";
+ my $null_mode = 0 x 6;
+ my $null_sha1 = 0 x 40;
+ my $lindex = "";
+ my $rindex = "";
+ my @working_tree;
+ my %submodule;
+ my @rawdiff = split('\0', $diffrtn);
+
+ for (my $i=0; $i<@rawdiff; $i+=2) {
+ my ($lmode, $rmode, $lsha1, $rsha1, $status) = split(' ', substr($rawdiff[$i], 1));
+ my $path = $rawdiff[$i + 1];
+
+ if (($lmode eq $submodule_mode) or ($rmode eq $submodule_mode)) {
+ $submodule{$path}{left} = $lsha1;
+ $submodule{$path}{right} = $rsha1;
+ next;
+ }
+
+ if ($lmode ne $null_mode) {
+ $lindex .= "$lmode $lsha1\t$path\0";
+ }
+
+ if ($rmode ne $null_mode) {
+ if ($rsha1 ne $null_sha1) {
+ $rindex .= "$rmode $rsha1\t$path\0";
+ } else {
+ push(@working_tree, $path);
+ }
+ }
+ }
+
+ # Populate the left and right directories based on each index file
+ my ($inpipe, $ctx);
+ $ENV{GIT_INDEX_FILE} = "$tmpdir/lindex";
+ ($inpipe, $ctx) = $repo->command_input_pipe(qw/update-index -z --index-info/);
+ print($inpipe $lindex);
+ $repo->command_close_pipe($inpipe, $ctx);
+ $repo->command_oneline(["checkout-index", "-a", "--prefix=$ldir/"]);
+
+ $ENV{GIT_INDEX_FILE} = "$tmpdir/rindex";
+ ($inpipe, $ctx) = $repo->command_input_pipe(qw/update-index -z --index-info/);
+ print($inpipe $rindex);
+ $repo->command_close_pipe($inpipe, $ctx);
+ $repo->command_oneline(["checkout-index", "-a", "--prefix=$rdir/"]);
+
+ # Changes in the working tree need special treatment since they are
+ # not part of the index
+ my $workdir = $repo->wc_path();
+ for (@working_tree) {
+ my $dir = dirname($_);
+ unless (-d "$rdir/$dir") {
+ mkpath("$rdir/$dir") or die $!;
+ }
+ copy("$workdir/$_", "$rdir/$_") or die $!;
+ chmod(stat("$workdir/$_")->mode, "$rdir/$_") or die $!;
+ }
+
+ # Changes to submodules require special treatment. This loop writes a
+ # temporary file to both the left and right directories to show the
+ # change in the recorded SHA1 for the submodule.
+ foreach my $path (keys %submodule) {
+ if (defined $submodule{$path}{left}) {
+ open(my $fh, ">", "$ldir/$path") or die $!;
+ print($fh "Subproject commit $submodule{$path}{left}");
+ close($fh);
+ }
+ if (defined $submodule{$path}{right}) {
+ open(my $fh, ">", "$rdir/$path") or die $!;
+ print($fh "Subproject commit $submodule{$path}{right}");
+ close($fh);
+ }
+ }
+
+ return ($ldir, $rdir);
+}
+
# parse command-line options. all unrecognized options and arguments
# are passed through to the 'git diff' command.
-my ($difftool_cmd, $extcmd, $gui, $help, $no_prompt, $prompt);
+my ($difftool_cmd, $dirdiff, $extcmd, $gui, $help, $no_prompt, $prompt);
GetOptions('g|gui' => \$gui,
+ 'd|dir-diff' => \$dirdiff,
'h' => \$help,
'prompt' => \$prompt,
't|tool:s' => \$difftool_cmd,
@@ -63,13 +163,33 @@ if (defined($gui)) {
$ENV{GIT_DIFF_TOOL} = $guitool;
}
}
-if (defined($prompt)) {
- $ENV{GIT_DIFFTOOL_PROMPT} = 'true';
-}
-elsif (defined($no_prompt)) {
- $ENV{GIT_DIFFTOOL_NO_PROMPT} = 'true';
-}
-$ENV{GIT_PAGER} = '';
-$ENV{GIT_EXTERNAL_DIFF} = 'git-difftool--helper';
-git_cmd_try { Git::command_noisy(('diff', @ARGV)) } 'exit code %d';
+# In directory diff mode, 'git-difftool--helper' is called once
+# to compare the a/b directories. In file diff mode, 'git diff'
+# will invoke a separate instance of 'git-difftool--helper' for
+# each file that changed.
+if (defined($dirdiff)) {
+ my ($a, $b) = setup_dir_diff();
+ if (defined($extcmd)) {
+ system(($extcmd, $a, $b));
+ } else {
+ $ENV{GIT_DIFFTOOL_DIRDIFF} = 'true';
+ git_cmd_try {
+ Git::command_noisy(('difftool--helper', $a, $b))
+ } 'exit code %d';
+ }
+ # TODO: 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
+} else {
+ if (defined($prompt)) {
+ $ENV{GIT_DIFFTOOL_PROMPT} = 'true';
+ }
+ elsif (defined($no_prompt)) {
+ $ENV{GIT_DIFFTOOL_NO_PROMPT} = 'true';
+ }
+
+ $ENV{GIT_PAGER} = '';
+ $ENV{GIT_EXTERNAL_DIFF} = 'git-difftool--helper';
+ git_cmd_try { Git::command_noisy(('diff', @ARGV)) } 'exit code %d';
+}
--
1.7.9.1.290.gbd444
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 8/9 v2] difftool: teach dir-diff to copy modified files back to working tree
2012-03-18 1:55 ` [PATCH 0/9 v2] " Tim Henigan
` (6 preceding siblings ...)
2012-03-18 1:55 ` [PATCH 7/9 v2] difftool: teach difftool to handle directory diffs Tim Henigan
@ 2012-03-18 1:55 ` Tim Henigan
2012-03-18 1:55 ` [PATCH 9/9 v2] difftool: print list of valid tools with '--tool-help' Tim Henigan
2012-03-19 21:00 ` [PATCH 0/9 v2] difftool: teach command to perform directory diffs Junio C Hamano
9 siblings, 0 replies; 26+ messages in thread
From: Tim Henigan @ 2012-03-18 1:55 UTC (permalink / raw)
To: gitster, davvid, git; +Cc: Tim Henigan
If the user runs a diff when there are modified files in the working copy,
they expect to be able to be edit the files in the diff viewer and save
the changes. When using difftool in file mode (i.e. diffing one file at
a time), this works as expected. However, when using difftool in directory
diff mode, the changes are not saved.
This commit teaches the directory diff mode to copy changes from the diff
viewer back to the working copy.
Signed-off-by: Tim Henigan <tim.henigan@gmail.com>
---
git-difftool.perl | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/git-difftool.perl b/git-difftool.perl
index 1f7b8f2..e306977 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -22,6 +22,8 @@ use File::Temp qw(tempdir);
use Getopt::Long qw(:config pass_through);
use Git;
+my @working_tree;
+
sub usage
{
my $exitcode = shift;
@@ -54,7 +56,6 @@ sub setup_dir_diff
my $null_sha1 = 0 x 40;
my $lindex = "";
my $rindex = "";
- my @working_tree;
my %submodule;
my @rawdiff = split('\0', $diffrtn);
@@ -178,9 +179,16 @@ if (defined($dirdiff)) {
Git::command_noisy(('difftool--helper', $a, $b))
} 'exit code %d';
}
- # TODO: if the diff including working copy files and those
+
+ # 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
+ my $repo = Git->repository();
+ my $workdir = $repo->wc_path();
+ for (@working_tree) {
+ copy("$b/$_", "$workdir/$_") or die $!;
+ chmod(stat("$b/$_")->mode, "$workdir/$_") or die $!;
+ }
} else {
if (defined($prompt)) {
$ENV{GIT_DIFFTOOL_PROMPT} = 'true';
--
1.7.9.1.290.gbd444
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 9/9 v2] difftool: print list of valid tools with '--tool-help'
2012-03-18 1:55 ` [PATCH 0/9 v2] " Tim Henigan
` (7 preceding siblings ...)
2012-03-18 1:55 ` [PATCH 8/9 v2] difftool: teach dir-diff to copy modified files back to working tree Tim Henigan
@ 2012-03-18 1:55 ` Tim Henigan
2012-03-19 21:00 ` [PATCH 0/9 v2] difftool: teach command to perform directory diffs Junio C Hamano
9 siblings, 0 replies; 26+ messages in thread
From: Tim Henigan @ 2012-03-18 1:55 UTC (permalink / raw)
To: gitster, davvid, git; +Cc: Tim Henigan
Since bc7a96a (mergetool--lib: Refactor tools into separate files,
2011-08-18), it is possible to add a new diff tool by creating a simple
script in the '$(git --exec-path)/mergetools' directory. Updating the
difftool help text is still a manual process, and the documentation can
easily go out of sync.
Teach the command to read the list of valid tools from the 'mergetools'
directory and print them for the user when the '--tool-help' option is
given.
Signed-off-by: Tim Henigan <tim.henigan@gmail.com>
---
Changes in v2:
The glob statement used to find the mergetool options now uses the
fully qualified path. This insures that 'git difftool --tool-help'
can be called from any diretory.
Documentation/git-difftool.txt | 11 ++++++-----
git-difftool.perl | 17 ++++++++++++++---
2 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt
index aba5e76..31fc2e3 100644
--- a/Documentation/git-difftool.txt
+++ b/Documentation/git-difftool.txt
@@ -36,11 +36,9 @@ OPTIONS
-t <tool>::
--tool=<tool>::
- Use the diff tool specified by <tool>.
- Valid diff tools are:
- araxis, bc3, deltawalker, diffuse, emerge, ecmerge, gvimdiff,
- kdiff3, kompare, meld, opendiff, p4merge, tkdiff, vimdiff and
- xxdiff.
+ Use the diff tool specified by <tool>. Valid values include
+ emerge, kompare, meld, and vimdiff. Run `git difftool --tool-help`
+ for the list of valid <tool> settings.
+
If a diff tool is not specified, 'git difftool'
will use the configuration variable `diff.tool`. If the
@@ -68,6 +66,9 @@ of the diff post-image. `$MERGED` is the name of the file which is
being compared. `$BASE` is provided for compatibility
with custom merge tool commands and has the same value as `$MERGED`.
+--tool-help::
+ Print a list of diff tools that may be used with `--tool`.
+
-x <command>::
--extcmd=<command>::
Specify a custom command for viewing diffs.
diff --git a/git-difftool.perl b/git-difftool.perl
index e306977..fb4e3e6 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -14,7 +14,7 @@
use 5.008;
use strict;
use warnings;
-use File::Basename qw(dirname);
+use File::Basename qw(dirname basename);
use File::Copy;
use File::stat;
use File::Path qw(mkpath);
@@ -28,7 +28,8 @@ sub usage
{
my $exitcode = shift;
print << 'USAGE';
-usage: git difftool [-t|--tool=<tool>] [-x|--extcmd=<cmd>]
+usage: git difftool [-t|--tool=<tool>] [--tool-help]
+ [-x|--extcmd=<cmd>]
[-y|--no-prompt] [-g|--gui]
[-d|--dir-diff]
['git diff' options]
@@ -129,18 +130,28 @@ sub setup_dir_diff
# parse command-line options. all unrecognized options and arguments
# are passed through to the 'git diff' command.
-my ($difftool_cmd, $dirdiff, $extcmd, $gui, $help, $no_prompt, $prompt);
+my ($difftool_cmd, $dirdiff, $extcmd, $gui, $help, $no_prompt, $prompt, $tool_help);
GetOptions('g|gui' => \$gui,
'd|dir-diff' => \$dirdiff,
'h' => \$help,
'prompt' => \$prompt,
't|tool:s' => \$difftool_cmd,
+ 'tool-help' => \$tool_help,
'x|extcmd:s' => \$extcmd,
'y|no-prompt' => \$no_prompt);
if (defined($help)) {
usage(0);
}
+if (defined($tool_help)) {
+ my $gitpath = Git::exec_path();
+ print "'git difftool --tool=<tool>' may be set to one of the following:\n";
+ for (glob "$gitpath/mergetools/*") {
+ next if /defaults$/;
+ print "\t" . basename($_) . "\n";
+ }
+ exit(0);
+}
if (defined($difftool_cmd)) {
if (length($difftool_cmd) > 0) {
$ENV{GIT_DIFF_TOOL} = $difftool_cmd;
--
1.7.9.1.290.gbd444
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 0/9 v2] difftool: teach command to perform directory diffs
2012-03-18 1:55 ` [PATCH 0/9 v2] " Tim Henigan
` (8 preceding siblings ...)
2012-03-18 1:55 ` [PATCH 9/9 v2] difftool: print list of valid tools with '--tool-help' Tim Henigan
@ 2012-03-19 21:00 ` Junio C Hamano
2012-03-20 2:52 ` David Aguilar
2012-03-20 13:07 ` Tim Henigan
9 siblings, 2 replies; 26+ messages in thread
From: Junio C Hamano @ 2012-03-19 21:00 UTC (permalink / raw)
To: Tim Henigan; +Cc: davvid, git
Thanks.
I do not know a difftool user, and I wasn't paying close attention to the
discussion but I recall these points raised and I do not recall the
resolutions:
* In [1/9], use of pass_through would mean 'git difftool' must be fed the
options for difftool and then options meant for underlying diff
machinery. Is this limitation still there? If so, isn't this a
regression? Shouldn't it at least be advertised to the users a lot
stronger in documentation?
* In [4/9], you remove the .exe extension when running git, which was
there since the beginning of difftool 5c38ea3 (contrib: add 'git
difftool' for launching common merge tools, 2009-01-16). I think it is
safe but are Windows folks OK with this?
* In [6/9], the exit code in the failure case seem to be modified from
that of the underlying "git diff" to whatever croak gives us. Is this
a regression, or nobody cares error status from difftool? I personally
suspect it is the latter, but just double-checking if you have
considered it.
* In [7/9], difftool--helper declares SUBDIRECTORY_OK, but there doesn't
seem to be any inclusion of git-sh-setup in this script, and the patch
does not have any effort to prepend $prefix to paths relative to $cwd.
What good does the variable do here?
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 0/9 v2] difftool: teach command to perform directory diffs
2012-03-19 21:00 ` [PATCH 0/9 v2] difftool: teach command to perform directory diffs Junio C Hamano
@ 2012-03-20 2:52 ` David Aguilar
2012-03-20 5:52 ` Junio C Hamano
2012-03-20 13:07 ` Tim Henigan
1 sibling, 1 reply; 26+ messages in thread
From: David Aguilar @ 2012-03-20 2:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Tim Henigan, git
On Mon, Mar 19, 2012 at 2:00 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Thanks.
>
> I do not know a difftool user, and I wasn't paying close attention to the
> discussion but I recall these points raised and I do not recall the
> resolutions:
>
> * In [1/9], use of pass_through would mean 'git difftool' must be fed the
> options for difftool and then options meant for underlying diff
> machinery. Is this limitation still there? If so, isn't this a
> regression? Shouldn't it at least be advertised to the users a lot
> stronger in documentation?
Tim asserted that this is not the case. The options should pass
through. Hopefully there aren't any behavior changes between perl
versions and this option.
I sent a patch adding a test case to cover this scenario. I would
prefer that we avoid a regression. If there's a regression then we
can do without Getopt::Long, IMO.
I believe users should be oblivious as to which options are for
difftool and which are for diff. E.g. `git difftool --cached -t
xxdiff` -- the user does not care that --cached is for diff and -t is
for difftool.
> * In [4/9], you remove the .exe extension when running git, which was
> there since the beginning of difftool 5c38ea3 (contrib: add 'git
> difftool' for launching common merge tools, 2009-01-16). I think it is
> safe but are Windows folks OK with this?
I do not have Windows to test with, but this supposedly works since
Git.pm does not use git.exe either. The git.exe stuff was originally
there because difftool originally did exec('git.exe', ...). It was
later changed to use system() and it's possible that we could have
dropped the .exe extension at that time.
But, like I said, I don't have any Windows machines with which to
verify this behavior, and highly appreciate feedback from Windows
folks.
> * In [6/9], the exit code in the failure case seem to be modified from
> that of the underlying "git diff" to whatever croak gives us. Is this
> a regression, or nobody cares error status from difftool? I personally
> suspect it is the latter, but just double-checking if you have
> considered it.
This doesn't seem like too big of an issue. Had we documented the old
exit codes then it would be a bigger concern. I would personally
prefer to preserve the exit code from diff itself, but if that
complicates it too much then the new behavior is ok.
> * In [7/9], difftool--helper declares SUBDIRECTORY_OK, but there doesn't
> seem to be any inclusion of git-sh-setup in this script, and the patch
> does not have any effort to prepend $prefix to paths relative to $cwd.
> What good does the variable do here?
I'll defer to Tim on this one. This seems like an oversight. It
seems like something should be done to handle it.
I recall that we made $GIT_PREFIX available to aliases and builtins
not too long ago. That may help here. See
1f5d271f5e8f7b1e2a5b296ff43ca4087eb08244.
Also.. I think we need some tests to cover the new behavior. A test
to cover the subdirectory behavior would be especially helpful given
the note about [7/9].
--
David
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 0/9 v2] difftool: teach command to perform directory diffs
2012-03-20 2:52 ` David Aguilar
@ 2012-03-20 5:52 ` Junio C Hamano
0 siblings, 0 replies; 26+ messages in thread
From: Junio C Hamano @ 2012-03-20 5:52 UTC (permalink / raw)
To: David Aguilar; +Cc: Tim Henigan, git
David Aguilar <davvid@gmail.com> writes:
> On Mon, Mar 19, 2012 at 2:00 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Thanks.
>>
>> I do not know a difftool user, and I wasn't paying close attention to the
>> discussion but I recall these points raised and I do not recall the
>> resolutions:
>>
>> * In [1/9], use of pass_through would mean 'git difftool' must be fed the
>> options for difftool and then options meant for underlying diff
>> machinery. Is this limitation still there? If so, isn't this a
>> regression? Shouldn't it at least be advertised to the users a lot
>> stronger in documentation?
>
> Tim asserted that this is not the case. The options should pass
> through. Hopefully there aren't any behavior changes between perl
> versions and this option.
>
> I sent a patch adding a test case to cover this scenario. I would
> prefer that we avoid a regression. If there's a regression then we
> can do without Getopt::Long, IMO.
Yeah. Does tonight's 'pu' pass for you?
> I do not have Windows to test with, but this supposedly works since
> Git.pm does not use git.exe either.
Yeah, that matches my guess.
>> * In [7/9], difftool--helper declares SUBDIRECTORY_OK, but there doesn't
>> seem to be any inclusion of git-sh-setup in this script, and the patch
>> does not have any effort to prepend $prefix to paths relative to $cwd.
>> What good does the variable do here?
>
> I'll defer to Tim on this one. This seems like an oversight. It
> seems like something should be done to handle it.
Well, it does not seem to dot-source git-sh-setup so it probably stays
where it was launched, so in that case there is nothing that needs to be
done, including SUBDIRECTORY_OK which nobody would look at IIRC.
> Also.. I think we need some tests to cover the new behavior. A test
> to cover the subdirectory behavior would be especially helpful given
> the note about [7/9].
Yeah, that makes sense.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 0/9 v2] difftool: teach command to perform directory diffs
2012-03-19 21:00 ` [PATCH 0/9 v2] difftool: teach command to perform directory diffs Junio C Hamano
2012-03-20 2:52 ` David Aguilar
@ 2012-03-20 13:07 ` Tim Henigan
2012-03-20 16:36 ` Junio C Hamano
1 sibling, 1 reply; 26+ messages in thread
From: Tim Henigan @ 2012-03-20 13:07 UTC (permalink / raw)
To: Junio C Hamano; +Cc: davvid, git
I have successfully tested on:
- Linux with Git v1.7.10-rc0 and 1
- Windows 7 with msysgit v1.7.9
I installed cygwin Git on Windows 7, but found that 'git difftool'
would not work, even before my patches were applied. It appears that
I need to learn more about cygwin before I can use it as a test
platform.
On Mon, Mar 19, 2012 at 5:00 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> * In [1/9], use of pass_through would mean 'git difftool' must be fed the
> options for difftool and then options meant for underlying diff
> machinery. Is this limitation still there? If so, isn't this a
> regression? Shouldn't it at least be advertised to the users a lot
> stronger in documentation?
Both the documentation and testing has shown that 'pass_through' works
as I expected. That is, options that difftool cares about are
extracted from @ARGV while all others are passed on to the underlying
'git diff' command. The order of the arguments/options does not
matter. We "use 5.008;" at the top of the script, so I believe this
change is correct and safe. That being said, I have a limited number
of computers/configurations to test on.
> * In [4/9], you remove the .exe extension when running git, which was
> there since the beginning of difftool 5c38ea3 (contrib: add 'git
> difftool' for launching common merge tools, 2009-01-16). I think it is
> safe but are Windows folks OK with this?
My testing on Windows with msysgit has shown that this change is safe.
Also, even though I had other problems when testing on cygwin (noted
at the top of the email), the 'difftool' script was able to
successfully execute 'git diff --raw -z' without appending '.exe'.
> * In [6/9], the exit code in the failure case seem to be modified from
> that of the underlying "git diff" to whatever croak gives us. Is this
> a regression, or nobody cares error status from difftool? I personally
> suspect it is the latter, but just double-checking if you have
> considered it.
In my testing, if "Git::command_noisy(('diff', @ARGV))" fails, the
exit code from 'git diff' is passed back to the terminal. So not only
is the 'git diff' exit code echoed to the terminal, but '$?' also
contains the correct exit code.
> * In [7/9], difftool--helper declares SUBDIRECTORY_OK, but there doesn't
> seem to be any inclusion of git-sh-setup in this script, and the patch
> does not have any effort to prepend $prefix to paths relative to $cwd.
> What good does the variable do here?
This was an oversight. It was copied over from a previous
implementation that still used ". git-sh-setup".
So the only outstanding changes that I am planning are:
1) Remove SUBDIRECTORY_OK
2) Add tests for the new '--dir-diff' option
Would it be better to resend the complete patches series with these
changes or just add new patches to the end?
Please let me know if I missed anything else.
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 0/9 v2] difftool: teach command to perform directory diffs
2012-03-20 13:07 ` Tim Henigan
@ 2012-03-20 16:36 ` Junio C Hamano
0 siblings, 0 replies; 26+ messages in thread
From: Junio C Hamano @ 2012-03-20 16:36 UTC (permalink / raw)
To: Tim Henigan; +Cc: davvid, git
Tim Henigan <tim.henigan@gmail.com> writes:
> So the only outstanding changes that I am planning are:
> 1) Remove SUBDIRECTORY_OK
> 2) Add tests for the new '--dir-diff' option
>
> Would it be better to resend the complete patches series with these
> changes or just add new patches to the end?
For the former, a replacement patch ("[PATCH v3 7/9]" with "this replaces
patch 5/9 from the v2" after "---" line) for the ones that need replacing
is preferred, as the topic is still in 'pu'. For the latter, we can go
either with a separate patch "[PATCH v3 10/9]" or an update to the one
that finishes introduction of the new option.
> Please let me know if I missed anything else.
I think I saw t7800 failed with this series somewhwere around "last flag
wins" test.
^ permalink raw reply [flat|nested] 26+ messages in thread