Re: [PATCH V2 2/3] wt-status: pass struct repository and wt_status through function parameters

[Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>]

> Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com> writes:
> 
> >  branch.c    |  4 ++--
> >  worktree.c  |  4 ++--
> >  wt-status.c | 48 +++++++++++++++++++++++++-----------------------
> >  wt-status.h |  6 ++++--
> >  4 files changed, 33 insertions(+), 29 deletions(-)
> >
> > diff --git a/branch.c b/branch.c
> > index 243db7d0fc..e3cf273339 100644
> > --- a/branch.c
> > +++ b/branch.c
> > @@ -412,7 +412,7 @@ static void prepare_checked_out_branches(void)
> >  			free(old);
> >  		}
> >  
> > -		if (wt_status_check_rebase(wt, &state) &&
> > +		if (wt_status_check_rebase(wt->repo, wt, &state) &&
> 
> I am not sure if this is an improvement for callers of the API.
> 
> Isn't wt_anything() that takes a worktree "wt" supposed to work with
> the wt->repo repository?  Or is the API designed to be used to take
> any repository object that is _different_ from wt->repo?  I am
> assuming it is the former, and if so, the only effect of adding a
> repository parameter to a function that already takes struct
> worktree is to invite a programming error to pass a repository that
> the wt is not designed to work with, isn't it?
> 
> > -		if (wt_status_check_bisect(wt, &state) &&
> > +		if (wt_status_check_bisect(wt->repo, wt, &state) &&
> 
> Ditto.

You are right that when a worktree is present, the repository
associated with should be wt->repo, and callers should not be expected
to provide something different.

The reason I introduced an explicit struct repository *r
parameter I have included in the previous reply.

So the intention is not to allow a repository different from
wt->repo, but to make it possible to operate in the absence of a
worktree. When wt is non-NULL, the expectation remains that callers
pass wt->repo, and in the case in which passed repo and wt->repo are
different worktree_git_path() would handle this case in a BUG().

Best,
Shreyansh