Don't print a stray blank line when try with catch or finally collects an external - #18811
Merged
fdncred merged 2 commits intoSep 2, 2026
Merged
Conversation
A `try` with a `catch` or `finally` block collects the block's output to know whether it failed before anything streams (7f69d75). When the tried command is an external whose stdout was inherited, its output has already gone to the terminal, so there is nothing to collect. Collecting anyway turned the data-less child stream into an empty string, and printing that empty string produced a stray blank line after the external's own output: > nu -c "try { ^nu -c 'print hi' } catch {}" hi > The same happened with a `finally` block and when the external's stdout was redirected to a file inside the block. Now `collect` waits for such a child, so a failure still surfaces and `catch` still runs, and produces Empty, matching what draining the same stream produces in the plain `try { }` case. Thanks to neurolag for bisecting the inconsistency to 7f69d75 and to weirdan for pinning it on collection. Fixes nushell#18765
fdncred
reviewed
Aug 10, 2026
Contributor
|
Thanks |
Dheebz
pushed a commit
to Dheebz/nushell
that referenced
this pull request
Sep 3, 2026
…s an external (nushell#18811) # Description Fixes nushell#18765. Byte for byte, before: ``` > nu -c "try { ^nu -c 'print hi' } catch {}" | od -c 0000000 h i \n \n ``` and after: ``` 0000000 h i \n ``` Since 7f69d75, a `try` block followed by `catch` or `finally` collects the block's output first, so it can tell whether the block failed before anything starts streaming. If the tried command is an external whose stdout was not captured (it is inherited at the statement level, or redirected to a file inside the block), the output has already left the process by the time the collect runs. The collect then converted that data-less child stream into an empty string value, and printing the collected result rendered the empty string as an extra blank line. The fix is in the shared `collect` helper in `eval_ir.rs`: when the collected data is a byte stream whose child has no captured stdout, wait for the child and produce `Empty`, which is exactly what the plain `try { }` path produces when it drains the same stream. Waiting keeps the failure semantics: a non-zero exit still becomes an error, so `catch` still runs (covered by a test). Behavior that did not change, verified against an unpatched build: - Value context still collects and trims the captured output: `let x = (try { ^cmd } catch {})` still gives the trimmed string. - `$env.LAST_EXIT_CODE` was not set by the collect path before and still is not. Credit where due: neurolag bisected the inconsistency to 7f69d75, and weirdan showed it is the collection step that differs. # User-Facing Changes `try { ^external } catch { }` and `try { ^external } finally { }` no longer print a stray blank line after the external's output. Collecting an uncaptured external (for example `try { ^cmd o> file } catch {}` as the value of an assignment) now produces nothing instead of an empty string. # Tests + Formatting Added `try_catch_inherited_external_output_has_no_extra_blank_line` in `tests/shell/mod.rs`. It spawns the real `nu` binary and compares raw stdout bytes for the `catch`, `finally`, and `catch` plus `finally` cases, plus a failing external to prove `catch` still runs. The `try` sits in final statement position on purpose: only the final statement's value is printed, so that is the position where the fabricated empty string became visible. The test fails on current main with exactly the extra `\n` and passes with this change. - `cargo fmt --all -- --check` clean - `cargo clippy --workspace --all-targets -- -D warnings` clean - `shell::` suite 255 passed, `commands::try_::` suite 41 passed (includes the pipefail experimental tests) - `cargo check -p nu-engine --target wasm32-unknown-unknown --no-default-features` compiles (the new block is gated on the `os` feature) # After Submitting ## Release notes summary - What our users need to know `try` with a `catch` or `finally` block no longer prints a stray blank line after an external command's output.
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
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
Fixes #18765. Byte for byte, before:
and after:
Since 7f69d75, a
tryblock followed bycatchorfinallycollects the block's output first, so it can tell whether the block failed before anything starts streaming. If the tried command is an external whose stdout was not captured (it is inherited at the statement level, or redirected to a file inside the block), the output has already left the process by the time the collect runs. The collect then converted that data-less child stream into an empty string value, and printing the collected result rendered the empty string as an extra blank line.The fix is in the shared
collecthelper ineval_ir.rs: when the collected data is a byte stream whose child has no captured stdout, wait for the child and produceEmpty, which is exactly what the plaintry { }path produces when it drains the same stream. Waiting keeps the failure semantics: a non-zero exit still becomes an error, socatchstill runs (covered by a test).Behavior that did not change, verified against an unpatched build:
let x = (try { ^cmd } catch {})still gives the trimmed string.$env.LAST_EXIT_CODEwas not set by the collect path before and still is not.Credit where due: neurolag bisected the inconsistency to 7f69d75, and weirdan showed it is the collection step that differs.
User-Facing Changes
try { ^external } catch { }andtry { ^external } finally { }no longer print a stray blank line after the external's output. Collecting an uncaptured external (for exampletry { ^cmd o> file } catch {}as the value of an assignment) now produces nothing instead of an empty string.Tests + Formatting
Added
try_catch_inherited_external_output_has_no_extra_blank_lineintests/shell/mod.rs. It spawns the realnubinary and compares raw stdout bytes for thecatch,finally, andcatchplusfinallycases, plus a failing external to provecatchstill runs. Thetrysits in final statement position on purpose: only the final statement's value is printed, so that is the position where the fabricated empty string became visible. The test fails on current main with exactly the extra\nand passes with this change.cargo fmt --all -- --checkcleancargo clippy --workspace --all-targets -- -D warningscleanshell::suite 255 passed,commands::try_::suite 41 passed (includes the pipefail experimental tests)cargo check -p nu-engine --target wasm32-unknown-unknown --no-default-featurescompiles (the new block is gated on theosfeature)After Submitting
Release notes summary - What our users need to know
trywith acatchorfinallyblock no longer prints a stray blank line after an external command's output.