Re: zsh virsh completion

[Marko Myllynen <myllynen@xxxxxxxxxx>]

Hi,

On 2016-07-12 01:07, Oliver Kiddle wrote:
> Marko Myllynen wrote:
>> Below is a patch to implement basic completion for the virsh(1) [1]
>> command from libvirt [2], it completes all virsh commands and their
>> respective options.
> 
> The function should probably be named _libvirt then - in much the same
> way as we use e.g. _subversion for svn.

Ok, changed.

>> - virsh help <command> output is pretty much like --help output of any
>> other program, is there a trick to make the option descriptions also
>> available when completing virsh command options?
> 
> If you don't use _arguments --, you can parse the output manually.

The options are pretty self-describing so description for them might be
an additional improvement in the future, based on the comment from Bart
it sounded this might be tricky to get right.

>> - would it make sense (or is it perhaps already possible somehow) to
>> optionally enable "best guess" completion for all commands which have
>> no command specific completion rules available yet? There are lots of
> 
> There are commands where running the command with --help/-h or
> whatever could do something destructive. I forget which command it
> was but I remember years ago finding that a completion was spewing
> messages into syslog. If it was going to be a catch-all default,
> it might be safer to use man pages as the source.

Interesting, thanks for the background info.

>> +_arguments -A "-*" -C -S -s -w \
> 
> -w, really? That's rare.

Yes, virsh accepts e.g. -r -d 0, -r -d0, -rd 0, or -rd0.

>> +  _cache_virsh_cmds=( ${="$(virsh help 2>&1 | awk '!/:/ {print $1}')"} )
> 
> You should use _call_program as a wrapper around virsh.

Ok, using virsh as the tag.

>> +    _values -w options ${=_cache_virsh_cmdopts[$cmd]} && ret=0
> 
> I'm not convinced that _values is gaining you anything over a plain
> compadd (with _wanted). I wouldn't typically use it with generated
> matches. If you want the duplicate removal, use -F words

I started with _wanted/compadd but couldn't get it working and since
_values did the trick I was happy with.

> With this in place there's no filename completion which can be quite
> annoying if you need to complete files. So unless virsh sub-commands
> never take filenames as arguments, I'd have it fallback to _default.
> In this case, using _alternative.  Also, for option completion, it
> should check the prefix-needed style.

Good point on filenames. There are 227 virsh commands (as of libvirt
2.0), I haven't gone thru them all, but some of them have options which
take file names as arguments while others don't. I'm not sure what
would be the most optimal approach here, I now added a _default
fallback in case the user types something starting with / or ./.

Below is the updated patch, hopefully it addresses most of the feedback
properly.

---
 Completion/Unix/Command/_libvirt | 59 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)
 create mode 100644 Completion/Unix/Command/_libvirt

diff --git a/Completion/Unix/Command/_libvirt b/Completion/Unix/Command/_libvirt
new file mode 100644
index 0000000..ac19ac3
--- /dev/null
+++ b/Completion/Unix/Command/_libvirt
@@ -0,0 +1,59 @@
+#compdef virsh
+
+_libvirt () {
+  local curcontext="$curcontext" state line expl ret=1
+
+  case $service in
+  virsh)
+    if (( ! $+_cache_virsh_cmds )); then
+      _cache_virsh_cmds=( ${="$(_call_program virsh virsh help 2>&1 | awk '!/:/ {print $1}')"} )
+    fi
+    if (( ! $+_cache_virsh_cmdopts )); then
+      typeset -gA _cache_virsh_cmdopts
+    fi
+    _arguments -A "-*" -C -S -s -w \
+      '(- *)'{-h,--help}'[print help information and exit]' \
+      '(- *)'{-v,--version=short}'[print short version information and exit]' \
+      '(- *)'{-V,--version=long}'[print long version information and exit]' \
+      '(-c --connect)'{-c+,--connect}'[specify connection URI]:URI:_hosts' \
+      '(-d --debug)'{-d+,--debug}'[set debug level]:level:(0 1 2 3 4)' \
+      '(-e --escape)'{-e+,--escape}'[set escape sequence for console]:sequence' \
+      '(-k --keepalive-interval)'{-k+,--keepalive-interval}'[set keepalive interval]:interval' \
+      '(-K --keepalive-count)'{-K+,--keepalive-count}'[set keepalive count]:count' \
+      '(-l --log)'{-l+,--log}'[specify log file]:file:_files' \
+      '(-q --quiet)'{-q,--quiet}'[quiet mode]' \
+      '(-r --readonly)'{-r,--readonly}'[connect readonly]' \
+      '(-t --timing)'{-t,--timing}'[print timing information]' \
+      '1:command:->virsh_cmds' \
+      '*:cmdopt:->virsh_cmdopts' \
+      && return 0
+      # We accept only virsh command options after the first non-option argument
+      # (i.e., the virsh command itself), this makes it so with the -A "-*" above
+      [[ -z $state ]] && state=virsh_cmdopts
+  ;;
+  esac
+
+  case $state in
+    virsh_cmds)
+      _wanted commands expl 'virsh command' compadd -a _cache_virsh_cmds && ret=0
+    ;;
+    virsh_cmdopts)
+      local cmd
+      if [[ $words[-1] == /* || $words[-1] == ./* ]]; then
+        _default && return 0
+      fi
+      for (( i = 2; i <= $#words; i++ )); do
+        [[ -n "${_cache_virsh_cmds[(r)$words[$i]]}" ]] && cmd=$words[$i] && break
+      done
+      [[ -z $cmd ]] && return 1
+      if [[ -z $_cache_virsh_cmdopts[$cmd] ]]; then
+        _cache_virsh_cmdopts[$cmd]=${(M)${${${${=${(f)"$(_call_program virsh virsh help $cmd 2>&1)"}}/\[}/\]}/\;}:#-[-0-9A-Za-z]*}
+      fi
+      _values -w options ${=_cache_virsh_cmdopts[$cmd]} && ret=0
+    ;;
+  esac
+
+  return ret
+}
+
+_libvirt "$@"

Thanks,

-- 
Marko Myllynen