Re: git 1.8.0.rc0.18.gf84667d trouble with "git commit -p file"

On Fri, Oct 05, 2012 at 11:26:47PM -0700, Junio C Hamano wrote:

In the case of "add/commit --interactive", it is much more clear
what state the index is in when the command gave interactive control
to the user.  The short-cut "add/commit -p" interface, however, does
not give you an access to its s)tatus subcommand, making the user
experience somewhat different.

That makes the problem much more severe for "-p" compared to
"--interactive", but the fundamental UI consistency it introduces is
the same as the issue under discussion in this thread.

Agreed.

Suppose you have two paths E and F, both of which have differences
between HEAD and the index, and the index and the working tree file
(i.e. you earlier edited E and F, did "git add E F" and further
edited them).

You say "git commit -p F".

What should happen?  It is clear that the resulting commit should
record no change since its parent commit at path E (that is what
"only" semantics mean).

What state should the "add -p" interaction start from for path F?
Should you be picking from a patch between the state you previously
"git add"ed to the index and the working tree, or should the entire
difference between HEAD and the working tree eligible to be picked
or deferred during the "add -p" session?  Starting from a temporary
index that matches HEAD essentially means that you lose the earlier
"git add F" [*1*].

Another case to consider is to start from the same condition, and
instead to say "git commit -p" without any pathspec.  What should
happen?

Hmm. Good questions. In the former case, I would have said you should
definitely omit E and then start from the staged point of F, as that is
almost certainly what the user meant. But that is utterly inconsistent
with what we are discussing for the no-pathspec case.

I have a gut feeling that what I would expect for "-p" is roughly:

  1. Feed add--interactive the current index state.

  2. Feed add--interactive the set of pathspecs on the command line to
     limit its work.

  3. For any path that is updated by the interactive session, keep the
     result.

  4. For other paths, revert to HEAD.

I think that would "do what I mean" most of the time. But it is a
horrible set of rules to try to explain to someone (and it is off the
top of my head; I wouldn't be surprised if you can come up with a
situation where those rules do not behave well).

Just doing "use a temporary index that is initialized to HEAD" may
be an expedient thing to do, but I suspect that I will be saying the
same "I should have said 'You cannot have a pony' back then" again
in a not so distant future if we did so without thinking these
things through.

As I do not see any practical value in "commit -p", I do not think
it is worth my time thinking these things through thoroughly myself.

Unless somebody who cares about "commit -p" does so to come up with
reasonable semantics, and updates the code to match that desired
behaviour, the responsible thing to do is to error out "-p" when
your index is different from HEAD, I think.

Yeah. I did not agree with your conclusion here when we started the
conversation, but I am starting to now. I am not opposed at all to
somebody working out the semantics, but I do not really care to work on
it myself. In the meantime, I would rather not do any halfway fixes
that will just make things worse.

Another option is to leave it with "-i" semantics in the meantime, which
are at least easy to explain: it is simply a shorthand for running "git
add -p && git commit". That may be inconsistent with other aspects of
commit, but people have (apparently) been happy with it, and there has
not been a rash of complaints.

As a non-user of "commit -p" myself, I don't have a strong opinion
either way.

-Peff