| Message ID | b3f3c3ba6ab33d269f7a707eae1456c87cdfeddb.1714730170.git.ps@pks.im (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | builtin/config: introduce subcommands | expand |
Patrick Steinhardt <ps@pks.im> writes: > While git-config(1) has several modes, those modes are not exposed with > subcommands but instead by specifying e.g. `--unset` or `--list`. This s/specifying/specifying flags/ perhaps? > 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`. Tangent: I totally agree with the patch, but it would be nice to have a 'DesigningCommands' document which would highlight UX do's and don'ts. It would be nice to add that as reference in discussions. > 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. > As tests are now run twice, state from the first run may be still be > around in the second run and thus cause tests to fail. Add cleanup logic > as required to fix such tests. > > Signed-off-by: Patrick Steinhardt <ps@pks.im> > --- > Documentation/git-config.txt | 26 ++++++--- > builtin/config.c | 90 ++++++++++++++++++++++++---- > t/t1300-config.sh | 110 +++++++++++++++++++++-------------- > 3 files changed, 162 insertions(+), 64 deletions(-) > > diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt > index ac61113fcc..c83c97cb7e 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>] [--comment=<message>] [--fixed-value] [--show-origin] [--show-scope] [-z|--null] <name> [<value> [<value-pattern>]] > 'git config' [<file-option>] [--type=<type>] [--comment=<message>] --add <name> <value> > 'git config' [<file-option>] [--type=<type>] [--comment=<message>] [--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 > ------- > @@ -190,10 +196,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 > @@ -248,7 +250,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:: > @@ -300,10 +302,20 @@ Valid `<type>`'s include: > When using `--get`, and the requested variable is not found, behave as if > <value> were the value assigned to 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 59877065f8..f89ddce8da 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; > @@ -706,14 +713,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), > @@ -737,15 +754,12 @@ 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_STRING(0, "comment", &comment_arg, N_("value"), N_("human-readable comment string (# will be prepended as needed)")), > 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(), > }; > > @@ -754,6 +768,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, *comment = NULL; > @@ -763,6 +813,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 legacy-style modes that don't use subcommands. > + */ > + 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 86dc70769a..f77d2f7847 100755 > --- a/t/t1300-config.sh > +++ b/t/t1300-config.sh > @@ -11,6 +11,20 @@ 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="" > + ;; > +*) > + BUG "unknown mode $mode";; > +esac > + > test_expect_success 'setup whitespace config' ' > sed -e "s/^|//" \ > -e "s/[$]$//" \ > @@ -460,11 +474,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 > ' > > @@ -476,7 +490,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 > ' > > @@ -614,17 +628,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 > ' > > @@ -637,6 +651,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 > ' > @@ -847,7 +862,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 > ' > > @@ -1139,7 +1154,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 > ' > > @@ -1163,7 +1178,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 > @@ -1198,6 +1213,7 @@ test_expect_success 'inner whitespace kept verbatim, horizontal tabs and spaces' > ' > > 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 && > @@ -1218,10 +1234,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' ' > @@ -1478,7 +1495,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' ' > @@ -1669,31 +1686,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' ' > @@ -1733,7 +1750,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 > ' > > @@ -1742,7 +1759,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 > ' > > @@ -2093,7 +2110,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 > ' > > @@ -2110,7 +2127,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. > @@ -2124,7 +2141,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 > ' > > @@ -2162,7 +2179,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 > ' > > @@ -2170,7 +2187,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 > ' > > @@ -2197,7 +2214,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 > ) > ' > @@ -2213,7 +2230,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 > ) > ' > @@ -2239,13 +2256,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 > ' > > @@ -2254,7 +2272,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 > ' > > @@ -2264,7 +2282,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 > ' > > @@ -2288,7 +2306,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 > ' > > @@ -2329,7 +2347,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 && > @@ -2338,20 +2356,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 > ' > > @@ -2478,7 +2496,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 > @@ -2491,7 +2509,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 > @@ -2501,7 +2519,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 > @@ -2516,7 +2534,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 > @@ -2534,7 +2552,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 && > > @@ -2555,7 +2573,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 > @@ -2564,7 +2582,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 > @@ -2582,7 +2600,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 > @@ -2590,7 +2608,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 > @@ -2751,4 +2769,6 @@ test_expect_success 'specifying multiple modes causes failure' ' > test_cmp expect err > ' > > +done > + > Nit: Wouldn't it be better if the tests are indented here? That way you know it's part of a loop.
On Friday, May 3, 2024 9:09 AM. Karthik Nayak wrote: >Patrick Steinhardt <ps@pks.im> writes: > >> While git-config(1) has several modes, those modes are not exposed >> with subcommands but instead by specifying e.g. `--unset` or `--list`. >> This > >s/specifying/specifying flags/ perhaps? > >> 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`. > >Tangent: I totally agree with the patch, but it would be nice to have a >'DesigningCommands' document which would highlight UX do's and don'ts. >It would be nice to add that as reference in discussions. > >> 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. >> As tests are now run twice, state from the first run may be still be >> around in the second run and thus cause tests to fail. Add cleanup >> logic as required to fix such tests. >> >> Signed-off-by: Patrick Steinhardt <ps@pks.im> >> --- >> Documentation/git-config.txt | 26 ++++++--- >> builtin/config.c | 90 ++++++++++++++++++++++++---- >> t/t1300-config.sh | 110 +++++++++++++++++++++-------------- >> 3 files changed, 162 insertions(+), 64 deletions(-) >> >> diff --git a/Documentation/git-config.txt >> b/Documentation/git-config.txt index ac61113fcc..c83c97cb7e 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>] [--comment=<message>] >> [--fixed-value] [--show-origin] [--show-scope] [-z|--null] <name> >> [<value> [<value-pattern>]] 'git config' [<file-option>] >> [--type=<type>] [--comment=<message>] --add <name> <value> 'git >> config' [<file-option>] [--type=<type>] [--comment=<message>] >> [--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 >> ------- >> @@ -190,10 +196,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 >> @@ -248,7 +250,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:: >> @@ -300,10 +302,20 @@ Valid `<type>`'s include: >> When using `--get`, and the requested variable is not found, behave as if >> <value> were the value assigned to 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 >> 59877065f8..f89ddce8da 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; @@ -706,14 +713,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), @@ -737,15 +754,12 @@ 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_STRING(0, "comment", &comment_arg, N_("value"), N_("human- >readable comment string (# will be prepended as needed)")), >> 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(), >> }; >> >> @@ -754,6 +768,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, *comment = NULL; @@ -763,6 +813,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 legacy-style modes that don't use subcommands. >> + */ >> + 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 86dc70769a..f77d2f7847 100755 >> --- a/t/t1300-config.sh >> +++ b/t/t1300-config.sh >> @@ -11,6 +11,20 @@ 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="" >> + ;; >> +*) >> + BUG "unknown mode $mode";; >> +esac >> + >> test_expect_success 'setup whitespace config' ' >> sed -e "s/^|//" \ >> -e "s/[$]$//" \ >> @@ -460,11 +474,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 >> ' >> >> @@ -476,7 +490,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 >> ' >> >> @@ -614,17 +628,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 >> ' >> >> @@ -637,6 +651,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 >> ' >> @@ -847,7 +862,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 ' >> >> @@ -1139,7 +1154,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 >> ' >> >> @@ -1163,7 +1178,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 >> @@ -1198,6 +1213,7 @@ test_expect_success 'inner whitespace kept verbatim, >horizontal tabs and spaces' >> ' >> >> 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 && >> @@ -1218,10 +1234,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' ' >> @@ -1478,7 +1495,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' ' >> @@ -1669,31 +1686,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' ' >> @@ -1733,7 +1750,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 >> ' >> >> @@ -1742,7 +1759,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 >> ' >> >> @@ -2093,7 +2110,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 >> ' >> >> @@ -2110,7 +2127,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. >> @@ -2124,7 +2141,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 >> ' >> >> @@ -2162,7 +2179,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 >> ' >> >> @@ -2170,7 +2187,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 >> ' >> >> @@ -2197,7 +2214,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 >> ) >> ' >> @@ -2213,7 +2230,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 >> ) >> ' >> @@ -2239,13 +2256,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 >> ' >> >> @@ -2254,7 +2272,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 >> ' >> >> @@ -2264,7 +2282,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 >> ' >> >> @@ -2288,7 +2306,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 >> ' >> >> @@ -2329,7 +2347,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 && >> @@ -2338,20 +2356,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 ' >> >> @@ -2478,7 +2496,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 >> @@ -2491,7 +2509,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 >> @@ -2501,7 +2519,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 >> @@ -2516,7 +2534,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 >> @@ -2534,7 +2552,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 && >> >> @@ -2555,7 +2573,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 >> @@ -2564,7 +2582,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 >> @@ -2582,7 +2600,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 >> @@ -2590,7 +2608,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 >> @@ -2751,4 +2769,6 @@ test_expect_success 'specifying multiple modes causes >failure' ' >> test_cmp expect err >> ' >> >> +done >> + >> >Nit: Wouldn't it be better if the tests are indented here? That way you know it's part >of a loop. Removing the --list option is going to break backward compatibility for users who script the use of config for things like setup, clone automation, etc. Adding list as a sub-command could (but should not) cause ambiguities between a list and configuration value. If you are going to add 'list', please do not remove '--list'. That is a breaking change. Thanks, Randall
<rsbecker@nexbridge.com> writes: >>> +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`. Can you trim quotes in your response that has no relevance to what you are responding to? > Removing the --list option is going to break backward > compatibility for users who script the use of config for things > like setup, clone automation, etc. Yes, that is why this is merely a deprecation. > Adding list as a sub-command could (but should not) cause > ambiguities between a list and configuration value. Because there is no sectionless variable, there exists no ambiguity. In other words, $ git config list cannot be asking for the value of the variable "list", because such a variable cannot exist. When you start your command with $ git config list ... whatever other tokens follow that token "list" on the command line, it is a "--list" subcommand. A traditional and lazy "get a single variable" without an explicit subcommand would look like $ git config section.variable that is a short-hand to $ git config --get section.variable and would become $ git config get section.variable in the new world order. Thanks.
On Fri, May 03, 2024 at 09:01:06AM -0700, Junio C Hamano wrote: > <rsbecker@nexbridge.com> writes: > > >>> +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`. > > Can you trim quotes in your response that has no relevance to what > you are responding to? > > > Removing the --list option is going to break backward > > compatibility for users who script the use of config for things > > like setup, clone automation, etc. > > Yes, that is why this is merely a deprecation. I will probably send a follow-up patch series that converts other tests to use the new syntax and that does some more cleanups. I was pondering whether we want to introduce a document as part of that patch series that starts to keep track of upcoming removals for a potential Git 3.0 release. There are multiple items that could be added: - Removal of the old syntax of git-config(1). - Removal of the dumb HTTP transport. - Removal of `info/grafts`. There are probably other items. In any case, the old actions are here to stay for the foreseeable future until we commit to a breaking major release. > > Adding list as a sub-command could (but should not) cause > > ambiguities between a list and configuration value. > > Because there is no sectionless variable, there exists no ambiguity. > > In other words, > > $ git config list > > cannot be asking for the value of the variable "list", because such > a variable cannot exist. When you start your command with > > $ git config list ... > > whatever other tokens follow that token "list" on the command line, > it is a "--list" subcommand. A traditional and lazy "get a single > variable" without an explicit subcommand would look like > > $ git config section.variable > > that is a short-hand to > > $ git config --get section.variable > > and would become > > $ git config get section.variable > > in the new world order. Thanks for the thorough explanation, I have nothing to add! Patrick
On Fri, May 03, 2024 at 06:08:57AM -0700, Karthik Nayak wrote: > Patrick Steinhardt <ps@pks.im> writes: > > > While git-config(1) has several modes, those modes are not exposed with > > subcommands but instead by specifying e.g. `--unset` or `--list`. This > > s/specifying/specifying flags/ perhaps? > > > 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`. > > Tangent: I totally agree with the patch, but it would be nice to have a > 'DesigningCommands' document which would highlight UX do's and don'ts. > It would be nice to add that as reference in discussions. I agree that we should have that discussion, but feel like it should be part of a separate patch series. If I were to adopt that into this series I very much think that the resulting discussions would take quite a while to settle. I'll start the discussion soonish in a separate thread. [snip] > > @@ -2590,7 +2608,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 > > @@ -2751,4 +2769,6 @@ test_expect_success 'specifying multiple modes causes failure' ' > > test_cmp expect err > > ' > > > > +done > > + > > > Nit: Wouldn't it be better if the tests are indented here? That way you > know it's part of a loop. We would basically have to reindent 2500 lines of code. I don't think that'd be a helpful to reviewers :) Patrick
Patrick Steinhardt <ps@pks.im> writes: > On Fri, May 03, 2024 at 06:08:57AM -0700, Karthik Nayak wrote: >> Patrick Steinhardt <ps@pks.im> writes: >> >> > While git-config(1) has several modes, those modes are not exposed with >> > subcommands but instead by specifying e.g. `--unset` or `--list`. This >> >> s/specifying/specifying flags/ perhaps? >> >> > 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`. >> >> Tangent: I totally agree with the patch, but it would be nice to have a >> 'DesigningCommands' document which would highlight UX do's and don'ts. >> It would be nice to add that as reference in discussions. > > I agree that we should have that discussion, but feel like it should be > part of a separate patch series. If I were to adopt that into this > series I very much think that the resulting discussions would take quite > a while to settle. > > I'll start the discussion soonish in a separate thread. > Ofcourse, not part of this series. I was merely using this patch to bring up that topic. Thanks, will stay tuned. > [snip] >> > @@ -2590,7 +2608,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 >> > @@ -2751,4 +2769,6 @@ test_expect_success 'specifying multiple modes causes failure' ' >> > test_cmp expect err >> > ' >> > >> > +done >> > + >> > >> Nit: Wouldn't it be better if the tests are indented here? That way you >> know it's part of a loop. > > We would basically have to reindent 2500 lines of code. I don't think > that'd be a helpful to reviewers :) > > Patrick Fair enough. Karthik
Patrick Steinhardt <ps@pks.im> writes: > ... I was pondering > whether we want to introduce a document as part of that patch series > that starts to keep track of upcoming removals for a potential Git 3.0 > release. Finally somebody has bit it ;-) In the 2.44 cycle, I wrote The RelNotes symbolic link says we are now working towards Git 2.44. It may not be a bad idea to reflect on what technical debt and UI warts we have accumulated so far to see if we have enough of them to start planning for a breaking Git 3.0 release (or, of course, keep incrementally improve the system, which is much more preferrable--- continuity and stability is good). End of year being a relatively quiet period, it may be a good time to think about your favorite pet peeve, to be discussed early next year. in a few of the "What's cooking" reports. > There are multiple items that could be added: > > - Removal of the old syntax of git-config(1). > > - Removal of the dumb HTTP transport. > > - Removal of `info/grafts`. > > There are probably other items. A list of things I can think of that I won't be the primary advocate for but I do not mind too terribly if we had champions for the topics are attached at the end. > In any case, the old actions are here to stay for the foreseeable future > until we commit to a breaking major release. True. > Thanks for the thorough explanation, I have nothing to add! You could have avoided it if you copied some from the initial cover letter to each round (i.e. preparing the series to be read by some folks who did not read an earlier round). Possible additional Git 3.0 items: - Removing "git http-push" to push over HTTP/DAV. - Removing support of `$GIT_DIR/branches/` from remote.c API. - Removing "git update-server-info". - Removing "git annotate". - Removing "gitweb" and "git instaweb". - Removing "git filter-branch", now we have a better alternative "git filter-repo". - Removing discovery of hook script in "$GIT_DIR/hooks/", in favor of the configuration variables that point at them. - Switching to SHA-256 as the default hash algorithm. - Switching to reftable as the default ref backend. - Switching the hardcoded default branch name away from "master" to "main". - Declaring that "git restore" and "git switch" were failed experiments and deprecating them. - Declaring that "git submodule" was a failed experiment and deprecating it.
On Monday, May 6, 2024 1:13 PM, Junio C Hamano wrote: >Patrick Steinhardt <ps@pks.im> writes: > >> ... I was pondering >> whether we want to introduce a document as part of that patch series >> that starts to keep track of upcoming removals for a potential Git 3.0 >> release. > >Finally somebody has bit it ;-) In the 2.44 cycle, I wrote > > The RelNotes symbolic link says we are now working towards Git 2.44. > It may not be a bad idea to reflect on what technical debt and UI > warts we have accumulated so far to see if we have enough of them to > start planning for a breaking Git 3.0 release (or, of course, keep > incrementally improve the system, which is much more preferrable--- > continuity and stability is good). End of year being a relatively > quiet period, it may be a good time to think about your favorite pet > peeve, to be discussed early next year. > >in a few of the "What's cooking" reports. > >> There are multiple items that could be added: >> >> - Removal of the old syntax of git-config(1). >> >> - Removal of the dumb HTTP transport. >> >> - Removal of `info/grafts`. >> >> There are probably other items. > >A list of things I can think of that I won't be the primary advocate for but I do not >mind too terribly if we had champions for the topics are attached at the end. > >> In any case, the old actions are here to stay for the foreseeable >> future until we commit to a breaking major release. > >True. > >> Thanks for the thorough explanation, I have nothing to add! > >You could have avoided it if you copied some from the initial cover letter to each >round (i.e. preparing the series to be read by some folks who did not read an earlier >round). > > >Possible additional Git 3.0 items: > > - Removing "git http-push" to push over HTTP/DAV. > > - Removing support of `$GIT_DIR/branches/` from remote.c API. > > - Removing "git update-server-info". > > - Removing "git annotate". I must have missed this... I thought annotate was replacing blame. > - Removing "gitweb" and "git instaweb". > > - Removing "git filter-branch", now we have a better alternative > "git filter-repo". > > - Removing discovery of hook script in "$GIT_DIR/hooks/", in favor > of the configuration variables that point at them. > > - Switching to SHA-256 as the default hash algorithm. > > - Switching to reftable as the default ref backend. > > - Switching the hardcoded default branch name away from "master" to > "main". > > - Declaring that "git restore" and "git switch" were failed > experiments and deprecating them. This restore and switch have broad use in my community. I do not consider them failed experiments at all, but essential in scripts and usage. Removing these would block migration to git 3, in my view. I have actually been thinking about doing a YouTube video on these commands. > - Declaring that "git submodule" was a failed experiment and > deprecating it. This would be a very high-impact decision. Aside from my community's use (and my own company's dependence on submodules), this will break large numbers of GNU-based projects that use submodules for distribution, including build-aux. I would suggest staying away from this decision. Submodules have definite value. Please keep them. If you really want to get rid of stuff that has limited use instead of submodules, it is worktrees, the benefit of which is reduced given sparce checkouts and fetch depth. --Randall
On 2024-05-06 20:33, rsbecker@nexbridge.com wrote: > On Monday, May 6, 2024 1:13 PM, Junio C Hamano wrote: >> - Declaring that "git restore" and "git switch" were failed >> experiments and deprecating them. > > This restore and switch have broad use in my community. I do not > consider them failed experiments at all, but essential in scripts and > usage. Removing these would block migration to git 3, in my view. I > have actually been thinking about doing a YouTube video on these > commands. Huh, I was also really surprised to see "git switch" and "git restore" on the planned deprecated feature list. For example, I use "git switch" literally all the time, and "git restore" also sees a fair amount of use. Thus, I think that deprecating these two features is a really bad idea. >> - Declaring that "git submodule" was a failed experiment and >> deprecating it. > > This would be a very high-impact decision. Aside from my community's > use (and my own company's dependence on submodules), this will break > large numbers of GNU-based projects that use submodules for > distribution, including build-aux. I would suggest staying away from > this decision. Submodules have definite value. Please keep them. I agree that deprecating "git submodule" isn't a good idea.
Junio C Hamano <gitster@pobox.com> writes: > Patrick Steinhardt <ps@pks.im> writes: > >> ... I was pondering >> whether we want to introduce a document as part of that patch series >> that starts to keep track of upcoming removals for a potential Git 3.0 >> release. > > Finally somebody has bit it ;-) In the 2.44 cycle, I wrote > > The RelNotes symbolic link says we are now working towards Git 2.44. > It may not be a bad idea to reflect on what technical debt and UI > warts we have accumulated so far to see if we have enough of them to > start planning for a breaking Git 3.0 release (or, of course, keep > incrementally improve the system, which is much more preferrable--- > continuity and stability is good). End of year being a relatively > quiet period, it may be a good time to think about your favorite pet > peeve, to be discussed early next year. > > in a few of the "What's cooking" reports. > >> There are multiple items that could be added: >> >> - Removal of the old syntax of git-config(1). >> >> - Removal of the dumb HTTP transport. >> >> - Removal of `info/grafts`. >> >> There are probably other items. > > A list of things I can think of that I won't be the primary advocate > for but I do not mind too terribly if we had champions for the > topics are attached at the end. Just to avoid unnecessary misunderstanding, as I said, what I listed are not my proposals. Rather, take them together with Patrick's list that they are invitations for others to let their imagination go wild, trying to come up as many ideas regardless of how good they are, sort of the initial "brain-storming" phase of a discussion. We will need to cull bad ideas, pick good ones, and refine what we are going to implement in later stages, but this is not a time to shoot down what you do not like, yet. It is instead for you to add your pet peeve.
On Mon, May 06, 2024 at 10:13:25AM -0700, Junio C Hamano wrote: > Patrick Steinhardt <ps@pks.im> writes: > > > ... I was pondering > > whether we want to introduce a document as part of that patch series > > that starts to keep track of upcoming removals for a potential Git 3.0 > > release. > > Finally somebody has bit it ;-) In the 2.44 cycle, I wrote > > The RelNotes symbolic link says we are now working towards Git 2.44. > It may not be a bad idea to reflect on what technical debt and UI > warts we have accumulated so far to see if we have enough of them to > start planning for a breaking Git 3.0 release (or, of course, keep > incrementally improve the system, which is much more preferrable--- > continuity and stability is good). End of year being a relatively > quiet period, it may be a good time to think about your favorite pet > peeve, to be discussed early next year. > > in a few of the "What's cooking" reports. I know :) I have been thinking about this on and off, but never felt like pushing for it yet. > > There are multiple items that could be added: > > > > - Removal of the old syntax of git-config(1). > > > > - Removal of the dumb HTTP transport. > > > > - Removal of `info/grafts`. > > > > There are probably other items. > > A list of things I can think of that I won't be the primary advocate > for but I do not mind too terribly if we had champions for the > topics are attached at the end. I'd be happy to champion for each of those. > > In any case, the old actions are here to stay for the foreseeable future > > until we commit to a breaking major release. > > True. > > > Thanks for the thorough explanation, I have nothing to add! > > You could have avoided it if you copied some from the initial cover > letter to each round (i.e. preparing the series to be read by some > folks who did not read an earlier round). Fair enough. I should have known that this part is indeed quite important to the whole series and included it in the newer cover letters. > > Possible additional Git 3.0 items: Some pretty controversial takes in here ;) I guess that's by design. > - Removing "git http-push" to push over HTTP/DAV. > > - Removing support of `$GIT_DIR/branches/` from remote.c API. > > - Removing "git update-server-info". Yes, all of these are quite sensible. > - Removing "git annotate". I don't care much about this one, but it's nice indeed to get rid of duplicate functionality. > - Removing "gitweb" and "git instaweb". I don't care about this one, to be honest. It's basically unmaintained though as far as I know, so we might just be accepting the status quo. > - Removing "git filter-branch", now we have a better alternative > "git filter-repo". This one I'm sceptical about. The one upside of git-filter-branch(1) is that it's part of Git itself, whereas git-filter-repo(1) is not. I thus think that a prerequisite here should be that we first land the new script in Git before actually deprecating the old tool such that things remain discoverable. And upstreaming git-filter-repo(1) would be a worthy goal by itself already. > - Removing discovery of hook script in "$GIT_DIR/hooks/", in favor > of the configuration variables that point at them. Those have been an attack vector in the past, and I think that using config to set up hooks is quite a bit saner. But of course, we first need to land this topic :) > - Switching to SHA-256 as the default hash algorithm. > > - Switching to reftable as the default ref backend. > > - Switching the hardcoded default branch name away from "master" to > "main". All three of those may be nice. The first one is going to be hardest as it requires support in forges. GitLab started to support SHA256 a month ago, but it's still experimental. But to the best of my knowledge GitHub does not yet support SHA256 at all. > - Declaring that "git restore" and "git switch" were failed > experiments and deprecating them. I use those quite a lot, so it'd be a shame if those went away. > - Declaring that "git submodule" was a failed experiment and > deprecating it. Well. I know that most people think that submodules don't work and should just go away, and I count myself as part of that group. But realistically I don't see that as a feasible goal, even more so because we don't actually have a proper replacement. They may be somewhat broken, but there is no better substitute. You know, let me take this list and propose it as something like "Documents/UpcomingDeprecations". This will make the whole discussion here a lot more discoverable. Patrick
On Mon, May 06, 2024 at 02:33:33PM -0700, Junio C Hamano wrote: > Junio C Hamano <gitster@pobox.com> writes: > > > Patrick Steinhardt <ps@pks.im> writes: > > > >> ... I was pondering > >> whether we want to introduce a document as part of that patch series > >> that starts to keep track of upcoming removals for a potential Git 3.0 > >> release. > > > > Finally somebody has bit it ;-) In the 2.44 cycle, I wrote > > > > The RelNotes symbolic link says we are now working towards Git 2.44. > > It may not be a bad idea to reflect on what technical debt and UI > > warts we have accumulated so far to see if we have enough of them to > > start planning for a breaking Git 3.0 release (or, of course, keep > > incrementally improve the system, which is much more preferrable--- > > continuity and stability is good). End of year being a relatively > > quiet period, it may be a good time to think about your favorite pet > > peeve, to be discussed early next year. > > > > in a few of the "What's cooking" reports. > > > >> There are multiple items that could be added: > >> > >> - Removal of the old syntax of git-config(1). > >> > >> - Removal of the dumb HTTP transport. > >> > >> - Removal of `info/grafts`. > >> > >> There are probably other items. > > > > A list of things I can think of that I won't be the primary advocate > > for but I do not mind too terribly if we had champions for the > > topics are attached at the end. > > Just to avoid unnecessary misunderstanding, as I said, what I listed > are not my proposals. Rather, take them together with Patrick's list > that they are invitations for others to let their imagination go wild, > trying to come up as many ideas regardless of how good they are, sort > of the initial "brain-storming" phase of a discussion. We will need > to cull bad ideas, pick good ones, and refine what we are going to > implement in later stages, but this is not a time to shoot down what > you do not like, yet. It is instead for you to add your pet peeve. Just to avoid starting this discussion deep down in this thread, I plan to send an RFC-style patch later today that proposes several potential deprecations for Git 3.0. The intent of that patch will be to trigger discussion and it is thus by design going to be controversial :) Patrick
On Mon, May 6, 2024, at 20:33, rsbecker@nexbridge.com wrote: > If you really want to get rid of stuff that has limited use instead of > submodules, it is worktrees, the benefit of which is reduced given > sparce checkouts and fetch depth. Git-worktrees is very useful. One use-case is having a dedicated worktree for an old version of the program/app that you need to work on for whatever reason (using the same worktree for very different versions can confuse build systems and IDEs). These things aren’t covered by those other supposed alternatives (which I never use because I don’t work on big repositories). Maybe the idea is to use a fetch depth of 1 on N clones instead of N worktrees. But then you still have clones laying around in an ad hoc manner compared to having on overview of all the worktrees that you are using from one repository.
diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt index ac61113fcc..c83c97cb7e 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>] [--comment=<message>] [--fixed-value] [--show-origin] [--show-scope] [-z|--null] <name> [<value> [<value-pattern>]] 'git config' [<file-option>] [--type=<type>] [--comment=<message>] --add <name> <value> 'git config' [<file-option>] [--type=<type>] [--comment=<message>] [--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 ------- @@ -190,10 +196,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 @@ -248,7 +250,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:: @@ -300,10 +302,20 @@ Valid `<type>`'s include: When using `--get`, and the requested variable is not found, behave as if <value> were the value assigned to 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 59877065f8..f89ddce8da 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; @@ -706,14 +713,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), @@ -737,15 +754,12 @@ 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_STRING(0, "comment", &comment_arg, N_("value"), N_("human-readable comment string (# will be prepended as needed)")), 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(), }; @@ -754,6 +768,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, *comment = NULL; @@ -763,6 +813,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 legacy-style modes that don't use subcommands. + */ + 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 86dc70769a..f77d2f7847 100755 --- a/t/t1300-config.sh +++ b/t/t1300-config.sh @@ -11,6 +11,20 @@ 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="" + ;; +*) + BUG "unknown mode $mode";; +esac + test_expect_success 'setup whitespace config' ' sed -e "s/^|//" \ -e "s/[$]$//" \ @@ -460,11 +474,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 ' @@ -476,7 +490,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 ' @@ -614,17 +628,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 ' @@ -637,6 +651,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 ' @@ -847,7 +862,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 ' @@ -1139,7 +1154,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 ' @@ -1163,7 +1178,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 @@ -1198,6 +1213,7 @@ test_expect_success 'inner whitespace kept verbatim, horizontal tabs and spaces' ' 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 && @@ -1218,10 +1234,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' ' @@ -1478,7 +1495,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' ' @@ -1669,31 +1686,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' ' @@ -1733,7 +1750,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 ' @@ -1742,7 +1759,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 ' @@ -2093,7 +2110,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 ' @@ -2110,7 +2127,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. @@ -2124,7 +2141,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 ' @@ -2162,7 +2179,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 ' @@ -2170,7 +2187,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 ' @@ -2197,7 +2214,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 ) ' @@ -2213,7 +2230,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 ) ' @@ -2239,13 +2256,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 ' @@ -2254,7 +2272,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 ' @@ -2264,7 +2282,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 ' @@ -2288,7 +2306,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 ' @@ -2329,7 +2347,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 && @@ -2338,20 +2356,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 ' @@ -2478,7 +2496,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 @@ -2491,7 +2509,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 @@ -2501,7 +2519,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 @@ -2516,7 +2534,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 @@ -2534,7 +2552,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 && @@ -2555,7 +2573,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 @@ -2564,7 +2582,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 @@ -2582,7 +2600,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 @@ -2590,7 +2608,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 @@ -2751,4 +2769,6 @@ test_expect_success 'specifying multiple modes causes failure' ' test_cmp expect err ' +done + test_done
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. As tests are now run twice, state from the first run may be still be around in the second run and thus cause tests to fail. Add cleanup logic as required to fix such tests. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- Documentation/git-config.txt | 26 ++++++--- builtin/config.c | 90 ++++++++++++++++++++++++---- t/t1300-config.sh | 110 +++++++++++++++++++++-------------- 3 files changed, 162 insertions(+), 64 deletions(-)