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: 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 --- builtin/config.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) (limited to 'builtin/config.c') diff --git a/builtin/config.c b/builtin/config.c index 7188405f7..631db458e 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, -- cgit v1.2.1 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: 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(-) (limited to 'builtin/config.c') diff --git a/builtin/config.c b/builtin/config.c index 631db458e..810e10422 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 (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_print_delim) - strbuf_addch(buf, key_delim); - strbuf_addstr(buf, vptr); - strbuf_addch(buf, term); + if (must_add_delim) + strbuf_addch(buf, key_delim); + strbuf_addstr(buf, vptr); - if (must_free_vptr) - free((char *)vptr); + if (must_free_vptr) + free((char *)vptr); + } + strbuf_addch(buf, term); return 0; } -- cgit v1.2.1 From 9f1429df179adb7a315616d01c9b237b521a3733 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 20 Aug 2015 10:46:04 -0400 Subject: 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(-) (limited to 'builtin/config.c') diff --git a/builtin/config.c b/builtin/config.c index 810e10422..91aa56fee 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_); } -- cgit v1.2.1 From f2259877531ed2a58ec04aeaeb6beb5183f81f92 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 20 Aug 2015 10:47:34 -0400 Subject: 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(-) (limited to 'builtin/config.c') diff --git a/builtin/config.c b/builtin/config.c index 91aa56fee..04befce5b 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; -- cgit v1.2.1 From a92330d21c13cf244d8045f5c9d1df6e63893d58 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 20 Aug 2015 10:49:45 -0400 Subject: 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(-) (limited to 'builtin/config.c') diff --git a/builtin/config.c b/builtin/config.c index 04befce5b..71acc4414 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); -- cgit v1.2.1