[PATCH v9 1/4] var: represent multi-valued variables with a string_list

[Andrew Pleeter <andrewpleeter@gmail.com>]

A variable whose value is a list, of which GIT_CONFIG_GLOBAL is
currently the only one, is read into a single string whose elements are
joined with newlines, and "git var -l" splits that string apart again
before showing each element.  Round-tripping through a delimiter that
can legitimately appear in a value is fragile, and it forces every
future caller to know which variables need splitting.

Give "struct git_var" a separate "multiread" function that appends the
elements to a string_list, and let the single "read" function stay for
the variables that have exactly one value.  A variable uses one or the
other, never both, so which one is set also says whether the variable
is multi-valued.

The output is unchanged.  While at it, say in the documentation that
GIT_CONFIG_GLOBAL can have multiple values, so that callers know when
to expect a list.

Signed-off-by: Andrew Pleeter <andrewpleeter@gmail.com>
---
 Documentation/git-var.adoc |  1 +
 builtin/var.c              | 79 +++++++++++++++++++++-----------------
 2 files changed, 44 insertions(+), 36 deletions(-)

diff --git a/Documentation/git-var.adoc b/Documentation/git-var.adoc
index 697c10aded..de3007732d 100644
--- a/Documentation/git-var.adoc
+++ b/Documentation/git-var.adoc
@@ -83,6 +83,7 @@ endif::git-default-pager[]
 
 `GIT_CONFIG_GLOBAL`::
     The path to the global (per-user) configuration files, if any.
+    This variable can have multiple values.
 
 Most path values contain only one value. However, some can contain multiple
 values, which are separated by newlines, and are listed in order from highest to
diff --git a/builtin/var.c b/builtin/var.c
index cc3a43cde2..9f7c8a6113 100644
--- a/builtin/var.c
+++ b/builtin/var.c
@@ -14,10 +14,11 @@
 #include "environment.h"
 #include "ident.h"
 #include "pager.h"
-#include "refs.h"
 #include "path.h"
-#include "strbuf.h"
+#include "refs.h"
 #include "run-command.h"
+#include "strbuf.h"
+#include "string-list.h"
 
 static const char var_usage[] = "git var (-l | <variable>)";
 
@@ -90,35 +91,27 @@ static char *git_config_val_system(int ident_flag UNUSED)
 	return NULL;
 }
 
-static char *git_config_val_global(int ident_flag UNUSED)
+static void git_config_val_global(struct string_list *list)
 {
-	struct strbuf buf = STRBUF_INIT;
 	char *user, *xdg;
-	size_t unused;
 
 	git_global_config_paths(&user, &xdg);
 	if (xdg && *xdg) {
 		normalize_path_copy(xdg, xdg);
-		strbuf_addf(&buf, "%s\n", xdg);
+		string_list_append(list, xdg);
 	}
 	if (user && *user) {
 		normalize_path_copy(user, user);
-		strbuf_addf(&buf, "%s\n", user);
+		string_list_append(list, user);
 	}
 	free(xdg);
 	free(user);
-	strbuf_trim_trailing_newline(&buf);
-	if (buf.len == 0) {
-		strbuf_release(&buf);
-		return NULL;
-	}
-	return strbuf_detach(&buf, &unused);
 }
 
 struct git_var {
 	const char *name;
 	char *(*read)(int);
-	int multivalued;
+	void (*multiread)(struct string_list *);
 };
 static struct git_var git_vars[] = {
 	{
@@ -163,8 +156,7 @@ static struct git_var git_vars[] = {
 	},
 	{
 		.name = "GIT_CONFIG_GLOBAL",
-		.read = git_config_val_global,
-		.multivalued = 1,
+		.multiread = git_config_val_global,
 	},
 	{
 		.name = "",
@@ -175,28 +167,30 @@ static struct git_var git_vars[] = {
 static void list_vars(void)
 {
 	struct git_var *ptr;
-	char *val;
-
-	for (ptr = git_vars; ptr->read; ptr++)
-		if ((val = ptr->read(0))) {
-			if (ptr->multivalued && *val) {
-				struct string_list list = STRING_LIST_INIT_DUP;
-
-				string_list_split(&list, val, "\n", -1);
-				for (size_t i = 0; i < list.nr; i++)
-					printf("%s=%s\n", ptr->name, list.items[i].string);
-				string_list_clear(&list, 0);
-			} else {
+
+	for (ptr = git_vars; ptr->read || ptr->multiread; ptr++) {
+		if (ptr->read) {
+			char *val = ptr->read(0);
+
+			if (val) {
 				printf("%s=%s\n", ptr->name, val);
+				free(val);
 			}
-			free(val);
+		} else {
+			struct string_list list = STRING_LIST_INIT_DUP;
+
+			ptr->multiread(&list);
+			for (size_t i = 0; i < list.nr; i++)
+				printf("%s=%s\n", ptr->name, list.items[i].string);
+			string_list_clear(&list, 0);
 		}
+	}
 }
 
 static const struct git_var *get_git_var(const char *var)
 {
 	struct git_var *ptr;
-	for (ptr = git_vars; ptr->read; ptr++) {
+	for (ptr = git_vars; ptr->read || ptr->multiread; ptr++) {
 		if (strcmp(var, ptr->name) == 0) {
 			return ptr;
 		}
@@ -220,7 +214,6 @@ int cmd_var(int argc,
 	    struct repository *repo UNUSED)
 {
 	const struct git_var *git_var;
-	char *val;
 
 	show_usage_if_asked(argc, argv, var_usage);
 	if (argc != 2)
@@ -237,12 +230,26 @@ int cmd_var(int argc,
 	if (!git_var)
 		usage(var_usage);
 
-	val = git_var->read(IDENT_STRICT);
-	if (!val)
-		return 1;
+	if (git_var->read) {
+		char *val = git_var->read(IDENT_STRICT);
+
+		if (!val)
+			return 1;
 
-	printf("%s\n", val);
-	free(val);
+		printf("%s\n", val);
+		free(val);
+	} else {
+		struct string_list list = STRING_LIST_INIT_DUP;
+
+		git_var->multiread(&list);
+		if (!list.nr) {
+			string_list_clear(&list, 0);
+			return 1;
+		}
+		for (size_t i = 0; i < list.nr; i++)
+			printf("%s\n", list.items[i].string);
+		string_list_clear(&list, 0);
+	}
 
 	return 0;
 }
-- 
2.54.0 (Apple Git-157)