Subject: Re: [PATCH] refs: exit early from the loop if it is not a main worktree

From: Patrick Steinhardt <[email-masked]>
Date: Fri, 27 Dec 2024 15:34:17 +0100

On Wed, Dec 18, 2024 at 06:52:02PM -0500, Eric Sunshine wrote:
> On Wed, Dec 18, 2024 at 8:30 AM shejialuo <[email-masked]> wrote:
> > On Wed, Dec 18, 2024 at 02:20:45AM +0000, AreaZR via GitGitGadget wrote:
> > >               if (is_main_worktree(worktrees[i]))
> > >                       continue;
> > >               ret = 1;
> > > +             break;
> >
> > So, when we find a linked worktree, we just return the value. From my
> > perspective, if we decide to optimize like this way, we could drop the
> > loop because the first element of the result of `get_worktrees` is the
> > main worktree. And we could just check whether the "worktrees[1]" is
> > NULL to do above.
> 
> You're correct. get_worktrees() guarantees that the main worktree (or
> bare repository) is the first item in the list, so merely checking
> whether `worktrees[1]` is non-NULL would be sufficient to answer
> whether linked worktrees are present; no looping is required.
> 
> > However, I don't know whether it's a good idea to exit the loop early
> > in the first place. CC Patrick to help.
> 
> If the loop is retained for some reason (though it really isn't
> needed), then exiting early is indeed desirable. I suspect that the
> missing `break` was just a silly oversight on Patrick's part.

Yes, indeed, breaking out of the loop feels sensible to me. Checking
whether `worktrees[1]` is non-NULL would be fine, as well.

Patrick