Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make stderr works for failed external command #11914

Merged
merged 1 commit into from Feb 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 16 additions & 0 deletions crates/nu-command/tests/commands/redirection.rs
Expand Up @@ -171,6 +171,22 @@ fn redirection_keep_exit_codes() {
});
}

#[test]
fn redirection_stderr_with_failed_program() {
Playground::setup("redirection stderr with failed program", |dirs, _| {
let out = nu!(
cwd: dirs.test(),
r#"$env.FOO = "bar"; nu --testbin echo_env_stderr_fail FOO e> file.txt; echo 3"#
Copy link
Member

Choose a reason for hiding this comment

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

Was confused at first why FOO was passed bare but then I read the echo_ENV_stderr_fail 😆

);
// firstly echo 3 shouldn't run, because previous command runs to failed.
// second `file.txt` should contain "bar".
assert!(!out.out.contains('3'));
let expected_file = dirs.test().join("file.txt");
let actual = file_contents(expected_file);
assert_eq!(actual, "bar\n");
});
}

#[test]
fn redirection_with_non_zero_exit_code_should_stop_from_running() {
Playground::setup("redirection with non zero exit code", |dirs, _| {
Expand Down
16 changes: 15 additions & 1 deletion crates/nu-engine/src/eval.rs
Expand Up @@ -383,7 +383,21 @@ pub fn eval_expression_with_input(
}
};

Ok(might_consume_external_result(input))
// Given input is PipelineData::ExternalStream
// `might_consume_external_result` will consume `stderr` stream if `stdout` is empty.
// it's not intended if user want to redirect stderr message.
//
// e.g:
// 1. cargo check e>| less
// 2. cargo check e> result.txt
//
// In these two cases, stdout will be empty, but nushell shouldn't consume the `stderr`
// stream it needs be passed to next command.
if !redirect_stderr {
Ok(might_consume_external_result(input))
} else {
Ok((input, false))
}
}

// Try to catch and detect if external command runs to failed.
Expand Down
1 change: 1 addition & 0 deletions src/main.rs
Expand Up @@ -269,6 +269,7 @@ fn main() -> Result<()> {
match testbin.item.as_str() {
"echo_env" => test_bins::echo_env(true),
"echo_env_stderr" => test_bins::echo_env(false),
"echo_env_stderr_fail" => test_bins::echo_env_and_fail(false),
"echo_env_mixed" => test_bins::echo_env_mixed(),
"cococo" => test_bins::cococo(),
"meow" => test_bins::meow(),
Expand Down
5 changes: 5 additions & 0 deletions src/test_bins.rs
Expand Up @@ -16,6 +16,11 @@ pub fn echo_env(to_stdout: bool) {
}
}

pub fn echo_env_and_fail(to_stdout: bool) {
echo_env(to_stdout);
fail();
}

fn echo_one_env(arg: &str, to_stdout: bool) {
if let Ok(v) = std::env::var(arg) {
if to_stdout {
Expand Down
8 changes: 8 additions & 0 deletions tests/shell/pipeline/commands/external.rs
Expand Up @@ -158,6 +158,14 @@ fn basic_outerr_pipe_works() {
assert_eq!(actual.out, "8");
}

#[test]
fn err_pipe_with_failed_external_works() {
let actual =
nu!(r#"with-env [FOO "bar"] { nu --testbin echo_env_stderr_fail FOO e>| str length }"#);
// there is a `newline` output from nu --testbin
assert_eq!(actual.out, "4");
}

mod it_evaluation {
use super::nu;
use nu_test_support::fs::Stub::{EmptyFile, FileWithContent, FileWithContentToBeTrimmed};
Expand Down