add $pipestatus support#5632
Conversation
|
One big thing that #2039 left open was how logical operators are handled. E.g. ! truewould in bash have a $? of 1 and a $PIPESTATUS of 0. How does this handle that? |
|
Oh yes, I completely forgot about that. As it stands it behaves like bash and zsh. |
Personally, that seems entirely counter-intuitive, but it's what bash and zsh do, so there's precedent. I'd like to have that hashed out and nailed down before merging anything, because changing it would be tough. @fish-shell/contributors Any opinions? |
|
Yes completely agree. It is a tough one. It will either be confusing to other shell (zsh/bash) users switching to fish, or to new shell users that start out on fish. |
|
Okay, I can't find any further specification in the bash manual or the zsh manual In fact the sum total documentation for both seems to be
Most questions about PIPESTATUS on stackoverflow also don't reference this fact. The only answer I can find is https://stackoverflow.com/a/36447447/3150338, which explains:
and recommends using subshells. The (approximate) equivalent for that in fish would be begin; not a; end | b | cSo... we obviously don't have a problem with deviating from other shells. I'd argue that we should do so if there is a cause. E.g. if something is a common problem (like word splitting). In this case it seems like it's not a common problem (or I'd have found more on stackoverflow), and I'm not entirely sure it is a problem. Because the It's just quite a surprise in the So, as far as I'm concerned, applying logical operations to the first command doesn't make any sense, applying it to the last might. Or we could keep it like bash and zsh apparently do and explain it in the docs. |
Only if you look at the most basic examples. It's less obvious here: The core problem is that the fish doesn't have an elegant grouping operator. As a side effect of fish's adoption of undecorated parentheses as subshell operators, there's no clean way of explicitly specifying (or, even better, overriding) the order of operations at the prompt without using functions, if statements, temporary variables, etc. (Not that That said, I would lose literally zero sleep over deviating from other shells with our implementation. However, more to the point, I think any |
Sure, but should we? Is there enough of a point in doing so?
Bash's man page says:
So it seems to be just about the status. Which means it can be emulated with something like
Pipefail doesn't, errexit does. And errexit is terrible, because a non-zero return is not a universal indication of an exceptional error. (The "unofficial strict mode" is See e.g. de32665 and https://mywiki.wooledge.org/BashFAQ/105.
Why not? I mean it's nice to have, but we've gotten by without $pipestatus, $pipestatus is useful on its own and there's no backwards-compatibility implications here, so it would be harmless to ship $pipestatus without that stuff. |
|
The |
|
Okay, as it stands I have no idea how I'd explain this: $ ! true | false; echo $pipestatus = $status
0 1 = 0
$ begin; ! true; end | false; echo $pipestatus = $status
0 1 = 1The first I can explain with "the negation is applied to the entire pipeline", but the second does not make any sense. Even if I wrap it: function nottrue; not true; return $status; end
nottrue | false; echo $pipestatus = $statusAlso $pipestatus seems to be cleared by the prompt: $ nottrue | false
$ echo $pipestatus = $statusprints "0 = 1". @zabereer: IIRC, there's an idea of "apply_exit_status" that the prompt code sets to false. This should also affect $pipestatus. |
|
Having if not foo | bar | baz
# inspect $pipestatus to see the statuses of foo, bar, and baz
endRegarding conditional piping, $ begin; ! true; end | false; echo $pipestatus = $status
0 1 = 1This definitely seems wrong. I also agree that the prompt should not reset |
My idea was to apply However, function returns5
return 5
end
not return5; echo $status # prints 0 obviouslySince the It's a bit weird because it could be both true | not false | truehas a $status of 1. Because the So I'd be alright with just doing what bash does, as long as the documentation is better than theirs. |
|
Honestly, it makes more sense to me to have |
|
@mqudsi That doesn't scale past a single pipeline, such as |
|
@lilyball I think I'm missing something, because I'm envisioning |
|
Ok so you're actually proposing just the same I see no benefit to throwing away this value. Do note that the value you're proposing to throw away is not in fact the same thing as |
|
Really happy to see this PR, thanks for doing this! As faho points out fish does not have per-process negation (despite the fact that the syntax looks supported). I think the way to think about |
|
@lilyball I completely agree with you, that is how I would expect |
|
@ridiculousfish Thank you, that is very encouraging. |
|
Last commit preserves |
What's are you planning for negate? If we like the bash / zsh behavior then |
|
After adding this, we should soup-up at least one of the included prompts to display the pipeline status in a cool way, maybe iff. things aren't 0. |
|
My old Bash prompt displays pipe statuses. It displays the status area in red if the pipeline failed and grey if the pipeline succeeded. If there's a pipeline, it displays the individual command statuses. If Here's my entire prompt command. I don't remember what the commented-out code is for, I wrote this many years ago: __prompt_command() {
local status=$? pipestatus=("${PIPESTATUS[@]}")
local prompt=""
prompt+='\[\e[1;32m\]\u\[\e[m\]@\[\e[1;36m\]\h\[\e[m\]:\[\e[1;31m\]'
prompt+='$(__prompt_path)'
prompt+='\[\e[0;33m\]'
prompt+='$(__git_ps1 " (%s)")'
prompt+='\[\e[m\]'
local promptstatus=
if (( status > 255 )); then
# status > 255 indicates a special return from bash, such as
# a syntax error. It appears that this type of return does not
# modify PIPESTATUS.
promptstatus=$status
else
for x in "${pipestatus[@]}"; do
if (( x )); then
promptstatus=${pipestatus[*]}
break
fi
done
if (( status != pipestatus[${#pipestatus[@]}-1] )); then
promptstatus+="${promptstatus:+ }($status)"
#if [[ -n $promptstatus ]]; then
#promptstatus+=" ($status)"
#elif (( status )); then
#promptstatus=$status
#fi
fi
fi
if [[ -n $promptstatus ]]; then
local color=31
if (( status == 0 )); then color=2; fi
prompt+=" \\[\\e[${color}m\\][$promptstatus]\\[\\e[m\\]"
fi
prompt+='> '
PS1=$prompt
}I very much like the way this displays statues and would suggest doing something similar if we want to bundle this with a Fish prompt. I'd suggest wrapping up this logic in a prebaked Looking at that code, it has special handling for exit codes greater than ¹Testing right now, neither does Bash 4, but Bash 4 also doesn't use |
@ridiculousfish, sorry I should have been more clear. The job level negation works like bash/zsh. However when one does: it deviates from zsh and bash by returning |
|
Using blocks in pipes seem to work. When not using a pipe the results are as follows (similar to bash but not zsh): bash: zsh: I have left the tests for tying this down commented out as I am torn between what bash does and what zsh does regarding this. |
…bash but not zsh)
|
Committed the tests I mentioned above. |
|
I added a man page for pipestatus, it turned out longer than I had hoped (seems one can say a lot about it). I read in FORMATTING.md that it is undesirable to add new pages, so please suggest where to put documentation for this (bash and zsh is rather light on pipestatus documentation, so we can definitely do documentation better for fish). |
|
@lilyball that last commit should address what you said about setting |
|
@floam I will look into adding |
|
@faho are you happy with the changes I made to address your review? |
|
Rather than a separate manpage, the documentation should go into the manual (probably in a new subsection after "the status variable"). |
|
@zanchey Sure I will move it. |
|
I update the documentation as suggested, thank you. |



Add
$pipestatussupportAdd $pipestatus variable similar to other shells. It is handy for status lines.
I frequently build software piping
makeoutput intogrepand other commands, ending it with;exit $pipestatus[1]so final status is that ofmakeand not ofgrepetc.Fixes issue #2039
TODOs:
I will add CHANGELOG.md and update pull request. I love fish and have recently switched to it!!!