* [PATCH v2 0/5] difftool: Use symlinks in dir-diff mode @ 2012-07-23 3:57 David Aguilar 2012-07-23 3:57 ` [PATCH v2 1/5] difftool: Simplify print_tool_help() David Aguilar 2012-07-23 5:14 ` [PATCH v2 0/5] difftool: Use symlinks in dir-diff mode Junio C Hamano 0 siblings, 2 replies; 13+ messages in thread From: David Aguilar @ 2012-07-23 3:57 UTC (permalink / raw) To: Junio C Hamano; +Cc: Tim Henigan, git Teach the difftool script to use symlinks when doing directory diffs in --dir-diff mode. This is v2 of the patch because I had a typo in one of the commit messages and gmail ate 4/5 in the last round. David Aguilar (5): difftool: Simplify print_tool_help() difftool: Eliminate global variables difftool: Move option values into a hash difftool: Call the temp directory "git-difftool" difftool: Use symlinks when diffing against the worktree Documentation/git-difftool.txt | 8 ++ git-difftool.perl | 184 ++++++++++++++++++++++++----------------- 2 files changed, 115 insertions(+), 77 deletions(-) -- 1.7.11.2.255.g5f133da ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 1/5] difftool: Simplify print_tool_help() 2012-07-23 3:57 [PATCH v2 0/5] difftool: Use symlinks in dir-diff mode David Aguilar @ 2012-07-23 3:57 ` David Aguilar 2012-07-23 3:57 ` [PATCH 2/5] difftool: Eliminate global variables David Aguilar 2012-07-23 5:14 ` [PATCH v2 0/5] difftool: Use symlinks in dir-diff mode Junio C Hamano 1 sibling, 1 reply; 13+ messages in thread From: David Aguilar @ 2012-07-23 3:57 UTC (permalink / raw) To: Junio C Hamano; +Cc: Tim Henigan, git Eliminate a global variable and File::Find usage by building upon basename() and glob() instead. Signed-off-by: David Aguilar <davvid@gmail.com> --- Same as before, resending because gmail ate patch 4/5 git-difftool.perl | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-) diff --git a/git-difftool.perl b/git-difftool.perl index c079854..ac0ed63 100755 --- a/git-difftool.perl +++ b/git-difftool.perl @@ -13,17 +13,15 @@ use 5.008; use strict; use warnings; -use File::Basename qw(dirname); +use File::Basename qw(basename dirname); use File::Copy; use File::Compare; -use File::Find; use File::stat; use File::Path qw(mkpath); use File::Temp qw(tempdir); use Getopt::Long qw(:config pass_through); use Git; -my @tools; my @working_tree; my $rc; my $repo = Git->repository(); @@ -65,26 +63,13 @@ sub find_worktree my $workdir = find_worktree(); -sub filter_tool_scripts -{ - if (-d $_) { - if ($_ ne ".") { - # Ignore files in subdirectories - $File::Find::prune = 1; - } - } else { - if ((-f $_) && ($_ ne "defaults")) { - push(@tools, $_); - } - } -} - sub print_tool_help { my ($cmd, @found, @notfound); my $gitpath = Git::exec_path(); - find(\&filter_tool_scripts, "$gitpath/mergetools"); + my @files = map { basename($_) } glob("$gitpath/mergetools/*"); + my @tools = sort(grep { !m{^defaults$} } @files); foreach my $tool (@tools) { $cmd = "TOOL_MODE=diff"; @@ -99,10 +84,10 @@ sub print_tool_help } print "'git difftool --tool=<tool>' may be set to one of the following:\n"; - print "\t$_\n" for (sort(@found)); + print "\t$_\n" for (@found); print "\nThe following tools are valid, but not currently available:\n"; - print "\t$_\n" for (sort(@notfound)); + print "\t$_\n" for (@notfound); print "\nNOTE: Some of the tools listed above only work in a windowed\n"; print "environment. If run in a terminal-only session, they will fail.\n"; -- 1.7.11.2.255.g5f133da ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/5] difftool: Eliminate global variables 2012-07-23 3:57 ` [PATCH v2 1/5] difftool: Simplify print_tool_help() David Aguilar @ 2012-07-23 3:57 ` David Aguilar 2012-07-23 3:57 ` [PATCH 3/5] difftool: Move option values into a hash David Aguilar 0 siblings, 1 reply; 13+ messages in thread From: David Aguilar @ 2012-07-23 3:57 UTC (permalink / raw) To: Junio C Hamano; +Cc: Tim Henigan, git Organize the script so that it has a single main() function which calls out to dir_diff() and file_diff() functions. This eliminates "dir-diff"-specific variables that do not need to be calculated when performing a regular file-diff. Signed-off-by: David Aguilar <davvid@gmail.com> --- Same as before, resending because gmail ate patch 4/5 git-difftool.perl | 128 ++++++++++++++++++++++++++++++++---------------------- 1 file changed, 75 insertions(+), 53 deletions(-) diff --git a/git-difftool.perl b/git-difftool.perl index ac0ed63..41ba932 100755 --- a/git-difftool.perl +++ b/git-difftool.perl @@ -22,11 +22,6 @@ use File::Temp qw(tempdir); use Getopt::Long qw(:config pass_through); use Git; -my @working_tree; -my $rc; -my $repo = Git->repository(); -my $repo_path = $repo->repo_path(); - sub usage { my $exitcode = shift; @@ -43,6 +38,8 @@ USAGE sub find_worktree { + my ($repo) = @_; + # Git->repository->wc_path() does not honor changes to the working # tree location made by $ENV{GIT_WORK_TREE} or the 'core.worktree' # config variable. @@ -61,8 +58,6 @@ sub find_worktree return $worktree; } -my $workdir = find_worktree(); - sub print_tool_help { my ($cmd, @found, @notfound); @@ -97,10 +92,13 @@ sub print_tool_help sub setup_dir_diff { + my ($repo, $workdir) = @_; + # Run the diff; exit immediately if no diff found # 'Repository' and 'WorkingCopy' must be explicitly set to insure that # if $GIT_DIR and $GIT_WORK_TREE are set in ENV, they are actually used # by Git->repository->command*. + my $repo_path = $repo->repo_path(); my $diffrepo = Git->repository(Repository => $repo_path, WorkingCopy => $workdir); my $diffrtn = $diffrepo->command_oneline('diff', '--raw', '--no-abbrev', '-z', @ARGV); exit(0) if (length($diffrtn) == 0); @@ -121,6 +119,7 @@ sub setup_dir_diff my $rindex = ''; my %submodule; my %symlink; + my @working_tree = (); my @rawdiff = split('\0', $diffrtn); my $i = 0; @@ -188,7 +187,7 @@ sub setup_dir_diff ($inpipe, $ctx) = $repo->command_input_pipe(qw/update-index -z --index-info/); print($inpipe $lindex); $repo->command_close_pipe($inpipe, $ctx); - $rc = system('git', 'checkout-index', '--all', "--prefix=$ldir/"); + my $rc = system('git', 'checkout-index', '--all', "--prefix=$ldir/"); exit($rc | ($rc >> 8)) if ($rc != 0); $ENV{GIT_INDEX_FILE} = "$tmpdir/rindex"; @@ -238,7 +237,7 @@ sub setup_dir_diff } } - return ($ldir, $rdir); + return ($ldir, $rdir, @working_tree); } sub write_to_file @@ -261,54 +260,70 @@ sub write_to_file close($fh); } -# parse command-line options. all unrecognized options and arguments -# are passed through to the 'git diff' command. -my ($difftool_cmd, $dirdiff, $extcmd, $gui, $help, $prompt, $tool_help); -GetOptions('g|gui!' => \$gui, - 'd|dir-diff' => \$dirdiff, - 'h' => \$help, - 'prompt!' => \$prompt, - 'y' => sub { $prompt = 0; }, - 't|tool:s' => \$difftool_cmd, - 'tool-help' => \$tool_help, - 'x|extcmd:s' => \$extcmd); - -if (defined($help)) { - usage(0); -} -if (defined($tool_help)) { - print_tool_help(); -} -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(1); +sub main +{ + # parse command-line options. all unrecognized options and arguments + # are passed through to the 'git diff' command. + my ($difftool_cmd, $dirdiff, $extcmd, $gui, $help, $prompt, $tool_help); + GetOptions('g|gui!' => \$gui, + 'd|dir-diff' => \$dirdiff, + 'h' => \$help, + 'prompt!' => \$prompt, + 'y' => sub { $prompt = 0; }, + 't|tool:s' => \$difftool_cmd, + 'tool-help' => \$tool_help, + 'x|extcmd:s' => \$extcmd); + + if (defined($help)) { + usage(0); } -} -if (defined($extcmd)) { - if (length($extcmd) > 0) { - $ENV{GIT_DIFFTOOL_EXTCMD} = $extcmd; - } else { - print "No <cmd> given for --extcmd=<cmd>\n"; - usage(1); + if (defined($tool_help)) { + print_tool_help(); } -} -if ($gui) { - my $guitool = ''; - $guitool = Git::config('diff.guitool'); - if (length($guitool) > 0) { - $ENV{GIT_DIFF_TOOL} = $guitool; + 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(1); + } + } + if (defined($extcmd)) { + if (length($extcmd) > 0) { + $ENV{GIT_DIFFTOOL_EXTCMD} = $extcmd; + } else { + print "No <cmd> given for --extcmd=<cmd>\n"; + usage(1); + } + } + if ($gui) { + my $guitool = ''; + $guitool = Git::config('diff.guitool'); + if (length($guitool) > 0) { + $ENV{GIT_DIFF_TOOL} = $guitool; + } + } + + # 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)) { + dir_diff($extcmd); + } else { + file_diff($prompt); } } -# 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(); +sub dir_diff +{ + my ($extcmd) = @_; + + my $rc; + my $repo = Git->repository(); + + my $workdir = find_worktree($repo); + my ($a, $b, @working_tree) = setup_dir_diff($repo, $workdir); if (defined($extcmd)) { $rc = system($extcmd, $a, $b); } else { @@ -327,7 +342,12 @@ if (defined($dirdiff)) { chmod(stat("$b/$file")->mode, "$workdir/$file") or die $!; } } -} else { +} + +sub file_diff +{ + my ($prompt) = @_; + if (defined($prompt)) { if ($prompt) { $ENV{GIT_DIFFTOOL_PROMPT} = 'true'; @@ -347,3 +367,5 @@ if (defined($dirdiff)) { my $rc = system('git', 'diff', @ARGV); exit($rc | ($rc >> 8)); } + +main(); -- 1.7.11.2.255.g5f133da ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/5] difftool: Move option values into a hash 2012-07-23 3:57 ` [PATCH 2/5] difftool: Eliminate global variables David Aguilar @ 2012-07-23 3:57 ` David Aguilar 2012-07-23 3:57 ` [PATCH 4/5] difftool: Call the temp directory "git-difftool" David Aguilar 0 siblings, 1 reply; 13+ messages in thread From: David Aguilar @ 2012-07-23 3:57 UTC (permalink / raw) To: Junio C Hamano; +Cc: Tim Henigan, git Shorten the "my" declaration for all of the option-specific variables by wrapping all of them in a hash. This also gives us a place to specify default values, should we need them. Signed-off-by: David Aguilar <davvid@gmail.com> --- Fixed typo in the commit message git-difftool.perl | 55 +++++++++++++++++++++++++++++++------------------------ 1 file changed, 31 insertions(+), 24 deletions(-) diff --git a/git-difftool.perl b/git-difftool.perl index 41ba932..0ce6168 100755 --- a/git-difftool.perl +++ b/git-difftool.perl @@ -264,41 +264,48 @@ sub main { # parse command-line options. all unrecognized options and arguments # are passed through to the 'git diff' command. - my ($difftool_cmd, $dirdiff, $extcmd, $gui, $help, $prompt, $tool_help); - GetOptions('g|gui!' => \$gui, - 'd|dir-diff' => \$dirdiff, - 'h' => \$help, - 'prompt!' => \$prompt, - 'y' => sub { $prompt = 0; }, - 't|tool:s' => \$difftool_cmd, - 'tool-help' => \$tool_help, - 'x|extcmd:s' => \$extcmd); - - if (defined($help)) { + my %opts = ( + difftool_cmd => undef, + dirdiff => undef, + extcmd => undef, + gui => undef, + help => undef, + prompt => undef, + tool_help => undef, + ); + GetOptions('g|gui!' => \$opts{gui}, + 'd|dir-diff' => \$opts{dirdiff}, + 'h' => \$opts{help}, + 'prompt!' => \$opts{prompt}, + 'y' => sub { $opts{prompt} = 0; }, + 't|tool:s' => \$opts{difftool_cmd}, + 'tool-help' => \$opts{tool_help}, + 'x|extcmd:s' => \$opts{extcmd}); + + if (defined($opts{help})) { usage(0); } - if (defined($tool_help)) { + if (defined($opts{tool_help})) { print_tool_help(); } - if (defined($difftool_cmd)) { - if (length($difftool_cmd) > 0) { - $ENV{GIT_DIFF_TOOL} = $difftool_cmd; + if (defined($opts{difftool_cmd})) { + if (length($opts{difftool_cmd}) > 0) { + $ENV{GIT_DIFF_TOOL} = $opts{difftool_cmd}; } else { print "No <tool> given for --tool=<tool>\n"; usage(1); } } - if (defined($extcmd)) { - if (length($extcmd) > 0) { - $ENV{GIT_DIFFTOOL_EXTCMD} = $extcmd; + if (defined($opts{extcmd})) { + if (length($opts{extcmd}) > 0) { + $ENV{GIT_DIFFTOOL_EXTCMD} = $opts{extcmd}; } else { print "No <cmd> given for --extcmd=<cmd>\n"; usage(1); } } - if ($gui) { - my $guitool = ''; - $guitool = Git::config('diff.guitool'); + if ($opts{gui}) { + my $guitool = Git::config('diff.guitool'); if (length($guitool) > 0) { $ENV{GIT_DIFF_TOOL} = $guitool; } @@ -308,10 +315,10 @@ sub main # 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)) { - dir_diff($extcmd); + if (defined($opts{dirdiff})) { + dir_diff($opts{extcmd}); } else { - file_diff($prompt); + file_diff($opts{prompt}); } } -- 1.7.11.2.255.g5f133da ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 4/5] difftool: Call the temp directory "git-difftool" 2012-07-23 3:57 ` [PATCH 3/5] difftool: Move option values into a hash David Aguilar @ 2012-07-23 3:57 ` David Aguilar 2012-07-23 3:57 ` [PATCH 5/5] difftool: Use symlinks when diffing against the worktree David Aguilar 0 siblings, 1 reply; 13+ messages in thread From: David Aguilar @ 2012-07-23 3:57 UTC (permalink / raw) To: Junio C Hamano; +Cc: Tim Henigan, git The "diffall" name was left over from when this functionality was part of the "git-diffall" script in contrib/. Make the naming consistent. Signed-off-by: David Aguilar <davvid@gmail.com> --- Reworded the commit message to get through gmail filters. git-difftool.perl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-difftool.perl b/git-difftool.perl index 0ce6168..2ae344c 100755 --- a/git-difftool.perl +++ b/git-difftool.perl @@ -104,7 +104,7 @@ sub setup_dir_diff exit(0) if (length($diffrtn) == 0); # Setup temp directories - my $tmpdir = tempdir('git-diffall.XXXXX', CLEANUP => 1, TMPDIR => 1); + my $tmpdir = tempdir('git-difftool.XXXXX', CLEANUP => 1, TMPDIR => 1); my $ldir = "$tmpdir/left"; my $rdir = "$tmpdir/right"; mkpath($ldir) or die $!; -- 1.7.11.2.255.g5f133da ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 5/5] difftool: Use symlinks when diffing against the worktree 2012-07-23 3:57 ` [PATCH 4/5] difftool: Call the temp directory "git-difftool" David Aguilar @ 2012-07-23 3:57 ` David Aguilar 2012-07-23 4:57 ` Junio C Hamano 0 siblings, 1 reply; 13+ messages in thread From: David Aguilar @ 2012-07-23 3:57 UTC (permalink / raw) To: Junio C Hamano; +Cc: Tim Henigan, git Teach difftool's --dir-diff mode to use symlinks to represent files from the working copy, and make it the default behavior for the non-Windows platforms. Using symlinks is simpler and safer since we do not need to worry about copying files back into the worktree. The old behavior is still available as --no-symlinks. Signed-off-by: David Aguilar <davvid@gmail.com> --- Same as before, resending because gmail ate patch 4/5 Documentation/git-difftool.txt | 8 ++++++++ git-difftool.perl | 30 +++++++++++++++++++++++------- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt index 31fc2e3..313d54e 100644 --- a/Documentation/git-difftool.txt +++ b/Documentation/git-difftool.txt @@ -66,6 +66,14 @@ 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`. +--symlinks:: +--no-symlinks:: + 'git difftool''s default behavior is create symlinks to the + working tree when run in `--dir-diff` mode. ++ + Specifying `--no-symlinks` instructs 'git difftool' to create + copies instead. `--no-symlinks` is the default on Windows. + --tool-help:: Print a list of diff tools that may be used with `--tool`. diff --git a/git-difftool.perl b/git-difftool.perl index 2ae344c..b8f8057 100755 --- a/git-difftool.perl +++ b/git-difftool.perl @@ -92,7 +92,7 @@ sub print_tool_help sub setup_dir_diff { - my ($repo, $workdir) = @_; + my ($repo, $workdir, $symlinks) = @_; # Run the diff; exit immediately if no diff found # 'Repository' and 'WorkingCopy' must be explicitly set to insure that @@ -209,8 +209,13 @@ sub setup_dir_diff unless (-d "$rdir/$dir") { mkpath("$rdir/$dir") or die $!; } - copy("$workdir/$file", "$rdir/$file") or die $!; - chmod(stat("$workdir/$file")->mode, "$rdir/$file") or die $!; + if ($symlinks) { + symlink("$workdir/$file", "$rdir/$file") or die $!; + } else { + copy("$workdir/$file", "$rdir/$file") or die $!; + my $mode = stat("$workdir/$file")->mode; + chmod($mode, "$rdir/$file") or die $!; + } } # Changes to submodules require special treatment. This loop writes a @@ -271,6 +276,7 @@ sub main gui => undef, help => undef, prompt => undef, + symlinks => $^O ne 'MSWin32' && $^O ne 'msys', tool_help => undef, ); GetOptions('g|gui!' => \$opts{gui}, @@ -278,6 +284,8 @@ sub main 'h' => \$opts{help}, 'prompt!' => \$opts{prompt}, 'y' => sub { $opts{prompt} = 0; }, + 'symlinks' => \$opts{symlinks}, + 'no-symlinks' => sub { $opts{symlinks} = 0; }, 't|tool:s' => \$opts{difftool_cmd}, 'tool-help' => \$opts{tool_help}, 'x|extcmd:s' => \$opts{extcmd}); @@ -316,7 +324,7 @@ sub main # will invoke a separate instance of 'git-difftool--helper' for # each file that changed. if (defined($opts{dirdiff})) { - dir_diff($opts{extcmd}); + dir_diff($opts{extcmd}, $opts{symlinks}); } else { file_diff($opts{prompt}); } @@ -324,13 +332,13 @@ sub main sub dir_diff { - my ($extcmd) = @_; + my ($extcmd, $symlinks) = @_; my $rc; my $repo = Git->repository(); my $workdir = find_worktree($repo); - my ($a, $b, @working_tree) = setup_dir_diff($repo, $workdir); + my ($a, $b, @working_tree) = setup_dir_diff($repo, $workdir, $symlinks); if (defined($extcmd)) { $rc = system($extcmd, $a, $b); } else { @@ -340,15 +348,23 @@ sub dir_diff exit($rc | ($rc >> 8)) if ($rc != 0); + # Do not copy back files when symlinks are used + if ($symlinks) { + exit(0); + } + # 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")) { copy("$b/$file", "$workdir/$file") or die $!; - chmod(stat("$b/$file")->mode, "$workdir/$file") or die $!; + my $mode = stat("$b/$file")->mode; + chmod($mode, "$workdir/$file") or die $!; } } + exit(0); } sub file_diff -- 1.7.11.2.255.g5f133da ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 5/5] difftool: Use symlinks when diffing against the worktree 2012-07-23 3:57 ` [PATCH 5/5] difftool: Use symlinks when diffing against the worktree David Aguilar @ 2012-07-23 4:57 ` Junio C Hamano 2012-07-23 6:05 ` [PATCH v3 4/5] " David Aguilar 0 siblings, 1 reply; 13+ messages in thread From: Junio C Hamano @ 2012-07-23 4:57 UTC (permalink / raw) To: David Aguilar; +Cc: Tim Henigan, git David Aguilar <davvid@gmail.com> writes: > + # Do not copy back files when symlinks are used > + if ($symlinks) { > + exit(0); > + } > + Isn't this a bit risky, depending on the behaviour of the tool that eventually lead the user to invoke his favorite editor to muck with the files in the temporary directory? I think most sane people and their editors would follow symlinks and update the file the symlink points at when writing out the modified contents, but it should not be too much trouble to detect the case in which the editor unlinked the symlink and recreated a regular file in its place, and copy the file back when that happened, to make it even safer, no? The most lazy solution would be to just remove the above block, and let the compare() compare the symlink $b/$file and the working tree file $workdir/$file that is pointed by it. We will find data losing case where the editor unlinks and creates that way automatically. Optionally, you can update if (-e "$b/$file" && compare("$b/$file", "$workdir/$file")) { with if (! -l "$b/$file" && -f _ && compare("$b/$file", "$workdir/$file")) { to avoid the cost of comparison. > # 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")) { > copy("$b/$file", "$workdir/$file") or die $!; > - chmod(stat("$b/$file")->mode, "$workdir/$file") or die $!; > + my $mode = stat("$b/$file")->mode; > + chmod($mode, "$workdir/$file") or die $!; > } > } > + exit(0); > } Other than that, the series looked well thought-out. Thanks. ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3 4/5] difftool: Use symlinks when diffing against the worktree 2012-07-23 4:57 ` Junio C Hamano @ 2012-07-23 6:05 ` David Aguilar 2012-07-23 16:38 ` Junio C Hamano 2012-07-24 13:35 ` Tim Henigan 0 siblings, 2 replies; 13+ messages in thread From: David Aguilar @ 2012-07-23 6:05 UTC (permalink / raw) To: Junio C Hamano; +Cc: Tim Henigan, git Teach difftool's --dir-diff mode to use symlinks to represent files from the working copy, and make it the default behavior for the non-Windows platforms. Using symlinks is simpler and safer since we do not need to worry about copying files back into the worktree. The old behavior is still available as --no-symlinks. Signed-off-by: David Aguilar <davvid@gmail.com> --- Handles the case where an editor unlinks the original symlink, replacing it with a file. Documentation/git-difftool.txt | 8 ++++++++ git-difftool.perl | 33 +++++++++++++++++++++++---------- 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt index 31fc2e3..313d54e 100644 --- a/Documentation/git-difftool.txt +++ b/Documentation/git-difftool.txt @@ -66,6 +66,14 @@ 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`. +--symlinks:: +--no-symlinks:: + 'git difftool''s default behavior is create symlinks to the + working tree when run in `--dir-diff` mode. ++ + Specifying `--no-symlinks` instructs 'git difftool' to create + copies instead. `--no-symlinks` is the default on Windows. + --tool-help:: Print a list of diff tools that may be used with `--tool`. diff --git a/git-difftool.perl b/git-difftool.perl index 2ae344c..a5b371f 100755 --- a/git-difftool.perl +++ b/git-difftool.perl @@ -92,7 +92,7 @@ sub print_tool_help sub setup_dir_diff { - my ($repo, $workdir) = @_; + my ($repo, $workdir, $symlinks) = @_; # Run the diff; exit immediately if no diff found # 'Repository' and 'WorkingCopy' must be explicitly set to insure that @@ -209,8 +209,13 @@ sub setup_dir_diff unless (-d "$rdir/$dir") { mkpath("$rdir/$dir") or die $!; } - copy("$workdir/$file", "$rdir/$file") or die $!; - chmod(stat("$workdir/$file")->mode, "$rdir/$file") or die $!; + if ($symlinks) { + symlink("$workdir/$file", "$rdir/$file") or die $!; + } else { + copy("$workdir/$file", "$rdir/$file") or die $!; + my $mode = stat("$workdir/$file")->mode; + chmod($mode, "$rdir/$file") or die $!; + } } # Changes to submodules require special treatment. This loop writes a @@ -271,6 +276,7 @@ sub main gui => undef, help => undef, prompt => undef, + symlinks => $^O ne 'MSWin32' && $^O ne 'msys', tool_help => undef, ); GetOptions('g|gui!' => \$opts{gui}, @@ -278,6 +284,8 @@ sub main 'h' => \$opts{help}, 'prompt!' => \$opts{prompt}, 'y' => sub { $opts{prompt} = 0; }, + 'symlinks' => \$opts{symlinks}, + 'no-symlinks' => sub { $opts{symlinks} = 0; }, 't|tool:s' => \$opts{difftool_cmd}, 'tool-help' => \$opts{tool_help}, 'x|extcmd:s' => \$opts{extcmd}); @@ -316,7 +324,7 @@ sub main # will invoke a separate instance of 'git-difftool--helper' for # each file that changed. if (defined($opts{dirdiff})) { - dir_diff($opts{extcmd}); + dir_diff($opts{extcmd}, $opts{symlinks}); } else { file_diff($opts{prompt}); } @@ -324,13 +332,13 @@ sub main sub dir_diff { - my ($extcmd) = @_; + my ($extcmd, $symlinks) = @_; my $rc; my $repo = Git->repository(); my $workdir = find_worktree($repo); - my ($a, $b, @working_tree) = setup_dir_diff($repo, $workdir); + my ($a, $b, @worktree) = setup_dir_diff($repo, $workdir, $symlinks); if (defined($extcmd)) { $rc = system($extcmd, $a, $b); } else { @@ -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")) { copy("$b/$file", "$workdir/$file") or die $!; - chmod(stat("$b/$file")->mode, "$workdir/$file") or die $!; + my $mode = stat("$b/$file")->mode; + chmod($mode, "$workdir/$file") or die $!; } } + exit(0); } sub file_diff -- 1.7.7.2.448.gee6df ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v3 4/5] difftool: Use symlinks when diffing against the worktree 2012-07-23 6:05 ` [PATCH v3 4/5] " David Aguilar @ 2012-07-23 16:38 ` Junio C Hamano 2012-07-24 13:35 ` Tim Henigan 1 sibling, 0 replies; 13+ messages in thread From: Junio C Hamano @ 2012-07-23 16:38 UTC (permalink / raw) To: David Aguilar; +Cc: Tim Henigan, git David Aguilar <davvid@gmail.com> writes: > Teach difftool's --dir-diff mode to use symlinks to represent > files from the working copy, and make it the default behavior > for the non-Windows platforms. > > Using symlinks is simpler and safer since we do not need to > worry about copying files back into the worktree. > The old behavior is still available as --no-symlinks. > > Signed-off-by: David Aguilar <davvid@gmail.com> > --- > Handles the case where an editor unlinks the original symlink, > replacing it with a file. Thanks; will replace. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 4/5] difftool: Use symlinks when diffing against the worktree 2012-07-23 6:05 ` [PATCH v3 4/5] " David Aguilar 2012-07-23 16:38 ` Junio C Hamano @ 2012-07-24 13:35 ` Tim Henigan 2012-07-24 15:57 ` Junio C Hamano 1 sibling, 1 reply; 13+ messages in thread From: Tim Henigan @ 2012-07-24 13:35 UTC (permalink / raw) To: David Aguilar; +Cc: Junio C Hamano, git 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 <davvid@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. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 4/5] difftool: Use symlinks when diffing against the worktree 2012-07-24 13:35 ` Tim Henigan @ 2012-07-24 15:57 ` Junio C Hamano 0 siblings, 0 replies; 13+ messages in thread From: Junio C Hamano @ 2012-07-24 15:57 UTC (permalink / raw) To: Tim Henigan; +Cc: David Aguilar, git Tim Henigan <tim.henigan@gmail.com> writes: > 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 <davvid@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. It probably is safer to report the error, not touch anything and let the user take an appropriate action. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/5] difftool: Use symlinks in dir-diff mode 2012-07-23 3:57 [PATCH v2 0/5] difftool: Use symlinks in dir-diff mode David Aguilar 2012-07-23 3:57 ` [PATCH v2 1/5] difftool: Simplify print_tool_help() David Aguilar @ 2012-07-23 5:14 ` Junio C Hamano 2012-07-23 5:34 ` David Aguilar 1 sibling, 1 reply; 13+ messages in thread From: Junio C Hamano @ 2012-07-23 5:14 UTC (permalink / raw) To: David Aguilar; +Cc: Tim Henigan, git David Aguilar <davvid@gmail.com> writes: > Teach the difftool script to use symlinks when doing > directory diffs in --dir-diff mode. > > This is v2 of the patch because I had a typo in one of the > commit messages and gmail ate 4/5 in the last round. FWIW, I received all including 4/5 in my inboxes (at pobox and gmail---I am doubly subscribed). I still haven't figured out what in the original 4/5 was so special to be dropped somewhere in between. > David Aguilar (5): > difftool: Simplify print_tool_help() > difftool: Eliminate global variables > difftool: Move option values into a hash > difftool: Call the temp directory "git-difftool" > difftool: Use symlinks when diffing against the worktree > > Documentation/git-difftool.txt | 8 ++ > git-difftool.perl | 184 ++++++++++++++++++++++++----------------- > 2 files changed, 115 insertions(+), 77 deletions(-) ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/5] difftool: Use symlinks in dir-diff mode 2012-07-23 5:14 ` [PATCH v2 0/5] difftool: Use symlinks in dir-diff mode Junio C Hamano @ 2012-07-23 5:34 ` David Aguilar 0 siblings, 0 replies; 13+ messages in thread From: David Aguilar @ 2012-07-23 5:34 UTC (permalink / raw) To: Junio C Hamano; +Cc: Tim Henigan, git On Sun, Jul 22, 2012 at 10:14 PM, Junio C Hamano <gitster@pobox.com> wrote: > David Aguilar <davvid@gmail.com> writes: > >> Teach the difftool script to use symlinks when doing >> directory diffs in --dir-diff mode. >> >> This is v2 of the patch because I had a typo in one of the >> commit messages and gmail ate 4/5 in the last round. > > FWIW, I received all including 4/5 in my inboxes (at pobox and > gmail---I am doubly subscribed). I still haven't figured out what > in the original 4/5 was so special to be dropped somewhere in > between. I hastily blamed gmail but of course it was vger's spam filters. The original subject said "git-difftool.XXXXX". The exes triggered it. -- David ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2012-07-24 15:57 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-07-23 3:57 [PATCH v2 0/5] difftool: Use symlinks in dir-diff mode David Aguilar 2012-07-23 3:57 ` [PATCH v2 1/5] difftool: Simplify print_tool_help() David Aguilar 2012-07-23 3:57 ` [PATCH 2/5] difftool: Eliminate global variables David Aguilar 2012-07-23 3:57 ` [PATCH 3/5] difftool: Move option values into a hash David Aguilar 2012-07-23 3:57 ` [PATCH 4/5] difftool: Call the temp directory "git-difftool" David Aguilar 2012-07-23 3:57 ` [PATCH 5/5] difftool: Use symlinks when diffing against the worktree David Aguilar 2012-07-23 4:57 ` Junio C Hamano 2012-07-23 6:05 ` [PATCH v3 4/5] " David Aguilar 2012-07-23 16:38 ` Junio C Hamano 2012-07-24 13:35 ` Tim Henigan 2012-07-24 15:57 ` Junio C Hamano 2012-07-23 5:14 ` [PATCH v2 0/5] difftool: Use symlinks in dir-diff mode Junio C Hamano 2012-07-23 5:34 ` David Aguilar
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).