git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v8 1/2] builtin/config.c: treat type specifiers singularly

From
Taylor Blau <me@ttaylorr.com>
Date
Apr 11, 2018, 01:06 UTC
Message-ID
<20180411010658.GB28561@syl.local>
In-Reply-To
<cover.1523408336.git.me@ttaylorr.com>

Internally, we represent `git config`'s type specifiers as a bitset using OPT_BIT. 'bool' is 1<<0, 'int' is 1<<1, and so on. This technique allows for the representation of multiple type specifiers in the `int types` field, but this multi-representation is left unused.

In fact, `git config` will not accept multiple type specifiers at a time, as indicated by:

  $ git config --int --bool some.section
  error: only one type at a time.

This patch uses `OPT_SET_INT` to prefer the _last_ mentioned type specifier, so that the above command would instead be valid, and a synonym of:

  $ git config --bool some.section

This change is motivated by two urges: (1) it does not make sense to represent a singular type specifier internally as a bitset, only to complain when there are multiple bits in the set. `OPT_SET_INT` is more well-suited to this task than `OPT_BIT` is. (2) a future patch will introduce `--type=<type>`, and we would like not to complain in the following situation:

  $ git config --int --type=int
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 builtin/config.c       | 49 +++++++++++++++++++-----------------------
 t/t1300-repo-config.sh | 11 ++++++++++
 2 files changed, 33 insertions(+), 27 deletions(-)
diff --git a/builtin/config.c b/builtin/config.c
index 01169dd628..92fb8d56b1 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -25,7 +25,7 @@ static char term = '\n';
 
 static int use_global_config, use_system_config, use_local_config;
 static struct git_config_source given_config_source;
-static int actions, types;
+static int actions, type;
 static int end_null;
 static int respect_includes_opt = -1;
 static struct config_options config_options;
@@ -55,11 +55,11 @@ static int show_origin;
 #define PAGING_ACTIONS (ACTION_LIST | ACTION_GET_ALL | \
 			ACTION_GET_REGEXP | ACTION_GET_URLMATCH)
 
-#define TYPE_BOOL (1<<0)
-#define TYPE_INT (1<<1)
-#define TYPE_BOOL_OR_INT (1<<2)
-#define TYPE_PATH (1<<3)
-#define TYPE_EXPIRY_DATE (1<<4)
+#define TYPE_BOOL		1
+#define TYPE_INT		2
+#define TYPE_BOOL_OR_INT	3
+#define TYPE_PATH		4
+#define TYPE_EXPIRY_DATE	5
 
 static struct option builtin_config_options[] = {
 	OPT_GROUP(N_("Config file location")),
@@ -84,11 +84,11 @@ static struct option builtin_config_options[] = {
 	OPT_BIT(0, "get-color", &actions, N_("find the color configured: slot [default]"), ACTION_GET_COLOR),
 	OPT_BIT(0, "get-colorbool", &actions, N_("find the color setting: slot [stdout-is-tty]"), ACTION_GET_COLORBOOL),
 	OPT_GROUP(N_("Type")),
-	OPT_BIT(0, "bool", &types, N_("value is \"true\" or \"false\""), TYPE_BOOL),
-	OPT_BIT(0, "int", &types, N_("value is decimal number"), TYPE_INT),
-	OPT_BIT(0, "bool-or-int", &types, N_("value is --bool or --int"), TYPE_BOOL_OR_INT),
-	OPT_BIT(0, "path", &types, N_("value is a path (file or directory name)"), TYPE_PATH),
-	OPT_BIT(0, "expiry-date", &types, N_("value is an expiry date"), TYPE_EXPIRY_DATE),
+	OPT_SET_INT(0, "bool", &type, N_("value is \"true\" or \"false\""), TYPE_BOOL),
+	OPT_SET_INT(0, "int", &type, N_("value is decimal number"), TYPE_INT),
+	OPT_SET_INT(0, "bool-or-int", &type, N_("value is --bool or --int"), TYPE_BOOL_OR_INT),
+	OPT_SET_INT(0, "path", &type, N_("value is a path (file or directory name)"), TYPE_PATH),
+	OPT_SET_INT(0, "expiry-date", &type, N_("value is an expiry date"), TYPE_EXPIRY_DATE),
 	OPT_GROUP(N_("Other")),
 	OPT_BOOL('z', "null", &end_null, N_("terminate values with NUL byte")),
 	OPT_BOOL(0, "name-only", &omit_values, N_("show variable names only")),
@@ -149,26 +149,26 @@ static int format_config(struct strbuf *buf, const char *key_, const char *value
 		if (show_keys)
 			strbuf_addch(buf, key_delim);
 
-		if (types == TYPE_INT)
+		if (type == TYPE_INT)
 			strbuf_addf(buf, "%"PRId64,
 				    git_config_int64(key_, value_ ? value_ : ""));
-		else if (types == TYPE_BOOL)
+		else if (type == TYPE_BOOL)
 			strbuf_addstr(buf, git_config_bool(key_, value_) ?
 				      "true" : "false");
-		else if (types == TYPE_BOOL_OR_INT) {
+		else if (type == TYPE_BOOL_OR_INT) {
 			int is_bool, v;
 			v = git_config_bool_or_int(key_, value_, &is_bool);
 			if (is_bool)
 				strbuf_addstr(buf, v ? "true" : "false");
 			else
 				strbuf_addf(buf, "%d", v);
-		} else if (types == TYPE_PATH) {
+		} else if (type == TYPE_PATH) {
 			const char *v;
 			if (git_config_pathname(&v, key_, value_) < 0)
 				return -1;
 			strbuf_addstr(buf, v);
 			free((char *)v);
-		} else if (types == TYPE_EXPIRY_DATE) {
+		} else if (type == TYPE_EXPIRY_DATE) {
 			timestamp_t t;
 			if (git_config_expiry_date(&t, key_, value_) < 0)
 				return -1;
@@ -287,7 +287,7 @@ static char *normalize_value(const char *key, const char *value)
 	if (!value)
 		return NULL;
 
-	if (types == 0 || types == TYPE_PATH || types == TYPE_EXPIRY_DATE)
+	if (type == 0 || type == TYPE_PATH || type == TYPE_EXPIRY_DATE)
 		/*
 		 * We don't do normalization for TYPE_PATH here: If
 		 * the path is like ~/foobar/, we prefer to store
@@ -296,11 +296,11 @@ static char *normalize_value(const char *key, const char *value)
 		 * Also don't do normalization for expiry dates.
 		 */
 		return xstrdup(value);
-	if (types == TYPE_INT)
+	if (type == TYPE_INT)
 		return xstrfmt("%"PRId64, git_config_int64(key, value));
-	if (types == TYPE_BOOL)
+	if (type == TYPE_BOOL)
 		return xstrdup(git_config_bool(key, value) ?  "true" : "false");
-	if (types == TYPE_BOOL_OR_INT) {
+	if (type == TYPE_BOOL_OR_INT) {
 		int is_bool, v;
 		v = git_config_bool_or_int(key, value, &is_bool);
 		if (!is_bool)
@@ -309,7 +309,7 @@ static char *normalize_value(const char *key, const char *value)
 			return xstrdup(v ? "true" : "false");
 	}
 
-	die("BUG: cannot normalize type %d", types);
+	die("BUG: cannot normalize type %d", type);
 }
 
 static int get_color_found;
@@ -566,12 +566,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		key_delim = '\n';
 	}
 
-	if (HAS_MULTI_BITS(types)) {
-		error("only one type at a time.");
-		usage_with_options(builtin_config_usage, builtin_config_options);
-	}
-
-	if ((actions & (ACTION_GET_COLOR|ACTION_GET_COLORBOOL)) && types) {
+	if ((actions & (ACTION_GET_COLOR|ACTION_GET_COLORBOOL)) && type) {
 		error("--get-color and variable type are incoherent");
 		usage_with_options(builtin_config_usage, builtin_config_options);
 	}
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 4f8e6f5fde..24de37d544 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -1611,4 +1611,15 @@ test_expect_success '--local requires a repo' '
 	test_expect_code 128 nongit git config --local foo.bar
 '
 
+cat >.git/config <<-\EOF &&
+[core]
+number = 10
+EOF
+
+test_expect_success 'later legacy specifiers are given precedence' '
+	git config --bool --int core.number >actual &&
+	echo 10 >expect &&
+	test_cmp expect actual
+'
+
 test_done

-- 
2.17.0
Previous: Taylor BlauNext: Taylor Blau
Message 55 of 60 in “builtin/config.c: prefer `--type=bool` over `--bool`, etc.”
  1. builtin/config.c: prefer `--type=bool` over `--bool`, etc.Taylor Blau, Mar 28, 2018
  2. Junio C HamanoMar 29, 2018
  3. Jeff KingMar 29, 2018
  4. Taylor BlauMar 30, 2018
  5. Jeff KingMar 30, 2018
  6. Junio C HamanoMar 30, 2018
  7. Jeff KingMar 30, 2018
  8. 1/2 builtin/config.c: treat type specifiers singularlyTaylor Blau, Mar 30, 2018
  9. 2/2 builtin/config.c: prefer `--type=bool` over `--bool`, etc.Taylor Blau, Mar 30, 2018
  10. René ScharfeMar 30, 2018
  11. Jeff KingMar 30, 2018
  12. Jeff KingMar 30, 2018
  13. 0/2 builtin/config.c: prefer `--type=bool` over `--bool`, etc.Taylor Blau, Apr 4, 2018
  14. 1/2 builtin/config.c: treat type specifiers singularlyTaylor Blau, Apr 4, 2018
  15. Eric SunshineApr 4, 2018
  16. Taylor BlauApr 5, 2018
  17. Jeff KingApr 5, 2018
  18. 2/2 builtin/config.c: prefer `--type=bool` over `--bool`, etc.Taylor Blau, Apr 4, 2018
  19. Eric SunshineApr 4, 2018
  20. Taylor BlauApr 5, 2018
  21. 0/2 builtin/config.c: prefer `--type=bool` over `--bool`, etc.Taylor Blau, Apr 5, 2018
  22. Jeff KingApr 5, 2018
  23. Taylor BlauApr 5, 2018
  24. 0/2 builtin/config.c: support `--type=<type>` as preferred alias for `--type`Taylor Blau, Apr 6, 2018
  25. 0/2 builtin/config.c: support `--type=<type>` as preferred alias for `--type`Taylor Blau, Apr 9, 2018
  26. Eric SunshineApr 9, 2018
  27. 0/2 builtin/config.c: support `--type=<type>` as preferred alias for `--type`Taylor Blau, Apr 11, 2018
  28. Junio C HamanoApr 11, 2018
  29. Taylor BlauApr 11, 2018
  30. Junio C HamanoApr 11, 2018
  31. Taylor BlauApr 11, 2018
  32. 0/2 builtin/config.c: support `--type=<type>` as preferred alias for `--type`Taylor Blau, Apr 18, 2018
  33. 1/2 builtin/config.c: treat type specifiers singularlyTaylor Blau, Apr 5, 2018
  34. 2/2 builtin/config.c: prefer `--type=bool` over `--bool`, etc.Taylor Blau, Apr 5, 2018
  35. Jeff KingApr 5, 2018
  36. Taylor BlauApr 5, 2018
  37. Taylor BlauApr 6, 2018
  38. 2/2 builtin/config.c: prefer `--type=bool` over `--bool`, etc.Taylor Blau, Apr 5, 2018
  39. Eric SunshineApr 5, 2018
  40. Jeff KingApr 5, 2018
  41. Taylor BlauApr 6, 2018
  42. Taylor BlauApr 6, 2018
  43. 1/2 builtin/config.c: treat type specifiers singularlyTaylor Blau, Apr 6, 2018
  44. 2/2 builtin/config.c: support `--type=<type>` as preferred alias for `--type`Taylor Blau, Apr 6, 2018
  45. Eric SunshineApr 6, 2018
  46. Taylor BlauApr 7, 2018
  47. Eric SunshineApr 7, 2018
  48. 1/2 builtin/config.c: treat type specifiers singularlyTaylor Blau, Apr 9, 2018
  49. Junio C HamanoApr 10, 2018
  50. Taylor BlauApr 10, 2018
  51. Eric SunshineApr 10, 2018
  52. 2/2 builtin/config.c: support `--type=<type>` as preferred alias for `--type`Taylor Blau, Apr 9, 2018
  53. Junio C HamanoApr 10, 2018
  54. Taylor BlauApr 10, 2018
  55. 1/2 builtin/config.c: treat type specifiers singularlyTaylor Blau, Apr 11, 2018
  56. 2/2 builtin/config.c: support `--type=<type>` as preferred alias for `--type`Taylor Blau, Apr 11, 2018
  57. 1/2 builtin/config.c: treat type specifiers singularlyTaylor Blau, Apr 18, 2018
  58. 2/2 builtin/config.c: support `--type=<type>` as preferred alias for `--type`Taylor Blau, Apr 18, 2018
  59. Junio C HamanoApr 19, 2018
  60. Taylor BlauApr 19, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.