From: Daniel Shahaf <d.s@daniel.shahaf.name>
To: Maximilian Bosch <maximilian@mbosch.me>, zsh-workers@zsh.org
Subject: Re: PATCH: search XDG_DATA_DIRS in _setxkbmap completion
Date: Tue, 18 Sep 2018 19:32:52 +0000 [thread overview]
Message-ID: <1537299172.1213094.1512566000.19322CCD@webmail.messagingengine.com> (raw)
In-Reply-To: <20180918190516.2w3tyiugdmx6wzqw@hauptschuhle>
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
next prev parent reply other threads:[~2018-09-18 19:33 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-18 12:49 Maximilian Bosch
2018-09-18 16:10 ` Daniel Shahaf
2018-09-18 19:05 ` Maximilian Bosch
2018-09-18 19:32 ` Daniel Shahaf [this message]
2018-09-19 20:36 ` Maximilian Bosch
2018-09-19 21:30 ` Daniel Shahaf
2018-09-19 4:53 TS
2018-09-19 14:29 ` Daniel Shahaf
2018-09-19 19:53 ` TS
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1537299172.1213094.1512566000.19322CCD@webmail.messagingengine.com \
--to=d.s@daniel.shahaf.name \
--cc=maximilian@mbosch.me \
--cc=zsh-workers@zsh.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/zsh/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).