From f74694d5a35a402cb1e160f65bc80538684fe26f Mon Sep 17 00:00:00 2001 From: WindSoilder Date: Mon, 30 Jan 2023 23:49:31 +0800 Subject: [PATCH] Let redirection keep exit code (#7848) # Description Fixes: #7828 We delegate to `save` command to finish redirection, then if it runs to success, the relative exit code is set to 0. To fix it, in redirection context, we take exit_code stream before sending it to `save` command, than manually returns `PipelineData::ExternalStream` to make nushell set relative code properly. # User-Facing Changes _(List of all changes that impact the user experience here. This helps us keep track of breaking changes.)_ # Tests + Formatting Don't forget to add tests that cover your changes. Make sure you've run and fixed any issues with these commands: - `cargo fmt --all -- --check` to check standard code formatting (`cargo fmt --all` applies these changes) - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A clippy::needless_collect` to check that you're using the standard code style - `cargo test --workspace` to check that all tests pass # After Submitting If your PR had any user-facing changes, update [the documentation](https://github.com/nushell/nushell.github.io) after the PR is merged, if necessary. This will help us keep the docs up to date. --- .../nu-command/tests/commands/redirection.rs | 27 ++++++++++++ crates/nu-engine/src/eval.rs | 44 +++++++++++++++++-- 2 files changed, 68 insertions(+), 3 deletions(-) diff --git a/crates/nu-command/tests/commands/redirection.rs b/crates/nu-command/tests/commands/redirection.rs index f7371d9cf..115d08f23 100644 --- a/crates/nu-command/tests/commands/redirection.rs +++ b/crates/nu-command/tests/commands/redirection.rs @@ -126,6 +126,33 @@ fn separate_redirection() { ) } +#[test] +fn redirection_keep_exit_codes() { + Playground::setup( + "redirection should keep exit code the same", + |dirs, sandbox| { + let script_body = r#"exit 10"#; + #[cfg(not(windows))] + let output = { + sandbox.with_files(vec![FileWithContent("test.sh", script_body)]); + nu!( + cwd: dirs.test(), + r#"bash test.sh out> out.txt err> err.txt; echo $env.LAST_EXIT_CODE"# + ) + }; + #[cfg(windows)] + let output = { + sandbox.with_files(vec![FileWithContent("test.bat", script_body)]); + nu!( + cwd: dirs.test(), + r#"cmd /D /c test.bat out> out.txt err> err.txt; echo $env.LAST_EXIT_CODE"# + ) + }; + assert_eq!(output.out, "10") + }, + ) +} + #[cfg(not(windows))] #[test] fn redirection_with_pipeline_works() { diff --git a/crates/nu-engine/src/eval.rs b/crates/nu-engine/src/eval.rs index 6c1578479..f862d4e54 100644 --- a/crates/nu-engine/src/eval.rs +++ b/crates/nu-engine/src/eval.rs @@ -710,7 +710,7 @@ pub fn eval_element_with_input( engine_state: &EngineState, stack: &mut Stack, element: &PipelineElement, - input: PipelineData, + mut input: PipelineData, redirect_stdout: bool, redirect_stderr: bool, ) -> Result<(PipelineData, bool), ShellError> { @@ -725,6 +725,10 @@ pub fn eval_element_with_input( ), PipelineElement::Redirection(span, redirection, expr) => match &expr.expr { Expr::String(_) => { + let exit_code = match &mut input { + PipelineData::ExternalStream { exit_code, .. } => exit_code.take(), + _ => None, + }; let input = match (redirection, input) { ( Redirection::Stderr, @@ -823,7 +827,22 @@ pub fn eval_element_with_input( }, input, ) - .map(|x| (x, false)) + .map(|_| { + // save is internal command, normally it exists with non-ExternalStream + // but here in redirection context, we make it returns ExternalStream + // So nu handles exit_code correctly + ( + PipelineData::ExternalStream { + stdout: None, + stderr: None, + exit_code, + span: *span, + metadata: None, + trim_end_newline: false, + }, + false, + ) + }) } else { Err(ShellError::CommandNotFound(*span)) } @@ -836,6 +855,10 @@ pub fn eval_element_with_input( } => match (&out_expr.expr, &err_expr.expr) { (Expr::String(_), Expr::String(_)) => { if let Some(save_command) = engine_state.find_decl(b"save", &[]) { + let exit_code = match &mut input { + PipelineData::ExternalStream { exit_code, .. } => exit_code.take(), + _ => None, + }; eval_call( engine_state, stack, @@ -875,7 +898,22 @@ pub fn eval_element_with_input( }, input, ) - .map(|x| (x, false)) + .map(|_| { + // save is internal command, normally it exists with non-ExternalStream + // but here in redirection context, we make it returns ExternalStream + // So nu handles exit_code correctly + ( + PipelineData::ExternalStream { + stdout: None, + stderr: None, + exit_code, + span: *out_span, + metadata: None, + trim_end_newline: false, + }, + false, + ) + }) } else { Err(ShellError::CommandNotFound(*out_span)) }