Improve dangerous/undefined PID expansion behavior#1008
Improve dangerous/undefined PID expansion behavior#1008lunixbochs wants to merge 1 commit intofish-shell:masterfrom
Conversation
1. Use Bash-like expansion for empty searches (when you just use a '%' by itself). '%' will now *only* match the last valid backgrounded process. If there are no such processes, an expansion error will be generated. '%' by itself would previously match either *all* backgrounded processes, or failing that, all processes owned by your user. If you ever tried to run `kill -9 %`, it would either kill all backgrounded processes or *all* of your processes. I'm not sure why anyone would ever want that to be a single keystroke away. You could almost typo it. As a result, `fg %`, `bg %`, `kill %`, etc will all operate on the last process touched by job control. 2. Don't run 'by-name' matches when the search term is numeric. This prevents you from running a command like `kill %1` and accidentally killing a process named something like "1Command". Overloaded behavior can be dangerous, and we probably shouldn't play fast and loose with expansion characters that generate process IDs.
|
That is terrific behaviour, not unlike Solaris' |
|
I wonder if we should just error out on the expansion '%', or at least document it better. |
|
Indeed, it sounds dangerous. My suggestion would be to avoid current silly behavior of |
|
|
|
My biggest use case for solo |
|
@lunixbochs: Useless use of But aside of that – even normal |
|
What exactly does solo I found this article: http://web.mit.edu/gnu/doc/html/features_5.html#SEC33 which implies I was correct about the purpose of a standalone This is fish (with my patch): This is bash: The entire point of my habit is to hard kill a badly behaving process that responds to |
|
Yeah, sorry. I was referring to |
|
OK, so this patch which leads to It sounds like @lunixbochs is opposed to entirely removing the expansion of |
|
Merged with rebase: To git@github.com:fish-shell/fish-shell.git Thanks again! |
|
We should update the docs to reflect this new behavior. |
|
The documentation is now less incorrect than it was - the current text reads
|
|
So it is. Good work! |
Backstory
In Bash, % means something like 'last job backgrounded'. In upstream Fish, it means either "all backgrounded jobs" or "all processes owned by your user" if there are no processes backgrounded.
I usually run
kill -9 %in Fish if I actually wanted to hard kill the last job backgrounded (useful if it wasn't responding). This actually matched for all jobs backgrounded, but I never noticed until today.Even further, it turns out
kill -9 %in Fish kills all processes owned by your user if you ever accidentally run it without backgrounding something first.Go ahead,
echo %to get a feel for how it works right now.Change 1
Use Bash-like expansion for empty searches (when you just use a '%' by
itself).
'%' will now only match the last valid backgrounded process.
If there are no such processes, an expansion error will be generated.
'%' by itself would previously match either all backgrounded
processes, or failing that, all processes owned by your user. If you
ever tried to run
kill -9 %, it would either kill all backgroundedprocesses or all of your processes. I'm not sure why anyone would ever
want that to be a single keystroke away. You could almost typo it.
As a result of this change,
fg %,bg %,kill %, etc will all now operateon the last process touched by job control (or throw an expansion error).
Change 2
Don't run 'by-name' matches when the search term is numeric.
This prevents you from running a command like
kill %1and accidentallykilling a process named something like "1Command". Overloaded behavior
can be dangerous, and we probably shouldn't play fast and loose with
expansion characters that generate process IDs.