Re: Question about behaviour of git-checkout --patch option

From: Jeff King <peff@peff.net>
Date: May 27, 2020, 07:56 UTC
Message-ID: <20200527075648.GA4006373@coredump.intra.peff.net>
In-Reply-To: <20200525221100.31d36d4d.toni@bluenox07.de>

On Mon, May 25, 2020 at 10:11:00PM +0200, Merlin Büge wrote:

Show 16 quoted lines

> This works like expected, I end up with a 1:1 copy of the original
> worktree, including empty files. However, if I include the -p option in
> the last step:
> 
> 	git checkout -p master .
> 
> ... I correctly get asked for any non-empty files/hunks if I want to
> apply them - but not for empty ones. It would just display e.g.
> 
> 	diff --git b/emptyfile a/emptyfile
> 	new file mode 100644
> 	index 0000000..e69de29
> 
> and then skip over it, asking for the next non-empty hunk.
> 
> Why does it skip over empty hunks?

I think this case was just never anticipated, and it's a bug. The original patch-selection code was written for "add -p", and the fundamental unit it works on is a hunk. We hacked around that to handle deletions back in 24ab81ae4d (add-interactive: handle deletion of empty files, 2009-10-27). But "add -p" would never see a new file, since we only consider the set of tracked files in the index.

However, when it was extended for "checkout -p", etc, we could see new files. I guess the right fix is something along these lines, extending the deletion concept to new files:

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index c4b75bcb7f..9c8844434e 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -768,7 +768,7 @@ sub parse_diff_header {
 	for (my $i = 0; $i < @{$src->{TEXT}}; $i++) {
 		my $dest =
 		   $src->{TEXT}->[$i] =~ /^(old|new) mode (\d+)$/ ? $mode :
-		   $src->{TEXT}->[$i] =~ /^deleted file/ ? $deletion :
+		   $src->{TEXT}->[$i] =~ /^(deleted|new) file/ ? $deletion :
 		   $head;
 		push @{$dest->{TEXT}}, $src->{TEXT}->[$i];
 		push @{$dest->{DISPLAY}}, $src->{DISPLAY}->[$i];

which does do the right thing. But:

  - all of this is in the process of being rewritten in C, so it should
    probably go into add-interactive.c (it's not yet the default to use
    the C version, but it probably will be soon).

  - that still says "Apply deletion..." in the UI. We'd need to update
    the messages to differentiate the two cases.

-Peff