> 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. I had a look at the XDG specification[1], unfortunately it doesn't mention the case of empty search path entries. Do you know what's the preferred default behavior? Again, thanks a lot for your feedback, I've implemented your suggestions. Best regards, Maximilian [1] https://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html diff --git a/Completion/X/Command/_setxkbmap b/Completion/X/Command/_setxkbmap index f7310ecdd..486268ed7 100644 --- a/Completion/X/Command/_setxkbmap +++ 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 -a searchdirs=(${XDG_DATA_HOME:-~/.local/share} ${(s.:.)XDG_DATA_DIRS:-/usr/lib:/usr/share:/usr/local/lib:/usr/local/share}) + for dir in $searchdirs; do + fullname="$dir/X11/xkb" + if [ -d $fullname ] ; then + sourcedir=$fullname break fi done On Tue, Sep 18, 2018 at 07:32:52PM +0000, Daniel Shahaf wrote: > 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