Re: PATCH: mysql completions

[Sven Wischnowsky <wischnow@xxxxxxxxxxxxxxxxxxxxxxx>]

Adam Spiers wrote:

> This turned out to be a really good way of getting my head round
> recent changes to the completion system.  Comments on the style (and
> correctness, especially relating to tags) welcome ...

The tags stuff looks fine, as far as I can see.

> ...
> 
> +_mysql_databases () {
> +  local _mysql_user _mysql_port _mysql_host _mysql_params
> +  _mysql_get_identity
> +
> +  local _mysql_databases
> +  _mysql_databases=(
> +                    ${(f)~~"$( echo "show databases" |
> +                               mysql "$_mysql_params[@]" )"}
> +                   )
> +  shift _mysql_databases
> +
> +  compadd "$expl[@]" - $_mysql_databases
> +}

This works, because $expl is set in all callers, but I think it would
be nicer to make this explicit by using $@. _argument gives $expl as
the arguments anyway (the way _mysql_databases is called from it) and
for the call from _*_commands it could easily be added. Maybe that's
just personal taste.

> +_mysql_tables () {
> +  local _mysql_user _mysql_port _mysql_host _mysql_params
> +  _mysql_get_identity
> +
> +  local _mysql_tables
> +  _mysql_tables=(
> +                 ${(f)~~"$( echo "show tables" |
> +                              mysql "$_mysql_params[@]" $1 )"}
> +                )
> +  # remove header
> +  shift _mysql_tables
> +
> +  compadd "$expl[@]" - $_mysql_tables
> +}

Almost the same, I think it would be better to change the calls to use 
`{ _mysql_tables $line[1] $expl }'.

> ...
>
> +_mysql_utils "$@"

[[ -o kshautoload ]] || _mysql_utils "$@"

(I hate kshautoload, too ;-)

Bye
 Sven


--
Sven Wischnowsky                         wischnow@xxxxxxxxxxxxxxxxxxxxxxx