diff mbox series

[v2,06/13] builtin/config: introduce "list" subcommand

Message ID 53401299fa1f51954834e2507a2282cf60b02f20.1710198711.git.ps@pks.im (mailing list archive)
State Superseded
Headers show
Series builtin/config: introduce subcommands | expand

Commit Message

Patrick Steinhardt March 11, 2024, 11:20 p.m. UTC 
While git-config(1) has several modes, those modes are not exposed with
subcommands but instead by specifying e.g. `--unset` or `--list`. This
user interface is not really in line with how our more modern commands
work, where it is a lot more customary to say e.g. `git remote list`.
Furthermore, to add to the confusion, git-config(1) also allows the user
to request modes implicitly by just specifying the correct number of
arguments. Thus, `git config foo.bar` will retrieve the value of
"foo.bar" while `git config foo.bar baz` will set it to "baz".

Overall, this makes for a confusing interface that could really use a
makeover. It hurts discoverability of what you can do with git-config(1)
and is comparatively easy to get wrong. Converting the command to have
subcommands instead would go a long way to help address these issues.

One concern in this context is backwards compatibility. Luckily, we can
introduce subcommands without breaking backwards compatibility at all.
This is because all the implicit modes of git-config(1) require that the
first argument is a properly formatted config key. And as config keys
_must_ have a dot in their name, any value without a dot would have been
discarded by git-config(1) previous to this change. Thus, given that
none of the subcommands do have a dot, they are unambiguous.

Introduce the first such new subcommand, which is "git config list". To
retain backwards compatibility we only conditionally use subcommands and
will fall back to the old syntax in case no subcommand was detected.
This should help to transition to the new-style syntax until we
eventually deprecate and remove the old-style syntax.

Note that the way we handle this we're duplicating some functionality
across old and new syntax. While this isn't pretty, it helps us to
ensure that there really is no change in behaviour for the old syntax.

Amend tests such that we run them both with old and new style syntax.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 Documentation/git-config.txt |  26 +++++---
 builtin/config.c             |  90 ++++++++++++++++++++++++----
 t/t1300-config.sh            | 111 +++++++++++++++++++++--------------
 3 files changed, 163 insertions(+), 64 deletions(-)

Comments

Eric Sunshine March 13, 2024, 2:45 a.m. UTC | #1 
On Mon, Mar 11, 2024 at 7:20 PM Patrick Steinhardt <ps@pks.im> wrote:
> [...]
> Introduce the first such new subcommand, which is "git config list". To
> retain backwards compatibility we only conditionally use subcommands and
> will fall back to the old syntax in case no subcommand was detected.
> This should help to transition to the new-style syntax until we
> eventually deprecate and remove the old-style syntax.
> [...]
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> diff --git a/builtin/config.c b/builtin/config.c
> @@ -761,6 +811,22 @@ int cmd_config(int argc, const char **argv, const char *prefix)
> +       /*
> +        * This is somewhat hacky: we first parse the command line while
> +        * keeping all args intact in order to determine whether a subcommand
> +        * has been specified. If so, we re-parse it a second time, but this
> +        * time we drop KEEP_ARGV0. This is so that we don't munge the command
> +        * line in case no subcommand was given, which would otherwise confuse
> +        * us when parsing the implicit modes.
> +        */
> +       argc = parse_options(argc, argv, prefix, builtin_subcommand_options, builtin_config_usage,

Upon reading this, I wasn't quite sure what "when parsing the implicit
modes" meant. I suppose it is referring to the legacy style command
invocation?

> diff --git a/t/t1300-config.sh b/t/t1300-config.sh
> @@ -11,6 +11,21 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
> +for mode in legacy subcommands
> +do
> +
> +case "$mode" in
> +legacy)
> +       mode_prefix="--"
> +       ;;
> +subcommands)
> +       mode_prefix=""
> +       ;;
> +*)
> +       echo "unknown mode $mode" >&2
> +       exit 1;;
> +esac

t/test-lib.sh defines a BUG() function for signaling the sort of
programmer error handled by the "*" arm of this `case`.

An alternative simpler implementation, which wouldn't require any sort
of programmer-error fallback, would be:

    for mode_prefix in -- "" # legacy & subcommand modes
    do
        test_expect_success '...' '
            ...
        '
        ...
    done

> @@ -527,6 +542,7 @@ test_expect_success 'editing stdin is an error' '
>  test_expect_success 'refer config from subdirectory' '
> +       test_when_finished "rm -r x" &&
>         mkdir x &&
>         test_cmp_config -C x strasse --file=../other-config --get ein.bahn
>  '

Is this an unrelated cleanup?

> @@ -1072,6 +1088,7 @@ test_expect_success 'inner whitespace kept verbatim' '
>  test_expect_success SYMLINKS 'symlinked configuration' '
> +       test_when_finished "rm myconfig" &&
>         ln -s notyet myconfig &&
>         git config --file=myconfig test.frotz nitfol &&
>         test -h myconfig &&

Ditto.

Same question regarding similar changes to several later tests.
Patrick Steinhardt March 27, 2024, 8:42 a.m. UTC | #2 
On Tue, Mar 12, 2024 at 10:45:56PM -0400, Eric Sunshine wrote:
> On Mon, Mar 11, 2024 at 7:20 PM Patrick Steinhardt <ps@pks.im> wrote:
> > [...]
> > Introduce the first such new subcommand, which is "git config list". To
> > retain backwards compatibility we only conditionally use subcommands and
> > will fall back to the old syntax in case no subcommand was detected.
> > This should help to transition to the new-style syntax until we
> > eventually deprecate and remove the old-style syntax.
> > [...]
> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> > ---
> > diff --git a/builtin/config.c b/builtin/config.c
> > @@ -761,6 +811,22 @@ int cmd_config(int argc, const char **argv, const char *prefix)
> > +       /*
> > +        * This is somewhat hacky: we first parse the command line while
> > +        * keeping all args intact in order to determine whether a subcommand
> > +        * has been specified. If so, we re-parse it a second time, but this
> > +        * time we drop KEEP_ARGV0. This is so that we don't munge the command
> > +        * line in case no subcommand was given, which would otherwise confuse
> > +        * us when parsing the implicit modes.
> > +        */
> > +       argc = parse_options(argc, argv, prefix, builtin_subcommand_options, builtin_config_usage,
> 
> Upon reading this, I wasn't quite sure what "when parsing the implicit
> modes" meant. I suppose it is referring to the legacy style command
> invocation?

Yeah, indeed. Let me rephrase it as "when parsing the legacy-style
command modes", which is hopefully a bit clearer.

> > diff --git a/t/t1300-config.sh b/t/t1300-config.sh
> > @@ -11,6 +11,21 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
> > +for mode in legacy subcommands
> > +do
> > +
> > +case "$mode" in
> > +legacy)
> > +       mode_prefix="--"
> > +       ;;
> > +subcommands)
> > +       mode_prefix=""
> > +       ;;
> > +*)
> > +       echo "unknown mode $mode" >&2
> > +       exit 1;;
> > +esac
> 
> t/test-lib.sh defines a BUG() function for signaling the sort of
> programmer error handled by the "*" arm of this `case`.

Ah, right, I'll use that.

> An alternative simpler implementation, which wouldn't require any sort
> of programmer-error fallback, would be:
> 
>     for mode_prefix in -- "" # legacy & subcommand modes
>     do
>         test_expect_success '...' '
>             ...
>         '
>         ...
>     done

It works now, but will stop working in subsequent patches. Not all modes
can be translated this easily. E.g. "git config config.key value" needs
to be translated to "git config set config.key value".

> > @@ -527,6 +542,7 @@ test_expect_success 'editing stdin is an error' '
> >  test_expect_success 'refer config from subdirectory' '
> > +       test_when_finished "rm -r x" &&
> >         mkdir x &&
> >         test_cmp_config -C x strasse --file=../other-config --get ein.bahn
> >  '
> 
> Is this an unrelated cleanup?
> 
> > @@ -1072,6 +1088,7 @@ test_expect_success 'inner whitespace kept verbatim' '
> >  test_expect_success SYMLINKS 'symlinked configuration' '
> > +       test_when_finished "rm myconfig" &&
> >         ln -s notyet myconfig &&
> >         git config --file=myconfig test.frotz nitfol &&
> >         test -h myconfig &&
> 
> Ditto.
> 
> Same question regarding similar changes to several later tests.

These are required because we now re-run tests twice. Consequently,
state from the first run may still be around in the second run. The
tests here broke because of that, which I've fixed by cleaning up the
state. I'll clarify this in the commit messages.

Patrick
diff mbox series

Patch

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index dff39093b5..976ba26757 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -9,6 +9,7 @@  git-config - Get and set repository or global options
 SYNOPSIS
 --------
 [verse]
+'git config list' [<file-option>] [<display-option>] [--includes]
 'git config' [<file-option>] [--type=<type>] [--fixed-value] [--show-origin] [--show-scope] [-z|--null] <name> [<value> [<value-pattern>]]
 'git config' [<file-option>] [--type=<type>] --add <name> <value>
 'git config' [<file-option>] [--type=<type>] [--fixed-value] --replace-all <name> <value> [<value-pattern>]
@@ -20,7 +21,6 @@  SYNOPSIS
 'git config' [<file-option>] [--fixed-value] --unset-all <name> [<value-pattern>]
 'git config' [<file-option>] --rename-section <old-name> <new-name>
 'git config' [<file-option>] --remove-section <name>
-'git config' [<file-option>] [--show-origin] [--show-scope] [-z|--null] [--name-only] -l | --list
 'git config' [<file-option>] --get-color <name> [<default>]
 'git config' [<file-option>] --get-colorbool <name> [<stdout-is-tty>]
 'git config' [<file-option>] -e | --edit
@@ -74,6 +74,12 @@  On success, the command returns the exit code 0.
 A list of all available configuration variables can be obtained using the
 `git help --config` command.
 
+COMMANDS
+--------
+
+list::
+	List all variables set in config file, along with their values.
+
 [[OPTIONS]]
 OPTIONS
 -------
@@ -178,10 +184,6 @@  See also <<FILES>>.
 --unset-all::
 	Remove all lines matching the key from config file.
 
--l::
---list::
-	List all variables set in config file, along with their values.
-
 --fixed-value::
 	When used with the `value-pattern` argument, treat `value-pattern` as
 	an exact string instead of a regular expression. This will restrict
@@ -236,7 +238,7 @@  Valid `<type>`'s include:
 	contain line breaks.
 
 --name-only::
-	Output only the names of config variables for `--list` or
+	Output only the names of config variables for `list` or
 	`--get-regexp`.
 
 --show-origin::
@@ -287,10 +289,20 @@  Valid `<type>`'s include:
   When using `--get`, and the requested variable is not found, behave as if
   <value> were the value assigned to the that variable.
 
+DEPRECATED MODES
+----------------
+
+The following modes have been deprecated in favor of subcommands. It is
+recommended to migrate to the new syntax.
+
+-l::
+--list::
+	Replaced by `git config list`.
+
 CONFIGURATION
 -------------
 `pager.config` is only respected when listing configuration, i.e., when
-using `--list` or any of the `--get-*` which may return multiple results.
+using `list` or any of the `--get-*` which may return multiple results.
 The default is to use a pager.
 
 [[FILES]]
diff --git a/builtin/config.c b/builtin/config.c
index ce2d3fecd4..ee7ac9381e 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -16,10 +16,16 @@ 
 #include "worktree.h"
 
 static const char *const builtin_config_usage[] = {
+	N_("git config list [<file-option>] [<display-option>] [--includes]"),
 	N_("git config [<options>]"),
 	NULL
 };
 
+static const char *const builtin_config_list_usage[] = {
+	N_("git config list [<file-option>] [<display-option>] [--includes]"),
+	NULL
+};
+
 static char *key;
 static regex_t *key_regexp;
 static const char *value_pattern;
@@ -33,6 +39,7 @@  static char delim = '=';
 static char key_delim = ' ';
 static char term = '\n';
 
+static parse_opt_subcommand_fn *subcommand;
 static int use_global_config, use_system_config, use_local_config;
 static int use_worktree_config;
 static struct git_config_source given_config_source;
@@ -705,14 +712,24 @@  static void handle_nul(void) {
 	}
 }
 
+#define CONFIG_LOCATION_OPTIONS \
+	OPT_GROUP(N_("Config file location")), \
+	OPT_BOOL(0, "global", &use_global_config, N_("use global config file")), \
+	OPT_BOOL(0, "system", &use_system_config, N_("use system config file")), \
+	OPT_BOOL(0, "local", &use_local_config, N_("use repository config file")), \
+	OPT_BOOL(0, "worktree", &use_worktree_config, N_("use per-worktree config file")), \
+	OPT_STRING('f', "file", &given_config_source.file, N_("file"), N_("use given config file")), \
+	OPT_STRING(0, "blob", &given_config_source.blob, N_("blob-id"), N_("read config from given blob object"))
+
+#define CONFIG_DISPLAY_OPTIONS \
+	OPT_GROUP(N_("Display options")), \
+	OPT_BOOL('z', "null", &end_nul, N_("terminate values with NUL byte")), \
+	OPT_BOOL(0, "name-only", &omit_values, N_("show variable names only")), \
+	OPT_BOOL(0, "show-origin", &show_origin, N_("show origin of config (file, standard input, blob, command line)")), \
+	OPT_BOOL(0, "show-scope", &show_scope, N_("show scope of config (worktree, local, global, system, command)"))
+
 static struct option builtin_config_options[] = {
-	OPT_GROUP(N_("Config file location")),
-	OPT_BOOL(0, "global", &use_global_config, N_("use global config file")),
-	OPT_BOOL(0, "system", &use_system_config, N_("use system config file")),
-	OPT_BOOL(0, "local", &use_local_config, N_("use repository config file")),
-	OPT_BOOL(0, "worktree", &use_worktree_config, N_("use per-worktree config file")),
-	OPT_STRING('f', "file", &given_config_source.file, N_("file"), N_("use given config file")),
-	OPT_STRING(0, "blob", &given_config_source.blob, N_("blob-id"), N_("read config from given blob object")),
+	CONFIG_LOCATION_OPTIONS,
 	OPT_GROUP(N_("Action")),
 	OPT_CMDMODE(0, "get", &actions, N_("get value: name [<value-pattern>]"), ACTION_GET),
 	OPT_CMDMODE(0, "get-all", &actions, N_("get all values: key [<value-pattern>]"), ACTION_GET_ALL),
@@ -736,14 +753,11 @@  static struct option builtin_config_options[] = {
 	OPT_CALLBACK_VALUE(0, "bool-or-str", &type, N_("value is --bool or string"), TYPE_BOOL_OR_STR),
 	OPT_CALLBACK_VALUE(0, "path", &type, N_("value is a path (file or directory name)"), TYPE_PATH),
 	OPT_CALLBACK_VALUE(0, "expiry-date", &type, N_("value is an expiry date"), TYPE_EXPIRY_DATE),
+	CONFIG_DISPLAY_OPTIONS,
 	OPT_GROUP(N_("Other")),
-	OPT_BOOL('z', "null", &end_nul, N_("terminate values with NUL byte")),
-	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_BOOL(0, "show-scope", &show_scope, N_("show scope of config (worktree, local, global, system, command)")),
 	OPT_STRING(0, "default", &default_value, N_("value"), N_("with --get, use default value when missing entry")),
 	OPT_BOOL(0, "fixed-value", &fixed_value, N_("use string equality when comparing values to 'value-pattern'")),
+	OPT_BOOL(0, "includes", &respect_includes_opt, N_("respect include directives on lookup")),
 	OPT_END(),
 };
 
@@ -752,6 +766,42 @@  static NORETURN void usage_builtin_config(void)
 	usage_with_options(builtin_config_usage, builtin_config_options);
 }
 
+static int cmd_config_list(int argc, const char **argv, const char *prefix)
+{
+	struct option opts[] = {
+		CONFIG_LOCATION_OPTIONS,
+		CONFIG_DISPLAY_OPTIONS,
+		OPT_GROUP(N_("Other")),
+		OPT_BOOL(0, "includes", &respect_includes_opt, N_("respect include directives on lookup")),
+		OPT_END(),
+	};
+
+	argc = parse_options(argc, argv, prefix, opts, builtin_config_list_usage, 0);
+	check_argc(argc, 0, 0);
+
+	handle_config_location(prefix);
+	handle_nul();
+
+	setup_auto_pager("config", 1);
+
+	if (config_with_options(show_all_config, NULL,
+				&given_config_source, the_repository,
+				&config_options) < 0) {
+		if (given_config_source.file)
+			die_errno(_("unable to read config file '%s'"),
+				  given_config_source.file);
+		else
+			die(_("error processing config file(s)"));
+	}
+
+	return 0;
+}
+
+static struct option builtin_subcommand_options[] = {
+	OPT_SUBCOMMAND("list", &subcommand, cmd_config_list),
+	OPT_END(),
+};
+
 int cmd_config(int argc, const char **argv, const char *prefix)
 {
 	char *value = NULL;
@@ -761,6 +811,22 @@  int cmd_config(int argc, const char **argv, const char *prefix)
 
 	given_config_source.file = xstrdup_or_null(getenv(CONFIG_ENVIRONMENT));
 
+	/*
+	 * This is somewhat hacky: we first parse the command line while
+	 * keeping all args intact in order to determine whether a subcommand
+	 * has been specified. If so, we re-parse it a second time, but this
+	 * time we drop KEEP_ARGV0. This is so that we don't munge the command
+	 * line in case no subcommand was given, which would otherwise confuse
+	 * us when parsing the implicit modes.
+	 */
+	argc = parse_options(argc, argv, prefix, builtin_subcommand_options, builtin_config_usage,
+			     PARSE_OPT_SUBCOMMAND_OPTIONAL|PARSE_OPT_NO_INTERNAL_HELP|PARSE_OPT_KEEP_ARGV0|PARSE_OPT_KEEP_UNKNOWN_OPT);
+	if (subcommand) {
+		argc = parse_options(argc, argv, prefix, builtin_subcommand_options, builtin_config_usage,
+		       PARSE_OPT_SUBCOMMAND_OPTIONAL|PARSE_OPT_NO_INTERNAL_HELP|PARSE_OPT_KEEP_UNKNOWN_OPT);
+		return subcommand(argc, argv, prefix);
+	}
+
 	argc = parse_options(argc, argv, prefix, builtin_config_options,
 			     builtin_config_usage,
 			     PARSE_OPT_STOP_AT_NON_OPTION);
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 2d1bc1e27e..720f0ee929 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -11,6 +11,21 @@  export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
+for mode in legacy subcommands
+do
+
+case "$mode" in
+legacy)
+	mode_prefix="--"
+	;;
+subcommands)
+	mode_prefix=""
+	;;
+*)
+	echo "unknown mode $mode" >&2
+	exit 1;;
+esac
+
 test_expect_success 'clear default config' '
 	rm -f .git/config
 '
@@ -350,11 +365,11 @@  version.1.2.3eX.alpha=beta
 EOF
 
 test_expect_success 'working --list' '
-	git config --list > output &&
+	git config ${mode_prefix}list > output &&
 	test_cmp expect output
 '
 test_expect_success '--list without repo produces empty output' '
-	git --git-dir=nonexistent config --list >output &&
+	git --git-dir=nonexistent config ${mode_prefix}list >output &&
 	test_must_be_empty output
 '
 
@@ -366,7 +381,7 @@  version.1.2.3eX.alpha
 EOF
 
 test_expect_success '--name-only --list' '
-	git config --name-only --list >output &&
+	git config ${mode_prefix}list --name-only >output &&
 	test_cmp expect output
 '
 
@@ -504,17 +519,17 @@  ein.bahn=strasse
 EOF
 
 test_expect_success 'alternative GIT_CONFIG' '
-	GIT_CONFIG=other-config git config --list >output &&
+	GIT_CONFIG=other-config git config ${mode_prefix}list >output &&
 	test_cmp expect output
 '
 
 test_expect_success 'alternative GIT_CONFIG (--file)' '
-	git config --file other-config --list >output &&
+	git config ${mode_prefix}list --file other-config >output &&
 	test_cmp expect output
 '
 
 test_expect_success 'alternative GIT_CONFIG (--file=-)' '
-	git config --file - --list <other-config >output &&
+	git config ${mode_prefix}list --file - <other-config >output &&
 	test_cmp expect output
 '
 
@@ -527,6 +542,7 @@  test_expect_success 'editing stdin is an error' '
 '
 
 test_expect_success 'refer config from subdirectory' '
+	test_when_finished "rm -r x" &&
 	mkdir x &&
 	test_cmp_config -C x strasse --file=../other-config --get ein.bahn
 '
@@ -737,7 +753,7 @@  test_expect_success 'line number is reported correctly' '
 '
 
 test_expect_success 'invalid stdin config' '
-	echo "[broken" | test_must_fail git config --list --file - >output 2>&1 &&
+	echo "[broken" | test_must_fail git config ${mode_prefix}list --file - >output 2>&1 &&
 	test_grep "bad config line 1 in standard input" output
 '
 
@@ -1029,7 +1045,7 @@  section.quotecont=cont;inued
 EOF
 
 test_expect_success 'value continued on next line' '
-	git config --list > result &&
+	git config ${mode_prefix}list > result &&
 	test_cmp expect result
 '
 
@@ -1053,7 +1069,7 @@  Qsection.sub=section.val4
 Qsection.sub=section.val5Q
 EOF
 test_expect_success '--null --list' '
-	git config --null --list >result.raw &&
+	git config ${mode_prefix}list --null >result.raw &&
 	nul_to_q <result.raw >result &&
 	echo >>result &&
 	test_cmp expect result
@@ -1072,6 +1088,7 @@  test_expect_success 'inner whitespace kept verbatim' '
 '
 
 test_expect_success SYMLINKS 'symlinked configuration' '
+	test_when_finished "rm myconfig" &&
 	ln -s notyet myconfig &&
 	git config --file=myconfig test.frotz nitfol &&
 	test -h myconfig &&
@@ -1092,10 +1109,11 @@  test_expect_success SYMLINKS 'symlinked configuration' '
 '
 
 test_expect_success SYMLINKS 'symlink to nonexistent configuration' '
+	test_when_finished "rm linktonada linktolinktonada" &&
 	ln -s doesnotexist linktonada &&
 	ln -s linktonada linktolinktonada &&
-	test_must_fail git config --file=linktonada --list &&
-	test_must_fail git config --file=linktolinktonada --list
+	test_must_fail git config ${mode_prefix}list --file=linktonada &&
+	test_must_fail git config ${mode_prefix}list --file=linktolinktonada
 '
 
 test_expect_success 'check split_cmdline return' '
@@ -1352,7 +1370,7 @@  do
 done
 
 test_expect_success 'git -c is not confused by empty environment' '
-	GIT_CONFIG_PARAMETERS="" git -c x.one=1 config --list
+	GIT_CONFIG_PARAMETERS="" git -c x.one=1 config ${mode_prefix}list
 '
 
 test_expect_success 'GIT_CONFIG_PARAMETERS handles old-style entries' '
@@ -1543,31 +1561,31 @@  test_expect_success 'git config ignores pairs with empty count' '
 '
 
 test_expect_success 'git config fails with invalid count' '
-	test_must_fail env GIT_CONFIG_COUNT=10a git config --list 2>error &&
+	test_must_fail env GIT_CONFIG_COUNT=10a git config ${mode_prefix}list 2>error &&
 	test_grep "bogus count" error &&
-	test_must_fail env GIT_CONFIG_COUNT=9999999999999999 git config --list 2>error &&
+	test_must_fail env GIT_CONFIG_COUNT=9999999999999999 git config ${mode_prefix}list 2>error &&
 	test_grep "too many entries" error
 '
 
 test_expect_success 'git config fails with missing config key' '
 	test_must_fail env GIT_CONFIG_COUNT=1 GIT_CONFIG_VALUE_0="value" \
-		git config --list 2>error &&
+		git config ${mode_prefix}list 2>error &&
 	test_grep "missing config key" error
 '
 
 test_expect_success 'git config fails with missing config value' '
 	test_must_fail env GIT_CONFIG_COUNT=1 GIT_CONFIG_KEY_0="pair.one" \
-		git config --list 2>error &&
+		git config ${mode_prefix}list 2>error &&
 	test_grep "missing config value" error
 '
 
 test_expect_success 'git config fails with invalid config pair key' '
 	test_must_fail env GIT_CONFIG_COUNT=1 \
 		GIT_CONFIG_KEY_0= GIT_CONFIG_VALUE_0=value \
-		git config --list &&
+		git config ${mode_prefix}list &&
 	test_must_fail env GIT_CONFIG_COUNT=1 \
 		GIT_CONFIG_KEY_0=missing-section GIT_CONFIG_VALUE_0=value \
-		git config --list
+		git config ${mode_prefix}list
 '
 
 test_expect_success 'environment overrides config file' '
@@ -1607,7 +1625,7 @@  test_expect_success 'git config --edit works' '
 	git config -f tmp test.value no &&
 	echo test.value=yes >expect &&
 	GIT_EDITOR="echo [test]value=yes >" git config -f tmp --edit &&
-	git config -f tmp --list >actual &&
+	git config ${mode_prefix}list -f tmp >actual &&
 	test_cmp expect actual
 '
 
@@ -1616,7 +1634,7 @@  test_expect_success 'git config --edit respects core.editor' '
 	echo test.value=yes >expect &&
 	test_config core.editor "echo [test]value=yes >" &&
 	git config -f tmp --edit &&
-	git config -f tmp --list >actual &&
+	git config ${mode_prefix}list -f tmp >actual &&
 	test_cmp expect actual
 '
 
@@ -1967,7 +1985,7 @@  test_expect_success '--show-origin with --list' '
 	command line:	user.cmdline=true
 	EOF
 	GIT_CONFIG_COUNT=1 GIT_CONFIG_KEY_0=user.environ GIT_CONFIG_VALUE_0=true\
-		git -c user.cmdline=true config --list --show-origin >output &&
+		git -c user.cmdline=true config ${mode_prefix}list --show-origin >output &&
 	test_cmp expect output
 '
 
@@ -1984,7 +2002,7 @@  test_expect_success '--show-origin with --list --null' '
 	includeQcommand line:Quser.cmdline
 	trueQ
 	EOF
-	git -c user.cmdline=true config --null --list --show-origin >output.raw &&
+	git -c user.cmdline=true config ${mode_prefix}list --null --show-origin >output.raw &&
 	nul_to_q <output.raw >output &&
 	# The here-doc above adds a newline that the --null output would not
 	# include. Add it here to make the two comparable.
@@ -1998,7 +2016,7 @@  test_expect_success '--show-origin with single file' '
 	file:.git/config	user.override=local
 	file:.git/config	include.path=../include/relative.include
 	EOF
-	git config --local --list --show-origin >output &&
+	git config ${mode_prefix}list --local --show-origin >output &&
 	test_cmp expect output
 '
 
@@ -2036,7 +2054,7 @@  test_expect_success !MINGW '--show-origin escape special file name characters' '
 	cat >expect <<-\EOF &&
 	file:"file\" (dq) and spaces.conf"	user.custom=true
 	EOF
-	git config --file "$WEIRDLY_NAMED_FILE" --show-origin --list >output &&
+	git config ${mode_prefix}list --file "$WEIRDLY_NAMED_FILE" --show-origin >output &&
 	test_cmp expect output
 '
 
@@ -2044,7 +2062,7 @@  test_expect_success '--show-origin stdin' '
 	cat >expect <<-\EOF &&
 	standard input:	user.custom=true
 	EOF
-	git config --file - --show-origin --list <"$CUSTOM_CONFIG_FILE" >output &&
+	git config ${mode_prefix}list --file - --show-origin <"$CUSTOM_CONFIG_FILE" >output &&
 	test_cmp expect output
 '
 
@@ -2071,7 +2089,7 @@  test_expect_success '--show-origin blob' '
 		cat >expect <<-EOF &&
 		blob:$blob	user.custom=true
 		EOF
-		git config --blob=$blob --show-origin --list >output &&
+		git config ${mode_prefix}list --blob=$blob --show-origin >output &&
 		test_cmp expect output
 	)
 '
@@ -2087,7 +2105,7 @@  test_expect_success '--show-origin blob ref' '
 		cp "$CUSTOM_CONFIG_FILE" custom.conf &&
 		git add custom.conf &&
 		git commit -m "new config file" &&
-		git config --blob=main:custom.conf --show-origin --list >output &&
+		git config ${mode_prefix}list --blob=main:custom.conf --show-origin >output &&
 		test_cmp expect output
 	)
 '
@@ -2113,13 +2131,14 @@  test_expect_success '--show-scope with --list' '
 	worktree	user.worktree=true
 	command	user.cmdline=true
 	EOF
+	test_when_finished "git worktree remove wt1" &&
 	git worktree add wt1 &&
 	# We need these to test for worktree scope, but outside of this
 	# test, this is just noise
 	test_config core.repositoryformatversion 1 &&
 	test_config extensions.worktreeConfig true &&
 	git config --worktree user.worktree true &&
-	git -c user.cmdline=true config --list --show-scope >output &&
+	git -c user.cmdline=true config ${mode_prefix}list --show-scope >output &&
 	test_cmp expect output
 '
 
@@ -2128,7 +2147,7 @@  test_expect_success !MINGW '--show-scope with --blob' '
 	cat >expect <<-EOF &&
 	command	user.custom=true
 	EOF
-	git config --blob=$blob --show-scope --list >output &&
+	git config ${mode_prefix}list --blob=$blob --show-scope >output &&
 	test_cmp expect output
 '
 
@@ -2138,7 +2157,7 @@  test_expect_success '--show-scope with --local' '
 	local	user.override=local
 	local	include.path=../include/relative.include
 	EOF
-	git config --local --list --show-scope >output &&
+	git config ${mode_prefix}list --local --show-scope >output &&
 	test_cmp expect output
 '
 
@@ -2162,7 +2181,7 @@  test_expect_success '--show-scope with --show-origin' '
 	local	file:.git/../include/relative.include	user.relative=include
 	command	command line:	user.cmdline=true
 	EOF
-	git -c user.cmdline=true config --list --show-origin --show-scope >output &&
+	git -c user.cmdline=true config ${mode_prefix}list --show-origin --show-scope >output &&
 	test_cmp expect output
 '
 
@@ -2203,7 +2222,7 @@  test_expect_success 'override global and system config' '
 	global	home.config=true
 	local	local.config=true
 	EOF
-	git config --show-scope --list >output &&
+	git config ${mode_prefix}list --show-scope >output &&
 	test_cmp expect output &&
 
 	cat >expect <<-EOF &&
@@ -2212,20 +2231,20 @@  test_expect_success 'override global and system config' '
 	local	local.config=true
 	EOF
 	GIT_CONFIG_NOSYSTEM=false GIT_CONFIG_SYSTEM=custom-system-config GIT_CONFIG_GLOBAL=custom-global-config \
-		git config --show-scope --list >output &&
+		git config ${mode_prefix}list --show-scope >output &&
 	test_cmp expect output &&
 
 	cat >expect <<-EOF &&
 	local	local.config=true
 	EOF
 	GIT_CONFIG_NOSYSTEM=false GIT_CONFIG_SYSTEM=/dev/null GIT_CONFIG_GLOBAL=/dev/null \
-		git config --show-scope --list >output &&
+		git config ${mode_prefix}list --show-scope >output &&
 	test_cmp expect output
 '
 
 test_expect_success 'override global and system config with missing file' '
-	test_must_fail env GIT_CONFIG_GLOBAL=does-not-exist GIT_CONFIG_SYSTEM=/dev/null git config --global --list &&
-	test_must_fail env GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=does-not-exist git config --system --list &&
+	test_must_fail env GIT_CONFIG_GLOBAL=does-not-exist GIT_CONFIG_SYSTEM=/dev/null git config ${mode_prefix}list --global &&
+	test_must_fail env GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=does-not-exist git config ${mode_prefix}list --system &&
 	GIT_CONFIG_GLOBAL=does-not-exist GIT_CONFIG_SYSTEM=does-not-exist git version
 '
 
@@ -2352,7 +2371,7 @@  test_expect_success 'set all config with value-pattern' '
 	# no match => add new entry
 	cp initial config &&
 	git config --file=config abc.key two a+ &&
-	git config --file=config --list >actual &&
+	git config ${mode_prefix}list --file=config >actual &&
 	cat >expect <<-\EOF &&
 	abc.key=one
 	abc.key=two
@@ -2365,7 +2384,7 @@  test_expect_success 'set all config with value-pattern' '
 
 	# multiple values, no match => add
 	git config --file=config abc.key three a+ &&
-	git config --file=config --list >actual &&
+	git config ${mode_prefix}list --file=config >actual &&
 	cat >expect <<-\EOF &&
 	abc.key=one
 	abc.key=two
@@ -2375,7 +2394,7 @@  test_expect_success 'set all config with value-pattern' '
 
 	# single match => replace
 	git config --file=config abc.key four h+ &&
-	git config --file=config --list >actual &&
+	git config ${mode_prefix}list --file=config >actual &&
 	cat >expect <<-\EOF &&
 	abc.key=one
 	abc.key=two
@@ -2390,7 +2409,7 @@  test_expect_success '--replace-all and value-pattern' '
 	git config --file=config --add abc.key two &&
 	git config --file=config --add abc.key three &&
 	git config --file=config --replace-all abc.key four "o+" &&
-	git config --file=config --list >actual &&
+	git config ${mode_prefix}list --file=config >actual &&
 	cat >expect <<-\EOF &&
 	abc.key=four
 	abc.key=three
@@ -2408,7 +2427,7 @@  test_expect_success 'refuse --fixed-value for incompatible actions' '
 	test_must_fail git config --file=config --fixed-value --get-urlmatch dev.null bogus &&
 	test_must_fail git config --file=config --fixed-value --rename-section dev null &&
 	test_must_fail git config --file=config --fixed-value --remove-section dev &&
-	test_must_fail git config --file=config --fixed-value --list &&
+	test_must_fail git config ${mode_prefix}list --file=config --fixed-value &&
 	test_must_fail git config --file=config --fixed-value --get-color dev.null &&
 	test_must_fail git config --file=config --fixed-value --get-colorbool dev.null &&
 
@@ -2429,7 +2448,7 @@  test_expect_success '--fixed-value uses exact string matching' '
 
 	cp initial config &&
 	git config --file=config fixed.test bogus "$META" &&
-	git config --file=config --list >actual &&
+	git config ${mode_prefix}list --file=config >actual &&
 	cat >expect <<-EOF &&
 	fixed.test=$META
 	fixed.test=bogus
@@ -2438,7 +2457,7 @@  test_expect_success '--fixed-value uses exact string matching' '
 
 	cp initial config &&
 	git config --file=config --fixed-value fixed.test bogus "$META" &&
-	git config --file=config --list >actual &&
+	git config ${mode_prefix}list --file=config >actual &&
 	cat >expect <<-\EOF &&
 	fixed.test=bogus
 	EOF
@@ -2456,7 +2475,7 @@  test_expect_success '--fixed-value uses exact string matching' '
 
 	cp initial config &&
 	git config --file=config --replace-all fixed.test bogus "$META" &&
-	git config --file=config --list >actual &&
+	git config ${mode_prefix}list --file=config >actual &&
 	cat >expect <<-EOF &&
 	fixed.test=$META
 	fixed.test=bogus
@@ -2464,7 +2483,7 @@  test_expect_success '--fixed-value uses exact string matching' '
 	test_cmp expect actual &&
 
 	git config --file=config --fixed-value --replace-all fixed.test bogus "$META" &&
-	git config --file=config --list >actual &&
+	git config ${mode_prefix}list --file=config >actual &&
 	cat >expect <<-EOF &&
 	fixed.test=bogus
 	fixed.test=bogus
@@ -2625,4 +2644,6 @@  test_expect_success 'specifying multiple modes causes failure' '
 	test_cmp expect err
 '
 
+done
+
 test_done