Fix ir eval error on branched block redirection (Fixes #16582)#16676
Merged
WindSoilder merged 6 commits intonushell:mainfrom Sep 19, 2025
Merged
Fix ir eval error on branched block redirection (Fixes #16582)#16676WindSoilder merged 6 commits intonushell:mainfrom
WindSoilder merged 6 commits intonushell:mainfrom
Conversation
*Description* Attempt to fix issue [nushell#16582](nushell#16582) by avoiding duplicated `write` and `close` calls for an `out>`/`err>` redirected file for `try-catch` and `if-else` blocks. Similarly to [this PR](nushell#15617) a function to identify if a code block is being handeled, and if so skip the invocation of `finish_redirection` so that no additional writes are added. (The blocks perform redirections on their own and this then avoids the duplication). *User-Facing Changes* The following commands will no longer raise ir-eval errors: - `if true { echo "hey" } out> /dev/null` - `if false { echo "hey" } else { echo "ho" } out> /dev/null` - `try { 1/0 } catch { echo "hi" } out> /dev/null` *Tests* Added 8 tests.
fixerer
commented
Sep 13, 2025
sholderbach
reviewed
Sep 15, 2025
Member
sholderbach
left a comment
There was a problem hiding this comment.
Thanks for looking into this. Based on the tests it looks already pretty good for STDOUT. Unsure about the STDERR.
Would like to refer you to @WindSoilder for the details.
WindSoilder
reviewed
Sep 17, 2025
fixerer
commented
Sep 17, 2025
Question on the while construct - notably why it is allowed while the for one isn't. Should this be the case, and what should the output result be if so.
f6767be to
ad77ef3
Compare
WindSoilder
requested changes
Sep 17, 2025
2e0390d to
14e1536
Compare
WindSoilder
approved these changes
Sep 19, 2025
Contributor
WindSoilder
left a comment
There was a problem hiding this comment.
Thanks! Looks good to me
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Attempt to fix issue #16582 by avoiding duplicated
writeandclosecalls for anout>/err>redirected file fortry-catchandif-elseblocks.Similarly to this PR a function to identify if a code block is being handeled, and if so skip the invocation of
finish_redirectionso that no additional writes are added. (The blocks perform redirections on their own and this then avoids the duplication).Release notes summary - What our users need to know
Fixed IR evaluation error caused by redirecting output of branched blocks
The following commands will no longer raise ir-eval errors:
if true { echo "hey" } out> /dev/nullif false { echo "hey" } else { echo "ho" } out> /dev/nulltry { 1/0 } catch { echo "hi" } out> /dev/nullIR before changes:
before.txt
IR after changes:
after.txt
Tests
Added 8 tests.
toolkit fmttoolkit clippytoolkit testtoolkit test stdlib