{"thread":{"id":"30232","subject":"[PATCH 8/9 v13] difftool: teach difftool to handle directory diffs","startedAt":"2012-04-13T16:36:42Z","lastAt":"2012-04-20T16:58:18Z","messageCount":14,"participants":["Tim Henigan","David Aguilar","Junio C Hamano"],"isPatch":true,"patchVersion":13,"patchTotal":9},"messages":[{"id":"189198","messageId":"1334335002-30806-1-git-send-email-tim.henigan@gmail.com","threadId":"30232","inReplyTo":null,"subject":"[PATCH 8/9 v13] difftool: teach difftool to handle directory diffs","fromName":"Tim Henigan","fromEmail":"tim.henigan@gmail.com","sentAt":"2012-04-13T16:36:42Z","receivedAt":"2012-04-13T16:36:42Z","isPatch":true,"sender":{"key":"tim.henigan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42022?v=4"},"body":"When 'difftool' is called to compare a range of commits that modify\nmore than one file, it opens a separate instance of the diff tool for\neach file that changed.\n\nThe new '--dir-diff' option copies all the modified files to a temporary\nlocation and runs a directory diff on them in a single instance of the\ndiff tool.\n\nSigned-off-by: Tim Henigan <tim.henigan@gmail.com>\n---\n\nThis replaces v12 of the script that was sent to the list on April 12, 2011.\n\nChanges in v13:\n\nThe 'git diff' command is now called via 'Git->repository->command_oneline'\nagain. We need to run the command in a way that allows @ARGV to be given\nas a list, rather than a string, to insure that IFS and shell meta-\ncharacters are handled properly.  Thanks to Junio Hamano for pointing\nthis out [1].\n\n[1]: http://thread.gmane.org/gmane.comp.version-control.git/195326/focus=195353\n\n\n Documentation/git-difftool.txt |    6 ++\n git-difftool--helper.sh        |   19 ++--\n git-difftool.perl              |  219 ++++++++++++++++++++++++++++++++++++----\n t/t7800-difftool.sh            |   39 +++++++\n 4 files changed, 257 insertions(+), 26 deletions(-)\n\ndiff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt\nindex fe38f66..aba5e76 100644\n--- a/Documentation/git-difftool.txt\n+++ b/Documentation/git-difftool.txt\n@@ -19,6 +19,12 @@ linkgit:git-diff[1].\n \n OPTIONS\n -------\n+-d::\n+--dir-diff::\n+\tCopy the modified files to a temporary location and perform\n+\ta directory diff on them. This mode never prompts before\n+\tlaunching the diff tool.\n+\n -y::\n --no-prompt::\n \tDo not prompt before launching a diff tool.\ndiff --git a/git-difftool--helper.sh b/git-difftool--helper.sh\nindex e6558d1..3d0fe0c 100755\n--- a/git-difftool--helper.sh\n+++ b/git-difftool--helper.sh\n@@ -73,9 +73,16 @@ then\n \tfi\n fi\n \n-# Launch the merge tool on each path provided by 'git diff'\n-while test $# -gt 6\n-do\n-\tlaunch_merge_tool \"$1\" \"$2\" \"$5\"\n-\tshift 7\n-done\n+if test -n \"$GIT_DIFFTOOL_DIRDIFF\"\n+then\n+\tLOCAL=\"$1\"\n+\tREMOTE=\"$2\"\n+\trun_merge_tool \"$merge_tool\" false\n+else\n+\t# Launch the merge tool on each path provided by 'git diff'\n+\twhile test $# -gt 6\n+\tdo\n+\t\tlaunch_merge_tool \"$1\" \"$2\" \"$5\"\n+\t\tshift 7\n+\tdone\n+fi\ndiff --git a/git-difftool.perl b/git-difftool.perl\nindex aba3d2f..fc7a00b 100755\n--- a/git-difftool.perl\n+++ b/git-difftool.perl\n@@ -1,21 +1,31 @@\n-#!/usr/bin/env perl\n+#!/usr/bin/perl\n # Copyright (c) 2009, 2010 David Aguilar\n+# Copyright (c) 2012 Tim Henigan\n #\n # This is a wrapper around the GIT_EXTERNAL_DIFF-compatible\n # git-difftool--helper script.\n #\n # This script exports GIT_EXTERNAL_DIFF and GIT_PAGER for use by git.\n-# GIT_DIFFTOOL_NO_PROMPT, GIT_DIFFTOOL_PROMPT, and GIT_DIFF_TOOL\n-# are exported for use by git-difftool--helper.\n+# The GIT_DIFF* variables are exported for use by git-difftool--helper.\n #\n # Any arguments that are unknown to this script are forwarded to 'git diff'.\n \n use 5.008;\n use strict;\n use warnings;\n+use File::Basename qw(dirname);\n+use File::Copy;\n+use File::stat;\n+use File::Path qw(mkpath);\n+use File::Temp qw(tempdir);\n use Getopt::Long qw(:config pass_through);\n use Git;\n \n+my @working_tree;\n+my $rc;\n+my $repo = Git->repository();\n+my $repo_path = $repo->repo_path();\n+\n sub usage\n {\n \tmy $exitcode = shift;\n@@ -24,15 +34,160 @@ usage: git difftool [-t|--tool=<tool>]\n                     [-x|--extcmd=<cmd>]\n                     [-g|--gui] [--no-gui]\n                     [--prompt] [-y|--no-prompt]\n+                    [-d|--dir-diff]\n                     ['git diff' options]\n USAGE\n \texit($exitcode);\n }\n \n+sub find_worktree\n+{\n+\t# Git->repository->wc_path() does not honor changes to the working\n+\t# tree location made by $ENV{GIT_WORK_TREE} or the 'core.worktree'\n+\t# config variable.\n+\tmy $worktree;\n+\tmy $env_worktree = $ENV{GIT_WORK_TREE};\n+\tmy $core_worktree = Git::config('core.worktree');\n+\n+\tif (length($env_worktree) > 0) {\n+\t\t$worktree = $env_worktree;\n+\t} elsif (length($core_worktree) > 0) {\n+\t\t$worktree = $core_worktree;\n+\t} else {\n+\t\t$worktree = $repo->wc_path();\n+\t}\n+\n+\treturn $worktree;\n+}\n+\n+my $workdir = find_worktree();\n+\n+sub setup_dir_diff\n+{\n+\t# Run the diff; exit immediately if no diff found\n+\t# 'Repository' and 'WorkingCopy' must be explicitly set to insure that\n+\t# if $GIT_DIR and $GIT_WORK_TREE are set in ENV, they are actually used\n+\t# by Git->repository->command*.\n+\tmy $diffrepo = Git->repository(Repository => $repo_path, WorkingCopy => $workdir);\n+\tmy $diffrtn = $diffrepo->command_oneline('diff', '--raw', '--no-abbrev', '-z', @ARGV);\n+\texit(0) if (length($diffrtn) == 0);\n+\n+\t# Setup temp directories\n+\tmy $tmpdir = tempdir('git-diffall.XXXXX', CLEANUP => 1, TMPDIR => 1);\n+\tmy $ldir = \"$tmpdir/left\";\n+\tmy $rdir = \"$tmpdir/right\";\n+\tmkpath($ldir) or die $!;\n+\tmkpath($rdir) or die $!;\n+\n+\t# Build index info for left and right sides of the diff\n+\tmy $submodule_mode = \"160000\";\n+\tmy $null_mode = \"0\" x 6;\n+\tmy $null_sha1 = \"0\" x 40;\n+\tmy $lindex = \"\";\n+\tmy $rindex = \"\";\n+\tmy %submodule;\n+\tmy @rawdiff = split('\\0', $diffrtn);\n+\n+\tfor (my $i=0; $i<$#rawdiff; $i+=2) {\n+\t\tmy ($lmode, $rmode, $lsha1, $rsha1, $status) = split(' ', substr($rawdiff[$i], 1));\n+\t\tmy $path = $rawdiff[$i + 1];\n+\n+\t\tif (($lmode eq $submodule_mode) or ($rmode eq $submodule_mode)) {\n+\t\t\t$submodule{$path}{left} = $lsha1;\n+\t\t\tif ($lsha1 ne $rsha1) {\n+\t\t\t\t$submodule{$path}{right} = $rsha1;\n+\t\t\t} else {\n+\t\t\t\t$submodule{$path}{right} = \"$rsha1-dirty\";\n+\t\t\t}\n+\t\t\tnext;\n+\t\t}\n+\n+\t\tif ($lmode ne $null_mode) {\n+\t\t\t$lindex .= \"$lmode $lsha1\\t$path\\0\";\n+\t\t}\n+\n+\t\tif ($rmode ne $null_mode) {\n+\t\t\tif ($rsha1 ne $null_sha1) {\n+\t\t\t\t$rindex .= \"$rmode $rsha1\\t$path\\0\";\n+\t\t\t} else {\n+\t\t\t\tpush(@working_tree, $path);\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\t# If $GIT_DIR is not set prior to calling 'git update-index' and\n+\t# 'git checkout-index', then those commands will fail if difftool\n+\t# is called from a directory other than the repo root.\n+\tmy $must_unset_git_dir = 0;\n+\tif (not defined($ENV{GIT_DIR})) {\n+\t\t$must_unset_git_dir = 1;\n+\t\t$ENV{GIT_DIR} = $repo_path;\n+\t}\n+\n+\t# Populate the left and right directories based on each index file\n+\tmy ($inpipe, $ctx);\n+\t$ENV{GIT_INDEX_FILE} = \"$tmpdir/lindex\";\n+\t($inpipe, $ctx) = $repo->command_input_pipe(qw/update-index -z --index-info/);\n+\tprint($inpipe $lindex);\n+\t$repo->command_close_pipe($inpipe, $ctx);\n+\t$rc = system('git', 'checkout-index', '--all', \"--prefix=$ldir/\");\n+\texit($rc | ($rc >> 8)) if ($rc != 0);\n+\n+\t$ENV{GIT_INDEX_FILE} = \"$tmpdir/rindex\";\n+\t($inpipe, $ctx) = $repo->command_input_pipe(qw/update-index -z --index-info/);\n+\tprint($inpipe $rindex);\n+\t$repo->command_close_pipe($inpipe, $ctx);\n+\t$rc = system('git', 'checkout-index', '--all', \"--prefix=$rdir/\");\n+\texit($rc | ($rc >> 8)) if ($rc != 0);\n+\n+\t# If $GIT_DIR was explicitly set just for the update/checkout\n+\t# commands, then it should be unset before continuing.\n+\tdelete($ENV{GIT_DIR}) if ($must_unset_git_dir);\n+\tdelete($ENV{GIT_INDEX_FILE});\n+\n+\t# Changes in the working tree need special treatment since they are\n+\t# not part of the index\n+\tfor my $file (@working_tree) {\n+\t\tmy $dir = dirname($file);\n+\t\tunless (-d \"$rdir/$dir\") {\n+\t\t\tmkpath(\"$rdir/$dir\") or die $!;\n+\t\t}\n+\t\tcopy(\"$workdir/$file\", \"$rdir/$file\") or die $!;\n+\t\tchmod(stat(\"$workdir/$file\")->mode, \"$rdir/$file\") or die $!;\n+\t}\n+\n+\t# Changes to submodules require special treatment. This loop writes a\n+\t# temporary file to both the left and right directories to show the\n+\t# change in the recorded SHA1 for the submodule.\n+\tfor my $path (keys %submodule) {\n+\t\tif (defined($submodule{$path}{left})) {\n+\t\t\tmy $dir = dirname($path);\n+\t\t\tunless (-d \"$ldir/$dir\") {\n+\t\t\t\tmkpath(\"$ldir/$dir\") or die $!;\n+\t\t\t}\n+\t\t\topen(my $fh, \">\", \"$ldir/$path\") or die $!;\n+\t\t\tprint($fh \"Subproject commit $submodule{$path}{left}\");\n+\t\t\tclose($fh);\n+\t\t}\n+\t\tif (defined($submodule{$path}{right})) {\n+\t\t\tmy $dir = dirname($path);\n+\t\t\tunless (-d \"$rdir/$dir\") {\n+\t\t\t\tmkpath(\"$rdir/$dir\") or die $!;\n+\t\t\t}\n+\t\t\topen(my $fh, \">\", \"$rdir/$path\") or die $!;\n+\t\t\tprint($fh \"Subproject commit $submodule{$path}{right}\");\n+\t\t\tclose($fh);\n+\t\t}\n+\t}\n+\n+\treturn ($ldir, $rdir);\n+}\n+\n # parse command-line options. all unrecognized options and arguments\n # are passed through to the 'git diff' command.\n-my ($difftool_cmd, $extcmd, $gui, $help, $prompt);\n+my ($difftool_cmd, $dirdiff, $extcmd, $gui, $help, $prompt);\n GetOptions('g|gui!' => \\$gui,\n+\t'd|dir-diff' => \\$dirdiff,\n \t'h' => \\$help,\n \t'prompt!' => \\$prompt,\n \t'y' => sub { $prompt = 0; },\n@@ -65,22 +220,46 @@ if ($gui) {\n \t\t$ENV{GIT_DIFF_TOOL} = $guitool;\n \t}\n }\n-if (defined($prompt)) {\n-\tif ($prompt) {\n-\t\t$ENV{GIT_DIFFTOOL_PROMPT} = 'true';\n+\n+# In directory diff mode, 'git-difftool--helper' is called once\n+# to compare the a/b directories.  In file diff mode, 'git diff'\n+# will invoke a separate instance of 'git-difftool--helper' for\n+# each file that changed.\n+if (defined($dirdiff)) {\n+\tmy ($a, $b) = setup_dir_diff();\n+\tif (defined($extcmd)) {\n+\t\t$rc = system($extcmd, $a, $b);\n \t} else {\n-\t\t$ENV{GIT_DIFFTOOL_NO_PROMPT} = 'true';\n+\t\t$ENV{GIT_DIFFTOOL_DIRDIFF} = 'true';\n+\t\t$rc = system('git', 'difftool--helper', $a, $b);\n \t}\n-}\n \n-$ENV{GIT_PAGER} = '';\n-$ENV{GIT_EXTERNAL_DIFF} = 'git-difftool--helper';\n-my @command = ('git', 'diff', @ARGV);\n-\n-# ActiveState Perl for Win32 does not implement POSIX semantics of\n-# exec* system call. It just spawns the given executable and finishes\n-# the starting program, exiting with code 0.\n-# system will at least catch the errors returned by git diff,\n-# allowing the caller of git difftool better handling of failures.\n-my $rc = system(@command);\n-exit($rc | ($rc >> 8));\n+\texit($rc | ($rc >> 8)) if ($rc != 0);\n+\n+\t# If the diff including working copy files and those\n+\t# files were modified during the diff, then the changes\n+\t# should be copied back to the working tree\n+\tfor my $file (@working_tree) {\n+\t\tcopy(\"$b/$file\", \"$workdir/$file\") or die $!;\n+\t\tchmod(stat(\"$b/$file\")->mode, \"$workdir/$file\") or die $!;\n+\t}\n+} else {\n+\tif (defined($prompt)) {\n+\t\tif ($prompt) {\n+\t\t\t$ENV{GIT_DIFFTOOL_PROMPT} = 'true';\n+\t\t} else {\n+\t\t\t$ENV{GIT_DIFFTOOL_NO_PROMPT} = 'true';\n+\t\t}\n+\t}\n+\n+\t$ENV{GIT_PAGER} = '';\n+\t$ENV{GIT_EXTERNAL_DIFF} = 'git-difftool--helper';\n+\n+\t# ActiveState Perl for Win32 does not implement POSIX semantics of\n+\t# exec* system call. It just spawns the given executable and finishes\n+\t# the starting program, exiting with code 0.\n+\t# system will at least catch the errors returned by git diff,\n+\t# allowing the caller of git difftool better handling of failures.\n+\tmy $rc = system('git', 'diff', @ARGV);\n+\texit($rc | ($rc >> 8));\n+}\ndiff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\nindex e716d06..478c1be 100755\n--- a/t/t7800-difftool.sh\n+++ b/t/t7800-difftool.sh\n@@ -319,4 +319,43 @@ test_expect_success PERL 'say no to the second file' '\n \techo \"$diff\" | stdin_doesnot_contain br2\n '\n \n+test_expect_success PERL 'setup change in subdirectory' '\n+\tgit checkout master &&\n+\tmkdir sub &&\n+\techo master >sub/sub &&\n+\tgit add sub/sub &&\n+\tgit commit -m \"added sub/sub\" &&\n+\techo test >>file &&\n+\techo test >>sub/sub &&\n+\tgit add . &&\n+\tgit commit -m \"modified both\"\n+'\n+\n+test_expect_success PERL 'difftool -d' '\n+\tdiff=$(git difftool -d --extcmd ls branch) &&\n+\techo \"$diff\" | stdin_contains sub &&\n+\techo \"$diff\" | stdin_contains file\n+'\n+\n+test_expect_success PERL 'difftool --dir-diff' '\n+\tdiff=$(git difftool --dir-diff --extcmd ls branch) &&\n+\techo \"$diff\" | stdin_contains sub &&\n+\techo \"$diff\" | stdin_contains file\n+'\n+\n+test_expect_success PERL 'difftool --dir-diff ignores --prompt' '\n+\tdiff=$(git difftool --dir-diff --prompt --extcmd ls branch) &&\n+\techo \"$diff\" | stdin_contains sub &&\n+\techo \"$diff\" | stdin_contains file\n+'\n+\n+test_expect_success PERL 'difftool --dir-diff from subdirectory' '\n+\t(\n+\t\tcd sub &&\n+\t\tdiff=$(git difftool --dir-diff --extcmd ls branch) &&\n+\t\techo \"$diff\" | stdin_contains sub &&\n+\t\techo \"$diff\" | stdin_contains file\n+\t)\n+'\n+\n test_done\n-- \n1.7.10.rc1.26.g5a7d67\n"},{"id":"189364","messageId":"CAJDDKr7Uw3Nwg4p7F2zaY8f82j3_tRf3WiiO+YSN+nA6a9wY6w@mail.gmail.com","threadId":"30232","inReplyTo":"1334335002-30806-1-git-send-email-tim.henigan@gmail.com","subject":"Re: [PATCH 8/9 v13] difftool: teach difftool to handle directory diffs","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2012-04-15T22:20:22Z","receivedAt":"2012-04-15T22:20:22Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Fri, Apr 13, 2012 at 9:36 AM, Tim Henigan <tim.henigan@gmail.com> wrote:\n> When 'difftool' is called to compare a range of commits that modify\n> more than one file, it opens a separate instance of the diff tool for\n> each file that changed.\n>\n> The new '--dir-diff' option copies all the modified files to a temporary\n> location and runs a directory diff on them in a single instance of the\n> diff tool.\n>\n> Signed-off-by: Tim Henigan <tim.henigan@gmail.com>\n> ---\n>\n> This replaces v12 of the script that was sent to the list on April 12, 2011.\n>\n> Changes in v13:\n>\n> The 'git diff' command is now called via 'Git->repository->command_oneline'\n> again. We need to run the command in a way that allows @ARGV to be given\n> as a list, rather than a string, to insure that IFS and shell meta-\n> characters are handled properly.  Thanks to Junio Hamano for pointing\n> this out [1].\n>\n> [1]: http://thread.gmane.org/gmane.comp.version-control.git/195326/focus=195353\n\nThanks Tim.  Sorry for reading this patch out of context and missing\nthe obvious point that it needs the diff output to do something useful\nin my last review.\n\nI started testing this patch.  I started on the commit before what's\nin pu and then applied this patch:\n\n$ git checkout e9653615fafcbac6109da99fac4fa66b0b432048\n$ git am difftool.patch\n\nThe basics work and I know folks will be really happy when this\nfeature lands.  Folks have personally asked me for this feature in the\npast.  I dig it.  I'd also like to help pursue using symlinks sometime\nin the future if that sounds like a reasonable thing to you, but the\nstabilizing the existing implementation is more important right now.\n\nI ran into some issues when trying it against a few random commits.  I\nwent pretty far back in git's history to see what would happen.\n\n$ git difftool --dir-diff e5b06629de847663aaf0f7daae8de81338da3901 | tail\nUse of uninitialized value $rmode in string eq at\n/home/david/src/git/git-difftool line 96.\nUse of uninitialized value $lsha1 in concatenation (.) or string at\n/home/david/src/git/git-difftool line 107.\nUse of uninitialized value $rmode in string ne at\n/home/david/src/git/git-difftool line 110.\nUse of uninitialized value $rsha1 in string ne at\n/home/david/src/git/git-difftool line 111.\nUse of uninitialized value $rmode in concatenation (.) or string at\n/home/david/src/git/git-difftool line 112.\nUse of uninitialized value $rsha1 in concatenation (.) or string at\n/home/david/src/git/git-difftool line 112.\nUse of uninitialized value $rmode in string eq at\n/home/david/src/git/git-difftool line 96.\nUse of uninitialized value $lsha1 in concatenation (.) or string at\n/home/david/src/git/git-difftool line 107.\nUse of uninitialized value $rmode in string ne at\n/home/david/src/git/git-difftool line 110.\nUse of uninitialized value $rsha1 in string ne at\n/home/david/src/git/git-difftool line 111.\nUse of uninitialized value $rmode in concatenation (.) or string at\n/home/david/src/git/git-difftool line 112.\nUse of uninitialized value $rsha1 in concatenation (.) or string at\n/home/david/src/git/git-difftool line 112.\nUse of uninitialized value $rmode in string eq at\n/home/david/src/git/git-difftool line 96.\nUse of uninitialized value $lsha1 in concatenation (.) or string at\n/home/david/src/git/git-difftool line 107.\nUse of uninitialized value $rmode in string ne at\n/home/david/src/git/git-difftool line 110.\nUse of uninitialized value $rsha1 in string ne at\n/home/david/src/git/git-difftool line 111.\nUse of uninitialized value $rmode in concatenation (.) or string at\n/home/david/src/git/git-difftool line 112.\nUse of uninitialized value $rsha1 in concatenation (.) or string at\n/home/david/src/git/git-difftool line 112.\nfatal: malformed index info /t9800-git-p4-basic.sh \t:100755 100755\na25f18d36a196a4b85f6cac15a6a081744fe8fa1\nd41470541650590355bf0de1a1b556b3502492b5 M\nupdate-index -z --index-info: command returned error: 128\n\n\nThis one works fine but the difftool I used for testing (xxdiff)\ncomplained about a missing file:\n$ git difftool -d 86e15ff4fe9924b73af32d1bebe77eb5592b93cd\n\nYou can find more problematic commits by doing `git log -- xdiff`.  I\nwas originally going to test --dir-diff in a subdirectory (xdiff in\nthis example) and that's when I found these.  I don't think it has\nanything to do with subdirs; there's probably a commit in the history\nthat changes something (submodules? modes? not sure...) so once we go\nbeyond that point in the history it confuses --dir-diff.\n-- \nDavid\n"},{"id":"189366","messageId":"CAJDDKr78T1HNFXPPnvMUxBoJhAHP8XGdk9ZbpQCS1sZEQJfR8w@mail.gmail.com","threadId":"30232","inReplyTo":"CAJDDKr7Uw3Nwg4p7F2zaY8f82j3_tRf3WiiO+YSN+nA6a9wY6w@mail.gmail.com","subject":"Re: [PATCH 8/9 v13] difftool: teach difftool to handle directory diffs","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2012-04-16T01:01:06Z","receivedAt":"2012-04-16T01:01:06Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Sun, Apr 15, 2012 at 3:20 PM, David Aguilar <davvid@gmail.com> wrote:\n> On Fri, Apr 13, 2012 at 9:36 AM, Tim Henigan <tim.henigan@gmail.com> wrote:\n>> When 'difftool' is called to compare a range of commits that modify\n>> more than one file, it opens a separate instance of the diff tool for\n>> each file that changed.\n>>\n>> The new '--dir-diff' option copies all the modified files to a temporary\n>> location and runs a directory diff on them in a single instance of the\n>> diff tool.\n>>\n>> Signed-off-by: Tim Henigan <tim.henigan@gmail.com>\n>> ---\n>>\n>> This replaces v12 of the script that was sent to the list on April 12, 2011.\n>>\n>> Changes in v13:\n>>\n>> The 'git diff' command is now called via 'Git->repository->command_oneline'\n>> again. We need to run the command in a way that allows @ARGV to be given\n>> as a list, rather than a string, to insure that IFS and shell meta-\n>> characters are handled properly.  Thanks to Junio Hamano for pointing\n>> this out [1].\n>>\n>> [1]: http://thread.gmane.org/gmane.comp.version-control.git/195326/focus=195353\n>\n> Thanks Tim.  Sorry for reading this patch out of context and missing\n> the obvious point that it needs the diff output to do something useful\n> in my last review.\n>\n> I started testing this patch.  I started on the commit before what's\n> in pu and then applied this patch:\n>\n> $ git checkout e9653615fafcbac6109da99fac4fa66b0b432048\n> $ git am difftool.patch\n>\n> The basics work and I know folks will be really happy when this\n> feature lands.  Folks have personally asked me for this feature in the\n> past.  I dig it.  I'd also like to help pursue using symlinks sometime\n> in the future if that sounds like a reasonable thing to you, but the\n> stabilizing the existing implementation is more important right now.\n>\n> I ran into some issues when trying it against a few random commits.  I\n> went pretty far back in git's history to see what would happen.\n>\n> $ git difftool --dir-diff e5b06629de847663aaf0f7daae8de81338da3901 | tail\n> Use of uninitialized value $rmode in string eq at\n> /home/david/src/git/git-difftool line 96.\n\nI did some more investigating.  I think this happens when the diff\ncontains detected renames.\n\nThis command made it work:\n\n$ git difftool --dir-diff --no-renames e5b06629de847663aaf0f7daae8de81338da3901\n\nSo I think we might need to specify --no-renames when calling git diff --raw.\n\nxxdiff still gives an error message about \"$tmp/left/RelNotes: No such\nfile or directory\" with --no-renames so we may want to touch some\ndummy files to make the tools happy.\n-- \nDavid\n"},{"id":"189372","messageId":"CAJDDKr7dgW1-jyAuCw7raz47xtdUBuRscSq9pM=fvHe7CYJyDw@mail.gmail.com","threadId":"30232","inReplyTo":"CAJDDKr78T1HNFXPPnvMUxBoJhAHP8XGdk9ZbpQCS1sZEQJfR8w@mail.gmail.com","subject":"Re: [PATCH 8/9 v13] difftool: teach difftool to handle directory diffs","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2012-04-16T08:16:22Z","receivedAt":"2012-04-16T08:16:22Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Sun, Apr 15, 2012 at 6:01 PM, David Aguilar <davvid@gmail.com> wrote:\n> On Sun, Apr 15, 2012 at 3:20 PM, David Aguilar <davvid@gmail.com> wrote:\n>> On Fri, Apr 13, 2012 at 9:36 AM, Tim Henigan <tim.henigan@gmail.com> wrote:\n>>> When 'difftool' is called to compare a range of commits that modify\n>>> more than one file, it opens a separate instance of the diff tool for\n>>> each file that changed.\n>>>\n>>> The new '--dir-diff' option copies all the modified files to a temporary\n>>> location and runs a directory diff on them in a single instance of the\n>>> diff tool.\n>>>\n>>> Signed-off-by: Tim Henigan <tim.henigan@gmail.com>\n>>> ---\n>>>\n>>> This replaces v12 of the script that was sent to the list on April 12, 2011.\n>>>\n>>> Changes in v13:\n>>>\n>>> The 'git diff' command is now called via 'Git->repository->command_oneline'\n>>> again. We need to run the command in a way that allows @ARGV to be given\n>>> as a list, rather than a string, to insure that IFS and shell meta-\n>>> characters are handled properly.  Thanks to Junio Hamano for pointing\n>>> this out [1].\n>>>\n>>> [1]: http://thread.gmane.org/gmane.comp.version-control.git/195326/focus=195353\n>>\n>> Thanks Tim.  Sorry for reading this patch out of context and missing\n>> the obvious point that it needs the diff output to do something useful\n>> in my last review.\n>>\n>> I started testing this patch.  I started on the commit before what's\n>> in pu and then applied this patch:\n>>\n>> $ git checkout e9653615fafcbac6109da99fac4fa66b0b432048\n>> $ git am difftool.patch\n>>\n>> The basics work and I know folks will be really happy when this\n>> feature lands.  Folks have personally asked me for this feature in the\n>> past.  I dig it.  I'd also like to help pursue using symlinks sometime\n>> in the future if that sounds like a reasonable thing to you, but the\n>> stabilizing the existing implementation is more important right now.\n>>\n>> I ran into some issues when trying it against a few random commits.  I\n>> went pretty far back in git's history to see what would happen.\n>>\n>> $ git difftool --dir-diff e5b06629de847663aaf0f7daae8de81338da3901 | tail\n>> Use of uninitialized value $rmode in string eq at\n>> /home/david/src/git/git-difftool line 96.\n>\n> I did some more investigating.  I think this happens when the diff\n> contains detected renames.\n>\n> This command made it work:\n>\n> $ git difftool --dir-diff --no-renames e5b06629de847663aaf0f7daae8de81338da3901\n>\n> So I think we might need to specify --no-renames when calling git diff --raw.\n>\n> xxdiff still gives an error message about \"$tmp/left/RelNotes: No such\n> file or directory\" with --no-renames so we may want to touch some\n> dummy files to make the tools happy.\n\nEven better would be to parse the rename line in the diff output and\nmove/symlink the $left path to a location that corresponds to $right.\nThat way the diff tool will show a proper diff for renamed files.\n\nOne gotcha is that the $left side now has a name that didn't exist at\nthat commit, but it's a nicer experience then getting an empty file.\n\nRenaming brings along issues such as file paths becoming directories\nlater in the history, and vice versa.\n\nDeleted files may have similar issues to consider (I haven't checked yet).\n-- \nDavid\n"},{"id":"189520","messageId":"CAFoueth37aeHMorh-r2w_mwSp+uSgeF+PYbUfHNPy9-HVvL01w@mail.gmail.com","threadId":"30232","inReplyTo":"CAJDDKr78T1HNFXPPnvMUxBoJhAHP8XGdk9ZbpQCS1sZEQJfR8w@mail.gmail.com","subject":"Re: [PATCH 8/9 v13] difftool: teach difftool to handle directory diffs","fromName":"Tim Henigan","fromEmail":"tim.henigan@gmail.com","sentAt":"2012-04-17T13:25:26Z","receivedAt":"2012-04-17T13:25:26Z","isPatch":true,"sender":{"key":"tim.henigan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42022?v=4"},"body":"On Sun, Apr 15, 2012 at 9:01 PM, David Aguilar <davvid@gmail.com> wrote:\n> On Sun, Apr 15, 2012 at 3:20 PM, David Aguilar <davvid@gmail.com> wrote:\n>> On Fri, Apr 13, 2012 at 9:36 AM, Tim Henigan <tim.henigan@gmail.com> wrote:\n>>\n>> I started testing this patch.  I started on the commit before what's\n>> in pu and then applied this patch:\n>>\n>> $ git checkout e9653615fafcbac6109da99fac4fa66b0b432048\n>> $ git am difftool.patch\n>>\n>> The basics work and I know folks will be really happy when this\n>> feature lands.  Folks have personally asked me for this feature in the\n>> past.  I dig it.  I'd also like to help pursue using symlinks sometime\n>> in the future if that sounds like a reasonable thing to you, but the\n>> stabilizing the existing implementation is more important right now.\n\nI appreciate everyone's patience as this feature continues to develop.\n It has not gone as smoothly as I hoped ;)\n\n\n>> I ran into some issues when trying it against a few random commits.  I\n>> went pretty far back in git's history to see what would happen.\n>>\n>> $ git difftool --dir-diff e5b06629de847663aaf0f7daae8de81338da3901 | tail\n>> Use of uninitialized value $rmode in string eq at\n>> /home/david/src/git/git-difftool line 96.\n\nI ran the same test using both my local branch [1] and the tip of\nJunio's th/difftool-diffall branch [2].   In both cases, I see the\n\"RelNotes: no such file\" error (more on that below), but I do not see\nuninitialized value errors.\n\nThe section of code that reads the mode, SHA1 and path info from the\ndiff output looks like this:\n\n    my @rawdiff = split('\\0', $diffrtn);\n\n    for (my $i=0; $i<$#rawdiff; $i+=2) {\n        my ($lmode, $rmode, $lsha1, $rsha1, $status) = split(' ',\nsubstr($rawdiff[$i], 1));\n        ...\n\nIf the \"$lmode, ...\" variables are not defined, then the output of\n'git diff --raw' is malformed in some way (perhaps some error?).  At\nthe very end of your output, I saw this as well:\n\n>> fatal: malformed index info /t9800-git-p4-basic.sh      :100755 100755\n>> a25f18d36a196a4b85f6cac15a6a081744fe8fa1\n>> d41470541650590355bf0de1a1b556b3502492b5 M\n>> update-index -z --index-info: command returned error: 128\n\nWould it be possible for you to try either Junio's or my branch to\ninsure something did not go wrong with your locally applied patch?\nAlso, which platform are you testing on?\n\n\n> I did some more investigating.  I think this happens when the diff\n> contains detected renames.\n>\n> This command made it work:\n>\n> $ git difftool --dir-diff --no-renames e5b06629de847663aaf0f7daae8de81338da3901\n>\n> So I think we might need to specify --no-renames when calling git diff --raw.\n\nOne of my test repos includes added, removed and renamed files.  They\ndo not cause any errors.  Just like the output of 'git diff --raw', in\n'git difftool --dir-diff' renamed files are shown as the old file name\nbeing deleted and the new file name being added.\n\nI also looked to see if 'git diff --raw' reports a different result\nwhen '--no-renames' is used, but did not see any difference:\n\n    $ git diff --raw e5b0662 > diff_with_renames.txt\n    $ git diff --raw --no-renames e5b0662 > diff_without_renames.txt\n    $ diff diff_with_renames.txt diff_without_renames.txt\n\n\n> xxdiff still gives an error message about \"$tmp/left/RelNotes: No such\n> file or directory\" with --no-renames so we may want to touch some\n> dummy files to make the tools happy.\n\nI get the same error.  I looked into it and found that \"RelNotes\" is a\nsymbolic link.  If we look at the standard diff of the example you\ngave, we see the following:\n\n    $ git diff e5b0662 -- RelNotes\n    diff --git a/RelNotes b/RelNotes\n    index 7d92769..2c2a169 120000\n    --- a/RelNotes\n    +++ b/RelNotes\n    @@ -1 +1 @@\n    -Documentation/RelNotes/1.7.8.txt\n    \\ No newline at end of file\n    +Documentation/RelNotes/1.7.10.txt\n    \\ No newline at end of file\n\nWhen 'git-difftool' executes this command, it sees that the \"RelNotes\"\nfile changed, but it is not smart enough to copy the link target.  So\nthe \"/tmp/git.diffall.XXXXX/left\" directory has \"RelNotes\" in it, but\nnot the target of the link \"Documentation/RelNotes/1.7.8.txt\".\n\nIn xxdiff, this manifests as a \"file not found\" error.  In meld, it is\nshown as a \"Dangling symlink\".\n\nI will look into ways to deal with this, probably adding special logic\nto deal with file modes of \"120000\".\n\n[1]: https://github.com/thenigan/git/tree/th/difftool-phase2\n[2]: https://github.com/gitster/git/tree/th/difftool-diffall\n"},{"id":"189600","messageId":"CAJDDKr6djdBvUbV6qZZu75iR2UbFHt8_D0+V+K_C+-Dgx8BfVA@mail.gmail.com","threadId":"30232","inReplyTo":"CAFoueth37aeHMorh-r2w_mwSp+uSgeF+PYbUfHNPy9-HVvL01w@mail.gmail.com","subject":"Re: [PATCH 8/9 v13] difftool: teach difftool to handle directory diffs","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2012-04-18T03:23:33Z","receivedAt":"2012-04-18T03:23:33Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Tue, Apr 17, 2012 at 6:25 AM, Tim Henigan <tim.henigan@gmail.com> wrote:\n> On Sun, Apr 15, 2012 at 9:01 PM, David Aguilar <davvid@gmail.com> wrote:\n>> On Sun, Apr 15, 2012 at 3:20 PM, David Aguilar <davvid@gmail.com> wrote:\n>>> On Fri, Apr 13, 2012 at 9:36 AM, Tim Henigan <tim.henigan@gmail.com> wrote:\n>>>\n>>> I started testing this patch.  I started on the commit before what's\n>>> in pu and then applied this patch:\n>>>\n>>> $ git checkout e9653615fafcbac6109da99fac4fa66b0b432048\n>>> $ git am difftool.patch\n>>>\n>>> The basics work and I know folks will be really happy when this\n>>> feature lands.  Folks have personally asked me for this feature in the\n>>> past.  I dig it.  I'd also like to help pursue using symlinks sometime\n>>> in the future if that sounds like a reasonable thing to you, but the\n>>> stabilizing the existing implementation is more important right now.\n>\n> I appreciate everyone's patience as this feature continues to develop.\n>  It has not gone as smoothly as I hoped ;)\n>\n>\n>>> I ran into some issues when trying it against a few random commits.  I\n>>> went pretty far back in git's history to see what would happen.\n>>>\n>>> $ git difftool --dir-diff e5b06629de847663aaf0f7daae8de81338da3901 | tail\n>>> Use of uninitialized value $rmode in string eq at\n>>> /home/david/src/git/git-difftool line 96.\n>\n> I ran the same test using both my local branch [1] and the tip of\n> Junio's th/difftool-diffall branch [2].   In both cases, I see the\n> \"RelNotes: no such file\" error (more on that below), but I do not see\n> uninitialized value errors.\n>\n> The section of code that reads the mode, SHA1 and path info from the\n> diff output looks like this:\n>\n>    my @rawdiff = split('\\0', $diffrtn);\n>\n>    for (my $i=0; $i<$#rawdiff; $i+=2) {\n>        my ($lmode, $rmode, $lsha1, $rsha1, $status) = split(' ',\n> substr($rawdiff[$i], 1));\n>        ...\n>\n> If the \"$lmode, ...\" variables are not defined, then the output of\n> 'git diff --raw' is malformed in some way (perhaps some error?).  At\n> the very end of your output, I saw this as well:\n>\n>>> fatal: malformed index info /t9800-git-p4-basic.sh      :100755 100755\n>>> a25f18d36a196a4b85f6cac15a6a081744fe8fa1\n>>> d41470541650590355bf0de1a1b556b3502492b5 M\n>>> update-index -z --index-info: command returned error: 128\n>\n> Would it be possible for you to try either Junio's or my branch to\n> insure something did not go wrong with your locally applied patch?\n> Also, which platform are you testing on?\n\nLinux.  I also test on OS X occasionally, but it's not my primary platform.\n\nI think I narrowed it down.  I have this in my ~/.gitconfig\n--\n[diff]\n    renames = copy\n--\nThat means we should be able to reproduce this by doing:\n\n    $ git difftool --dir-diff -M -C baf5aaa33383af656a34b7ba9039e9eb3c9e678c\n\nThat ends up calling `git diff --raw -M -C\nbaf5aaa33383af656a34b7ba9039e9eb3c9e678c`, whose output contains:\n\n...[snip]...\n:000000 100755 0000000... 05824fa... A  t/lib-gpg.sh\n:100644 100644 83855fa... 83855fa... R100       t/t7004/pubring.gpg\n t/lib-gpg/pubring.gpg\n:100644 100644 8fed133... 8fed133... R100       t/t7004/random_seed\n t/lib-gpg/random_seed\n:100644 100644 d831cd9... d831cd9... R100       t/t7004/secring.gpg\n t/lib-gpg/secring.gpg\n:100644 100644 abace96... abace96... R100       t/t7004/trustdb.gpg\n t/lib-gpg/trustdb.gpg\n:100644 100644 3f24384... f7dc078... M  t/lib-httpd.sh\n...[snip]...\n\nI suspect the R100 lines are the ones that are throwing it off.\n\n\n>> xxdiff still gives an error message about \"$tmp/left/RelNotes: No such\n>> file or directory\" with --no-renames so we may want to touch some\n>> dummy files to make the tools happy.\n>\n> I get the same error.  I looked into it and found that \"RelNotes\" is a\n> symbolic link.  If we look at the standard diff of the example you\n> gave, we see the following:\n>\n>    $ git diff e5b0662 -- RelNotes\n>    diff --git a/RelNotes b/RelNotes\n>    index 7d92769..2c2a169 120000\n>    --- a/RelNotes\n>    +++ b/RelNotes\n>    @@ -1 +1 @@\n>    -Documentation/RelNotes/1.7.8.txt\n>    \\ No newline at end of file\n>    +Documentation/RelNotes/1.7.10.txt\n>    \\ No newline at end of file\n>\n> When 'git-difftool' executes this command, it sees that the \"RelNotes\"\n> file changed, but it is not smart enough to copy the link target.  So\n> the \"/tmp/git.diffall.XXXXX/left\" directory has \"RelNotes\" in it, but\n> not the target of the link \"Documentation/RelNotes/1.7.8.txt\".\n>\n> In xxdiff, this manifests as a \"file not found\" error.  In meld, it is\n> shown as a \"Dangling symlink\".\n>\n> I will look into ways to deal with this, probably adding special logic\n> to deal with file modes of \"120000\".\n\nSounds reasonable.\n\nThanks Tim,\n-- \nDavid\n"},{"id":"189626","messageId":"CAFouetjbHewYzQXZr33xGKgwk0k7D8R0XfoP7k2qAV6Nq_d+Ow@mail.gmail.com","threadId":"30232","inReplyTo":"CAJDDKr6djdBvUbV6qZZu75iR2UbFHt8_D0+V+K_C+-Dgx8BfVA@mail.gmail.com","subject":"Re: [PATCH 8/9 v13] difftool: teach difftool to handle directory diffs","fromName":"Tim Henigan","fromEmail":"tim.henigan@gmail.com","sentAt":"2012-04-18T13:13:02Z","receivedAt":"2012-04-18T13:13:02Z","isPatch":true,"sender":{"key":"tim.henigan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42022?v=4"},"body":"On Tue, Apr 17, 2012 at 11:23 PM, David Aguilar <davvid@gmail.com> wrote:\n> On Tue, Apr 17, 2012 at 6:25 AM, Tim Henigan <tim.henigan@gmail.com> wrote:\n>> On Sun, Apr 15, 2012 at 9:01 PM, David Aguilar <davvid@gmail.com> wrote:\n>>> On Sun, Apr 15, 2012 at 3:20 PM, David Aguilar <davvid@gmail.com> wrote:\n>>>> On Fri, Apr 13, 2012 at 9:36 AM, Tim Henigan <tim.henigan@gmail.com> wrote:\n>\n> I think I narrowed it down.  I have this in my ~/.gitconfig\n> --\n> [diff]\n>    renames = copy\n> --\n> That means we should be able to reproduce this by doing:\n>\n>    $ git difftool --dir-diff -M -C baf5aaa33383af656a34b7ba9039e9eb3c9e678c\n>\n> That ends up calling `git diff --raw -M -C\n> baf5aaa33383af656a34b7ba9039e9eb3c9e678c`, whose output contains:\n>\n> ...[snip]...\n> :000000 100755 0000000... 05824fa... A  t/lib-gpg.sh\n> :100644 100644 83855fa... 83855fa... R100       t/t7004/pubring.gpg\n>  t/lib-gpg/pubring.gpg\n> :100644 100644 8fed133... 8fed133... R100       t/t7004/random_seed\n>  t/lib-gpg/random_seed\n> :100644 100644 d831cd9... d831cd9... R100       t/t7004/secring.gpg\n>  t/lib-gpg/secring.gpg\n> :100644 100644 abace96... abace96... R100       t/t7004/trustdb.gpg\n>  t/lib-gpg/trustdb.gpg\n> :100644 100644 3f24384... f7dc078... M  t/lib-httpd.sh\n> ...[snip]...\n>\n> I suspect the R100 lines are the ones that are throwing it off.\n\nIt is indeed the lines that show the renames/copies that cause the\nerror.  Adding the '-C' or '-M' option to 'git diff --raw' changes the\ndiff output for copied/modified files from:\n\n    :<mode> <mode> <SHA1> <SHA1> <status> <file>\nto\n    :<mode> <mode> <SHA1> <SHA1> <status> <file before> <file after>\n\nThe difftool code assumes that the diff output will be of the first\nform and fails when given the second form.\n\nSo now we must decide how to handle deal with this use case.  It seems\nthere are two options:\n\n1) Append '--no-renames' to the end of the 'git diff --raw' argument\nlist.  This will override any '-C' or '-M' settings.  This is a simple\nsolution, but it loses some information about copies and renames.\n\n2) Add new logic to parse copies and renames.  Your earlier email\nadvocated this approach, but I am concerned that the implementation\nwill include some tough choices.\n\nSo, I would like to simply amend the current patch to include solution\n1.  Solution 2 can be considered in the future.\n\nDoes this sound reasonable?\n"},{"id":"189633","messageId":"7vsjg1knwr.fsf@alter.siamese.dyndns.org","threadId":"30232","inReplyTo":"CAFouetjbHewYzQXZr33xGKgwk0k7D8R0XfoP7k2qAV6Nq_d+Ow@mail.gmail.com","subject":"Re: [PATCH 8/9 v13] difftool: teach difftool to handle directory diffs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-04-18T16:25:24Z","receivedAt":"2012-04-18T16:25:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tim Henigan <tim.henigan@gmail.com> writes:\n\n> So now we must decide how to handle deal with this use case.  It seems\n> there are two options:\n>\n> 1) Append '--no-renames' to the end of the 'git diff --raw' argument\n> list.  This will override any '-C' or '-M' settings.  This is a simple\n> solution, but it loses some information about copies and renames.\n\nOr not use Porcelain \"git diff\", but use the plumbing \"git diff-index\" or\n\"git diff-files\" so that you won't get bitten by such end user settings.\n\nIn either case, this \"feature\", by feeding two entire trees to an external\nprogram, makes it the responsibility of that external program to match up\nfiles in these two trees, so we shouldn't be doing rename detection\nourselves at all.\n"},{"id":"189639","messageId":"CAFouetgWpyUC9SPo_QwpESrbfib7ct111WesKPP14HQ+SqpFaQ@mail.gmail.com","threadId":"30232","inReplyTo":"7vsjg1knwr.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 8/9 v13] difftool: teach difftool to handle directory diffs","fromName":"Tim Henigan","fromEmail":"tim.henigan@gmail.com","sentAt":"2012-04-18T18:28:18Z","receivedAt":"2012-04-18T18:28:18Z","isPatch":true,"sender":{"key":"tim.henigan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42022?v=4"},"body":"On Wed, Apr 18, 2012 at 12:25 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Tim Henigan <tim.henigan@gmail.com> writes:\n>\n>> So now we must decide how to handle deal with this use case.  It seems\n>> there are two options:\n>>\n>> 1) Append '--no-renames' to the end of the 'git diff --raw' argument\n>> list.  This will override any '-C' or '-M' settings.  This is a simple\n>> solution, but it loses some information about copies and renames.\n>\n> Or not use Porcelain \"git diff\", but use the plumbing \"git diff-index\" or\n> \"git diff-files\" so that you won't get bitten by such end user settings.\n\nLooking back on it now, I agree that it would have been better to use\nthe plumbing commands from the beginning.  Changing from the porcelain\nto the plumbing commands will require new logic to parse the diff\noptions to figure out which of 'diff-index', 'diff-files' or\n'diff-tree' should be called.  We may also want to add support for\nsome specific standard diff options (like '-R').\n\nFor now, would you object to an updated patch that simply detects and\nignores options that change the output of 'git diff --raw'?  Or do you\nthink that we need to switch to the plumbing commands before the\ndirectory diff feature can be called stable?\n\nI was planning to look for the following:\n    --find-renames (and -M)\n    --find-copies (and -C)\n    --cc (and -c)\n\nIf any of the above are detected, 'difftool' would print a warning\nthat the option is not supported and then prune it from the arguments\npassed to 'git diff --raw'.\n\n\n> In either case, this \"feature\", by feeding two entire trees to an external\n> program, makes it the responsibility of that external program to match up\n> files in these two trees, so we shouldn't be doing rename detection\n> ourselves at all.\n\nI agree that we should not try to do it in the 'difftool' command.\nUnfortunately, it appears that none of the external tools can detect\nrenames or copies.\n"},{"id":"189649","messageId":"7v8vhsltk3.fsf@alter.siamese.dyndns.org","threadId":"30232","inReplyTo":"CAFouetgWpyUC9SPo_QwpESrbfib7ct111WesKPP14HQ+SqpFaQ@mail.gmail.com","subject":"Re: [PATCH 8/9 v13] difftool: teach difftool to handle directory diffs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-04-18T19:38:04Z","receivedAt":"2012-04-18T19:38:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tim Henigan <tim.henigan@gmail.com> writes:\n\n> Looking back on it now, I agree that it would have been better to use\n> the plumbing commands from the beginning.  Changing from the porcelain\n> to the plumbing commands will require new logic to parse the diff\n> options to figure out which of 'diff-index', 'diff-files' or\n> 'diff-tree' should be called.  We may also want to add support for\n> some specific standard diff options (like '-R').\n\nYeah, didn't I already suggest that it is the only sane avenue in the long\nterm to move the whole \"populate the two temporary trees\" thing down to C\nlevel?\n\n> For now, would you object to an updated patch that simply detects and\n> ignores options that change the output of 'git diff --raw'?\n\nAs a script that uses 'git diff' is a short-term hack anyway, I think the\nmost cost effective thing to do is to add '--no-renames' at the end and be\ndone with it.\n"},{"id":"189712","messageId":"CAFouetg6T1pgAiTfyAeSxseR-k_omsZDfqv8X8AifekwPLoE2g@mail.gmail.com","threadId":"30232","inReplyTo":"7v8vhsltk3.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 8/9 v13] difftool: teach difftool to handle directory diffs","fromName":"Tim Henigan","fromEmail":"tim.henigan@gmail.com","sentAt":"2012-04-19T17:11:00Z","receivedAt":"2012-04-19T17:11:00Z","isPatch":true,"sender":{"key":"tim.henigan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42022?v=4"},"body":"On Wed, Apr 18, 2012 at 3:38 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Tim Henigan <tim.henigan@gmail.com> writes:\n>\n>> For now, would you object to an updated patch that simply detects and\n>> ignores options that change the output of 'git diff --raw'?\n>\n> As a script that uses 'git diff' is a short-term hack anyway, I think the\n> most cost effective thing to do is to add '--no-renames' at the end and be\n> done with it.\n\nAdding '--no-renames' has no effect if the user specifies '-C -C' or\n'--find-copies-harder'.  Is protecting for these cases too paranoid?\n\nAlso, the '--cc' option for viewing merge diffs is not affected by\n'--no-renames'.\n\nI have a revised patch that prunes out all of the above and warns the\nuser when it does so [1].\n\nHowever, it also prunes them when difftool is called in serial diff\nmode (i.e. non --dir-diff).  Before, if 'difftool\n--find-[renames|copies]' was called it would open the external tool to\ncompare the two files, but the original file name was used for both\nsides of the diff.\n\nThis seems confusing, but I don't know if people rely on that\nbehavior.  If we need to keep that behavior in the serial diff mode, I\nwill need to modify the patch again to only prune the options in\ndirectory diff mode.\n\n[1]: https://github.com/thenigan/git/commit/c3479940a36f3c7c8fe360bc244303b125f711ff\n"},{"id":"189720","messageId":"xmqqy5prv9ol.fsf@junio.mtv.corp.google.com","threadId":"30232","inReplyTo":"CAFouetg6T1pgAiTfyAeSxseR-k_omsZDfqv8X8AifekwPLoE2g@mail.gmail.com","subject":"Re: [PATCH 8/9 v13] difftool: teach difftool to handle directory diffs","fromName":"Junio C Hamano","fromEmail":"jch@google.com","sentAt":"2012-04-19T18:49:30Z","receivedAt":"2012-04-19T18:49:30Z","isPatch":true,"sender":{"key":"jch@google.com","avatar":null},"body":"Tim Henigan <tim.henigan@gmail.com> writes:\n\n> I have a revised patch that prunes out all of the above and warns the\n> user when it does so [1].\n\nThanks.\n\nAs long as it works when the user uses \"two temporary trees\" mode\nwithout -M/-C, and it keeps working as well as before the change when\nthe user uses \"one invocation per matched path\" mode with -M/-C, I do\nnot care too deeply about how it is implemented in the script.\n\n> However, it also prunes them when difftool is called in serial diff\n> mode (i.e. non --dir-diff).\n\nI do not use difftool myself, but I would imagine that it is a grave\nregression, no?\n"},{"id":"189741","messageId":"CAJDDKr7JtauR8sR3YC+wj60sx9DEgf87iaDwue2Cz6FzQX_Z+Q@mail.gmail.com","threadId":"30232","inReplyTo":"xmqqy5prv9ol.fsf@junio.mtv.corp.google.com","subject":"Re: [PATCH 8/9 v13] difftool: teach difftool to handle directory diffs","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2012-04-20T07:34:53Z","receivedAt":"2012-04-20T07:34:53Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Thu, Apr 19, 2012 at 11:49 AM, Junio C Hamano <jch@google.com> wrote:\n> Tim Henigan <tim.henigan@gmail.com> writes:\n>\n>> I have a revised patch that prunes out all of the above and warns the\n>> user when it does so [1].\n>\n> Thanks.\n>\n> As long as it works when the user uses \"two temporary trees\" mode\n> without -M/-C, and it keeps working as well as before the change when\n> the user uses \"one invocation per matched path\" mode with -M/-C, I do\n> not care too deeply about how it is implemented in the script.\n>\n>> However, it also prunes them when difftool is called in serial diff\n>> mode (i.e. non --dir-diff).\n>\n> I do not use difftool myself, but I would imagine that it is a grave\n> regression, no?\n\nAn alternative would be teaching difftool to gracefully handle the R\nlines in the --raw output.\n\nSimply creating the files as they existed on each side is a reasonable\nstart.  It seems better form to handle all possible output from --raw\nanyways.  It's better than pretending it doesn't exist, I think.\n\nDoing \"smart\" things with the rename information is hairy so it's\ncertainly worth leaving that for another day.\n-- \nDavid\n"},{"id":"189766","messageId":"CAFouetiAgJzEhYFpXF5Tgr--VMRYqweQtn4t-QFvDBaDHDTQXg@mail.gmail.com","threadId":"30232","inReplyTo":"CAJDDKr7JtauR8sR3YC+wj60sx9DEgf87iaDwue2Cz6FzQX_Z+Q@mail.gmail.com","subject":"Re: [PATCH 8/9 v13] difftool: teach difftool to handle directory diffs","fromName":"Tim Henigan","fromEmail":"tim.henigan@gmail.com","sentAt":"2012-04-20T16:58:18Z","receivedAt":"2012-04-20T16:58:18Z","isPatch":true,"sender":{"key":"tim.henigan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42022?v=4"},"body":"On Fri, Apr 20, 2012 at 3:34 AM, David Aguilar <davvid@gmail.com> wrote:\n>\n> An alternative would be teaching difftool to gracefully handle the R\n> lines in the --raw output.\n>\n> Simply creating the files as they existed on each side is a reasonable\n> start.  It seems better form to handle all possible output from --raw\n> anyways.  It's better than pretending it doesn't exist, I think.\n\nI like this idea best.  I will send v14 (lucky 14!) with this change.\n\n\n> Doing \"smart\" things with the rename information is hairy so it's\n> certainly worth leaving that for another day.\n\nAgreed.\n"}]}