git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
May 27, 2020, 01:49 UTC
Message-ID
<nycvar.QRO.7.76.6.2005270348240.56@tvgsbejvaqbjf.bet>
In-Reply-To
<20200527075648.GA4006373@coredump.intra.peff.net>
Hi Peff & Merlin,
On Wed, 27 May 2020, Jeff King wrote:
Show 52 quoted lines
> On Mon, May 25, 2020 at 10:11:00PM +0200, Merlin Büge wrote:
>
> > 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.

The C version (with "Apply addition"... in the UI) would look somewhat like this:

-- snipsnap --
diff --git a/add-patch.c b/add-patch.c
index d8bfe379be4d..c0d8bf2df0c5 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -10,7 +10,7 @@
 #include "prompt.h"

 enum prompt_mode_type {
-	PROMPT_MODE_CHANGE = 0, PROMPT_DELETION, PROMPT_HUNK,
+	PROMPT_MODE_CHANGE = 0, PROMPT_DELETION, PROMPT_ADDITION, PROMPT_HUNK,
 	PROMPT_MODE_MAX, /* must be last */
 };

@@ -33,6 +33,7 @@ static struct patch_mode patch_mode_add = {
 	.prompt_mode = {
 		N_("Stage mode change [y,n,q,a,d%s,?]? "),
 		N_("Stage deletion [y,n,q,a,d%s,?]? "),
+		N_("Stage addition [y,n,q,a,d%s,?]? "),
 		N_("Stage this hunk [y,n,q,a,d%s,?]? ")
 	},
 	.edit_hunk_hint = N_("If the patch applies cleanly, the edited hunk "
@@ -54,6 +55,7 @@ static struct patch_mode patch_mode_stash = {
 	.prompt_mode = {
 		N_("Stash mode change [y,n,q,a,d%s,?]? "),
 		N_("Stash deletion [y,n,q,a,d%s,?]? "),
+		N_("Stash addition [y,n,q,a,d%s,?]? "),
 		N_("Stash this hunk [y,n,q,a,d%s,?]? "),
 	},
 	.edit_hunk_hint = N_("If the patch applies cleanly, the edited hunk "
@@ -77,6 +79,7 @@ static struct patch_mode patch_mode_reset_head = {
 	.prompt_mode = {
 		N_("Unstage mode change [y,n,q,a,d%s,?]? "),
 		N_("Unstage deletion [y,n,q,a,d%s,?]? "),
+		N_("Unstage addition [y,n,q,a,d%s,?]? "),
 		N_("Unstage this hunk [y,n,q,a,d%s,?]? "),
 	},
 	.edit_hunk_hint = N_("If the patch applies cleanly, the edited hunk "
@@ -99,6 +102,7 @@ static struct patch_mode patch_mode_reset_nothead = {
 	.prompt_mode = {
 		N_("Apply mode change to index [y,n,q,a,d%s,?]? "),
 		N_("Apply deletion to index [y,n,q,a,d%s,?]? "),
+		N_("Apply addition to index [y,n,q,a,d%s,?]? "),
 		N_("Apply this hunk to index [y,n,q,a,d%s,?]? "),
 	},
 	.edit_hunk_hint = N_("If the patch applies cleanly, the edited hunk "
@@ -121,6 +125,7 @@ static struct patch_mode patch_mode_checkout_index = {
 	.prompt_mode = {
 		N_("Discard mode change from worktree [y,n,q,a,d%s,?]? "),
 		N_("Discard deletion from worktree [y,n,q,a,d%s,?]? "),
+		N_("Discard addition from worktree [y,n,q,a,d%s,?]? "),
 		N_("Discard this hunk from worktree [y,n,q,a,d%s,?]? "),
 	},
 	.edit_hunk_hint = N_("If the patch applies cleanly, the edited hunk "
@@ -143,6 +148,7 @@ static struct patch_mode patch_mode_checkout_head = {
 	.prompt_mode = {
 		N_("Discard mode change from index and worktree [y,n,q,a,d%s,?]? "),
 		N_("Discard deletion from index and worktree [y,n,q,a,d%s,?]? "),
+		N_("Discard addition from index and worktree [y,n,q,a,d%s,?]? "),
 		N_("Discard this hunk from index and worktree [y,n,q,a,d%s,?]? "),
 	},
 	.edit_hunk_hint = N_("If the patch applies cleanly, the edited hunk "
@@ -164,6 +170,7 @@ static struct patch_mode patch_mode_checkout_nothead = {
 	.prompt_mode = {
 		N_("Apply mode change to index and worktree [y,n,q,a,d%s,?]? "),
 		N_("Apply deletion to index and worktree [y,n,q,a,d%s,?]? "),
+		N_("Apply addition to index and worktree [y,n,q,a,d%s,?]? "),
 		N_("Apply this hunk to index and worktree [y,n,q,a,d%s,?]? "),
 	},
 	.edit_hunk_hint = N_("If the patch applies cleanly, the edited hunk "
@@ -186,6 +193,7 @@ static struct patch_mode patch_mode_worktree_head = {
 	.prompt_mode = {
 		N_("Discard mode change from index and worktree [y,n,q,a,d%s,?]? "),
 		N_("Discard deletion from index and worktree [y,n,q,a,d%s,?]? "),
+		N_("Discard addition from index and worktree [y,n,q,a,d%s,?]? "),
 		N_("Discard this hunk from index and worktree [y,n,q,a,d%s,?]? "),
 	},
 	.edit_hunk_hint = N_("If the patch applies cleanly, the edited hunk "
@@ -207,6 +215,7 @@ static struct patch_mode patch_mode_worktree_nothead = {
 	.prompt_mode = {
 		N_("Apply mode change to index and worktree [y,n,q,a,d%s,?]? "),
 		N_("Apply deletion to index and worktree [y,n,q,a,d%s,?]? "),
+		N_("Apply addition to index and worktree [y,n,q,a,d%s,?]? "),
 		N_("Apply this hunk to index and worktree [y,n,q,a,d%s,?]? "),
 	},
 	.edit_hunk_hint = N_("If the patch applies cleanly, the edited hunk "
@@ -248,7 +257,7 @@ struct add_p_state {
 		struct hunk head;
 		struct hunk *hunk;
 		size_t hunk_nr, hunk_alloc;
-		unsigned deleted:1, mode_change:1,binary:1;
+		unsigned deleted:1, added:1, mode_change:1,binary:1;
 	} *file_diff;
 	size_t file_diff_nr;

@@ -442,7 +451,7 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)
 	pend = p + plain->len;
 	while (p != pend) {
 		char *eol = memchr(p, '\n', pend - p);
-		const char *deleted = NULL, *mode_change = NULL;
+		const char *deleted = NULL, *added = NULL, *mode_change = NULL;

 		if (!eol)
 			eol = pend;
@@ -465,7 +474,8 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)
 			; /* keep the rest of the file in a single "hunk" */
 		else if (starts_with(p, "@@ ") ||
 			 (hunk == &file_diff->head &&
-			  skip_prefix(p, "deleted file", &deleted))) {
+			  (skip_prefix(p, "deleted file", &deleted) ||
+			   skip_prefix(p, "new file", &added)))) {
 			if (marker == '-' || marker == '+')
 				/*
 				 * Should not happen; previous hunk did not end
@@ -485,6 +495,8 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)

 			if (deleted)
 				file_diff->deleted = 1;
+			else if (added)
+				file_diff->added = 1;
 			else if (parse_hunk_header(s, hunk) < 0)
 				return -1;

@@ -537,8 +549,10 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)
 			   starts_with(p, "Binary files "))
 			file_diff->binary = 1;

-		if (file_diff->deleted && file_diff->mode_change)
-			BUG("diff contains delete *and* a mode change?!?\n%.*s",
+		if (!!file_diff->deleted + !!file_diff->added +
+		    !!file_diff->mode_change > 1)
+			BUG("diff can only contain delete *or* add *or* a "
+			    "mode change?!?\n%.*s",
 			    (int)(eol - (plain->buf + file_diff->head.start)),
 			    plain->buf + file_diff->head.start);

@@ -1392,11 +1406,13 @@ static int patch_update_file(struct add_p_state *s,
 		if (hunk->splittable_into > 1)
 			strbuf_addstr(&s->buf, ",s");
 		if (hunk_index + 1 > file_diff->mode_change &&
-		    !file_diff->deleted)
+		    !file_diff->deleted && !file_diff->added)
 			strbuf_addstr(&s->buf, ",e");

 		if (file_diff->deleted)
 			prompt_mode_type = PROMPT_DELETION;
+		else if (file_diff->added)
+			prompt_mode_type = PROMPT_ADDITION;
 		else if (file_diff->mode_change && !hunk_index)
 			prompt_mode_type = PROMPT_MODE_CHANGE;
 		else
Previous: Merlin Büge
Message 7 of 7 in “Question about behaviour of git-checkout --patch option”
  1. Merlin BügeMay 25, 2020
  2. Jeff KingMay 27, 2020
  3. Jeff KingMay 27, 2020
  4. Merlin BügeMay 27, 2020
  5. Johannes SchindelinMay 27, 2020
  6. Merlin BügeMay 27, 2020
  7. Johannes SchindelinMay 27, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.