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

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 6, 2012, 06:26 UTC
Message-ID: <7v8vbkru8o.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20121005225758.GA1202@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 6 quoted lines

> Actually, I am not sure that thread or feature is to blame. Certainly it
> would have been an opportune time to notice the problem. But this issue
> goes back much further for "git commit --interactive", which has always
> assumed "-i" rather than "-o". This even predates the switch from shell
> to C; you can see the same behavior from 6cbf07e (git-commit: add a
> --interactive option, 2007-03-05).

Yes. That was after we started defaulting to "only" (not "also") semantics when the command is run with paths, and it should also have raised a red flag to reviewers.

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.

Show 8 quoted lines

>> I think the right thing to do is to fix "git commit -p" so that it
>> starts from the HEAD (on a temporary index), just like how partial
>> commits are made with "git commit file1 file2".   Or just forbid it
>> when the index does not match HEAD.
>
> Agreed. I am inclined to call this a bugfix, though it does worry me
> slightly that we would be changing a behavior that has existed for so
> many years.

I agree it will be a bugfix, but I am afraid that the fix may have to be much more involved than "start from a temporary index that matches HEAD when we are doing the '--only' semantics".

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?

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.

[Footnote]

*1* A not-so-deep thinking of the above might lead to "start from the index that match HEAD, except for paths specified on the pathspec given to the -p option". But I do not think it is satisfactory, either. With "add -i" (or "commit --interactive"), you have an option to selectively discard parts of your previous, overzealous "git add F" with its r)evert action, but because "commit -p" does not give an option to switch to "reset -p", you can only add hunks, People who did "git add E F" earlier and then wants to amend that earlier add with "git commit -p F", but it does not allow them to fully amend their earlier action. That is the one of the reasons why I think "commit -p" is a mistaken "we can save one command invocation" false economy that adds confusion without adding much value to the UI.