[PATCH v2 09/13] builtin/config: introduce "unset" subcommand

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 11749 bytes --]

Introduce a new "unset" subcommand to git-config(1). Please refer to
preceding commits regarding the motivation behind this change.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 Documentation/git-config.txt | 25 ++++++++++++--------
 builtin/config.c             | 38 +++++++++++++++++++++++++++++++
 t/t1300-config.sh            | 44 ++++++++++++++++++++++++------------
 3 files changed, 82 insertions(+), 25 deletions(-)

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index e540f41b56..42e659ae63 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -12,8 +12,7 @@ SYNOPSIS
 'git config list' [<file-option>] [<display-option>] [--includes]
 'git config get' [<file-option>] [<display-option>] [--includes] [--all] [--regexp=<regexp>] [--value=<value>] [--fixed-value] [--default=<default>] <name>
 'git config set' [<file-option>] [--type=<type>] [--all] [--value=<value>] [--fixed-value] <name> <value>
-'git config' [<file-option>] [--fixed-value] --unset <name> [<value-pattern>]
-'git config' [<file-option>] [--fixed-value] --unset-all <name> [<value-pattern>]
+'git config unset' [<file-option>] [--all] [--value=<value>] [--fixed-value] <name> <value>
 'git config' [<file-option>] --rename-section <old-name> <new-name>
 'git config' [<file-option>] --remove-section <name>
 'git config' [<file-option>] --get-colorbool <name> [<stdout-is-tty>]
@@ -87,6 +86,12 @@ set::
 	`--value=` will replace all config options whose values match the given
 	pattern.
 
+unset::
+	Unset value for one or more config options. By default, this command
+	refuses to unset multi-valued keys. Passing `--all` will unset all
+	multi-valued config options, whereas `--value` will unset all config
+	options whose values match the given pattern.
+
 [[OPTIONS]]
 OPTIONS
 -------
@@ -178,12 +183,6 @@ See also <<FILES>>.
 --rename-section::
 	Rename the given section to a new name.
 
---unset::
-	Remove the line matching the key from config file.
-
---unset-all::
-	Remove all lines matching the key from config file.
-
 --fixed-value::
 	When used with the `value-pattern` argument, treat `value-pattern` as
 	an exact string instead of a regular expression. This will restrict
@@ -312,6 +311,12 @@ recommended to migrate to the new syntax.
 --add <name> <value>::
 	Replaced by `git config set --append <name> <value>`.
 
+--unset <name> [<value-pattern>]::
+	Replaced by `git config unset [--value=<pattern>] <name>`.
+
+--unset-all <name> [<value-pattern>]::
+	Replaced by `git config unset [--value=<pattern>] --all <name>`.
+
 CONFIGURATION
 -------------
 `pager.config` is only respected when listing configuration, i.e., when
@@ -359,7 +364,7 @@ values of a key from all files will be used.
 
 By default, options are only written to the repository specific
 configuration file. Note that this also affects options like `set`
-and `--unset`. *'git config' will only ever change one file at a time*.
+and `unset`. *'git config' will only ever change one file at a time*.
 
 You can limit which configuration sources are read from or written to by
 specifying the path of a file with the `--file` option, or by specifying a
@@ -510,7 +515,7 @@ This makes sure that only the key/value pair for kernel.org is replaced.
 To delete the entry for renames, do
 
 ------------
-% git config --unset diff.renames
+% git config unset diff.renames
 ------------
 
 If you want to delete an entry for a multivar (like core.gitproxy above),
diff --git a/builtin/config.c b/builtin/config.c
index 18f6dc69d5..e62a09061f 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -19,6 +19,7 @@ static const char *const builtin_config_usage[] = {
 	N_("git config list [<file-option>] [<display-option>] [--includes]"),
 	N_("git config get [<file-option>] [<display-option>] [--includes] [--all] [--regexp=<regexp>] [--value=<value>] [--fixed-value] [--default=<default>] <name>"),
 	N_("git config set [<file-option>] [--type=<type>] [--all] [--value=<value>] [--fixed-value] <name> <value>"),
+	N_("git config unset [<file-option>] [--all] [--value=<value>] [--fixed-value] <name> <value>"),
 	NULL
 };
 
@@ -37,6 +38,11 @@ static const char *const builtin_config_set_usage[] = {
 	NULL
 };
 
+static const char *const builtin_config_unset_usage[] = {
+	N_("git config unset [<file-option>] [--all] [--value=<value>] [--fixed-value] <name> <value>"),
+	NULL
+};
+
 static char *key;
 static regex_t *key_regexp;
 static const char *value_pattern;
@@ -903,10 +909,42 @@ static int cmd_config_set(int argc, const char **argv, const char *prefix)
 	return ret;
 }
 
+static int cmd_config_unset(int argc, const char **argv, const char *prefix)
+{
+	const char *value_pattern = NULL;
+	int flags = 0;
+	struct option opts[] = {
+		CONFIG_LOCATION_OPTIONS,
+		OPT_GROUP(N_("Filter")),
+		OPT_BIT(0, "all", &flags, N_("replace multi-valued config option with new value"), CONFIG_FLAGS_MULTI_REPLACE),
+		OPT_STRING(0, "value", &value_pattern, N_("pattern"), N_("show config with values matching the pattern")),
+		OPT_BIT(0, "fixed-value", &flags, N_("use string equality when comparing values to value pattern"), CONFIG_FLAGS_FIXED_VALUE),
+		OPT_END(),
+	};
+
+	argc = parse_options(argc, argv, prefix, opts, builtin_config_unset_usage,
+			     PARSE_OPT_STOP_AT_NON_OPTION);
+	check_write();
+	check_argc(argc, 1, 1);
+
+	if ((flags & CONFIG_FLAGS_FIXED_VALUE) && !value_pattern)
+		die(_("--fixed-value only applies with 'value-pattern'"));
+
+	handle_config_location(prefix);
+
+	if ((flags & CONFIG_FLAGS_MULTI_REPLACE) || value_pattern)
+		return git_config_set_multivar_in_file_gently(given_config_source.file,
+							      argv[0], NULL, value_pattern,
+							      flags);
+	else
+		return git_config_set_in_file_gently(given_config_source.file, argv[0], NULL);
+}
+
 static struct option builtin_subcommand_options[] = {
 	OPT_SUBCOMMAND("list", &subcommand, cmd_config_list),
 	OPT_SUBCOMMAND("get", &subcommand, cmd_config_get),
 	OPT_SUBCOMMAND("set", &subcommand, cmd_config_set),
+	OPT_SUBCOMMAND("unset", &subcommand, cmd_config_unset),
 	OPT_END(),
 };
 
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 2c2d97e0e9..06d34b1d6a 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -22,6 +22,8 @@ legacy)
 	mode_get_regexp="--get-regexp"
 	mode_set=""
 	mode_replace_all="--replace-all"
+	mode_unset="--unset"
+	mode_unset_all="--unset-all"
 	;;
 subcommands)
 	mode_prefix=""
@@ -30,6 +32,8 @@ subcommands)
 	mode_get_regexp="get --regexp --all --show-names"
 	mode_set="set"
 	mode_replace_all="set --all"
+	mode_unset="unset"
+	mode_unset_all="unset --all"
 	;;
 *)
 	echo "unknown mode $mode" >&2
@@ -150,7 +154,7 @@ foo = bar
 EOF
 
 test_expect_success 'unset with cont. lines' '
-	git config --unset beta.baz
+	git config ${mode_unset} beta.baz
 '
 
 cat > expect <<\EOF
@@ -177,7 +181,7 @@ EOF
 cp .git/config .git/config2
 
 test_expect_success 'multiple unset' '
-	git config --unset-all beta.haha
+	git config ${mode_unset_all} beta.haha
 '
 
 cat > expect << EOF
@@ -263,7 +267,7 @@ noIndent= sillyValue ; 'nother silly comment
 	nonewline = wow
 EOF
 test_expect_success 'unset' '
-	git config --unset beta.haha &&
+	git config ${mode_unset} beta.haha &&
 	test_cmp expect .git/config
 '
 
@@ -319,11 +323,11 @@ test_expect_success 'multivar replace' '
 '
 
 test_expect_success 'ambiguous unset' '
-	test_must_fail git config --unset nextsection.nonewline
+	test_must_fail git config ${mode_unset} nextsection.nonewline
 '
 
 test_expect_success 'invalid unset' '
-	test_must_fail git config --unset somesection.nonewline
+	test_must_fail git config ${mode_unset} somesection.nonewline
 '
 
 cat > expect << EOF
@@ -337,7 +341,12 @@ noIndent= sillyValue ; 'nother silly comment
 EOF
 
 test_expect_success 'multivar unset' '
-	git config --unset nextsection.nonewline "wow3$" &&
+	case "$mode" in
+	legacy)
+		git config --unset nextsection.nonewline "wow3$";;
+	subcommands)
+		git config unset --value="wow3$" nextsection.nonewline;;
+	esac &&
 	test_cmp expect .git/config
 '
 
@@ -1888,7 +1897,7 @@ test_expect_success '--unset last key removes section (except if commented)' '
 	# please be careful when you update the above variable
 	EOF
 
-	git config --unset section.key &&
+	git config ${mode_unset} section.key &&
 	test_cmp expect .git/config &&
 
 	cat >.git/config <<-\EOF &&
@@ -1901,7 +1910,7 @@ test_expect_success '--unset last key removes section (except if commented)' '
 	[next-section]
 	EOF
 
-	git config --unset section.key &&
+	git config ${mode_unset} section.key &&
 	test_cmp expect .git/config &&
 
 	q_to_tab >.git/config <<-\EOF &&
@@ -1911,7 +1920,7 @@ test_expect_success '--unset last key removes section (except if commented)' '
 	[two]
 	key = true
 	EOF
-	git config --unset two.key &&
+	git config ${mode_unset} two.key &&
 	! grep two .git/config &&
 
 	q_to_tab >.git/config <<-\EOF &&
@@ -1921,7 +1930,7 @@ test_expect_success '--unset last key removes section (except if commented)' '
 	[one]
 	key = true
 	EOF
-	git config --unset-all one.key &&
+	git config ${mode_unset_all} one.key &&
 	test_line_count = 0 .git/config &&
 
 	q_to_tab >.git/config <<-\EOF &&
@@ -1931,7 +1940,7 @@ test_expect_success '--unset last key removes section (except if commented)' '
 	[two]
 	Qkey = true
 	EOF
-	git config --unset two.key &&
+	git config ${mode_unset} two.key &&
 	grep two .git/config &&
 
 	q_to_tab >.git/config <<-\EOF &&
@@ -1943,7 +1952,7 @@ test_expect_success '--unset last key removes section (except if commented)' '
 	[TWO "subsection"]
 	[one]
 	EOF
-	git config --unset two.subsection.key &&
+	git config ${mode_unset} two.subsection.key &&
 	test "not [two subsection]" = "$(git config ${mode_get} one.key)" &&
 	test_line_count = 3 .git/config
 '
@@ -1955,7 +1964,7 @@ test_expect_success '--unset-all removes section if empty & uncommented' '
 	key = value2
 	EOF
 
-	git config --unset-all section.key &&
+	git config ${mode_unset_all} section.key &&
 	test_line_count = 0 .git/config
 '
 
@@ -2479,8 +2488,8 @@ test_expect_success 'refuse --fixed-value for incompatible actions' '
 	test_must_fail git config ${mode_prefix}get --file=config --fixed-value dev.null &&
 	test_must_fail git config ${mode_get_all} --file=config --fixed-value dev.null &&
 	test_must_fail git config ${mode_get_regexp} --file=config --fixed-value "dev.*" &&
-	test_must_fail git config --file=config --fixed-value --unset dev.null &&
-	test_must_fail git config --file=config --fixed-value --unset-all dev.null
+	test_must_fail git config ${mode_unset} --file=config --fixed-value dev.null &&
+	test_must_fail git config ${mode_unset_all} --file=config --fixed-value dev.null
 '
 
 test_expect_success '--fixed-value uses exact string matching' '
@@ -2510,6 +2519,11 @@ test_expect_success '--fixed-value uses exact string matching' '
 	git config --file=config --fixed-value --unset fixed.test "$META" &&
 	test_must_fail git config ${mode_get} --file=config fixed.test &&
 
+	cp initial config &&
+	test_must_fail git config unset --file=config --value="$META" fixed.test &&
+	git config unset --file=config --fixed-value --value="$META" fixed.test &&
+	test_must_fail git config ${mode_get} --file=config fixed.test &&
+
 	cp initial config &&
 	test_must_fail git config --file=config --unset-all fixed.test "$META" &&
 	git config --file=config --fixed-value --unset-all fixed.test "$META" &&
-- 
2.44.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]