On Tue, Feb 4, 2025 at 12:43 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > 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. Perfect, I'll add the latter one then. Thank you!
