Skip to content

Respect non-zero exit code in subexpressions and blocks#8984

Merged
WindSoilder merged 2 commits intonushell:mainfrom
sophiajt:respect_non_zero_error
Dec 5, 2023
Merged

Respect non-zero exit code in subexpressions and blocks#8984
WindSoilder merged 2 commits intonushell:mainfrom
sophiajt:respect_non_zero_error

Conversation

@sophiajt
Copy link
Copy Markdown
Contributor

@sophiajt sophiajt commented Apr 24, 2023

Description

This PR changes the way we handled non-zero exit codes to be and early exit between foo; bar. If foo in the example has a non-zero exit code, bar wouldn't be run.

This also affects subexpressions.

Note: this breaks some tests, so we'll want to think through a) if we want this approach, b) how do we easily want to say "continue if there is an error" in the same way bash differentiates between ; and &&.

User-Facing Changes

This would be a breaking change, as both interactive and scripts could work differently in the presence of non-zero exit codes.

Tests + Formatting

After Submitting

@sophiajt sophiajt marked this pull request as draft April 24, 2023 20:53
@WindSoilder
Copy link
Copy Markdown
Contributor

Thanks @jntrnr, I really like the change, it's much better than my attempt :-D

@WindSoilder
Copy link
Copy Markdown
Contributor

It will fix #7690

@WindSoilder WindSoilder marked this pull request as ready for review December 5, 2023 06:24
Copy link
Copy Markdown
Contributor

@WindSoilder WindSoilder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it works fine after some redirection fixes and we disabled pipeline echo in #8292

It has bothered me for a long time, thanks for the fix!

@WindSoilder WindSoilder merged commit 2ffe30e into nushell:main Dec 5, 2023
@sholderbach sholderbach added notes:breaking-changes This PR implies a change affecting users and has to be noted in the release notes A:error-handling How errors in externals/nu code are caught or handled programmatically (see also unhelpful-error) labels Dec 5, 2023
@sophiajt sophiajt deleted the respect_non_zero_error branch December 8, 2023 19:36
@amtoine
Copy link
Copy Markdown
Member

amtoine commented Dec 10, 2023

@WindSoilder
i just tried

(^false; "hello,world!")

on this PR and, even though it correctly does not print the string, it does not return the exit code of 1 from ^false, is that expected?

hardfau1t pushed a commit to hardfau1t/nushell that referenced this pull request Dec 14, 2023
# Description

This PR changes the way we handled non-zero exit codes to be and early
exit between `foo; bar`. If `foo` in the example has a non-zero exit
code, `bar` wouldn't be run.

This also affects subexpressions.
dmatos2012 pushed a commit to dmatos2012/nushell that referenced this pull request Feb 20, 2024
# Description

This PR changes the way we handled non-zero exit codes to be and early
exit between `foo; bar`. If `foo` in the example has a non-zero exit
code, `bar` wouldn't be run.

This also affects subexpressions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A:error-handling How errors in externals/nu code are caught or handled programmatically (see also unhelpful-error) notes:breaking-changes This PR implies a change affecting users and has to be noted in the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SubExpression with external command failure does not stop command running

4 participants