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

[Junio C Hamano <gitster@pobox.com>]

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.