Re: [PATCH v3] worktree: detect from secondary worktree if main worktree is bare

[Olga Pilipenco <olga.pilipenco@shopify.com>]

On Tue, Feb 4, 2025 at 12:43 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Olga Pilipenco <olga.pilipenco@shopify.com> writes:
>
> > I have 2 versions for comment:
> >
> > 1. Since is_main_worktree_bare explains quite well what it does we can have
> > a shorter explanation of `!worktree->is_current` part, something like:
> >
> > /* Additional checks are needed if main worktree is not current
> > (checking from secondary worktree) */
> > (!worktree->is_current && is_main_worktree_bare(the_repository));
>
> For somebody who thought about the issue themselves (like me, before
> writing the message you are responding to), this shorter form would
> suffice.  I'd rephrase it more like so
>
>     /* When a secondary worktree, an extra check is needed */
>
> for brevity, though.
>
>
> > 2. Or a bit longer inline explanation that partially repeats the
> > explanation of is_main_worktree_bare
> > + adds explanation about efficiency:
> >  /*
> >   * When in a secondary worktree we have to also verify if the main worktree
> >   * is bare in $commondir/config.worktree.
> >   * This check is unnecessary if we're currently in the main worktree,
> >   * as prior checks already consulted all configs of the current worktree.
> >  */
> > (!worktree->is_current && is_main_worktree_bare(the_repository));
>
> And this more extended version would have helped me by not having to
> ask
>
>     Is "this worktree does not have is_current bit set" equivalent
>     to "this worktree is the main one, so is_main_worktree_bare()
>     needs to be consulted"?  That linkage between "the is_current
>     bit unset" and "is the main worktree" is not obvious to me.
>
> in the first place.
>
> In short, both should work, and I personally find that the latter
> may be a bit more helpful to readers.
>
> THanks.

Perfect, I'll add the latter one then. Thank you!