completion: complete all possible -no-<options>

This is not a complete topic but I'd like to present the problem and my approach to see if it's a good way to go.

We have started recently to rely on parse-options to help complete options. One of the leftover items is allowing completing --no- form. This patch enables that (some options should not have --no- form, but that's easy not included here).

The problem with completing --no- form is that the number of completable options now usually doubles, taking precious screen space and also making it hard to find the option you want.

So the other half of this patch, the part in git-completion.bash, is to uncomplete --no- options. When you do "git checkout --<tab>", instead of displaying all --no- options, this patch simply displays one item: the --no- prefix. If you do "git checkout --no-<tab>" then all negative options are displayed. This helps reduce completable options quite efficiently.

Of course life is not that simple, we do have --no- options by default sometimes (taking priority over the positive form), e.g. "git clone --no-checkout". Collapsing all --no-options into --no- would be a regression.

To avoid it, the order of options --git-completion-helper returns does matter. The first 4 negative options are not collapsed. Only options after the 4th are. Extra --no- options are always printed at the end, after all the --no- defined in struct option, this kinda works. Not pretty but works.

After all this "git checkout --<tab>" now looks like this

    --conflict=                   --orphan=
    --detach                      --ours 
    --ignore-other-worktrees      --patch 
    --ignore-skip-worktree-bits   --progress 
    --merge                       --quiet 
    --no-                         --recurse-submodules 
    --no-detach                   --theirs 
    --no-quiet                    --track 
    --no-track                    

and all the no options

    --no-conflict                    --no-patch 
    --no-detach                      --no-progress 
    --no-ignore-other-worktrees      --no-quiet 
    --no-ignore-skip-worktree-bits   --no-recurse-submodules 
    --no-merge                       --no-track 
    --no-orphan                      

---
 contrib/completion/git-completion.bash | 25 +++++++++++++++-
 parse-options.c                        | 40 +++++++++++++++++++++++---
 2 files changed, 60 insertions(+), 5 deletions(-)

index a757073945..85b9f24465 100644
@@ -266,7 +266,7 @@ __gitcomp ()
 	case "$cur_" in
 	--*=)
 		;;
-	*)
+	--no-*)
 		local c i=0 IFS=$' \t\n'
 		for c in $1; do
 			c="$c${4-}"

@@ -279,6 +279,29 @@ __gitcomp ()
 			fi
 		done
 		;;
+	*)
+		local c i=0 IFS=$' \t\n' n=0
+		for c in $1; do
+			c="$c${4-}"
+			if [[ $c == "$cur_"* ]]; then
+				case $c in
+				--*=*|*.) ;;
+				--no-*)
+					n=$(($n+1))
+					if [ "$n" -eq 4 ]; then
+						c="--no-${4-} "
+					elif [ "$n" -gt 4 ]; then
+						continue
+					else
+						c="$c "
+					fi
+					;;
+				*) c="$c " ;;
+				esac
+				COMPREPLY[i++]="${2-}$c"
+			fi
+		done
+		;;
 	esac
 }
 

index 0f7059a8ab..f6cd7ca8d2 100644
@@ -427,13 +427,11 @@ void parse_options_start(struct parse_opt_ctx_t *ctx,
 	parse_options_check(options);
 }
 
-/*
- * TODO: we are not completing the --no-XXX form yet because there are
- * many options that do not suppress it properly.
- */
 static int show_gitcomp(struct parse_opt_ctx_t *ctx,
 			const struct option *opts)
 {
+	const struct option *original_opts = opts;
+
 	for (; opts->type != OPTION_END; opts++) {
 		const char *suffix = "";
 

@@ -465,6 +463,40 @@ static int show_gitcomp(struct parse_opt_ctx_t *ctx,
 			suffix = "=";
 		printf(" --%s%s", opts->long_name, suffix);
 	}
+	for (opts = original_opts; opts->type != OPTION_END; opts++) {
+		int has_unset_form = 0;
+
+		if (!opts->long_name)
+			continue;
+		if (opts->flags & (PARSE_OPT_HIDDEN | PARSE_OPT_NOCOMPLETE))
+			continue;
+		if (opts->flags & PARSE_OPT_NONEG)
+			continue;
+
+		switch (opts->type) {
+		case OPTION_STRING:
+		case OPTION_FILENAME:
+		case OPTION_INTEGER:
+		case OPTION_MAGNITUDE:
+		case OPTION_CALLBACK:
+		case OPTION_BIT:
+		case OPTION_NEGBIT:
+		case OPTION_COUNTUP:
+		case OPTION_SET_INT:
+			has_unset_form = 1;
+			break;
+		default:
+			break;
+		}
+		if (has_unset_form) {
+			const char *name;
+
+			if (skip_prefix(opts->long_name, "no-", &name))
+				printf(" --%s", name);
+			else
+				printf(" --no-%s", opts->long_name);
+		}
+	}
 	fputc('\n', stdout);
 	exit(0);
 }

2.17.0.367.g5dd2e386c3

Clever.

So, the earlier mention of "clone --no-checkout" sounded about not losing this historical practice, but (desirabilty of magic number 4 aside) this "show first handful of --no-foo" feature is not about historical practice but is forward looking, in the sense that you do not mark "important" negated options in the source, which would be a way to handle the histrical "clone --no-checkout", but let the machinery mechanically choose among --no-foo (with the stupid choice criterion "first four are shown"). That allows other commands to have many --no-foo form without overwhelming the choices, but I am not sure if it is much better than a possible alternative of only showing --no-foo for more "important" foo's when show_gitcomp() is asked to list all of things. It would certainly be a more involved solution, that might require an update to the way how the choices are precomputed (you'd end up having to keep a separate "use this list when completing '--no-'" in addition to the normal list).

In any case, count this as a vote to support an update in this direction. A quite promising work ;-)

Thanks.

Well you kinda mark important in the source too. --no-checkout for exampled is declared as OPT_BOOL(0, "no-checkout"... and parse-options code has to add the double-negative form --checkout back [1].

The "first four" is chosen after carefully examining all commands and observing that none of them have more than 4 "important" --no-. But yes it is questionable and I should be able to do better to separate the favorable --no- from the other extra and most-of-the-time-useless --no- options.

I did think about this alternative and was still undecided. Suppose that you have less than 4 "important" --no- options, showing some extra ones to me does not really hurt anything and if we could show more options (within the same screen space) we should. But on the other hand maintaining this magic number could be a maintenance nightmare... Yeah I think I'm shifting towards no magic number now.

[1] These double negative options will _always_ show up, there is no easy way to hide them because they don't start with --no-. But we don't have a lot of options starting with "no-" so it's probably fine.

Duy

I haven't looked at the implementation, so this may be an entirely stupid suggestion, but would it be possible to instead render the completions as?

    % git checkout --<tab>
    --[no-]conflict=                   --[no-]patch
    --[no-]detach                      --[no-]progress
    --[no-]ignore-other-worktrees      --[no-]quiet
    --[no-]ignore-skip-worktree-bits   --[no-]recurse-submodules
    --[no-]merge                       --theirs
    --[no-]orphan=                     --[no-]track
    --ours

This would address the problem of the --no-* options taking double the screen space.

It's also more intuitive than that lone and somewhat weird-looking "--no-" suggestion.

It took me so long to reply partly because I remember seeing some guy doing clever trick with tab completion that also shows a short help text in addition to the complete words. I could not find that again and from my reading (also internet searching) it's probably not possible to do this without trickery.

It's not that weird if you think about file path completion, where you complete one path component at a time not full path, bash just does not show you full paths to everything.

I'm arguing about this because I want to see your reaction, because I'm thinking of doing the very same thing for config completion. Right now "git config <tab>" gives you two pages of all available config variables. I'm thinking that we "git config <tab>" just shows the groups, e.g.

add.              interactive.
advice.           log.
alias.            mailmap.
am.               man.

Only when you do "git config log.<tab>" that it shows you log.*

Duy

How cool is that? I'd love it.

Was that perhaps using zsh rather than bash? Below is some of the display from its git completion (this is likely affected somewhat by my configuration). The group descriptions (lines that begin with "Completing") appear in a different color, and are not available for selection.

1113$ git c<tab>
Completing alias

ci               -- alias for 'commit -v'
cia              -- alias for 'commit -v -a'
co               -- alias for 'checkout'
conf             -- alias for 'config'
Completing main porcelain command
checkout         -- checkout branch or paths to working tree
cherry-pick      -- apply changes introduced by some existing commits
citool           -- graphical alternative to git commit
clean            -- remove untracked files from working tree
clone            -- clone repository into new directory
commit           -- record changes to repository

Completing ancillary manipulator command
config -- get and set repository or global options
Completing ancillary interrogator command
cherry -- find commits not merged upstream
count-objects -- count unpacked objects and display their disk consumption
Completing plumbing manipulator command
checkout-index -- copy files from index to working directory
commit-tree -- create new commit object
Completing plumbing interrogator command
cat-file -- provide content or type information for repository objects

1114$ git commit -<tab>
Completing option

--all                  -a       -- stage all modified and deleted paths
--allow-empty                   -- allow recording an empty commit
--allow-empty-message           -- allow recording a commit with an empty message
--amend                         -- amend the tip of the current branch
--author                        -- override the author name used in the commit

Ah. That's probably it.

It's interesting that zsh could do this. I looked at the script and these texts are hard coded in there. I don't use zsh myself and won't be doing this, but this information should be now available from git binary so you can lower maintenance cost for the zsh completion script.

Hm.. no idea where this is from. Maybe zsh can extract "git <command> -h"? Anyway it does not matter.

Duy

Okay.

The "path completion" analogy and the dotted configuration variable analogy (below) don't really help me find "--no-" less weird. We're used to "/" as a separator in paths, and "." a separator in configuration variables, so they are easier to digest than "-" somehow being a separator for --no-<option>.

It _might_ feel as bit less weird if it was presented as --no-<option> or --no-{...} or --no-<...> or --no-... or something, but those seem pretty weird too, so perhaps not. Anyhow, it's not a major issue; the --[no-]foo idea seems pretty intuitive, but if it can't be easily implemented, then falling back to your --no- idea makes sense.

Just wondering out loud (again): add.<var> | add.{...} | add.<...> | add...; those aren't very attractive either, so plain "add." may indeed be best.

Oh good I was thinking --no-... too or we could even do "--no- (press tab for more)" or something to make it more obvious. As long as we make sure there's another --no-option somewhere, then we will only complete the --no- part and can replace the "..." (or "press tab for more") with real candidates in the next tab/
--
Duy

Am 08.05.2018 um 17:24 schrieb Duy Nguyen:

The fish-shell does something like that.

--branch (Show the branch and tracking info even in short-format)
--help (Display the manual of a git command)
--ignore-submodules (Ignore changes to submodules)
--porcelain (Give the output in a stable, easy-to-parse format)
--short (Give the output in the short-format)
--untracked-files (The untracked files handling mode)

Another tab will put a selection-cursor on the displayed list - you can navigate that list with Cursor-Up/Cursor-Down, select an entry and that entry will be inserted into the commandline. That selection process would be useless if the options are presented as "--[no-]x" because THAT cannot be inserted into the commandline without manual editing. And that's the point of the fast option selection process.

Good to know.

BTW I looked at the git.fish completion script [1] and see that recent effort to help automate more in git-completion.bash might help there too. I notice a lot of options and help text hard coded there, if someone can explain to me how git.fish uses those, maybe I can change git to export something suitable for git.fish to use too [2].

For example with latest git (in 'master') doing this

    ./git add --git-completion-helper

gives you the list of all options of "git add". Giving the help text for each option is definitely possible (I just didn't see any use for it until I looked at zsh/fish completion scripts) and maybe more in the future.

[1] https://github.com/fish-shell/fish-shell/blob/master/share/completions/git.fish
[2] But then if your script has to work with old git versions too then this is a moot point.

Duy

Am 14.05.2018 um 19:26 schrieb Duy Nguyen:

I'm no expert, but some additional things required by fish (and I suppose zsh too) but not by bash:

- grouping of long and short options
- help text
- argument types for options

Help text and long/short option grouping look like this:

--force-rebase -f (Force the rebase)
--merge -m (Use merging strategies to rebase)

All these infos seem to be available in struct option (for C stuff at least). So I guess It would be easiest for Fish & Co if git just exports the complete info in some stable format.

Well, sooner or later those old git versions might not be supported by those shells exactly due to the involved maintenance overhead. So providing some helper is a step in the right direction. Not providing only fossilizes the current state.

The RFC was here [1]. We have started recently to rely on parse-options to help complete options. One of the leftover items is allowing completing --no- form. This series enables that.

Changes since the RFC version:

- There's no magic numbers (previously we keep 3 --no- options)
- When there are some --no- options hidden, then we show --no-... instead of just --no-

[1] https://public-inbox.org/git/[email protected]/

Nguyễn Thái Ngọc Duy (3):
parse-options: option to let --git-completion-helper show negative form
completion: suppress some -no- options
completion: collapse extra --no-.. options

 builtin/checkout.c                     | 10 +++--
 contrib/completion/git-completion.bash | 58 +++++++++++++++-----------
 parse-options.c                        | 58 ++++++++++++++++++++++++--
 t/t9902-completion.sh                  |  5 ++-
 4 files changed, 97 insertions(+), 34 deletions(-)

2.17.0.705.g3525833791

When 7fb6aefd2a (Merge branch 'nd/parseopt-completion' - 2018-03-14) is merged, the completion for negative form is left out because the series is alread long and it could be done in a follow up series. This is it.

--git-completion-helper now provides --no-xxx so that git-completion.bash can drop the extra custom --no-xxx in the script. It adds a lot more --no-xxx than what's current provided by the git-completion.bash script. We'll trim that down later.

---
 contrib/completion/git-completion.bash | 38 ++++++++++----------------
 parse-options.c                        | 22 ++++++++++++---
 t/t9902-completion.sh                  | 16 +++++++++--
 3 files changed, 46 insertions(+), 30 deletions(-)

index 961a0ed76f..952e660f06 100644
@@ -1126,7 +1126,7 @@ _git_am ()
 		return
 		;;
 	--*)
-		__gitcomp_builtin am "--no-utf8" \
+		__gitcomp_builtin am "" \
 			"$__git_am_inprogress_options"
 		return
 	esac

@@ -1226,9 +1226,7 @@ _git_branch ()
 		__git_complete_refs --cur="${cur##--set-upstream-to=}"
 		;;
 	--*)
-		__gitcomp_builtin branch "--no-color --no-abbrev
-			--no-track --no-column
-			"
+		__gitcomp_builtin branch
 		;;
 	*)
 		if [ $only_local_ref = "y" -a $has_r = "n" ]; then

@@ -1269,7 +1267,7 @@ _git_checkout ()
 		__gitcomp "diff3 merge" "" "${cur##--conflict=}"
 		;;
 	--*)
-		__gitcomp_builtin checkout "--no-track --no-recurse-submodules"
+		__gitcomp_builtin checkout
 		;;
 	*)
 		# check if --track, --no-track, or --no-guess was specified

@@ -1332,7 +1330,7 @@ _git_clone ()
 {
 	case "$cur" in
 	--*)
-		__gitcomp_builtin clone "--no-single-branch"
+		__gitcomp_builtin clone
 		return
 		;;
 	esac

@@ -1365,7 +1363,7 @@ _git_commit ()
 		return
 		;;
 	--*)
-		__gitcomp_builtin commit "--no-edit --verify"
+		__gitcomp_builtin commit
 		return
 	esac
 

@@ -1468,7 +1466,7 @@ _git_fetch ()
 		return
 		;;
 	--*)
-		__gitcomp_builtin fetch "--no-tags"
+		__gitcomp_builtin fetch
 		return
 		;;
 	esac

@@ -1505,7 +1503,7 @@ _git_fsck ()
 {
 	case "$cur" in
 	--*)
-		__gitcomp_builtin fsck "--no-reflogs"
+		__gitcomp_builtin fsck
 		return
 		;;
 	esac

@@ -1612,7 +1610,7 @@ _git_ls_files ()
 {
 	case "$cur" in
 	--*)
-		__gitcomp_builtin ls-files "--no-empty-directory"
+		__gitcomp_builtin ls-files
 		return
 		;;
 	esac

@@ -1763,12 +1761,7 @@ _git_merge ()
 
 	case "$cur" in
 	--*)
-		__gitcomp_builtin merge "--no-rerere-autoupdate
-				--no-commit --no-edit --no-ff
-				--no-log --no-progress
-				--no-squash --no-stat
-				--no-verify-signatures
-				"
+		__gitcomp_builtin merge
 		return
 	esac
 	__git_complete_refs

@@ -1867,10 +1860,7 @@ _git_pull ()
 		return
 		;;
 	--*)
-		__gitcomp_builtin pull "--no-autostash --no-commit --no-edit
-					--no-ff --no-log --no-progress --no-rebase
-					--no-squash --no-stat --no-tags
-					--no-verify-signatures"
+		__gitcomp_builtin pull
 
 		return
 		;;

@@ -2061,7 +2051,7 @@ _git_status ()
 		return
 		;;
 	--*)
-		__gitcomp_builtin status "--no-column"
+		__gitcomp_builtin status
 		return
 		;;
 	esac

@@ -2615,7 +2605,7 @@ _git_remote ()
 
 	case "$subcommand,$cur" in
 	add,--*)
-		__gitcomp_builtin remote_add "--no-tags"
+		__gitcomp_builtin remote_add
 		;;
 	add,*)
 		;;

@@ -2695,7 +2685,7 @@ _git_revert ()
 	fi
 	case "$cur" in
 	--*)
-		__gitcomp_builtin revert "--no-edit" \
+		__gitcomp_builtin revert "" \
 			"$__git_revert_inprogress_options"
 		return
 		;;

@@ -2765,7 +2755,7 @@ _git_show_branch ()
 {
 	case "$cur" in
 	--*)
-		__gitcomp_builtin show-branch "--no-color"
+		__gitcomp_builtin show-branch
 		return
 		;;
 	esac

index 0f7059a8ab..b86612148f 100644
@@ -427,15 +427,12 @@ void parse_options_start(struct parse_opt_ctx_t *ctx,
 	parse_options_check(options);
 }
 
-/*
- * TODO: we are not completing the --no-XXX form yet because there are
- * many options that do not suppress it properly.
- */
 static int show_gitcomp(struct parse_opt_ctx_t *ctx,
 			const struct option *opts)
 {
 	for (; opts->type != OPTION_END; opts++) {
 		const char *suffix = "";
+		int has_unset_form = 0;
 
 		if (!opts->long_name)
 			continue;

@@ -450,6 +447,8 @@ static int show_gitcomp(struct parse_opt_ctx_t *ctx,
 		case OPTION_INTEGER:
 		case OPTION_MAGNITUDE:
 		case OPTION_CALLBACK:
+			has_unset_form = 1;
+
 			if (opts->flags & PARSE_OPT_NOARG)
 				break;
 			if (opts->flags & PARSE_OPT_OPTARG)

@@ -458,12 +457,27 @@ static int show_gitcomp(struct parse_opt_ctx_t *ctx,
 				break;
 			suffix = "=";
 			break;
+		case OPTION_BIT:
+		case OPTION_NEGBIT:
+		case OPTION_COUNTUP:
+		case OPTION_SET_INT:
+			has_unset_form = 1;
+			break;
 		default:
 			break;
 		}
 		if (opts->flags & PARSE_OPT_COMP_ARG)
 			suffix = "=";
 		printf(" --%s%s", opts->long_name, suffix);
+
+		if (has_unset_form && !(opts->flags & PARSE_OPT_NONEG)) {
+			const char *name;
+
+			if (skip_prefix(opts->long_name, "no-", &name))
+				printf(" --%s", name);
+			else
+				printf(" --no-%s", opts->long_name);
+		}
 	}
 	fputc('\n', stdout);
 	exit(0);

index 1b34caa1e1..07c3e3b760 100755
@@ -1237,20 +1237,31 @@ test_expect_success 'double dash "git" itself' '
 test_expect_success 'double dash "git checkout"' '
 	test_completion "git checkout --" <<-\EOF
 	--quiet Z
+	--no-quiet Z
 	--detach Z
+	--no-detach Z
 	--track Z
+	--no-track Z
 	--orphan=Z
+	--no-orphan Z
 	--ours Z
+	--no-ours Z
 	--theirs Z
+	--no-theirs Z
 	--merge Z
+	--no-merge Z
 	--conflict=Z
+	--no-conflict Z
 	--patch Z
+	--no-patch Z
 	--ignore-skip-worktree-bits Z
+	--no-ignore-skip-worktree-bits Z
 	--ignore-other-worktrees Z
+	--no-ignore-other-worktrees Z
 	--recurse-submodules Z
-	--progress Z
-	--no-track Z
 	--no-recurse-submodules Z
+	--progress Z
+	--no-progress Z
 	EOF
 '
 

@@ -1457,6 +1468,7 @@ test_expect_success 'completion used <cmd> completion for alias: !f() { : git <c
 test_expect_success 'completion without explicit _git_xxx function' '
 	test_completion "git version --" <<-\EOF
 	--build-options Z
+	--no-build-options Z
 	EOF
 '
 

2.17.0.705.g3525833791

Most --no- options do have some use, even if rarely to negate some option that's specified in an alias.

These options --no-ours and --no-theirs however have no clear semantics. If I specify "--ours --no-theirs", the second will reset writeout stage and is equivalent of "--no-ours --no-theirs" which is not that easy to see. Drop them. You can either switch from --ours to --theirs and back but you can never negate them.

---
 builtin/checkout.c    | 10 ++++++----
 t/t9902-completion.sh |  2 --
 2 files changed, 6 insertions(+), 6 deletions(-)

index 2b3b768eff..c7670dbbfe 100644
@@ -1119,10 +1119,12 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		OPT_SET_INT('t', "track",  &opts.track, N_("set upstream info for new branch"),
 			BRANCH_TRACK_EXPLICIT),
 		OPT_STRING(0, "orphan", &opts.new_orphan_branch, N_("new-branch"), N_("new unparented branch")),
-		OPT_SET_INT('2', "ours", &opts.writeout_stage, N_("checkout our version for unmerged files"),
-			    2),
-		OPT_SET_INT('3', "theirs", &opts.writeout_stage, N_("checkout their version for unmerged files"),
-			    3),
+		OPT_SET_INT_F('2', "ours", &opts.writeout_stage,
+			      N_("checkout our version for unmerged files"),
+			      2, PARSE_OPT_NONEG),
+		OPT_SET_INT_F('3', "theirs", &opts.writeout_stage,
+			      N_("checkout their version for unmerged files"),
+			      3, PARSE_OPT_NONEG),
 		OPT__FORCE(&opts.force, N_("force checkout (throw away local modifications)"),
 			   PARSE_OPT_NOCOMPLETE),
 		OPT_BOOL('m', "merge", &opts.merge, N_("perform a 3-way merge with the new branch")),

index 07c3e3b760..7e5e3ad5b1 100755
@@ -1245,9 +1245,7 @@ test_expect_success 'double dash "git checkout"' '
 	--orphan=Z
 	--no-orphan Z
 	--ours Z
-	--no-ours Z
 	--theirs Z
-	--no-theirs Z
 	--merge Z
 	--no-merge Z
 	--conflict=Z

2.17.0.705.g3525833791

The commands that make use of --git-completion-helper feature could now produce a lot of --no-xxx options that a command can take. This in many case could nearly double the amount of completable options, using more screen estate and also harder to search for the wanted option.

This patch attempts to mitigate that by collapsing extra --no- options, the ones that are added by --git-completion-helper and not in original struct option arrays. The "--no-..." option will be displayed in this case to hint about more options, e.g.

    --bare                 --origin=
    --branch=              --progress
    --checkout             --quiet
    --config=              --recurse-submodules
    --depth=               --reference=
    --dissociate           --reference-if-able=
    --filter=              --separate-git-dir=
    --hardlinks            --shallow-exclude=
    --ipv4                 --shallow-since=
    --ipv6                 --shallow-submodules
    --jobs=                --shared
    --local                --single-branch
    --mirror               --tags
    --no-...               --template=
    --no-checkout          --upload-pack=
    --no-hardlinks         --verbose
    --no-tags

and when you complete it with --no-<tab>, all negative options will be presented:

    --no-bare                 --no-quiet
    --no-branch               --no-recurse-submodules
    --no-checkout             --no-reference
    --no-config               --no-reference-if-able
    --no-depth                --no-separate-git-dir
    --no-dissociate           --no-shallow-exclude
    --no-filter               --no-shallow-since
    --no-hardlinks            --no-shallow-submodules
    --no-ipv4                 --no-shared
    --no-ipv6                 --no-single-branch
    --no-jobs                 --no-tags
    --no-local                --no-template
    --no-mirror               --no-upload-pack
    --no-origin               --no-verbose

--no-progress

Corner case: to make sure that people will never accidentally complete the fake option "--no-..." there must be one real --no- in the first complete listing even if it's not from the original struct option.

PS. This could could be made simpler with ";&" to fall through from "--no-*" block and share the code but ";&" is not available on bash-3 (i.e. Mac)

---
 contrib/completion/git-completion.bash | 20 +++++++
 parse-options.c                        | 72 +++++++++++++++++++-------
 t/t9902-completion.sh                  | 13 +----
 3 files changed, 76 insertions(+), 29 deletions(-)

index 952e660f06..4eef353ee2 100644
@@ -266,9 +266,29 @@ __gitcomp ()
 	case "$cur_" in
 	--*=)
 		;;
+	--no-*)
+		local c i=0 IFS=$' \t\n'
+		for c in $1; do
+			if [[ $c == "--" ]]; then
+				continue
+			fi
+			c="$c${4-}"
+			if [[ $c == "$cur_"* ]]; then
+				case $c in
+				--*=*|*.) ;;
+				*) c="$c " ;;
+				esac
+				COMPREPLY[i++]="${2-}$c"
+			fi
+		done
+		;;
 	*)
 		local c i=0 IFS=$' \t\n'
 		for c in $1; do
+			if [[ $c == "--" ]]; then
+				COMPREPLY[i++]="${2-}--no-...${4-} "
+				break
+			fi
 			c="$c${4-}"
 			if [[ $c == "$cur_"* ]]; then
 				case $c in

index 7e5e3ad5b1..eb4a43584a 100755
@@ -1237,29 +1237,20 @@ test_expect_success 'double dash "git" itself' '
 test_expect_success 'double dash "git checkout"' '
 	test_completion "git checkout --" <<-\EOF
 	--quiet Z
-	--no-quiet Z
 	--detach Z
-	--no-detach Z
 	--track Z
-	--no-track Z
 	--orphan=Z
-	--no-orphan Z
 	--ours Z
 	--theirs Z
 	--merge Z
-	--no-merge Z
 	--conflict=Z
-	--no-conflict Z
 	--patch Z
-	--no-patch Z
 	--ignore-skip-worktree-bits Z
-	--no-ignore-skip-worktree-bits Z
 	--ignore-other-worktrees Z
-	--no-ignore-other-worktrees Z
 	--recurse-submodules Z
-	--no-recurse-submodules Z
 	--progress Z
-	--no-progress Z
+	--no-quiet Z
+	--no-... Z
 	EOF
 '
 

2.17.0.705.g3525833791

https://public-inbox.org/git/[email protected]/
" There's no magic numbers (previously we keep 3 --no- options)"

Here I see 3 no- options, is the number how many no's to show configurable now?

In a sense, yes. If you write OPT_BOOL(0, "no-foo",...) then that --no-foo _always_ shows. "git clone" just happens to have three of them.

Duy

v3 fixes an annoying bug in 3/3. If you do "git commit --fi<tab>" then the "fi" part is eaten up by bash and you got back to "git commit --" on the command line.

This is because we give COMPREPLY with two options "--fixup" and "--no-...". The second one forces the common prefix "--" for both of them, instead of "--fi". Bash does the rest according to the book.

Some tests are added to verify new behavior in __gitcomp and make sure this does not happen again.

index 4eef353ee2..425d06256f 100644
@@ -286,7 +286,10 @@ __gitcomp ()
 		local c i=0 IFS=$' \t\n'
 		for c in $1; do
 			if [[ $c == "--" ]]; then
-				COMPREPLY[i++]="${2-}--no-...${4-} "
+				c="--no-...${4-}"
+				if [[ $c == "$cur_"* ]]; then
+					COMPREPLY[i++]="${2-}$c "
+				fi
 				break
 			fi
 			c="$c${4-}"

index eb4a43584a..157ee7085d 100755
@@ -459,6 +459,42 @@ test_expect_success '__gitcomp - suffix' '
 	EOF
 '
 
+test_expect_success '__gitcomp - ignore optional negative options' '
+	test_gitcomp "--" "--abc --def --no-one -- --no-two" <<-\EOF
+	--abc Z
+	--def Z
+	--no-one Z
+	--no-... Z
+	EOF
+'
+
+test_expect_success '__gitcomp - ignore/narrow optional negative options' '
+	test_gitcomp "--a" "--abc --abcdef --no-one -- --no-two" <<-\EOF
+	--abc Z
+	--abcdef Z
+	EOF
+'
+
+test_expect_success '__gitcomp - ignore/narrow optional negative options' '
+	test_gitcomp "--n" "--abc --def --no-one -- --no-two" <<-\EOF
+	--no-one Z
+	--no-... Z
+	EOF
+'
+
+test_expect_success '__gitcomp - expand all negative options' '
+	test_gitcomp "--no-" "--abc --def --no-one -- --no-two" <<-\EOF
+	--no-one Z
+	--no-two Z
+	EOF
+'
+
+test_expect_success '__gitcomp - expand/narrow all negative options' '
+	test_gitcomp "--no-o" "--abc --def --no-one -- --no-two" <<-\EOF
+	--no-one Z
+	EOF
+'
+
 test_expect_success '__gitcomp - doesnt fail because of invalid variable name' '
 	__gitcomp "$invalid_variable_name"
 '

Nguyễn Thái Ngọc Duy (3):
parse-options: option to let --git-completion-helper show negative
form
completion: suppress some -no- options
completion: collapse extra --no-.. options

 builtin/checkout.c                     | 10 +++--
 contrib/completion/git-completion.bash | 61 ++++++++++++++++----------
 parse-options.c                        | 58 ++++++++++++++++++++++--
 t/t9902-completion.sh                  | 41 ++++++++++++++++-
 4 files changed, 136 insertions(+), 34 deletions(-)

2.18.0.rc0.333.g22e6ee6cdf

When 7fb6aefd2a (Merge branch 'nd/parseopt-completion' - 2018-03-14) is merged, the completion for negative form is left out because the series is alread long and it could be done in a follow up series. This is it.

--git-completion-helper now provides --no-xxx so that git-completion.bash can drop the extra custom --no-xxx in the script. It adds a lot more --no-xxx than what's current provided by the git-completion.bash script. We'll trim that down later.

---
 contrib/completion/git-completion.bash | 38 ++++++++++----------------
 parse-options.c                        | 22 ++++++++++++---
 t/t9902-completion.sh                  | 16 +++++++++--
 3 files changed, 46 insertions(+), 30 deletions(-)

index 961a0ed76f..952e660f06 100644
@@ -1126,7 +1126,7 @@ _git_am ()
 		return
 		;;
 	--*)
-		__gitcomp_builtin am "--no-utf8" \
+		__gitcomp_builtin am "" \
 			"$__git_am_inprogress_options"
 		return
 	esac

@@ -1226,9 +1226,7 @@ _git_branch ()
 		__git_complete_refs --cur="${cur##--set-upstream-to=}"
 		;;
 	--*)
-		__gitcomp_builtin branch "--no-color --no-abbrev
-			--no-track --no-column
-			"
+		__gitcomp_builtin branch
 		;;
 	*)
 		if [ $only_local_ref = "y" -a $has_r = "n" ]; then

@@ -1269,7 +1267,7 @@ _git_checkout ()
 		__gitcomp "diff3 merge" "" "${cur##--conflict=}"
 		;;
 	--*)
-		__gitcomp_builtin checkout "--no-track --no-recurse-submodules"
+		__gitcomp_builtin checkout
 		;;
 	*)
 		# check if --track, --no-track, or --no-guess was specified

@@ -1332,7 +1330,7 @@ _git_clone ()
 {
 	case "$cur" in
 	--*)
-		__gitcomp_builtin clone "--no-single-branch"
+		__gitcomp_builtin clone
 		return
 		;;
 	esac

@@ -1365,7 +1363,7 @@ _git_commit ()
 		return
 		;;
 	--*)
-		__gitcomp_builtin commit "--no-edit --verify"
+		__gitcomp_builtin commit
 		return
 	esac
 

@@ -1468,7 +1466,7 @@ _git_fetch ()
 		return
 		;;
 	--*)
-		__gitcomp_builtin fetch "--no-tags"
+		__gitcomp_builtin fetch
 		return
 		;;
 	esac

@@ -1505,7 +1503,7 @@ _git_fsck ()
 {
 	case "$cur" in
 	--*)
-		__gitcomp_builtin fsck "--no-reflogs"
+		__gitcomp_builtin fsck
 		return
 		;;
 	esac

@@ -1612,7 +1610,7 @@ _git_ls_files ()
 {
 	case "$cur" in
 	--*)
-		__gitcomp_builtin ls-files "--no-empty-directory"
+		__gitcomp_builtin ls-files
 		return
 		;;
 	esac

@@ -1763,12 +1761,7 @@ _git_merge ()
 
 	case "$cur" in
 	--*)
-		__gitcomp_builtin merge "--no-rerere-autoupdate
-				--no-commit --no-edit --no-ff
-				--no-log --no-progress
-				--no-squash --no-stat
-				--no-verify-signatures
-				"
+		__gitcomp_builtin merge
 		return
 	esac
 	__git_complete_refs

@@ -1867,10 +1860,7 @@ _git_pull ()
 		return
 		;;
 	--*)
-		__gitcomp_builtin pull "--no-autostash --no-commit --no-edit
-					--no-ff --no-log --no-progress --no-rebase
-					--no-squash --no-stat --no-tags
-					--no-verify-signatures"
+		__gitcomp_builtin pull
 
 		return
 		;;

@@ -2061,7 +2051,7 @@ _git_status ()
 		return
 		;;
 	--*)
-		__gitcomp_builtin status "--no-column"
+		__gitcomp_builtin status
 		return
 		;;
 	esac

@@ -2615,7 +2605,7 @@ _git_remote ()
 
 	case "$subcommand,$cur" in
 	add,--*)
-		__gitcomp_builtin remote_add "--no-tags"
+		__gitcomp_builtin remote_add
 		;;
 	add,*)
 		;;

@@ -2695,7 +2685,7 @@ _git_revert ()
 	fi
 	case "$cur" in
 	--*)
-		__gitcomp_builtin revert "--no-edit" \
+		__gitcomp_builtin revert "" \
 			"$__git_revert_inprogress_options"
 		return
 		;;

@@ -2765,7 +2755,7 @@ _git_show_branch ()
 {
 	case "$cur" in
 	--*)
-		__gitcomp_builtin show-branch "--no-color"
+		__gitcomp_builtin show-branch
 		return
 		;;
 	esac

index 0f7059a8ab..b86612148f 100644
@@ -427,15 +427,12 @@ void parse_options_start(struct parse_opt_ctx_t *ctx,
 	parse_options_check(options);
 }
 
-/*
- * TODO: we are not completing the --no-XXX form yet because there are
- * many options that do not suppress it properly.
- */
 static int show_gitcomp(struct parse_opt_ctx_t *ctx,
 			const struct option *opts)
 {
 	for (; opts->type != OPTION_END; opts++) {
 		const char *suffix = "";
+		int has_unset_form = 0;
 
 		if (!opts->long_name)
 			continue;

@@ -450,6 +447,8 @@ static int show_gitcomp(struct parse_opt_ctx_t *ctx,
 		case OPTION_INTEGER:
 		case OPTION_MAGNITUDE:
 		case OPTION_CALLBACK:
+			has_unset_form = 1;
+
 			if (opts->flags & PARSE_OPT_NOARG)
 				break;
 			if (opts->flags & PARSE_OPT_OPTARG)

@@ -458,12 +457,27 @@ static int show_gitcomp(struct parse_opt_ctx_t *ctx,
 				break;
 			suffix = "=";
 			break;
+		case OPTION_BIT:
+		case OPTION_NEGBIT:
+		case OPTION_COUNTUP:
+		case OPTION_SET_INT:
+			has_unset_form = 1;
+			break;
 		default:
 			break;
 		}
 		if (opts->flags & PARSE_OPT_COMP_ARG)
 			suffix = "=";
 		printf(" --%s%s", opts->long_name, suffix);
+
+		if (has_unset_form && !(opts->flags & PARSE_OPT_NONEG)) {
+			const char *name;
+
+			if (skip_prefix(opts->long_name, "no-", &name))
+				printf(" --%s", name);
+			else
+				printf(" --no-%s", opts->long_name);
+		}
 	}
 	fputc('\n', stdout);
 	exit(0);

index 1b34caa1e1..07c3e3b760 100755
@@ -1237,20 +1237,31 @@ test_expect_success 'double dash "git" itself' '
 test_expect_success 'double dash "git checkout"' '
 	test_completion "git checkout --" <<-\EOF
 	--quiet Z
+	--no-quiet Z
 	--detach Z
+	--no-detach Z
 	--track Z
+	--no-track Z
 	--orphan=Z
+	--no-orphan Z
 	--ours Z
+	--no-ours Z
 	--theirs Z
+	--no-theirs Z
 	--merge Z
+	--no-merge Z
 	--conflict=Z
+	--no-conflict Z
 	--patch Z
+	--no-patch Z
 	--ignore-skip-worktree-bits Z
+	--no-ignore-skip-worktree-bits Z
 	--ignore-other-worktrees Z
+	--no-ignore-other-worktrees Z
 	--recurse-submodules Z
-	--progress Z
-	--no-track Z
 	--no-recurse-submodules Z
+	--progress Z
+	--no-progress Z
 	EOF
 '
 

@@ -1457,6 +1468,7 @@ test_expect_success 'completion used <cmd> completion for alias: !f() { : git <c
 test_expect_success 'completion without explicit _git_xxx function' '
 	test_completion "git version --" <<-\EOF
 	--build-options Z
+	--no-build-options Z
 	EOF
 '
 

2.18.0.rc0.333.g22e6ee6cdf

Most --no- options do have some use, even if rarely to negate some option that's specified in an alias.

These options --no-ours and --no-theirs however have no clear semantics. If I specify "--ours --no-theirs", the second will reset writeout stage and is equivalent of "--no-ours --no-theirs" which is not that easy to see. Drop them. You can either switch from --ours to --theirs and back but you can never negate them.

---
 builtin/checkout.c    | 10 ++++++----
 t/t9902-completion.sh |  2 --
 2 files changed, 6 insertions(+), 6 deletions(-)

index 2b3b768eff..c7670dbbfe 100644
@@ -1119,10 +1119,12 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		OPT_SET_INT('t', "track",  &opts.track, N_("set upstream info for new branch"),
 			BRANCH_TRACK_EXPLICIT),
 		OPT_STRING(0, "orphan", &opts.new_orphan_branch, N_("new-branch"), N_("new unparented branch")),
-		OPT_SET_INT('2', "ours", &opts.writeout_stage, N_("checkout our version for unmerged files"),
-			    2),
-		OPT_SET_INT('3', "theirs", &opts.writeout_stage, N_("checkout their version for unmerged files"),
-			    3),
+		OPT_SET_INT_F('2', "ours", &opts.writeout_stage,
+			      N_("checkout our version for unmerged files"),
+			      2, PARSE_OPT_NONEG),
+		OPT_SET_INT_F('3', "theirs", &opts.writeout_stage,
+			      N_("checkout their version for unmerged files"),
+			      3, PARSE_OPT_NONEG),
 		OPT__FORCE(&opts.force, N_("force checkout (throw away local modifications)"),
 			   PARSE_OPT_NOCOMPLETE),
 		OPT_BOOL('m', "merge", &opts.merge, N_("perform a 3-way merge with the new branch")),

index 07c3e3b760..7e5e3ad5b1 100755
@@ -1245,9 +1245,7 @@ test_expect_success 'double dash "git checkout"' '
 	--orphan=Z
 	--no-orphan Z
 	--ours Z
-	--no-ours Z
 	--theirs Z
-	--no-theirs Z
 	--merge Z
 	--no-merge Z
 	--conflict=Z

2.18.0.rc0.333.g22e6ee6cdf

The commands that make use of --git-completion-helper feature could now produce a lot of --no-xxx options that a command can take. This in many case could nearly double the amount of completable options, using more screen estate and also harder to search for the wanted option.

This patch attempts to mitigate that by collapsing extra --no- options, the ones that are added by --git-completion-helper and not in original struct option arrays. The "--no-..." option will be displayed in this case to hint about more options, e.g.

    --bare                 --origin=
    --branch=              --progress
    --checkout             --quiet
    --config=              --recurse-submodules
    --depth=               --reference=
    --dissociate           --reference-if-able=
    --filter=              --separate-git-dir=
    --hardlinks            --shallow-exclude=
    --ipv4                 --shallow-since=
    --ipv6                 --shallow-submodules
    --jobs=                --shared
    --local                --single-branch
    --mirror               --tags
    --no-...               --template=
    --no-checkout          --upload-pack=
    --no-hardlinks         --verbose
    --no-tags

and when you complete it with --no-<tab>, all negative options will be presented:

    --no-bare                 --no-quiet
    --no-branch               --no-recurse-submodules
    --no-checkout             --no-reference
    --no-config               --no-reference-if-able
    --no-depth                --no-separate-git-dir
    --no-dissociate           --no-shallow-exclude
    --no-filter               --no-shallow-since
    --no-hardlinks            --no-shallow-submodules
    --no-ipv4                 --no-shared
    --no-ipv6                 --no-single-branch
    --no-jobs                 --no-tags
    --no-local                --no-template
    --no-mirror               --no-upload-pack
    --no-origin               --no-verbose

--no-progress

Corner case: to make sure that people will never accidentally complete the fake option "--no-..." there must be one real --no- in the first complete listing even if it's not from the original struct option.

PS. This could could be made simpler with ";&" to fall through from "--no-*" block and share the code but ";&" is not available on bash-3 (i.e. Mac)

---
 contrib/completion/git-completion.bash | 23 ++++++++
 parse-options.c                        | 72 +++++++++++++++++++-------
 t/t9902-completion.sh                  | 49 ++++++++++++++----
 3 files changed, 115 insertions(+), 29 deletions(-)

index 952e660f06..425d06256f 100644
@@ -266,9 +266,32 @@ __gitcomp ()
 	case "$cur_" in
 	--*=)
 		;;
+	--no-*)
+		local c i=0 IFS=$' \t\n'
+		for c in $1; do
+			if [[ $c == "--" ]]; then
+				continue
+			fi
+			c="$c${4-}"
+			if [[ $c == "$cur_"* ]]; then
+				case $c in
+				--*=*|*.) ;;
+				*) c="$c " ;;
+				esac
+				COMPREPLY[i++]="${2-}$c"
+			fi
+		done
+		;;
 	*)
 		local c i=0 IFS=$' \t\n'
 		for c in $1; do
+			if [[ $c == "--" ]]; then
+				c="--no-...${4-}"
+				if [[ $c == "$cur_"* ]]; then
+					COMPREPLY[i++]="${2-}$c "
+				fi
+				break
+			fi
 			c="$c${4-}"
 			if [[ $c == "$cur_"* ]]; then
 				case $c in

2.18.0.rc0.333.g22e6ee6cdf