Re: [PATCH v3 42/42] git-completion.bash: add GIT_COMPLETION_OPTIONS=all config
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Feb 11, 2018, 14:47 UTC
- Message-ID
- <874lmnk2gb.fsf@evledraar.gmail.com>
- In-Reply-To
- <20180210092945.GA6370@ash>
On Sat, Feb 10 2018, Duy Nguyen jotted:
Show 62 quoted lines
> 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.
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.
Show 38 quoted lines
> 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< --