Re: [PATCH] new test fails "add -p" for adds on the top line

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Thomas Rast <trast@xxxxxxxxxxxxxxx> writes:

> Nanako Shiraishi wrote:
>> Quoting Matt Graham <mdg149@xxxxxxxxx>:
>> 
>> > add -p doesn't work for some diffs.  diffs adding a new line at the top of
>> > the file with other adds later in the file are one way to trigger the problem.
>> >
>> > during add -p, split the diff and then answer y for all segments.  the file
>> > won't have been added to the index.
>> >
>> > Signed-off-by: Matthew Graham <mdg149@xxxxxxxxx>
>> 
>> I tried "git-add -p" from different versions and I found out that versions before the commit 0beee4c6dec15292415e3d56075c16a76a22af54 doesn't have this problem.
>> 
>> commit 0beee4c6dec15292415e3d56075c16a76a22af54
>> Author: Thomas Rast <trast@xxxxxxxxxxxxxxx>
>> Date:   Wed Jul 2 23:59:44 2008 +0200
>> 
>>     git-add--interactive: remove hunk coalescing
>>     
>>     Current git-apply has no trouble at all applying chunks that have
>>     overlapping context, as produced by the splitting feature. So we can
>>     drop the manual coalescing.
>>     
>>     Signed-off-by: Thomas Rast <trast@xxxxxxxxxxxxxxx>
>>     Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
>
> The above commit still reverts cleanly, but AFAICS merge_hunk blindly
> trusts the hunk headers, an assumption that is no longer valid due to
> the 'edit' feature.

Heh, here is my "I told you so" moment ;-).

We could also remove that "edit" thing; I do not use nor trust it
(fundamentally you cannot trust it).

But how about doing it this way?

-- >8 --
Subject: Revert "git-add--interactive: remove hunk coalescing"

This reverts commit 0beee4c6dec15292415e3d56075c16a76a22af54 but with a
bit of twist, as we have added "edit hunk manually" hack and we cannot
rely on the original line numbers of the hunks that were manually edited.
---
 git-add--interactive.perl  |   96 +++++++++++++++++++++++++++++++++++++++++++-
 t/t3701-add-interactive.sh |    2 +-
 2 files changed, 96 insertions(+), 2 deletions(-)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index f6e536e..a06172c 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -767,6 +767,96 @@ sub split_hunk {
 	return @split;
 }
 
+sub find_last_o_ctx {
+	my ($it) = @_;
+	my $text = $it->{TEXT};
+	my ($o_ofs, $o_cnt) = parse_hunk_header($text->[0]);
+	my $i = @{$text};
+	my $last_o_ctx = $o_ofs + $o_cnt;
+	while (0 < --$i) {
+		my $line = $text->[$i];
+		if ($line =~ /^ /) {
+			$last_o_ctx--;
+			next;
+		}
+		last;
+	}
+	return $last_o_ctx;
+}
+
+sub merge_hunk {
+	my ($prev, $this) = @_;
+	my ($o0_ofs, $o0_cnt, $n0_ofs, $n0_cnt) =
+	    parse_hunk_header($prev->{TEXT}[0]);
+	my ($o1_ofs, $o1_cnt, $n1_ofs, $n1_cnt) =
+	    parse_hunk_header($this->{TEXT}[0]);
+
+	my (@line, $i, $ofs, $o_cnt, $n_cnt);
+	$ofs = $o0_ofs;
+	$o_cnt = $n_cnt = 0;
+	for ($i = 1; $i < @{$prev->{TEXT}}; $i++) {
+		my $line = $prev->{TEXT}[$i];
+		if ($line =~ /^\+/) {
+			$n_cnt++;
+			push @line, $line;
+			next;
+		}
+
+		last if ($o1_ofs <= $ofs);
+
+		$o_cnt++;
+		$ofs++;
+		if ($line =~ /^ /) {
+			$n_cnt++;
+		}
+		push @line, $line;
+	}
+
+	for ($i = 1; $i < @{$this->{TEXT}}; $i++) {
+		my $line = $this->{TEXT}[$i];
+		if ($line =~ /^\+/) {
+			$n_cnt++;
+			push @line, $line;
+			next;
+		}
+		$ofs++;
+		$o_cnt++;
+		if ($line =~ /^ /) {
+			$n_cnt++;
+		}
+		push @line, $line;
+	}
+	my $head = ("@@ -$o0_ofs" .
+		    (($o_cnt != 1) ? ",$o_cnt" : '') .
+		    " +$n0_ofs" .
+		    (($n_cnt != 1) ? ",$n_cnt" : '') .
+		    " @@\n");
+	@{$prev->{TEXT}} = ($head, @line);
+}
+
+sub coalesce_overlapping_hunks {
+	my (@in) = @_;
+	my @out = ();
+
+	my ($last_o_ctx, $last_was_dirty);
+
+	for (grep { $_->{USE} } @in) {
+		my $text = $_->{TEXT};
+		my ($o_ofs) = parse_hunk_header($text->[0]);
+		if (defined $last_o_ctx &&
+		    $o_ofs <= $last_o_ctx &&
+		    !$_->{DIRTY} &&
+		    !$last_was_dirty) {
+			merge_hunk($out[-1], $_);
+		}
+		else {
+			push @out, $_;
+		}
+		$last_o_ctx = find_last_o_ctx($out[-1]);
+		$last_was_dirty = $_->{DIRTY};
+	}
+	return @out;
+}
 
 sub color_diff {
 	return map {
@@ -878,7 +968,8 @@ sub edit_hunk_loop {
 		my $newhunk = {
 			TEXT => $text,
 			TYPE => $hunk->[$ix]->{TYPE},
-			USE => 1
+			USE => 1,
+			DIRTY => 1,
 		};
 		if (diff_applies($head,
 				 @{$hunk}[0..$ix-1],
@@ -1210,6 +1301,8 @@ sub patch_update_file {
 		}
 	}
 
+	@hunk = coalesce_overlapping_hunks(@hunk);
+
 	my $n_lofs = 0;
 	my @result = ();
 	for (@hunk) {
@@ -1224,6 +1317,7 @@ sub patch_update_file {
 		open $fh, '| git apply --cached --recount';
 		for (@{$head->{TEXT}}, @result) {
 			print $fh $_;
+			print STDERR $_;
 		}
 		if (!close $fh) {
 			for (@{$head->{TEXT}}, @result) {
diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index 45da6c8..c5220a1 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -187,7 +187,7 @@ index b6f2c08..61b9053 100755
 +lastline
 EOF
 # Test splitting the first patch, then adding both
-test_expect_failure 'add first line works' '
+test_expect_success 'add first line works' '
 	git commit -am "clear local changes" &&
 	git apply patch &&
 	(echo s; echo y; echo y) | git add -p file &&
-- 
1.6.3.1.9.g95405b

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]