[WIP PATCH v2] git-add--interactive: manual hunk editing mode

[Thomas Rast <trast@student.ethz.ch>]

[-- Attachment #1: Type: text/plain, Size: 7804 bytes --]

Adds a new option 'e' to the 'add -p' command loop that lets you
edit the current hunk in your favourite editor.

---

Jeff King wrote:
> Hmm. Repeating this chunk is a bit ugly. I wonder if the whole loop
> might be a bit more readable in two sections: first parse into a list of
> hunks, and then fixup each hunk.
[...]
>  - Minor fixups and style comments. All of my style comments are "I
>    would have done it as..." and not "Oh God, this is horrible" so I
>    don't think any block acceptance.

This is the squashed patch, changed according to your comments, and
with the loop in question rewritten as you suggested.  (It's indeed
far less obscure.)

Not ready for commit yet because

>  - I'd really like to see a few testcases before that, though.

is still missing.

- Thomas


 git-add--interactive.perl |  164 ++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 147 insertions(+), 17 deletions(-)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 903953e..5fb8402 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -2,6 +2,7 @@
 
 use strict;
 use Git;
+use File::Temp qw/tempfile/;
 
 my $repo = Git->repository();
 
@@ -18,6 +19,18 @@ my ($fraginfo_color) =
 	$diff_use_color ? (
 		$repo->get_color('color.diff.frag', 'cyan'),
 	) : ();
+my ($diff_plain_color) =
+	$diff_use_color ? (
+		$repo->get_color('color.diff.plain', ''),
+	) : ();
+my ($diff_old_color) =
+	$diff_use_color ? (
+		$repo->get_color('color.diff.old', 'red'),
+	) : ();
+my ($diff_new_color) =
+	$diff_use_color ? (
+		$repo->get_color('color.diff.new', 'green'),
+	) : ();
 
 my $normal_color = $repo->get_color("", "reset");
 
@@ -581,6 +594,13 @@ sub parse_hunk_header {
 	return ($o_ofs, $o_cnt, $n_ofs, $n_cnt);
 }
 
+sub format_hunk_header {
+	my ($o_ofs, $o_cnt, $n_ofs, $n_cnt) = @_;
+	return ("@@ -$o_ofs" . (($o_cnt != 1) ? ",$o_cnt" : '')
+		. " +$n_ofs" . (($n_cnt != 1) ? ",$n_cnt" : '')
+		. " @@\n");
+}
+
 sub split_hunk {
 	my ($text, $display) = @_;
 	my @split = ();
@@ -667,11 +687,7 @@ sub split_hunk {
 		my $o_cnt = $hunk->{OCNT};
 		my $n_cnt = $hunk->{NCNT};
 
-		my $head = ("@@ -$o_ofs" .
-			    (($o_cnt != 1) ? ",$o_cnt" : '') .
-			    " +$n_ofs" .
-			    (($n_cnt != 1) ? ",$n_cnt" : '') .
-			    " @@\n");
+		my $head = format_hunk_header($o_ofs, $o_cnt, $n_ofs, $n_cnt);
 		my $display_head = $head;
 		unshift @{$hunk->{TEXT}}, $head;
 		if ($diff_use_color) {
@@ -741,11 +757,7 @@ sub merge_hunk {
 		}
 		push @line, $line;
 	}
-	my $head = ("@@ -$o0_ofs" .
-		    (($o_cnt != 1) ? ",$o_cnt" : '') .
-		    " +$n0_ofs" .
-		    (($n_cnt != 1) ? ",$n_cnt" : '') .
-		    " @@\n");
+	my $head = format_hunk_header($o0_ofs, $o_cnt, $n0_ofs, $n_cnt);
 	@{$prev->{TEXT}} = ($head, @line);
 }
 
@@ -770,6 +782,122 @@ sub coalesce_overlapping_hunks {
 	return @out;
 }
 
+sub edit_hunk_manually {
+	my @oldtext = map { @{$_->{TEXT}} } @_;
+
+	my ($fh, $editpath) = tempfile($repo->repo_path() . "/git-hunk-edit.XXXXXX",
+				       SUFFIX => ".diff", UNLINK => 0);
+	print $fh "# Manual hunk edit mode -- see bottom for a quick guide\n";
+	print $fh @oldtext;
+	print $fh <<EOF;
+# ---
+# To remove '-' lines, make them ' ' lines (context).
+# To remove '+' lines, delete them.
+# Empty lines and lines starting with # will be removed.
+#
+# Lines starting with @ start a new hunk. Line counts will be adjusted
+# according to contents. If the line numbers are missing altogether,
+# they will be inferred from the previous hunk.
+#
+# You can change the hunk to your heart's content, but it will be
+# refused if the end result (the entire patch including your edited
+# hunk) does not apply cleanly.
+EOF
+	close $fh;
+
+	my $editor = $ENV{GIT_EDITOR} || $repo->config("core.editor")
+		|| $ENV{VISUAL} || $ENV{EDITOR} || "vi";
+	system('sh', '-c', $editor.' "$@"', $editor, $editpath);
+
+	open $fh, '<', $editpath
+		or die "failed to open hunk edit file for reading: " . $!;
+	my @newtext = grep { !/^#/ } <$fh>;
+	close $fh;
+	unlink(glob($editpath . "*"));
+	# Reinsert the first hunk header if the user accidentally deleted it
+	if ($newtext[0] !~ /^@/) {
+		splice @newtext, 0, 0, $oldtext[0];
+	}
+	# Split into hunks
+	my @hunktexts = ();
+	my $curhunk = [];
+	for (@newtext) {
+		if (/^@/ && @{$curhunk} > 0) {
+			push @hunktexts, $curhunk;
+			$curhunk = [];
+		}
+		push @{$curhunk}, $_;
+	}
+	push @hunktexts, $curhunk;
+	# Fix the hunk headers
+	my ($guess_o_ofs, undef, $guess_n_ofs, undef) = parse_hunk_header($oldtext[0]);
+	for my $hunk (@hunktexts) {
+		my ($o_ofs, undef, $n_ofs, undef) = parse_hunk_header($hunk->[0]);
+		$o_ofs = $guess_o_ofs unless defined $o_ofs;
+		$n_ofs = $guess_n_ofs unless defined $n_ofs;
+		my $plus_cnt = grep /^\+/, @{$hunk};
+		my $minus_cnt = grep /^-/, @{$hunk};
+		my $context_cnt = grep { /^ / || /^$/ } @{$hunk};
+		my $o_cnt = $context_cnt + $minus_cnt;
+		my $n_cnt = $context_cnt + $plus_cnt;
+		$hunk->[0] = format_hunk_header($o_ofs, $o_cnt, $n_ofs, $n_cnt);
+		$guess_o_ofs = $o_ofs + $o_cnt;
+		$guess_n_ofs = $n_ofs + $n_cnt;
+	}
+	# Recolor the hunks
+	my (@hunks) = ();
+	for my $hunk (@hunktexts) {
+		my @hunkdisplay = map {
+			colored((/^@/  ? $fraginfo_color :
+				 /^\+/ ? $diff_new_color :
+				 /^-/  ? $diff_old_color :
+				 $diff_plain_color),
+				$_);
+		} @{$hunk};
+		push @hunks, {TEXT => $hunk, DISPLAY => \@hunkdisplay};
+	}
+
+	return @hunks;
+}
+
+sub diff_applies {
+	my $fh;
+	open $fh, '| git apply --cached --check';
+	for my $h (@_) {
+		print $fh @{$h->{TEXT}};
+	}
+	return close $fh;
+}
+
+sub edit_hunk_loop {
+	my ($head, $hunks, $ix) = @_;
+
+	my @newhunks = ($hunks->[$ix]);
+
+      EDIT:
+	while (1) {
+		@newhunks = edit_hunk_manually(@newhunks);
+		if (!diff_applies($head, @$hunks[0..$ix-1], @newhunks,
+				  @$hunks[$ix+1..$#{$hunks}])) {
+			while (1) {
+				print colored $prompt_color, 'Your edited hunk does not apply. Edit again (saying "no" discards!) [y/n]? ';
+				my $line = <STDIN>;
+				if ($line =~ /^y/i) {
+					redo EDIT;
+				}
+				elsif ($line =~ /^n/i) {
+					return $hunks->[$ix];
+				}
+			}
+		}
+		if (1 < @newhunks) {
+			print colored $header_color, "Manually edited into ",
+			scalar(@newhunks), " hunks.\n";
+		}
+		return @newhunks;
+	}
+}
+
 sub help_patch_cmd {
 	print colored $help_color, <<\EOF ;
 y - stage this hunk
@@ -781,6 +909,7 @@ J - leave this hunk undecided, see next hunk
 k - leave this hunk undecided, see previous undecided hunk
 K - leave this hunk undecided, see previous hunk
 s - split the current hunk into smaller hunks
+e - manually edit the current hunk
 ? - print help
 EOF
 }
@@ -885,6 +1014,7 @@ sub patch_update_file {
 		if (hunk_splittable($hunk[$ix]{TEXT})) {
 			$other .= '/s';
 		}
+		$other .= '/e';
 		for (@{$hunk[$ix]{DISPLAY}}) {
 			print;
 		}
@@ -949,6 +1079,11 @@ sub patch_update_file {
 				$num = scalar @hunk;
 				next;
 			}
+			elsif ($line =~ /^e/) {
+				splice @hunk, $ix, 1, edit_hunk_loop($head, \@hunk, $ix);
+				$num = scalar @hunk;
+				next;
+			}
 			else {
 				help_patch_cmd($other);
 				next;
@@ -985,13 +1120,8 @@ sub patch_update_file {
 		else {
 			if ($n_lofs) {
 				$n_ofs += $n_lofs;
-				$text->[0] = ("@@ -$o_ofs" .
-					      (($o_cnt != 1)
-					       ? ",$o_cnt" : '') .
-					      " +$n_ofs" .
-					      (($n_cnt != 1)
-					       ? ",$n_cnt" : '') .
-					      " @@\n");
+				$text->[0] = format_hunk_header($o_ofs, $o_cnt,
+								$n_ofs, $n_cnt);
 			}
 			for (@$text) {
 				push @result, $_;
--
1.5.6.rc1.137.g537d1


[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 194 bytes --]