zsh-workers
 help / color / mirror / code / Atom feed
From: dana <dana@dana.is>
To: Marlon Richert <marlon.richert@gmail.com>
Cc: Zsh hackers list <zsh-workers@zsh.org>
Subject: Re: [PATCH] Fix pgrep/pkill -f completion
Date: Tue, 25 May 2021 02:42:00 -0500	[thread overview]
Message-ID: <2FA8FD65-1BE2-4E22-BD0A-ABA922D814BE@dana.is> (raw)
In-Reply-To: <CAHLkEDsJ=EfsTZ0ddJK91sz3p_RqEfT8ENKDcr1GA7yZPx+C+g@mail.gmail.com>

On 21 May 2021, at 07:59, Marlon Richert <marlon.richert@gmail.com> wrote:
> Old completion produced false positives & took too much screen space.

There are a few issues with this:

1. procps-ng needs -a or -la instead of -lf to get the expected output. I
   can't recall off the top of my head whether *BSD and Solaris -lf output
   will work, but for now i'll assume so

2. The pgrep command needs -- in case the pattern starts with -

3. The pattern needs quoted again, since it's being passed to eval

4. There's an error in the :-escaping

5. On my macOS machine there's now a delay of about 5 seconds before the full
   possibilities are shown, whereas the previous method was pretty much
   instantaneous. I guess this is something to do with the large size of the
   full output on my Mac (65'000+ chars due to the paths i mention below),
   since it's much faster if i include a $PREFIX. But the parameter expansions
   shouldn't be that slow with just 64K of text, so maybe it's an inefficiency
   in _describe? I didn't look into it much

6. It's not a problem with the change *per se*, but the default
   max-matches-width can make the menu really irritating when you have several
   PIDs corresponding to the same command; they push the actual command line
   off the edge of the screen, and since on macOS you have a million things in
   the list that begin with long absolute paths, you just get a whole screen
   full of...

   1008                                   /System/Library/PrivateFrameworks/Pa
   1077                                   /System/Library/PrivateFrameworks/Us
   1099                                   /System/Library/PrivateFrameworks/IM
   1204                                   /System/Library/PrivateFrameworks/Ke

   ... which is not very helpful at a glance.

   idk if anything should even be done about that, but i thought i'd mention
   it at least

Attached fixes the first four issues

(Not sure i understand the weird indentation rules for continuation in
completion functions — i never followed them before but i remember someone
mentioned them recently — lmk if i messed it up)

PS: This change didn't introduce it, but the $ispat thing doesn't make sense
to me. It implies that -x is a literal match, which isn't the case; it's a
pattern match either way. I've included a second patch that hopefully makes
that distinction more accurate/meaningful. (Use -C2 if applying out of order)

dana


diff --git a/Completion/Unix/Command/_pgrep b/Completion/Unix/Command/_pgrep
index e10f42f53..bde681dd2 100644
--- a/Completion/Unix/Command/_pgrep
+++ b/Completion/Unix/Command/_pgrep
@@ -170,10 +170,13 @@ case $state in
       ispat=""
     fi
     if (( ${+opt_args[-f]} )); then
+      local -a opts=( -lf )
+      [[ $OSTYPE == linux* ]] && opts=( -a )
       local -a matches=( ${(f)"$(
-              _call_program process-args pgrep -lf ${${:-$PREFIX$SUFFIX}:-.\*}
+              _call_program process-args pgrep ${(@q)opts} -- \
+                  ${(q)${${:-$PREFIX$SUFFIX}:-.\*}}
           )"} )
-      local -a displ=( "${${matches[@]//':'/'\:'}[@]/ /:}" )
+      local -a displ=( "${${matches[@]//:/\:}[@]/ /:}" )
       matches=( "${matches[@]##<-> }" )
 
       local desc=$ispat'process command line'


diff --git a/Completion/Unix/Command/_pgrep b/Completion/Unix/Command/_pgrep
index bde681dd2..38b1aebd8 100644
--- a/Completion/Unix/Command/_pgrep
+++ b/Completion/Unix/Command/_pgrep
@@ -167,7 +167,7 @@ case $state in
   (pname)
     local ispat="pattern matching "
     if (( ${+opt_args[-x]} )); then
-      ispat=""
+      ispat+="full "
     fi
     if (( ${+opt_args[-f]} )); then
       local -a opts=( -lf )



  reply	other threads:[~2021-05-25  7:40 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-21 12:59 Marlon Richert
2021-05-25  7:42 ` dana [this message]
2021-06-07 22:32   ` Lawrence Velázquez
2021-06-07 22:36     ` Lawrence Velázquez
2021-06-09  4:11       ` Bart Schaefer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2FA8FD65-1BE2-4E22-BD0A-ABA922D814BE@dana.is \
    --to=dana@dana.is \
    --cc=marlon.richert@gmail.com \
    --cc=zsh-workers@zsh.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).