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

[Duy Nguyen <pclouds@gmail.com>]

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 <pclouds@gmail.com>
> > ---
> >  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.

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< --