All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 8/9 v13] difftool: teach difftool to handle directory diffs
@ 2012-04-13 16:36 Tim Henigan
  2012-04-15 22:20 ` David Aguilar
  0 siblings, 1 reply; 14+ messages in thread
From: Tim Henigan @ 2012-04-13 16:36 UTC (permalink / raw)
  To: gitster, git, davvid, ramsay; +Cc: Tim Henigan

When 'difftool' is called to compare a range of commits that modify
more than one file, it opens a separate instance of the diff tool for
each file that changed.

The new '--dir-diff' option copies all the modified files to a temporary
location and runs a directory diff on them in a single instance of the
diff tool.

Signed-off-by: Tim Henigan <tim.henigan@gmail.com>
---

This replaces v12 of the script that was sent to the list on April 12, 2011.

Changes in v13:

The 'git diff' command is now called via 'Git->repository->command_oneline'
again. We need to run the command in a way that allows @ARGV to be given
as a list, rather than a string, to insure that IFS and shell meta-
characters are handled properly.  Thanks to Junio Hamano for pointing
this out [1].

[1]: http://thread.gmane.org/gmane.comp.version-control.git/195326/focus=195353


 Documentation/git-difftool.txt |    6 ++
 git-difftool--helper.sh        |   19 ++--
 git-difftool.perl              |  219 ++++++++++++++++++++++++++++++++++++----
 t/t7800-difftool.sh            |   39 +++++++
 4 files changed, 257 insertions(+), 26 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..3d0fe0c 100755
--- a/git-difftool--helper.sh
+++ b/git-difftool--helper.sh
@@ -73,9 +73,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 aba3d2f..fc7a00b 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -1,21 +1,31 @@
-#!/usr/bin/env perl
+#!/usr/bin/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.
+# The GIT_DIFF* variables 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;
 
+my @working_tree;
+my $rc;
+my $repo = Git->repository();
+my $repo_path = $repo->repo_path();
+
 sub usage
 {
 	my $exitcode = shift;
@@ -24,15 +34,160 @@ usage: git difftool [-t|--tool=<tool>]
                     [-x|--extcmd=<cmd>]
                     [-g|--gui] [--no-gui]
                     [--prompt] [-y|--no-prompt]
+                    [-d|--dir-diff]
                     ['git diff' options]
 USAGE
 	exit($exitcode);
 }
 
+sub find_worktree
+{
+	# 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.
+	my $worktree;
+	my $env_worktree = $ENV{GIT_WORK_TREE};
+	my $core_worktree = Git::config('core.worktree');
+
+	if (length($env_worktree) > 0) {
+		$worktree = $env_worktree;
+	} elsif (length($core_worktree) > 0) {
+		$worktree = $core_worktree;
+	} else {
+		$worktree = $repo->wc_path();
+	}
+
+	return $worktree;
+}
+
+my $workdir = find_worktree();
+
+sub setup_dir_diff
+{
+	# 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 $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);
+
+	# 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 %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;
+			if ($lsha1 ne $rsha1) {
+				$submodule{$path}{right} = $rsha1;
+			} else {
+				$submodule{$path}{right} = "$rsha1-dirty";
+			}
+			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);
+			}
+		}
+	}
+
+	# If $GIT_DIR is not set prior to calling 'git update-index' and
+	# 'git checkout-index', then those commands will fail if difftool
+	# is called from a directory other than the repo root.
+	my $must_unset_git_dir = 0;
+	if (not defined($ENV{GIT_DIR})) {
+		$must_unset_git_dir = 1;
+		$ENV{GIT_DIR} = $repo_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);
+	$rc = system('git', 'checkout-index', '--all', "--prefix=$ldir/");
+	exit($rc | ($rc >> 8)) if ($rc != 0);
+
+	$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);
+	$rc = system('git', 'checkout-index', '--all', "--prefix=$rdir/");
+	exit($rc | ($rc >> 8)) if ($rc != 0);
+
+	# If $GIT_DIR was explicitly set just for the update/checkout
+	# commands, then it should be unset before continuing.
+	delete($ENV{GIT_DIR}) if ($must_unset_git_dir);
+	delete($ENV{GIT_INDEX_FILE});
+
+	# Changes in the working tree need special treatment since they are
+	# not part of the index
+	for my $file (@working_tree) {
+		my $dir = dirname($file);
+		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 $!;
+	}
+
+	# 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.
+	for my $path (keys %submodule) {
+		if (defined($submodule{$path}{left})) {
+			my $dir = dirname($path);
+			unless (-d "$ldir/$dir") {
+				mkpath("$ldir/$dir") or die $!;
+			}
+			open(my $fh, ">", "$ldir/$path") or die $!;
+			print($fh "Subproject commit $submodule{$path}{left}");
+			close($fh);
+		}
+		if (defined($submodule{$path}{right})) {
+			my $dir = dirname($path);
+			unless (-d "$rdir/$dir") {
+				mkpath("$rdir/$dir") or die $!;
+			}
+			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, $prompt);
+my ($difftool_cmd, $dirdiff, $extcmd, $gui, $help, $prompt);
 GetOptions('g|gui!' => \$gui,
+	'd|dir-diff' => \$dirdiff,
 	'h' => \$help,
 	'prompt!' => \$prompt,
 	'y' => sub { $prompt = 0; },
@@ -65,22 +220,46 @@ if ($gui) {
 		$ENV{GIT_DIFF_TOOL} = $guitool;
 	}
 }
-if (defined($prompt)) {
-	if ($prompt) {
-		$ENV{GIT_DIFFTOOL_PROMPT} = 'true';
+
+# 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)) {
+		$rc = system($extcmd, $a, $b);
 	} else {
-		$ENV{GIT_DIFFTOOL_NO_PROMPT} = 'true';
+		$ENV{GIT_DIFFTOOL_DIRDIFF} = 'true';
+		$rc = system('git', 'difftool--helper', $a, $b);
 	}
-}
 
-$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));
+	exit($rc | ($rc >> 8)) if ($rc != 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) {
+		copy("$b/$file", "$workdir/$file") or die $!;
+		chmod(stat("$b/$file")->mode, "$workdir/$file") or die $!;
+	}
+} else {
+	if (defined($prompt)) {
+		if ($prompt) {
+			$ENV{GIT_DIFFTOOL_PROMPT} = 'true';
+		} else {
+			$ENV{GIT_DIFFTOOL_NO_PROMPT} = 'true';
+		}
+	}
+
+	$ENV{GIT_PAGER} = '';
+	$ENV{GIT_EXTERNAL_DIFF} = 'git-difftool--helper';
+
+	# 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('git', 'diff', @ARGV);
+	exit($rc | ($rc >> 8));
+}
diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index e716d06..478c1be 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -319,4 +319,43 @@ test_expect_success PERL 'say no to the second file' '
 	echo "$diff" | stdin_doesnot_contain br2
 '
 
+test_expect_success PERL 'setup change in subdirectory' '
+	git checkout master &&
+	mkdir sub &&
+	echo master >sub/sub &&
+	git add sub/sub &&
+	git commit -m "added sub/sub" &&
+	echo test >>file &&
+	echo test >>sub/sub &&
+	git add . &&
+	git commit -m "modified both"
+'
+
+test_expect_success PERL 'difftool -d' '
+	diff=$(git difftool -d --extcmd ls branch) &&
+	echo "$diff" | stdin_contains sub &&
+	echo "$diff" | stdin_contains file
+'
+
+test_expect_success PERL 'difftool --dir-diff' '
+	diff=$(git difftool --dir-diff --extcmd ls branch) &&
+	echo "$diff" | stdin_contains sub &&
+	echo "$diff" | stdin_contains file
+'
+
+test_expect_success PERL 'difftool --dir-diff ignores --prompt' '
+	diff=$(git difftool --dir-diff --prompt --extcmd ls branch) &&
+	echo "$diff" | stdin_contains sub &&
+	echo "$diff" | stdin_contains file
+'
+
+test_expect_success PERL 'difftool --dir-diff from subdirectory' '
+	(
+		cd sub &&
+		diff=$(git difftool --dir-diff --extcmd ls branch) &&
+		echo "$diff" | stdin_contains sub &&
+		echo "$diff" | stdin_contains file
+	)
+'
+
 test_done
-- 
1.7.10.rc1.26.g5a7d67

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 8/9 v13] difftool: teach difftool to handle directory diffs
  2012-04-13 16:36 [PATCH 8/9 v13] difftool: teach difftool to handle directory diffs Tim Henigan
@ 2012-04-15 22:20 ` David Aguilar
  2012-04-16  1:01   ` David Aguilar
  0 siblings, 1 reply; 14+ messages in thread
From: David Aguilar @ 2012-04-15 22:20 UTC (permalink / raw)
  To: Tim Henigan; +Cc: gitster, git, ramsay

On Fri, Apr 13, 2012 at 9:36 AM, Tim Henigan <tim.henigan@gmail.com> wrote:
> When 'difftool' is called to compare a range of commits that modify
> more than one file, it opens a separate instance of the diff tool for
> each file that changed.
>
> The new '--dir-diff' option copies all the modified files to a temporary
> location and runs a directory diff on them in a single instance of the
> diff tool.
>
> Signed-off-by: Tim Henigan <tim.henigan@gmail.com>
> ---
>
> This replaces v12 of the script that was sent to the list on April 12, 2011.
>
> Changes in v13:
>
> The 'git diff' command is now called via 'Git->repository->command_oneline'
> again. We need to run the command in a way that allows @ARGV to be given
> as a list, rather than a string, to insure that IFS and shell meta-
> characters are handled properly.  Thanks to Junio Hamano for pointing
> this out [1].
>
> [1]: http://thread.gmane.org/gmane.comp.version-control.git/195326/focus=195353

Thanks Tim.  Sorry for reading this patch out of context and missing
the obvious point that it needs the diff output to do something useful
in my last review.

I started testing this patch.  I started on the commit before what's
in pu and then applied this patch:

$ git checkout e9653615fafcbac6109da99fac4fa66b0b432048
$ git am difftool.patch

The basics work and I know folks will be really happy when this
feature lands.  Folks have personally asked me for this feature in the
past.  I dig it.  I'd also like to help pursue using symlinks sometime
in the future if that sounds like a reasonable thing to you, but the
stabilizing the existing implementation is more important right now.

I ran into some issues when trying it against a few random commits.  I
went pretty far back in git's history to see what would happen.

$ git difftool --dir-diff e5b06629de847663aaf0f7daae8de81338da3901 | tail
Use of uninitialized value $rmode in string eq at
/home/david/src/git/git-difftool line 96.
Use of uninitialized value $lsha1 in concatenation (.) or string at
/home/david/src/git/git-difftool line 107.
Use of uninitialized value $rmode in string ne at
/home/david/src/git/git-difftool line 110.
Use of uninitialized value $rsha1 in string ne at
/home/david/src/git/git-difftool line 111.
Use of uninitialized value $rmode in concatenation (.) or string at
/home/david/src/git/git-difftool line 112.
Use of uninitialized value $rsha1 in concatenation (.) or string at
/home/david/src/git/git-difftool line 112.
Use of uninitialized value $rmode in string eq at
/home/david/src/git/git-difftool line 96.
Use of uninitialized value $lsha1 in concatenation (.) or string at
/home/david/src/git/git-difftool line 107.
Use of uninitialized value $rmode in string ne at
/home/david/src/git/git-difftool line 110.
Use of uninitialized value $rsha1 in string ne at
/home/david/src/git/git-difftool line 111.
Use of uninitialized value $rmode in concatenation (.) or string at
/home/david/src/git/git-difftool line 112.
Use of uninitialized value $rsha1 in concatenation (.) or string at
/home/david/src/git/git-difftool line 112.
Use of uninitialized value $rmode in string eq at
/home/david/src/git/git-difftool line 96.
Use of uninitialized value $lsha1 in concatenation (.) or string at
/home/david/src/git/git-difftool line 107.
Use of uninitialized value $rmode in string ne at
/home/david/src/git/git-difftool line 110.
Use of uninitialized value $rsha1 in string ne at
/home/david/src/git/git-difftool line 111.
Use of uninitialized value $rmode in concatenation (.) or string at
/home/david/src/git/git-difftool line 112.
Use of uninitialized value $rsha1 in concatenation (.) or string at
/home/david/src/git/git-difftool line 112.
fatal: malformed index info /t9800-git-p4-basic.sh 	:100755 100755
a25f18d36a196a4b85f6cac15a6a081744fe8fa1
d41470541650590355bf0de1a1b556b3502492b5 M
update-index -z --index-info: command returned error: 128


This one works fine but the difftool I used for testing (xxdiff)
complained about a missing file:
$ git difftool -d 86e15ff4fe9924b73af32d1bebe77eb5592b93cd

You can find more problematic commits by doing `git log -- xdiff`.  I
was originally going to test --dir-diff in a subdirectory (xdiff in
this example) and that's when I found these.  I don't think it has
anything to do with subdirs; there's probably a commit in the history
that changes something (submodules? modes? not sure...) so once we go
beyond that point in the history it confuses --dir-diff.
-- 
David

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 8/9 v13] difftool: teach difftool to handle directory diffs
  2012-04-15 22:20 ` David Aguilar
@ 2012-04-16  1:01   ` David Aguilar
  2012-04-16  8:16     ` David Aguilar
  2012-04-17 13:25     ` Tim Henigan
  0 siblings, 2 replies; 14+ messages in thread
From: David Aguilar @ 2012-04-16  1:01 UTC (permalink / raw)
  To: Tim Henigan; +Cc: gitster, git, ramsay

On Sun, Apr 15, 2012 at 3:20 PM, David Aguilar <davvid@gmail.com> wrote:
> On Fri, Apr 13, 2012 at 9:36 AM, Tim Henigan <tim.henigan@gmail.com> wrote:
>> When 'difftool' is called to compare a range of commits that modify
>> more than one file, it opens a separate instance of the diff tool for
>> each file that changed.
>>
>> The new '--dir-diff' option copies all the modified files to a temporary
>> location and runs a directory diff on them in a single instance of the
>> diff tool.
>>
>> Signed-off-by: Tim Henigan <tim.henigan@gmail.com>
>> ---
>>
>> This replaces v12 of the script that was sent to the list on April 12, 2011.
>>
>> Changes in v13:
>>
>> The 'git diff' command is now called via 'Git->repository->command_oneline'
>> again. We need to run the command in a way that allows @ARGV to be given
>> as a list, rather than a string, to insure that IFS and shell meta-
>> characters are handled properly.  Thanks to Junio Hamano for pointing
>> this out [1].
>>
>> [1]: http://thread.gmane.org/gmane.comp.version-control.git/195326/focus=195353
>
> Thanks Tim.  Sorry for reading this patch out of context and missing
> the obvious point that it needs the diff output to do something useful
> in my last review.
>
> I started testing this patch.  I started on the commit before what's
> in pu and then applied this patch:
>
> $ git checkout e9653615fafcbac6109da99fac4fa66b0b432048
> $ git am difftool.patch
>
> The basics work and I know folks will be really happy when this
> feature lands.  Folks have personally asked me for this feature in the
> past.  I dig it.  I'd also like to help pursue using symlinks sometime
> in the future if that sounds like a reasonable thing to you, but the
> stabilizing the existing implementation is more important right now.
>
> I ran into some issues when trying it against a few random commits.  I
> went pretty far back in git's history to see what would happen.
>
> $ git difftool --dir-diff e5b06629de847663aaf0f7daae8de81338da3901 | tail
> Use of uninitialized value $rmode in string eq at
> /home/david/src/git/git-difftool line 96.

I did some more investigating.  I think this happens when the diff
contains detected renames.

This command made it work:

$ git difftool --dir-diff --no-renames e5b06629de847663aaf0f7daae8de81338da3901

So I think we might need to specify --no-renames when calling git diff --raw.

xxdiff still gives an error message about "$tmp/left/RelNotes: No such
file or directory" with --no-renames so we may want to touch some
dummy files to make the tools happy.
-- 
David

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 8/9 v13] difftool: teach difftool to handle directory diffs
  2012-04-16  1:01   ` David Aguilar
@ 2012-04-16  8:16     ` David Aguilar
  2012-04-17 13:25     ` Tim Henigan
  1 sibling, 0 replies; 14+ messages in thread
From: David Aguilar @ 2012-04-16  8:16 UTC (permalink / raw)
  To: Tim Henigan; +Cc: gitster, git, ramsay

On Sun, Apr 15, 2012 at 6:01 PM, David Aguilar <davvid@gmail.com> wrote:
> On Sun, Apr 15, 2012 at 3:20 PM, David Aguilar <davvid@gmail.com> wrote:
>> On Fri, Apr 13, 2012 at 9:36 AM, Tim Henigan <tim.henigan@gmail.com> wrote:
>>> When 'difftool' is called to compare a range of commits that modify
>>> more than one file, it opens a separate instance of the diff tool for
>>> each file that changed.
>>>
>>> The new '--dir-diff' option copies all the modified files to a temporary
>>> location and runs a directory diff on them in a single instance of the
>>> diff tool.
>>>
>>> Signed-off-by: Tim Henigan <tim.henigan@gmail.com>
>>> ---
>>>
>>> This replaces v12 of the script that was sent to the list on April 12, 2011.
>>>
>>> Changes in v13:
>>>
>>> The 'git diff' command is now called via 'Git->repository->command_oneline'
>>> again. We need to run the command in a way that allows @ARGV to be given
>>> as a list, rather than a string, to insure that IFS and shell meta-
>>> characters are handled properly.  Thanks to Junio Hamano for pointing
>>> this out [1].
>>>
>>> [1]: http://thread.gmane.org/gmane.comp.version-control.git/195326/focus=195353
>>
>> Thanks Tim.  Sorry for reading this patch out of context and missing
>> the obvious point that it needs the diff output to do something useful
>> in my last review.
>>
>> I started testing this patch.  I started on the commit before what's
>> in pu and then applied this patch:
>>
>> $ git checkout e9653615fafcbac6109da99fac4fa66b0b432048
>> $ git am difftool.patch
>>
>> The basics work and I know folks will be really happy when this
>> feature lands.  Folks have personally asked me for this feature in the
>> past.  I dig it.  I'd also like to help pursue using symlinks sometime
>> in the future if that sounds like a reasonable thing to you, but the
>> stabilizing the existing implementation is more important right now.
>>
>> I ran into some issues when trying it against a few random commits.  I
>> went pretty far back in git's history to see what would happen.
>>
>> $ git difftool --dir-diff e5b06629de847663aaf0f7daae8de81338da3901 | tail
>> Use of uninitialized value $rmode in string eq at
>> /home/david/src/git/git-difftool line 96.
>
> I did some more investigating.  I think this happens when the diff
> contains detected renames.
>
> This command made it work:
>
> $ git difftool --dir-diff --no-renames e5b06629de847663aaf0f7daae8de81338da3901
>
> So I think we might need to specify --no-renames when calling git diff --raw.
>
> xxdiff still gives an error message about "$tmp/left/RelNotes: No such
> file or directory" with --no-renames so we may want to touch some
> dummy files to make the tools happy.

Even better would be to parse the rename line in the diff output and
move/symlink the $left path to a location that corresponds to $right.
That way the diff tool will show a proper diff for renamed files.

One gotcha is that the $left side now has a name that didn't exist at
that commit, but it's a nicer experience then getting an empty file.

Renaming brings along issues such as file paths becoming directories
later in the history, and vice versa.

Deleted files may have similar issues to consider (I haven't checked yet).
-- 
David

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 8/9 v13] difftool: teach difftool to handle directory diffs
  2012-04-16  1:01   ` David Aguilar
  2012-04-16  8:16     ` David Aguilar
@ 2012-04-17 13:25     ` Tim Henigan
  2012-04-18  3:23       ` David Aguilar
  1 sibling, 1 reply; 14+ messages in thread
From: Tim Henigan @ 2012-04-17 13:25 UTC (permalink / raw)
  To: David Aguilar; +Cc: gitster, git, ramsay

On Sun, Apr 15, 2012 at 9:01 PM, David Aguilar <davvid@gmail.com> wrote:
> On Sun, Apr 15, 2012 at 3:20 PM, David Aguilar <davvid@gmail.com> wrote:
>> On Fri, Apr 13, 2012 at 9:36 AM, Tim Henigan <tim.henigan@gmail.com> wrote:
>>
>> I started testing this patch.  I started on the commit before what's
>> in pu and then applied this patch:
>>
>> $ git checkout e9653615fafcbac6109da99fac4fa66b0b432048
>> $ git am difftool.patch
>>
>> The basics work and I know folks will be really happy when this
>> feature lands.  Folks have personally asked me for this feature in the
>> past.  I dig it.  I'd also like to help pursue using symlinks sometime
>> in the future if that sounds like a reasonable thing to you, but the
>> stabilizing the existing implementation is more important right now.

I appreciate everyone's patience as this feature continues to develop.
 It has not gone as smoothly as I hoped ;)


>> I ran into some issues when trying it against a few random commits.  I
>> went pretty far back in git's history to see what would happen.
>>
>> $ git difftool --dir-diff e5b06629de847663aaf0f7daae8de81338da3901 | tail
>> Use of uninitialized value $rmode in string eq at
>> /home/david/src/git/git-difftool line 96.

I ran the same test using both my local branch [1] and the tip of
Junio's th/difftool-diffall branch [2].   In both cases, I see the
"RelNotes: no such file" error (more on that below), but I do not see
uninitialized value errors.

The section of code that reads the mode, SHA1 and path info from the
diff output looks like this:

    my @rawdiff = split('\0', $diffrtn);

    for (my $i=0; $i<$#rawdiff; $i+=2) {
        my ($lmode, $rmode, $lsha1, $rsha1, $status) = split(' ',
substr($rawdiff[$i], 1));
        ...

If the "$lmode, ..." variables are not defined, then the output of
'git diff --raw' is malformed in some way (perhaps some error?).  At
the very end of your output, I saw this as well:

>> fatal: malformed index info /t9800-git-p4-basic.sh      :100755 100755
>> a25f18d36a196a4b85f6cac15a6a081744fe8fa1
>> d41470541650590355bf0de1a1b556b3502492b5 M
>> update-index -z --index-info: command returned error: 128

Would it be possible for you to try either Junio's or my branch to
insure something did not go wrong with your locally applied patch?
Also, which platform are you testing on?


> I did some more investigating.  I think this happens when the diff
> contains detected renames.
>
> This command made it work:
>
> $ git difftool --dir-diff --no-renames e5b06629de847663aaf0f7daae8de81338da3901
>
> So I think we might need to specify --no-renames when calling git diff --raw.

One of my test repos includes added, removed and renamed files.  They
do not cause any errors.  Just like the output of 'git diff --raw', in
'git difftool --dir-diff' renamed files are shown as the old file name
being deleted and the new file name being added.

I also looked to see if 'git diff --raw' reports a different result
when '--no-renames' is used, but did not see any difference:

    $ git diff --raw e5b0662 > diff_with_renames.txt
    $ git diff --raw --no-renames e5b0662 > diff_without_renames.txt
    $ diff diff_with_renames.txt diff_without_renames.txt


> xxdiff still gives an error message about "$tmp/left/RelNotes: No such
> file or directory" with --no-renames so we may want to touch some
> dummy files to make the tools happy.

I get the same error.  I looked into it and found that "RelNotes" is a
symbolic link.  If we look at the standard diff of the example you
gave, we see the following:

    $ git diff e5b0662 -- RelNotes
    diff --git a/RelNotes b/RelNotes
    index 7d92769..2c2a169 120000
    --- a/RelNotes
    +++ b/RelNotes
    @@ -1 +1 @@
    -Documentation/RelNotes/1.7.8.txt
    \ No newline at end of file
    +Documentation/RelNotes/1.7.10.txt
    \ No newline at end of file

When 'git-difftool' executes this command, it sees that the "RelNotes"
file changed, but it is not smart enough to copy the link target.  So
the "/tmp/git.diffall.XXXXX/left" directory has "RelNotes" in it, but
not the target of the link "Documentation/RelNotes/1.7.8.txt".

In xxdiff, this manifests as a "file not found" error.  In meld, it is
shown as a "Dangling symlink".

I will look into ways to deal with this, probably adding special logic
to deal with file modes of "120000".

[1]: https://github.com/thenigan/git/tree/th/difftool-phase2
[2]: https://github.com/gitster/git/tree/th/difftool-diffall

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 8/9 v13] difftool: teach difftool to handle directory diffs
  2012-04-17 13:25     ` Tim Henigan
@ 2012-04-18  3:23       ` David Aguilar
  2012-04-18 13:13         ` Tim Henigan
  0 siblings, 1 reply; 14+ messages in thread
From: David Aguilar @ 2012-04-18  3:23 UTC (permalink / raw)
  To: Tim Henigan; +Cc: gitster, git, ramsay

On Tue, Apr 17, 2012 at 6:25 AM, Tim Henigan <tim.henigan@gmail.com> wrote:
> On Sun, Apr 15, 2012 at 9:01 PM, David Aguilar <davvid@gmail.com> wrote:
>> On Sun, Apr 15, 2012 at 3:20 PM, David Aguilar <davvid@gmail.com> wrote:
>>> On Fri, Apr 13, 2012 at 9:36 AM, Tim Henigan <tim.henigan@gmail.com> wrote:
>>>
>>> I started testing this patch.  I started on the commit before what's
>>> in pu and then applied this patch:
>>>
>>> $ git checkout e9653615fafcbac6109da99fac4fa66b0b432048
>>> $ git am difftool.patch
>>>
>>> The basics work and I know folks will be really happy when this
>>> feature lands.  Folks have personally asked me for this feature in the
>>> past.  I dig it.  I'd also like to help pursue using symlinks sometime
>>> in the future if that sounds like a reasonable thing to you, but the
>>> stabilizing the existing implementation is more important right now.
>
> I appreciate everyone's patience as this feature continues to develop.
>  It has not gone as smoothly as I hoped ;)
>
>
>>> I ran into some issues when trying it against a few random commits.  I
>>> went pretty far back in git's history to see what would happen.
>>>
>>> $ git difftool --dir-diff e5b06629de847663aaf0f7daae8de81338da3901 | tail
>>> Use of uninitialized value $rmode in string eq at
>>> /home/david/src/git/git-difftool line 96.
>
> I ran the same test using both my local branch [1] and the tip of
> Junio's th/difftool-diffall branch [2].   In both cases, I see the
> "RelNotes: no such file" error (more on that below), but I do not see
> uninitialized value errors.
>
> The section of code that reads the mode, SHA1 and path info from the
> diff output looks like this:
>
>    my @rawdiff = split('\0', $diffrtn);
>
>    for (my $i=0; $i<$#rawdiff; $i+=2) {
>        my ($lmode, $rmode, $lsha1, $rsha1, $status) = split(' ',
> substr($rawdiff[$i], 1));
>        ...
>
> If the "$lmode, ..." variables are not defined, then the output of
> 'git diff --raw' is malformed in some way (perhaps some error?).  At
> the very end of your output, I saw this as well:
>
>>> fatal: malformed index info /t9800-git-p4-basic.sh      :100755 100755
>>> a25f18d36a196a4b85f6cac15a6a081744fe8fa1
>>> d41470541650590355bf0de1a1b556b3502492b5 M
>>> update-index -z --index-info: command returned error: 128
>
> Would it be possible for you to try either Junio's or my branch to
> insure something did not go wrong with your locally applied patch?
> Also, which platform are you testing on?

Linux.  I also test on OS X occasionally, but it's not my primary platform.

I think I narrowed it down.  I have this in my ~/.gitconfig
--
[diff]
    renames = copy
--
That means we should be able to reproduce this by doing:

    $ git difftool --dir-diff -M -C baf5aaa33383af656a34b7ba9039e9eb3c9e678c

That ends up calling `git diff --raw -M -C
baf5aaa33383af656a34b7ba9039e9eb3c9e678c`, whose output contains:

...[snip]...
:000000 100755 0000000... 05824fa... A  t/lib-gpg.sh
:100644 100644 83855fa... 83855fa... R100       t/t7004/pubring.gpg
 t/lib-gpg/pubring.gpg
:100644 100644 8fed133... 8fed133... R100       t/t7004/random_seed
 t/lib-gpg/random_seed
:100644 100644 d831cd9... d831cd9... R100       t/t7004/secring.gpg
 t/lib-gpg/secring.gpg
:100644 100644 abace96... abace96... R100       t/t7004/trustdb.gpg
 t/lib-gpg/trustdb.gpg
:100644 100644 3f24384... f7dc078... M  t/lib-httpd.sh
...[snip]...

I suspect the R100 lines are the ones that are throwing it off.


>> xxdiff still gives an error message about "$tmp/left/RelNotes: No such
>> file or directory" with --no-renames so we may want to touch some
>> dummy files to make the tools happy.
>
> I get the same error.  I looked into it and found that "RelNotes" is a
> symbolic link.  If we look at the standard diff of the example you
> gave, we see the following:
>
>    $ git diff e5b0662 -- RelNotes
>    diff --git a/RelNotes b/RelNotes
>    index 7d92769..2c2a169 120000
>    --- a/RelNotes
>    +++ b/RelNotes
>    @@ -1 +1 @@
>    -Documentation/RelNotes/1.7.8.txt
>    \ No newline at end of file
>    +Documentation/RelNotes/1.7.10.txt
>    \ No newline at end of file
>
> When 'git-difftool' executes this command, it sees that the "RelNotes"
> file changed, but it is not smart enough to copy the link target.  So
> the "/tmp/git.diffall.XXXXX/left" directory has "RelNotes" in it, but
> not the target of the link "Documentation/RelNotes/1.7.8.txt".
>
> In xxdiff, this manifests as a "file not found" error.  In meld, it is
> shown as a "Dangling symlink".
>
> I will look into ways to deal with this, probably adding special logic
> to deal with file modes of "120000".

Sounds reasonable.

Thanks Tim,
-- 
David

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 8/9 v13] difftool: teach difftool to handle directory diffs
  2012-04-18  3:23       ` David Aguilar
@ 2012-04-18 13:13         ` Tim Henigan
  2012-04-18 16:25           ` Junio C Hamano
  0 siblings, 1 reply; 14+ messages in thread
From: Tim Henigan @ 2012-04-18 13:13 UTC (permalink / raw)
  To: David Aguilar; +Cc: gitster, git, ramsay

On Tue, Apr 17, 2012 at 11:23 PM, David Aguilar <davvid@gmail.com> wrote:
> On Tue, Apr 17, 2012 at 6:25 AM, Tim Henigan <tim.henigan@gmail.com> wrote:
>> On Sun, Apr 15, 2012 at 9:01 PM, David Aguilar <davvid@gmail.com> wrote:
>>> On Sun, Apr 15, 2012 at 3:20 PM, David Aguilar <davvid@gmail.com> wrote:
>>>> On Fri, Apr 13, 2012 at 9:36 AM, Tim Henigan <tim.henigan@gmail.com> wrote:
>
> I think I narrowed it down.  I have this in my ~/.gitconfig
> --
> [diff]
>    renames = copy
> --
> That means we should be able to reproduce this by doing:
>
>    $ git difftool --dir-diff -M -C baf5aaa33383af656a34b7ba9039e9eb3c9e678c
>
> That ends up calling `git diff --raw -M -C
> baf5aaa33383af656a34b7ba9039e9eb3c9e678c`, whose output contains:
>
> ...[snip]...
> :000000 100755 0000000... 05824fa... A  t/lib-gpg.sh
> :100644 100644 83855fa... 83855fa... R100       t/t7004/pubring.gpg
>  t/lib-gpg/pubring.gpg
> :100644 100644 8fed133... 8fed133... R100       t/t7004/random_seed
>  t/lib-gpg/random_seed
> :100644 100644 d831cd9... d831cd9... R100       t/t7004/secring.gpg
>  t/lib-gpg/secring.gpg
> :100644 100644 abace96... abace96... R100       t/t7004/trustdb.gpg
>  t/lib-gpg/trustdb.gpg
> :100644 100644 3f24384... f7dc078... M  t/lib-httpd.sh
> ...[snip]...
>
> I suspect the R100 lines are the ones that are throwing it off.

It is indeed the lines that show the renames/copies that cause the
error.  Adding the '-C' or '-M' option to 'git diff --raw' changes the
diff output for copied/modified files from:

    :<mode> <mode> <SHA1> <SHA1> <status> <file>
to
    :<mode> <mode> <SHA1> <SHA1> <status> <file before> <file after>

The difftool code assumes that the diff output will be of the first
form and fails when given the second form.

So now we must decide how to handle deal with this use case.  It seems
there are two options:

1) Append '--no-renames' to the end of the 'git diff --raw' argument
list.  This will override any '-C' or '-M' settings.  This is a simple
solution, but it loses some information about copies and renames.

2) Add new logic to parse copies and renames.  Your earlier email
advocated this approach, but I am concerned that the implementation
will include some tough choices.

So, I would like to simply amend the current patch to include solution
1.  Solution 2 can be considered in the future.

Does this sound reasonable?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 8/9 v13] difftool: teach difftool to handle directory diffs
  2012-04-18 13:13         ` Tim Henigan
@ 2012-04-18 16:25           ` Junio C Hamano
  2012-04-18 18:28             ` Tim Henigan
  0 siblings, 1 reply; 14+ messages in thread
From: Junio C Hamano @ 2012-04-18 16:25 UTC (permalink / raw)
  To: Tim Henigan; +Cc: David Aguilar, git, ramsay

Tim Henigan <tim.henigan@gmail.com> writes:

> So now we must decide how to handle deal with this use case.  It seems
> there are two options:
>
> 1) Append '--no-renames' to the end of the 'git diff --raw' argument
> list.  This will override any '-C' or '-M' settings.  This is a simple
> solution, but it loses some information about copies and renames.

Or not use Porcelain "git diff", but use the plumbing "git diff-index" or
"git diff-files" so that you won't get bitten by such end user settings.

In either case, this "feature", by feeding two entire trees to an external
program, makes it the responsibility of that external program to match up
files in these two trees, so we shouldn't be doing rename detection
ourselves at all.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 8/9 v13] difftool: teach difftool to handle directory diffs
  2012-04-18 16:25           ` Junio C Hamano
@ 2012-04-18 18:28             ` Tim Henigan
  2012-04-18 19:38               ` Junio C Hamano
  0 siblings, 1 reply; 14+ messages in thread
From: Tim Henigan @ 2012-04-18 18:28 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: David Aguilar, git, ramsay

On Wed, Apr 18, 2012 at 12:25 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Tim Henigan <tim.henigan@gmail.com> writes:
>
>> So now we must decide how to handle deal with this use case.  It seems
>> there are two options:
>>
>> 1) Append '--no-renames' to the end of the 'git diff --raw' argument
>> list.  This will override any '-C' or '-M' settings.  This is a simple
>> solution, but it loses some information about copies and renames.
>
> Or not use Porcelain "git diff", but use the plumbing "git diff-index" or
> "git diff-files" so that you won't get bitten by such end user settings.

Looking back on it now, I agree that it would have been better to use
the plumbing commands from the beginning.  Changing from the porcelain
to the plumbing commands will require new logic to parse the diff
options to figure out which of 'diff-index', 'diff-files' or
'diff-tree' should be called.  We may also want to add support for
some specific standard diff options (like '-R').

For now, would you object to an updated patch that simply detects and
ignores options that change the output of 'git diff --raw'?  Or do you
think that we need to switch to the plumbing commands before the
directory diff feature can be called stable?

I was planning to look for the following:
    --find-renames (and -M)
    --find-copies (and -C)
    --cc (and -c)

If any of the above are detected, 'difftool' would print a warning
that the option is not supported and then prune it from the arguments
passed to 'git diff --raw'.


> In either case, this "feature", by feeding two entire trees to an external
> program, makes it the responsibility of that external program to match up
> files in these two trees, so we shouldn't be doing rename detection
> ourselves at all.

I agree that we should not try to do it in the 'difftool' command.
Unfortunately, it appears that none of the external tools can detect
renames or copies.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 8/9 v13] difftool: teach difftool to handle directory diffs
  2012-04-18 18:28             ` Tim Henigan
@ 2012-04-18 19:38               ` Junio C Hamano
  2012-04-19 17:11                 ` Tim Henigan
  0 siblings, 1 reply; 14+ messages in thread
From: Junio C Hamano @ 2012-04-18 19:38 UTC (permalink / raw)
  To: Tim Henigan; +Cc: David Aguilar, git, ramsay

Tim Henigan <tim.henigan@gmail.com> writes:

> Looking back on it now, I agree that it would have been better to use
> the plumbing commands from the beginning.  Changing from the porcelain
> to the plumbing commands will require new logic to parse the diff
> options to figure out which of 'diff-index', 'diff-files' or
> 'diff-tree' should be called.  We may also want to add support for
> some specific standard diff options (like '-R').

Yeah, didn't I already suggest that it is the only sane avenue in the long
term to move the whole "populate the two temporary trees" thing down to C
level?

> For now, would you object to an updated patch that simply detects and
> ignores options that change the output of 'git diff --raw'?

As a script that uses 'git diff' is a short-term hack anyway, I think the
most cost effective thing to do is to add '--no-renames' at the end and be
done with it.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 8/9 v13] difftool: teach difftool to handle directory diffs
  2012-04-18 19:38               ` Junio C Hamano
@ 2012-04-19 17:11                 ` Tim Henigan
  2012-04-19 18:49                   ` Junio C Hamano
  0 siblings, 1 reply; 14+ messages in thread
From: Tim Henigan @ 2012-04-19 17:11 UTC (permalink / raw)
  To: Junio C Hamano, David Aguilar; +Cc: git, ramsay

On Wed, Apr 18, 2012 at 3:38 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Tim Henigan <tim.henigan@gmail.com> writes:
>
>> For now, would you object to an updated patch that simply detects and
>> ignores options that change the output of 'git diff --raw'?
>
> As a script that uses 'git diff' is a short-term hack anyway, I think the
> most cost effective thing to do is to add '--no-renames' at the end and be
> done with it.

Adding '--no-renames' has no effect if the user specifies '-C -C' or
'--find-copies-harder'.  Is protecting for these cases too paranoid?

Also, the '--cc' option for viewing merge diffs is not affected by
'--no-renames'.

I have a revised patch that prunes out all of the above and warns the
user when it does so [1].

However, it also prunes them when difftool is called in serial diff
mode (i.e. non --dir-diff).  Before, if 'difftool
--find-[renames|copies]' was called it would open the external tool to
compare the two files, but the original file name was used for both
sides of the diff.

This seems confusing, but I don't know if people rely on that
behavior.  If we need to keep that behavior in the serial diff mode, I
will need to modify the patch again to only prune the options in
directory diff mode.

[1]: https://github.com/thenigan/git/commit/c3479940a36f3c7c8fe360bc244303b125f711ff

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 8/9 v13] difftool: teach difftool to handle directory diffs
  2012-04-19 17:11                 ` Tim Henigan
@ 2012-04-19 18:49                   ` Junio C Hamano
  2012-04-20  7:34                     ` David Aguilar
  0 siblings, 1 reply; 14+ messages in thread
From: Junio C Hamano @ 2012-04-19 18:49 UTC (permalink / raw)
  To: Tim Henigan; +Cc: David Aguilar, git, ramsay

Tim Henigan <tim.henigan@gmail.com> writes:

> I have a revised patch that prunes out all of the above and warns the
> user when it does so [1].

Thanks.

As long as it works when the user uses "two temporary trees" mode
without -M/-C, and it keeps working as well as before the change when
the user uses "one invocation per matched path" mode with -M/-C, I do
not care too deeply about how it is implemented in the script.

> However, it also prunes them when difftool is called in serial diff
> mode (i.e. non --dir-diff).

I do not use difftool myself, but I would imagine that it is a grave
regression, no?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 8/9 v13] difftool: teach difftool to handle directory diffs
  2012-04-19 18:49                   ` Junio C Hamano
@ 2012-04-20  7:34                     ` David Aguilar
  2012-04-20 16:58                       ` Tim Henigan
  0 siblings, 1 reply; 14+ messages in thread
From: David Aguilar @ 2012-04-20  7:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Tim Henigan, git, ramsay

On Thu, Apr 19, 2012 at 11:49 AM, Junio C Hamano <jch@google.com> wrote:
> Tim Henigan <tim.henigan@gmail.com> writes:
>
>> I have a revised patch that prunes out all of the above and warns the
>> user when it does so [1].
>
> Thanks.
>
> As long as it works when the user uses "two temporary trees" mode
> without -M/-C, and it keeps working as well as before the change when
> the user uses "one invocation per matched path" mode with -M/-C, I do
> not care too deeply about how it is implemented in the script.
>
>> However, it also prunes them when difftool is called in serial diff
>> mode (i.e. non --dir-diff).
>
> I do not use difftool myself, but I would imagine that it is a grave
> regression, no?

An alternative would be teaching difftool to gracefully handle the R
lines in the --raw output.

Simply creating the files as they existed on each side is a reasonable
start.  It seems better form to handle all possible output from --raw
anyways.  It's better than pretending it doesn't exist, I think.

Doing "smart" things with the rename information is hairy so it's
certainly worth leaving that for another day.
-- 
David

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 8/9 v13] difftool: teach difftool to handle directory diffs
  2012-04-20  7:34                     ` David Aguilar
@ 2012-04-20 16:58                       ` Tim Henigan
  0 siblings, 0 replies; 14+ messages in thread
From: Tim Henigan @ 2012-04-20 16:58 UTC (permalink / raw)
  To: David Aguilar; +Cc: Junio C Hamano, git, ramsay

On Fri, Apr 20, 2012 at 3:34 AM, David Aguilar <davvid@gmail.com> wrote:
>
> An alternative would be teaching difftool to gracefully handle the R
> lines in the --raw output.
>
> Simply creating the files as they existed on each side is a reasonable
> start.  It seems better form to handle all possible output from --raw
> anyways.  It's better than pretending it doesn't exist, I think.

I like this idea best.  I will send v14 (lucky 14!) with this change.


> Doing "smart" things with the rename information is hairy so it's
> certainly worth leaving that for another day.

Agreed.

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2012-04-20 16:58 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-13 16:36 [PATCH 8/9 v13] difftool: teach difftool to handle directory diffs Tim Henigan
2012-04-15 22:20 ` David Aguilar
2012-04-16  1:01   ` David Aguilar
2012-04-16  8:16     ` David Aguilar
2012-04-17 13:25     ` Tim Henigan
2012-04-18  3:23       ` David Aguilar
2012-04-18 13:13         ` Tim Henigan
2012-04-18 16:25           ` Junio C Hamano
2012-04-18 18:28             ` Tim Henigan
2012-04-18 19:38               ` Junio C Hamano
2012-04-19 17:11                 ` Tim Henigan
2012-04-19 18:49                   ` Junio C Hamano
2012-04-20  7:34                     ` David Aguilar
2012-04-20 16:58                       ` Tim Henigan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.