[PATCH 1/2] config: add options to list only variable names

Subsystems: documentation, the rest

STALE3738d

12 messages, 3 authors, 2016-06-15 · open the first message on its own page

[PATCH 1/2] config: add options to list only variable names

From: SZEDER Gábor <hidden>
Date: 2016-06-15 23:05:01 

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 <TAB>
  ...
  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
subsequent lines don't have to contain either '.' or '=' to fool the
completion script.

Though 'git config' can produce null-terminated output for newline-safe
parsing, that's of no use in this case, becase we can't cope with nulls in
the shell.

Help the completion script by introducing the '--list-names' and
'--get-names-regexp' options, the "names-only" equivalents of '--list' and
'--get-regexp', so it doesn't have to separate variable names from their
values anymore.

Signed-off-by: SZEDER Gábor <redacted>
---
 Documentation/git-config.txt           | 10 +++++++++-
 builtin/config.c                       | 22 ++++++++++++++++++----
 contrib/completion/git-completion.bash |  4 ++--
 t/t1300-repo-config.sh                 | 22 ++++++++++++++++++++++
 4 files changed, 51 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 02ec096..e0d27d5 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -16,11 +16,12 @@ SYNOPSIS
 'git config' [<file-option>] [type] [-z|--null] --get-all name [value_regex]
 'git config' [<file-option>] [type] [-z|--null] --get-regexp name_regex [value_regex]
 'git config' [<file-option>] [type] [-z|--null] --get-urlmatch name URL
+'git config' [<file-option>] [-z|--null] --get-name-regexp name_regex
 'git config' [<file-option>] --unset name [value_regex]
 'git config' [<file-option>] --unset-all name [value_regex]
 'git config' [<file-option>] --rename-section old_name new_name
 'git config' [<file-option>] --remove-section name
-'git config' [<file-option>] [-z|--null] -l | --list
+'git config' [<file-option>] [-z|--null] -l | --list | --list-name
 'git config' [<file-option>] --get-color name [default]
 'git config' [<file-option>] --get-colorbool name [stdout-is-tty]
 'git config' [<file-option>] -e | --edit
@@ -96,6 +97,10 @@ OPTIONS
 	in which section and variable names are lowercased, but subsection
 	names are not.
 
+--get-name-regexp::
+	Like --get-regexp, but shows only matching variable names, not its
+	values.
+
 --get-urlmatch name URL::
 	When given a two-part name section.key, the value for
 	section.<url>.key whose <url> part matches the best to the
@@ -161,6 +166,9 @@ See also <<FILES>>.
 --list::
 	List all variables set in config file.
 
+--list-name::
+	List the names of all variables set in config file.
+
 --bool::
 	'git config' will ensure that the output is "true" or "false"
 
diff --git a/builtin/config.c b/builtin/config.c
index 7188405..38bcf83 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 show_only_keys;
 static int use_key_regexp;
 static int do_all;
 static int do_not_match;
@@ -43,6 +44,8 @@ static int respect_includes = -1;
 #define ACTION_GET_COLOR (1<<13)
 #define ACTION_GET_COLORBOOL (1<<14)
 #define ACTION_GET_URLMATCH (1<<15)
+#define ACTION_LIST_NAMES (1<<16)
+#define ACTION_GET_NAME_REGEXP (1<<17)
 
 #define TYPE_BOOL (1<<0)
 #define TYPE_INT (1<<1)
@@ -60,6 +63,7 @@ static struct option builtin_config_options[] = {
 	OPT_BIT(0, "get", &actions, N_("get value: name [value-regex]"), ACTION_GET),
 	OPT_BIT(0, "get-all", &actions, N_("get all values: key [value-regex]"), ACTION_GET_ALL),
 	OPT_BIT(0, "get-regexp", &actions, N_("get values for regexp: name-regex [value-regex]"), ACTION_GET_REGEXP),
+	OPT_BIT(0, "get-name-regexp", &actions, N_("get names for regexp: name-regex"), ACTION_GET_NAME_REGEXP),
 	OPT_BIT(0, "get-urlmatch", &actions, N_("get value specific for the URL: section[.var] URL"), ACTION_GET_URLMATCH),
 	OPT_BIT(0, "replace-all", &actions, N_("replace all matching variables: name value [value_regex]"), ACTION_REPLACE_ALL),
 	OPT_BIT(0, "add", &actions, N_("add a new variable: name value"), ACTION_ADD),
@@ -68,6 +72,7 @@ static struct option builtin_config_options[] = {
 	OPT_BIT(0, "rename-section", &actions, N_("rename section: old-name new-name"), ACTION_RENAME_SECTION),
 	OPT_BIT(0, "remove-section", &actions, N_("remove a section: name"), ACTION_REMOVE_SECTION),
 	OPT_BIT('l', "list", &actions, N_("list all"), ACTION_LIST),
+	OPT_BIT(0, "list-names", &actions, N_("list all variable names"), ACTION_LIST_NAMES),
 	OPT_BIT('e', "edit", &actions, N_("open an editor"), ACTION_EDIT),
 	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),
@@ -91,7 +96,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 (!show_only_keys && value_)
 		printf("%s%c%s%c", key_, delim, value_, term);
 	else
 		printf("%s%c", key_, term);
@@ -165,7 +170,14 @@ static int collect_config(const char *key_, const char *value_, void *cb)
 
 	ALLOC_GROW(values->items, values->nr + 1, values->alloc);
 
-	return format_config(&values->items[values->nr++], key_, value_);
+	if (show_only_keys) {
+		struct strbuf *buf = &values->items[values->nr++];
+		strbuf_init(buf, 0);
+		strbuf_addstr(buf, key_);
+		strbuf_addch(buf, term);
+		return 0;
+	} else
+		return format_config(&values->items[values->nr++], key_, value_);
 }
 
 static int get_value(const char *key_, const char *regex_)
@@ -550,7 +562,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 			usage_with_options(builtin_config_usage, builtin_config_options);
 		}
 
-	if (actions == ACTION_LIST) {
+	if (actions == ACTION_LIST || actions == ACTION_LIST_NAMES) {
+		show_only_keys = (actions == ACTION_LIST_NAMES);
 		check_argc(argc, 0, 0);
 		if (git_config_with_options(show_all_config, NULL,
 					    &given_config_source,
@@ -631,8 +644,9 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		check_argc(argc, 1, 2);
 		return get_value(argv[0], argv[1]);
 	}
-	else if (actions == ACTION_GET_REGEXP) {
+	else if (actions == ACTION_GET_REGEXP || actions == ACTION_GET_NAME_REGEXP) {
 		show_keys = 1;
+		show_only_keys = (actions == ACTION_GET_NAME_REGEXP);
 		use_key_regexp = 1;
 		do_all = 1;
 		check_argc(argc, 1, 2);
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index bfc74e9..6abbd56 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1883,8 +1883,8 @@ _git_config ()
 	--*)
 		__gitcomp "
 			--system --global --local --file=
-			--list --replace-all
-			--get --get-all --get-regexp
+			--list --list-names --replace-all
+			--get --get-all --get-regexp --get-name-regexp
 			--add --unset --unset-all
 			--remove-section --rename-section
 			"
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 66dd286..fa83944 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -353,6 +353,18 @@ test_expect_success '--list without repo produces empty output' '
 '
 
 cat > expect << EOF
+beta.noindent
+nextsection.nonewline
+123456.a123
+version.1.2.3eX.alpha
+EOF
+
+test_expect_success 'working --list-names' '
+	git config --list-names > output &&
+	test_cmp expect output
+'
+
+cat > expect << EOF
 beta.noindent sillyValue
 nextsection.nonewline wow2 for me
 EOF
@@ -363,6 +375,16 @@ test_expect_success '--get-regexp' '
 '
 
 cat > expect << EOF
+beta.noindent
+nextsection.nonewline
+EOF
+
+test_expect_success '--get-name-regexp' '
+	git config --get-name-regexp in >output &&
+	test_cmp expect output
+'
+
+cat > expect << EOF
 wow2 for me
 wow4 for you
 EOF
-- 
2.4.2.347.ge926c0d

[PATCH 2/2] completion: use new 'git config' options to reliably list variable names

From: SZEDER Gábor <hidden>
Date: 2016-06-15 23:05:01 

List all set config variable names with 'git config --list-names' instead
of '--list' post processing.  Similarly, use 'git config
--get-name-regexp' instead of '--get-regexp' to get config variables in a
given section.

Signed-off-by: SZEDER Gábor <redacted>
---
 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 6abbd56..121aa31 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 --get-name-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 --list-names 2>/dev/null
 }
 
 _git_config ()
-- 
2.4.2.347.ge926c0d

[PATCH 1.5/2] config: add options to list only variable names

From: SZEDER Gábor <hidden>
Date: 2016-06-15 23:05:01 

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 <TAB>
  ...
  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
subsequent lines don't have to contain either '.' or '=' to fool the
completion script.

Though 'git config' can produce null-terminated output for newline-safe
parsing, that's of no use in this case, becase we can't cope with nulls in
the shell.

Help the completion script by introducing the '--list-names' and
'--get-names-regexp' options, the "names-only" equivalents of '--list' and
'--get-regexp', so it doesn't have to separate variable names from their
values anymore.

Signed-off-by: SZEDER Gábor <redacted>
---
D'oh, misspelled the option in the docs...

 Documentation/git-config.txt           | 10 +++++++++-
 builtin/config.c                       | 22 ++++++++++++++++++----
 contrib/completion/git-completion.bash |  4 ++--
 t/t1300-repo-config.sh                 | 22 ++++++++++++++++++++++
 4 files changed, 51 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 02ec096faa..fd519f81d8 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -16,11 +16,12 @@ SYNOPSIS
 'git config' [<file-option>] [type] [-z|--null] --get-all name [value_regex]
 'git config' [<file-option>] [type] [-z|--null] --get-regexp name_regex [value_regex]
 'git config' [<file-option>] [type] [-z|--null] --get-urlmatch name URL
+'git config' [<file-option>] [-z|--null] --get-name-regexp name_regex
 'git config' [<file-option>] --unset name [value_regex]
 'git config' [<file-option>] --unset-all name [value_regex]
 'git config' [<file-option>] --rename-section old_name new_name
 'git config' [<file-option>] --remove-section name
-'git config' [<file-option>] [-z|--null] -l | --list
+'git config' [<file-option>] [-z|--null] -l | --list | --list-names
 'git config' [<file-option>] --get-color name [default]
 'git config' [<file-option>] --get-colorbool name [stdout-is-tty]
 'git config' [<file-option>] -e | --edit
@@ -96,6 +97,10 @@ OPTIONS
 	in which section and variable names are lowercased, but subsection
 	names are not.
 
+--get-name-regexp::
+	Like --get-regexp, but shows only matching variable names, not its
+	values.
+
 --get-urlmatch name URL::
 	When given a two-part name section.key, the value for
 	section.<url>.key whose <url> part matches the best to the
@@ -161,6 +166,9 @@ See also <<FILES>>.
 --list::
 	List all variables set in config file.
 
+--list-names::
+	List the names of all variables set in config file.
+
 --bool::
 	'git config' will ensure that the output is "true" or "false"
 
diff --git a/builtin/config.c b/builtin/config.c
index 7188405f7e..38bcf838c5 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 show_only_keys;
 static int use_key_regexp;
 static int do_all;
 static int do_not_match;
@@ -43,6 +44,8 @@ static int respect_includes = -1;
 #define ACTION_GET_COLOR (1<<13)
 #define ACTION_GET_COLORBOOL (1<<14)
 #define ACTION_GET_URLMATCH (1<<15)
+#define ACTION_LIST_NAMES (1<<16)
+#define ACTION_GET_NAME_REGEXP (1<<17)
 
 #define TYPE_BOOL (1<<0)
 #define TYPE_INT (1<<1)
@@ -60,6 +63,7 @@ static struct option builtin_config_options[] = {
 	OPT_BIT(0, "get", &actions, N_("get value: name [value-regex]"), ACTION_GET),
 	OPT_BIT(0, "get-all", &actions, N_("get all values: key [value-regex]"), ACTION_GET_ALL),
 	OPT_BIT(0, "get-regexp", &actions, N_("get values for regexp: name-regex [value-regex]"), ACTION_GET_REGEXP),
+	OPT_BIT(0, "get-name-regexp", &actions, N_("get names for regexp: name-regex"), ACTION_GET_NAME_REGEXP),
 	OPT_BIT(0, "get-urlmatch", &actions, N_("get value specific for the URL: section[.var] URL"), ACTION_GET_URLMATCH),
 	OPT_BIT(0, "replace-all", &actions, N_("replace all matching variables: name value [value_regex]"), ACTION_REPLACE_ALL),
 	OPT_BIT(0, "add", &actions, N_("add a new variable: name value"), ACTION_ADD),
@@ -68,6 +72,7 @@ static struct option builtin_config_options[] = {
 	OPT_BIT(0, "rename-section", &actions, N_("rename section: old-name new-name"), ACTION_RENAME_SECTION),
 	OPT_BIT(0, "remove-section", &actions, N_("remove a section: name"), ACTION_REMOVE_SECTION),
 	OPT_BIT('l', "list", &actions, N_("list all"), ACTION_LIST),
+	OPT_BIT(0, "list-names", &actions, N_("list all variable names"), ACTION_LIST_NAMES),
 	OPT_BIT('e', "edit", &actions, N_("open an editor"), ACTION_EDIT),
 	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),
@@ -91,7 +96,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 (!show_only_keys && value_)
 		printf("%s%c%s%c", key_, delim, value_, term);
 	else
 		printf("%s%c", key_, term);
@@ -165,7 +170,14 @@ static int collect_config(const char *key_, const char *value_, void *cb)
 
 	ALLOC_GROW(values->items, values->nr + 1, values->alloc);
 
-	return format_config(&values->items[values->nr++], key_, value_);
+	if (show_only_keys) {
+		struct strbuf *buf = &values->items[values->nr++];
+		strbuf_init(buf, 0);
+		strbuf_addstr(buf, key_);
+		strbuf_addch(buf, term);
+		return 0;
+	} else
+		return format_config(&values->items[values->nr++], key_, value_);
 }
 
 static int get_value(const char *key_, const char *regex_)
@@ -550,7 +562,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 			usage_with_options(builtin_config_usage, builtin_config_options);
 		}
 
-	if (actions == ACTION_LIST) {
+	if (actions == ACTION_LIST || actions == ACTION_LIST_NAMES) {
+		show_only_keys = (actions == ACTION_LIST_NAMES);
 		check_argc(argc, 0, 0);
 		if (git_config_with_options(show_all_config, NULL,
 					    &given_config_source,
@@ -631,8 +644,9 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		check_argc(argc, 1, 2);
 		return get_value(argv[0], argv[1]);
 	}
-	else if (actions == ACTION_GET_REGEXP) {
+	else if (actions == ACTION_GET_REGEXP || actions == ACTION_GET_NAME_REGEXP) {
 		show_keys = 1;
+		show_only_keys = (actions == ACTION_GET_NAME_REGEXP);
 		use_key_regexp = 1;
 		do_all = 1;
 		check_argc(argc, 1, 2);
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index bfc74e9d57..6abbd564b6 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1883,8 +1883,8 @@ _git_config ()
 	--*)
 		__gitcomp "
 			--system --global --local --file=
-			--list --replace-all
-			--get --get-all --get-regexp
+			--list --list-names --replace-all
+			--get --get-all --get-regexp --get-name-regexp
 			--add --unset --unset-all
 			--remove-section --rename-section
 			"
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 66dd28644f..fa83944ad9 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -353,6 +353,18 @@ test_expect_success '--list without repo produces empty output' '
 '
 
 cat > expect << EOF
+beta.noindent
+nextsection.nonewline
+123456.a123
+version.1.2.3eX.alpha
+EOF
+
+test_expect_success 'working --list-names' '
+	git config --list-names > output &&
+	test_cmp expect output
+'
+
+cat > expect << EOF
 beta.noindent sillyValue
 nextsection.nonewline wow2 for me
 EOF
@@ -363,6 +375,16 @@ test_expect_success '--get-regexp' '
 '
 
 cat > expect << EOF
+beta.noindent
+nextsection.nonewline
+EOF
+
+test_expect_success '--get-name-regexp' '
+	git config --get-name-regexp in >output &&
+	test_cmp expect output
+'
+
+cat > expect << EOF
 wow2 for me
 wow4 for you
 EOF
-- 
2.4.2.347.ge926c0d

Re: [PATCH 1/2] config: add options to list only variable names

From: Jeff King <hidden>
Date: 2016-06-15 23:05:01 

On Wed, May 27, 2015 at 10:07:19PM +0200, SZEDER Gábor wrote:

Help the completion script by introducing the '--list-names' and
'--get-names-regexp' options, the "names-only" equivalents of '--list' and
'--get-regexp', so it doesn't have to separate variable names from their
values anymore.
Thanks, this sounds like the best solution. It should be a tiny bit more
efficient, too, though I doubt it matters much in practice.

-'git config' [<file-option>] [-z|--null] -l | --list
+'git config' [<file-option>] [-z|--null] -l | --list | --list-name
s/list-name/&s/, to match the code (and your commit message).
quoted hunk
@@ -161,6 +166,9 @@ See also <<FILES>>.
 --list::
 	List all variables set in config file.
 
+--list-name::
+	List the names of all variables set in config file.
Ditto here. Also, now that we have two similar modes, perhaps the
"--list" description above should become:

  List all variables set in config file, along with their values.
quoted hunk
@@ -165,7 +170,14 @@ static int collect_config(const char *key_, const char *value_, void *cb)
 
 	ALLOC_GROW(values->items, values->nr + 1, values->alloc);
 
-	return format_config(&values->items[values->nr++], key_, value_);
+	if (show_only_keys) {
+		struct strbuf *buf = &values->items[values->nr++];
+		strbuf_init(buf, 0);
+		strbuf_addstr(buf, key_);
+		strbuf_addch(buf, term);
+		return 0;
+	} else
+		return format_config(&values->items[values->nr++], key_, value_);
 }
Might it flow a little better to always enter format_config, and then
just return early (before writing the value) when show_key_only is set?

 cat > expect << EOF
+beta.noindent
+nextsection.nonewline
+123456.a123
+version.1.2.3eX.alpha
+EOF
+
+test_expect_success 'working --list-names' '
+	git config --list-names > output &&
+	test_cmp expect output
+'
+
+cat > expect << EOF
We usually avoid the extra space after redirection operators. But we
also usually match existing code. I'm not sure which is more evil in
this case. ;)

+test_expect_success '--get-name-regexp' '
+	git config --get-name-regexp in >output &&
+	test_cmp expect output
+'
This one is the odd man out if you are following existing style,
though.

The rest of the patch looks good to me.

-Peff

Re: [PATCH 2/2] completion: use new 'git config' options to reliably list variable names

From: Jeff King <hidden>
Date: 2016-06-15 23:05:01 

On Wed, May 27, 2015 at 10:07:20PM +0200, SZEDER Gábor wrote:

List all set config variable names with 'git config --list-names' instead
of '--list' post processing.  Similarly, use 'git config
--get-name-regexp' instead of '--get-regexp' to get config variables in a
given section.

Signed-off-by: SZEDER Gábor <redacted>
---
 contrib/completion/git-completion.bash | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)
Nice diffstat. The less hacky bash parsing we can do, the better.

-Peff

Re: [PATCH 1/2] config: add options to list only variable names

From: Jeff King <hidden>
Date: 2016-06-15 23:05:01 

On Wed, May 27, 2015 at 05:04:38PM -0400, Jeff King wrote:
quoted
-'git config' [<file-option>] [-z|--null] -l | --list
+'git config' [<file-option>] [-z|--null] -l | --list | --list-name
s/list-name/&s/, to match the code (and your commit message).
Doh, just saw your "1.5".

FWIW, I expected "PATCH 1.5/2" to be "eh, I should have put this in
between patches 1 and 2". I expect a re-roll to be "v1.5" (or just
"v2").

This was the only real error in the patch, so your 1.5 looks OK to me.
Though I hope you will consider my other suggestions, too.

-Peff

Re: [PATCH 1/2] config: add options to list only variable names

From: SZEDER Gábor <hidden>
Date: 2016-06-15 23:05:01 

Quoting Jeff King [off-list ref]:

On Wed, May 27, 2015 at 10:07:19PM +0200, SZEDER Gábor wrote:
quoted
Help the completion script by introducing the '--list-names' and
'--get-names-regexp' options, the "names-only" equivalents of '--list' and
'--get-regexp', so it doesn't have to separate variable names from their
values anymore.
Thanks, this sounds like the best solution. It should be a tiny bit more
efficient, too, though I doubt it matters much in practice.
quoted
-'git config' [<file-option>] [-z|--null] -l | --list
+'git config' [<file-option>] [-z|--null] -l | --list | --list-name
s/list-name/&s/, to match the code (and your commit message).
And note how I added an extra 's' to the other option in the commit message!
quoted
 cat > expect << EOF
+beta.noindent
+nextsection.nonewline
+123456.a123
+version.1.2.3eX.alpha
+EOF
+
+test_expect_success 'working --list-names' '
+	git config --list-names > output &&
+	test_cmp expect output
+'
+
+cat > expect << EOF
We usually avoid the extra space after redirection operators. But we
also usually match existing code. I'm not sure which is more evil in
this case. ;)
quoted
+test_expect_success '--get-name-regexp' '
+	git config --get-name-regexp in >output &&
+	test_cmp expect output
+'
This one is the odd man out if you are following existing style,
though.
Heh, in both cases I simply copied the existing "name-less" test, and  
only adjusted the expected output and the name of the option to test. :)

Will reroll.

Gábor

Re: [PATCH 1.5/2] config: add options to list only variable names

From: Christian Couder <hidden>
Date: 2016-06-15 23:05:02 

On Wed, May 27, 2015 at 10:11 PM, SZEDER Gábor [off-list ref] wrote:
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 <TAB>
  ...
  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
subsequent lines don't have to contain either '.' or '=' to fool the
completion script.

Though 'git config' can produce null-terminated output for newline-safe
parsing, that's of no use in this case, becase we can't cope with nulls in
the shell.

Help the completion script by introducing the '--list-names' and
'--get-names-regexp' options, the "names-only" equivalents of '--list' and
'--get-regexp', so it doesn't have to separate variable names from their
values anymore.
Why don't you just add a '--name-only' option that can be used only
with '--list' and '--get-regexp'?

Like:

'git config' [<file-option>] [-z|--null] [--name-only] --get-regexp name_regex

and

'git config' [<file-option>] [-z|--null] [--name-only] -l | --list

?

It seems to me that it would reduce the number of options, and later
if we want to pass a format we could have maybe:

'git config' [<file-option>] [-z|--null] [--name-only |
--format=<format>] --get-regexp name_regex

and

'git config' [<file-option>] [-z|--null] [--name-only |
--format=<format>] -l | --list

Thanks,
Christian.

[PATCH v2 1/2] config: add options to list only variable names

From: SZEDER Gábor <hidden>
Date: 2016-06-15 23:05:02 

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 <TAB>
  ...
  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
subsequent lines don't have to contain either '.' or '=' to fool the
completion script.

Though 'git config' can produce null-terminated output for newline-safe
parsing, that's of no use in this case, becase we can't cope with nulls in
the shell.

Help the completion script by introducing the '--list-names' and
'--get-name-regexp' options, the "names-only" equivalents of '--list' and
'--get-regexp', so it doesn't have to separate variable names from their
values anymore.

Signed-off-by: SZEDER Gábor <redacted>
---
 Documentation/git-config.txt           | 12 ++++++++++--
 builtin/config.c                       | 17 ++++++++++++++---
 contrib/completion/git-completion.bash |  4 ++--
 t/t1300-repo-config.sh                 | 22 ++++++++++++++++++++++
 4 files changed, 48 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 02ec096faa..b69c8592ac 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -16,11 +16,12 @@ SYNOPSIS
 'git config' [<file-option>] [type] [-z|--null] --get-all name [value_regex]
 'git config' [<file-option>] [type] [-z|--null] --get-regexp name_regex [value_regex]
 'git config' [<file-option>] [type] [-z|--null] --get-urlmatch name URL
+'git config' [<file-option>] [-z|--null] --get-name-regexp name_regex
 'git config' [<file-option>] --unset name [value_regex]
 'git config' [<file-option>] --unset-all name [value_regex]
 'git config' [<file-option>] --rename-section old_name new_name
 'git config' [<file-option>] --remove-section name
-'git config' [<file-option>] [-z|--null] -l | --list
+'git config' [<file-option>] [-z|--null] -l | --list | --list-names
 'git config' [<file-option>] --get-color name [default]
 'git config' [<file-option>] --get-colorbool name [stdout-is-tty]
 'git config' [<file-option>] -e | --edit
@@ -96,6 +97,10 @@ OPTIONS
 	in which section and variable names are lowercased, but subsection
 	names are not.
 
+--get-name-regexp::
+	Like --get-regexp, but shows only matching variable names, not its
+	values.
+
 --get-urlmatch name URL::
 	When given a two-part name section.key, the value for
 	section.<url>.key whose <url> part matches the best to the
@@ -159,7 +164,10 @@ See also <<FILES>>.
 
 -l::
 --list::
-	List all variables set in config file.
+	List all variables set in config file, along with their values.
+
+--list-names::
+	List the names of all variables set in config file.
 
 --bool::
 	'git config' will ensure that the output is "true" or "false"
diff --git a/builtin/config.c b/builtin/config.c
index 7188405f7e..c23f329b00 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;
@@ -43,6 +44,8 @@ static int respect_includes = -1;
 #define ACTION_GET_COLOR (1<<13)
 #define ACTION_GET_COLORBOOL (1<<14)
 #define ACTION_GET_URLMATCH (1<<15)
+#define ACTION_LIST_NAMES (1<<16)
+#define ACTION_GET_NAME_REGEXP (1<<17)
 
 #define TYPE_BOOL (1<<0)
 #define TYPE_INT (1<<1)
@@ -60,6 +63,7 @@ static struct option builtin_config_options[] = {
 	OPT_BIT(0, "get", &actions, N_("get value: name [value-regex]"), ACTION_GET),
 	OPT_BIT(0, "get-all", &actions, N_("get all values: key [value-regex]"), ACTION_GET_ALL),
 	OPT_BIT(0, "get-regexp", &actions, N_("get values for regexp: name-regex [value-regex]"), ACTION_GET_REGEXP),
+	OPT_BIT(0, "get-name-regexp", &actions, N_("get names for regexp: name-regex"), ACTION_GET_NAME_REGEXP),
 	OPT_BIT(0, "get-urlmatch", &actions, N_("get value specific for the URL: section[.var] URL"), ACTION_GET_URLMATCH),
 	OPT_BIT(0, "replace-all", &actions, N_("replace all matching variables: name value [value_regex]"), ACTION_REPLACE_ALL),
 	OPT_BIT(0, "add", &actions, N_("add a new variable: name value"), ACTION_ADD),
@@ -68,6 +72,7 @@ static struct option builtin_config_options[] = {
 	OPT_BIT(0, "rename-section", &actions, N_("rename section: old-name new-name"), ACTION_RENAME_SECTION),
 	OPT_BIT(0, "remove-section", &actions, N_("remove a section: name"), ACTION_REMOVE_SECTION),
 	OPT_BIT('l', "list", &actions, N_("list all"), ACTION_LIST),
+	OPT_BIT(0, "list-names", &actions, N_("list all variable names"), ACTION_LIST_NAMES),
 	OPT_BIT('e', "edit", &actions, N_("open an editor"), ACTION_EDIT),
 	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),
@@ -91,7 +96,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 +122,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_ : ""));
@@ -550,7 +559,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 			usage_with_options(builtin_config_usage, builtin_config_options);
 		}
 
-	if (actions == ACTION_LIST) {
+	if (actions == ACTION_LIST || actions == ACTION_LIST_NAMES) {
+		omit_values = (actions == ACTION_LIST_NAMES);
 		check_argc(argc, 0, 0);
 		if (git_config_with_options(show_all_config, NULL,
 					    &given_config_source,
@@ -631,8 +641,9 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		check_argc(argc, 1, 2);
 		return get_value(argv[0], argv[1]);
 	}
-	else if (actions == ACTION_GET_REGEXP) {
+	else if (actions == ACTION_GET_REGEXP || actions == ACTION_GET_NAME_REGEXP) {
 		show_keys = 1;
+		omit_values = (actions == ACTION_GET_NAME_REGEXP);
 		use_key_regexp = 1;
 		do_all = 1;
 		check_argc(argc, 1, 2);
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index bfc74e9d57..6abbd564b6 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1883,8 +1883,8 @@ _git_config ()
 	--*)
 		__gitcomp "
 			--system --global --local --file=
-			--list --replace-all
-			--get --get-all --get-regexp
+			--list --list-names --replace-all
+			--get --get-all --get-regexp --get-name-regexp
 			--add --unset --unset-all
 			--remove-section --rename-section
 			"
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 66dd28644f..525b093c59 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -353,6 +353,18 @@ test_expect_success '--list without repo produces empty output' '
 '
 
 cat > expect << EOF
+beta.noindent
+nextsection.nonewline
+123456.a123
+version.1.2.3eX.alpha
+EOF
+
+test_expect_success 'working --list-names' '
+	git config --list-names >output &&
+	test_cmp expect output
+'
+
+cat > expect << EOF
 beta.noindent sillyValue
 nextsection.nonewline wow2 for me
 EOF
@@ -363,6 +375,16 @@ test_expect_success '--get-regexp' '
 '
 
 cat > expect << EOF
+beta.noindent
+nextsection.nonewline
+EOF
+
+test_expect_success '--get-name-regexp' '
+	git config --get-name-regexp in >output &&
+	test_cmp expect output
+'
+
+cat > expect << EOF
 wow2 for me
 wow4 for you
 EOF
-- 
2.4.2.349.g6883b65

[PATCH v2 2/2] completion: use new 'git config' options to reliably list variable names

From: SZEDER Gábor <hidden>
Date: 2016-06-15 23:05:02 

List all set config variable names with 'git config --list-names' instead
of '--list' and post processing.  Similarly, use 'git config
--get-name-regexp' instead of '--get-regexp' to get config variables in a
given section.

Signed-off-by: SZEDER Gábor <redacted>
---
 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 6abbd564b6..121aa31342 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 --get-name-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 --list-names 2>/dev/null
 }
 
 _git_config ()
-- 
2.4.2.349.g6883b65

[PATCH v2 0/2] config: list only variable names for completion

From: SZEDER Gábor <hidden>
Date: 2016-06-15 23:05:02 

Fixed misspelled option names in docs and in commit message, incorporated
Peff's suggestions, and renamed 'show_only_keys' to 'omit_values' in 1/2.

Only minor tweaking in 2/2's commit message.

SZEDER Gábor (2):
  config: add options to list only variable names
  completion: use new 'git config' options to reliably list variable
    names

 Documentation/git-config.txt           | 12 ++++++++++--
 builtin/config.c                       | 17 ++++++++++++++---
 contrib/completion/git-completion.bash | 19 +++++--------------
 t/t1300-repo-config.sh                 | 22 ++++++++++++++++++++++
 4 files changed, 51 insertions(+), 19 deletions(-)

-- 
2.4.2.349.g6883b65

Re: [PATCH v2 0/2] config: list only variable names for completion

From: Jeff King <hidden>
Date: 2016-06-15 23:05:02 

On Thu, May 28, 2015 at 02:29:33PM +0200, SZEDER Gábor wrote:

Fixed misspelled option names in docs and in commit message, incorporated
Peff's suggestions, and renamed 'show_only_keys' to 'omit_values' in 1/2.

Only minor tweaking in 2/2's commit message.
This version looks good to me (though I agree with Junio's proposed
squash about the '[value_regex]' documentation).

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help