From 9856ea6785c3dcd19ffb3946c6a9adbd16d9c1da Mon Sep 17 00:00:00 2001 From: Ævar Arnfjörð Bjarmason Date: Wed, 22 Sep 2021 00:40:32 +0200 Subject: 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 --- t/t0012-help.sh | 4 ++++ 1 file changed, 4 insertions(+) (limited to 't') 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 && -- cgit v1.3-5-g9baa From ff76fc841f4787e7c91329ecda280f293100eccb Mon Sep 17 00:00:00 2001 From: Ævar Arnfjörð Bjarmason Date: Wed, 22 Sep 2021 00:40:33 +0200 Subject: 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(+) (limited to 't') 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 ' -- cgit v1.3-5-g9baa From 1ed4bef6b438d25ce605f6bdefb4c98569dad137 Mon Sep 17 00:00:00 2001 From: Ævar Arnfjörð Bjarmason Date: Wed, 22 Sep 2021 00:40:34 +0200 Subject: 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(-) (limited to 't') 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" ' -- cgit v1.3-5-g9baa From 0a5940fbe7e453652266e765509a576e4df333c7 Mon Sep 17 00:00:00 2001 From: Ævar Arnfjörð Bjarmason Date: Wed, 22 Sep 2021 00:40:35 +0200 Subject: 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(-) (limited to 't') 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" ' -- cgit v1.3-5-g9baa From 5a5f04d86b870bfec1c8cfefa8207a4e110fa128 Mon Sep 17 00:00:00 2001 From: Ævar Arnfjörð Bjarmason Date: Wed, 22 Sep 2021 00:40:37 +0200 Subject: 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(-) (limited to 't') 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 ' -- cgit v1.3-5-g9baa From a9bacccae54cd449821416199f70c4dd2fcb9be4 Mon Sep 17 00:00:00 2001 From: Ævar Arnfjörð Bjarmason Date: Wed, 22 Sep 2021 00:40:38 +0200 Subject: 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(-) (limited to 't') 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 ' -- cgit v1.3-5-g9baa