Re: PATCH: search XDG_DATA_DIRS in _setxkbmap completion

[Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx>]

Maximilian Bosch wrote on Tue, 18 Sep 2018 21:05 +0200:
> Thanks a lot for your feedback!
> 
> I use ZSH as interactive shell quite excessively, but as you might've
> noticed, I'm not that experienced with ZSH scripting, so thanks for
> your patience :D
> 

You're welcome.

> I applied your suggestions and re-checked the patch.
> 
> > How should empty array elements in XDG_DATA_DIRS be handled?  E.g.,
> > XDG_DATA_DIRS=/foo:/bar::/baz ?
> 
> Isn't this handled by `[ -d $fullname ]`?

Depends on how empty elements should be handled.  In some contexts an
empty element means "look in the current working directory", and I
wasn't sure how the XDG spec defines this case.  It's not a blocker but
it would be nice to get this right while we're here.
 
> +++ b/Completion/X/Command/_setxkbmap
> @@ -9,10 +9,12 @@ _setxkbmap() {
>      setopt extendedglob
>  
>      # xkb files may be in different places depending on system
> -    local dir sourcedir
> -    for dir in /usr/lib/X11/xkb /usr/share/X11/xkb; do
> -        if [ -d $dir ] ; then
> -           sourcedir=$dir
> +    local dir sourcedir fullname
> +    local searchdirs="${XDG_DATA_DIRS:-/usr/lib:/usr/share:/usr/local/lib:/usr/local/share}:${XDG_DATA_HOME:-~/.local/share}"

Two tweaks here:

1. XDG_DATA_HOME should be listed before XDG_DATA_DIRS, according to
   the XDG spec.

2. I'd use an array, both because it's cleaner and in case
   $XDG_DATA_HOME contains colons (not likely, but there's no
   maintenance cost to supporting that).  Thus:

    local -a searchdirs=( ${XDG_DATA_HOME:-...} ${(s.:.)XDG_DATA_DIRS:-...} )
    for dir in $searchdirs; do

> +    for dir in ${(s.:.)searchdirs}; do
> +        fullname="$dir/X11/xkb"
> +        if [ -d $fullname ] ; then
> +           sourcedir=$fullname
>             break
>          fi
>      done

Cheers,

Daniel