Subject: Re: [PATCH v3 1/2] worktrees: add die_if_shared_symref

From: David Turner <[email-masked]>
Date: Fri, 31 Jul 2015 18:08:45 -0400

On Fri, 2015-07-31 at 17:36 -0400, Eric Sunshine wrote:
> On Fri, Jul 31, 2015 at 5:15 PM, David Turner <[email-masked]> wrote:
> > On Fri, 2015-07-31 at 15:35 -0400, Eric Sunshine wrote:
> >> On Fri, Jul 31, 2015 at 3:01 PM, David Turner <[email-masked]> wrote:
> >> > Add a new function, die_if_shared_symref, which works like
> >> > die_if_checked_out, but for all references.  Refactor
> >> > die_if_checked_out to work in terms of die_if_shared_symref.
> >> >
> >> > Soon, we will use die_if_shared_symref to protect notes merges in
> >> > worktrees.
> >>
> >> I wonder if the diagnostic:
> >>
> >>     'blorp' is already checked out at '/path/name/'
> >>
> >> emitted by check_linked_checkout() needs to be adjusted for this
> >> change. It still makes sense for die_if_checked_out(), but sounds odd
> >> for die_if_shared_symref().
> >
> > How about:
> >
> > 'refs/notes/y' is already referenced from 'NOTES_MERGE_REF' in
> > '/home/dturner/git/t/trash directory.t3320-notes-merge-worktrees/'
> >
> > Does that make sense?
> 
> That might be the best we can do for the generic case of
> die_if_shared_symref(), but I wonder how easily the typical user would
> understand it when trying to checkout a branch already checked out
> elsewhere. One concern is that that message almost comes across as an
> internal Git error (thus inscrutable), whereas:
> 
>     % git checkout blorp
>     'blorp' is already checked out at '/some/path/'
> 
> seems (to me) pretty clearly a user error, thus more easily understood.

Sorry, I meant that for the non-HEAD case.  For the
die_if_already_checked_out case, the original error message would
remain.

But I'll just go ahead and do it the nice way.