From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Thu, 24 Jul 2008 12:20:51 GMT
Subject: Re: [PATCH] rebase -i: only automatically amend commit if HEAD did not change
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:

> 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.

> > @@ -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