Re: [PATCH] rebase -i: only automatically amend commit if HEAD did not change

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jul 24, 2008, 12:20 UTC
Message-ID: <alpine.DEB.1.00.0807241311450.8986@racer>
In-Reply-To: <7vmyk888z5.fsf@gitster.siamese.dyndns.org>

Hi,

On Wed, 23 Jul 2008, Junio C Hamano wrote:

Show 11 quoted lines

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > If the user called "rebase -i", marked a commit as "edit", "rebase 
> > --continue" would automatically amend the commit when there were 
> > staged changes.
> >
> > However, this is actively wrong when the current commit is not the one 
> > marked with "edit".  So guard against this.
> 
> At what point in what valid workflow sequence does HEAD become different 
> from dotest/amend?

$ rebase -i HEAD~5

	<mark one commit as edit>

	<Whoa! While editing, I realize that this contains an unrelated 
	 bugfix>

$ git reset HEAD^ $ git add -p $ git commit

	<Edit a bit here, a bit there>

$ git rebase --continue

Sure it is a pilot error.  It hit this pilot, too.

Show 13 quoted lines

> > @@ -419,7 +419,9 @@ do
> >  		else
> >  			. "$DOTEST"/author-script ||
> >  				die "Cannot find the author identity"
> > -			if test -f "$DOTEST"/amend
> > +			if test -f "$DOTEST"/amend &&
> > +				test $(git rev-parse HEAD) = \
> > +					$(cat "$DOTEST"/amend)
> >  			then
> >  				git reset --soft HEAD^ ||
> >  				die "Cannot rewind the HEAD"
> 
> In what way is this "guarding against it"?

In the way that the user certainly did not mean to amend _this_ HEAD. Another HEAD was marked with "edit".

By not amending, the user has a chance to fix anything unintended in another rebase -i more easily, since the two commits have not been squashed together.

'course, the user should have seen in the editor that popped up that she is squashing two different commits, should have deleted the whole commit message to abort, make the independent commit herself (finding the correct commit to steal the commit message from), then run rebase --continue (which would no longer commit anything, there being nothing to).

However, that course of action is a bit unintuitive.

The way it runs with my patch, at least a user has a chance to fix it up without a Git expert standing nearby.

I will definitely keep this in my personal fork, even in my personal fork of "master" during the rc period. But if you think it is not worth it, and others seem to be utterly disinterested (instead discussing behavior changes), I will not push further.

Ciao, Dscho