Thanks for working on this. I anticipated that the completion script lack some options, but wow, I didn't expect that there are so many missing. On Fri, Feb 9, 2018 at 12:01 PM, Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> wrote: > This is a __gitcomp wrapper that will execute > > git ... --git-completion-helper > > to get the list of completable options. The call will be made only > once and cached to avoid performance issues, especially on Windows. Nit: the call will be made every time; 'git ... --git-completion-helper' will be executed only once. > __gitcomp_builtin() allows callers to change its output a bit by adding > some more options, or removing some. > > - Current --git-completion-helper for example does not output --no-foo > form, this has to be added manually by __gitcomp_builtin() callers > when necessary > > - Some options from --git-completion-helper should only be available in > certain conditions (e.g. --continue and friends). __gitcomp_builtin() > callers can remove them if the conditions are not met. > > Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> > --- > contrib/completion/git-completion.bash | 33 ++++++++++++++++++++++++++ > 1 file changed, 33 insertions(+) > > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash > index 3683c772c5..85e7f26328 100644 > --- a/contrib/completion/git-completion.bash > +++ b/contrib/completion/git-completion.bash > @@ -280,6 +280,39 @@ __gitcomp () > esac > } > > +# This function is equivalent to > +# > +# __gitcomp "$(git xxx --git-completion-helper) ..." > +# > +# except that the output is cached. Accept 1-3 arguments: > +# 1: the git command to execute, this is also the cache key > +# 2: extra options to be added on top (e.g. negative forms) > +# 3: options to be excluded > +__gitcomp_builtin () Please excuse the bikeshed at v3, but I don't like the name of this function. It indicates that it completes builtins, but it completes options of builtins, and even then only the options of those using parse options. Furthermore, the '__gitcomp' prefix is usually used for functions that merely put words into COMPREPLY, but this function does a whole lot more (getting the options from builtins, include and exclude options, caching). Alas I don't have any great name; __git_complete_options is better, because it uses the right function name prefix, but only slightly better, because it can't generally be used to complete options, as it won't work with scripts or with builtins not using parse options (though with time more scripts will be turned into builtins and more builtins will use parse options). I'm not sure it's that match better to make it worth changing fourty-odd patches. > +{ > + # spaces must be replaced with underscore for multi-word > + # commands, e.g. "git remote add" becomes remote_add. > + local cmd="$1" The alternative would be 'command subcommand', i.e. keeping that space, and in that case we could spare the ${cmd/_/ } in this function, and could even say '__gitcomp "$(git $1 --git-completion-helper)" in the equivalent example above; OTOH we would need quoting on all callsites with subcommands. Again, I'm not sure it's worth it. > + local incl="$2" > + local excl="$3" > + > + local var=__gitcomp_builtin_"${cmd/-/_}" > + local options > + eval "options=\$$var" > + > + if [ -z "$options" ]; then > + # leading and trailing spaces are significant to make > + # option removal work correctly. > + options=" $(__git ${cmd/_/ } --git-completion-helper) $incl " > + for i in $excl; do > + options="${options/ $i / }" > + done > + eval "$var=\"$options\"" > + fi > + > + __gitcomp "$options" > +} > + > # Variation of __gitcomp_nl () that appends to the existing list of > # completion candidates, COMPREPLY. > __gitcomp_nl_append () > -- > 2.16.1.207.gedba492059 >