Improved output from bash scripts to allow separation of results from…

… different lines and more helper methods (#28)

* Improved output from bash scripts to allow separation of results from different lines and more helper methods

* Fix windows

rust/bitbazaar/cli/bash.rs

@@ -3,7 +3,7 @@ use std::{
     path::{Path, PathBuf},
 };
 
-use super::{errs::ShellErr, shell::Shell, BashErr, CmdOut};
+use super::{errs::ShellErr, shell::Shell, BashErr, BashOut};
 use crate::prelude::*;
 
 /// Execute an arbitrary bash script.
@@ -95,13 +95,13 @@ impl Bash {
     }
 
     /// Execute the current contents of the bash script.
-    pub fn run(self) -> Result<CmdOut, BashErr> {
+    pub fn run(self) -> Result<BashOut, BashErr> {
         if self.cmds.is_empty() {
-            return Ok(CmdOut::empty());
+            return Ok(BashOut::empty());
         }
 
         let mut shell = Shell::new(self.env_vars, self.root_dir)
-            .map_err(|e| shell_to_bash_err(CmdOut::empty(), e))?;
+            .map_err(|e| shell_to_bash_err(BashOut::empty(), e))?;
 
         if let Err(e) = shell.execute_command_strings(self.cmds) {
             return Err(shell_to_bash_err(shell.into(), e));
@@ -112,19 +112,19 @@ impl Bash {
 }
 
 fn shell_to_bash_err(
-    mut cmd_out: CmdOut,
+    mut bash_out: BashOut,
     e: error_stack::Report<ShellErr>,
 ) -> error_stack::Report<BashErr> {
     // Doesn't really make sense, but set the exit code to 1 if 0, as technically the command errored even though it was the runner itself that errored and the command might not have been attempted.
-    if cmd_out.code == 0 {
-        cmd_out.code = 1;
+    if bash_out.code() == 0 {
+        bash_out.override_code(1);
     }
     match e.current_context() {
-        ShellErr::Exit => e.change_context(BashErr::InternalError(cmd_out)).attach_printable(
+        ShellErr::Exit => e.change_context(BashErr::InternalError(bash_out)).attach_printable(
             "Shouldn't occur, shell exit errors should have been managed internally, not an external error.",
         ),
-        ShellErr::InternalError => e.change_context(BashErr::InternalError(cmd_out)),
-        ShellErr::BashFeatureUnsupported => e.change_context(BashErr::BashFeatureUnsupported(cmd_out)),
-        ShellErr::BashSyntaxError => e.change_context(BashErr::BashSyntaxError(cmd_out)),
+        ShellErr::InternalError => e.change_context(BashErr::InternalError(bash_out)),
+        ShellErr::BashFeatureUnsupported => e.change_context(BashErr::BashFeatureUnsupported(bash_out)),
+        ShellErr::BashSyntaxError => e.change_context(BashErr::BashSyntaxError(bash_out)),
     }
 }

rust/bitbazaar/cli/bash_out.rs

@@ -0,0 +1,178 @@
+use crate::prelude::*;
+
+/// The result of an individual command.
+#[derive(Debug, Clone)]
+pub struct CmdResult {
+    /// The command that was run
+    pub command: String,
+    /// The exit code of the command
+    pub code: i32,
+    /// The stdout of the command
+    pub stdout: String,
+    /// The stderr of the command
+    pub stderr: String,
+}
+
+impl CmdResult {
+    /// Create a new CmdResult.
+    pub(crate) fn new(
+        command: impl Into<String>,
+        code: i32,
+        stdout: impl Into<String>,
+        stderr: impl Into<String>,
+    ) -> Self {
+        Self {
+            command: command.into(),
+            code,
+            stdout: stdout.into(),
+            stderr: stderr.into(),
+        }
+    }
+}
+
+/// The result of running a command
+#[derive(Debug, Clone)]
+pub struct BashOut {
+    /// All commands that were run, if a command fails, it will be the last command in this vec, the remaining were not attempted.
+    pub command_results: Vec<CmdResult>,
+
+    code_override: Option<i32>,
+}
+
+impl From<CmdResult> for BashOut {
+    fn from(result: CmdResult) -> Self {
+        Self {
+            command_results: vec![result],
+            code_override: None,
+        }
+    }
+}
+
+/// Public interface
+impl BashOut {
+    /// Returns the exit code of the last command that was run.
+    pub fn code(&self) -> i32 {
+        if let Some(code) = self.code_override {
+            code
+        } else {
+            self.command_results.last().map(|r| r.code).unwrap_or(0)
+        }
+    }
+
+    /// Returns true when the command exited with a zero exit code.
+    pub fn success(&self) -> bool {
+        self.code() == 0
+    }
+
+    /// Combines the stdout from each run command into a single string.
+    pub fn stdout(&self) -> String {
+        let mut out = String::new();
+        for result in &self.command_results {
+            out.push_str(&result.stdout);
+        }
+        out
+    }
+
+    /// Combines the stderr from each run command into a single string.
+    pub fn stderr(&self) -> String {
+        let mut out = String::new();
+        for result in &self.command_results {
+            out.push_str(&result.stderr);
+        }
+        out
+    }
+
+    /// Combines the stdout AND stderr from each run command into a single string.
+    pub fn std_all(&self) -> String {
+        let mut out = String::new();
+        for result in &self.command_results {
+            out.push_str(&result.stdout);
+            out.push_str(&result.stderr);
+        }
+        out
+    }
+
+    /// Returns the stdout from the final command that was run.
+    pub fn last_stdout(&self) -> String {
+        self.command_results
+            .last()
+            .map(|r| r.stdout.clone())
+            .unwrap_or_default()
+    }
+
+    /// Returns the stderr from the final command that was run.
+    pub fn last_stderr(&self) -> String {
+        self.command_results
+            .last()
+            .map(|r| r.stderr.clone())
+            .unwrap_or_default()
+    }
+
+    /// Returns the stdout AND stderr from the final command that was run.
+    pub fn last_std_all(&self) -> String {
+        let mut out = String::new();
+        out.push_str(&self.last_stdout());
+        out.push_str(&self.last_stderr());
+        out
+    }
+
+    /// Pretty format the attempted commands, with the exit code included on the final line.
+    pub fn fmt_attempted_commands(&self) -> String {
+        if !self.command_results.is_empty() {
+            let mut out = "Attempted commands:\n".to_string();
+            for (index, result) in self.command_results.iter().enumerate() {
+                // Indent the commands by a bit of whitespace:
+                out.push_str("   ");
+                // Add cmd number:
+                out.push_str(format!("{}. ", index).as_str());
+                out.push_str(result.command.trim());
+                // Newline if not last:
+                if index < self.command_results.len() - 1 {
+                    out.push('\n');
+                }
+            }
+            // On the last line, add <-- exited with code: X
+            out.push_str(&format!(" <-- exited with code: {}", self.code()));
+            out
+        } else {
+            "No commands run!".to_string()
+        }
+    }
+
+    /// Throw an error if the last command run was not successful.
+    pub fn throw_on_bad_code<T: error_stack::Context>(&self, err_variant: T) -> Result<(), T> {
+        if self.success() {
+            Ok(())
+        } else {
+            Err(err!(
+                err_variant,
+                "Cli var command returned a non zero exit code: {}. Std output: {}",
+                self.code(),
+                self.std_all()
+            )
+            .attach_printable(self.fmt_attempted_commands()))
+        }
+    }
+}
+
+/// Private interface
+impl BashOut {
+    pub(crate) fn new(command_results: Vec<CmdResult>) -> Self {
+        Self {
+            command_results,
+            code_override: None,
+        }
+    }
+
+    /// Create a new BashOut.
+    pub(crate) fn empty() -> Self {
+        Self {
+            command_results: Vec::new(),
+            code_override: None,
+        }
+    }
+
+    pub(crate) fn override_code(&mut self, code: i32) {
+        self.code_override = Some(code);
+    }
+}

rust/bitbazaar/cli/builtins/cd.rs

@@ -4,12 +4,12 @@ use normpath::PathExt;
 
 use super::bad_call;
 use crate::{
-    cli::{errs::BuiltinErr, shell::Shell, CmdOut},
+    cli::{errs::BuiltinErr, shell::Shell, BashOut, CmdResult},
     prelude::*,
 };
 
 /// https://www.gnu.org/software/bash/manual/bash.html#index-cd
-pub fn cd(shell: &mut Shell, args: &[String]) -> Result<CmdOut, BuiltinErr> {
+pub fn cd(shell: &mut Shell, args: &[String]) -> Result<BashOut, BuiltinErr> {
     macro_rules! hd {
         () => {
             if let Ok(hd) = shell.home_dir() {
@@ -82,7 +82,7 @@ pub fn cd(shell: &mut Shell, args: &[String]) -> Result<CmdOut, BuiltinErr> {
         .chdir(target_path)
         .change_context(BuiltinErr::InternalError)?;
 
-    Ok(CmdOut::empty())
+    Ok(BashOut::empty())
 }
 
 // Should be tested quite well in cli/mod.rs and other builtin tests.

rust/bitbazaar/cli/builtins/echo.rs

@@ -1,10 +1,10 @@
 use crate::{
-    cli::{errs::BuiltinErr, shell::Shell, CmdOut},
+    cli::{errs::BuiltinErr, shell::Shell, BashOut, CmdResult},
     prelude::*,
 };
 
 /// https://www.gnu.org/software/bash/manual/bash.html#index-echo
-pub fn echo(_shell: &mut Shell, args: &[String]) -> Result<CmdOut, BuiltinErr> {
+pub fn echo(_shell: &mut Shell, args: &[String]) -> Result<BashOut, BuiltinErr> {
     let mut newline = true;
 
     let mut stdout = String::new();
@@ -34,12 +34,7 @@ pub fn echo(_shell: &mut Shell, args: &[String]) -> Result<CmdOut, BuiltinErr> {
         stdout.push('\n');
     }
 
-    Ok(CmdOut {
-        stdout,
-        stderr: "".to_string(),
-        code: 0,
-        attempted_commands: vec![], // This is a top level attribute, in theory should have a different struct for internal.
-    })
+    Ok(CmdResult::new("", 0, stdout, "").into())
 }
 
 #[cfg(test)]

rust/bitbazaar/cli/builtins/exit.rs

@@ -1,18 +1,18 @@
 use super::bad_call;
 use crate::{
-    cli::{errs::BuiltinErr, shell::Shell, CmdOut},
+    cli::{errs::BuiltinErr, shell::Shell, BashOut, CmdResult},
     prelude::*,
 };
 
 /// https://www.gnu.org/software/bash/manual/bash.html#index-exit
 ///
-/// Exit the shell, returning a status of n to the shell’s parent.
+/// Exit the shell, returning a status of n to the shell's parent.
 /// If n is omitted, the exit status is that of the last command executed.
 /// Any trap on EXIT is executed before the shell terminates.
-pub fn exit(shell: &mut Shell, args: &[String]) -> Result<CmdOut, BuiltinErr> {
+pub fn exit(shell: &mut Shell, args: &[String]) -> Result<BashOut, BuiltinErr> {
     let exit_code = if args.is_empty() {
         // Last code
-        shell.code
+        shell.code()
     } else if let Some(code_str) = args.first() {
         match code_str.parse::<i32>() {
             Ok(code) => code,
@@ -23,7 +23,7 @@ pub fn exit(shell: &mut Shell, args: &[String]) -> Result<CmdOut, BuiltinErr> {
     };
 
     // Set the code and propagate upwards:
-    shell.code = exit_code;
+    shell.set_code(exit_code);
     Err(err!(BuiltinErr::Exit))
 }
 

rust/bitbazaar/cli/builtins/mod.rs

@@ -8,24 +8,19 @@ use std::collections::HashMap;
 
 use once_cell::sync::Lazy;
 
-use super::{errs::BuiltinErr, shell::Shell, CmdOut};
+use super::{errs::BuiltinErr, shell::Shell, BashOut};
 use crate::prelude::*;
 
 /// A builtin function, implemented internally, might implement for reasons:
 /// - Performance
 /// - Needs to implement the pseudo rust shell
 /// - Windows compatibility - all implement builtins conform to linux/mac/bash expected usage.
-pub type Builtin = fn(&mut Shell, &[String]) -> Result<CmdOut, BuiltinErr>;
+pub type Builtin = fn(&mut Shell, &[String]) -> Result<BashOut, BuiltinErr>;
 
-/// Helper for creating CmdOut with an error code and writing to stderr.
+/// Helper for creating BashOut with an error code and writing to stderr.
 macro_rules! bad_call {
     ($($arg:tt)*) => {
-        return Ok(CmdOut {
-            stdout: "".to_string(),
-            stderr: format!($($arg)*),
-            code: 1,
-            attempted_commands: vec![], // This is a top level attribute, in theory should have a different struct for internal.
-        })
+        return Ok(CmdResult::new("", 1, "", format!($($arg)*)).into())
     };
 }
 
@@ -46,11 +41,8 @@ pub static BUILTINS: Lazy<HashMap<&'static str, Builtin>> = Lazy::new(|| {
 });
 
 #[cfg(test)]
-fn std_err_echo(_shell: &mut Shell, args: &[String]) -> Result<CmdOut, BuiltinErr> {
-    Ok(CmdOut {
-        stdout: "".to_string(),
-        stderr: args.join(" "),
-        code: 0,
-        attempted_commands: vec![], // This is a top level attribute, in theory should have a different struct for internal.
-    })
+fn std_err_echo(_shell: &mut Shell, args: &[String]) -> Result<BashOut, BuiltinErr> {
+    use super::CmdResult;
+
+    Ok(CmdResult::new("", 0, "", args.join(" ")).into())
 }