Re: [PATCH v3 04/42] git-completion.bash: introduce __gitcomp_builtin

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
>




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux