Olga Pilipenco <olga.pilipenco@xxxxxxxxxxx> 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.
