[PATCH] _pgrep: simplify the processing of '->state' actions

[PATCH] _pgrep: simplify the processing of '->state' actions

[Jun T]

Simplify 'case $state in ... esac' in _pgrep (and fix a few bugs).

'ps -A' works on all systems (including FreeBSD, although not yet
documented), and 'ps -o args=' can be used to get the list of full
command lines on all systems.
# I didn't check on solaris, but '-o args=' has been used in solaris
# before this patch and I assume it works.

When completing a comma-separated list of ID's, commands like
    compset -P '*,'
    used=(${(s:,:)IPREFIX})
    compadd -F used
were used to ignore the ID's already in the list. But this was not
working if there was no space between the option letter and the list,
for example 'pgrep -P<TAB>'. I think it is better to use _sequence.

On FreeBSD, if the option -S (search also in system processes) is on
the command line, 'ps -axH' was used to get the list of process names.
But the "system processes" are already included by 'ps -A'; 'ps -H'
includes all the threads in multi-threaded processes, and introduces
lots of wrong completions. I just removed the special treatment of
"FreeBSD with option -S".


diff --git a/Completion/Unix/Command/_pgrep b/Completion/Unix/Command/_pgrep
index 86aef3462..3b4d082a7 100644
--- a/Completion/Unix/Command/_pgrep
+++ b/Completion/Unix/Command/_pgrep
@@ -1,7 +1,7 @@
 #compdef pgrep pkill
 
 # Notes:
-# - We assume that Linux systems use procps-ng — specifically, procps-ng >=3.3.4
+# - We assume that Linux systems use procps-ng - specifically, procps-ng >=3.3.4
 #   (which changed the behaviour of -f and added -a)
 # - We don't really need to keep pgopts and pkopts separate, but it seems like
 #   it should make things a bit easier to follow
@@ -26,7 +26,6 @@ arguments=(
   '(: * -)'{-h,--help}'[display help information]'
   '-I[request confirmation before signalling each process]'
   '-i[ignore case distinctions]'
-  '-J+[match only on specified project IDs]: :->projid'
   '-j+[match only on specified jail IDs]:jail:_sequence _jails -0 -o jid'
   '(-L --logpidfile)'{-L,--logpidfile}'[fail if PID file not locked (with -F)]'
   '(-N)-M+[extract name list from specified core]:core file:_files'
@@ -37,7 +36,7 @@ arguments=(
   '(-l)-q[suppress normal output]'
   '-S[search also in system processes (kernel threads)]'
   '(-s --session)'{-s+,--session=}'[match only on specified process session IDs]: :->sid'
-  # _signals is OK here — we do it differently below
+  # _signals is OK here - we do it differently below
   '(ss)--signal=[specify signal to send to process]: :_signals -s'
   '-T+[match only on specified routing table]:routing table'
   '(-t --terminal)'{-t+,--terminal=}'[match only on specified controlling terminals]: :_sequence _ttys -do'
@@ -87,11 +86,12 @@ case $OSTYPE in
     pkopts=aFfGgIiLlnoPtUuvx
     ;;
   solaris*)
-    pgopts=cdfGgJlnoPsTtUuvxz
-    pkopts=cfGgJnoPsTtUuvxz
+    pgopts=cdfGglnoPsTtUuvxz
+    pkopts=cfGgnoPsTtUuvxz
     arguments=(
       ${arguments:#((#s)|*\))(\*|)-[cT]*}
       '-c+[match only on specified contract IDs]: :->contract'
+      '-J+[match only on specified project IDs]: :->projid'
       '-T+[match only on specified task IDs]: :->task'
     )
     ;;
@@ -130,115 +130,48 @@ arguments+=( $sig_arguments + o '*: :->pname' )
 [[ $OSTYPE == linux* ]] || aopts+=( -A '*-' )
 _arguments -C -s -S $aopts : $arguments && ret=0
 
+# complete comma-separated list of various IDs
+# $1: tag, $2: description, $3: keyword for 'ps -o'
+_pgrep_sequence () {
+    _sequence _wanted $1 expl "$2" \
+	      compadd - ${(un)$(_call_program $1 ps -A -o $3=)}
+}
+
 case $state in
   (sid)
     if [[ $OSTYPE == openbsd* ]]; then
-      break
-    fi
-
-    compset -P '*,'
-
-    local -a used sid
-    used=(${(s:,:)IPREFIX})
-    if [[ $OSTYPE == freebsd* ]]; then
-      sid=(${(uon)$(ps -ax -o sid=)})
+      _message 'session ID'
     else
-      sid=(${(uon)$(ps -A -o sid=)})
+      _pgrep_sequence session-ids 'session ID' sid
     fi
-
-    _wanted sid expl 'session ID' compadd -S ',' -q -F used $sid
     ;;
-
   (ppid)
-    compset -P '*,'
-
-    local -a used ppid
-    used=(${(s:,:)IPREFIX})
-    if [[ $OSTYPE == (freebsd|openbsd|darwin)* ]]; then
-      ppid=(${(uon)$(ps -ax -o ppid=)})
-    else
-      ppid=(${(uon)$(ps -A -o ppid=)})
-    fi
-
-    _wanted ppid expl 'parent process ID' compadd -S ',' -q -F used $ppid
+    _pgrep_sequence ppids 'parent process ID' ppid
     ;;
-
   (pgid)
-    compset -P '*,'
-
-    local -a used pgid
-    used=(${(s:,:)IPREFIX})
-    if [[ $OSTYPE == (freebsd|openbsd|darwin)* ]]; then
-      pgid=(${(uon)$(ps -ax -o pgid=)})
-    else
-      pgid=(${(uon)$(ps -A -o pgid=)})
-    fi
-
-    _wanted pgid expl 'process group ID' compadd -S ',' -q -F used $pgid
+    _pgrep_sequence pgids 'process group ID' pgid
     ;;
-
   (projid)
-    compset -P '*,'
-
-    local -a used projid
-    used=(${(s:,:)IPREFIX})
-    projid=(${(uon)$(ps -A -o project=)})
-
-    _wanted projid expl 'project ID' compadd -S ',' -q -F used $projid
+    _pgrep_sequence project-ids 'project ID' project
     ;;
-
   (contract)
-    compset -P '*,'
-
-    local -a used ctid
-    used=(${(s:,:)IPREFIX})
-    ctid=(${(uon)$(ps -A -o ctid=)})
-
-    _wanted ctid expl 'contract ID' compadd -S ',' -q -F used $ctid
+    _pgrep_sequence contract-ids 'contract ID' ctid
     ;;
-
   (task)
-    compset -P '*,'
-
-    local -a used taskid
-    used=(${(s:,:)IPREFIX})
-    taskid=(${(uon)$(ps -A -o project=)})
-
-    _wanted taskid expl 'task ID' compadd -S ',' -q -F used $taskid
+    _pgrep_sequence task-ids 'task ID' taskid
     ;;
-
   (pname)
     local ispat="pattern matching "
     if (( ${+opt_args[-x]} )); then
       ispat=""
     fi
-
-    local command
     if (( ${+opt_args[-f]} )); then
-      if [[ "$OSTYPE" == freebsd* ]] && (( ${+opt_args[-S]} )); then
-        command="$(ps -axH -o command=)"
-      elif [[ "$OSTYPE" == (freebsd|openbsd|darwin)* ]]; then
-        command="$(ps -ax -o command=)"
-      elif [[ "$OSTYPE" == solaris* ]]; then
-        command="$(ps -A -o args=)"
-      else
-        command="$(ps -A o cmd=)"
-      fi
-      _wanted pname expl $ispat'process command line' compadd ${(u)${(f)${command}}}
+      _wanted process-args expl $ispat'process command line' \
+	compadd ${${(f)"$(_call_program process-args ps -A -o args=)"}% *}
     else
-      if [[ "$OSTYPE" == freebsd* ]] && (( ${+opt_args[-S]} )); then
-        command="$(ps -axcH -o command=)"
-      elif [[ "$OSTYPE" == (freebsd|openbsd|darwin)* ]]; then
-        command="$(ps -axc -o command=)"
-      elif [[ "$OSTYPE" == solaris* ]]; then
-        command="$(ps -A -o comm=)"
-      else
-        command="$(ps -A co cmd=)"
-      fi
-      _wanted pname expl $ispat'process name' compadd ${(u)${(f)${command}}}
+      _wanted processes-names expl $ispat'process name' _process_names -a -t
     fi
     ;;
-
 esac && ret=0
 
 return ret

Re: [PATCH] _pgrep: simplify the processing of '->state' actions

[dana]

On 19 Jul 2018, at 05:06, Jun T <takimoto-j@kba.biglobe.ne.jp> wrote:
>-# - We assume that Linux systems use procps-ng — specifically, procps-ng >=3.3.4
>+# - We assume that Linux systems use procps-ng - specifically, procps-ng >=3.3.4

Would you guys prefer that i not use em dashes in source comments? I guess i'd
just assumed anything UTF-8 would be fine, but maybe that's a selfish
assumption. I can use '--' instead if it's a problem.

On 19 Jul 2018, at 05:06, Jun T <takimoto-j@kba.biglobe.ne.jp> wrote:
>-  '-J+[match only on specified project IDs]: :->projid'

I'm not sure about moving this. I mean it'll work either way, but why make this
a special case? Why not also break out -z? And then if we're doing that for
Solaris, why not also break out -S for FreeBSD, -T for OpenBSD, and so on?

If we don't like the p[gk]opts filtering method, that's one thing (i had just
extended the way the function was already set up, but obv we could build arrays
in case clauses or whatever instead) — but as long as we're keeping it i'm not
sure why -J alone should be exempt.

dana

Re: [PATCH] _pgrep: simplify the processing of '->state' actions

[Jun T]

> 2018/07/20 4:44, dana <dana@dana.is> wrote:
> 
> On 19 Jul 2018, at 05:06, Jun T <takimoto-j@kba.biglobe.ne.jp> wrote:
>> -# - We assume that Linux systems use procps-ng — specifically, procps-ng >=3.3.4
>> +# - We assume that Linux systems use procps-ng - specifically, procps-ng >=3.3.4
> 
> Would you guys prefer that i not use em dashes in source comments?

Could you please use only 7bit ASCII?
It sometimes happens that I need to use vanilla vi without multibyte
support (e.g., testing on a minimal install of BSD's).

Em dash is especially problematic because it is one of the characters
with "East Asian Ambiguous Width"; how it is displayed depends on the
setting of the terminal and/or font.


> On 19 Jul 2018, at 05:06, Jun T <takimoto-j@kba.biglobe.ne.jp> wrote:
>> -  '-J+[match only on specified project IDs]: :->projid'
> 
> I'm not sure about moving this. I mean it'll work either way, but why make this
> a special case? Why not also break out -z?

I just wanted to make sure that the '(projid)' part of the case
statements is called only on Solaris because 'ps -o project' would not
work on other OS's.

Re: [PATCH] _pgrep: simplify the processing of '->state' actions

[dana]

On 19 Jul 2018, at 19:30, Jun T <takimoto-j@kba.biglobe.ne.jp> wrote:
>Could you please use only 7bit ASCII?
>It sometimes happens that I need to use vanilla vi without multibyte
>support (e.g., testing on a minimal install of BSD's).

Ah, OK. I'll try to remember that; sorry for causing trouble.

On 19 Jul 2018, at 19:30, Jun T <takimoto-j@kba.biglobe.ne.jp> wrote:
>I just wanted to make sure that the '(projid)' part of the case
>statements is called only on Solaris because 'ps -o project' would not
>work on other OS's.

Seems like the option filtering would ensure that, but oh well

dana