Re: [PATCH V2 2/3] wt-status: pass struct repository and wt_status through function parameters
Junio C Hamano <gitster@pobox.com> writes:
Show 25 quoted lines
> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> I would like to point out that we already have a function which provides
>> a main worktree, see both `get_main_worktree()` & `is_main_worktree()`.
>> In short, a worktree with id = NULL seems to be treated as the main
>> worktree.
>>
>> The harder part would be correcting all code where `struct worktree *`
>> is passed and has special meaning for NULL vs non-NULL. See
>> `strbuf_worktree_gitdir()` which also distinguishes between `wt ==
>> NULL`, `wt->id == NULL` and `wt->id != NULL`.
>>
>> So cleanup would require identifying all such spots and fixing them too.
>
> Yup. That is why I upfront said "if you are ambitious" ;-)
>
>> This also is tricky. We currently already store all worktrees in the
>> repository in `struct strmap worktree_ref_stores`. Here, for the main
>> worktree we use '\' (see `get_worktree_ref_store()`). So perhaps we
>> should formalize using `\` for the main worktree everywhere.
>
> Is this a joke, is my terminal broken, or is my MUA hallucinating?
> I see a couple of backslashes in the above, and in the code I have
> a forward slash instead.
>
Seems like my fingers didn't type what my mind thought of.
Show 8 quoted lines
> But you are right, ref-store-map does use a slash to indicate the
> primary one, while worktree itself uses a NULL, which is somewhat
> understandable (NULL would not be a convenient hashmap key). And I
> do not think I see any downsides (other than "This used to take NULL
> as the sign of primari-ness but now we need to use a '/' instead"
> fixes we need everywhere) to use "/" on the wt->id side offhand.
>
> Thanks.
Yeah I share the same sentiment.