Re: [PATCH v2 3/3] rebase: rebasing can also be done when HEAD is detached

From: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
Date: Nov 28, 2017, 16:15 UTC
Message-ID: <1511885730.10193.8.camel@gmail.com>
In-Reply-To: <xmqqlgir8770.fsf@gitster.mtv.corp.google.com>

On Tue, 2017-11-28 at 11:31 +0900, Junio C Hamano wrote:

Show 6 quoted lines

> Kaartic Sivaraam <kaartic.sivaraam@gmail.com> writes:
> 
> > +		if test "$branch_or_commit" = "HEAD" &&
> > +			 !(git symbolic-ref -q HEAD)
> 
> Did you need a subshell here?

No. That's a consequence of me not remembering that I would span a sub- shell with a simple '()' when I was doing that part. (partial transition from C to shell)

Show 12 quoted lines

>  Now with a proper test with
> "symbolic-ref -q HEAD", I wonder if you'd need to check if the
> original was named HEAD in the first place (I do not feel strongly
> enough to say that checking is wrong, at least not yet, but the
> above does make me wonder), and instead something like
> 
> 	if ! git symbolic-ref -q HEAD
> 	then
> 		...
> 
> might be sufficient.  I dunno.
>

It does seem you're right. The only thing we would be losing is the short-circuiting when $branch_or_commit is not HEAD (which I suspect to be the case most of the time). So, I'm not sure if I should remove the check (of course, I'll change the check to avoid spawning a sub-shell).

Thanks, Kaartic