On Mon, Mar 11, 2024 at 7:20 PM Patrick Steinhardt <ps@xxxxxx> 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@xxxxxx>
> ---
> 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.
