-
-
Notifications
You must be signed in to change notification settings - Fork 137
feat: add option to display exit pipe status #395
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add option to display exit pipe status #395
Conversation
…Linux OS Rewrite the expected value for "pipeline of the first 15 signals, conversion enabled" to account for the difference in signal names between operating systems.
52d71f0 to
34407d7
Compare
|
Just pushed a fix for the failing test, sorry for having overlooked it! |
edouard-lopez
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR!
Could you share some screenshots ?
I added some todo, will check again later as I’m not on a computer
| # true - separate prompt character | ||
| _pure_set_default pure_separate_prompt_on_error false | ||
|
|
||
| # Prefix the prompt with a list of exit statuses ($pipestatus) if at least one is non-zero |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: Variable $pipestatus was introduced in Fish 3.1b1, our minimal supported version is 3.3.1
| before_each | ||
| @test "_pure_prompt_exit_status: pipeline of the first 15 signals, conversion enabled" ( | ||
| _pure_unmock _pure_set_color # enable colors | ||
| set --local statuses (seq 129 143) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thought: I see you are limiting to POSIX-compliant UNIX systems codes. That’s fine by me, we can add more when use cases arise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Much as I'd have liked that to be the case, I just pulled up the kernel manpage you linked and thought "I've never seen most of these statuses, I think 15 is going to be good enough" 😅
|
Thanks for the screenshots. Question: Did you get inspiration from another theme or tool ? |
…nges Refs: pure-fish#395 test(_pure_prompt_exit_status): add expected output description to test titles test(_pure_prompt_exit_status): refactor signal conversion test to target the intended CI environment only
I learned of
Not really, I think the display is intuitive enough without any prefix but I could add one in if you wish. |
|
Please add a |
|
I pushed a PR to update the doc about feature design based on our exchange #396 |
Done, here's how it looks now. |
…nges Refs: #395 test(_pure_prompt_exit_status): add expected output description to test titles test(_pure_prompt_exit_status): refactor signal conversion test to target the intended CI environment only




related: none
Adds an option to display the exit status if the last command failed. Includes an extra option to convert signal numbers (129 and above) to their respective names (SIGHUP, SIGINT, SIGQUIT, etc.), as well as two configuration options.
How to test pre-release?
Specs
New config in
conf.d/pure.fishDocumentation
Usage
Acceptance Checks
conf.d/pure.fishfor:tests/feature_name.test.fish;