Subject: Re: [PATCH v3 42/42] git-completion.bash: add GIT_COMPLETION_OPTIONS=all config

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Sun, 11 Feb 2018 15:47:48 +0100

On Sat, Feb 10 2018, Duy Nguyen jotted:

> On Fri, Feb 09, 2018 at 03:19:57PM +0100, Ævar Arnfjörð Bjarmason wrote:
>>
>> On Fri, Feb 09 2018, Nguyễn Thái Ngọc Duy jotted:
>>
>> > By default, some option names (mostly --force, scripting related or for
>> > internal use) are not completable for various reasons. When
>> > GIT_COMPLETION_OPTIONS is set to all, all options (except hidden ones)
>> > are completable.
>> >
>> > Signed-off-by: Nguyễn Thái Ngọc Duy <[email-masked]>
>> > ---
>> >  contrib/completion/git-completion.bash |  6 +++++-
>> >  parse-options.c                        | 11 +++++++----
>> >  2 files changed, 12 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
>> > index 0ddf40063b..0cfa489a8e 100644
>> > --- a/contrib/completion/git-completion.bash
>> > +++ b/contrib/completion/git-completion.bash
>> > @@ -36,6 +36,10 @@
>> >  #
>> >  #     When set to "1", do not include "DWIM" suggestions in git-checkout
>> >  #     completion (e.g., completing "foo" when "origin/foo" exists).
>> > +#
>> > +#   GIT_COMPLETION_OPTIONS
>> > +#
>> > +#     When set to "all", complete all possible options
>>
>> I was going to suggest some wording like:
>>
>>     When set to "all", include options considered unsafe such as --force
>>     in the completion.
>>
>> However per your cover letter it's not just used for that:
>>
>>      10 --force
>>       4 --rerere-autoupdate
>>       1 --unsafe-paths
>>       1 --thin
>>       1 --overwrite-ignore
>>       1 --open-files-in-pager
>>       1 --null
>>       1 --ext-grep
>>       1 --exit-code
>>       1 --auto
>>
>> I wonder if we shouldn't just make this only about --force, I don't see
>> why "git grep --o<TAB>" should only show --or but not
>> --open-files-in-pager, and e.g. "git grep --<TAB>" is already verbose so
>> we're not saving much by excluding those.
>>
>> Then this could just become:
>>
>>     GIT_COMPLETION_SHOWUNSAFEOPTIONS=1
>>
>> Or other similar boolean variable, for consistency with all the "*SHOW*
>> variables already in git-completion.bash.
>
> No. You're asking for a second default. I'm not adding plenty of
> GIT_COMPLETION_* variables for that. You either have all options, or
> you convince people that --force should be part of the current
> default.

No sorry, I mean that IMO the current patch you have could be simplified
where instead of saying "=all" there's just another variable that only
hides "dangerous" options, i.e. only "--force" (unless I've missed
another supposedly dangerous one).

But as previously discussed I think it just makes sense to stop doing
this conditionally and include --force too, the only stuff we should
hide is stuff like rebase --continue when not in an interactive rebase.

> Or you could push for a generic mechanism that allows you to customize
> your own default. Something like the below patch could give you what
> you want with:
>
>     GIT_COMPLETION_OPTIONS=all
>     GIT_COMPLETION_EXCLUDES=--open-files-in-pager # and some more
>     . /path/to/git-completion.bash
>
> I'm not going to make a real patch for this since people may want to
> ignore --foo in one command and complete --foo in others... I'm just
> not interested in trying to cover all cases.
>
> -- 8< --
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 0cfa489a8e..9ca0d80cd7 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -40,6 +40,10 @@
>  #   GIT_COMPLETION_OPTIONS
>  #
>  #     When set to "all", complete all possible options
> +#
> +#   GIT_COMPLETION_EXCLUDES
> +#
> +#     Exclude some options from the complete list
>
>  case "$COMP_WORDBREAKS" in
>  *:*) : great ;;
> @@ -298,7 +302,7 @@ __gitcomp_builtin ()
>  	# commands, e.g. "git remote add" becomes remote_add.
>  	local cmd="$1"
>  	local incl="$2"
> -	local excl="$3"
> +	local excl="$3 $GIT_COMPLETION_EXCLUDES"
>
>  	local var=__gitcomp_builtin_"${cmd/-/_}"
>  	local options
> -- 8< --