The PARSE_OPT_HIDDEN is, per the documentation of the "option" struct in option parse-options.h, only supposed to affect -h output, not completion. That's what the PARSE_OPT_NOCOMPLETE flag is supposed to be for. Since 2e29dca66a ("completion: use __gitcomp_builtin in _git_commit", 2018-02-09) we've been using e.g. "git commit --git-completion-helper" to get the bash completion for the git-commit command. Due to PARSE_OPT_HIDDEN this excluded e.g. --allow-empty and --allow-empty-message. Now, this wasn't a behavior change in that commit. Before that we had a hardcoded list of options, removed in 2e29dca66a ("completion: use __gitcomp_builtin in _git_commit", 2018-02-09). This list didn't contain those two options. But as noted in the follow-up discussion to c9b5fde759 ("Add option to git-commit to allow empty log messages", 2010-04-06) in https://public-inbox.org/git/20100406055530.GE3901@xxxxxxxxxxxxxxxxxxxxxxx/ the motivation for PARSE_OPT_HIDDEN was to keep the "git commit -h" output to a minimum, not to hide it from completion. I think it makes sense to exclude options like these from -h output, but for the completion the user is usually not trying to complete "git commit --<TAB>", but e.g. "git commit --allo<TAB>", and because of this behavior we don't show these options at all there. However, manually going over: git grep -e OPT_HIDDEN_BOOL -e PARSE_OPT_HIDDEN Shows many options we don't want to show in completion either, e.g. "git am --binary" or "git branch -l". Many of these are internal, deprecated, or no-ops. There's also things like "git difftool --prompt" (the default behavior) which are arguably pointless to add, we just have "--no-prompt" to inverse the default. The options we'll now show on completion, that we didn't show before, are: OPT_HIDDEN_BOOL: * checkout: --guess (no idea how this works, not documented, but it's not deprecated and is there..) * commit: --allow-empty and --allow-empty-message * help: --exclude-guides (because why not?) * receive-pack: All three (non --quiet) options it supports. It doesn't have any completion now, but if we ever add it why not complete these? PARSE_OPT_HIDDEN (without PARSE_OPT_NOCOMPLETE): * fetch: --recurse-submodules-default (a legitimate documented option, but perhaps we should blacklist this because it's rarely used and interferes with --recurse-submodules?). * ls-remote: --exec (as with "fetch" this is a synonym for --upload-pack, but unlike "fetch" it wasn't documented. Document it while I'm at it). I don't know if that "o->flags |= PARSE_OPT_HIDDEN" line in cmd_parseopt() in builtin/rev-parse.c should also be setting PARSE_OPT_NOCOMPLETE. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> --- On Thu, Aug 16, 2018 at 9:46 AM, Hadi Safari <hadi@xxxxxxxxxxxxx> wrote: > > Hi! > > I'm wondering why there isn't --allow-empty and > --allow-empty-message in completeion list of git commit command. I'm > getting only following flags from v2.18.0 on `git commit --`: It's because we've been conflating the desire to include something in "git <cmd> -h" output, and having "git <cmd> --some-rare-option" work or not. Here's an attempt to fix it. Documentation/git-ls-remote.txt | 3 +++ archive.c | 3 ++- builtin/add.c | 5 +++-- builtin/am.c | 8 ++++---- builtin/branch.c | 5 +++-- builtin/clone.c | 8 ++++---- builtin/difftool.c | 4 ++-- builtin/fetch.c | 3 ++- builtin/fmt-merge-msg.c | 3 ++- builtin/grep.c | 2 +- builtin/help.c | 2 +- builtin/name-rev.c | 3 ++- builtin/pull.c | 2 +- builtin/show-ref.c | 4 ++-- builtin/write-tree.c | 4 ++-- parse-options.c | 4 ++-- parse-options.h | 3 +++ t/t9902-completion.sh | 1 + 18 files changed, 40 insertions(+), 27 deletions(-) diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt index b9fd3770a6..39192f4e2a 100644 --- a/Documentation/git-ls-remote.txt +++ b/Documentation/git-ls-remote.txt @@ -43,6 +43,9 @@ OPTIONS SSH and where the SSH daemon does not use the PATH configured by the user. +--exec=<git-upload-pack>:: + Same as --upload-pack=<git-upload-pack>. + --exit-code:: Exit with status "2" when no matching refs are found in the remote repository. Usually the command exits with status "0" to indicate diff --git a/archive.c b/archive.c index 78b0a398a0..d5e02127a1 100644 --- a/archive.c +++ b/archive.c @@ -414,7 +414,8 @@ static void parse_treeish_arg(const char **argv, #define OPT__COMPR(s, v, h, p) \ OPT_SET_INT_F(s, NULL, v, h, p, PARSE_OPT_NONEG) #define OPT__COMPR_HIDDEN(s, v, p) \ - OPT_SET_INT_F(s, NULL, v, "", p, PARSE_OPT_NONEG | PARSE_OPT_HIDDEN) + OPT_SET_INT_F(s, NULL, v, "", p, \ + PARSE_OPT_NONEG | PARSE_OPT_HIDDEN | PARSE_OPT_NOCOMPLETE) static int parse_archive_args(int argc, const char **argv, const struct archiver **ar, struct archiver_args *args, diff --git a/builtin/add.c b/builtin/add.c index 8a155dd41e..f283eda6d8 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -305,8 +305,9 @@ static struct option builtin_add_options[] = { OPT_BOOL( 0 , "ignore-errors", &ignore_add_errors, N_("just skip files which cannot be added because of errors")), OPT_BOOL( 0 , "ignore-missing", &ignore_missing, N_("check if - even missing - files are ignored in dry run")), OPT_STRING( 0 , "chmod", &chmod_arg, N_("(+/-)x"), N_("override the executable bit of the listed files")), - OPT_HIDDEN_BOOL(0, "warn-embedded-repo", &warn_on_embedded_repo, - N_("warn when adding an embedded repository")), + OPT_HIDDEN_NOCOMPLETE_BOOL(0, "warn-embedded-repo", + &warn_on_embedded_repo, + N_("warn when adding an embedded repository")), OPT_END(), }; diff --git a/builtin/am.c b/builtin/am.c index 2c19e69f58..d5353c5954 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -2215,8 +2215,8 @@ int cmd_am(int argc, const char **argv, const char *prefix) struct option options[] = { OPT_BOOL('i', "interactive", &state.interactive, N_("run interactively")), - OPT_HIDDEN_BOOL('b', "binary", &binary, - N_("historical option -- no-op")), + OPT_HIDDEN_NOCOMPLETE_BOOL('b', "binary", &binary, + N_("historical option -- no-op")), OPT_BOOL('3', "3way", &state.threeway, N_("allow fall back on 3way merging if needed")), OPT__QUIET(&state.quiet, N_("be quiet")), @@ -2298,8 +2298,8 @@ int cmd_am(int argc, const char **argv, const char *prefix) { OPTION_STRING, 'S', "gpg-sign", &state.sign_commit, N_("key-id"), N_("GPG-sign commits"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" }, - OPT_HIDDEN_BOOL(0, "rebasing", &state.rebasing, - N_("(internal use for git-rebase)")), + OPT_HIDDEN_NOCOMPLETE_BOOL(0, "rebasing", &state.rebasing, + N_("(internal use for git-rebase)")), OPT_END() }; diff --git a/builtin/branch.c b/builtin/branch.c index 4fc55c3508..366ffb175d 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -598,7 +598,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix) OPT_SET_INT('t', "track", &track, N_("set up tracking mode (see git-pull(1))"), BRANCH_TRACK_EXPLICIT), OPT_SET_INT_F(0, "set-upstream", &track, N_("do not use"), - BRANCH_TRACK_OVERRIDE, PARSE_OPT_HIDDEN), + BRANCH_TRACK_OVERRIDE, + PARSE_OPT_HIDDEN | PARSE_OPT_NOCOMPLETE), OPT_STRING('u', "set-upstream-to", &new_upstream, N_("upstream"), N_("change the upstream info")), OPT_BOOL(0, "unset-upstream", &unset_upstream, N_("Unset the upstream info")), OPT__COLOR(&branch_use_color, N_("use colored output")), @@ -624,7 +625,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix) { OPTION_CALLBACK, 'l', NULL, &reflog, NULL, N_("deprecated synonym for --create-reflog"), - PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, + PARSE_OPT_NOARG | PARSE_OPT_HIDDEN | PARSE_OPT_NOCOMPLETE, deprecated_reflog_option_cb }, OPT_BOOL(0, "edit-description", &edit_description, diff --git a/builtin/clone.c b/builtin/clone.c index fd2c3ef090..27d2a1a459 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -87,8 +87,8 @@ static struct option builtin_clone_options[] = { OPT_BOOL('n', "no-checkout", &option_no_checkout, N_("don't create a checkout")), OPT_BOOL(0, "bare", &option_bare, N_("create a bare repository")), - OPT_HIDDEN_BOOL(0, "naked", &option_bare, - N_("create a bare repository")), + OPT_HIDDEN_NOCOMPLETE_BOOL(0, "naked", &option_bare, + N_("create a bare repository")), OPT_BOOL(0, "mirror", &option_mirror, N_("create a mirror repository (implies bare)")), OPT_BOOL('l', "local", &option_local, @@ -99,8 +99,8 @@ static struct option builtin_clone_options[] = { N_("setup as shared repository")), { OPTION_CALLBACK, 0, "recursive", &option_recurse_submodules, N_("pathspec"), N_("initialize submodules in the clone"), - PARSE_OPT_OPTARG | PARSE_OPT_HIDDEN, recurse_submodules_cb, - (intptr_t)"." }, + PARSE_OPT_OPTARG | PARSE_OPT_HIDDEN | PARSE_OPT_NOCOMPLETE, + recurse_submodules_cb, (intptr_t)"." }, { OPTION_CALLBACK, 0, "recurse-submodules", &option_recurse_submodules, N_("pathspec"), N_("initialize submodules in the clone"), PARSE_OPT_OPTARG, recurse_submodules_cb, (intptr_t)"." }, diff --git a/builtin/difftool.c b/builtin/difftool.c index 3018e61d04..f63e866dd7 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -699,8 +699,8 @@ int cmd_difftool(int argc, const char **argv, const char *prefix) OPT_SET_INT_F('y', "no-prompt", &prompt, N_("do not prompt before launching a diff tool"), 0, PARSE_OPT_NONEG), - OPT_SET_INT_F(0, "prompt", &prompt, NULL, - 1, PARSE_OPT_NONEG | PARSE_OPT_HIDDEN), + OPT_SET_INT_F(0, "prompt", &prompt, NULL, 1, + PARSE_OPT_NONEG | PARSE_OPT_HIDDEN | PARSE_OPT_NOCOMPLETE), OPT_BOOL(0, "symlinks", &symlinks, N_("use symlinks in dir-diff mode")), OPT_STRING('t', "tool", &difftool_cmd, N_("<tool>"), diff --git a/builtin/fetch.c b/builtin/fetch.c index 61bec5d213..3f11d743ee 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -153,7 +153,8 @@ static struct option builtin_fetch_options[] = { &recurse_submodules_default, N_("on-demand"), N_("default for recursive fetching of submodules " "(lower priority than config files)"), - PARSE_OPT_HIDDEN, option_fetch_parse_recurse_submodules }, + PARSE_OPT_HIDDEN | PARSE_OPT_NOCOMPLETE, + option_fetch_parse_recurse_submodules }, OPT_BOOL(0, "update-shallow", &update_shallow, N_("accept refs that update .git/shallow")), { OPTION_CALLBACK, 0, "refmap", NULL, N_("refmap"), diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c index f35ff1612b..ba0a8426a0 100644 --- a/builtin/fmt-merge-msg.c +++ b/builtin/fmt-merge-msg.c @@ -672,7 +672,8 @@ int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix) PARSE_OPT_OPTARG, NULL, DEFAULT_MERGE_LOG_LEN }, { OPTION_INTEGER, 0, "summary", &shortlog_len, N_("n"), N_("alias for --log (deprecated)"), - PARSE_OPT_OPTARG | PARSE_OPT_HIDDEN, NULL, + PARSE_OPT_OPTARG | PARSE_OPT_HIDDEN | PARSE_OPT_NOCOMPLETE, + NULL, DEFAULT_MERGE_LOG_LEN }, OPT_STRING('m', "message", &message, N_("text"), N_("use <text> as start of message")), diff --git a/builtin/grep.c b/builtin/grep.c index ee5a1bd355..2ecd941101 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -892,7 +892,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix) N_("show only matches from files that match all patterns")), OPT_SET_INT_F(0, "debug", &opt.debug, N_("show parse tree for grep expression"), - 1, PARSE_OPT_HIDDEN), + 1, PARSE_OPT_HIDDEN | PARSE_OPT_NOCOMPLETE), OPT_GROUP(""), { OPTION_STRING, 'O', "open-files-in-pager", &show_in_pager, N_("pager"), N_("show matching files in the pager"), diff --git a/builtin/help.c b/builtin/help.c index 8d4f6dd301..90c7a85cba 100644 --- a/builtin/help.c +++ b/builtin/help.c @@ -47,7 +47,7 @@ static struct option builtin_help_options[] = { 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_F(0, "config-for-completion", &show_config, "", 2, PARSE_OPT_HIDDEN | PARSE_OPT_NOCOMPLETE), 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), diff --git a/builtin/name-rev.c b/builtin/name-rev.c index f1cb45c227..0a63d23761 100644 --- a/builtin/name-rev.c +++ b/builtin/name-rev.c @@ -426,7 +426,8 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix) /* A Hidden OPT_BOOL */ OPTION_SET_INT, 0, "peel-tag", &peel_tag, NULL, N_("dereference tags in the input (internal use)"), - PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, NULL, 1, + PARSE_OPT_NOARG | PARSE_OPT_HIDDEN | PARSE_OPT_NOCOMPLETE, + NULL, 1, }, OPT_END(), }; diff --git a/builtin/pull.c b/builtin/pull.c index 4e78935392..cdc67cd17f 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -146,7 +146,7 @@ static struct option pull_options[] = { PARSE_OPT_NOARG), OPT_PASSTHRU(0, "summary", &opt_diffstat, NULL, N_("(synonym to --stat)"), - PARSE_OPT_NOARG | PARSE_OPT_HIDDEN), + PARSE_OPT_NOARG | PARSE_OPT_HIDDEN | PARSE_OPT_NOCOMPLETE), OPT_PASSTHRU(0, "log", &opt_log, N_("n"), N_("add (at most <n>) entries from shortlog to merge commit message"), PARSE_OPT_OPTARG), diff --git a/builtin/show-ref.c b/builtin/show-ref.c index 2f13f1316f..afd916fa06 100644 --- a/builtin/show-ref.c +++ b/builtin/show-ref.c @@ -161,8 +161,8 @@ static const struct option show_ref_options[] = { OPT_BOOL(0, "heads", &heads_only, N_("only show heads (can be combined with tags)")), OPT_BOOL(0, "verify", &verify, N_("stricter reference checking, " "requires exact ref path")), - OPT_HIDDEN_BOOL('h', NULL, &show_head, - N_("show the HEAD reference, even if it would be filtered out")), + OPT_HIDDEN_NOCOMPLETE_BOOL('h', NULL, &show_head, + N_("show the HEAD reference, even if it would be filtered out")), OPT_BOOL(0, "head", &show_head, N_("show the HEAD reference, even if it would be filtered out")), OPT_BOOL('d', "dereference", &deref_tags, diff --git a/builtin/write-tree.c b/builtin/write-tree.c index c9d3c544e7..489a5e46b0 100644 --- a/builtin/write-tree.c +++ b/builtin/write-tree.c @@ -29,8 +29,8 @@ int cmd_write_tree(int argc, const char **argv, const char *unused_prefix) PARSE_OPT_LITERAL_ARGHELP }, { OPTION_BIT, 0, "ignore-cache-tree", &flags, NULL, N_("only useful for debugging"), - PARSE_OPT_HIDDEN | PARSE_OPT_NOARG, NULL, - WRITE_TREE_IGNORE_CACHE_TREE }, + PARSE_OPT_HIDDEN | PARSE_OPT_NOARG | PARSE_OPT_NOCOMPLETE, + NULL, WRITE_TREE_IGNORE_CACHE_TREE }, OPT_END() }; diff --git a/parse-options.c b/parse-options.c index 7db84227ab..a9c82e518a 100644 --- a/parse-options.c +++ b/parse-options.c @@ -437,7 +437,7 @@ static void show_negated_gitcomp(const struct option *opts, int nr_noopts) if (!opts->long_name) continue; - if (opts->flags & (PARSE_OPT_HIDDEN | PARSE_OPT_NOCOMPLETE)) + if (opts->flags & PARSE_OPT_NOCOMPLETE) continue; if (opts->flags & PARSE_OPT_NONEG) continue; @@ -485,7 +485,7 @@ static int show_gitcomp(struct parse_opt_ctx_t *ctx, if (!opts->long_name) continue; - if (opts->flags & (PARSE_OPT_HIDDEN | PARSE_OPT_NOCOMPLETE)) + if (opts->flags & PARSE_OPT_NOCOMPLETE) continue; switch (opts->type) { diff --git a/parse-options.h b/parse-options.h index dd14911a29..5f78e2e164 100644 --- a/parse-options.h +++ b/parse-options.h @@ -139,6 +139,9 @@ 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_HIDDEN_NOCOMPLETE_BOOL(s, l, v, h) { OPTION_SET_INT, (s), (l), (v), NULL, \ + (h), PARSE_OPT_NOARG | PARSE_OPT_HIDDEN | PARSE_OPT_NOCOMPLETE, \ + NULL, 1} #define OPT_CMDMODE(s, l, v, h, i) { OPTION_CMDMODE, (s), (l), (v), NULL, \ (h), PARSE_OPT_NOARG|PARSE_OPT_NONEG, NULL, (i) } #define OPT_INTEGER(s, l, v, h) { OPTION_INTEGER, (s), (l), (v), N_("n"), (h) } diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh index 5ff43b9cbb..2e0b9c78e7 100755 --- a/t/t9902-completion.sh +++ b/t/t9902-completion.sh @@ -1423,6 +1423,7 @@ test_expect_success 'double dash "git checkout"' ' test_completion "git checkout --" <<-\EOF --quiet Z --detach Z + --guess Z --track Z --orphan=Z --ours Z -- 2.18.0.865.gffc8e1a3cd6