Re: [PATCH 02/20] path: expose `do_git_common_path()` as `strbuf_git_common_pathv()`

[Patrick Steinhardt <ps@pks.im>]

On Fri, Aug 09, 2024 at 10:32:54AM -0700, Junio C Hamano wrote:
> Justin Tobler <jltobler@gmail.com> writes:
> 
> >> -static void do_git_common_path(const struct repository *repo,
> >> -			       struct strbuf *buf,
> >> -			       const char *fmt,
> >> -			       va_list args)
> >> +void strbuf_git_common_pathv(struct strbuf *sb,
> >> +			     const struct repository *repo,
> >> +			     const char *fmt,
> >> +			     va_list args)
> >
> > Here we reorder the arguments to make `strbuf` first. I assume we are do
> > this to align with the preexisting `strbuf_git_common_path()` and use
> > the "strbuf_" prefix in the function name.
> 
> I thought that we already established as a general guideline that
> "strbuf_" should be cleaned up so that functions that happen to use
> strbuf merely as a way to carry parameters into or results out of
> them but are not primarily about string manipulation are renamed out
> of the "strbuf_" namespace.
> 
>   https://lore.kernel.org/git/ZqiLA0bGYZfH1OWD@tanuki/
> 
> And this is about getting a path, which is communicated via a
> "struct strbuf", and not the standard "char *".  That is a prime
> example of a function that we do *not* want to stress strbuf-ness
> of the function.
> 
> > In the previous commit we used the "repo_" prefix for
> > `repo_git_pathv()`. Would it make sense to be consistent here? All these
> > functions are operating on the provided buffer, but for a given
> > repository. Not sure what would be most appropriate here.
> 
> Yes, if the function is about obtaining the path for a file in a
> given repository's metadata directory, and its association with
> "strbuf" is that it merely happens to use it instead of "char *",
> it should not be named as if "strbuf_" ness is the primary
> characteristics of the function.
> 
> strbuf_cleanup_path() should also be renamed for the same reason.

Agreed. I was doing it for consistency's sake in this case, but let's
rather not make the overall interface any weirder than it already is.

Patrick