On Wed, May 27, 2020 at 09:09:06PM +0000, Johannes Schindelin via GitGitGadget wrote: > However, since the same machinery was used for `git checkout -p` & > friends, we can see new files. > > Handle this case specifically, adding a new prompt for it that is > modeled after the `deleted file` case. Thanks! I was planning to dig further into this topic today, and here it is all wrapped up with a bow. :) > add-patch.c | 30 +++++++++++++++++++++++------- > git-add--interactive.perl | 21 +++++++++++++++++++-- Ooh, you even fixed the perl version, too. I was just going to leave it in the dust and add a test that set GIT_TEST_ADD_I_USE_BUILTIN. Both versions look good, and are similar to what I expected from looking at it last night. > The original patch selection code was written for `git add -p`, and the > fundamental unit on which it works is a hunk. > > We hacked around that to handle deletions back in 24ab81ae4d > (add-interactive: handle deletion of empty files, 2009-10-27). But `git > add -p` would never see a new file, since we only consider the set of > tracked files in the index. I lied a little with "would never see a new file". There _is_ a related case with "add -p" that might be worth thinking about: intent-to-add files. $ git init $ >empty $ echo content >not-empty $ git add -N . $ git add -p diff --git a/not-empty b/not-empty index e69de29..d95f3ad 100644 --- a/not-empty +++ b/not-empty @@ -0,0 +1 @@ +content (1/1) Stage this hunk [y,n,q,a,d,e,?]? n [no mention of empty file!] I think the culprit here is diff-files, though, which doesn't show a patch for intent-to-add: $ git diff-files :100644 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0000000000000000000000000000000000000000 M empty :100644 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0000000000000000000000000000000000000000 M not-empty $ git diff-files -p diff --git a/not-empty b/not-empty index e69de29..d95f3ad 100644 --- a/not-empty +++ b/not-empty @@ -0,0 +1 @@ +content I don't think this really intersects with the patch here at all, because diff-files is not producing "new file" lines for these entries (even for the non-empty one). The solution _might_ be to convince diff-files to treat i-t-a entries as creations. And then with your patch here, we'd naturally do the right thing. So I don't think this needs to hold up your patch in any way, nor do we necessarily need to deal with i-t-a now. I was mostly curious how they worked, since we don't support added files. The answer is just that they don't always. ;) -Peff
