prompts: fix pipestatus for jobs prefixed with "not"#6566
Conversation
6902459 was an attempt to not print $status twice in the prompt. As a result we print $pipestatus but not $status, which /usually/ is the same as $pipestatus[-1] --- unless the builtin "not" is used, which inverts the $status of a job (it does not alter $pipestatus). As a result, the default prompt prints unexpected status codes: ~ > not false ~ [1]> not true ~ > not true | true ~ > not false | false ~ [1|1]> This commit reintroduces printing of $status after $pipestatus, but only if it is different from $pipestatus[-1]. Additionally, we only print anything at all if the $status is nonzero, to avoid confusing output on `not false | false` ~ > not false ~ > not true ~ [0] 1> not true | true ~ [0|0] 1> not false | false ~ > I think this is closer to users' expectations for those cases; they should not have to think about this implementation detail of the not-statement.
ridiculousfish
left a comment
There was a problem hiding this comment.
This LGTM. I really like the new default prompt.
| # SIGPIPE (141 = 128 + 13) is usually not a failure, see #6375. | ||
| if string match -qvr '^(0|141)$' $argv | ||
| # Only print status codes if the job failed. | ||
| if test $last_status -ne 0 |
There was a problem hiding this comment.
Probably best to quote this due to test's parsing quirks. "$last_status"
|
I agree with the idea, but I think it needs to be a bit more intuitive. |
For the record, an alternative I was considering is to replace Which would avoid the trailing status when using |
6902459 was an attempt to not print
$status twice in the prompt. As a result we print $pipestatus but
not $status, which usually is the same as $pipestatus[-1] --- unless
the builtin "not" is used, which inverts the $status of a job (it does
not alter $pipestatus).
As a result, the default prompt prints unexpected status codes:
This commit reintroduces printing of $status after $pipestatus, but only
if it is different from $pipestatus[-1].
Additionally, we only print anything at all if the $status is nonzero,
to avoid confusing output on
not false | falseI think this is closer to users' expectations for those cases; they should
not have to think about this implementation detail of the not-statement.