Re: [PATCH 1/2] git-commit: Disallow unchanged tree in non-merge mode

["Dmitry V. Levin" <ldv@altlinux.org>]

[-- Attachment #1: Type: text/plain, Size: 2474 bytes --]

On Wed, Sep 05, 2007 at 10:25:39PM -0400, Shawn O. Pearce wrote:
> "Dmitry V. Levin" <ldv@altlinux.org> wrote:
> > Do not commit an unchanged tree in non-merge mode.
> 
> A laudable goal.  git-gui also does this.  Turns out the other
> checks within git-gui prevent the user from ever getting that far.
> I probably should remove the empty tree check as it costs CPU time
> to get the old tree.  But I'd rather have the safety check.
>  
> > The idea is that amend should not commit an unchanged tree,
> > one should just remove the top commit using git-reset instead.
> 
> NO.  `git commit --amend` is *often* used for fixing the commit
> message.

You see, my proposed change does not affect this usage case at all.

> Or adding additional detail.

If that "additional" detail just undoes the latest commit, why should
"git commit --amend" welcome such thing?  I did not get your pint here.

> Forcing the user to do
> a `git reset --soft HEAD^ && git commit --amend` just because
> you don't want git-commit to make an "empty commit" (which it
> doesn't usually like to do now anyway!) is a major step back
> in functionality.

I suppose that helping users to avoid doing really stupid things
does not look as a major step back in functionality, just otherwise.

> > +		current_tree="$(git cat-file commit "$current${amend:+^}" 2>/dev/null |
> > +				sed -e '/^tree \+/!d' -e 's///' -e q)"
> 
> The better way to get the old tree would be this:
> 
> 		current_tree="$(git rev-parse "$current${amend:+^}^{tree}" 2>/dev/null
> 
> as it avoids the tool from needing to know about the internal
> representation of a commit object.  It also avoids an entire
> fork+exec of a sed process.

Agreed.

> > +		if test "$tree" = "$current_tree"
> > +		then
> > +			echo >&2 "nothing to commit${amend:+ (use \"git reset HEAD^\" to remove the top commit)}"
> 
> That message is a bad idea.  Doing a mixed mode reset will also
> reset the index, causing the user to lose any changes that had
> already been staged.  This may actually be difficult for him/her to
> recover from if they have used `git add -i` or git-gui to stage only
> certain hunks of files, or if their working tree has been further
> modified after the commit but they want to go back and amend the
> message only of the prior commit.

Would "git reset --soft HEAD^" advice be better than first one?
Could you suggest a better message, please?


-- 
ldv

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]