Change behavior of git add --patch on newly added file?

Change behavior of git add --patch on newly added file?

[Emily Shaffer]

Should 'git add -p <newly-added-file>' do the same thing as 'git add -N
<newly-added-file && git add -p <newly-added-file>'?

To demonstrate:
  git init .
  printf "%s\n" {1..10} >newfile
  git add -p newfile

This outputs "No changes." 'git status' shows that 'newfile' is
untracked.

However, since I want each of my commits to be atomic, I want only lines
1-5 in my first commit. (Or more realistically, maybe I only want
function stubs.)

I *can* do:
  git add -N newfile
  git add -p newfile

But, why doesn't 'git add -p' just do that on its own? At the very
least, "No changes" is a pretty cryptic output - there ARE changes,
right here in my workspace! (I think it means "there isn't a 'newfile' in
the index, so we can't say whether there is a difference between
'newfile' in the index and 'newfile' in your working tree".)

And if I reason to myself, "I can only add --patch a file which is
tracked, so I need to track this file first" and go skimming through the
documentation for "git add", -N doesn't jump out very much:

  -N, --intent-to-add   record only the fact that the path will be added later

or,

  -N, --intent-to-add
      Record only the fact that the path will be added later. An entry for the path is placed in
      the index with no content. This is useful for, among other things, showing the unstaged
      content of such files with git diff and committing them with git commit -a.

Considering that other parts of the add documentation talk about
tracked or untracked files, I personally miss the upshot that we are
tracking a file which was previously untracked with '-N'. (My gut
guess is that while many Git users are familiar with "tracked" or
"untracked" but less Git users are familiar with what it means to "place
in the index".)

Interestingly, in the whole-file deletion case, I *do* get interactive
support....kind of.

  rm trackedfile
  git add -p trackedfile

  diff --git a/trackedfile b/trackedfile
  index f00c965..0000000
  --- a/trackedfile
  +++ /dev/null
  deleted file mode 100644
  @@ -1,10 +0,0 @@
  -1
  -2
  -3
  -4
  -5
  -6
  -7
  -8
  -9
  -10
  (1/1) Stage deletion [y,n,q,a,d,?]? e
  Sorry, cannot edit this hunk


Is there a reason that git add -p can't do whole-file support this way?
While I'm less sure about what I'd like to see for copied files, I do
feel like there's a strong argument for patch adding new or deleted
files.

 - Emily

Re: Change behavior of git add --patch on newly added file?

[Junio C Hamano]

Emily Shaffer <emilyshaffer@google.com> writes:

> Should 'git add -p <newly-added-file>' do the same thing as 'git add -N
> <newly-added-file && git add -p <newly-added-file>'?

Probably.  

I originally wrote "git add -i" with the intention that the
interactive mode is _the_ primary interface to the machinery, so the
expected way to work with a new file was "git add -i", tell the
command to add that <newly-added-file>, and do the "patch" thing
using the interactive subcommand to do so within the "git add -i"
session.

Later people liked (only) the patch part, and "git add -p" (and
various "--patch" options that invoke "add -p" internally from other
commands like "checkout", "reset" were added) was born.  I think
nobody thought things through when they did so.

If I were designing "git add -p" from scratch and explicitly asked
not to do the other parts of the "--interactive" feature, I would
imagine "add -N && add -p" combination is what I would make it
mimic.

Patches welcome, but you may want to check with Dscho as there is an
effort going on to reimplement the entire "add -i" machinery in C.

Thanks.

Re: Change behavior of git add --patch on newly added file?

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Emily Shaffer <emilyshaffer@google.com> writes:
>
>> Should 'git add -p <newly-added-file>' do the same thing as 'git add -N
>> <newly-added-file && git add -p <newly-added-file>'?
> ...
> Patches welcome, but you may want to check with Dscho as there is an
> effort going on to reimplement the entire "add -i" machinery in C.

Oh, having said all that, for a newly added file, all you have is a
hunk that is full of added lines and nothing else, so even 's'plit
interactive subcommand of "add -p" interface would not do anything.

So I am not sure if performing an implicit "add -N" upfront would
help your use case that much.  For that, you'd need to extend hunk
splitting UI a bit more, so that the user can split a hunk that only
adds lines into two at a desired point in the hunk.

Re: Change behavior of git add --patch on newly added file?

[Emily Shaffer]

On Sat, Nov 09, 2019 at 01:27:16PM +0900, Junio C Hamano wrote:
> Emily Shaffer <emilyshaffer@google.com> writes:
> 
> > Should 'git add -p <newly-added-file>' do the same thing as 'git add -N
> > <newly-added-file && git add -p <newly-added-file>'?
> 
> Probably.  
> 
> I originally wrote "git add -i" with the intention that the
> interactive mode is _the_ primary interface to the machinery, so the
> expected way to work with a new file was "git add -i", tell the
> command to add that <newly-added-file>, and do the "patch" thing
> using the interactive subcommand to do so within the "git add -i"
> session.
> 
> Later people liked (only) the patch part, and "git add -p" (and
> various "--patch" options that invoke "add -p" internally from other
> commands like "checkout", "reset" were added) was born.  I think
> nobody thought things through when they did so.
> 
> If I were designing "git add -p" from scratch and explicitly asked
> not to do the other parts of the "--interactive" feature, I would
> imagine "add -N && add -p" combination is what I would make it
> mimic.
> 
> Patches welcome, but you may want to check with Dscho as there is an
> effort going on to reimplement the entire "add -i" machinery in C.

Ah, this is a compelling point. I imagine the landscape will be fairly
different when that effort is finished.


From the replies, it sounds like it's a favorable change, but it makes
sense to wait on it considering the refactor to use C. Thanks, all.

 - Emily

Re: Change behavior of git add --patch on newly added file?

[Johannes Schindelin]

Hi Emily,

On Tue, 12 Nov 2019, Emily Shaffer wrote:

> On Sat, Nov 09, 2019 at 01:27:16PM +0900, Junio C Hamano wrote:
> > Emily Shaffer <emilyshaffer@google.com> writes:
> >
> > > Should 'git add -p <newly-added-file>' do the same thing as 'git add -N
> > > <newly-added-file && git add -p <newly-added-file>'?
> >
> > Probably.
> >
> > I originally wrote "git add -i" with the intention that the
> > interactive mode is _the_ primary interface to the machinery, so the
> > expected way to work with a new file was "git add -i", tell the
> > command to add that <newly-added-file>, and do the "patch" thing
> > using the interactive subcommand to do so within the "git add -i"
> > session.
> >
> > Later people liked (only) the patch part, and "git add -p" (and
> > various "--patch" options that invoke "add -p" internally from other
> > commands like "checkout", "reset" were added) was born.  I think
> > nobody thought things through when they did so.
> >
> > If I were designing "git add -p" from scratch and explicitly asked
> > not to do the other parts of the "--interactive" feature, I would
> > imagine "add -N && add -p" combination is what I would make it
> > mimic.
> >
> > Patches welcome, but you may want to check with Dscho as there is an
> > effort going on to reimplement the entire "add -i" machinery in C.
>
> Ah, this is a compelling point. I imagine the landscape will be fairly
> different when that effort is finished.
>
>
> From the replies, it sounds like it's a favorable change, but it makes
> sense to wait on it considering the refactor to use C. Thanks, all.

The patch series can already be viewed at PRs #170-175 on
https://github.com/gitgitgadget/git; maybe you want to have a look at
implementing this feature on top of
https://github.com/dscho/git/tree/add-p-in-c-config-settings?

Ciao,
Dscho

Re: Change behavior of git add --patch on newly added file?

[Junio C Hamano]

Emily Shaffer <emilyshaffer@google.com> writes:

>> Patches welcome, but you may want to check with Dscho as there is an
>> effort going on to reimplement the entire "add -i" machinery in C.
>
> Ah, this is a compelling point. I imagine the landscape will be fairly
> different when that effort is finished.
>
>
> From the replies, it sounds like it's a favorable change, but it makes
> sense to wait on it considering the refactor to use C. Thanks, all.

Peff's "how would you tell between genuinely untracked ones and
those you would want to add" and "should 'add -p' with no pathspec
attempt to 'add -N' everything under the sun?" are both quite valid
points.  I already raised "'add -N && add -p' would give you a
single hunk that adds everything. Are you willing to manually split,
and how well would it work?" in a separate message.  These design
issues can be resolved without coding, and should be resolved before
you start randomly typing ;-)

Thanks.

Re: Change behavior of git add --patch on newly added file?

[Jeff King]

On Fri, Nov 08, 2019 at 02:50:35PM -0800, Emily Shaffer wrote:

> Is there a reason that git add -p can't do whole-file support this way?
> While I'm less sure about what I'd like to see for copied files, I do
> feel like there's a strong argument for patch adding new or deleted
> files.

I have been mildly annoyed by trying to "add -p" an untracked file
before, too. But there is some complexity with broader pathspecs, I
think. For instance, imagine I have a subdirectory in my repo with three
files:

  - subdir/tracked -- this one is a real tracked file with modifications

  - subdir/new -- this is a new file I've just added

  - subdir/cruft -- this is some junk I wrote while debugging

Then what should:

  git add -p subdir

do? Obviously it should ask about "tracked". And we'd want it to do the
"add -N" thing on "new", but not on "cruft".

I don't mind being _asked_ about the "cruft" file if I can just say "n"
to ignore it. But I wouldn't want it left in the index as intent-to-add.
So just pretending that it was the same as:

  git add -N "$@" && git add -p "$@"

seems poor. But it would be fine if instead we tentatively consider
files matching the pathspec as tracked (assuming they're not
.gitignored), and then the end result for each path is either that it
has content staged by "-p", or it's left unchanged (including untracked
if it was not previously tracked).

I'm not sure when you intended for this to kick in. The case of "git add
-p an-actual-file", where the pathspec matches exactly one file, is an
obvious one. But because the shell expands wildcards, too, I don't know
if it's a good heuristic for "the user really meant to mention this
file". I.e., if I do:

  git add -p *.c

Git will see individual filenames on the command-line, but the user may
not have wanted to include all of them.

There's also a question of what "git add -p" without any pathspecs
should do. I'd think you would probably want no change from the current
behavior (i.e., not even asking about untracked files).

-Peff