Introduce feature to search for processes

[cr-mitmit]

Allows searching for PIDs of any running process.
Useful for commands such as

kill -STOP \co

Todos

• make the fzf options for pid configurable https://github.com/PatrickF1/fzf.fish#pass-fzf-options-for-a-specific-feature
• add process owner to main fzf window: CANCELLED not enough space
• add memory and CPU use to preview window
• remove columns that are very rarely used from preview window
• make sure everything is OS-portable (checked Mac OS, debian, and Ubuntu manpages)
• feature in readme
• add a gif to readme
• add a smoke test
• consider setting key to ctrl+option+p because it's more mnemonic (which is advertised in the readme, so we better follow it)
• vsz vs rss vs %mem
• use -o format to make names cleaner in preview

[PatrickF1]

Hi @cr-mitmit ! I love how thorough you were with this PR, even updating the printed help and the readme!

Unfortunately, and I'll be upfront, this one is gonna be tricky!
The first problem is that there is no information presented for each browser--it only shows the PIDs. Take a look at search git log. Even though it only outputs the git SHA, it still displays the log message and a preview of the diff for each git commit. I think users will need something similar. They will need to know at least which command started the process and what. And probably other things but to be frank I know too little about being a sys admin to say what other info is important.
And the second problem that you'll run into with generating that information is that the ps command options varies widely between OSes and I haven't figured out which options are reliable.

If you can tackle both, then this just might be merged (eventually).

Also, may I ask which use cases you see this being useful for?

[cr-mitmit]

Hi.

Re preview, the flags I use are ps -Aa -opid,command. They seem to documented for many years on the linux side, and work and tested on Macos Monterey. This seems to be as portable as it gets.

The default table shows the pid and the full command line (including arguments).
We could generate a preview with more information, but I'm also unsure what else to show. I could theoretically add parent pid to the table as well.
The current implementation already returns only the pid upon selection.
Thanks for the pointer to the search git log, I'll see if there's better way than an awk call.

Would love your thoughts

[PatrickF1]

Re preview, the flags I use are ps -Aa -opid,command. They seem to documented for many years on the linux side, and work and tested on Macos Monterey. This seems to be as portable as it gets.

Great! Let's keep the cross-OS compatibility in mind. Also, let's be sure to include Cygwin.

We could generate a preview with more information, but I'm also unsure what else to show. I could theoretically add parent pid to the table as well.

Yeah definitely being able to see the parent pid and command would be helpful. Maybe start time as well? TBH I'm not sure. That's why I'd like to explore this topic more by re-asking this question: "may I ask which use cases you see this being useful for?" Personally, I've never had to kill a process using the commandline so I have no intuition as to what users will need.

[cr-mitmit]

Hi.
Will do. I don't have access to a Cygwin machine and documentation has conflicting views on whether the ps command in Cygwin respects or ignores -o formatting. The one in WSL, ironically, does.

I'll look into adding the preview including the parent information.
As for use-cases, I sometimes find myself doing trying to kill a process / service. I liked the idea of using fzf to search for the process. Basically the interactive equivalent of kill (pgrep XXX)

[cr-mitmit]

Also added a preview. As far as I can tell uses only portable flags across both Linux, WSL and Macos

[PatrickF1]

Will do. I don't have access to a Cygwin machine and documentation has conflicting views on whether the ps command in Cygwin respects or ignores -o formatting. The one in WSL, ironically, does.

Hmm that's tough. @kidonng sorry to bother you. Do you have Windows experience to help out here?

[PatrickF1]

Nice! I think we're very close. The last thing I'd like to see if if we can show the username instead of uid in the preview. Is that possible and portable? Reason: I noticed that Mac's Activity Monitor shows username in all of its windows and so it seems to be a pretty important thing to know, and users don't want to have to deal with ids.

[PatrickF1]

this says 0 but you got it bought to o. which one do you think is best?

[cr-mitmit]

Might it be a font issue? I've verified again all letters are O (capital o) and not Zero
The binding is done to \co to mimic the way Ctrl-R is bound to \cr (and since Ctrl binding are case insensitive).
Therefore, Ctrl-O is bound to the Process list.

Am I missing something?

[PatrickF1]

Oops my bad it is a capital o indeed. Ok I think that's fine...
Btw can you make the option --processes instead of --pids?

[cr-mitmit]

Also added the -O user to add the user-name to the process list. Good idea

[PatrickF1]

Also added the -O user to add the user-name to the process list. Good idea

Can you put -O user so it comes first in the output?

[cr-mitmit]

Changed the command to user processes instead of pids (Not just the doc, across the commit).

As for where the user listing appears, the only way to specific the order explicitly is to forgo the -f flag in favor of explicitly listing a set of options. While it should work, I'm not sure how portable that would be. I think the value isn't worth the risk in this case.

[PatrickF1]

As for where the user listing appears, the only way to specific the order explicitly is to forgo the -f flag in favor of explicitly listing a set of options. While it should work, I'm not sure how portable that would be. I think the value isn't worth the risk in this case.

Good point. I will take it upon myself to figure out the set of portable options and figure out the best one to use from here. As the plugin author, I need to worry about having a sane set of defaults and portability so I'll need to do this anyway. Please wait patiently as I have a lot of real work to catch up on returning from vacation, but rest assured I should get this in within a week. Thanks for opening up this PR and your hard work so far!

[PatrickF1]

Changed the command to user processes instead of pids (Not just the doc, across the commit).

Also I think you might've forgotten to push this commit.

[cr-mitmit]

Changed the command to user processes instead of pids (Not just the doc, across the commit).

Also I think you might've forgotten to push this commit.

Pushed. Sorry

[PatrickF1]

Todos for myself (feel free to chime in with your feedback)

• make the fzf options for pid configurable https://github.com/PatrickF1/fzf.fish#pass-fzf-options-for-a-specific-feature
• add process owner to main fzf window
• add memory and CPU use to preview window
• remove columns that are very rarely used from preview window
• make sure everything is OS-portable
• add a gif to readme
• add a test that makes sure the ps command succeeds (don't need to check the output sent into fzf, this is purely for compatibility testing in case we change the ps command)

[cr-mitmit]

1. I had actually added a fzf_processes_opts but then forgot to actually pass it to fzf_wrapper 🤦‍♂️
2. Should be easy, add user to the options -opid,user,command. It should be portable enough across Mac, Linux, and WSL. Can't tell re Cygwin as it uses a weird version of PS
3. We can likely achieve it with a set of flags to -o. Portability worries me here though

Sounds great.
If I have time today I'll try to add a patch that does exactly that.

[cr-mitmit]

I've implemented a new preview, but getting memory into M and G (as opposed to KB which is what PS returns) is ugly as hell.
Any thoughts on how to make this preview bearable?
Technically, it sets the PS, converts the RSS number into strings with M and G depending on the memory usage and the painfully recreates a table and accounting for processes whose name contains a space (so all "columns" after the 6th are actually fake because of spaces in the command line).

It's ugly as hell... but works...

                --preview='ps -o pid,user,%cpu,time,rss,command -p {1} | awk \'NR>1 {if ($5 > 1048576) {$5=sprintf("%.02fG", $5/1024/1024);} else {$5=sprintf("%.02fM",$5/1024);}}{for(i=1;i<=5;i++){printf("%s|", $i); $i=""}; $0=$0; print; }\' | column -s"|" -t' \

[PatrickF1]

What if you just returned the memory and CPU as percentages? It’s already built in

…

On Fri, Jan 7, 2022 at 9:25 AM cr-mitmit ***@***.***> wrote: I've implemented a new preview, but getting memory into M and G (as opposed to KB which is what PS returns) is ugly as hell. Any thoughts on how to make this preview bearable? Technically, it sets the PS, converts the RSS number into strings with M and G depending on the memory usage and the painfully recreates a table and accounting for processes whose name contains a space (so all "columns" after the 6th are actually fake because of spaces in the command line). It's ugly as hell... but works... --preview='ps -o pid,user,%cpu,time,rss,command -p {1} | awk \'NR>1 {if ($5 > 1048576) {$5=sprintf("%.02fG", $5/1024/1024);} else {$5=sprintf("%.02fM",$5/1024);}}{for(i=1;i<=5;i++){printf("%s|", $i); $i=""}; $0=$0; print; }\' | column -s"|" -t' \ — Reply to this email directly, view it on GitHub <#216 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAPAJEE6CXE7SHV3T5LWWODUU4O2FANCNFSM5LANTWAA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you commented.Message ID: <PatrickF1/fzf. ***@***.***>

[cr-mitmit]

I find it less usable.
My Mac has 64G of ram, knowing my browser is taking 5% is just depressing, but not as useful.

I'll push the change that shows pid, user, %cpu, ram, command line with the awk based printing.

Let me know what you think.

Also, I experimented with adding the user to the main table, but found that it takes too much screen real-estate.
Love to hear your thoughts.

[PatrickF1]

how come you still need awk?

[cr-mitmit]

Good point. The output from FZF is the entire line, but I'm trying to extract just the PID here to be useful for commands such as "kill". the awk here is simply extracting the pid from the selected line

[PatrickF1]

Ah so we want to reduce dependencies so you could just use standard fish functions to do this. Check out how I did it in _fzf_search_git_log

[PatrickF1]

Don't worry I haven't forgotten about this! I will try to take another pass at it today. We are close!

[cr-mitmit]

Hi. Any news?

[PatrickF1]

Hi @cr-mitmit. I am so sorry. I'm honestly pretty behind on life but this is still important to me and still high up on my to do list (except other things keep getting added on top if it as fast as I can complete them). If you'd like to speed it up, you could try to tackle one of my last todos:

• add a gif to readme (see Illustrate features using gifs instead of screenshots #47 for how)
• add a test that makes sure the ps command succeeds (don't need to check the output sent into fzf, this is purely for compatibility testing in case we change the ps command)
• after reading https://unix.stackexchange.com/questions/35129/need-explanation-on-resident-set-size-virtual-size and https://utcc.utoronto.ca/~cks/space/blog/unix/UnderstandingRSS, I wonder if we should revert back to %mem or vsz instead. Still need to decide

[PatrickF1]

found a bug: this works for one process but not multiple. probably should be doing $process_selected)[1]

[PatrickF1]

also, is string escape necessary?

[PatrickF1]

Hey I decided to switch to using %mem despite the argument you made earlier. rss and vsz are too confusing and do not match with Mac's Activity Monitor's . Plus we can change this easily without worrying about backwards compatibility since this is only used in the preview

According to https://manpages.ubuntu.com/manpages/xenial/man1/ps.1.html, %mem is simply rss over physical memory on the machine so I changed it back for the reasons you gave before.

[PatrickF1]

Ok sorry for the delay. I decided to just merge this in so you don't have to wait any longer and just implement the other changes I want by myself. Sorry it took so long--besides being very busy, I am also a bit of a perfectionist.

[cr-mitmit]

Thank you - for the merge as well as writing fzf.fish!

[PatrickF1]

You're welcome! I'm spurred on by everyone who contributes and uses it! :)
Btw, now that you've got contributed such a major feature to fzf.fish, would you mind starring it? A personal goal of mine is to reach 1k stars.