Re: [PATCH 01/20] path: expose `do_git_path()` as `repo_git_pathv()`

On Fri, Aug 09, 2024 at 11:58:31AM -0500, Justin Tobler wrote:

On 24/08/07 08:56AM, Patrick Steinhardt wrote:

quoted

We're about to move functions of the "path" subsytem that do not use a

s/subsytem/subsystem/

quoted

`struct repository` into "path.h" as static inlined functions. This will
require us to call `do_git_path()`, which is internal to "path.c".

So in other words, functions leveraging `the_repository` in "path.c" are
going to be moved to "path.h". Since these functions depend on
`do_git_path()`, we need to expose it. Makes sense so far.

Yup.

quoted

Expose the function as `repo_git_pathv()` to prepare for the change.

Signed-off-by: Patrick Steinhardt <redacted>

[snip]

quoted

+/*
+ * Print a path into the git directory of repository `repo` into the provided
+ * buffer.
+ */
+void repo_git_pathv(const struct repository *repo,
+		    const struct worktree *wt, struct strbuf *buf,
+		    const char *fmt, va_list args);
+

Out of curiousity, do we have a preferred convention for how functions
accepting `va_list` are named? Searching through the codebase, I don't
see a ton of consistency, but I have noticed examples prefixed with "v".

We of course have `strbuf_vaddf()` and the likes of `vsnprintf()` and
`xstrvmt()`, which indeed have the "v" as a prefix. But is
`repo_git_vpath()` better than `repo_git_pathv()` in this case here? I
dunno.

In any case, we already have `strbuf_git_common_pathv()`. So I'm leaning
more towards local consistency as opposed to global consistency. Even if
naming in our config interface feels like the wild west anyway.

Patrick