git add -p with new file

git add -p with new file

[Ariel]

If you do git add -p new_file it says:

No changes.

Which is a rather confusing message. I would expect it to show me the 
content of the file in patch form, in the normal way that -p works, let me 
edit it, etc.

(Note: I am aware I can do -N first, but when I specifically enter the 
name of a new file I feel it should figure out what I mean.)

 	-Ariel

Re: git add -p with new file

[Duy Nguyen]

On Wed, Dec 7, 2016 at 8:18 AM, Ariel <asgit@dsgml.com> wrote:
>
> If you do git add -p new_file it says:
>
> No changes.
>
> Which is a rather confusing message. I would expect it to show me the
> content of the file in patch form, in the normal way that -p works, let me
> edit it, etc.

We could improve it a bit, suggesting the user to do git add -N. But
is there a point of using -p on a new file? It will be one big chunk,
you can't split anything. Perhaps maybe you want to use 'e' to edit
what's added?

> (Note: I am aware I can do -N first, but when I specifically enter the name
> of a new file I feel it should figure out what I mean.)
-- 
Duy

Re: git add -p with new file

[Jeff King]

On Tue, Dec 06, 2016 at 08:18:59PM -0500, Ariel wrote:

> If you do git add -p new_file it says:
> 
> No changes.
> 
> Which is a rather confusing message. I would expect it to show me the
> content of the file in patch form, in the normal way that -p works, let me
> edit it, etc.
> 
> (Note: I am aware I can do -N first, but when I specifically enter the name
> of a new file I feel it should figure out what I mean.)

Keep in mind that the argument to "git add -p" is not a filename, but a
"pathspec" that can match many files. What should:

  git init
  mkdir subdir
  for i in one two; do echo $i >subdir/$i; done
  git add -p subdir

do?  Or for that matter, "git add -p ."? It's contrary to the rest of
git-add for specifying pathspecs to actually make things _more_
inclusive rather than less.

So I don't think triggering the change of behavior based on the presence
of a pathspec makes sense.

Historically "add -p" has been more like "add -u" in updating tracked
files. We have "-A" for "update everything _and_ new files". It doesn't
seem unreasonable to me to have a variant of "-p" that is similar.

I don't think that variant should just be "add -N everything, and then
run add -p". As Duy points out, the patch for a new file is just one big
block. But the decision of "do I want to start tracking this untracked
file" is potentially an interesting one.

-Peff

Re: git add -p with new file

[Ariel]

On Wed, 7 Dec 2016, Duy Nguyen wrote:

> On Wed, Dec 7, 2016 at 8:18 AM, Ariel <asgit@dsgml.com> wrote:

>> If you do git add -p new_file it says:

>> No changes.

>> Which is a rather confusing message. I would expect it to show me the
>> content of the file in patch form, in the normal way that -p works, let me
>> edit it, etc.

> We could improve it a bit, suggesting the user to do git add -N. But
> is there a point of using -p on a new file?

I got into the habit of always using -p as a way of checking my diffs 
before committing, so I ran it out of habit on a new file as well and got 
that confusing message.

It's even worse if you run it on multiple files, some changed, some added 
- the added ones are ignored completely, without any message at all.

> It will be one big chunk, you can't split anything.

That's fine, that's what I would expect.

> Perhaps maybe you want to use 'e' to edit what's added?

I might, but mainly it would show me what it was adding.

 	-Ariel

Re: git add -p with new file

[Ariel]

On Fri, 9 Dec 2016, Jeff King wrote:

> On Tue, Dec 06, 2016 at 08:18:59PM -0500, Ariel wrote:

>> If you do git add -p new_file it says:

>> No changes.

>> Which is a rather confusing message. I would expect it to show me the
>> content of the file in patch form, in the normal way that -p works, let me
>> edit it, etc.

> What should:
[git add directory with two new files]
> do?

It should do the exact same thing that git add without -p does: Add the 
directory and both files, but because of the -p also show the diff on the 
screen and ask.

> It's contrary to the rest of git-add for specifying pathspecs to 
> actually make things _more_ inclusive rather than less.

Is it? Because git add without -p is happy to add new files.

> Historically "add -p" has been more like "add -u" in updating tracked
> files.

But it doesn't have to be that way. You could make add -p identical to add 
without options, except the -p prompts to review diffs first.

> We have "-A" for "update everything _and_ new files". It doesn't
> seem unreasonable to me to have a variant of "-p" that is similar.

That seems unnecessarily complex because -p asks about each file, so you 
will never find new files added without realizing it.

> I don't think that variant should just be "add -N everything, and then
> run add -p". As Duy points out, the patch for a new file is just one big
> block. But the decision of "do I want to start tracking this untracked
> file" is potentially an interesting one.

What makes sense to me is a two part question: First ask 'Add new path', 
and then if yes, ask to stage the hunk (where the hunk is the entire 
file).

This makes -p useful on new files, without hurting prior expectations of 
how it works.

 	-Ariel

Re: git add -p with new file

[Jeff King]

On Fri, Dec 09, 2016 at 01:43:24PM -0500, Ariel wrote:

> > It's contrary to the rest of git-add for specifying pathspecs to
> > actually make things _more_ inclusive rather than less.
> 
> Is it? Because git add without -p is happy to add new files.

I was just speaking there of whether the presence of the file on the
command-line was relevant. In other words, "git add -u untracked-file"
does not countermand the "-u" to say "also add this file".

> > Historically "add -p" has been more like "add -u" in updating tracked
> > files.
> 
> But it doesn't have to be that way. You could make add -p identical to add
> without options, except the -p prompts to review diffs first.

The question is whether you would annoy people using "-p" if you started
including untracked files by default. I agree because it's inherently an
interactive process that we can be looser with backwards compatibility.

Perhaps a config option would be the best path forward (even if we were
to switch the default to "true", it leaves an escape hatch for people
who do not like the new behavior).

> > We have "-A" for "update everything _and_ new files". It doesn't
> > seem unreasonable to me to have a variant of "-p" that is similar.
> 
> That seems unnecessarily complex because -p asks about each file, so you
> will never find new files added without realizing it.

If you care about adding new files, wouldn't you just always use "-P"
instead of "-p"?

-Peff

Re: git add -p with new file

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Fri, Dec 09, 2016 at 01:43:24PM -0500, Ariel wrote:
> ...
>> But it doesn't have to be that way. You could make add -p identical to add
>> without options, except the -p prompts to review diffs first.
>
> The question is whether you would annoy people using "-p" if you started
> including untracked files by default. I agree because it's inherently an
> interactive process that we can be looser with backwards compatibility.

It might be interactive, but it will be irritating that we suddenly
have to see hundreds of lines in an untracked file before we are
asked to say "no I do not want to add this file" and have to do so
for all the untracked files that happen to match the given pathspec.

It might make it less irritating if one of the interactive choices
offered in the first prompt were N that tells the command: "No,
ignore all the untracked paths", though.  I dunno.

Also, because "git add -p" (no pathspec) is NOT a no-op, the
similarity Ariel sees with "git add" (no other options) does not
hold.  As you kept explaining (but perhaps not successfully being
understood yet), "add -p" works like "add -u", and it will make the
command incoherent if we allowed "git add -p <path>" to add new
paths, exactly because "git add -u <path>" does not do so.  Of
course that can be fixed by allowing "git add -u" to also add new
paths that match pathspec.

Of course, Ariel can vote for making "add -p" more like "add" (with
no options) than "add -u".  I _think_ it is a better way to solve
the incoherence than making "add -u" to add new paths.  But what it
means is that "add -p <paths>" works on both tracked and untracked
paths that match the given pathspec <paths>, and also "add -p" (no
pathspec) must become a no-op (because "add" without option and
pathspec is).

Re: git add -p with new file

[Jeff King]

On Sat, Dec 10, 2016 at 02:04:33PM -0800, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > On Fri, Dec 09, 2016 at 01:43:24PM -0500, Ariel wrote:
> > ...
> >> But it doesn't have to be that way. You could make add -p identical to add
> >> without options, except the -p prompts to review diffs first.
> >
> > The question is whether you would annoy people using "-p" if you started
> > including untracked files by default. I agree because it's inherently an
> > interactive process that we can be looser with backwards compatibility.
> 
> It might be interactive, but it will be irritating that we suddenly
> have to see hundreds of lines in an untracked file before we are
> asked to say "no I do not want to add this file" and have to do so
> for all the untracked files that happen to match the given pathspec.
> 
> It might make it less irritating if one of the interactive choices
> offered in the first prompt were N that tells the command: "No,
> ignore all the untracked paths", though.  I dunno.

Yeah, I agree dumping the contents automatically is annoying. Ariel
suggested asking twice about each path, which sounds clunky to me. I'd
probably give a simple question, with an option to dump the contents.
Like:

  $ echo foo >untracked
  $ git add -p
  New file: untracked
  Stage this file [y,n,v,q,a,d,/,e,?]? v     <-- user types 'v' for "view"

  diff --git a/untracked b/untracked
  index e69de29..257cc56 100644
  --- a/untracked
  +++ b/untracked
  @@ -0,0 +1 @@
  +foo
  Stage this file [y,n,v,q,a,d,/,e?]? y

Alternatively, "v" could just run "$GIT_PAGER <file>". The point is to
refresh your memory on what is in it before making a decision.

I'd also probably add interactive.showUntracked to make the whole thing
optional (but I think it would be OK to default it to on).

Some thought would have to be given handling binary files, as we
wouldn't want to dump their contents (but maybe showing them in a pager
would be OK).. We skip them entirely right now. So a related feature may
be asking "Stage this file" for binary files, with an option to somehow
view the contents.

I don't have plans to work on any of this myself. Just dumping thoughts
on what I'd expect an implementation to deal with.

-Peff

Re: git add -p with new file

[Stephan Beyer]

Hi Ariel,

On 12/09/2016 07:26 PM, Ariel wrote:
> On Wed, 7 Dec 2016, Duy Nguyen wrote:
>> We could improve it a bit, suggesting the user to do git add -N. But
>> is there a point of using -p on a new file?
> 
> I got into the habit of always using -p as a way of checking my diffs
> before committing,

Not commenting on the main issue here, but if you want to check your new
files with a "git add -p"-like tool, you can do

	git add <untracked files>
	git reset -p

(this is unstaging, but it's still useful)

Best
  Stephan

Re: git add -p with new file

[Stephan Beyer]

Hi,

On 12/11/2016 02:00 PM, Jeff King wrote:
> On Sat, Dec 10, 2016 at 02:04:33PM -0800, Junio C Hamano wrote:
>> Jeff King <peff@peff.net> writes:
>>> On Fri, Dec 09, 2016 at 01:43:24PM -0500, Ariel wrote:
>>> ...
>>>> But it doesn't have to be that way. You could make add -p identical to add
>>>> without options, except the -p prompts to review diffs first.
>>>
>>> The question is whether you would annoy people using "-p" if you started
>>> including untracked files by default. I agree because it's inherently an
>>> interactive process that we can be looser with backwards compatibility.
>>
>> It might be interactive, but it will be irritating that we suddenly
>> have to see hundreds of lines in an untracked file before we are
>> asked to say "no I do not want to add this file" and have to do so
>> for all the untracked files that happen to match the given pathspec.
>>
>> It might make it less irritating if one of the interactive choices
>> offered in the first prompt were N that tells the command: "No,
>> ignore all the untracked paths", though.  I dunno.
> 
> Yeah, I agree dumping the contents automatically is annoying. Ariel
> suggested asking twice about each path, which sounds clunky to me. I'd
> probably give a simple question, with an option to dump the contents.
> Like:
> 
>   $ echo foo >untracked
>   $ git add -p
>   New file: untracked
>   Stage this file [y,n,v,q,a,d,/,e,?]? v     <-- user types 'v' for "view"

I am also a "git add -p"-only user (except for new files and merges).
However, I usually keep a lot of untracked files in my repositories.
Files that I do not (git)ignore because I want to see them when I type
"git status".

Hence, the imagination only that "git add -p" starts to ask me for each
untracked file feels like a lot of annoying "n" presses. I could imagine
that it is okay-ish when it asks about the untracked files *after* all
tracked paths have been processed (I guess this has been proposed
before), so that I can safely quit.

I am also not really sure what problem this feature is trying to solve.
If the "problem"(?) is that it should act more like "git add" instead of
"git add -u", for whatever reason, this may be fine (but the
configuration option is a must-have then).

For me, I often had the problem that I simply forgot to add new
files...¹ Still I doubt that one would benefit from such a feature
because either:

- you do not have many untracked files (unlike me). You will see the
untracked file (that should be tracked) in the "git status" output when
you edit the commit message², then you abort-add-commit or
commit-add-amend and everything is fine.

Or:

- you have a lot of untracked files (like me). You won't see the
untracked file (that should be tracked) in the "git status" output (you
ignore it because the list is so long) and you won't see it in the "git
add -p" run because you quit before seeing that file.

So I have mixed feelings...

> I'd also probably add interactive.showUntracked to make the whole thing
> optional (but I think it would be OK to default it to on).
Hm, "interactive.showUntracked" is a confusing name because "git add -i"
(interactive) already handles untracked files.

Best
  Stephan

¹ I do not have that problem any more because I got used to add new
files right after saving, in a very early state, and then I simply use
"git add -p"...

² Unless you use git commit -m ... But using "git commit -m" without
"git status" before is user-issue anyway.

Re: git add -p with new file

[Jeff King]

On Mon, Dec 12, 2016 at 09:31:03PM +0100, Stephan Beyer wrote:

> I am also a "git add -p"-only user (except for new files and merges).
> However, I usually keep a lot of untracked files in my repositories.
> Files that I do not (git)ignore because I want to see them when I type
> "git status".
> 
> Hence, the imagination only that "git add -p" starts to ask me for each
> untracked file feels like a lot of annoying "n" presses. I could imagine
> that it is okay-ish when it asks about the untracked files *after* all
> tracked paths have been processed (I guess this has been proposed
> before), so that I can safely quit.

Yeah, this is the "some people might be annoyed" that I mentioned
originally. If your workflow leaves a lot of untracked files that you
don't care about it, then I think you'd want this feature disabled
entirely via a config option (or vice versa, that it would only be
enabled by config option).

> I am also not really sure what problem this feature is trying to solve.
> If the "problem"(?) is that it should act more like "git add" instead of
> "git add -u", for whatever reason, this may be fine (but the
> configuration option is a must-have then).

I think the problem is just that "add -p" does not give the whole story
of what you might want to do before making a commit.

> > I'd also probably add interactive.showUntracked to make the whole thing
> > optional (but I think it would be OK to default it to on).
> Hm, "interactive.showUntracked" is a confusing name because "git add -i"
> (interactive) already handles untracked files.

Sure, that was just meant for illustration. I agree there's probably a
better name.

-Peff

Re: git add -p with new file

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

>> I am also not really sure what problem this feature is trying to solve.
>> If the "problem"(?) is that it should act more like "git add" instead of
>> "git add -u", for whatever reason, this may be fine (but the
>> configuration option is a must-have then).
>
> I think the problem is just that "add -p" does not give the whole story
> of what you might want to do before making a commit.

The same is shared by "git diff [HEAD]", by the way.  It is beyond
me why people use "add -p", not "git diff [HEAD]", for the final
sanity check before committing.  

Perhaps the latter is not advertised well enough?  "add -p" does not
even page so it is not very useful way to check what is being added
if you are adding a new file (unless you are doing a toy example to
add a 7-line file).

>> > I'd also probably add interactive.showUntracked to make the whole thing
>> > optional (but I think it would be OK to default it to on).
>> Hm, "interactive.showUntracked" is a confusing name because "git add -i"
>> (interactive) already handles untracked files.
>
> Sure, that was just meant for illustration. I agree there's probably a
> better name.

"interactive.*" is not a sensible hierarchy to use, because things
other than "add" can go interactive.

addPatch.showUntracked?

Re: git add -p with new file

[Jeff King]

On Tue, Dec 13, 2016 at 10:48:07AM -0800, Junio C Hamano wrote:

> > I think the problem is just that "add -p" does not give the whole story
> > of what you might want to do before making a commit.
> 
> The same is shared by "git diff [HEAD]", by the way.  It is beyond
> me why people use "add -p", not "git diff [HEAD]", for the final
> sanity check before committing.  
> 
> Perhaps the latter is not advertised well enough?  "add -p" does not
> even page so it is not very useful way to check what is being added
> if you are adding a new file (unless you are doing a toy example to
> add a 7-line file).

I use "add -p" routinely for my final add-and-sanity-check, and it is
certainly not because I don't know about "git diff". I think it's just
nice to break it into bite-size chunks and sort them into "yes, OK" or
"no, not yet" bins. The lack of paging isn't usually a problem, because
this "add -p" is useful precisely when you have a lot of little changes
spread across the code base.

I'd probably also run "git diff" if I wanted to look at something
bigger. And I routinely use "git status", too, to see the full state of
my tree.

To me they are all tools in the toolbox, and I can pick the one that
works best in any given situation, or that I just feel like using that
day.

> >> Hm, "interactive.showUntracked" is a confusing name because "git add -i"
> >> (interactive) already handles untracked files.
> >
> > Sure, that was just meant for illustration. I agree there's probably a
> > better name.
> 
> "interactive.*" is not a sensible hierarchy to use, because things
> other than "add" can go interactive.
> 
> addPatch.showUntracked?

Hmm, I wonder if interactive.diffFilter was mis-named then. The
description is written in such a way as to cover other possible
interactive commands showing a diff.

It might be possible to do the same here: come up with a general option
that _could_ cover new situations, but right now just applies here. Or
maybe it would be too confusing.

TBH, I wasn't all that concerned with the name yet. Whoever writes the
patch can figure it out, and _then_ we can all bikeshed it. :)

-Peff

Re: git add -p with new file

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

>> Perhaps the latter is not advertised well enough?  "add -p" does not
>> even page so it is not very useful way to check what is being added
>> if you are adding a new file (unless you are doing a toy example to
>> add a 7-line file).
>
> I use "add -p" routinely for my final add-and-sanity-check,...
> ... To me they are all tools in the toolbox, and I can pick the one that
> works best in any given situation, or that I just feel like using that
> day.

Oh, there is no question about that.  I was just pointing out that
"add -p" is not the "one that works best" when dealing with a path
that is not yet even in the index.

git add -p with unmerged files (was: git add -p with new file)

[Stephan Beyer]

Hi,

While we're on the topic that "git add -p" should behave like the
"normal" "git add" (not "git add -u"): what about unmerged changes?

When I have merge conflicts, I almost always use my aliases
"edit-unmerged" and "add-unmerged":

$ git config --global --list | grep unmerged
alias.list-unmerged=diff --name-only --diff-filter=U
alias.edit-unmerged=!vim `git list-unmerged`
alias.add-unmerged=!git add `git list-unmerged`
alias.reset-unmerged=!uf=`git list-unmerged`; git reset HEAD $uf; git
checkout -- $uf

The "add-unmerged" alias is always a little scary because I'd rather
like to check the changes with the "git add -p" workflow I am used to.

Opinions?

Best
  Stephan

Re: git add -p with unmerged files (was: git add -p with new file)

[Jeff King]

On Tue, Dec 13, 2016 at 08:21:59PM +0100, Stephan Beyer wrote:

> While we're on the topic that "git add -p" should behave like the
> "normal" "git add" (not "git add -u"): what about unmerged changes?

I agree that's a related part of the workflow, though the implementation
is a bit harder.

> When I have merge conflicts, I almost always use my aliases
> "edit-unmerged" and "add-unmerged":
> 
> $ git config --global --list | grep unmerged
> alias.list-unmerged=diff --name-only --diff-filter=U
> alias.edit-unmerged=!vim `git list-unmerged`

You might like contrib/git-jump for that, which makes it easier to go to
the specific spots within files.

When "git jump merge" produces no hits, I know I've dealt with all of
the conflicts (textual ones, anyway). I do often want to run "git add
-p" then to review the changes before staging.

I think what is most helpful there is probably "git diff HEAD" to see
what the merge is bringing in (or the cherry-pick, or "am", or
whatever). If I wanted "add -p" to do anything, I think it would be to
act as if stage 2 ("ours", which should be the same as what is in HEAD)
was already in the index. I.e., show the diff against that, apply any
hunks we select, and store the whole thing as stage 0, losing the
unmerged bit.

When you select all hunks, this is equivalent to "git add unmerge-file".
If you choose only a subset of hunks, it leaves the unselected ones for
you to examine later via "git diff". And if you choose none, it should
probably leave unmerged.

That's just a scheme I thought up, though. I've never actually tried it
in practice.

-Peff

Re: git add -p with unmerged files

[Junio C Hamano]

Stephan Beyer <s-beyer@gmx.net> writes:

> While we're on the topic that "git add -p" should behave like the
> "normal" "git add" (not "git add -u"): what about unmerged changes?
>
>
> When I have merge conflicts, I almost always use my aliases
> "edit-unmerged" and "add-unmerged":
>
> $ git config --global --list | grep unmerged
> alias.list-unmerged=diff --name-only --diff-filter=U
> alias.edit-unmerged=!vim `git list-unmerged`
> alias.add-unmerged=!git add `git list-unmerged`
> alias.reset-unmerged=!uf=`git list-unmerged`; git reset HEAD $uf; git
> checkout -- $uf
>
> The "add-unmerged" alias is always a little scary because I'd rather
> like to check the changes with the "git add -p" workflow I am used to.
>
> Opinions?

For this, you would NEVER want to use "add -p" to pick and choose.
By definition, while you are in conflicted merge, the path that had
conflicts before you started the merge-y operation (be it "pull",
"am -3", or "cherry-pick") did not have any change since HEAD, and
"pick this hunk, drop that hunk" cannot be correct for the conflict
resolution.

"git diff" while conflicted will highlight what conflicted by
showing the three-way diff (similar to "diff --cc" on a merge
result) and after conflict is resolved you can view "diff HEAD"
on the path to see what the merge brought in.