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

> > 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.