[PATCH 0/2] reset -p and unborn branches

From: Jeff King <peff@peff.net>
Date: Oct 25, 2013, 06:51 UTC
Message-ID: <20131025065117.GA15192@sigill.intra.peff.net>
In-Reply-To: <CANiSa6iYLp-iNdcv_qbFTitrfFxaDBVUy9YyUAF4QKM+-35P4A@mail.gmail.com>

On Thu, Oct 24, 2013 at 10:42:52PM -0700, Martin von Zweigbergk wrote:

Show 8 quoted lines

> > I think that's the correct fix for the regression.  You are restoring
> > the original, pre-166ec2e9 behavior for just the HEAD case. I do not
> > think add--interactive does any other magic between a symbolic rev and
> > its sha1, except for recognizing HEAD specially. However, if you wanted
> > to minimize the potential impact of 166ec2e9, you could pass the sha1
> > _only_ in the unborn case, like this:
> 
> Plus, the end result is more readable, IMHO.

Agreed. Unfortunately it is slightly wrong, because for the non-patch cases, we may look at "rev" later, and we would want it to still say "HEAD" rather than a sha1. This is fixed in my patches below.

Show 11 quoted lines

> >   1. Pass the head/not-head flag as a separate option.
> >
> >   2. Pass HEAD even in the unborn case; teach add--interactive to
> >      convert an unborn HEAD to the empty tree.
> >
> >   3. Teach add--interactive to recognize the empty tree sha1 as an
> >      "unstage" path.
> [...]
> Makes sense to me. I'm sure others can implement that much faster than
> I can, but I feel a little guilty, so I'm happy to do it if no one
> else wants to, as long as we agree this is the way we want to go.

As it turns out, add--interactive already _does_ know how to handle an unborn HEAD. It just didn't use it for this particular code path. So I think doing (2) makes the most sense, and the result is that the patch in reset.c ends up nice and simple.

  [1/2]: add-interactive: handle unborn branch in patch mode
  [2/2]: reset: pass real rev name to add--interactive

-Peff