Restore original do -i behavior and add flags to break down shell vs program errors (#7122)

Closes https://github.com/nushell/nushell/issues/7076, fixes
https://github.com/nushell/nushell/issues/6956

cc @WindSoilder @fdncred

Signed-off-by: Alex Saveau <saveau.alexandre@gmail.com>

crates/nu-command/src/core_commands/do_.rs

@@ -2,8 +2,7 @@ use nu_engine::{eval_block, CallExt};
 use nu_protocol::ast::Call;
 use nu_protocol::engine::{Closure, Command, EngineState, Stack};
 use nu_protocol::{
-    Category, Example, ListStream, PipelineData, RawStream, ShellError, Signature, SyntaxShape,
+    Category, Example, ListStream, PipelineData, ShellError, Signature, SyntaxShape, Type, Value,
-    Type, Value,
 };
 
 #[derive(Clone)]
@@ -18,15 +17,25 @@ impl Command for Do {
         "Run a block"
     }
 
-    fn signature(&self) -> nu_protocol::Signature {
+    fn signature(&self) -> Signature {
         Signature::build("do")
             .required("closure", SyntaxShape::Any, "the closure to run")
             .input_output_types(vec![(Type::Any, Type::Any)])
             .switch(
                 "ignore-errors",
-                "ignore shell errors as the block runs",
+                "ignore errors as the block runs",
                 Some('i'),
             )
+            .switch(
+                "ignore-shell-errors",
+                "ignore shell errors as the block runs",
+                Some('s'),
+            )
+            .switch(
+                "ignore-program-errors",
+                "ignore program errors as the block runs",
+                Some('p'),
+            )
             .switch(
                 "capture-errors",
                 "capture errors as the block runs and return it",
@@ -42,10 +51,12 @@ impl Command for Do {
         stack: &mut Stack,
         call: &Call,
         input: PipelineData,
-    ) -> Result<nu_protocol::PipelineData, nu_protocol::ShellError> {
+    ) -> Result<PipelineData, ShellError> {
         let block: Closure = call.req(engine_state, stack, 0)?;
         let rest: Vec<Value> = call.rest(engine_state, stack, 1)?;
-        let ignore_errors = call.has_flag("ignore-errors");
+        let ignore_all_errors = call.has_flag("ignore-errors");
+        let ignore_shell_errors = ignore_all_errors || call.has_flag("ignore-shell-errors");
+        let ignore_program_errors = ignore_all_errors || call.has_flag("ignore-program-errors");
         let capture_errors = call.has_flag("capture-errors");
 
         let mut stack = stack.captures_to_stack(&block.captures);
@@ -95,17 +106,9 @@ impl Command for Do {
             block,
             input,
             call.redirect_stdout,
-            ignore_errors || capture_errors,
+            capture_errors || ignore_shell_errors || ignore_program_errors,
         );
 
-        if ignore_errors {
-            match result {
-                Ok(x) => Ok(x),
-                Err(_) => Ok(PipelineData::new(call.head)),
-            }
-        } else if capture_errors {
-            // collect stdout and stderr and check exit code.
-            // if exit code is not 0, return back ShellError.
         match result {
             Ok(PipelineData::ExternalStream {
                 stdout,
@@ -113,26 +116,7 @@ impl Command for Do {
                 exit_code,
                 span,
                 metadata,
-                }) => {
+            }) if capture_errors => {
-                    // collect all output first.
-                    let mut stderr_ctrlc = None;
-                    let stderr_msg = match stderr {
-                        None => "".to_string(),
-                        Some(stderr_stream) => {
-                            stderr_ctrlc = stderr_stream.ctrlc.clone();
-                            stderr_stream.into_string().map(|s| s.item)?
-                        }
-                    };
-
-                    let mut stdout_ctrlc = None;
-                    let stdout_msg = match stdout {
-                        None => "".to_string(),
-                        Some(stdout_stream) => {
-                            stdout_ctrlc = stdout_stream.ctrlc.clone();
-                            stdout_stream.into_string().map(|s| s.item)?
-                        }
-                    };
-
                 let mut exit_code_ctrlc = None;
                 let exit_code: Vec<Value> = match exit_code {
                     None => vec![],
@@ -142,27 +126,23 @@ impl Command for Do {
                     }
                 };
                 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 {
+                        let stderr_msg = match stderr {
+                            None => "".to_string(),
+                            Some(stderr_stream) => stderr_stream.into_string().map(|s| s.item)?,
+                        };
+
                         return Err(ShellError::ExternalCommand(
-                                "External command runs to failed".to_string(),
+                            "External command failed".to_string(),
                             stderr_msg,
                             span,
                         ));
                     }
                 }
-                    // construct pipeline data to our caller
+
                 Ok(PipelineData::ExternalStream {
-                        stdout: Some(RawStream::new(
+                    stdout,
-                            Box::new(vec![Ok(stdout_msg.into_bytes())].into_iter()),
+                    stderr,
-                            stdout_ctrlc,
-                            span,
-                        )),
-                        stderr: Some(RawStream::new(
-                            Box::new(vec![Ok(stderr_msg.into_bytes())].into_iter()),
-                            stderr_ctrlc,
-                            span,
-                        )),
                     exit_code: Some(ListStream::from_stream(
                         exit_code.into_iter(),
                         exit_code_ctrlc,
@@ -171,11 +151,21 @@ impl Command for Do {
                     metadata,
                 })
             }
-                Ok(other) => Ok(other),
+            Ok(PipelineData::ExternalStream {
-                Err(e) => Err(e),
+                stdout,
-            }
+                stderr,
-        } else {
+                exit_code: _,
-            result
+                span,
+                metadata,
+            }) if ignore_program_errors => Ok(PipelineData::ExternalStream {
+                stdout,
+                stderr,
+                exit_code: None,
+                span,
+                metadata,
+            }),
+            Err(_) if ignore_shell_errors => Ok(PipelineData::new(call.head)),
+            r => r,
         }
     }
 
@@ -187,10 +177,20 @@ impl Command for Do {
                 result: Some(Value::test_string("hello")),
             },
             Example {
-                description: "Run the block and ignore shell errors",
+                description: "Run the block and ignore both shell and program errors",
                 example: r#"do -i { thisisnotarealcommand }"#,
                 result: None,
             },
+            Example {
+                description: "Run the block and ignore shell errors",
+                example: r#"do -s { thisisnotarealcommand }"#,
+                result: None,
+            },
+            Example {
+                description: "Run the block and ignore program errors",
+                example: r#"do -p { nu -c 'exit 1' }; echo "I'll still run""#,
+                result: None,
+            },
             Example {
                 description: "Abort the pipeline if a program returns a non-zero exit code",
                 example: r#"do -c { nu -c 'exit 1' } | myscarycommand"#,

crates/nu-command/tests/commands/do_.rs

@@ -20,7 +20,7 @@ fn capture_errors_works_for_external() {
         do -c {nu --testbin fail}
         "#
     ));
-    assert!(actual.err.contains("External command runs to failed"));
+    assert!(actual.err.contains("External command failed"));
     assert_eq!(actual.out, "");
 }
 
@@ -32,7 +32,7 @@ fn capture_errors_works_for_external_with_pipeline() {
         do -c {nu --testbin fail} | echo `text`
         "#
     ));
-    assert!(actual.err.contains("External command runs to failed"));
+    assert!(actual.err.contains("External command failed"));
     assert_eq!(actual.out, "");
 }
 
@@ -44,7 +44,7 @@ fn capture_errors_works_for_external_with_semicolon() {
         do -c {nu --testbin fail}; echo `text`
         "#
     ));
-    assert!(actual.err.contains("External command runs to failed"));
+    assert!(actual.err.contains("External command failed"));
     assert_eq!(actual.out, "");
 }
 
@@ -60,6 +60,42 @@ fn do_with_semicolon_break_on_failed_external() {
     assert_eq!(actual.out, "");
 }
 
+#[test]
+fn ignore_shell_errors_works_for_external_with_semicolon() {
+    let actual = nu!(
+        cwd: ".", pipeline(
+        r#"
+        do -s { fail }; `text`
+        "#
+    ));
+
+    assert_eq!(actual.err, "");
+    assert_eq!(actual.out, "text");
+}
+
+#[test]
+fn ignore_program_errors_works_for_external_with_semicolon() {
+    let actual = nu!(
+        cwd: ".", pipeline(
+        r#"
+        do -p { nu -c 'exit 1' }; `text`
+        "#
+    ));
+
+    assert_eq!(actual.err, "");
+    assert_eq!(actual.out, "text");
+}
+
+#[test]
+fn ignore_error_should_work_for_external_command() {
+    let actual = nu!(cwd: ".", pipeline(
+        r#"do -i { nu --testbin fail asdf }; echo post"#
+    ));
+
+    assert_eq!(actual.err, "");
+    assert_eq!(actual.out, "post");
+}
+
 #[test]
 #[cfg(not(windows))]
 fn ignore_error_with_too_much_stderr_not_hang_nushell() {