config: make 'git config list --type=<X>' work

Previously, the --type=<X> argument to 'git config list' was ignored and
did nothing. Now, we add the use of format_config() to the
show_all_config() function so each key-value pair is attempted to be
parsed. This is our first use of the 'gently' parameter with a nonzero
value.

When listing multiple values, our initial settings for the output format
is different. Add a new init helper to specify the fact that keys should
be shown and also add the default delimiters as they were unset in some
cases.

Our intention is that if there is an error in parsing, then the row is
not output. This is necessary to avoid the caller needing to build their
own validator to understand the difference between valid, canonicalized
types and other raw string values. The raw values will always be
available to the user if they do not specify the --type=<X> option.

The current behavior is more complicated, including error messages on
bad parsing or potentially complete failure of the command.  We add
tests at this point that demonstrate the current behavior so we can
witness the fix in future changes that parse these values quietly and
gently.

This is a change in behavior! We are starting to respect an option that
was previously ignored, leading to potential user confusion. This is
probably still a good option, since the --type argument did not change
behavior at all previously, so users can get the behavior they expect by
removing the --type argument or adding the --no-type argument.

t1300-config.sh is updated with the current behavior of this formatting
logic to justify the upcoming refactoring of format_config() that will
incrementally fix some of these cases to be more user-friendly.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/git-config.adoc b/Documentation/git-config.adoc
index ac3b536a155c6eb766a4847d3245d88370bbf551..5300dd4c51257acb981fabd935ed41f80e7bd0fd 100644 (file)
--- a/Documentation/git-config.adoc
+++ b/Documentation/git-config.adoc
@@ -240,6 +240,9 @@ Valid `<type>`'s include:
   that the given value is canonicalize-able as an ANSI color, but it is written
   as-is.
 +
+If the command is in `list` mode, then the `--type <type>` argument will apply
+to each listed config value. If the value does not successfully parse in that
+format, then it will be omitted from the list.
 
 --bool::
 --int::

diff --git a/builtin/config.c b/builtin/config.c
index b4c4228311dde15d48e83bfcd9319e37f4397ec3..4c4c79188382a36b8f4a5d66e6eae791dedd86d6 100644 (file)
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -318,21 +318,12 @@ static int show_all_config(const char *key_, const char *value_,
 {
        const struct config_display_options *opts = cb;
        const struct key_value_info *kvi = ctx->kvi;
+       struct strbuf formatted = STRBUF_INIT;
 
-       if (opts->show_origin || opts->show_scope) {
-               struct strbuf buf = STRBUF_INIT;
-               if (opts->show_scope)
-                       show_config_scope(opts, kvi, &buf);
-               if (opts->show_origin)
-                       show_config_origin(opts, kvi, &buf);
-               /* Use fwrite as "buf" can contain \0's if "end_null" is set. */
-               fwrite(buf.buf, 1, buf.len, stdout);
-               strbuf_release(&buf);
-       }
-       if (!opts->omit_values && value_)
-               printf("%s%c%s%c", key_, opts->delim, value_, opts->term);
-       else
-               printf("%s%c", key_, opts->term);
+       if (format_config(opts, &formatted, key_, value_, kvi, 1) >= 0)
+               fwrite(formatted.buf, 1, formatted.len, stdout);
+
+       strbuf_release(&formatted);
        return 0;
 }
 
@@ -872,6 +863,19 @@ static void display_options_init(struct config_display_options *opts)
        }
 }
 
+static void display_options_init_list(struct config_display_options *opts)
+{
+       opts->show_keys = 1;
+
+       if (opts->end_nul) {
+               display_options_init(opts);
+       } else {
+               opts->term = '\n';
+               opts->delim = ' ';
+               opts->key_delim = '=';
+       }
+}
+
 static int cmd_config_list(int argc, const char **argv, const char *prefix,
                           struct repository *repo UNUSED)
 {
@@ -890,7 +894,7 @@ static int cmd_config_list(int argc, const char **argv, const char *prefix,
        check_argc(argc, 0, 0);
 
        location_options_init(&location_opts, prefix);
-       display_options_init(&display_opts);
+       display_options_init_list(&display_opts);
 
        setup_auto_pager("config", 1);
 
@@ -1321,6 +1325,7 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix)
 
        if (actions == ACTION_LIST) {
                check_argc(argc, 0, 0);
+               display_options_init_list(&display_opts);
                if (config_with_options(show_all_config, &display_opts,
                                        &location_opts.source, the_repository,
                                        &location_opts.options) < 0) {

diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 9850fcd5b567a51c5f4edb6c7550597de3b62dcb..dc744c0baef10c58ace7f7d44a3ca9f3daec2cab 100755 (executable)
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -2459,9 +2459,15 @@ done
 
 cat >.git/config <<-\EOF &&
 [section]
-foo = true
+foo = True
 number = 10
 big = 1M
+path = ~/dir
+red = red
+blue = Blue
+date = Fri Jun 4 15:46:55 2010
+missing=:(optional)no-such-path
+exists=:(optional)expect
 EOF
 
 test_expect_success 'identical modern --type specifiers are allowed' '
@@ -2503,6 +2509,95 @@ test_expect_success 'unset type specifiers may be reset to conflicting ones' '
        test_cmp_config 1048576 --type=bool --no-type --type=int section.big
 '
 
+test_expect_success 'list --type=int shows only canonicalizable int values' '
+       cat >expect <<-EOF &&
+       section.number=10
+       section.big=1048576
+       EOF
+
+       test_must_fail git config ${mode_prefix}list --type=int
+'
+
+test_expect_success 'list --type=bool shows only canonicalizable bool values' '
+       cat >expect <<-EOF &&
+       section.foo=true
+       section.number=true
+       section.big=true
+       EOF
+
+       test_must_fail git config ${mode_prefix}list --type=bool
+'
+
+test_expect_success 'list --type=bool-or-int shows only canonicalizable values' '
+       cat >expect <<-EOF &&
+       section.foo=true
+       section.number=10
+       section.big=1048576
+       EOF
+
+       test_must_fail git config ${mode_prefix}list --type=bool-or-int
+'
+
+test_expect_success 'list --type=path shows only canonicalizable path values' '
+       # TODO: handling of missing path is incorrect here.
+       cat >expect <<-EOF &&
+       section.foo=True
+       section.number=10
+       section.big=1M
+       section.path=$HOME/dir
+       section.red=red
+       section.blue=Blue
+       section.date=Fri Jun 4 15:46:55 2010
+       section.missing=section.exists=expect
+       EOF
+
+       git config ${mode_prefix}list --type=path >actual 2>err &&
+       test_cmp expect actual &&
+       test_must_be_empty err
+'
+
+test_expect_success 'list --type=expiry-date shows only canonicalizable dates' '
+       cat >expecterr <<-EOF &&
+       error: '\''True'\'' for '\''section.foo'\'' is not a valid timestamp
+       error: '\''~/dir'\'' for '\''section.path'\'' is not a valid timestamp
+       error: '\''red'\'' for '\''section.red'\'' is not a valid timestamp
+       error: '\''Blue'\'' for '\''section.blue'\'' is not a valid timestamp
+       error: '\'':(optional)no-such-path'\'' for '\''section.missing'\'' is not a valid timestamp
+       error: '\'':(optional)expect'\'' for '\''section.exists'\'' is not a valid timestamp
+       EOF
+
+       git config ${mode_prefix}list --type=expiry-date >actual 2>err &&
+
+       # section.number and section.big parse as relative dates that could
+       # have clock skew in their results.
+       test_grep section.big actual &&
+       test_grep section.number actual &&
+       test_grep "section.date=$(git config --type=expiry-date section.$key)" actual &&
+       test_cmp expecterr err
+'
+
+test_expect_success 'list --type=color shows only canonicalizable color values' '
+       cat >expect <<-EOF &&
+       section.number=<>
+       section.red=<RED>
+       section.blue=<BLUE>
+       EOF
+
+       cat >expecterr <<-EOF &&
+       error: invalid color value: True
+       error: invalid color value: 1M
+       error: invalid color value: ~/dir
+       error: invalid color value: Fri Jun 4 15:46:55 2010
+       error: invalid color value: :(optional)no-such-path
+       error: invalid color value: :(optional)expect
+       EOF
+
+       git config ${mode_prefix}list --type=color >actual.raw 2>err &&
+       test_decode_color <actual.raw >actual &&
+       test_cmp expect actual &&
+       test_cmp expecterr err
+'
+
 test_expect_success '--type rejects unknown specifiers' '
        test_must_fail git config --type=nonsense section.foo 2>error &&
        test_grep "unrecognized --type argument" error