Add fish_job_summary, called whenever a job ends, stops, or is signalled
#6959
Conversation
|
Do not worry about the CI failure, there is one flaky test. |
There was a problem hiding this comment.
This is a really good idea!
Right now fish has logic to suppress messages for foreground jobs (this lives in job_wants_message), because printing status for foreground jobs would be too noisy. Do we want to invoke fish_job_summary for all jobs and let it filter out foreground ones? Or do we just not invoke it for foreground jobs?
| wcstring buffer = wcstring(L"fish_job_summary"); | ||
| for (const wcstring &arg : args) { | ||
| buffer.push_back(L' '); | ||
| buffer.append(escape_string(arg, ESCAPE_ALL)); |
There was a problem hiding this comment.
I think escaping is not desirable here - the function should receive the actual command that was run. It can be escaped with string escape if the function chooses to (maybe it should).
There was a problem hiding this comment.
This is building a single string for execution, so the individual arguments need to be escaped once in order to be split up correctly.
With escaping the execution string will be fish_job_summary 3 sleep\ 10\ \&, which will correctly provide the args "3" and "sleep 10 &". Without escaping the execution string will be fish_job_summary 3 sleep 10 &, which in addition to providing the wrong args "3", "sleep", "10", will also try to background the function call.
You can see from the expect based tests that the special characters are not escaped when the function prints them, so I think that covers what you want.
I like that idea! It would work very well for me, since I can then avoid duplicated code between this function and postexec. I can also simplify my own postexec which currently has special handling for "^C" on the foreground job. A flag indicating fg vs bg would also very much help with customisations, since bg means the function could need to handle an existing prompt on the screen. (It could also defer printing the line until next prompt to avoid interleaving with command output, or print some more obvious borders to make it clear that the output is coming from a background job rather than whatever is running). I ended up not calling the function for foreground job ends, because that is covered by postexec already (and it became complicated to filter out, e.g. 'down-or-search', and it caused a segfault, so something there is tough to get working). |
|
Can you take a look at this PR? I've fixed the tests (mostly, clang is still failing for some reason I don't fully understand), and I think I've addressed the comments raised earlier. |
…alled This allows users to customise the behaviour of the shell by redefining the function. This is similar to how fish_title or fish_greeting behave, where the default implementation can be easily overridden. The function receives as arguments the job id, command line, signal name and signal description.
The default implementation will not print any output in that case, but this provides users with additional flexibility when it comes to customising the shell's behaviour.
|
Done. Increasing sleep times seems to have fixed the tests as well, thanks! |
|
Splendid, thank you! |
Description
Add
fish_job_summary, called whenever a job ends, stops, or is signalledThis allows users to customise the behaviour of the shell by redefining the function. This is similar to how fish_title or fish_greeting behave, where the default implementation can be easily overridden.
The function receives as arguments the job id, command line, signal name and signal description.
Fixes #4319, maybe #1018 and #2727.
TODOs: