[PATCH] add--interactive: respect diff.algorithm

When staging hunks interactively it is sometimes useful to use an
alternative diff algorithm which splits the changes into hunks in a more
logical manner.  This is not possible because the plumbing commands
called by add--interactive ignore the "diff.algorithm" configuration
option (as they should).

Since add--interactive is a porcelain command it should respect this
configuration variable.  To do this, make it read diff.algorithm and
pass its value to the underlying diff-index and diff-files invocations.

At this point, do not add options to "git add", "git reset" or "git
checkout" (all of which can call git-add--interactive).  If a user want
to override the value on the command line they can use:

	git -c diff.algorithm=$ALGO ...

Signed-off-by: John Keeping <redacted>
---
On Mon, Jun 10, 2013 at 05:56:56PM -0400, Jeff King wrote:

On Mon, Jun 10, 2013 at 10:46:38PM +0100, John Keeping wrote:

quoted

quoted

Overall, I think respecting diff.algorithm in add--interactive is a very
sane thing to do. I would even be tempted to say we should allow a few
other select diff options (e.g., fewer or more context lines). If you
allowed diff options like this:

  git add --patch="--patience -U5"

that is very flexible, but I would not want to think about what the code
does when you pass --patch="--raw" or equal nonsense.

An alternative would be to permit them to be set from within the
interactive UI.  I'd find it quite useful to experiment with various
diff options when I encounter a hunk that isn't as easy to pick as I'd
like.  I expect it would be very hard to do that on a per-hunk basis,
although per-file doesn't seem like it would be too hard.

That's an interesting idea, for a subset of options (e.g., "increase
context for this hunk"). I suspect implementing it would be painful,
though, as you would have to re-run diff, and you have no guarantee of
getting the same set of hunks (e.g., the hunk might end up coalesced
with another).

I think you'd need to re-run the diff over the whole file and then skip
hunks until you reach one that overlaps with the original hunk.  But I
suspect it would end up being quite a lot more complicated than that.

quoted

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index d2c4ce6..0b0fac2 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl

@@ -44,6 +44,8 @@ my ($diff_new_color) =
 
 my $normal_color = $repo->get_color("", "reset");
 
+my $diff_algorithm = ($repo->config('diff.algorithm') or 'default');
+
 my $use_readkey = 0;
 my $use_termcap = 0;
 my %term_escapes;

@@ -731,6 +733,9 @@ sub run_git_apply {
 sub parse_diff {
 	my ($path) = @_;
 	my @diff_cmd = split(" ", $patch_mode_flavour{DIFF});
+	if ($diff_algorithm ne "default") {
+		push @diff_cmd, "--diff-algorithm=${diff_algorithm}";
+	}
 	if (defined $patch_mode_revision) {
 		push @diff_cmd, $patch_mode_revision;

Yeah, that looks like the sane way to do it to me. As a perl style
thing, I think the usual way of spelling 'default' is 'undef'. I.e.:

  my $diff_algorithm = $repo->config('diff.algorithm');
  ...
  if (defined $diff_algorithm) {
          push @diff_cmd, "--diff-algorithm=$diff_algorithm";
  }

OK.  The "default" is actually "the value that is equivalent to 'myers'
for diff.algorithm" and I was originally going to add --diff-algorithm
to the command line unconditionally.

But I think it's better to add it only when it has been specified so
I've changed it as you suggest.


 git-add--interactive.perl | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index d2c4ce6..5310959 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl

@@ -44,6 +44,8 @@ my ($diff_new_color) =
 
 my $normal_color = $repo->get_color("", "reset");
 
+my $diff_algorithm = $repo->config('diff.algorithm');
+
 my $use_readkey = 0;
 my $use_termcap = 0;
 my %term_escapes;

@@ -731,6 +733,9 @@ sub run_git_apply {
 sub parse_diff {
 	my ($path) = @_;
 	my @diff_cmd = split(" ", $patch_mode_flavour{DIFF});
+	if (defined $diff_algorithm) {
+		push @diff_cmd, "--diff-algorithm=${diff_algorithm}";
+	}
 	if (defined $patch_mode_revision) {
 		push @diff_cmd, $patch_mode_revision;
 	}

-- 
1.8.3.779.g691e267