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
next prev parent 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