Make semicolon works better for internal commands (#6643)

* make semicolon works with some internal command like do

* refactor, make consume external result logic out of eval_external

* update comment

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, "");
+}

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<PipelineData, ShellError> {
     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
+    command.run(engine_state, stack, &call, input)
-    // 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<Value> = 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))
-    }
 }
 
 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<Value> = 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(