From 578625fa918922713a2ecce2b06611e4566778f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Mon, 10 Aug 2015 11:46:06 +0200 Subject: [PATCH 1/6] config: add '--name-only' option to list only variable names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 'git config' can only show values or name-value pairs, so if a shell script needs the names of set config variables it has to run 'git config --list' or '--get-regexp' and parse the output to separate config variable names from their values. However, such a parsing can't cope with multi-line values. Though 'git config' can produce null-terminated output for newline-safe parsing, that's of no use in such a case, becase shells can't cope with null characters. Even our own bash completion script suffers from these issues. Help the completion script, and shell scripts in general, by introducing the '--name-only' option to modify the output of '--list' and '--get-regexp' to list only the names of config variables, so they don't have to perform error-prone post processing to separate variable names from their values anymore. Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano --- Documentation/git-config.txt | 10 +++++++--- builtin/config.c | 14 ++++++++++++-- contrib/completion/git-completion.bash | 1 + t/t1300-repo-config.sh | 22 ++++++++++++++++++++++ 4 files changed, 42 insertions(+), 5 deletions(-) diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt index 02ec096faa..2608ca74ac 100644 --- a/Documentation/git-config.txt +++ b/Documentation/git-config.txt @@ -14,13 +14,13 @@ SYNOPSIS 'git config' [] [type] --replace-all name value [value_regex] 'git config' [] [type] [-z|--null] --get name [value_regex] 'git config' [] [type] [-z|--null] --get-all name [value_regex] -'git config' [] [type] [-z|--null] --get-regexp name_regex [value_regex] +'git config' [] [type] [-z|--null] [--name-only] --get-regexp name_regex [value_regex] 'git config' [] [type] [-z|--null] --get-urlmatch name URL 'git config' [] --unset name [value_regex] 'git config' [] --unset-all name [value_regex] 'git config' [] --rename-section old_name new_name 'git config' [] --remove-section name -'git config' [] [-z|--null] -l | --list +'git config' [] [-z|--null] [--name-only] -l | --list 'git config' [] --get-color name [default] 'git config' [] --get-colorbool name [stdout-is-tty] 'git config' [] -e | --edit @@ -159,7 +159,7 @@ See also <>. -l:: --list:: - List all variables set in config file. + List all variables set in config file, along with their values. --bool:: 'git config' will ensure that the output is "true" or "false" @@ -190,6 +190,10 @@ See also <>. output without getting confused e.g. by values that contain line breaks. +--name-only:: + Output only the names of config variables for `--list` or + `--get-regexp`. + --get-colorbool name [stdout-is-tty]:: Find the color setting for `name` (e.g. `color.diff`) and output diff --git a/builtin/config.c b/builtin/config.c index 7188405f7e..631db458ec 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -13,6 +13,7 @@ static char *key; static regex_t *key_regexp; static regex_t *regexp; static int show_keys; +static int omit_values; static int use_key_regexp; static int do_all; static int do_not_match; @@ -78,6 +79,7 @@ static struct option builtin_config_options[] = { OPT_BIT(0, "path", &types, N_("value is a path (file or directory name)"), TYPE_PATH), 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_BOOL(0, "includes", &respect_includes, N_("respect include directives on lookup")), OPT_END(), }; @@ -91,7 +93,7 @@ static void check_argc(int argc, int min, int max) { static int show_all_config(const char *key_, const char *value_, void *cb) { - if (value_) + if (!omit_values && value_) printf("%s%c%s%c", key_, delim, value_, term); else printf("%s%c", key_, term); @@ -117,6 +119,10 @@ static int format_config(struct strbuf *buf, const char *key_, const char *value strbuf_addstr(buf, key_); must_print_delim = 1; } + if (omit_values) { + strbuf_addch(buf, term); + return 0; + } if (types == TYPE_INT) sprintf(value, "%"PRId64, git_config_int64(key_, value_ ? value_ : "")); @@ -549,7 +555,11 @@ int cmd_config(int argc, const char **argv, const char *prefix) default: usage_with_options(builtin_config_usage, builtin_config_options); } - + if (omit_values && + !(actions == ACTION_LIST || actions == ACTION_GET_REGEXP)) { + error("--name-only is only applicable to --list or --get-regexp"); + usage_with_options(builtin_config_usage, builtin_config_options); + } if (actions == ACTION_LIST) { check_argc(argc, 0, 0); if (git_config_with_options(show_all_config, NULL, diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index bfc74e9d57..e9877e1fc8 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -1887,6 +1887,7 @@ _git_config () --get --get-all --get-regexp --add --unset --unset-all --remove-section --rename-section + --name-only " return ;; diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh index 66dd28644f..52678e7d0a 100755 --- a/t/t1300-repo-config.sh +++ b/t/t1300-repo-config.sh @@ -352,6 +352,18 @@ test_expect_success '--list without repo produces empty output' ' test_cmp expect output ' +cat > expect << EOF +beta.noindent +nextsection.nonewline +123456.a123 +version.1.2.3eX.alpha +EOF + +test_expect_success '--name-only --list' ' + git config --name-only --list >output && + test_cmp expect output +' + cat > expect << EOF beta.noindent sillyValue nextsection.nonewline wow2 for me @@ -362,6 +374,16 @@ test_expect_success '--get-regexp' ' test_cmp expect output ' +cat > expect << EOF +beta.noindent +nextsection.nonewline +EOF + +test_expect_success '--name-only --get-regexp' ' + git config --name-only --get-regexp in >output && + test_cmp expect output +' + cat > expect << EOF wow2 for me wow4 for you From 905f2036d0975a828f764947384e732c2908d6eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Mon, 10 Aug 2015 11:46:07 +0200 Subject: [PATCH 2/6] completion: list variable names reliably with 'git config --name-only' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Recenty I created a multi-line branch description with '.' and '=' characters on one of the lines, and noticed that fragments of that line show up when completing set variable names for 'git config', e.g.: $ git config --get branch.b.description Branch description to fool the completion script with a second line containing dot . and equals = characters. $ git config --unset ... second line containing dot . and equals ... The completion script runs 'git config --list' and processes its output to strip the values and keep only the variable names. It does so by looking for lines containing '.' and '=' and outputting everything before the '=', which was fooled by my multi-line branch description. A similar issue exists with aliases and pretty format aliases with multi-line values, but in that case 'git config --get-regexp' is run and lines in its output are simply stripped after the first space, so subsequent lines don't even have to contain '.' and '=' to fool the completion script. Use the new '--name-only' option added in the previous commit to list config variable names reliably in both cases, without error-prone post processing. Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano --- contrib/completion/git-completion.bash | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index e9877e1fc8..0ec56942f6 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -744,9 +744,8 @@ __git_compute_porcelain_commands () __git_get_config_variables () { local section="$1" i IFS=$'\n' - for i in $(git --git-dir="$(__gitdir)" config --get-regexp "^$section\..*" 2>/dev/null); do - i="${i#$section.}" - echo "${i/ */}" + for i in $(git --git-dir="$(__gitdir)" config --name-only --get-regexp "^$section\..*" 2>/dev/null); do + echo "${i#$section.}" done } @@ -1774,15 +1773,7 @@ __git_config_get_set_variables () c=$((--c)) done - git --git-dir="$(__gitdir)" config $config_file --list 2>/dev/null | - while read -r line - do - case "$line" in - *.*=*) - echo "${line/=*/}" - ;; - esac - done + git --git-dir="$(__gitdir)" config $config_file --name-only --list 2>/dev/null } _git_config () From ebca2d49577665db0318a9c91c0bcca7e4eed963 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Thu, 20 Aug 2015 16:14:22 +0200 Subject: [PATCH 3/6] config: restructure format_config() for better control flow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit 578625fa91 (config: add '--name-only' option to list only variable names, 2015-08-10) modified format_config() such that it returned from the middle of the function when showing only keys, resulting in ugly code structure. Reorganize the if statements and dealing with the key-value delimiter to make the function easier to read. Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano --- builtin/config.c | 78 +++++++++++++++++++++++------------------------- 1 file changed, 37 insertions(+), 41 deletions(-) diff --git a/builtin/config.c b/builtin/config.c index 631db458ec..810e104224 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -108,52 +108,48 @@ struct strbuf_list { static int format_config(struct strbuf *buf, const char *key_, const char *value_) { - int must_free_vptr = 0; - int must_print_delim = 0; - char value[256]; - const char *vptr = value; - strbuf_init(buf, 0); - if (show_keys) { + if (show_keys) strbuf_addstr(buf, key_); - must_print_delim = 1; - } - if (omit_values) { - strbuf_addch(buf, term); - return 0; - } - if (types == TYPE_INT) - sprintf(value, "%"PRId64, - git_config_int64(key_, value_ ? value_ : "")); - else if (types == TYPE_BOOL) - vptr = git_config_bool(key_, value_) ? "true" : "false"; - else if (types == TYPE_BOOL_OR_INT) { - int is_bool, v; - v = git_config_bool_or_int(key_, value_, &is_bool); - if (is_bool) - vptr = v ? "true" : "false"; - else - sprintf(value, "%d", v); - } else if (types == TYPE_PATH) { - if (git_config_pathname(&vptr, key_, value_) < 0) - return -1; - must_free_vptr = 1; - } else if (value_) { - vptr = value_; - } else { - /* Just show the key name */ - vptr = ""; - must_print_delim = 0; - } + if (!omit_values) { + int must_free_vptr = 0; + int must_add_delim = show_keys; + char value[256]; + const char *vptr = value; - if (must_print_delim) - strbuf_addch(buf, key_delim); - strbuf_addstr(buf, vptr); + if (types == TYPE_INT) + sprintf(value, "%"PRId64, + git_config_int64(key_, value_ ? value_ : "")); + else if (types == TYPE_BOOL) + vptr = git_config_bool(key_, value_) ? "true" : "false"; + else if (types == TYPE_BOOL_OR_INT) { + int is_bool, v; + v = git_config_bool_or_int(key_, value_, &is_bool); + if (is_bool) + vptr = v ? "true" : "false"; + else + sprintf(value, "%d", v); + } else if (types == TYPE_PATH) { + if (git_config_pathname(&vptr, key_, value_) < 0) + return -1; + must_free_vptr = 1; + } else if (value_) { + vptr = value_; + } else { + /* Just show the key name */ + vptr = ""; + must_add_delim = 0; + } + + if (must_add_delim) + strbuf_addch(buf, key_delim); + strbuf_addstr(buf, vptr); + + if (must_free_vptr) + free((char *)vptr); + } strbuf_addch(buf, term); - - if (must_free_vptr) - free((char *)vptr); return 0; } From 9f1429df179adb7a315616d01c9b237b521a3733 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 20 Aug 2015 10:46:04 -0400 Subject: [PATCH 4/6] format_config: don't init strbuf It's unusual for a function which writes to a passed-in strbuf to call strbuf_init; that will throw away anything already there, leaking memory. In this case, there are exactly two callers; one relies on this initialization and the other passes in an already-initialized buffer. There's no leak, as the initialized buffer doesn't have anything in it. But let's bump the strbuf_init out to the one caller who needs it, making format_config more idiomatic. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/config.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/builtin/config.c b/builtin/config.c index 810e104224..91aa56feed 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -108,8 +108,6 @@ struct strbuf_list { static int format_config(struct strbuf *buf, const char *key_, const char *value_) { - strbuf_init(buf, 0); - if (show_keys) strbuf_addstr(buf, key_); if (!omit_values) { @@ -166,6 +164,7 @@ static int collect_config(const char *key_, const char *value_, void *cb) return 0; ALLOC_GROW(values->items, values->nr + 1, values->alloc); + strbuf_init(&values->items[values->nr], 0); return format_config(&values->items[values->nr++], key_, value_); } From f2259877531ed2a58ec04aeaeb6beb5183f81f92 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 20 Aug 2015 10:47:34 -0400 Subject: [PATCH 5/6] format_config: simplify buffer handling When formatting a config value into a strbuf, we may end up stringifying it into a fixed-size buffer using sprintf, and then copying that buffer into the strbuf. We can eliminate the middle-man (and drop some calls to sprintf!) by writing directly to the strbuf. The reason it was written this way in the first place is that we need to know before writing the value whether to insert a delimiter. Instead of delaying the write of the value, we speculatively write the delimiter, and roll it back in the single case that cares. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/config.c | 38 ++++++++++++++++---------------------- 1 file changed, 16 insertions(+), 22 deletions(-) diff --git a/builtin/config.c b/builtin/config.c index 91aa56feed..04befce5b7 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -111,41 +111,35 @@ static int format_config(struct strbuf *buf, const char *key_, const char *value if (show_keys) strbuf_addstr(buf, key_); if (!omit_values) { - int must_free_vptr = 0; - int must_add_delim = show_keys; - char value[256]; - const char *vptr = value; + if (show_keys) + strbuf_addch(buf, key_delim); if (types == TYPE_INT) - sprintf(value, "%"PRId64, - git_config_int64(key_, value_ ? value_ : "")); + strbuf_addf(buf, "%"PRId64, + git_config_int64(key_, value_ ? value_ : "")); else if (types == TYPE_BOOL) - vptr = git_config_bool(key_, value_) ? "true" : "false"; + strbuf_addstr(buf, git_config_bool(key_, value_) ? + "true" : "false"); else if (types == TYPE_BOOL_OR_INT) { int is_bool, v; v = git_config_bool_or_int(key_, value_, &is_bool); if (is_bool) - vptr = v ? "true" : "false"; + strbuf_addstr(buf, v ? "true" : "false"); else - sprintf(value, "%d", v); + strbuf_addf(buf, "%d", v); } else if (types == TYPE_PATH) { - if (git_config_pathname(&vptr, key_, value_) < 0) + const char *v; + if (git_config_pathname(&v, key_, value_) < 0) return -1; - must_free_vptr = 1; + strbuf_addstr(buf, v); + free((char *)v); } else if (value_) { - vptr = value_; + strbuf_addstr(buf, value_); } else { - /* Just show the key name */ - vptr = ""; - must_add_delim = 0; + /* Just show the key name; back out delimiter */ + if (show_keys) + strbuf_setlen(buf, buf->len - 1); } - - if (must_add_delim) - strbuf_addch(buf, key_delim); - strbuf_addstr(buf, vptr); - - if (must_free_vptr) - free((char *)vptr); } strbuf_addch(buf, term); return 0; From a92330d21c13cf244d8045f5c9d1df6e63893d58 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 20 Aug 2015 10:49:45 -0400 Subject: [PATCH 6/6] get_urlmatch: avoid useless strbuf write We create a strbuf only to insert a single string, pass the resulting buffer to a function (which does not modify the string), and then free it. We can just pass the original string instead. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/config.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/builtin/config.c b/builtin/config.c index 04befce5b7..71acc44143 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -425,14 +425,11 @@ static int get_urlmatch(const char *var, const char *url) for_each_string_list_item(item, &values) { struct urlmatch_current_candidate_value *matched = item->util; - struct strbuf key = STRBUF_INIT; struct strbuf buf = STRBUF_INIT; - strbuf_addstr(&key, item->string); - format_config(&buf, key.buf, + format_config(&buf, item->string, matched->value_is_null ? NULL : matched->value.buf); fwrite(buf.buf, 1, buf.len, stdout); - strbuf_release(&key); strbuf_release(&buf); strbuf_release(&matched->value);