Skip to content

make pipefail works with let#16879

Merged
132ikl merged 2 commits intonushell:mainfrom
WindSoilder:collect_on_pipefail
Oct 17, 2025
Merged

make pipefail works with let#16879
132ikl merged 2 commits intonushell:mainfrom
WindSoilder:collect_on_pipefail

Conversation

@WindSoilder
Copy link
Copy Markdown
Contributor

This pr address comment by @132ikl : #16760 (comment)

After visiting IR of let x = ^false | into int:

# 2 registers, 8 instructions, 5 bytes of data
   0: load-literal           %1, glob-pattern("false", no_expand = false)
   1: push-positional        %1
   2: redirect-out           pipe
   3: call                   decl 142 "run-external", %0
   4: redirect-out           value
   5: call                   decl 299 "into int", %0
   6: store-variable         var 320, %0 # let
   7: return                 %0

Nushell runs store-variable instruction, which collect values from registry through collect_reg. This pr adds check_exit_status_future logic inside collect_reg to make pipefail works with let.

Release notes summary - What our users need to know

pipefail works well with let

let x = bash -c "echo 123; exit 1" | into int; $x
Error: nu::shell::non_zero_exit_code

  × External command had a non-zero exit code
   ╭─[entry #28:1:9]
 1 │ let x = bash -c "echo 123; exit 1" | into int; $x
   ·         ──┬─
   ·           ╰── exited with code 1
   ╰────

Tasks after submitting

@132ikl
Copy link
Copy Markdown
Member

132ikl commented Oct 16, 2025

lgtm although looks like we need an os feature gate here

@WindSoilder
Copy link
Copy Markdown
Contributor Author

Thanks for reminding this!

@132ikl 132ikl merged commit 1025ddb into nushell:main Oct 17, 2025
16 checks passed
@132ikl 132ikl added the notes:fixes Include the release notes summary in the "Bug fixes" section label Oct 17, 2025
@github-actions github-actions bot added this to the v0.109.0 milestone Oct 17, 2025
@WindSoilder WindSoilder deleted the collect_on_pipefail branch January 29, 2026 06:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

notes:fixes Include the release notes summary in the "Bug fixes" section

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants