| Message ID | 7ce3278f649ce70453242e5458d28c5fd54576ba.1723013714.git.ps@pks.im (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Stop using `the_repository` in "config.c" | expand |
On 24/08/07 08:56AM, Patrick Steinhardt wrote: > We're about to move functions of the "path" subsytem that do not use a s/subsytem/subsystem/ > `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. > Expose the function as `repo_git_pathv()` to prepare for the change. > > Signed-off-by: Patrick Steinhardt <ps@pks.im> [snip] > +/* > + * 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".
On Fri, Aug 09, 2024 at 11:58:31AM -0500, Justin Tobler wrote: > On 24/08/07 08:56AM, Patrick Steinhardt wrote: > > We're about to move functions of the "path" subsytem that do not use a > > s/subsytem/subsystem/ > > > `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. > > Expose the function as `repo_git_pathv()` to prepare for the change. > > > > Signed-off-by: Patrick Steinhardt <ps@pks.im> > [snip] > > +/* > > + * 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
diff --git a/path.c b/path.c index 19f7684f38..71f1cb4dfb 100644 --- a/path.c +++ b/path.c @@ -417,9 +417,9 @@ static void strbuf_worktree_gitdir(struct strbuf *buf, strbuf_git_common_path(buf, repo, "worktrees/%s", wt->id); } -static void do_git_path(const struct repository *repo, - const struct worktree *wt, struct strbuf *buf, - const char *fmt, va_list args) +void repo_git_pathv(const struct repository *repo, + const struct worktree *wt, struct strbuf *buf, + const char *fmt, va_list args) { int gitdir_len; strbuf_worktree_gitdir(buf, repo, wt); @@ -438,7 +438,7 @@ char *repo_git_path(const struct repository *repo, struct strbuf path = STRBUF_INIT; va_list args; va_start(args, fmt); - do_git_path(repo, NULL, &path, fmt, args); + repo_git_pathv(repo, NULL, &path, fmt, args); va_end(args); return strbuf_detach(&path, NULL); } @@ -449,7 +449,7 @@ void strbuf_repo_git_path(struct strbuf *sb, { va_list args; va_start(args, fmt); - do_git_path(repo, NULL, sb, fmt, args); + repo_git_pathv(repo, NULL, sb, fmt, args); va_end(args); } @@ -458,7 +458,7 @@ char *git_path_buf(struct strbuf *buf, const char *fmt, ...) va_list args; strbuf_reset(buf); va_start(args, fmt); - do_git_path(the_repository, NULL, buf, fmt, args); + repo_git_pathv(the_repository, NULL, buf, fmt, args); va_end(args); return buf->buf; } @@ -467,7 +467,7 @@ void strbuf_git_path(struct strbuf *sb, const char *fmt, ...) { va_list args; va_start(args, fmt); - do_git_path(the_repository, NULL, sb, fmt, args); + repo_git_pathv(the_repository, NULL, sb, fmt, args); va_end(args); } @@ -476,7 +476,7 @@ const char *git_path(const char *fmt, ...) struct strbuf *pathname = get_pathname(); va_list args; va_start(args, fmt); - do_git_path(the_repository, NULL, pathname, fmt, args); + repo_git_pathv(the_repository, NULL, pathname, fmt, args); va_end(args); return pathname->buf; } @@ -486,7 +486,7 @@ char *git_pathdup(const char *fmt, ...) struct strbuf path = STRBUF_INIT; va_list args; va_start(args, fmt); - do_git_path(the_repository, NULL, &path, fmt, args); + repo_git_pathv(the_repository, NULL, &path, fmt, args); va_end(args); return strbuf_detach(&path, NULL); } @@ -517,7 +517,7 @@ const char *worktree_git_path(const struct worktree *wt, const char *fmt, ...) struct strbuf *pathname = get_pathname(); va_list args; va_start(args, fmt); - do_git_path(the_repository, wt, pathname, fmt, args); + repo_git_pathv(the_repository, wt, pathname, fmt, args); va_end(args); return pathname->buf; } diff --git a/path.h b/path.h index a6f0b70692..94e7030f0b 100644 --- a/path.h +++ b/path.h @@ -66,6 +66,14 @@ char *repo_git_path(const struct repository *repo, const char *fmt, ...) __attribute__((format (printf, 2, 3))); +/* + * 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); + /* * Construct a path into the git directory of repository `repo` and append it * to the provided buffer `sb`.
We're about to move functions of the "path" subsytem that do not use a `struct repository` into "path.h" as static inlined functions. This will require us to call `do_git_path()`, which is internal to "path.c". Expose the function as `repo_git_pathv()` to prepare for the change. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- path.c | 20 ++++++++++---------- path.h | 8 ++++++++ 2 files changed, 18 insertions(+), 10 deletions(-)