builtin/config.c: treat type specifiers singularly

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> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Taylor Blau committed Apr 9, 2018 at 15:46 UTC 0a8950be5d5d8699b4c1f31116bdf896278db484
2 files changed +33 -27
builtin/config.c
+22 -27
@@ -25,7 +25,7 @@ static char term = '\n';
25
26 static int use_global_config, use_system_config, use_local_config;
27 static struct git_config_source given_config_source;
28 -static int actions, types;
28 +static int actions, type;
29 static int end_null;
30 static int respect_includes_opt = -1;
31 static struct config_options config_options;
@@ -55,11 +55,11 @@ static int show_origin;
55 #define PAGING_ACTIONS (ACTION_LIST | ACTION_GET_ALL | \
56 ACTION_GET_REGEXP | ACTION_GET_URLMATCH)
57
58 -#define TYPE_BOOL (1<<0)
59 -#define TYPE_INT (1<<1)
60 -#define TYPE_BOOL_OR_INT (1<<2)
61 -#define TYPE_PATH (1<<3)
62 -#define TYPE_EXPIRY_DATE (1<<4)
58 +#define TYPE_BOOL 1
59 +#define TYPE_INT 2
60 +#define TYPE_BOOL_OR_INT 3
61 +#define TYPE_PATH 4
62 +#define TYPE_EXPIRY_DATE 5
63
64 static struct option builtin_config_options[] = {
65 OPT_GROUP(N_("Config file location")),
@@ -84,11 +84,11 @@ static struct option builtin_config_options[] = {
84 OPT_BIT(0, "get-color", &actions, N_("find the color configured: slot [default]"), ACTION_GET_COLOR),
85 OPT_BIT(0, "get-colorbool", &actions, N_("find the color setting: slot [stdout-is-tty]"), ACTION_GET_COLORBOOL),
86 OPT_GROUP(N_("Type")),
87 - OPT_BIT(0, "bool", &types, N_("value is \"true\" or \"false\""), TYPE_BOOL),
88 - OPT_BIT(0, "int", &types, N_("value is decimal number"), TYPE_INT),
89 - OPT_BIT(0, "bool-or-int", &types, N_("value is --bool or --int"), TYPE_BOOL_OR_INT),
90 - OPT_BIT(0, "path", &types, N_("value is a path (file or directory name)"), TYPE_PATH),
91 - OPT_BIT(0, "expiry-date", &types, N_("value is an expiry date"), TYPE_EXPIRY_DATE),
87 + OPT_SET_INT(0, "bool", &type, N_("value is \"true\" or \"false\""), TYPE_BOOL),
88 + OPT_SET_INT(0, "int", &type, N_("value is decimal number"), TYPE_INT),
89 + OPT_SET_INT(0, "bool-or-int", &type, N_("value is --bool or --int"), TYPE_BOOL_OR_INT),
90 + OPT_SET_INT(0, "path", &type, N_("value is a path (file or directory name)"), TYPE_PATH),
91 + OPT_SET_INT(0, "expiry-date", &type, N_("value is an expiry date"), TYPE_EXPIRY_DATE),
92 OPT_GROUP(N_("Other")),
93 OPT_BOOL('z', "null", &end_null, N_("terminate values with NUL byte")),
94 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
149 if (show_keys)
150 strbuf_addch(buf, key_delim);
151
152 - if (types == TYPE_INT)
152 + if (type == TYPE_INT)
153 strbuf_addf(buf, "%"PRId64,
154 git_config_int64(key_, value_ ? value_ : ""));
155 - else if (types == TYPE_BOOL)
155 + else if (type == TYPE_BOOL)
156 strbuf_addstr(buf, git_config_bool(key_, value_) ?
157 "true" : "false");
158 - else if (types == TYPE_BOOL_OR_INT) {
158 + else if (type == TYPE_BOOL_OR_INT) {
159 int is_bool, v;
160 v = git_config_bool_or_int(key_, value_, &is_bool);
161 if (is_bool)
162 strbuf_addstr(buf, v ? "true" : "false");
163 else
164 strbuf_addf(buf, "%d", v);
165 - } else if (types == TYPE_PATH) {
165 + } else if (type == TYPE_PATH) {
166 const char *v;
167 if (git_config_pathname(&v, key_, value_) < 0)
168 return -1;
169 strbuf_addstr(buf, v);
170 free((char *)v);
171 - } else if (types == TYPE_EXPIRY_DATE) {
171 + } else if (type == TYPE_EXPIRY_DATE) {
172 timestamp_t t;
173 if (git_config_expiry_date(&t, key_, value_) < 0)
174 return -1;
@@ -287,7 +287,7 @@ static char *normalize_value(const char *key, const char *value)
287 if (!value)
288 return NULL;
289
290 - if (types == 0 || types == TYPE_PATH || types == TYPE_EXPIRY_DATE)
290 + if (type == 0 || type == TYPE_PATH || type == TYPE_EXPIRY_DATE)
291 /*
292 * We don't do normalization for TYPE_PATH here: If
293 * the path is like ~/foobar/, we prefer to store
@@ -296,11 +296,11 @@ static char *normalize_value(const char *key, const char *value)
296 * Also don't do normalization for expiry dates.
297 */
298 return xstrdup(value);
299 - if (types == TYPE_INT)
299 + if (type == TYPE_INT)
300 return xstrfmt("%"PRId64, git_config_int64(key, value));
301 - if (types == TYPE_BOOL)
301 + if (type == TYPE_BOOL)
302 return xstrdup(git_config_bool(key, value) ? "true" : "false");
303 - if (types == TYPE_BOOL_OR_INT) {
303 + if (type == TYPE_BOOL_OR_INT) {
304 int is_bool, v;
305 v = git_config_bool_or_int(key, value, &is_bool);
306 if (!is_bool)
@@ -309,7 +309,7 @@ static char *normalize_value(const char *key, const char *value)
309 return xstrdup(v ? "true" : "false");
310 }
311
312 - die("BUG: cannot normalize type %d", types);
312 + die("BUG: cannot normalize type %d", type);
313 }
314
315 static int get_color_found;
@@ -566,12 +566,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
566 key_delim = '\n';
567 }
568
569 - if (HAS_MULTI_BITS(types)) {
570 - error("only one type at a time.");
571 - usage_with_options(builtin_config_usage, builtin_config_options);
572 - }
573 -
574 - if ((actions & (ACTION_GET_COLOR|ACTION_GET_COLORBOOL)) && types) {
569 + if ((actions & (ACTION_GET_COLOR|ACTION_GET_COLORBOOL)) && type) {
570 error("--get-color and variable type are incoherent");
571 usage_with_options(builtin_config_usage, builtin_config_options);
572 }
t/t1300-repo-config.sh
+11
@@ -1611,4 +1611,15 @@ test_expect_success '--local requires a repo' '
1611 test_expect_code 128 nongit git config --local foo.bar
1612 '
1613
1614 +cat >.git/config <<-\EOF &&
1615 +[core]
1616 +number = 10
1617 +EOF
1618 +
1619 +test_expect_success 'later legacy specifiers are given precedence' '
1620 + git config --bool --int core.number >actual &&
1621 + echo 10 >expect &&
1622 + test_cmp expect actual
1623 +'
1624 +
1625 test_done