[PATCH] Completion: Improve _pgrep

[dana <dana@xxxxxxx>]

Some pgrep/pkill improvements i'd had sitting for a while:

* Add full support for procps/-ng (Linux)
* Support Dragonfly as with FreeBSD
* Add one or two options missing from other variants (like -l)
* Use -o+ form for options accepting arguments
* Improve option exclusivity in a few places
* Improve descriptions in several places
* Don't complete shell-specific signals (hopefully there aren't any systems out
  there that actually have a SIGEXIT or SIGDEBUG)
* Account for numeric signals and symbolic ones prefixed by 'SIG', so that e.g.
  `kill -9 -<TAB>` doesn't still offer -KILL
* On non-Linux systems, only complete signals as the first argument (let me know
  if there's a better way than how i did this)

I also alphabetised the option specs. Sorry if that makes an extra mess of the
diff, but i had a really hard time keeping track of which ones were accounted
for when they were all randomly ordered. :/

dana


diff --git a/Completion/Unix/Command/_pgrep b/Completion/Unix/Command/_pgrep
index ec3f8bfd0..c4db2b73f 100644
--- a/Completion/Unix/Command/_pgrep
+++ b/Completion/Unix/Command/_pgrep
@@ -1,75 +1,138 @@
 #compdef pgrep pkill
 
-local curcontext="$curcontext" state line ret=1 expl
+# Notes:
+# - We assume that Linux systems use procps-ng — specifically, procps-ng >=3.3.4
+#   (which changed the behaviour of -f and added -a)
+# - The (s) exclusion group is used here to account for the fact that a signal
+#   given to pkill must be the first argument on non-Linux platforms. The (ss)
+#   exclusion group is used on Linux to keep the other signal options exclusive
+#   with --signal
+# - We don't really need to keep pgopts and pkopts separate, but it seems like
+#   it should make things a bit easier to follow
+# - @todo We could complete log-in classes given to -c
+# - @todo We could complete routing tables given to -T
+
+local curcontext="$curcontext" state line ret=1 expl pgopts pkopts i
 typeset -A opt_args
-typeset -a arguments
-
-arguments=('-P[parent process id]:parent process id:->ppid'
-     '-F[match only in process in pidfile]:files:_files'
-     '-g[match only in process group ids]:group:->pgid'
-     '-G[match only real group id]:group:_groups'
-     '-j[match only in processes inside jails]:jail id:_sequence _jails -0 -o jid'
-     '-J[match only in project ids]:project id:->projid'
-     '-M[extract the name list from the specified core]:files:_files'
-     '-N[extract the name list from the specified system]:files:_files'
-     '-s[match only session id]:session id:->sid'
-     '-t[match only controlled by terminal]:terminal device:_sequence _ttys -d'
-     '-T[match only in processes specified routing table in rtable]'
-     '-u[match only effective user id]:user:_users'
-     '-U[match only real user id]:user:_users'
-     '(-n)-o[oldest process]'
-     '(-o)-n[newest process]'
-     '-a[include process ancestors in the match list]'
-     '-c[print a count of matching processes]'
-     '-f[match against full command line]'
-     '-i[ignore case distinctions]'
-     '-I[confirmation before attempting to single each process]'
-     '-L[given pidfile must be locked]'
-     '-q[do not write anything to standard output]'
-     '-S[search also in system processes]'
-     '-v[negate matching]'
-     '-w[show thread ids instead of pids]'
-     '-x[match exactly]'
-     '-z[match only in zones]:zone:_zones')
-
-if [[ $service == 'pgrep' ]]; then
-  arguments+=('-d[output delimiter]:delimiter:compadd ${(s\:\:)IFS}'
-        '-l[list name in addition to id]')
-fi
-
-local optchars
-case "$OSTYPE" in
+typeset -a arguments sig_arguments aopts
+
+# These arguments (a) are common to all variants (like -x), (b) are the most
+# common amongst all variants (like -a), or (c) have a single unambiguous
+# meaning amongst all variants (like --help). Many of them are filtered out or
+# overridden below
+arguments=(
+  '(s)-a[include process ancestors in match list]'
+  '(s)-c+[match only on specified login class]:login class'
+  '(s -F --pidfile)'{-F+,--pidfile=}'[match only processes in specified PID file]:PID file:_files'
+  '(s -f --full)'{-f,--full}'[match against full command line]'
+  '(s -G --group)'{-G+,--group=}'[match only on specified real group IDs]: :_sequence _groups'
+  '(s -g --pgroup)'{-g+,--pgroup=}'[match only on specified process group IDs]: :->pgid'
+  '(s : * -)'{-h,--help}'[display help information]'
+  '(s)-I[request confirmation before signalling each process]'
+  '(s)-i[ignore case distinctions]'
+  '(s)-J+[match only on specified project IDs]: :->projid'
+  '(s)-j+[match only on specified jail IDs]:jail:_sequence _jails -0 -o jid'
+  '(s -L --logpidfile)'{-L,--logpidfile}'[fail if PID file not locked (with -F)]'
+  '(s)-M+[extract name list from specified core]:core file:_files'
+  '(s)-N+[extract name list from specified system]:system file:_files'
+  '(s -o -n --oldest --newest)'{-n,--newest}'[match newest process]'
+  '(s -o -n --oldest --newest)'{-o,--oldest}'[match oldest process]'
+  '(s -P --parent)'{-P+,--parent=}'[match only on specified parent process IDs]: :->ppid'
+  '(s -l)-q[suppress normal output]'
+  '(s)-S[search also in system processes (kernel threads)]'
+  '(s -s --session)'{-s+,--session=}'[match only on specified process session IDs]: :->sid'
+  '(ss)--signal=[specify signal to send to process]:signal:(${signals\:#(EXIT|ZERR|DEBUG)})'
+  '(s)-T+[match only on specified routing table]:routing table'
+  '(s -t --terminal)'{-t+,--terminal=}'[match only on specified controlling terminals]:terminal device:_sequence _ttys -d'
+  '(s -U --uid)'{-U+,--uid=}'[match only on specified real user IDs]: :_sequence _users'
+  '(s -u --euid)'{-u+,--euid=}'[match only on specified effective user IDs]: :_sequence _users'
+  '(s -v --inverse)'{-v,--inverse}'[negate matching]'
+  '(s -x --exact)'{-x,--exact}'[match process name or command line (with -f) exactly]'
+  '(s)--ns=[match only on same namespaces as specified PID]: :_pids'
+  '(s)--nslist=[match only on specified namespaces (with --ns)]:namespace:(ipc mnt net pid user uts)'
+  '(s : * -)'{-V,--version}'[display version information]'
+  '(s)-z+[match only on specified zone IDs]:zone:_sequence _zones'
+)
+[[ $service == pgrep ]] && arguments+=(
+  '(s -d --delimiter)'{-d+,--delimiter=}'[specify output delimiter]:delimiter:compadd ${(s<>)IFS}'
+  '(s -q)-l[display process name (and arguments with -f)]'
+  '(s -w --lightweight)'{-w,--lightweight}'[show all thread IDs instead of PID]'
+)
+[[ $service == pkill ]] && arguments+=(
+  '(s -e --echo)'{-e,--echo}'[display signalled process]'
+  '(s)-l[display kill command]'
+)
+
+case $OSTYPE in
   linux*)
-    optchars="cflvxdnoPgsuUGtw"
+    # Note: We deliberately exclude -v but not --inverse from pkill
+    pgopts=acdFfGghLlnoPstUuVvwx-
+    pkopts=ceFfGghLnoPstUuVx-
+    arguments=(
+      ${arguments:#((#s)|*\))(\*|)-[acl]*}
+      '(-c --count)'{-c,--count}'[display count of matching processes]'
+    )
+    [[ $service == pgrep ]] && arguments+=(
+      '(-a -l --list-full --list-name)'{-a,--list-full}'[display full command line]'
+      '(-a -l --list-full --list-name)'{-l,--list-name}'[display process name]'
+    )
     ;;
-  freebsd*)
-    optchars="LSafilnoqvxFGMNPUdgjstu"
+  dragonfly*|freebsd*)
+    pgopts=acdFfGgijLlMNnoPqSstUuvx
+    pkopts=acFfGgIijLlMNnoPstUuvx
     ;;
   openbsd*)
-    optchars="flnoqvxdGgPsTtUu"
+    pgopts=dfGglnoPqsTtUuvx
+    pkopts=fGgIlnoPqsTtUuvx
     ;;
   darwin*)
-    optchars="LafilnoqvxFGPUdgtu"
+    pgopts=adFfGgiLlnoPqtUuvx
+    pkopts=aFfGgIiLlnoPtUuvx
     ;;
   solaris*)
-    optchars="flvxdnoPgsuUGJtTcz"
-    arguments=( ${arguments##-T*} )
-    arguments=( ${arguments##-c*} )
-    arguments+=( '-T[match only processes in task ids]:taskid:->task' )
-    arguments+=( '-c[match only processes in contract ids]:taskid:->contract' )
+    pgopts=cdfGgJlnoPsTtUuvxz
+    pkopts=cfGgJnoPsTtUuvxz
+    arguments=(
+      ${arguments:#((#s)|*\))(\*|)-[cT]*}
+      '-c+[match only on specified contract IDs]: :->contract'
+      '-T+[match only on specified task IDs]: :->task'
+    )
     ;;
   *)
-    optchars="flvxdnoPgsuUGt"
+    pgopts=dfGgilnPstUuvx
+    pkopts=fGgilnPstUuvx
     ;;
 esac
-# Only keep relevant arguments according to $optchars.
-arguments=( ${(M)arguments:#(|\*)(|\(*\))-[$optchars]*}
-     '*:process name:->pname')
-if [[ $service == 'pkill' ]]; then
-  arguments+=('-'${^signals}'[signal]')
+
+if [[ $service == pgrep ]]; then
+  arguments=( ${(M)arguments:#((#s)|*\))(\*|)-[$pgopts]*} )
+else
+  arguments=( ${(M)arguments:#((#s)|*\))(\*|)-[$pkopts]*} )
+
+  if [[ $OSTYPE == linux* ]]; then
+    sig_arguments=( + '(ss)' )
+  else
+    sig_arguments=( + '(s)' )
+  fi
+
+  # Make sure to exclude zsh-specific signals from our signal list. This
+  # presumes that EXIT is always listed first and ZERR/DEBUG last — see
+  # signames.c
+  for (( i = 1; i <= $#signals; i++ )); do
+    [[ $signals[i] == (EXIT|ZERR|DEBUG) ]] && continue
+    sig_arguments+=( '(--signal)-'$signals[i] )
+    # Sometimes zsh will only have the number for a signal
+    [[ $signals[i] == <-> ]] || sig_arguments+=(
+      '!(--signal)-SIG'$signals[i]
+      '!(--signal)-'$(( i - 1 ))
+    )
+  done
 fi
 
-_arguments -C -s -w $arguments && ret=0
+arguments+=( $sig_arguments + o '(s)*: :->pname' )
+
+[[ $OSTYPE == linux* ]] || aopts+=( -A '*-' )
+_arguments -C -s -S $aopts : $arguments && ret=0
 
 case $state in
   (sid)
@@ -87,7 +150,7 @@ case $state in
       sid=(${(uon)$(ps -A -o sid=)})
     fi
 
-    _wanted sid expl 'session id' compadd -S ',' -q -F used $sid
+    _wanted sid expl 'session ID' compadd -S ',' -q -F used $sid
     ;;
 
   (ppid)
@@ -101,7 +164,7 @@ case $state in
       ppid=(${(uon)$(ps -A -o ppid=)})
     fi
 
-    _wanted ppid expl 'parent process id' compadd -S ',' -q -F used $ppid
+    _wanted ppid expl 'parent process ID' compadd -S ',' -q -F used $ppid
     ;;
 
   (pgid)
@@ -115,7 +178,7 @@ case $state in
       pgid=(${(uon)$(ps -A -o pgid=)})
     fi
 
-    _wanted pgid expl 'process group id' compadd -S ',' -q -F used $pgid
+    _wanted pgid expl 'process group ID' compadd -S ',' -q -F used $pgid
     ;;
 
   (projid)
@@ -125,7 +188,7 @@ case $state in
     used=(${(s:,:)IPREFIX})
     projid=(${(uon)$(ps -A -o project=)})
 
-    _wanted projid expl 'project id' compadd -S ',' -q -F used $projid
+    _wanted projid expl 'project ID' compadd -S ',' -q -F used $projid
     ;;
 
   (contract)
@@ -135,7 +198,7 @@ case $state in
     used=(${(s:,:)IPREFIX})
     ctid=(${(uon)$(ps -A -o ctid=)})
 
-    _wanted ctid expl 'contract id' compadd -S ',' -q -F used $ctid
+    _wanted ctid expl 'contract ID' compadd -S ',' -q -F used $ctid
     ;;
 
   (task)
@@ -145,7 +208,7 @@ case $state in
     used=(${(s:,:)IPREFIX})
     taskid=(${(uon)$(ps -A -o project=)})
 
-    _wanted taskid expl 'task id' compadd -S ',' -q -F used $taskid
+    _wanted taskid expl 'task ID' compadd -S ',' -q -F used $taskid
     ;;
 
   (pname)