All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Thomas Rast <trast@inf.ethz.ch>
Cc: Shezan Baig <shezbaig.wk@gmail.com>,
	Thomas Rast <trast@student.ethz.ch>, <git@vger.kernel.org>,
	Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
Subject: Re: [PATCH] rebase -i: avoid checking out $branch when possible
Date: Fri, 18 May 2012 08:25:45 -0700	[thread overview]
Message-ID: <xmqqaa154ikm.fsf@junio.mtv.corp.google.com> (raw)
In-Reply-To: <87obplhp12.fsf@thomas.inf.ethz.ch> (Thomas Rast's message of "Fri, 18 May 2012 10:27:53 +0200")

Thomas Rast <trast@inf.ethz.ch> writes:

>>  - The user told us not to do the rebase by making insn sheet empty.  In
>>    other words, this is "aborting the entire rebase", so ideally it
>>    should come back to the state before the user ran "git rebase"
>>    command (i.e. where she was before we switched to <branch>).
>> 
>>    I do not think this ideal behaviour is something neither batch or
>>    interactive rebase has traditionally implemented, but I can see how
>>    we can sell this as a bugfix to the end users.
>
> That's a convincing argument, so let's make it so.
>
>>  - It turns out that everything is already contained and there is
>>    nothing to apply, i.e. after this sequence:
>> 
>> 	git checkout branch
>> 	git checkout $onto_or_merge_base_between_base_and_branch
>> 
>>    we find out that "git cherry $onto_or_merge_base branch" is empty.
>
> Is there a command missing here?  This alone does not make them the
> same, perhaps you meant some resetting.

Actually that was phrased rather poorly.  At least "after this sequence"
should have been "during this sequence inside the implementation of
'rebase -i'".  In other words, the scenario is still where the user
typed:

  git rebase [-i] [--onto $onto] $base $branch

which is defined to be equivalent to

  git checkout $branch && git rebase [-i] [--onto $onto] $base

The first half of the equivalent command succeeds.  We are on $branch.
The second half finds there is nothing to be done.

I think the current code (and probably with your patch) does this in
such a case:

has_action "$todo" ||
	die_abort "Nothing to do"

and the die happens on $branch before we detach (iow, between the two
checkouts), and the user should end up on $branch.  Unlike the case
where the user told us to abort the entire rebase, which we agreed that
we should come back to the original branch above, I think in this case
we should not nullify the first "checkout $branch".

  reply	other threads:[~2012-05-18 15:25 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-20 15:05 [PATCH] rebase -i: avoid checking out $branch when possible Thomas Rast
2012-04-20 15:52 ` Junio C Hamano
2012-04-20 16:01   ` Thomas Rast
2012-04-20 20:06     ` Junio C Hamano
2012-05-15 16:26       ` Shezan Baig
2012-05-15 17:08         ` Junio C Hamano
2012-05-18  8:27           ` Thomas Rast
2012-05-18 15:25             ` Junio C Hamano [this message]
2012-04-20 16:45 ` Johannes Sixt

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=xmqqaa154ikm.fsf@junio.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=martin.von.zweigbergk@gmail.com \
    --cc=shezbaig.wk@gmail.com \
    --cc=trast@inf.ethz.ch \
    --cc=trast@student.ethz.ch \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.