Re: [PATCH 02/16] path: refactor `repo_git_path()` family of functions

From: Patrick Steinhardt <ps@pks.im>
Date: Feb 7, 2025, 06:15 UTC
Message-ID: <Z6WlCg8rP_JCKulR@pks.im>
In-Reply-To: <CAOLa=ZT8CrZA7Se98XBwS1CZjgvFFtKj5SBQfJNkG48QCvB=JQ@mail.gmail.com>

On Thu, Feb 06, 2025 at 03:53:59AM -0800, Karthik Nayak wrote:

Show 23 quoted lines

> Patrick Steinhardt <ps@pks.im> writes:
> 
> [snip]
> 
> > @@ -241,11 +228,14 @@ struct strbuf *get_pathname(void);
> >  #  include "strbuf.h"
> >  #  include "repository.h"
> >
> > -/* Internal implementation detail that should not be used. */
> > +/* Internal implementation details that should not be used. */
> >  void repo_common_pathv(const struct repository *repo,
> >  		       struct strbuf *buf,
> >  		       const char *fmt,
> >  		       va_list args);
> > +void repo_git_pathv(struct repository *repo,
> > +		    const struct worktree *wt, struct strbuf *buf,
> > +		    const char *fmt, va_list args);
> >
> 
> The only thing that stood out to me was that we loose the `const` here
> for `repository`, I couldn't really find a reason why we do that,
> especially since `repo_common_pathv()` right above still has it.
> Everything else looks good in this patch.

You're right, there is none yet over here, so it's pointless to adapt `repo_git_pathv()`. Later on this'll change because we're converting the "core.hooksPath" variable to be stored in `struct repo_settings`, and that'll require us to pass in a non-const pointer.

I'll shuffle this around a bit.

Patrick