On Wed, Oct 01, 2008 at 11:06:54PM +0200, Andreas Ericsson wrote:
> of the commit result output. To make it read properly
> we get rid of "Created", which I just can't fit into
> a sentence without putting the branch-name last.
All of the other proposals indicate the hash and subject as the object
of creation. IOW, "created: <hash>: subject" or similar.
> Having taken inspiration from the "git reset" command,
> output for the three conceivable cases now look thus:
>
> normal commit
> <branch> is now at b930c4a (i386: Snib the sprock)
I think I still like your other proposal:
[branch] created b930c4a: "i386: Snib the sprock"
better. But in the interests of just agreeing on something, I am willing
to accept this. FWIW, the git-reset command doesn't use any delimiter
for the message:
<branch> is now at <hash> <subject>
So perhaps they should be the same. I don't think it overly matters.
> detached commit
> DETACHED HEAD is now at b930c4a (i386: Snib the sprock)
You mentioned the shouty caps before. I think "detached HEAD" is
probably caps enough, but not enough to argue for it (I just want to
mention as an informal vote if Shawn wants to mark it up while
applying).
> initial commit
> History has begun anew. Root-commit created.
> <branch> is now at bc930c4a (i386: Snib the sprock)
Heh.
> "Created" is a problem when one wants to put branch-name before the
> subject line, because the subject has to follow the hash (it doesn't
> describe the pre-state of the branch/detached head), but the newly
> added commit. "Created, on branch, hash (subject)" just looks
> stilted and stupid, so I had to change it. Hopefully this can be
> accepted. If not, count me out.
That was the reason for the helper function that was deleted. It
actually created a format string like "Created %h on <branch>: %s" and
properly escaped the percents in <branch>. So you would have to keep it
if you wanted to interleave the data (but I think what you have is
better -- the branch name or the detached status is the thing that
should be first).
> I'm not sure if the last "else" case setting branch = head; can
> ever happen, but I figured it can't hurt to make sure. Feel free
> to modify commentary around it or the entire section when applying.
It should definitely be there, if only for the sanity of future
expansion (and because I can technically put whatever ref I want into
HEAD :) ).
> - printf("%s\n", buf.buf);
> - strbuf_release(&buf);
> + printf("%s\n", strbuf_detach(&buf, NULL));
This change is bogus. "release" frees a strbuf. "detach" says "Give me
the buffer, and I will take care of freeing it later myself". So you
introduced a leak.
-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
