Hi Peff & Merlin,
On Wed, 27 May 2020, Jeff King wrote:
> 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
