Git development
 help / color / mirror / Atom feed
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] rebase -i: only automatically amend commit if HEAD did not change
Date: Thu, 24 Jul 2008 13:20:51 +0100 (BST)	[thread overview]
Message-ID: <alpine.DEB.1.00.0807241311450.8986@racer> (raw)
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

  reply	other threads:[~2008-07-24 12:21 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-07-22 21:36 [PATCH] rebase -i: only automatically amend commit if HEAD did not change Johannes Schindelin
2008-07-22 21:48 ` Stephan Beyer
2008-07-22 22:22 ` Avery Pennarun
2008-07-22 23:55   ` Junio C Hamano
2008-07-23 15:55     ` Avery Pennarun
2008-07-23 12:01   ` Dmitry Potapov
2008-07-23 15:53     ` Avery Pennarun
2008-07-23 16:09       ` Johannes Schindelin
2008-07-23 16:19         ` Avery Pennarun
2008-07-23 23:41 ` Junio C Hamano
2008-07-24 12:20   ` Johannes Schindelin [this message]
2008-07-24 12:35     ` Stephan Beyer
2008-07-25  8:44     ` Junio C Hamano
2008-07-25 10:35       ` Johannes Schindelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.1.00.0807241311450.8986@racer \
    --to=johannes.schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox