From: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
Date: Sun, 29 Mar 2026 09:55:34 GMT
Subject: Re: [PATCH v2 1/5] refs: make branchname helpers repository aware
Message-ID: <CAPYXD64H654k0RoKDiU91dsj0Fr9mf+f46Vfe54K+zueMzh+0A@mail.gmail.com>
In-Reply-To: <c3f032e4-d50b-4e35-846f-5f5da95f82a6@gmail.com>

On Sat, Mar 28, 2026 at 10:24 PM Tian Yuchen <a3205153416@gmail.com> wrote:
>
> Hi Shreyansh,
>
> On 3/28/26 22:09, Shreyansh Paliwal wrote:
>
>
> > @@ -5,6 +5,7 @@
> >   #include "refs.h"
> >   #include "setup.h"
> >   #include "strbuf.h"
> > +#include "repository.c"
>
> I'm surprised that it doesn't cause any errors. Or maybe you haven't
> build it yet?
>

Thanks for pointing this out. Apparently it didn't raise any errors in the
build or test suite. Will send a reroll.

>
> ---
>
>
> > -int check_branch_ref(struct strbuf *sb, const char *name)
> > +int check_branch_ref(struct repository *repo, struct strbuf *sb, const char *name)
> >   {
> >       if (startup_info->have_repository)
> > -             copy_branchname(sb, name, INTERPRET_BRANCH_LOCAL);
> > +             copy_branchname(repo, sb, name, INTERPRET_BRANCH_LOCAL);
> >       else
> >               strbuf_addstr(sb, name);
> >
>
> startup_info itself is a global variable, isn't it?
>
> I think a more appropriate approach is something like:
>
>         if (repo && repo->gitdir)
>                 copy_branchname(repo, sb, name, INTERPRET_BRANCH_LOCAL);
>
> I dunno, just a thought.

I hadn’t considered this as it is outside my scope of this patch series.
While the change makes sense, I am not very sure whether it could cause
any behavioral change or not.
Thanks.