Re: [PATCH 6/9] worktree: teach 'add' to respect --force for registered but missing path

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Aug 30, 2018, 08:26 UTC
Message-ID: <CAPig+cSSd4kgdfCsgmJ7omTTEn9Qve1DfDUtmJ7qkL5o0JS-hA@mail.gmail.com>
In-Reply-To: <20180830073642.GC11944@sigill.intra.peff.net>

On Thu, Aug 30, 2018 at 3:36 AM Jeff King <peff@peff.net> wrote:

Show 12 quoted lines

> On Tue, Aug 28, 2018 at 05:20:23PM -0400, Eric Sunshine wrote:
> > +     if ((!locked && opts->force) || (locked && opts->force > 1)) {
> > +             if (delete_git_dir(wt->id))
> > +                 die(_("unable to re-add worktree '%s'"), path);
> > +             goto done;
> > +     }
>
> This "unable to re-add" seemed funny to me at first, since the failure
> is in deletion. I guess we're relying on delete_git_dir() to already
> have said "I had trouble deleting $GIT_DIR/worktrees/foo", and this is
> just the follow-up to tell that the whole operation is cancelled. So
> that makes sense.

Correct, delete_git_dir() has already used error() to explain that it couldn't delete .git/worktrees/<id>, so this finalizing die() doesn't need to repeat that.

> I wonder if we should volunteer the information that we're overwriting
> an existing worktree. I guess the user would generally know that
> already, though, since they just specified "-f", so it's probably just
> being overly chatty to do so.

That's my conclusion, as well. An explicit use of --force shouldn't need such chattiness.