diff --git a/crates/nu-command/tests/commands/do_.rs b/crates/nu-command/tests/commands/do_.rs index 8d64b0375..27b4394b2 100644 --- a/crates/nu-command/tests/commands/do_.rs +++ b/crates/nu-command/tests/commands/do_.rs @@ -11,3 +11,15 @@ fn capture_errors_works() { assert_eq!(actual.out, "error"); } + +#[test] +fn do_with_semicolon_break_on_failed_external() { + let actual = nu!( + cwd: ".", pipeline( + r#" + do { nu --not_exist_flag }; `text` + "# + )); + + assert_eq!(actual.out, ""); +} diff --git a/crates/nu-engine/src/eval.rs b/crates/nu-engine/src/eval.rs index 3d1e02fd4..2c9350fab 100644 --- a/crates/nu-engine/src/eval.rs +++ b/crates/nu-engine/src/eval.rs @@ -201,7 +201,7 @@ fn eval_external( input: PipelineData, redirect_stdout: bool, redirect_stderr: bool, -) -> Result<(PipelineData, bool), ShellError> { +) -> Result { let decl_id = engine_state .find_decl("run-external".as_bytes(), &[]) .ok_or(ShellError::ExternalNotSupported(head.span))?; @@ -238,54 +238,7 @@ fn eval_external( )) } - // when the external command doesn't redirect output, we eagerly check the result - // and find if the command runs to failed. - let mut runs_to_failed = false; - let result = command.run(engine_state, stack, &call, input)?; - if let PipelineData::ExternalStream { - stdout: None, - stderr, - mut exit_code, - span, - metadata, - } = result - { - let exit_code = exit_code.take(); - match exit_code { - Some(exit_code_stream) => { - let ctrlc = exit_code_stream.ctrlc.clone(); - let exit_code: Vec = exit_code_stream.into_iter().collect(); - if let Some(Value::Int { val: code, .. }) = exit_code.last() { - // if exit_code is not 0, it indicates error occured, return back Err. - if *code != 0 { - runs_to_failed = true; - } - } - Ok(( - PipelineData::ExternalStream { - stdout: None, - stderr, - exit_code: Some(ListStream::from_stream(exit_code.into_iter(), ctrlc)), - span, - metadata, - }, - runs_to_failed, - )) - } - None => Ok(( - PipelineData::ExternalStream { - stdout: None, - stderr, - exit_code: None, - span, - metadata, - }, - runs_to_failed, - )), - } - } else { - Ok((result, runs_to_failed)) - } + command.run(engine_state, stack, &call, input) } pub fn eval_expression( @@ -383,7 +336,6 @@ pub fn eval_expression( false, false, )? - .0 .into_value(span)) } Expr::DateTime(dt) => Ok(Value::Date { @@ -682,7 +634,6 @@ pub fn eval_expression_with_input( redirect_stdout: bool, redirect_stderr: bool, ) -> Result<(PipelineData, bool), ShellError> { - let mut external_failed = false; match expr { Expression { expr: Expr::Call(call), @@ -702,7 +653,7 @@ pub fn eval_expression_with_input( expr: Expr::ExternalCall(head, args), .. } => { - let external_result = eval_external( + input = eval_external( engine_state, stack, head, @@ -711,8 +662,6 @@ pub fn eval_expression_with_input( redirect_stdout, redirect_stderr, )?; - input = external_result.0; - external_failed = external_result.1 } Expression { @@ -728,9 +677,63 @@ pub fn eval_expression_with_input( elem => { input = eval_expression(engine_state, stack, elem)?.into_pipeline_data(); } - } + }; - Ok((input, external_failed)) + Ok(might_consume_external_result(input)) +} + +// if the result is ExternalStream without redirecting output. +// that indicates we have no more commands to execute currently. +// we can try to catch and detect if external command runs to failed. +// +// This is useful to commands with semicolon, we can detect errors early to avoid +// commands after semicolon running. +fn might_consume_external_result(input: PipelineData) -> (PipelineData, bool) { + let mut runs_to_failed = false; + if let PipelineData::ExternalStream { + stdout: None, + stderr, + mut exit_code, + span, + metadata, + } = input + { + let exit_code = exit_code.take(); + match exit_code { + Some(exit_code_stream) => { + let ctrlc = exit_code_stream.ctrlc.clone(); + let exit_code: Vec = exit_code_stream.into_iter().collect(); + if let Some(Value::Int { val: code, .. }) = exit_code.last() { + // if exit_code is not 0, it indicates error occured, return back Err. + if *code != 0 { + runs_to_failed = true; + } + } + ( + PipelineData::ExternalStream { + stdout: None, + stderr, + exit_code: Some(ListStream::from_stream(exit_code.into_iter(), ctrlc)), + span, + metadata, + }, + runs_to_failed, + ) + } + None => ( + PipelineData::ExternalStream { + stdout: None, + stderr, + exit_code: None, + span, + metadata, + }, + runs_to_failed, + ), + } + } else { + (input, false) + } } pub fn eval_block(