From b40845293b57aa73d319aa1cec4184f114d8ce65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Fri, 10 Sep 2021 13:28:42 +0200 Subject: [PATCH 1/9] help: correct the usage string in -h and documentation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clarify the usage string in the documentation so we group e.g. -i and --info, and add the missing short options to the "-h" output. The alignment of the second line is off now, but will be fixed with another series of mine[1]. In the meantime let's just assume that fix will make it in eventually for the purposes of this patch, if it's misaligned for a bit it doesn't matter much. 1. https://lore.kernel.org/git/cover-0.2-00000000000-20210901T110917Z-avarab@gmail.com Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- Documentation/git-help.txt | 2 +- builtin/help.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Documentation/git-help.txt b/Documentation/git-help.txt index 44fe8860b3..568a0b606f 100644 --- a/Documentation/git-help.txt +++ b/Documentation/git-help.txt @@ -9,7 +9,7 @@ SYNOPSIS -------- [verse] 'git help' [-a|--all [--[no-]verbose]] [-g|--guides] - [-i|--info|-m|--man|-w|--web] [COMMAND|GUIDE] + [[-i|--info] [-m|--man] [-w|--web]] [COMMAND|GUIDE] DESCRIPTION ----------- diff --git a/builtin/help.c b/builtin/help.c index b7eec06c3d..44ea2798cd 100644 --- a/builtin/help.c +++ b/builtin/help.c @@ -59,7 +59,8 @@ static struct option builtin_help_options[] = { }; static const char * const builtin_help_usage[] = { - N_("git help [--all] [--guides] [--man | --web | --info] []"), + N_("git help [-a|--all] [-g|--guides] [--[no-]verbose]]\n" + " [[-i|--info] [-m|--man] [-w|--web]] []"), NULL }; From 9856ea6785c3dcd19ffb3946c6a9adbd16d9c1da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Wed, 22 Sep 2021 00:40:32 +0200 Subject: [PATCH 2/9] help: correct usage & behavior of "git help --guides" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As noted in 65f98358c0c (builtin/help.c: add --guide option, 2013-04-02) and a133737b809 (doc: include --guide option description for "git help", 2013-04-02) which introduced the --guide option, it cannot be combined with e.g. . Change the command and the "SYNOPSIS" section to reflect that desired behavior. Now that we assert this in code we don't need to exhaustively describe the previous confusing behavior in the documentation either, instead of silently ignoring the provided argument we'll now error out. The "We're done. Ignore any remaining args" comment added in 15f7d494380 (builtin/help.c: split "-a" processing into two, 2013-04-02) can now be removed, it's obvious that we're asserting the behavior with the check of "argc". The "--config" option is still missing from the synopsis, it will be added in a subsequent commit where we'll fix bugs in its implementation. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- Documentation/git-help.txt | 6 +++--- builtin/help.c | 11 +++++++---- t/t0012-help.sh | 4 ++++ 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/Documentation/git-help.txt b/Documentation/git-help.txt index 568a0b606f..cb8e3d4da9 100644 --- a/Documentation/git-help.txt +++ b/Documentation/git-help.txt @@ -8,8 +8,9 @@ git-help - Display help information about Git SYNOPSIS -------- [verse] -'git help' [-a|--all [--[no-]verbose]] [-g|--guides] +'git help' [-a|--all [--[no-]verbose]] [[-i|--info] [-m|--man] [-w|--web]] [COMMAND|GUIDE] +'git help' [-g|--guides] DESCRIPTION ----------- @@ -58,8 +59,7 @@ OPTIONS -g:: --guides:: - Prints a list of the Git concept guides on the standard output. This - option overrides any given command or guide name. + Prints a list of the Git concept guides on the standard output. -i:: --info:: diff --git a/builtin/help.c b/builtin/help.c index 44ea2798cd..51b18c291d 100644 --- a/builtin/help.c +++ b/builtin/help.c @@ -59,8 +59,9 @@ static struct option builtin_help_options[] = { }; static const char * const builtin_help_usage[] = { - N_("git help [-a|--all] [-g|--guides] [--[no-]verbose]]\n" + N_("git help [-a|--all] [--[no-]verbose]]\n" " [[-i|--info] [-m|--man] [-w|--web]] []"), + N_("git help [-g|--guides]"), NULL }; @@ -552,6 +553,11 @@ int cmd_help(int argc, const char **argv, const char *prefix) builtin_help_usage, 0); parsed_help_format = help_format; + /* Options that take no further arguments */ + if (argc && show_guides) + usage_msg_opt(_("--guides cannot be combined with other options"), + builtin_help_usage, builtin_help_options); + if (show_all) { git_config(git_help_config, NULL); if (verbose) { @@ -582,9 +588,6 @@ int cmd_help(int argc, const char **argv, const char *prefix) if (show_all || show_guides) { printf("%s\n", _(git_more_info_string)); - /* - * We're done. Ignore any remaining args - */ return 0; } diff --git a/t/t0012-help.sh b/t/t0012-help.sh index 5679e29c62..0525ec3ee5 100755 --- a/t/t0012-help.sh +++ b/t/t0012-help.sh @@ -34,6 +34,10 @@ test_expect_success 'basic help commands' ' git help -a >/dev/null ' +test_expect_success 'invalid usage' ' + test_expect_code 129 git help -g add +' + test_expect_success "works for commands and guides by default" ' configure_help && git help status && From ff76fc841f4787e7c91329ecda280f293100eccb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Wed, 22 Sep 2021 00:40:33 +0200 Subject: [PATCH 3/9] help tests: add test for --config output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a missing test for checking what the --config output added in ac68a93fd2 (help: add --config to list all available config, 2018-05-26) looks like. We should not be emitting anything except config variables and the brief usage information at the end here. The second test regexp here might not match three-level variables in general, as their second level could contain ".", but in this case we're always emitting what we extract from the documentation, so it's all strings like: foo..bar If we did introduce something like variable example content here we'd like this to break, since we'd then be likely to break the git-completion.bash. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- t/t0012-help.sh | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/t/t0012-help.sh b/t/t0012-help.sh index 0525ec3ee5..63c4adb99b 100755 --- a/t/t0012-help.sh +++ b/t/t0012-help.sh @@ -77,6 +77,19 @@ test_expect_success 'git help -g' ' test_i18ngrep "^ tutorial " help.output ' +test_expect_success 'git help -c' ' + git help -c >help.output && + cat >expect <<-\EOF && + + '\''git help config'\'' for more information + EOF + grep -v -E \ + -e "^[^.]+\.[^.]+$" \ + -e "^[^.]+\.[^.]+\.[^.]+$" \ + help.output >actual && + test_cmp expect actual +' + test_expect_success 'generate builtin list' ' git --list-cmds=builtins >builtins ' From 1ed4bef6b438d25ce605f6bdefb4c98569dad137 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Wed, 22 Sep 2021 00:40:34 +0200 Subject: [PATCH 4/9] help: correct logic error in combining --all and --config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix a bug in the --config option that's been there ever since its introduction in 3ac68a93fd2 (help: add --config to list all available config, 2018-05-26). Die when --all and --config are combined, combining them doesn't make sense. The code for the --config option when combined with an earlier refactoring done to support the --guide option in 65f98358c0c (builtin/help.c: add --guide option, 2013-04-02) would cause us to take the "--all" branch early and ignore the --config option. Let's instead list these as incompatible, both in the synopsis and help output, and enforce it in the code itself. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- Documentation/git-help.txt | 1 + builtin/help.c | 31 ++++++++++++++++++++++--------- t/t0012-help.sh | 3 ++- 3 files changed, 25 insertions(+), 10 deletions(-) diff --git a/Documentation/git-help.txt b/Documentation/git-help.txt index cb8e3d4da9..96d5f598b4 100644 --- a/Documentation/git-help.txt +++ b/Documentation/git-help.txt @@ -11,6 +11,7 @@ SYNOPSIS 'git help' [-a|--all [--[no-]verbose]] [[-i|--info] [-m|--man] [-w|--web]] [COMMAND|GUIDE] 'git help' [-g|--guides] +'git help' [-c|--config] DESCRIPTION ----------- diff --git a/builtin/help.c b/builtin/help.c index 51b18c291d..d0c9605dbb 100644 --- a/builtin/help.c +++ b/builtin/help.c @@ -62,6 +62,7 @@ static const char * const builtin_help_usage[] = { N_("git help [-a|--all] [--[no-]verbose]]\n" " [[-i|--info] [-m|--man] [-w|--web]] []"), N_("git help [-g|--guides]"), + N_("git help [-c|--config]"), NULL }; @@ -553,9 +554,21 @@ int cmd_help(int argc, const char **argv, const char *prefix) builtin_help_usage, 0); parsed_help_format = help_format; + /* Incompatible options */ + if (show_all && show_config) + usage_msg_opt(_("--config and --all cannot be combined"), + builtin_help_usage, builtin_help_options); + + if (show_config && show_guides) + usage_msg_opt(_("--config and --guides cannot be combined"), + builtin_help_usage, builtin_help_options); + /* Options that take no further arguments */ + if (argc && show_config) + usage_msg_opt(_("--config cannot be combined with command or guide names"), + builtin_help_usage, builtin_help_options); if (argc && show_guides) - usage_msg_opt(_("--guides cannot be combined with other options"), + usage_msg_opt(_("--guides cannot be combined with command or guide names"), builtin_help_usage, builtin_help_options); if (show_all) { @@ -570,6 +583,14 @@ int cmd_help(int argc, const char **argv, const char *prefix) list_commands(colopts, &main_cmds, &other_cmds); } + if (show_guides) + list_guides_help(); + + if (show_all || show_guides) { + printf("%s\n", _(git_more_info_string)); + return 0; + } + if (show_config) { int for_human = show_config == 1; @@ -583,14 +604,6 @@ int cmd_help(int argc, const char **argv, const char *prefix) return 0; } - if (show_guides) - list_guides_help(); - - if (show_all || show_guides) { - printf("%s\n", _(git_more_info_string)); - return 0; - } - if (!argv[0]) { printf(_("usage: %s%s"), _(git_usage_string), "\n\n"); list_common_cmds_help(); diff --git a/t/t0012-help.sh b/t/t0012-help.sh index 63c4adb99b..b4ed6229ed 100755 --- a/t/t0012-help.sh +++ b/t/t0012-help.sh @@ -35,7 +35,8 @@ test_expect_success 'basic help commands' ' ' test_expect_success 'invalid usage' ' - test_expect_code 129 git help -g add + test_expect_code 129 git help -g add && + test_expect_code 129 git help -a -c ' test_expect_success "works for commands and guides by default" ' From 0a5940fbe7e453652266e765509a576e4df333c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Wed, 22 Sep 2021 00:40:35 +0200 Subject: [PATCH 5/9] help: correct logic error in combining --all and --guides MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The --all and --guides commands could be combined, which wouldn't have any impact on the output except for: git help --all --guides --no-verbose Listing the guide alongside that output was clearly not intended, so let's error out here. See 002b726a400 (builtin/help.c: add list_common_guides_help() function, 2013-04-02) for the initial implementation. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/help.c | 4 ++++ t/t0012-help.sh | 7 ++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/builtin/help.c b/builtin/help.c index d0c9605dbb..30f160a466 100644 --- a/builtin/help.c +++ b/builtin/help.c @@ -559,6 +559,10 @@ int cmd_help(int argc, const char **argv, const char *prefix) usage_msg_opt(_("--config and --all cannot be combined"), builtin_help_usage, builtin_help_options); + if (show_all && show_guides) + usage_msg_opt(_("--config and --guides cannot be combined"), + builtin_help_usage, builtin_help_options); + if (show_config && show_guides) usage_msg_opt(_("--config and --guides cannot be combined"), builtin_help_usage, builtin_help_options); diff --git a/t/t0012-help.sh b/t/t0012-help.sh index b4ed6229ed..69e385d3b6 100755 --- a/t/t0012-help.sh +++ b/t/t0012-help.sh @@ -36,7 +36,12 @@ test_expect_success 'basic help commands' ' test_expect_success 'invalid usage' ' test_expect_code 129 git help -g add && - test_expect_code 129 git help -a -c + test_expect_code 129 git help -a -c && + + test_expect_code 129 git help -g add && + test_expect_code 129 git help -a -g && + + test_expect_code 129 git help -g -c ' test_expect_success "works for commands and guides by default" ' From d35d03cf93ef0dba3e975c78fce73db91d52ba42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Wed, 22 Sep 2021 00:40:36 +0200 Subject: [PATCH 6/9] help: simplify by moving to OPT_CMDMODE() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As preceding commits have incrementally established all of the --all, --guides, --config and hidden --config-for-completion options are mutually exclusive. So let's use OPT_CMDMODE() to parse the command-line instead, and take advantage of its conflicting options detection. This is the first command with a hidden CMDMODE, so let's introduce a OPT_CMDMODE_F() macro to go along with OPT_CMDMODE(). I think this makes the usage information that we emit slightly worse, e.g. before we'd emit: $ git help --all --config fatal: --config and --all cannot be combined usage: git help [-a|--all] [--[no-]verbose]] [[-i|--info] [-m|--man] [-w|--web]] [] or: git help [-g|--guides] or: git help [-c|--config] [...] $ And now: $ git help --all --config error: option `config' is incompatible with --all $ But improving that is a general topic for parse-options.c improvement, i.e. we should probably emit the full usage in that case. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/help.c | 81 ++++++++++++++++++++++--------------------------- parse-options.h | 6 ++-- 2 files changed, 40 insertions(+), 47 deletions(-) diff --git a/builtin/help.c b/builtin/help.c index 30f160a466..6a022d9803 100644 --- a/builtin/help.c +++ b/builtin/help.c @@ -34,27 +34,36 @@ enum help_format { HELP_FORMAT_WEB }; -static const char *html_path; +static enum help_action { + HELP_ACTION_ALL = 1, + HELP_ACTION_GUIDES, + HELP_ACTION_CONFIG, + HELP_ACTION_CONFIG_FOR_COMPLETION, +} cmd_mode; -static int show_all = 0; -static int show_guides = 0; -static int show_config; +static const char *html_path; static int verbose = 1; static unsigned int colopts; static enum help_format help_format = HELP_FORMAT_NONE; static int exclude_guides; static struct option builtin_help_options[] = { - OPT_BOOL('a', "all", &show_all, N_("print all available commands")), + OPT_CMDMODE('a', "all", &cmd_mode, N_("print all available commands"), + HELP_ACTION_ALL), OPT_HIDDEN_BOOL(0, "exclude-guides", &exclude_guides, N_("exclude guides")), - OPT_BOOL('g', "guides", &show_guides, N_("print list of useful guides")), - OPT_BOOL('c', "config", &show_config, N_("print all configuration variable names")), - OPT_SET_INT_F(0, "config-for-completion", &show_config, "", 2, PARSE_OPT_HIDDEN), OPT_SET_INT('m', "man", &help_format, N_("show man page"), HELP_FORMAT_MAN), OPT_SET_INT('w', "web", &help_format, N_("show manual in web browser"), HELP_FORMAT_WEB), OPT_SET_INT('i', "info", &help_format, N_("show info page"), HELP_FORMAT_INFO), OPT__VERBOSE(&verbose, N_("print command description")), + + OPT_CMDMODE('g', "guides", &cmd_mode, N_("print list of useful guides"), + HELP_ACTION_GUIDES), + OPT_CMDMODE('c', "config", &cmd_mode, N_("print all configuration variable names"), + HELP_ACTION_CONFIG), + OPT_CMDMODE_F(0, "config-for-completion", &cmd_mode, "", + HELP_ACTION_CONFIG_FOR_COMPLETION, PARSE_OPT_HIDDEN), + OPT_END(), }; @@ -544,6 +553,13 @@ static const char *check_git_cmd(const char* cmd) return cmd; } +static void no_extra_argc(int argc) +{ + if (argc) + usage_msg_opt(_("this option doesn't take any other arguments"), + builtin_help_usage, builtin_help_options); +} + int cmd_help(int argc, const char **argv, const char *prefix) { int nongit; @@ -554,28 +570,8 @@ int cmd_help(int argc, const char **argv, const char *prefix) builtin_help_usage, 0); parsed_help_format = help_format; - /* Incompatible options */ - if (show_all && show_config) - usage_msg_opt(_("--config and --all cannot be combined"), - builtin_help_usage, builtin_help_options); - - if (show_all && show_guides) - usage_msg_opt(_("--config and --guides cannot be combined"), - builtin_help_usage, builtin_help_options); - - if (show_config && show_guides) - usage_msg_opt(_("--config and --guides cannot be combined"), - builtin_help_usage, builtin_help_options); - - /* Options that take no further arguments */ - if (argc && show_config) - usage_msg_opt(_("--config cannot be combined with command or guide names"), - builtin_help_usage, builtin_help_options); - if (argc && show_guides) - usage_msg_opt(_("--guides cannot be combined with command or guide names"), - builtin_help_usage, builtin_help_options); - - if (show_all) { + switch (cmd_mode) { + case HELP_ACTION_ALL: git_config(git_help_config, NULL); if (verbose) { setup_pager(); @@ -585,25 +581,20 @@ int cmd_help(int argc, const char **argv, const char *prefix) printf(_("usage: %s%s"), _(git_usage_string), "\n\n"); load_command_list("git-", &main_cmds, &other_cmds); list_commands(colopts, &main_cmds, &other_cmds); - } - - if (show_guides) + printf("%s\n", _(git_more_info_string)); + break; + case HELP_ACTION_GUIDES: + no_extra_argc(argc); list_guides_help(); - - if (show_all || show_guides) { printf("%s\n", _(git_more_info_string)); return 0; - } - - if (show_config) { - int for_human = show_config == 1; - - if (!for_human) { - list_config_help(for_human); - return 0; - } + case HELP_ACTION_CONFIG_FOR_COMPLETION: + list_config_help(0); + return 0; + case HELP_ACTION_CONFIG: + no_extra_argc(argc); setup_pager(); - list_config_help(for_human); + list_config_help(1); printf("\n%s\n", _("'git help config' for more information")); return 0; } diff --git a/parse-options.h b/parse-options.h index a845a9d952..0e9271dde5 100644 --- a/parse-options.h +++ b/parse-options.h @@ -169,8 +169,10 @@ struct option { #define OPT_BOOL(s, l, v, h) OPT_BOOL_F(s, l, v, h, 0) #define OPT_HIDDEN_BOOL(s, l, v, h) { OPTION_SET_INT, (s), (l), (v), NULL, \ (h), PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, NULL, 1} -#define OPT_CMDMODE(s, l, v, h, i) { OPTION_SET_INT, (s), (l), (v), NULL, \ - (h), PARSE_OPT_CMDMODE|PARSE_OPT_NOARG|PARSE_OPT_NONEG, NULL, (i) } +#define OPT_CMDMODE_F(s, l, v, h, i, f) { OPTION_SET_INT, (s), (l), (v), NULL, \ + (h), PARSE_OPT_CMDMODE|PARSE_OPT_NOARG|PARSE_OPT_NONEG | (f), NULL, (i) } +#define OPT_CMDMODE(s, l, v, h, i) OPT_CMDMODE_F(s, l, v, h, i, 0) + #define OPT_INTEGER(s, l, v, h) OPT_INTEGER_F(s, l, v, h, 0) #define OPT_MAGNITUDE(s, l, v, h) { OPTION_MAGNITUDE, (s), (l), (v), \ N_("n"), (h), PARSE_OPT_NONEG } From 5a5f04d86b870bfec1c8cfefa8207a4e110fa128 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Wed, 22 Sep 2021 00:40:37 +0200 Subject: [PATCH 7/9] help tests: test --config-for-completion option & output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a regression test for the --config-for-completion option, this was tested for indirectly with the test added in 7a09a8f093e (completion: add tests for 'git config' completion, 2019-08-13), but let's do it directly here as well. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- t/t0012-help.sh | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/t/t0012-help.sh b/t/t0012-help.sh index 69e385d3b6..25bbaf0d58 100755 --- a/t/t0012-help.sh +++ b/t/t0012-help.sh @@ -41,7 +41,8 @@ test_expect_success 'invalid usage' ' test_expect_code 129 git help -g add && test_expect_code 129 git help -a -g && - test_expect_code 129 git help -g -c + test_expect_code 129 git help -g -c && + test_expect_code 0 git help --config-for-completion add ' test_expect_success "works for commands and guides by default" ' @@ -96,6 +97,20 @@ test_expect_success 'git help -c' ' test_cmp expect actual ' +test_expect_success 'git help --config-for-completion' ' + git help -c >human && + grep -E \ + -e "^[^.]+\.[^.]+$" \ + -e "^[^.]+\.[^.]+\.[^.]+$" human | + sed -e "s/\*.*//" -e "s/<.*//" | + sort -u >human.munged && + + git help --config-for-completion >vars && + sort -u vars.new && + mv vars.new vars && + test_cmp human.munged vars +' + test_expect_success 'generate builtin list' ' git --list-cmds=builtins >builtins ' From a9bacccae54cd449821416199f70c4dd2fcb9be4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Wed, 22 Sep 2021 00:40:38 +0200 Subject: [PATCH 8/9] help / completion: make "git help" do the hard work MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "help" builtin has been able to emit configuration variables since e17ca926371 (completion: drop the hard coded list of config vars, 2018-05-26), but it hasn't produced exactly the format the completion script wanted. Let's do that. We got partway there in 2675ea1cc0f (completion: use 'sort -u' to deduplicate config variable names, 2019-08-13) and d9438873c4d (completion: deduplicate configuration sections, 2019-08-13), but after both we still needed some sorting, de-duplicating and awk post-processing of the list. We can instead simply do the relevant parsing ourselves (we were doing most of it already), and call string_list_remove_duplicates() after already sorting the list, so the caller doesn't need to invoke "sort -u". The "--config-for-completion" output is the same as before after being passed through "sort -u". Then add a new "--config-sections-for-completion" option. Under that output we'll emit config sections like "alias" (instead of "alias." in the --config-for-completion output). We need to be careful to leave the "--config-for-completion" option compatible with users git, but are still running a shell with an older git-completion.bash. If we e.g. changed the option name they'd see messages about git-completion.bash being unable to find the "--config-for-completion" option. Such backwards compatibility isn't something we should bend over backwards for, it's only helping users who: * Upgrade git * Are in an old shell * The git-completion.bash in that shell hasn't cached the old "--config-for-completion" output already. But since it's easy in this case to retain compatibility, let's do it, the older versions of git-completion.bash won't care that the input they get doesn't change after a "sort -u". While we're at it let's make "--config-for-completion" die if there's anything left over in "argc", and do the same in the new "--config-sections-for-completion" option. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/help.c | 54 +++++++++++++++++++------- contrib/completion/git-completion.bash | 21 +++++----- t/t0012-help.sh | 17 ++++++-- 3 files changed, 65 insertions(+), 27 deletions(-) diff --git a/builtin/help.c b/builtin/help.c index 6a022d9803..9a255a9aee 100644 --- a/builtin/help.c +++ b/builtin/help.c @@ -34,11 +34,18 @@ enum help_format { HELP_FORMAT_WEB }; +enum show_config_type { + SHOW_CONFIG_HUMAN, + SHOW_CONFIG_VARS, + SHOW_CONFIG_SECTIONS, +}; + static enum help_action { HELP_ACTION_ALL = 1, HELP_ACTION_GUIDES, HELP_ACTION_CONFIG, HELP_ACTION_CONFIG_FOR_COMPLETION, + HELP_ACTION_CONFIG_SECTIONS_FOR_COMPLETION, } cmd_mode; static const char *html_path; @@ -63,6 +70,8 @@ static struct option builtin_help_options[] = { HELP_ACTION_CONFIG), OPT_CMDMODE_F(0, "config-for-completion", &cmd_mode, "", HELP_ACTION_CONFIG_FOR_COMPLETION, PARSE_OPT_HIDDEN), + OPT_CMDMODE_F(0, "config-sections-for-completion", &cmd_mode, "", + HELP_ACTION_CONFIG_SECTIONS_FOR_COMPLETION, PARSE_OPT_HIDDEN), OPT_END(), }; @@ -82,7 +91,7 @@ struct slot_expansion { int found; }; -static void list_config_help(int for_human) +static void list_config_help(enum show_config_type type) { struct slot_expansion slot_expansions[] = { { "advice", "*", list_config_advices }, @@ -100,6 +109,8 @@ static void list_config_help(int for_human) const char **p; struct slot_expansion *e; struct string_list keys = STRING_LIST_INIT_DUP; + struct string_list keys_uniq = STRING_LIST_INIT_DUP; + struct string_list_item *item; int i; for (p = config_name_list; *p; p++) { @@ -130,34 +141,46 @@ static void list_config_help(int for_human) for (i = 0; i < keys.nr; i++) { const char *var = keys.items[i].string; const char *wildcard, *tag, *cut; + const char *dot = NULL; + struct strbuf sb = STRBUF_INIT; - if (for_human) { + switch (type) { + case SHOW_CONFIG_HUMAN: puts(var); continue; + case SHOW_CONFIG_SECTIONS: + dot = strchr(var, '.'); + break; + case SHOW_CONFIG_VARS: + break; } - wildcard = strchr(var, '*'); tag = strchr(var, '<'); - if (!wildcard && !tag) { - puts(var); + if (!dot && !wildcard && !tag) { + string_list_append(&keys_uniq, var); continue; } - if (wildcard && !tag) + if (dot) + cut = dot; + else if (wildcard && !tag) cut = wildcard; else if (!wildcard && tag) cut = tag; else cut = wildcard < tag ? wildcard : tag; - /* - * We may produce duplicates, but that's up to - * git-completion.bash to handle - */ - printf("%.*s\n", (int)(cut - var), var); + strbuf_add(&sb, var, cut - var); + string_list_append(&keys_uniq, sb.buf); + strbuf_release(&sb); + } string_list_clear(&keys, 0); + string_list_remove_duplicates(&keys_uniq, 0); + for_each_string_list_item(item, &keys_uniq) + puts(item->string); + string_list_clear(&keys_uniq, 0); } static enum help_format parse_help_format(const char *format) @@ -589,12 +612,17 @@ int cmd_help(int argc, const char **argv, const char *prefix) printf("%s\n", _(git_more_info_string)); return 0; case HELP_ACTION_CONFIG_FOR_COMPLETION: - list_config_help(0); + no_extra_argc(argc); + list_config_help(SHOW_CONFIG_VARS); + return 0; + case HELP_ACTION_CONFIG_SECTIONS_FOR_COMPLETION: + no_extra_argc(argc); + list_config_help(SHOW_CONFIG_SECTIONS); return 0; case HELP_ACTION_CONFIG: no_extra_argc(argc); setup_pager(); - list_config_help(1); + list_config_help(SHOW_CONFIG_HUMAN); printf("\n%s\n", _("'git help config' for more information")); return 0; } diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 8108eda1e8..b9f6370193 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -2503,7 +2503,14 @@ __git_config_vars= __git_compute_config_vars () { test -n "$__git_config_vars" || - __git_config_vars="$(git help --config-for-completion | sort -u)" + __git_config_vars="$(git help --config-for-completion)" +} + +__git_config_sections= +__git_compute_config_sections () +{ + test -n "$__git_config_sections" || + __git_config_sections="$(git help --config-sections-for-completion)" } # Completes possible values of various configuration variables. @@ -2717,16 +2724,8 @@ __git_complete_config_variable_name () __gitcomp "$__git_config_vars" "" "$cur_" "$sfx" ;; *) - __git_compute_config_vars - __gitcomp "$(echo "$__git_config_vars" | - awk -F . '{ - sections[$1] = 1 - } - END { - for (s in sections) - print s "." - } - ')" "" "$cur_" + __git_compute_config_sections + __gitcomp "$__git_config_sections" "" "$cur_" "." ;; esac } diff --git a/t/t0012-help.sh b/t/t0012-help.sh index 25bbaf0d58..60d713021f 100755 --- a/t/t0012-help.sh +++ b/t/t0012-help.sh @@ -42,7 +42,8 @@ test_expect_success 'invalid usage' ' test_expect_code 129 git help -a -g && test_expect_code 129 git help -g -c && - test_expect_code 0 git help --config-for-completion add + test_expect_code 129 git help --config-for-completion add && + test_expect_code 129 git help --config-sections-for-completion add ' test_expect_success "works for commands and guides by default" ' @@ -106,11 +107,21 @@ test_expect_success 'git help --config-for-completion' ' sort -u >human.munged && git help --config-for-completion >vars && - sort -u vars.new && - mv vars.new vars && test_cmp human.munged vars ' +test_expect_success 'git help --config-sections-for-completion' ' + git help -c >human && + grep -E \ + -e "^[^.]+\.[^.]+$" \ + -e "^[^.]+\.[^.]+\.[^.]+$" human | + sed -e "s/\..*//" | + sort -u >human.munged && + + git help --config-sections-for-completion >sections && + test_cmp human.munged sections +' + test_expect_success 'generate builtin list' ' git --list-cmds=builtins >builtins ' From 06fa4db3f7e450deb0bb849b338d7a5453f0d183 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Wed, 22 Sep 2021 00:40:39 +0200 Subject: [PATCH 9/9] help: move column config discovery to help.c library MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a git_config() call was added in dbfae689690 (help: reuse print_columns() for help -a, 2012-04-13) to read the column config we'd always use the resulting "colopts" variable. Then in 63eae83f8f3 (help: add "-a --verbose" to list all commands with synopsis, 2018-05-20) we started only using the "colopts" config under "--all" if "--no-verbose" was also given, but the "git_config()" call was not moved inside the "verbose" branch of the code. This change effectively does that, we'll only call list_commands() under "--all --no-verbose", so let's have it look up the config it needs. See 26c7d067832 (help -a: improve and make --verbose default, 2018-09-29) for another case in help.c where we look up config. The get_colopts() function is named for consistency with the existing get_alias() function added in 26c7d067832. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/help.c | 7 +------ help.c | 16 ++++++++++++++-- help.h | 2 +- 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/builtin/help.c b/builtin/help.c index 9a255a9aee..0a40d8cf09 100644 --- a/builtin/help.c +++ b/builtin/help.c @@ -7,7 +7,6 @@ #include "exec-cmd.h" #include "parse-options.h" #include "run-command.h" -#include "column.h" #include "config-list.h" #include "help.h" #include "alias.h" @@ -50,7 +49,6 @@ static enum help_action { static const char *html_path; static int verbose = 1; -static unsigned int colopts; static enum help_format help_format = HELP_FORMAT_NONE; static int exclude_guides; static struct option builtin_help_options[] = { @@ -384,8 +382,6 @@ static int add_man_viewer_info(const char *var, const char *value) static int git_help_config(const char *var, const char *value, void *cb) { - if (starts_with(var, "column.")) - return git_column_config(var, value, "help", &colopts); if (!strcmp(var, "help.format")) { if (!value) return config_error_nonbool(var); @@ -595,7 +591,6 @@ int cmd_help(int argc, const char **argv, const char *prefix) switch (cmd_mode) { case HELP_ACTION_ALL: - git_config(git_help_config, NULL); if (verbose) { setup_pager(); list_all_cmds_help(); @@ -603,7 +598,7 @@ int cmd_help(int argc, const char **argv, const char *prefix) } printf(_("usage: %s%s"), _(git_usage_string), "\n\n"); load_command_list("git-", &main_cmds, &other_cmds); - list_commands(colopts, &main_cmds, &other_cmds); + list_commands(&main_cmds, &other_cmds); printf("%s\n", _(git_more_info_string)); break; case HELP_ACTION_GUIDES: diff --git a/help.c b/help.c index be2fa64241..973e47cdc3 100644 --- a/help.c +++ b/help.c @@ -293,9 +293,21 @@ void load_command_list(const char *prefix, exclude_cmds(other_cmds, main_cmds); } -void list_commands(unsigned int colopts, - struct cmdnames *main_cmds, struct cmdnames *other_cmds) +static int get_colopts(const char *var, const char *value, void *data) { + unsigned int *colopts = data; + + if (starts_with(var, "column.")) + return git_column_config(var, value, "help", colopts); + + return 0; +} + +void list_commands(struct cmdnames *main_cmds, struct cmdnames *other_cmds) +{ + unsigned int colopts = 0; + git_config(get_colopts, &colopts); + if (main_cmds->cnt) { const char *exec_path = git_exec_path(); printf_ln(_("available git commands in '%s'"), exec_path); diff --git a/help.h b/help.h index 5871e93ba2..9d383f1a0b 100644 --- a/help.h +++ b/help.h @@ -37,7 +37,7 @@ void add_cmdname(struct cmdnames *cmds, const char *name, int len); /* Here we require that excludes is a sorted list. */ void exclude_cmds(struct cmdnames *cmds, struct cmdnames *excludes); int is_in_cmdlist(struct cmdnames *cmds, const char *name); -void list_commands(unsigned int colopts, struct cmdnames *main_cmds, struct cmdnames *other_cmds); +void list_commands(struct cmdnames *main_cmds, struct cmdnames *other_cmds); void get_version_info(struct strbuf *buf, int show_build_options); /*