From 3689eb17d244fa630c08b05e0d4b76e4b57c9585 Mon Sep 17 00:00:00 2001
From: Filip Tibell <filip.tibell@gmail.com>
Date: Sat, 21 Jan 2023 17:02:49 -0500
Subject: [PATCH] More output formatting improvements

---
 CHANGELOG.md                |  5 +++++
 src/cli/cli.rs              |  5 ++++-
 src/lib/globals/task.rs     |  3 ++-
 src/lib/lib.rs              |  2 +-
 src/lib/utils/formatting.rs | 41 +++++++++++++++++++++++++++++++------
 5 files changed, 47 insertions(+), 9 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8ab52b0..b9dd527 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -14,9 +14,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+- Improved general formatting of errors to make them more readable & glanceable
 - Improved output formatting of non-primitive types
 - Improved output formatting of empty tables
 
+### Fixed
+
+- Fixed double stack trace for certain kinds of errors
+
 ## `0.0.4` - January 21st, 2023
 
 ### Added
diff --git a/src/cli/cli.rs b/src/cli/cli.rs
index 4195b52..18b5a8f 100644
--- a/src/cli/cli.rs
+++ b/src/cli/cli.rs
@@ -96,7 +96,10 @@ impl Cli {
         // Display the file path relative to cwd with no extensions in stack traces
         let file_display_name = file_path.with_extension("").display().to_string();
         // Create a new lune object with all globals & run the script
-        run_lune(&file_display_name, &file_contents, self.script_args).await?;
+        if let Err(e) = run_lune(&file_display_name, &file_contents, self.script_args).await {
+            eprintln!("{e}");
+            std::process::exit(1);
+        };
         Ok(())
     }
 }
diff --git a/src/lib/globals/task.rs b/src/lib/globals/task.rs
index 801af47..abd9792 100644
--- a/src/lib/globals/task.rs
+++ b/src/lib/globals/task.rs
@@ -10,6 +10,7 @@ use crate::utils::table_builder::ReadonlyTableBuilder;
 
 const DEFAULT_SLEEP_DURATION: f32 = 1.0 / 60.0;
 
+#[allow(dead_code)]
 pub struct WaitingThread<'a> {
     is_delayed_for: Option<f32>,
     is_deferred: Option<bool>,
@@ -17,7 +18,7 @@ pub struct WaitingThread<'a> {
     args: Variadic<Value<'a>>,
 }
 
-pub fn new<'a>(lua: &'a Lua, threads: &Arc<Mutex<Vec<WaitingThread<'a>>>>) -> Result<Table<'a>> {
+pub fn new<'a>(lua: &'a Lua, _threads: &Arc<Mutex<Vec<WaitingThread<'a>>>>) -> Result<Table<'a>> {
     // TODO: Figure out how to insert into threads vec
     ReadonlyTableBuilder::new(lua)?
         .with_async_function(
diff --git a/src/lib/lib.rs b/src/lib/lib.rs
index d5af275..cb139b6 100644
--- a/src/lib/lib.rs
+++ b/src/lib/lib.rs
@@ -57,7 +57,7 @@ mod tests {
                         .await
                         .unwrap();
                     if let Err(e) = run_lune($value, &script, args).await {
-                        panic!("Test '{}' failed!\n{}", $value, e.to_string())
+                        panic!("\nTest '{}' failed!\n{}\n", $value, e.to_string())
                     }
                 }
             )*
diff --git a/src/lib/utils/formatting.rs b/src/lib/utils/formatting.rs
index 08f85e5..89d763f 100644
--- a/src/lib/utils/formatting.rs
+++ b/src/lib/utils/formatting.rs
@@ -191,24 +191,34 @@ pub fn pretty_format_multi_value(multi: &MultiValue) -> mlua::Result<String> {
 }
 
 pub fn pretty_format_luau_error(e: &mlua::Error) -> String {
-    match e {
+    let stack_begin = format!("[{}Stack Begin{}]", COLOR_BLUE, COLOR_RESET);
+    let stack_end = format!("[{}Stack End{}]", COLOR_BLUE, COLOR_RESET);
+    let err_string = match e {
         mlua::Error::RuntimeError(e) => {
+            // Add "Stack Begin" instead of default stack traceback string
             let err_string = e.to_string();
-            let mut err_lines = err_string.lines().collect::<Vec<_>>();
+            let mut err_lines = err_string
+                .lines()
+                .map(|s| s.to_string())
+                .collect::<Vec<String>>();
             for (index, line) in err_lines.clone().iter().enumerate().rev() {
                 if *line == "stack traceback:" {
-                    err_lines[index] = "Stack Begin";
+                    err_lines[index] = stack_begin;
                     break;
                 }
             }
-            err_lines.push("Stack End");
+            // Add "Stack End" to the very end of the stack trace for symmetry
+            err_lines.push(stack_end);
             err_lines.join("\n")
         }
         mlua::Error::CallbackError { cause, traceback } => {
+            // Same error formatting as above
             format!(
-                "{}\nStack Begin{}Stack End",
+                "{}\n{}{}{}",
                 pretty_format_luau_error(cause.as_ref()),
-                traceback.strip_prefix("stack traceback:\n").unwrap()
+                stack_begin,
+                traceback.strip_prefix("stack traceback:\n").unwrap(),
+                stack_end
             )
         }
         mlua::Error::ToLuaConversionError { from, to, message } => {
@@ -230,5 +240,24 @@ pub fn pretty_format_luau_error(e: &mlua::Error) -> String {
             )
         }
         e => format!("{e}"),
+    };
+    let mut err_lines = err_string.lines().collect::<Vec<_>>();
+    // Remove the script path from the error message
+    // itself, it can be found in the stack trace
+    if let Some(first_line) = err_lines.first() {
+        if first_line.starts_with("[string \"") {
+            if let Some(closing_bracket) = first_line.find("]:") {
+                let after_closing_bracket = &first_line[closing_bracket + 2..first_line.len()];
+                if let Some(last_colon) = after_closing_bracket.find(": ") {
+                    err_lines[0] = &after_closing_bracket
+                        [last_colon + 2..first_line.len() - closing_bracket - 2];
+                } else {
+                    err_lines[0] = after_closing_bracket
+                }
+            }
+        }
     }
+    // Reformat stack trace lines, ignore lines that just mention C functions
+    // Merge all lines back together into one string
+    err_lines.join("\n")
 }