On Mon, Oct 06, 2014 at 09:37:20PM +0200, Oliver Kiddle wrote:
> fREW Schmidt wrote:
> >
> > If anything is wrong just let me know, this would be my first
> > contribution to zsh so mistakes wouldn't surprise me.
>
> I've got a few comments.
>
> > +#compdef sv
> > +
> > +_arguments \
> > + '-v[verbose]' \
> > + '-w:wait time' \
>
> What's the units for the time? How about something like:
> '-w[specify wait time]:time (seconds)'
Good idea
> > + ':command:->command' \
> > + '*::options:->options'
>
> If you use states with _arguments, you should declare a few array
> variables local: context, state and line. You then need to take care to
> make use of the context variable when calling other functions. In most
> cases, including the one above, only one state is possible at a time. So
> in this case, you should declare:
> local curcontext="$curcontext" state line
> and pass the -C option to _arguments.
Ok, I think I did this right this time.
> > + sv_ary=(
> > + 'status':'Get status of service (and log service if available)'
>
> The usual convention, which you have done elsewhere, is to start
> descriptions with a lowercase letter.
Woops! Fixed.
> > + _describe -t commands "sv commands" sv_ary -V sv_commands
> > + _describe -t commands "sv LSM init compat" sv_lsb_ary -V sv_init_compat
> > + _describe -t commands "sv additional commands" '("check:check status of service")' -V sv_addl_comm
> > + return
>
> This is ignoring the return status of the first two calls to _describe.
> The return value of a completion function determines whether the
> completion system carries on trying to find more matches, perhaps
> approximate matching if you have that configured. It's common to use a
> variable named ret for the return status: look for some examples.
>
> Perhaps also consider using three different tags for the matches and using
> _alternative or _tags with a loop.
ok, I switched it to use _alternative and 3 functions as seems to be
somewhat common in other compltion scripts in the codebase.
> > + $SVDIR/*(N)
>
> Is SVDIR really always set or is there a default value that it could
> fall back on, e.g: ${SVDIR:-/service}
This raises an interesting question. I did what you said, but as
Christian Neukirchen pointed out, on his system the default SVDIR is
/var/service, and on ubuntu the default is /etc/service. I guess it's
up to the packager to tweak the script or something?
Additionally, I like Christian Neukirchen's single-character shortcut
completion. For majority of the commands they work already since
nothing else starts with that letter, but s and c both have multiple
options. If I understood what I was doing more I'd take his idea, but
I don't.
Anyway, see a new version attached. Let me know what else is (still?)
wrong.
Thanks!
(Again, please CC me in responses.)
--
fREW Schmidt
https://blog.afoolishmanifesto.com
From 05f0fab3a7ddc666c8c0b89fd48b8c05f41b7b52 Mon Sep 17 00:00:00 2001
From: Arthur Axel 'fREW' Schmidt <frioux@xxxxxxxxx>
Date: Sun, 5 Oct 2014 14:52:42 -0500
Subject: [PATCH] new completion for "sv"
See http://smarden.org/runit/sv.8.html
---
Completion/Unix/Command/_sv | 73 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 73 insertions(+)
create mode 100644 Completion/Unix/Command/_sv
diff --git a/Completion/Unix/Command/_sv b/Completion/Unix/Command/_sv
new file mode 100644
index 0000000..b2fedc5
--- /dev/null
+++ b/Completion/Unix/Command/_sv
@@ -0,0 +1,73 @@
+#compdef sv
+
+_sv_commands() {
+ local -a sv_ary
+ sv_ary=(
+ 'status':'get status of service (and log service if available)'
+ 'up':'start if service is running. If service stops, restart'
+ 'down':'send SIGTERM and SIGCONT if service is running. After it stops, do not restart'
+ 'once':'start if service is not running. Do not restart if it stops'
+ 'pause':'send SIGSTOP if service is running'
+ 'cont':'send SIGCONT if service is running'
+ 'hup':'send SIGHUP if service is running'
+ 'alarm':'send SIGALRM if service is running'
+ 'interrupt':'send SIGINT if service is running'
+ 'quit':'send SIGQUIT if service is running'
+ '1':'send SIGUSR1 if service is running'
+ '2':'send SIGUSR2 if service is running'
+ 'term':'send SIGTERM if service is running'
+ 'kill':'send SIGKILL if service is running'
+ 'exit':'send SIGTERM and SIGCONT if service is running. Do not restart service.'
+ )
+ _describe -t commands "sv commands" sv_ary -V sv_commands
+}
+
+_sv_lsb() {
+ local -a sv_lsb_ary
+ sv_lsb_ary=(
+ 'start':'up with check/timeout'
+ 'stop':'down with check/timeout'
+ 'reload':'hup with check'
+ 'restart':'down and up with check'
+ 'shutdown':'exit with check/timeout'
+ 'force-stop':'stop with kill on timeout'
+ 'force-reload':'reload with kill on timeout'
+ 'force-restart':'restart with kill on timeout'
+ 'force-shutdown':'shutdown with kill on timeout'
+ 'try-restart':'restart if service is already running'
+ )
+
+ _describe -t lsb-commands "sv LSM init compat" sv_lsb_ary -V sv_init_compat
+}
+
+_sv_additional() {
+ _describe -t additional-commands "sv additional commands" '("check:check status of service")' -V sv_addl_comm
+}
+
+local curcontext="$curcontext" state line
+_arguments -C \
+ '-v[verbose]' \
+ '-w[wait time]:time (seconds)' \
+ '1: :->command' \
+ '*:: :->options'
+
+case $state in
+ (command)
+ _alternative \
+ _sv_commands \
+ _sv_lsb \
+ _sv_additional
+ ;;
+
+ (options)
+ local -a sv_services
+ local svdir=${SVDIR:-/service}
+ sv_services=(
+ $svdir/*(N)
+ $svdir/*/log(N)
+ )
+
+ sv_services=( ${sv_services#$svdir/} )
+ _describe -t services "sv services" sv_services
+ ;;
+esac
--
2.0.0.390.gcb682f8
Attachment:
pgpT1XymHH9ND.pgp
Description: PGP signature