Re: [PATCH] add -p: do not attempt to coalesce mode changes

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

 



On Sat, Aug 15, 2009 at 03:56:39PM +0200, Thomas Rast wrote:

> In 0392513 (add-interactive: refactor mode hunk handling, 2009-04-16),
> we merged the interaction loops for mode changes and hunk staging.
> This was fine at the time, because 0beee4c (git-add--interactive:
> remove hunk coalescing, 2008-07-02) removed hunk coalescing.
> 
> However, in 7a26e65 (Revert "git-add--interactive: remove hunk
> coalescing", 2009-05-16), we resurrected it.  Since then, the code
> would attempt in vain to merge mode changes with diff hunks,
> corrupting both in the process.

Thanks for the writeup; I bisected the problem to 7a26e65, as well, but
hadn't yet figured out what was going on. :)

> --- a/git-add--interactive.perl
> +++ b/git-add--interactive.perl
> @@ -841,6 +841,10 @@
>  	my ($last_o_ctx, $last_was_dirty);
>  
>  	for (grep { $_->{USE} } @in) {
> +		if ($_->{TYPE} ne 'hunk') {
> +			push @out, $_;
> +			next;
> +		}
>  		my $text = $_->{TEXT};
>  		my ($o_ofs) = parse_hunk_header($text->[0]);
>  		if (defined $last_o_ctx &&

Hmm. I am not too familiar with the coalesce_overlapping_hunks code, but
it looks like we peek at $out[-1] based on $last_o_ctx, assuming that
$last_o_ctx comes from the last hunk pushed (either because we just
pushed it, or we merged into it).  So a non-hunk in the middle of some
coalescing hunks is going to violate that assumption.

As it is now, I think we always put the 'mode' hunk at the very
beginning, so that shouldn't happen (and IIRC, that order is preserved
throughout). So maybe it is not worth worrying about. But an alternate
patch is below.

---
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index a06172c..c859bb4 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -838,21 +838,26 @@ sub coalesce_overlapping_hunks {
 	my (@in) = @_;
 	my @out = ();
 
-	my ($last_o_ctx, $last_was_dirty);
+	my ($last_o_ctx, $last_was_dirty, $last_hunk);
 
 	for (grep { $_->{USE} } @in) {
+		if ($_->{TYPE} ne 'hunk') {
+			push @out, $_;
+			next;
+		}
 		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], $_);
+			merge_hunk($last_hunk, $_);
 		}
 		else {
 			push @out, $_;
+			$last_hunk = $_;
 		}
-		$last_o_ctx = find_last_o_ctx($out[-1]);
+		$last_o_ctx = find_last_o_ctx($last_hunk);
 		$last_was_dirty = $_->{DIRTY};
 	}
 	return @out;
--
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]