[PATCH] completion: include PARSE_OPT_HIDDEN in completion output

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux