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

[Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>]

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

Actually wt_status_prepare() is the only function which is taking both
struct wt_status *s and struct repository *r, because it has to initialize
's' with the help of 'r'.
But all the other wt_status_.. helper functions only take one of the following,
i.e. either they take struct wt_status *s which is fine, or they take
struct repository *r. 

If we trace the callers of the functions with struct repository *r,
they are either being called in wt_status.c in which s->repo is being passed
as a parameter at the end so it shouldn't cause any issues,
and for any other file callers, the_repository is being passed,
so to remove struct repository *r, we would have to setup struct wt_status
in those files as well.

Best,
Shreyansh