Re: [PATCH 1/3] wt-status: replace uses of the_repository with local repository instances

[Junio C Hamano <gitster@pobox.com>]

Junio C Hamano <gitster@pobox.com> writes:

> Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com> writes:
>
>>> > diff --git a/wt-status.c b/wt-status.c
>>> > index e12adb26b9..9f4d8fda7f 100644
>>> > --- a/wt-status.c
>>> > +++ b/wt-status.c
>>> > @@ -150,11 +150,11 @@ void wt_status_prepare(struct repository *r, struct wt_status *s)
>>> >	s->show_untracked_files = SHOW_NORMAL_UNTRACKED_FILES;
>>> >	s->use_color = GIT_COLOR_UNKNOWN;
>>> >	s->relative_paths = 1;
>>> > -	s->branch = refs_resolve_refdup(get_main_ref_store(the_repository),
>>> > +	s->branch = refs_resolve_refdup(get_main_ref_store(s->repo),
>>> >					"HEAD", 0, NULL, NULL);
>>> 
>>> Wouldn't it make more sense to use the function argument 'r' here?
>>
>> In wt_status_prepare(), s->repo is initialized to r at the top of
>> the function, so both refer to the same repository instance. However,
>> using r directly is more explicit and avoids indirect use.
>> will change this in V2.
>
> Would we benefit from further clean-up, either before or after this
> change, to lose the "struct repository *r" parameter, if we know
> that we can depend on s->repo being the repository we are collecting
> the status information in the wt_status structure for?

Clarification.  This function is like an initializer for the struct
wt_status instance at 's', so it has to take both "struct repository"
parameter, but what I meant was other wt_status_foo() functions that
take both r and s as parameters.  Once s has been initialized and
s->repo becomes valid, passing r as a separate parameter, as if you
can feed a different instance of "struct repo", becomes confusing
and a source of bugs.