{"thread":{"id":"61486","subject":"[PATCH] completion: adapt git-config(1) to complete subcommands","startedAt":"2024-05-16T04:56:59Z","lastAt":"2024-05-21T06:23:37Z","messageCount":6,"participants":["Patrick Steinhardt","Junio C Hamano","Rubén Justo"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"494851","messageId":"e0039edb9b7dce1e33c4cb6e964c50281c815e97.1715835356.git.ps@pks.im","threadId":"61486","inReplyTo":null,"subject":"[PATCH] completion: adapt git-config(1) to complete subcommands","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-05-16T04:56:54Z","receivedAt":"2024-05-16T04:56:59Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"With fe3ccc7aab (Merge branch 'ps/config-subcommands', 2024-05-15),\ngit-config(1) has gained support for subcommands. These subcommands live\nnext to the old, action-based mode, so that both the old and new way\ncontinue to work.\n\nThe manpage for this command has been updated to prominently show the\nsubcommands, and the action-based modes are marked as deprecated. Update\nBash completion scripts accordingly to advertise subcommands instead of\nactions.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n\nI have based this on top of 19fe900cfc (The fourth batch, 2024-05-15),\nwhich contains the config subcommands topic by now.\n\n contrib/completion/git-completion.bash | 38 ++++++++++++-----\n t/t9902-completion.sh                  | 56 +++++++++++++++++++-------\n 2 files changed, 69 insertions(+), 25 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 5c0ddeb3d4..ab370f8b49 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -2989,22 +2989,38 @@ __git_complete_config_variable_name_and_value ()\n \n _git_config ()\n {\n-\tcase \"$prev\" in\n-\t--get|--get-all|--unset|--unset-all)\n-\t\t__gitcomp_nl \"$(__git_config_get_set_variables)\"\n+\tlocal subcommands=\"list get set unset rename-section remove-section edit\"\n+\tlocal subcommand=\"$(__git_find_on_cmdline \"$subcommands\")\"\n+\n+\tif [ -z \"$subcommand\" ]\n+\tthen\n+\t\t__gitcomp \"$subcommands\"\n \t\treturn\n-\t\t;;\n-\t*.*)\n-\t\t__git_complete_config_variable_value\n+\tfi\n+\n+\tcase \"$cur\" in\n+\t--*)\n+\t\t__gitcomp_builtin \"config_$subcommand\"\n \t\treturn\n \t\t;;\n \tesac\n-\tcase \"$cur\" in\n-\t--*)\n-\t\t__gitcomp_builtin config\n+\n+\tcase \"$subcommand\" in\n+\tget)\n+\t\t__gitcomp_nl \"$(__git_config_get_set_variables)\"\n \t\t;;\n-\t*)\n-\t\t__git_complete_config_variable_name\n+\tset)\n+\t\tcase \"$prev\" in\n+\t\t*.*)\n+\t\t\t__git_complete_config_variable_value\n+\t\t\t;;\n+\t\t*)\n+\t\t\t__git_complete_config_variable_name\n+\t\t\t;;\n+\t\tesac\n+\t\t;;\n+\tunset)\n+\t\t__gitcomp_nl \"$(__git_config_get_set_variables)\"\n \t\t;;\n \tesac\n }\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex 963f865f27..64c9979423 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -2742,30 +2742,58 @@ do\n \t'\n done\n \n-test_expect_success 'git config - section' '\n-\ttest_completion \"git config br\" <<-\\EOF\n+test_expect_success 'git config subcommand' '\n+\ttest_completion \"git config \" <<-\\EOF\n+\tedit Z\n+\tget Z\n+\tlist Z\n+\tremove-section Z\n+\trename-section Z\n+\tset Z\n+\tunset Z\n+\tEOF\n+'\n+\n+test_expect_success 'git config subcommand options' '\n+\ttest_completion \"git config get --show-\" <<-\\EOF\n+\t--show-names Z\n+\t--show-origin Z\n+\t--show-scope Z\n+\tEOF\n+'\n+\n+test_expect_success 'git config get' '\n+\ttest_when_finished \"rm -f cfgfile\" &&\n+\tgit config set --file cfgfile foo.bar baz &&\n+\ttest_completion \"git config get --file cfgfile foo.\" <<-\\EOF\n+\tfoo.bar Z\n+\tEOF\n+'\n+\n+test_expect_success 'git config set - section' '\n+\ttest_completion \"git config set br\" <<-\\EOF\n \tbranch.Z\n \tbrowser.Z\n \tEOF\n '\n \n-test_expect_success 'git config - section include, includeIf' '\n-\ttest_completion \"git config inclu\" <<-\\EOF\n+test_expect_success 'git config set - section include, includeIf' '\n+\ttest_completion \"git config set inclu\" <<-\\EOF\n \tinclude.Z\n \tincludeIf.Z\n \tEOF\n '\n \n-test_expect_success 'git config - variable name' '\n-\ttest_completion \"git config log.d\" <<-\\EOF\n+test_expect_success 'git config set - variable name' '\n+\ttest_completion \"git config set log.d\" <<-\\EOF\n \tlog.date Z\n \tlog.decorate Z\n \tlog.diffMerges Z\n \tEOF\n '\n \n-test_expect_success 'git config - variable name include' '\n-\ttest_completion \"git config include.p\" <<-\\EOF\n+test_expect_success 'git config set - variable name include' '\n+\ttest_completion \"git config set include.p\" <<-\\EOF\n \tinclude.path Z\n \tEOF\n '\n@@ -2776,8 +2804,8 @@ test_expect_success 'setup for git config submodule tests' '\n \tgit submodule add ./sub\n '\n \n-test_expect_success 'git config - variable name - submodule and __git_compute_first_level_config_vars_for_section' '\n-\ttest_completion \"git config submodule.\" <<-\\EOF\n+test_expect_success 'git config set - variable name - submodule and __git_compute_first_level_config_vars_for_section' '\n+\ttest_completion \"git config set submodule.\" <<-\\EOF\n \tsubmodule.active Z\n \tsubmodule.alternateErrorStrategy Z\n \tsubmodule.alternateLocation Z\n@@ -2788,8 +2816,8 @@ test_expect_success 'git config - variable name - submodule and __git_compute_fi\n \tEOF\n '\n \n-test_expect_success 'git config - variable name - __git_compute_second_level_config_vars_for_section' '\n-\ttest_completion \"git config submodule.sub.\" <<-\\EOF\n+test_expect_success 'git config set - variable name - __git_compute_second_level_config_vars_for_section' '\n+\ttest_completion \"git config set submodule.sub.\" <<-\\EOF\n \tsubmodule.sub.url Z\n \tsubmodule.sub.update Z\n \tsubmodule.sub.branch Z\n@@ -2799,8 +2827,8 @@ test_expect_success 'git config - variable name - __git_compute_second_level_con\n \tEOF\n '\n \n-test_expect_success 'git config - value' '\n-\ttest_completion \"git config color.pager \" <<-\\EOF\n+test_expect_success 'git config set - value' '\n+\ttest_completion \"git config set color.pager \" <<-\\EOF\n \tfalse Z\n \ttrue Z\n \tEOF\n-- \n2.45.1.190.g19fe900cfc.dirty\n\n"},{"id":"494896","messageId":"xmqqv83dsqhn.fsf@gitster.g","threadId":"61486","inReplyTo":"e0039edb9b7dce1e33c4cb6e964c50281c815e97.1715835356.git.ps@pks.im","subject":"Re: [PATCH] completion: adapt git-config(1) to complete subcommands","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-05-16T15:53:08Z","receivedAt":"2024-05-16T15:53:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> +\tlocal subcommands=\"list get set unset rename-section remove-section edit\"\n\nWouldn't \"git config --git-completion-helper[-all]\" work here?\n"},{"id":"494925","messageId":"ZkbzkYhJVO-uEeX5@tanuki","threadId":"61486","inReplyTo":"xmqqv83dsqhn.fsf@gitster.g","subject":"Re: [PATCH] completion: adapt git-config(1) to complete subcommands","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-05-17T06:05:05Z","receivedAt":"2024-05-17T06:05:11Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, May 16, 2024 at 08:53:08AM -0700, Junio C Hamano wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> \n> > +\tlocal subcommands=\"list get set unset rename-section remove-section edit\"\n> \n> Wouldn't \"git config --git-completion-helper[-all]\" work here?\n\nI knew there was something here, but I just wasn't able to find it\nyesterday.\n\nWill fix, thanks!\n\nPatrick\n"},{"id":"494926","messageId":"8d43dee33289969a5afbbf7635ac40b7312d8e19.1715926344.git.ps@pks.im","threadId":"61486","inReplyTo":"e0039edb9b7dce1e33c4cb6e964c50281c815e97.1715835356.git.ps@pks.im","subject":"[PATCH v2] completion: adapt git-config(1) to complete subcommands","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-05-17T06:13:36Z","receivedAt":"2024-05-17T06:13:42Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"With fe3ccc7aab (Merge branch 'ps/config-subcommands', 2024-05-15),\ngit-config(1) has gained support for subcommands. These subcommands live\nnext to the old, action-based mode, so that both the old and new way\ncontinue to work.\n\nThe manpage for this command has been updated to prominently show the\nsubcommands, and the action-based modes are marked as deprecated. Update\nBash completion scripts accordingly to advertise subcommands instead of\nactions.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\nRange-diff against v1:\n1:  e0039edb9b ! 1:  8d43dee332 completion: adapt git-config(1) to complete subcommands\n    @@ contrib/completion/git-completion.bash: __git_complete_config_variable_name_and_\n     -\tcase \"$prev\" in\n     -\t--get|--get-all|--unset|--unset-all)\n     -\t\t__gitcomp_nl \"$(__git_config_get_set_variables)\"\n    -+\tlocal subcommands=\"list get set unset rename-section remove-section edit\"\n    -+\tlocal subcommand=\"$(__git_find_on_cmdline \"$subcommands\")\"\n    ++\tlocal subcommands subcommand\n    ++\n    ++\t__git_resolve_builtins \"config\"\n    ++\n    ++\tsubcommands=\"$___git_resolved_builtins\"\n    ++\tsubcommand=\"$(__git_find_subcommand \"$subcommands\")\"\n     +\n     +\tif [ -z \"$subcommand\" ]\n     +\tthen\n\n contrib/completion/git-completion.bash | 42 ++++++++++++++-----\n t/t9902-completion.sh                  | 56 +++++++++++++++++++-------\n 2 files changed, 73 insertions(+), 25 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 5c0ddeb3d4..60a22d619a 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -2989,22 +2989,42 @@ __git_complete_config_variable_name_and_value ()\n \n _git_config ()\n {\n-\tcase \"$prev\" in\n-\t--get|--get-all|--unset|--unset-all)\n-\t\t__gitcomp_nl \"$(__git_config_get_set_variables)\"\n+\tlocal subcommands subcommand\n+\n+\t__git_resolve_builtins \"config\"\n+\n+\tsubcommands=\"$___git_resolved_builtins\"\n+\tsubcommand=\"$(__git_find_subcommand \"$subcommands\")\"\n+\n+\tif [ -z \"$subcommand\" ]\n+\tthen\n+\t\t__gitcomp \"$subcommands\"\n \t\treturn\n-\t\t;;\n-\t*.*)\n-\t\t__git_complete_config_variable_value\n+\tfi\n+\n+\tcase \"$cur\" in\n+\t--*)\n+\t\t__gitcomp_builtin \"config_$subcommand\"\n \t\treturn\n \t\t;;\n \tesac\n-\tcase \"$cur\" in\n-\t--*)\n-\t\t__gitcomp_builtin config\n+\n+\tcase \"$subcommand\" in\n+\tget)\n+\t\t__gitcomp_nl \"$(__git_config_get_set_variables)\"\n \t\t;;\n-\t*)\n-\t\t__git_complete_config_variable_name\n+\tset)\n+\t\tcase \"$prev\" in\n+\t\t*.*)\n+\t\t\t__git_complete_config_variable_value\n+\t\t\t;;\n+\t\t*)\n+\t\t\t__git_complete_config_variable_name\n+\t\t\t;;\n+\t\tesac\n+\t\t;;\n+\tunset)\n+\t\t__gitcomp_nl \"$(__git_config_get_set_variables)\"\n \t\t;;\n \tesac\n }\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex 963f865f27..64c9979423 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -2742,30 +2742,58 @@ do\n \t'\n done\n \n-test_expect_success 'git config - section' '\n-\ttest_completion \"git config br\" <<-\\EOF\n+test_expect_success 'git config subcommand' '\n+\ttest_completion \"git config \" <<-\\EOF\n+\tedit Z\n+\tget Z\n+\tlist Z\n+\tremove-section Z\n+\trename-section Z\n+\tset Z\n+\tunset Z\n+\tEOF\n+'\n+\n+test_expect_success 'git config subcommand options' '\n+\ttest_completion \"git config get --show-\" <<-\\EOF\n+\t--show-names Z\n+\t--show-origin Z\n+\t--show-scope Z\n+\tEOF\n+'\n+\n+test_expect_success 'git config get' '\n+\ttest_when_finished \"rm -f cfgfile\" &&\n+\tgit config set --file cfgfile foo.bar baz &&\n+\ttest_completion \"git config get --file cfgfile foo.\" <<-\\EOF\n+\tfoo.bar Z\n+\tEOF\n+'\n+\n+test_expect_success 'git config set - section' '\n+\ttest_completion \"git config set br\" <<-\\EOF\n \tbranch.Z\n \tbrowser.Z\n \tEOF\n '\n \n-test_expect_success 'git config - section include, includeIf' '\n-\ttest_completion \"git config inclu\" <<-\\EOF\n+test_expect_success 'git config set - section include, includeIf' '\n+\ttest_completion \"git config set inclu\" <<-\\EOF\n \tinclude.Z\n \tincludeIf.Z\n \tEOF\n '\n \n-test_expect_success 'git config - variable name' '\n-\ttest_completion \"git config log.d\" <<-\\EOF\n+test_expect_success 'git config set - variable name' '\n+\ttest_completion \"git config set log.d\" <<-\\EOF\n \tlog.date Z\n \tlog.decorate Z\n \tlog.diffMerges Z\n \tEOF\n '\n \n-test_expect_success 'git config - variable name include' '\n-\ttest_completion \"git config include.p\" <<-\\EOF\n+test_expect_success 'git config set - variable name include' '\n+\ttest_completion \"git config set include.p\" <<-\\EOF\n \tinclude.path Z\n \tEOF\n '\n@@ -2776,8 +2804,8 @@ test_expect_success 'setup for git config submodule tests' '\n \tgit submodule add ./sub\n '\n \n-test_expect_success 'git config - variable name - submodule and __git_compute_first_level_config_vars_for_section' '\n-\ttest_completion \"git config submodule.\" <<-\\EOF\n+test_expect_success 'git config set - variable name - submodule and __git_compute_first_level_config_vars_for_section' '\n+\ttest_completion \"git config set submodule.\" <<-\\EOF\n \tsubmodule.active Z\n \tsubmodule.alternateErrorStrategy Z\n \tsubmodule.alternateLocation Z\n@@ -2788,8 +2816,8 @@ test_expect_success 'git config - variable name - submodule and __git_compute_fi\n \tEOF\n '\n \n-test_expect_success 'git config - variable name - __git_compute_second_level_config_vars_for_section' '\n-\ttest_completion \"git config submodule.sub.\" <<-\\EOF\n+test_expect_success 'git config set - variable name - __git_compute_second_level_config_vars_for_section' '\n+\ttest_completion \"git config set submodule.sub.\" <<-\\EOF\n \tsubmodule.sub.url Z\n \tsubmodule.sub.update Z\n \tsubmodule.sub.branch Z\n@@ -2799,8 +2827,8 @@ test_expect_success 'git config - variable name - __git_compute_second_level_con\n \tEOF\n '\n \n-test_expect_success 'git config - value' '\n-\ttest_completion \"git config color.pager \" <<-\\EOF\n+test_expect_success 'git config set - value' '\n+\ttest_completion \"git config set color.pager \" <<-\\EOF\n \tfalse Z\n \ttrue Z\n \tEOF\n\nbase-commit: 19fe900cfce8096b7645ec9611a0b981f6bbd154\n-- \n2.45.1.190.g19fe900cfc.dirty\n\n"},{"id":"494981","messageId":"c48488d1-3135-493d-a9e8-2f787bdd2b4c@gmail.com","threadId":"61486","inReplyTo":"8d43dee33289969a5afbbf7635ac40b7312d8e19.1715926344.git.ps@pks.im","subject":"Re: [PATCH v2] completion: adapt git-config(1) to complete subcommands","fromName":"Rubén Justo","fromEmail":"rjusto@gmail.com","sentAt":"2024-05-17T16:27:54Z","receivedAt":"2024-05-17T16:27:57Z","isPatch":true,"sender":{"key":"rjusto@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5685487?v=4"},"body":"On Fri, May 17, 2024 at 08:13:36AM +0200, Patrick Steinhardt wrote:\n\n>     ++\t__git_resolve_builtins \"config\"\n\nThe __git_resolve_builtins() function executes \"git config\n--git-completion-helper\" and caches the result for future calls.  And\non return ...\n\n>     ++\n>     ++\tsubcommands=\"$___git_resolved_builtins\"\n\n... it populates the ___git_resolved_builtins variable with the result:\nthe available subcommands for \"git config\".\n\n>     ++\tsubcommand=\"$(__git_find_subcommand \"$subcommands\")\"\n\nThen, we look for a subcommand among those returned, at\n${words[__git_cmd_idx+1]}, where a possible command must reside.\n\nNicely done.  This looks good to me.\n\nI wonder, if we might consider the possibility of having \"list\" as\na default command:\n\n-\tsubcommand=\"$(__git_find_subcommand \"$subcommands\")\"\n+\tsubcommand=\"$(__git_find_subcommand \"$subcommands\" list)\"\n\nThese lines are only meant to express the idea, as other changes are\nalso necessary and the documentation needs to be updated.  Of course, it\ncould be done in a future series.\n\nI think that \"git config -h\" is an intuitive enough way to offer the\nhelp text and that using 'git config' as a shortcut for 'git config\nlist' can be convenient.\n\nBy the way, having used '__git_find_subcommand' instead of\n'__git_find_on_cmdline' is reassuring when it comes to having a default\nsubcommand :-)\n\nAnyway, as I said, this series looks good to me.  Thanks!\n"},{"id":"495145","messageId":"Zkw95Ev9DRhCWLq1@tanuki","threadId":"61486","inReplyTo":"c48488d1-3135-493d-a9e8-2f787bdd2b4c@gmail.com","subject":"Re: [PATCH v2] completion: adapt git-config(1) to complete subcommands","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-05-21T06:23:32Z","receivedAt":"2024-05-21T06:23:37Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, May 17, 2024 at 06:27:54PM +0200, Rubén Justo wrote:\n> On Fri, May 17, 2024 at 08:13:36AM +0200, Patrick Steinhardt wrote:\n[snip]\n> I wonder, if we might consider the possibility of having \"list\" as\n> a default command:\n> \n> -\tsubcommand=\"$(__git_find_subcommand \"$subcommands\")\"\n> +\tsubcommand=\"$(__git_find_subcommand \"$subcommands\" list)\"\n> \n> These lines are only meant to express the idea, as other changes are\n> also necessary and the documentation needs to be updated.  Of course, it\n> could be done in a future series.\n> \n> I think that \"git config -h\" is an intuitive enough way to offer the\n> help text and that using 'git config' as a shortcut for 'git config\n> list' can be convenient.\n\nHm. I don't really know whether it is sensible to second-guess what the\nuser wants to do. They may want to list variables, but they may just as\nwell not want to do that. I myself use tab completion to learn about\nwhich subcommands exist quite often, even though there is `-h` to do\nthat for me. So I think I lean more towards not having a default\nsubcommand here.\n\n> By the way, having used '__git_find_subcommand' instead of\n> '__git_find_on_cmdline' is reassuring when it comes to having a default\n> subcommand :-)\n> \n> Anyway, as I said, this series looks good to me.  Thanks!\n\nThanks for your review!\n\nPatrick\n"}]}