Re: [PATCH v3 1/4] cache.h: rename "xdg_config_home" to "xdg_config_home_git"
flat view
From: Đoàn Trần Công Danh <hidden>
Date: 2021-05-20 23:44:31
On 2021-05-21 00:13:56+0200, Lénaïc Huard [off-list ref] wrote:
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)
quoted hunk ↗ jump to hunk
{ 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 ...
quoted hunk ↗ jump to hunk
- 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