Re: [PATCH v6 1/3] builtin/config: introduce `--default`

2018-04-09 Thread Junio C Hamano
Taylor Blau  writes:

> For some use cases, callers of the `git-config(1)` builtin would like to
> fallback to default values when the variable asked for does not exist.
> In addition, users would like to use existing type specifiers to ensure
> that values are parsed correctly when they do exist in the
> configuration.

Nicely done.


[PATCH v6 1/3] builtin/config: introduce `--default`

2018-04-09 Thread Taylor Blau
For some use cases, callers of the `git-config(1)` builtin would like to
fallback to default values when the variable asked for does not exist.
In addition, users would like to use existing type specifiers to ensure
that values are parsed correctly when they do exist in the
configuration.

For example, to fetch a value without a type specifier and fallback to
`$fallback`, the following is required:

  $ git config core.foo || echo "$fallback"

This is fine for most values, but can be tricky for difficult-to-express
`$fallback`'s, like ANSI color codes.

This motivates `--get-color`, which is a one-off exception to the normal
type specifier rules wherein a user specifies both the configuration
variable and an optional fallback. Both are formatted according to their
type specifier, which eases the burden on the user to ensure that values
are correctly formatted.

This commit (and those following it in this series) aim to eventually
replace `--get-color` with a consistent alternative. By introducing
`--default`, we allow the `--get-color` action to be promoted to a
`--type=color` type specifier, retaining the "fallback" behavior via the
`--default` flag introduced in this commit.

For example, we aim to replace:

  $ git config --get-color variable [default] [...]

with:

  $ git config --default default --type=color variable [...]

Values filled by `--default` behave exactly as if they were present in
the affected configuration file; they will be parsed by type specifiers
without the knowledge that they are not themselves present in the
configuration.

Specifically, this means that the following will work:

  $ git config --int --default 1M does.not.exist
  1048576

In subsequent commits, we will offer `--type=color`, which (in
conjunction with `--default`) will be sufficient to replace
`--get-color`.

Signed-off-by: Taylor Blau 
---
 Documentation/git-config.txt |  4 
 builtin/config.c | 18 ++
 t/t1310-config-default.sh| 36 
 3 files changed, 58 insertions(+)
 create mode 100755 t/t1310-config-default.sh

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index a1e3ffe750..b644a44ae9 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -240,6 +240,10 @@ Valid ``'s include:
using `--file`, `--global`, etc) and `on` when searching all
config files.
 
+--default ::
+  When using `--get`, and the requested variable is not found, behave as if
+   were the value assigned to the that variable.
+
 CONFIGURATION
 -
 `pager.config` is only respected when listing configuration, i.e., when
diff --git a/builtin/config.c b/builtin/config.c
index 5c8952a17c..014e3a0d53 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -26,6 +26,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, type;
+static char *default_value;
 static int end_null;
 static int respect_includes_opt = -1;
 static struct config_options config_options;
@@ -121,6 +122,7 @@ static struct option builtin_config_options[] = {
OPT_BOOL(0, "name-only", &omit_values, N_("show variable names only")),
OPT_BOOL(0, "includes", &respect_includes_opt, N_("respect include 
directives on lookup")),
OPT_BOOL(0, "show-origin", &show_origin, N_("show origin of config 
(file, standard input, blob, command line)")),
+   OPT_STRING(0, "default", &default_value, N_("value"), N_("with --get, 
use default value when missing entry")),
OPT_END(),
 };
 
@@ -285,6 +287,16 @@ static int get_value(const char *key_, const char *regex_)
config_with_options(collect_config, &values,
&given_config_source, &config_options);
 
+   if (!values.nr && default_value) {
+   struct strbuf *item;
+   ALLOC_GROW(values.items, values.nr + 1, values.alloc);
+   item = &values.items[values.nr++];
+   strbuf_init(item, 0);
+   if (format_config(item, key_, default_value) < 0)
+   die(_("failed to format default config value: %s"),
+   default_value);
+   }
+
ret = !values.nr;
 
for (i = 0; i < values.nr; i++) {
@@ -623,6 +635,12 @@ int cmd_config(int argc, const char **argv, const char 
*prefix)
usage_with_options(builtin_config_usage, 
builtin_config_options);
}
 
+   if (default_value && !(actions & ACTION_GET)) {
+   error("--default is only applicable to --get");
+   usage_with_options(builtin_config_usage,
+   builtin_config_options);
+   }
+
if (actions & PAGING_ACTIONS)
setup_auto_pager("config", 1);
 
diff --git a/t/t1310-config-default.sh b/t/t1310-config-default.sh
new file mode 100755
index 00..6049d91708
--- /dev/nul