Re: [PATCH 06/20] path: stop relying on `the_repository` in `worktree_git_path()`

From: Patrick Steinhardt <ps@pks.im>
Date: Aug 13, 2024, 09:25 UTC
Message-ID: <Zrsmjl7BVWhxuolz@tanuki>
In-Reply-To: <cqc4xlbrusu5edfsh2snvekkd2qfwjxu3x33pzizp22rsuidbw@pz4e3fsqfhgv>

On Fri, Aug 09, 2024 at 02:02:21PM -0500, Justin Tobler wrote:

Show 9 quoted lines

> On 24/08/07 08:57AM, Patrick Steinhardt wrote:
> > When not provided a worktree, then `worktree_git_path()` will fall back
> > to returning a path relative to the main repository. In this case, we
> > implicitly rely on `the_repository` to derive the path. Remove this
> > dependency by passing a `struct repository` as parameter.
> 
> Are there many situations where `worktree_git_path()` would expect to
> not be provided a worktree? I wonder whether this implicit behavior is
> really necessary to begin with.

Yeah, there are cases. I found that to be somewhat weird at first, but I does make the logic easier to handle because you don't need to special case whether you do or don't have a worktree.

Show 12 quoted lines

> > diff --git a/path.h b/path.h
> > index 3d21b9cd16..6228ca03d7 100644
> > --- a/path.h
> > +++ b/path.h
> > @@ -97,9 +97,10 @@ const char *git_path(const char *fmt, ...)
> >   * Similar to git_path() but can produce paths for a specified
> >   * worktree instead of current one
> >   */
> 
> Now that the previously implicit behavior is more explicit, it might be
> update the comment to explain that the provided repository is used as a
> fallback.

Good idea.

Patrick