diff mbox series

[v3,1/4] cache.h: rename "xdg_config_home" to "xdg_config_home_git"

Message ID 20210520221359.75615-2-lenaic@lhuard.fr (mailing list archive)
State New, archived
Headers show
Series maintenance: use systemd timers on Linux | expand

Commit Message

Lénaïc Huard May 20, 2021, 10:13 p.m. UTC 
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.

The previous functionality is now provided by a new
`xdg_config_home_git(…)` function whose implementation leverages
`xdg_config_home(…)`.

`xdg_cache_home(…)` has been renamed `xdg_cache_home_git(…)` for
consistency.

Signed-off-by: Lénaïc Huard <lenaic@lhuard.fr>
---
 attr.c                     |  2 +-
 builtin/config.c           |  2 +-
 builtin/credential-cache.c |  2 +-
 builtin/credential-store.c |  2 +-
 cache.h                    | 12 ++++++++++--
 config.c                   |  2 +-
 dir.c                      |  2 +-
 path.c                     | 35 +++++++++++++++++++++++++++--------
 sequencer.c                |  2 +-
 9 files changed, 44 insertions(+), 17 deletions(-)

Comments

Đoàn Trần Công Danh May 20, 2021, 11:44 p.m. UTC | #1 
On 2021-05-21 00:13:56+0200, Lénaïc Huard <lenaic@lhuard.fr> 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)

>  {
>  	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 ...

>  
> -	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.
diff mbox series

Patch

diff --git a/attr.c b/attr.c
index ac8ec7ce51..0efafbd308 100644
--- a/attr.c
+++ b/attr.c
@@ -831,7 +831,7 @@  static const char *git_etc_gitattributes(void)
 static const char *get_home_gitattributes(void)
 {
 	if (!git_attributes_file)
-		git_attributes_file = xdg_config_home("attributes");
+		git_attributes_file = xdg_config_home_git("attributes");
 
 	return git_attributes_file;
 }
diff --git a/builtin/config.c b/builtin/config.c
index f71fa39b38..8cf2394eb8 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -672,7 +672,7 @@  int cmd_config(int argc, const char **argv, const char *prefix)
 
 	if (use_global_config) {
 		char *user_config = expand_user_path("~/.gitconfig", 0);
-		char *xdg_config = xdg_config_home("config");
+		char *xdg_config = xdg_config_home_git("config");
 
 		if (!user_config)
 			/*
diff --git a/builtin/credential-cache.c b/builtin/credential-cache.c
index 76a6ba3722..4c0b7c4d43 100644
--- a/builtin/credential-cache.c
+++ b/builtin/credential-cache.c
@@ -94,7 +94,7 @@  static char *get_socket_path(void)
 	if (old_dir && !stat(old_dir, &sb) && S_ISDIR(sb.st_mode))
 		socket = xstrfmt("%s/socket", old_dir);
 	else
-		socket = xdg_cache_home("credential/socket");
+		socket = xdg_cache_home_git("credential/socket");
 	free(old_dir);
 	return socket;
 }
diff --git a/builtin/credential-store.c b/builtin/credential-store.c
index ae3c1ba75f..34ca419bb6 100644
--- a/builtin/credential-store.c
+++ b/builtin/credential-store.c
@@ -175,7 +175,7 @@  int cmd_credential_store(int argc, const char **argv, const char *prefix)
 	} else {
 		if ((file = expand_user_path("~/.git-credentials", 0)))
 			string_list_append_nodup(&fns, file);
-		file = xdg_config_home("credentials");
+		file = xdg_config_home_git("credentials");
 		if (file)
 			string_list_append_nodup(&fns, file);
 	}
diff --git a/cache.h b/cache.h
index 148d9ab5f1..34fa48a438 100644
--- a/cache.h
+++ b/cache.h
@@ -1263,19 +1263,27 @@  int is_ntfs_dotgitattributes(const char *name);
  */
 int looks_like_command_line_option(const char *str);
 
+/**
+ * Return a newly allocated string with the evaluation of
+ * "$XDG_CONFIG_HOME/$fmt..." if $XDG_CONFIG_HOME is non-empty, otherwise
+ * "$HOME/.config/$fmt...". Return NULL upon error.
+ */
+char *xdg_config_home(const char *fmt, ...)
+	__attribute__((format (printf, 1, 2)));
+
 /**
  * Return a newly allocated string with the evaluation of
  * "$XDG_CONFIG_HOME/git/$filename" if $XDG_CONFIG_HOME is non-empty, otherwise
  * "$HOME/.config/git/$filename". Return NULL upon error.
  */
-char *xdg_config_home(const char *filename);
+char *xdg_config_home_git(const char *filename);
 
 /**
  * Return a newly allocated string with the evaluation of
  * "$XDG_CACHE_HOME/git/$filename" if $XDG_CACHE_HOME is non-empty, otherwise
  * "$HOME/.cache/git/$filename". Return NULL upon error.
  */
-char *xdg_cache_home(const char *filename);
+char *xdg_cache_home_git(const char *filename);
 
 int git_open_cloexec(const char *name, int flags);
 #define git_open(name) git_open_cloexec(name, O_RDONLY)
diff --git a/config.c b/config.c
index 6428393a41..648ae9b918 100644
--- a/config.c
+++ b/config.c
@@ -1883,7 +1883,7 @@  static int do_git_config_sequence(const struct config_options *opts,
 				  config_fn_t fn, void *data)
 {
 	int ret = 0;
-	char *xdg_config = xdg_config_home("config");
+	char *xdg_config = xdg_config_home_git("config");
 	char *user_config = expand_user_path("~/.gitconfig", 0);
 	char *repo_config;
 	enum config_scope prev_parsing_scope = current_parsing_scope;
diff --git a/dir.c b/dir.c
index 3474e67e8f..3fdba7b6fe 100644
--- a/dir.c
+++ b/dir.c
@@ -2990,7 +2990,7 @@  void setup_standard_excludes(struct dir_struct *dir)
 
 	/* core.excludesfile defaulting to $XDG_CONFIG_HOME/git/ignore */
 	if (!excludes_file)
-		excludes_file = xdg_config_home("ignore");
+		excludes_file = xdg_config_home_git("ignore");
 	if (excludes_file && !access_or_warn(excludes_file, R_OK, 0))
 		add_patterns_from_file_1(dir, excludes_file,
 					 dir->untracked ? &dir->ss_excludes_file : NULL);
diff --git a/path.c b/path.c
index 7b385e5eb2..15e2143e9f 100644
--- a/path.c
+++ b/path.c
@@ -1498,22 +1498,41 @@  int looks_like_command_line_option(const char *str)
 	return str && str[0] == '-';
 }
 
-char *xdg_config_home(const char *filename)
+char *xdg_config_home(const char *fmt, ...)
 {
 	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);
 
-	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);
+	return out;
+}
+
+char *xdg_config_home_git(const char *filename)
+{
+	assert(filename);
+	return xdg_config_home("git/%s", filename);
 }
 
-char *xdg_cache_home(const char *filename)
+char *xdg_cache_home_git(const char *filename)
 {
 	const char *home, *cache_home;
 
diff --git a/sequencer.c b/sequencer.c
index fd183b5593..25f467e685 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1241,7 +1241,7 @@  N_("Your name and email address were configured automatically based\n"
 static const char *implicit_ident_advice(void)
 {
 	char *user_config = expand_user_path("~/.gitconfig", 0);
-	char *xdg_config = xdg_config_home("config");
+	char *xdg_config = xdg_config_home_git("config");
 	int config_exists = file_exists(user_config) || file_exists(xdg_config);
 
 	free(user_config);