Re: [PATCH 1/2] git-commit: Disallow unchanged tree in non-merge mode
- From
Dmitry V. Levin <ldv@altlinux.org>
- Date
- Sep 6, 2007, 10:16 UTC
- Message-ID
- <20070906101648.GD6665@basalt.office.altlinux.org>
- In-Reply-To
- <20070906022539.GG18160@spearce.org>
On Wed, Sep 05, 2007 at 10:25:39PM -0400, Shawn O. Pearce wrote:
Show 13 quoted lines
> "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.
Show 5 quoted lines
> 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.
Show 10 quoted lines
> > + 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.
Show 11 quoted lines
> > + 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