[Search processes] account for exited processes in preview #228
Conversation
| @@ -56,15 +56,15 @@ Use `fzf.fish` to interactively find and insert the shell entities listed below | |||
| - **Search input:** all the variable names of the environment currently [in scope][var scope] | |||
| - **Key binding and mnemonic:** <kbd>Ctrl</kbd>+<kbd>V</kbd> (`V` for variable) | |||
| - **Preview window:** the scope info and values of the variable | |||
| - `$history` is excluded for technical reasons so use the search command history feature instead to inspect it | |||
| - `$history` is excluded for technical reasons so use the [search command history](#a-previously-run-command) feature instead to inspect it | |||
There was a problem hiding this comment.
Irrelevant but this is a small improvement I would like to see.
functions/_fzf_search_processes.fish
Outdated
| @@ -9,7 +9,8 @@ function _fzf_search_processes --description "Search all running processes. Repl | |||
| --ansi \ | |||
| # first line outputted by ps is a header, so we need to mark it as so | |||
| --header-lines=1 \ | |||
| --preview="ps -o '$ps_preview_fmt' -p {1}" \ | |||
| # 40 is the length from "PID" to "COMMAND" in the header "PID PARENT USER %CPU RSS_IN_KB START_TIME COMMAND" | |||
There was a problem hiding this comment.
This spacing may not be portable across different implementations of ps but we are not going to jump in the rabbit hole and check ps variant, just like in the original implementation PR 🤷
functions/_fzf_search_processes.fish
Outdated
| @@ -9,7 +9,8 @@ function _fzf_search_processes --description "Search all running processes. Repl | |||
| --ansi \ | |||
| # first line outputted by ps is a header, so we need to mark it as so | |||
| --header-lines=1 \ | |||
| --preview="ps -o '$ps_preview_fmt' -p {1}" \ | |||
| # 40 is the length from "PID" to "COMMAND" in the header "PID PARENT USER %CPU RSS_IN_KB START_TIME COMMAND" | |||
| --preview="ps -o '$ps_preview_fmt' -p {1} || begin; set_color red; echo {1}(string repeat --count 40 ' '){2..}; end" \ | |||
There was a problem hiding this comment.
can you make this a helper function that fish autoloads? similar to _fzf_extract_var_info
|
Just to check my understanding: what this change does is if the process can no longer be found by ps, take the info that is already stored in the fzf results and just echo that in red? |
|
@kidonng still interested in working on this? |
PatrickF1
changed the title
|
Hi @kidonng, I decided to go for nice-and-simple: just print an error message if the process has already exited. I want to reduce complexity as much as possible, and also I think the user can easily look to the left to find the pid and command. It's not perfect because sometimes the command is too long, but I think it's worth it. |
First, thanks for your effort in getting the process search feature in! I saw the mention but unfortunately don't have the time to chime in.
I was not satisfied as you did, and made some local changes to fzf.fish. It was until #227 got merged that I realized I never submitted those patches (yes, I have been putting the preview window on the bottom for the longest time).
So here is the change that I really want to upstream: mark exited process as red.
Since the preview is provided by a second
ps, the selected process may have already exited and all we get is a confusing header.Where is that process I want to look at?
Even if it exited, the full process name along with its parameter is still useful, but without a functioning preview, the only chance you see it in whole is to have a super-duper wide display (does this even exist?)
No more. The process info is right in fzf itself and we just need to instruct fzf to use that instead.