Re: [PATCH] Add completion for cpupower command

[Oliver Kiddle <okiddle@xxxxxxxxxxx>]

On 1 Aug, I wrote:
> Thanks for contributing this. I've taken a look and have a few comments.
> Mostly all minor points.

Following patch implements these comments. I've left the frequency
steps alone: it doesn't work on any system I've tried it on but
that's probably because my hardware doesn't support it. Newer
versions of cpupower list governors in a slightly different format,
hence the issue Christian reported. This should now work with both
the older format and the newer (at least up to version 4.8).

Oliver

diff --git a/Completion/Linux/Command/_cpupower b/Completion/Linux/Command/_cpupower
index 406893e..6713323 100644
--- a/Completion/Linux/Command/_cpupower
+++ b/Completion/Linux/Command/_cpupower
@@ -1,102 +1,119 @@
 #compdef cpupower
 
-local -a ret state cmds args expl context line
+local curcontext="$curcontext" ret=1
+local -a state line expl cmds args
+typeset -A opt_args
 
-_arguments \
-  '(-h --help)'{-h,--help}'[print usage information]' \
-  '(-v --version)'{-v,--version}'[print package name and version]' \
+_arguments -C \
+  '(- :)'{-h,--help}'[print help information]' \
+  '(- :)'{-v,--version}'[print version information]' \
+  '(-d --debug)'{-d,--debug}'[enable debug output]' \
+  '(-c --cpu)'{-c,--cpu}'[limit values to specific processor cores]:cpu' \
   ':cmd:->cmds' \
   '*::arg:->args' && ret=0
 
+cmds=(
+  'frequency-info:show current frequency info'
+  'frequency-set:set frequency parameters'
+  'idle-info:show current idle state info'
+  'idle-set:set idle state parameters'
+  'info:show global power parameters'
+  'set:set global power parameters'
+  'monitor:report frequency and idle statistics'
+  'help:print usage information'
+)
 case $state in
   cmds)
-    cmds=(
-      'frequency-info:show current frequency info'
-      'frequency-set:set frequency parameters'
-      'idle-info:show current idle state info'
-      'idle-set:set idle state parameters'
-      'info:show global power parameters'
-      'set:set global power parameters'
-      'monitor:report frequency and idle statistics'
-      'help:print usage information'
-    )
-    _describe cmd cmds && ret=0
+    _describe command cmds && ret=0
   ;;
   args)
-    args=( )
+    curcontext="${curcontext%:*}-$words[1]"
     case ${words[1]} in
+      help)
+        _describe command cmds
+        return
+      ;;
       frequency-info)
-        args+=(
-          '(-e --debug)'{-e,--debug}'[print debug info]'
-          '(-f --freq)'{-f,--freq}'[show current frequency]'
-          '(-w --hwfreq)'{-w,--hwfreq}'[show current hardware frequency]'
-          '(-l --hwlimits)'{-l,--hwlimits}'[show min/max frequency allowed]'
-          '(-d --driver)'{-d,--driver}'[show the kernel driver in use]'
-          '(-p --policy)'{-p,--policy}'[show the current cpufreq policy]'
-          '(-g --governors)'{-g,--governors}'[show available governers]'
-          '(-r --related-cpus)'{-a,--related-cpus}'[show cpus that run at the same frequency]'
-          '(-a --affected-cpus)'{-a,--affected-cpus}'[show software controlled cpus]'
-          '(-s --stats)'{-s,--stats}'[show cpufreq statistics]'
-          '(-y --latency)'{-y,--latency}'[show frequency change latency]'
-          '(-o --proc)'{-o,--proc}'[print old style proc info]'
+        args=(
           '(-m --human)'{-m,--human}'[use human readable output]'
           '(-n --no-rounding)'{-n,--no-rounding}'[disable rounding of values]'
+          - '(info)'
+          {-e,--debug}'[print debug info]'
+          {-f,--freq}'[show current frequency]'
+          {-w,--hwfreq}'[show current hardware frequency]'
+          {-l,--hwlimits}'[show min/max frequency allowed]'
+          {-d,--driver}'[show the kernel driver in use]'
+          {-p,--policy}'[show the current cpufreq policy]'
+          {-g,--governors}'[show available governers]'
+          {-r,--related-cpus}'[show cpus that run at the same frequency]'
+          {-a,--affected-cpus}'[show software controlled cpus]'
+          {-s,--stats}'[show cpufreq statistics]'
+          {-y,--latency}'[show frequency change latency]'
         )
-        ;;
+        [[ -n $opt_args[(i)-(c|-cpu)] ]] || args+=(
+          {-o,--proc}'[print old style proc info]'
+        )
+      ;;
       frequency-set)
-        args+=(
-          '(-d --min)'{-d,--min}'[new minimum frequency]:freq:->freq'
-          '(-u --max)'{-u,--max}'[new maximum frequency]:freq:->freq'
-          '(-g --governor)'{-g,--governor}'[new cpufreq governor]:gov:->gov'
-          '(-f --freq)'{-f,--freq}'[new frequency for userspace governor]:freq->freq'
+        args=(
+          '(-d --min)'{-d+,--min}'[new minimum frequency]:frequency:->frequencies'
+          '(-u --max)'{-u+,--max}'[new maximum frequency]:frequency:->frequencies'
+          '(-g --governor)'{-g+,--governor}'[new cpufreq governor]:governor:->governors'
+          '(-)'{-f+,--freq}'[new frequency for userspace governor]:frequency:->frequencies'
           '(-r --related)'{-r,--related}'[modify all hardware related cpus]'
         )
-        ;;
+      ;;
       idle-info)
-        args+=(
+        args=(
           '(-f --silent)'{-f,--silent}'[print summary only]'
-          '(-e --proc)'{-e,--proc}'[print old style proc info]'
+          '(-e --proc)'{-e,--proc}'[print old style proc info (deprecated)]'
         )
-        ;;
+      ;;
       idle-set)
-        args+=(
-          '(-d --disable)'{-d,--disable}'[disable specific sleep state]:stateno'
-          '(-e --enable)'{-e,--enable}'[enable specific sleep state]:stateno'
-          '(-D --disable-by-latency)'{-D,--disable-by-latency}'[disable state based on latency]:latency'
+        args=(
+          '(-d --disable)'{-d+,--disable}'[disable specific sleep state]:state no'
+          '(-e --enable)'{-e+,--enable}'[enable specific sleep state]:state no'
+          '(-D --disable-by-latency)'{-D+,--disable-by-latency}'[disable state based on latency]:latency'
           '(-E --enable-all)'{-E,--enable-all}'[enable all idle states]'
         )
-        ;;
+      ;;
       info)
-        args+=(
+        args=(
           '(-b --perf-bias)'{-b,--perf-bias}'[show intel performance bias value]'
         )
-        ;;
+      ;;
       set)
-        args+=(
-          '(-b --perf-bias)'{-b,--perf-bias}'[set intel performance bias value]:pbias'
+        args=(
+          '(-b --perf-bias)'{-b+,--perf-bias}'[set intel performance bias value]:performance bias'
         )
-        ;;
+      ;;
       monitor)
-        args+=(
-          '-l[list available monitors]'
-          '-m[display the given monitors]:mon'
-          '-i[mesurement interval]:secs'
+        args=(
+          '(-)-l[list available monitors]'
+          '-m+[display specified monitors]:monitor:->monitors'
+          '-i+[measurement interval]:interval (seconds)'
           '-c[schedule on every core]'
           '-v[increase verbosity]'
+          '*:::command: _normal'
         )
-        ;;
+      ;;
     esac
-    _arguments "${args[@]}" && ret=0
+    _arguments -C -s "$args[@]" && ret=0
     case $state in
-      freq)
-	compadd $(cpupower frequency-info |
-		sed -n 's/ //g; s/,/ /g; s/availablefrequencysteps://p')
-        ;;
-      gov)
-        compadd $(cpupower frequency-info -g | sed 1d)
-        ;;
+      frequencies)
+        _wanted -x frequencies expl frequency compadd $(cpupower frequency-info |
+               sed -n 's/ //g; s/,/ /g; s/availablefrequencysteps://p') && ret=0
+      ;;
+      governors)
+        _wanted governors expl 'scaling governor' compadd \
+            ${=${"$(_call_program governors cpupower frequency-info -g)"}##*:} && ret=0
+      ;;
+      monitors)
+        _sequence _wanted monitors expl 'monitor' compadd - ${${${(M)${(f)"$(_call_program monitors \
+            cpupower monitor -l)"}:#Monitor *}#*\"}%%\"*} && ret=0
+      ;;
     esac
-    ;;
+  ;;
 esac
 
 return ret