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