On Sat, Apr 02, 2011 at 12:16:23PM -0700, Junio C Hamano wrote:
> When running "commit" and "status" with files marked with "intent to add",
> I think there are three possible interpretations of what the user wants to
> do.
>
> (1) I earlier said "I'll decide the exact contents to be committed for
> these paths and tell you by running 'git add' later." when I said
> 'git add -N'. But I forgot to do so before running "git commit".
> Thanks for catching this mistake and erroring out.
>
> (2) I said "I'll decide the exact content to be committed ... later."
> when I said 'git add -N'. I am running "git commit" now, but I still
> don't know what the final contents for this path should be. I
> changed my mind, and I do not want to include the addition of these
> paths in the commit I am making. Please do not error out, but just
> ignore the earlier 'add -N' for now.
>
> (3) I said "I'll decide the exact content to be committed ... later."
> when I said 'git add -N'. I am running "git commit" now, without
> explicitly telling you with 'git add' about the final contents for
> these paths. Please take it as an implicit hint that I am happy with
> the contents in the working tree and commit them as if I said 'git
> add' on these paths, but do leave modifications to already tracked
> paths that I haven't added with 'git add'.
>
> The current behaviour of "git commit" that refuse to commit without
> telling git what the final contents for these pathse is a very deliberate
> design and implementation of (1).
I think that all three of those are reasonable things to want to do, and
that the current behavior of (1) is a nice, conservative default. But
where we could do better is in helping the user understand what happened
and what their options are.
In Laszlo's example, he saw:
$ git commit
lib/achievement.cpp: not added yet
lib/achievement.h: not added yet
lib/achievementparser.cpp: not added yet
lib/achievementparser.h: not added yet
error: Error building trees
I see a couple of places to improve:
1. We say "not added yet", which is a good start. We at least point
out the problematic paths. But it's perhaps a little confusing.
Those paths _have_ been added. It's just that no content was added
at them yet.
2. The "error: Error building trees" is unnecessarily scary, as it
looks like git ran into some error while trying to do what you
wanted (and indeed, that is the same message you get for something
like "oops, we couldn't write to the object db"). It's not even
clear from that output that the "not added yet" lines are the cause
of the error, unless you understand how "git commit" is
implemented.
3. There is no advice given on how to proceed.
So I think a much nicer output would be something like:
$ git commit
error: some paths could not be committed
The following paths were marked with intent-to-add, but have
not had any content added:
lib/achievement.cpp
lib/achievement.h
lib/achievementparser.cpp
lib/achievementparser.h
If you want to commit them with their current content, use "git add".
If you want git to forget about them, use "git rm --cached".
We could also provide some options to git commit to make those
operations easier. Especially "ignore these for this commit but leave
them in the index" is a little awkward to do.
> Having said all that, we might want to differentiate these paths in the
> output from 'status'.
Yes, that was my first thought on reading Laszlo's message. We are
somewhat lying to say "changes to be committed". Your suggestion:
> # ... to be committed ...
> # new file: RENAMING (needs 'git add')
> # ... have local changes ...
> # modified: RENAMING
You could also give them a separate stanza, like:
# Changes to be committed:
# (use "git reset HEAD <file>..." to unsage)
#
# modified: some-other-file
#
# Paths marked for addition, but needing content:
# (use "git add <file>..." to update what will be committed)
#
# new file: RENAMING
> if we don't care about the backward compatibility.
I don't know how much we need to care about "git status" output of this
form. It is parsed mainly be editors' syntax highlighters. Adding this
new information might not get highlighted as nicely, but it's probably
not a big deal.
I am much more concerned with whether and how this information would be
represented in the "git status --porcelain" format.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html