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>

diff --git a/builtin/config.c b/builtin/config.c
index 01169dd628b24a7b5502550a6342ab73cb8154c5..92fb8d56b11e0be5d781f91aa1e5b1fcae7002a7 100644 (file)
--- 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 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;
 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 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")),
 
 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, "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")),
        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 (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_ : ""));
                        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");
                        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);
                        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);
                        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;
                        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 (!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
                /*
                 * 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);
                 * 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));
                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");
                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)
                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");
        }
 
                        return xstrdup(v ? "true" : "false");
        }
 

-       die("BUG: cannot normalize type %d", types);
+       die("BUG: cannot normalize type %d", type);

 }
 
 static int get_color_found;
 }
 
 static int get_color_found;


@@ -566,12 +566,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
                key_delim = '\n';
        }
 
                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);
        }
                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 4f8e6f5fde3295cc91d2feb1fd85fa007e56016c..24de37d5447c7def8a36213872b8bb369bb98458 100755 (executable)
--- 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
 '
 
        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
 test_done