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

[Jeff King <peff@peff.net>]

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