Skip to content

Commit

Permalink
add-interactive: fix bogus diff header line ordering
Browse files Browse the repository at this point in the history
When we look at a patch for adding hunks interactively, we
first split it into a header and a list of hunks. Some of
the header lines, such as mode changes and deletion, however,
become their own selectable hunks. Later when we reassemble
the patch, we simply concatenate the header and the selected
hunks. This leads to patches like this:

  diff --git a/file b/file
  index d95f3ad..0000000
  --- a/file
  +++ /dev/null
  deleted file mode 100644
  @@ -1 +0,0 @@
  -content

Notice how the deletion comes _after_ the ---/+++ lines,
when it should come before.

In many cases, we can get away with this as git-apply
accepts the slightly bogus input. However, in the specific
case of a deletion line that is being applied via "apply
-R", this malformed patch triggers an assert in git-apply.
This comes up when discarding a deletion via "git checkout
-p".

Rather than try to make git-apply accept our odd input,
let's just reassemble the patch in the correct order.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
Jeff King authored and Junio C Hamano committed Feb 23, 2010
1 parent 003c6ab commit e1327ed
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 1 deletion.
24 changes: 23 additions & 1 deletion git-add--interactive.perl
Original file line number Diff line number Diff line change
Expand Up @@ -957,6 +957,28 @@ sub coalesce_overlapping_hunks {
return @out;
}

sub reassemble_patch {
my $head = shift;
my @patch;

# Include everything in the header except the beginning of the diff.
push @patch, (grep { !/^[-+]{3}/ } @$head);

# Then include any headers from the hunk lines, which must
# come before any actual hunk.
while (@_ && $_[0] !~ /^@/) {
push @patch, shift;
}

# Then begin the diff.
push @patch, grep { /^[-+]{3}/ } @$head;

# And then the actual hunks.
push @patch, @_;

return @patch;
}

sub color_diff {
return map {
colored((/^@/ ? $fraginfo_color :
Expand Down Expand Up @@ -1454,7 +1476,7 @@ sub patch_update_file {

if (@result) {
my $fh;
my @patch = (@{$head->{TEXT}}, @result);
my @patch = reassemble_patch($head->{TEXT}, @result);
my $apply_routine = $patch_mode_flavour{APPLY};
&$apply_routine(@patch);
refresh();
Expand Down
8 changes: 8 additions & 0 deletions t/t2016-checkout-patch.sh
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,14 @@ test_expect_success 'git checkout -p HEAD^' '
verify_state dir/foo parent parent
'

test_expect_success 'git checkout -p handles deletion' '
set_state dir/foo work index &&
rm dir/foo &&
(echo n; echo y) | git checkout -p &&
verify_saved_state bar &&
verify_state dir/foo index index
'

# The idea in the rest is that bar sorts first, so we always say 'y'
# first and if the path limiter fails it'll apply to bar instead of
# dir/foo. There's always an extra 'n' to reject edits to dir/foo in
Expand Down

0 comments on commit e1327ed

Please sign in to comment.