threads / discuss / 52223

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

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

## tl;dr

7 messages between Nov 8, 2019 and Nov 13, 2019.

replies: 6people: 4as markdown or json

Emily Shaffer· Nov 8, 2019, 22:50 UTC · lore

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
Junio C Hamano· Nov 9, 2019, 04:27 UTC · re: Emily Shaffer · lore

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

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.
Junio C Hamano· Nov 9, 2019, 07:14 UTC · re: Junio C Hamano · lore

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

Junio C Hamano <gitster@pobox.com> writes:
Show 7 quoted lines
> 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.

Emily Shaffer· Nov 12, 2019, 18:47 UTC · re: Junio C Hamano · lore

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

On Sat, Nov 09, 2019 at 01:27:16PM +0900, Junio C Hamano wrote:
Show 26 quoted lines
> 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
Johannes Schindelin· Nov 12, 2019, 20:16 UTC · re: Emily Shaffer · lore

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

Hi Emily,
On Tue, 12 Nov 2019, Emily Shaffer wrote:
Show 34 quoted lines
> 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

Junio C Hamano· Nov 13, 2019, 01:47 UTC · re: Emily Shaffer · lore

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

Emily Shaffer <emilyshaffer@google.com> writes:
Show 9 quoted lines
>> 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.
Jeff King· Nov 11, 2019, 03:41 UTC · re: Emily Shaffer · lore

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

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

← back to recent threads