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

!MAILaRCHIVE_VOTE_RePLACE
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
To: Dmitry V. Levin <ldv@...>
Cc: Git Mailing List <git@...>, Junio C Hamano <gitster@...>
Date: Wednesday, September 5, 2007 - 10:25 pm

"Dmitry V. Levin" <ldv@altlinux.org> wrote:

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.
 

NO.  `git commit --amend` is *often* used for fixing the commit
message.  Or adding additional detail.  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.

NACK.
 

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.


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.

-- 
Shawn.
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
Re: [PATCH 1/2] git-commit: Disallow unchanged tree in non-m..., Shawn O. Pearce, (Wed Sep 5, 10:25 pm)