From b9105757e26338fe2f5cd3b8aee710391379288f Mon Sep 17 00:00:00 2001
From: "Gregory N. Schmit" <schmitgreg@gmail.com>
Date: Tue, 14 Feb 2023 21:45:30 -0600
Subject: [PATCH] Show line when logging warnings/errors and add reg test.
 Fixes #2.

---
 src/context.rs                                | 38 +++++++++++++++
 src/error.rs                                  | 48 +++++++++++--------
 tests/scenarios/simple_makefiles/mod.rs       |  1 +
 .../t3_invalid_line_type/Makefile             |  2 +
 .../t3_invalid_line_type/mod.rs               |  8 ++++
 5 files changed, 77 insertions(+), 20 deletions(-)
 create mode 100644 tests/scenarios/simple_makefiles/t3_invalid_line_type/Makefile
 create mode 100644 tests/scenarios/simple_makefiles/t3_invalid_line_type/mod.rs

diff --git a/src/context.rs b/src/context.rs
index 23d483c..5b2876b 100644
--- a/src/context.rs
+++ b/src/context.rs
@@ -27,6 +27,44 @@ impl Context {
             line: None,
         }
     }
+
+    pub fn label(&self) -> Option<String> {
+        self.path.as_ref().map(|path| {
+            if self.line_number == 0 {
+                if self.column_number == 0 {
+                    format!("{}:{}", path.display(), self.line_number)
+                } else {
+                    format!(
+                        "{}:{}:{}",
+                        path.display(),
+                        self.line_number,
+                        self.column_number
+                    )
+                }
+            } else {
+                path.display().to_string()
+            }
+        })
+    }
+
+    pub fn display_line(&self) -> Option<String> {
+        self.line.as_ref().map(|line| {
+            let line_number_s = if self.line_number == 0 {
+                String::new()
+            } else {
+                self.line_number.to_string()
+            };
+            let pad = " ".repeat(line_number_s.len());
+            let caret = String::new();
+            format!(
+                "{pad} |\n{line_number} | {line}\n{pad} | {caret}\n",
+                pad = pad,
+                line_number = line_number_s,
+                line = line,
+                caret = caret,
+            )
+        })
+    }
 }
 
 impl Default for Context {
diff --git a/src/error.rs b/src/error.rs
index ff11b51..3a91306 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -3,43 +3,51 @@ use std::fmt;
 
 use super::context::Context;
 
+const INFO: &str = "INFO";
+const WARN: &str = "WARN";
+const ERROR: &str = "ERROR";
+const MAX_SEVERITY_LENGTH: usize = 5;
+
 /// Formatter for all log messages.
 fn format_log(msg: impl AsRef<str>, level: &str, context: Option<&Context>) -> String {
-    // Format log level.
-    let level_display = format!("{:5}", level);
+    // Format log level and context label/line.
+    let level_display = format!("{:0width$}", level, width = MAX_SEVERITY_LENGTH);
+    let context_label = context
+        .and_then(|c| c.label())
+        .map(|l| format!("[{}] ", l))
+        .unwrap_or_default();
 
-    // Format context.
-    let context_display = match context {
-        None => String::new(),
-        Some(context) => match &context.path {
-            None => String::new(),
-            Some(path) => {
-                if context.line_number == 0 {
-                    format!("[{}:{}] ", path.display(), context.line_number)
-                } else {
-                    format!("[{}] ", path.display())
-                }
-            }
-        },
+    // Only show the context line if we are logging warnings or errors.
+    let context_line = if level == "WARN" || level == "ERROR" {
+        context
+            .and_then(|c| c.display_line())
+            .map(|l| format!("\n{}", l))
+            .unwrap_or_default()
+    } else {
+        String::new()
     };
 
-    // Print the log message.
-    format!("make: {level_display} {context_display}| {}", msg.as_ref())
+    // Return the formatted message.
+    format!(
+        "make: {level_display} {context_label}| {}{}",
+        msg.as_ref(),
+        context_line
+    )
 }
 
 /// Helper to format info.
 fn format_info(msg: impl AsRef<str>, context: Option<&Context>) -> String {
-    format_log(msg, "INFO", context)
+    format_log(msg, INFO, context)
 }
 
 /// Helper to format warnings.
 fn format_warn(msg: impl AsRef<str>, context: Option<&Context>) -> String {
-    format_log(msg, "WARN", context)
+    format_log(msg, WARN, context)
 }
 
 /// Helper to format errors.
 fn format_err(msg: impl AsRef<str>, context: Option<&Context>) -> String {
-    format_log(msg, "ERROR", context)
+    format_log(msg, ERROR, context)
 }
 
 /// Helper to log info to STDERR.
diff --git a/tests/scenarios/simple_makefiles/mod.rs b/tests/scenarios/simple_makefiles/mod.rs
index a59b816..1cb1559 100644
--- a/tests/scenarios/simple_makefiles/mod.rs
+++ b/tests/scenarios/simple_makefiles/mod.rs
@@ -1,2 +1,3 @@
 mod t1_simple;
 mod t2_prereq;
+mod t3_invalid_line_type;
diff --git a/tests/scenarios/simple_makefiles/t3_invalid_line_type/Makefile b/tests/scenarios/simple_makefiles/t3_invalid_line_type/Makefile
new file mode 100644
index 0000000..8adeb11
--- /dev/null
+++ b/tests/scenarios/simple_makefiles/t3_invalid_line_type/Makefile
@@ -0,0 +1,2 @@
+a:
+  echo "bad indentation on this line is intentional" > a
\ No newline at end of file
diff --git a/tests/scenarios/simple_makefiles/t3_invalid_line_type/mod.rs b/tests/scenarios/simple_makefiles/t3_invalid_line_type/mod.rs
new file mode 100644
index 0000000..91218ed
--- /dev/null
+++ b/tests/scenarios/simple_makefiles/t3_invalid_line_type/mod.rs
@@ -0,0 +1,8 @@
+crate::system_test_cases!(
+    {
+        args: &[],
+        expected_stdout: "",
+        expected_stderr: "make: ERROR [Makefile] | Invalid line type.\n  |\n2 |   echo \"bad indentation on this line is intentional\" > a\n  | \n\n",
+        expected_files: &[],
+    },
+);