Re: Completion script for the ctags program

[Oliver Kiddle <opk@xxxxxxx>]

On 11 Mar, Jacob Gelbman wrote:
> Yes, here it is: 

Thanks for sending the update. I've committed it as-is to the repository
because it is a definite improvement on the earlier file and that
doesn't prevent us from further iterating on it. It is easier if you can
send changes as patches. Use the diff -u, or git diff commands to create
patches rather than posting full files. The previous files also had DOS
line endings - if someone other than me pushes any updates, please
ensure we only have Unix line endings.

> #compdef ctags arduino-ctags ctags-exuberant ctags-universal

exuberant is installed as exctags on my system so that name could be
added.

> local context state line expl
> local -A opt_args
> local -a arguments
>
> if [ -z "$_ctags_type" ]; then
>   local output=`ctags --version 2>&1`
>   if [[ "$output" = *Universal\ Ctags* ]]; then
>     _ctags_type="universal"
>   elif [[ "$output" = *Exuberant\ Ctags* ]]; then
>     _ctags_type="exuberant"
>   elif [[ "$output" = *usage:\ ctags* ]]; then
>     _ctags_type="bsd"
>   elif [[ "$output" = *Emacs* ]]; then
>     _ctags_type="etags"
>   else
>     _ctags_type="universal"
>   fi
> fi

As was mentioned in a previous review, use _pick_variant for this. It
does exactly this in a generic manner and handles things like caching
results. This code above will cache one result even if the user has both
ctags and ctags-exuberant installed.

> if [ "$_ctags_type" = "etags" ]; then
>   _etags
>   return $?
> fi

WE should perhaps pull the contents of _etags into this function and
handle them all together.

>   if [[ "$PREFIX" = -* ]]; then

If the prefix-needed style is set to false, that condition is not
applicable. If generating the list of languages is fast enough, I would
generate it unconditionally and store the result in an array so that it
can be used from a variety of places.

>     local -a languages=(`_ctags_languages`)

In general, I would recommend using $(...) instead of `...` because it
is simple to nest. That doesn't matter here as such.

>     local -a languages2

That variable isn't used.

>     for language in $languages; do
>       arguments+=("--$language-kinds=-:kinds")

The latter kinds should be singular - kind. You can actually avoid the
need for a for loop by using --${^languages}"-kinds=-:kind"

If you keep the for loop, language does need to be declared local.

> if [ "$state" = "language" ]; then

The [[ ... ]] condition form is usually preferable to [ ... ] because
the parser handles it and avoids certain tricky issues. A case statement
would work here though. Or even better, make _ctags_languages a proper
completion function so that you can call it directly with options such
as -S = or with _sequence for the comma-separated list.

Thanks,

Oliver