Re: [PATCH v3 1/4] cache.h: rename "xdg_config_home" to "xdg_config_home_git"
- From
Đoàn Trần Công Danh <congdanhqx@gmail.com>
- Date
- May 20, 2021, 23:44 UTC
- Message-ID
- <YKb0WImFLYpluHrQ@danh.dev>
- In-Reply-To
- <20210520221359.75615-2-lenaic@lhuard.fr>
On 2021-05-21 00:13:56+0200, Lénaïc Huard <lenaic@lhuard.fr> wrote:
Show 8 quoted lines
> Current implementation of `xdg_config_home(str)` returns > `$XDG_CONFIG_HOME/git/$str`, with the `git` subdirectory inserted > between the `XDG_CONFIG_HOME` environment variable and the parameter. > > This patch re-purposes `xdg_config_home(…)` to be more generic. It now > only concatenates "$XDG_CONFIG_HOME", or "$HOME/.config" if the former > isn’t defined, with the parameter, without adding `git` in between. > Its parameter is now a format string.
Intended or not, this change is going to make a logical conflict, should other topics also call to xdg_config_home, i.e. no textual conflicts, programs compiled successfully but run into failure.
I think we shouldn't re-purpose xdg_config_home, we should add a new function named something like xdg_config_home_{for,nongit,other,generic} instead.
> -char *xdg_config_home(const char *filename) > +char *xdg_config_home(const char *fmt, ...)
In my opinion, we don't even need to over-engineer this function with variadic arguments, I think below function should be enough for most (all?) cases:
char *xdg_config_home_prog(const char *prog, const char *filename)
Show 9 quoted lines
> {
> const char *home, *config_home;
> + struct strbuf buf = STRBUF_INIT;
> + char *out = NULL;
> + va_list args;
> +
> + va_start(args, fmt);
> + strbuf_vaddf(&buf, fmt, args);
> + va_end(args);If my imagination is sensible, it's not necessary to use a temporary strbuf and strbuf_vaddf here ...
Show 21 quoted lines
>
> - assert(filename);
> config_home = getenv("XDG_CONFIG_HOME");
> - if (config_home && *config_home)
> - return mkpathdup("%s/git/%s", config_home, filename);
> + if (config_home && *config_home) {
> + out = mkpathdup("%s/%s", config_home, buf.buf);
> + goto done;
> + }
>
> home = getenv("HOME");
> - if (home)
> - return mkpathdup("%s/.config/git/%s", home, filename);
> - return NULL;
> + if (home) {
> + out = mkpathdup("%s/.config/%s", home, buf.buf);
> + goto done;
> + }
> +
> +done:
> + strbuf_release(&buf);... and go though the restructure of this function.
-- Danh