ttiimm · ttiimm · Sep 29, 2025 · Sep 26, 2025
diff --git a/src/lib.rs b/src/lib.rs
@@ -25,7 +25,7 @@ use crate::progress::WorkGuard;
 use crate::source_hier::{ScanEvent, SourceFileID, SourceHierContent, SourceHierTree};
 use crate::source_ref::FormatArgument;
 pub use code_source::CodeSource;
-use log_format::LogFormat;
+pub use log_format::LogFormat;
 pub use progress::ProgressTracker;
 pub use progress::ProgressUpdate;
 pub use progress::WorkInfo;
@@ -35,6 +35,16 @@ pub use source_ref::SourceRef;
 
 #[derive(Error, Debug, Diagnostic, Clone)]
 pub enum LogError {
+    #[error("invalid log format regular expression")]
+    InvalidFormatRegex { source: regex::Error },
+    #[error("unknown capture in log format: {name}")]
+    #[diagnostic(help(
+        "The supported captures are: timestamp, thread, level, file, line, method, and body"
+    ))]
+    UnknownFormatCapture { name: String },
+    #[error("log format is missing capture: {name}")]
+    #[diagnostic(help("A log format must have a 'body' capture at a minimum"))]
+    FormatMissingCapture { name: String },
     #[error("\"{path}\" is already covered by \"{root}\"")]
     PathExists { path: PathBuf, root: PathBuf },
     #[error("cannot read source file \"{path}\"")]
@@ -421,23 +431,29 @@ pub struct VariablePair {
 
 #[derive(Serialize)]
 pub struct LogMapping<'a> {
-    #[serde(skip_serializing)]
+    #[serde(rename(serialize = "logRef"))]
     pub log_ref: LogRef<'a>,
     #[serde(rename(serialize = "srcRef"))]
     pub src_ref: Option<SourceRef>,
     pub variables: Vec<VariablePair>,
 }
 
-#[derive(Copy, Clone, Debug, PartialEq)]
+#[derive(Copy, Clone, Debug, PartialEq, Serialize)]
 pub struct LogRef<'a> {
+    #[serde(skip_serializing)]
     pub line: &'a str,
     pub details: Option<LogDetails<'a>>,
 }
 
-#[derive(Copy, Clone, Debug, PartialEq)]
+#[derive(Copy, Clone, Debug, PartialEq, Serialize)]
 pub struct LogDetails<'a> {
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub thread: Option<&'a str>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub file: Option<&'a str>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub lineno: Option<u32>,
+    #[serde(skip_serializing)]
     pub body: Option<&'a str>,
 }
 
@@ -449,28 +465,17 @@ impl<'a> LogRef<'a> {
         }
     }
 
-    pub fn from_parsed(file: Option<&'a str>, lineno: Option<u32>, body: &'a str) -> Self {
-        let details = Some(LogDetails {
-            file,
-            lineno,
-            body: Some(body),
-        });
-        Self {
-            line: body,
-            details,
-        }
-    }
-
     pub fn with_format(line: &'a str, log_format: LogFormat) -> Self {
         let captures = log_format.captures(line);
+        let thread = captures.name("thread").map(|thread_match| thread_match.as_str());
         let file = captures.name("file").map(|file_match| file_match.as_str());
         let lineno = captures
             .name("line")
             .and_then(|lineno| lineno.as_str().parse::<u32>().ok());
         let body = captures.name("body").map(|body| body.as_str());
         Self {
             line,
-            details: Some(LogDetails { file, lineno, body }),
+            details: Some(LogDetails { thread, file, lineno, body }),
         }
     }
 
@@ -542,11 +547,10 @@ pub fn extract_variables<'a>(log_ref: &LogRef<'a>, src_ref: &'a SourceRef) -> Ve
     variables
 }
 
-pub fn filter_log<R>(buffer: &str, filter: R, log_format: Option<String>) -> Vec<LogRef<'_>>
+pub fn filter_log<R>(buffer: &str, filter: R, log_format: Option<LogFormat>) -> Vec<LogRef<'_>>
 where
     R: RangeBounds<usize>,
 {
-    let log_format = log_format.map(LogFormat::new);
     buffer
         .lines()
         .enumerate()
@@ -659,12 +663,10 @@ mod tests {
         let buffer = String::from(
             "2025-04-10 22:12:52 INFO  JvmPauseMonitor:146 - JvmPauseMonitor-n0: Started",
         );
-        let regex = String::from(
-            r"^(?<timestamp>\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}) (?<level>\w+)\s+ (?<file>[\w$.]+):(?<line>\d+) - (?<body>.*)$",
-        );
-        let log_format = Some(regex);
-        let result = filter_log(&buffer, .., log_format);
+        let regex = r"^(?<timestamp>\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}) (?<level>\w+)\s+ (?<file>[\w$.]+):(?<line>\d+) - (?<body>.*)$";
+        let result = filter_log(&buffer, .., Some(regex.try_into().unwrap()));
         let details = Some(LogDetails {
+            thread: None,
             file: Some("JvmPauseMonitor"),
             lineno: Some(146),
             body: Some("JvmPauseMonitor-n0: Started"),
@@ -724,9 +726,9 @@ fn namedarg2(salutation: &str, name: &str) {
 
     #[test]
     fn test_link_to_source() {
-        let lf = LogFormat::new(
-            r#"^\[\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z \w+ \w+\]\s+(?<body>.*)"#.to_string(),
-        );
+        let lf = r#"^\[\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z \w+ \w+\]\s+(?<body>.*)"#
+            .try_into()
+            .unwrap();
         let log_ref = LogRef::with_format(
             "[2024-05-09T19:58:53Z DEBUG main] you're only as funky as your last cut",
             lf,
@@ -743,9 +745,9 @@ fn namedarg2(salutation: &str, name: &str) {
 
     #[test]
     fn test_link_to_quality_source() {
-        let lf = LogFormat::new(
-            r#"^\[\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z \w+ \w+\]\s+(?<body>.*)"#.to_string(),
-        );
+        let lf = r#"^\[\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z \w+ \w+\]\s+(?<body>.*)"#
+            .try_into()
+            .unwrap();
         let log_ref = LogRef::with_format("[2024-05-09T19:58:53Z DEBUG main] Hello, Leander!", lf);
         let code = CodeSource::from_string(&Path::new("in-mem.rs"), TEST_SOURCE);
         let src_refs = extract_logging(&[code], &ProgressTracker::new())
@@ -768,9 +770,9 @@ fn main() {
 "#;
     #[test]
     fn test_link_multiline() {
-        let lf = LogFormat::new(
-            r#"^\[\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z \w+ \w+\]\s+(?<body>.*)"#.to_string(),
-        );
+        let lf = r#"^\[\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z \w+ \w+\]\s+(?<body>.*)"#
+            .try_into()
+            .unwrap();
         let log_ref = LogRef::with_format(
             "[2024-05-09T19:58:53Z DEBUG main] you're only as funky\n as your last cut",
             lf,
@@ -864,13 +866,11 @@ fn main() {
 """#;
     #[test]
     fn test_extract_var_punctuation() {
-        let regex = String::from(
-            r"^(?<timestamp>\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}) (?<level>\w+)\s+ (?<file>[\w$.]+):(?<line>\d+) - (?<body>.*)$",
-        );
-        let log_format = LogFormat::new(regex);
+        let lf =
+            r"^(?<timestamp>\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}) (?<level>\w+)\s+ (?<file>[\w$.]+):(?<line>\d+) - (?<body>.*)$".try_into().unwrap();
         let log_ref = LogRef::with_format(
             "2025-04-10 22:12:52 INFO  JvmPauseMonitor:146 - JvmPauseMonitor-n0: Started",
-            log_format,
+            lf,
         );
         let code = CodeSource::from_string(&PathBuf::from("in-mem.java"), TEST_PUNC_SRC);
         let src_refs = extract_logging(&[code], &ProgressTracker::new())

diff --git a/src/log_format.rs b/src/log_format.rs
@@ -1,26 +1,13 @@
 use regex::{Captures, Regex, RegexBuilder};
 
-use crate::LogRef;
+use crate::{LogError, LogRef};
 
 #[derive(Clone, Debug)]
 pub struct LogFormat {
     regex: Regex,
 }
 
 impl LogFormat {
-    pub fn new(format: String) -> LogFormat {
-        LogFormat {
-            // TODO handle more gracefully if wrong format
-            regex: RegexBuilder::new(&format)
-                // XXX: This is kinda a hack to support multiline matching in lnav, but
-                // not really useful for log2src atm because its still filtering line-by-line,
-                // so this case would never come up
-                .dot_matches_new_line(true)
-                .build()
-                .unwrap(),
-        }
-    }
-
     pub fn has_src_hint(self: LogFormat) -> bool {
         let mut flatten = self.regex.capture_names().flatten();
         flatten.any(|name| name == "line") && flatten.any(|name| name == "file")
@@ -44,6 +31,82 @@ impl LogFormat {
     }
 }
 
-// TODO finish these tests
-#[test]
-fn test_has_line_support() {}
+impl TryFrom<&str> for LogFormat {
+    type Error = LogError;
+
+    fn try_from(value: &str) -> Result<Self, Self::Error> {
+        fn check_captures(regex: &Regex) -> Result<(), LogError> {
+            let mut seen = Vec::new();
+            for name in regex.capture_names().filter_map(|x| x) {
+                match name {
+                    "timestamp" | "thread" | "method" | "file" | "line" | "body" | "level" => {
+                        seen.push(name)
+                    }
+                    _ => {
+                        return Err(LogError::UnknownFormatCapture {
+                            name: name.to_string(),
+                        })
+                    }
+                }
+            }
+            if !seen.contains(&"body") {
+                return Err(LogError::FormatMissingCapture {
+                    name: "body".to_string(),
+                });
+            }
+            Ok(())
+        }
+
+        RegexBuilder::new(&value)
+            // XXX: This is kinda a hack to support multiline matching in lnav, but
+            // not really useful for log2src atm because its still filtering line-by-line,
+            // so this case would never come up
+            .dot_matches_new_line(true)
+            .build()
+            .map_err(|source| LogError::InvalidFormatRegex { source })
+            .and_then(|regex| {
+                check_captures(&regex)?;
+                Ok(LogFormat { regex })
+            })
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use crate::LogFormat;
+    use insta::assert_snapshot;
+    use miette::{IntoDiagnostic, NarratableReportHandler, Report};
+
+    fn get_pretty_report_string(error: Report) -> String {
+        let mut buffer = String::new();
+        let handler = NarratableReportHandler::new();
+        let handler = if let Some(help) = error.help() {
+            handler.with_footer(help.to_string())
+        } else {
+            handler
+        };
+        let _ = handler.render_report(&mut buffer, error.as_ref());
+        buffer
+    }
+
+    #[test]
+    fn test_invalid_regex() {
+        let res = Report::from(LogFormat::try_from("abc(").unwrap_err());
+        let rep = get_pretty_report_string(res);
+        assert_snapshot!(rep);
+    }
+
+    #[test]
+    fn test_no_body() {
+        let res = LogFormat::try_from("abc").into_diagnostic();
+        let rep = get_pretty_report_string(res.unwrap_err());
+        assert_snapshot!(rep);
+    }
+
+    #[test]
+    fn test_unknown_cap() {
+        let res = LogFormat::try_from("abc(?<extra>def)").into_diagnostic();
+        let rep = get_pretty_report_string(res.unwrap_err());
+        assert_snapshot!(rep);
+    }
+}
diff --git a/src/main.rs b/src/main.rs
@@ -77,6 +77,12 @@ fn main() -> miette::Result<()> {
         });
     }
 
+    let format_re = if let Some(format) = args.format {
+        Some(format.as_str().try_into()?)
+    } else {
+        None
+    };
+
     let input = args.log;
     let mut reader: Box<dyn io::Read> = match input {
         None => Box::new(io::stdin()),
@@ -87,7 +93,7 @@ fn main() -> miette::Result<()> {
     reader.read_to_string(&mut buffer).into_diagnostic()?;
     let filter = args.start.unwrap_or(0)..args.end.unwrap_or(usize::MAX);
 
-    let filtered = filter_log(&buffer, filter, args.format.clone());
+    let filtered = filter_log(&buffer, filter, format_re);
     let mut log_matcher = LogMatcher::new();
     log_matcher
         .add_root(&PathBuf::from(args.sources))

diff --git a/src/snapshots/log2src__log_format__tests__invalid_regex.snap b/src/snapshots/log2src__log_format__tests__invalid_regex.snap
@@ -0,0 +1,10 @@
+---
+source: src/log_format.rs
+expression: rep
+---
+invalid log format regular expression
+    Diagnostic severity: error
+    Caused by: regex parse error:
+    abc(
+       ^
+error: unclosed group
diff --git a/src/snapshots/log2src__log_format__tests__no_body.snap b/src/snapshots/log2src__log_format__tests__no_body.snap
@@ -0,0 +1,6 @@
+---
+source: src/log_format.rs
+expression: rep
+---
+log format is missing capture: body
+    Diagnostic severity: error
diff --git a/src/snapshots/log2src__log_format__tests__unknown_cap.snap b/src/snapshots/log2src__log_format__tests__unknown_cap.snap
@@ -0,0 +1,6 @@
+---
+source: src/log_format.rs
+expression: rep
+---
+unknown capture in log format: extra
+    Diagnostic severity: error
diff --git a/tests/resources/java/basic-slf4j.log b/tests/resources/java/basic-slf4j.log
@@ -1,2 +1,2 @@
-2024-05-08 14:46:47 Application starting
-2024-05-08 14:46:47 Debug message: args length = 0
+2024-05-08 14:46:47 123 Application starting
+2024-05-08 14:46:47 123 Debug message: args length = 0
diff --git a/tests/snapshots/test_java__basic.snap b/tests/snapshots/test_java__basic.snap
@@ -13,9 +13,9 @@ info:
 success: true
 exit_code: 0
 ----- stdout -----
-{"srcRef":{"sourcePath":"{java_dir}/Basic.java","language":"Java","lineNumber":18,"endLineNumber":18,"column":16,"name":"main","text":"\"Hello from main\"","quality":13,"pattern":"(?s)^Hello from main$","args":[],"vars":[]},"variables":[]}
-{"srcRef":{"sourcePath":"{java_dir}/Basic.java","language":"Java","lineNumber":25,"endLineNumber":25,"column":20,"name":"foo","text":"\"Hello from foo i=\\{i}\"","quality":14,"pattern":"(?s)^Hello from foo i=(.+)$","args":[{"Named":"i"}],"vars":[]},"variables":[{"expr":"i","value":"0"}]}
-{"srcRef":{"sourcePath":"{java_dir}/Basic.java","language":"Java","lineNumber":25,"endLineNumber":25,"column":20,"name":"foo","text":"\"Hello from foo i=\\{i}\"","quality":14,"pattern":"(?s)^Hello from foo i=(.+)$","args":[{"Named":"i"}],"vars":[]},"variables":[{"expr":"i","value":"1"}]}
-{"srcRef":{"sourcePath":"{java_dir}/Basic.java","language":"Java","lineNumber":25,"endLineNumber":25,"column":20,"name":"foo","text":"\"Hello from foo i=\\{i}\"","quality":14,"pattern":"(?s)^Hello from foo i=(.+)$","args":[{"Named":"i"}],"vars":[]},"variables":[{"expr":"i","value":"2"}]}
+{"logRef":{"details":{}},"srcRef":{"sourcePath":"{java_dir}/Basic.java","language":"Java","lineNumber":18,"endLineNumber":18,"column":16,"name":"main","text":"\"Hello from main\"","quality":13,"pattern":"(?s)^Hello from main$","args":[],"vars":[]},"variables":[]}
+{"logRef":{"details":{}},"srcRef":{"sourcePath":"{java_dir}/Basic.java","language":"Java","lineNumber":25,"endLineNumber":25,"column":20,"name":"foo","text":"\"Hello from foo i=\\{i}\"","quality":14,"pattern":"(?s)^Hello from foo i=(.+)$","args":[{"Named":"i"}],"vars":[]},"variables":[{"expr":"i","value":"0"}]}
+{"logRef":{"details":{}},"srcRef":{"sourcePath":"{java_dir}/Basic.java","language":"Java","lineNumber":25,"endLineNumber":25,"column":20,"name":"foo","text":"\"Hello from foo i=\\{i}\"","quality":14,"pattern":"(?s)^Hello from foo i=(.+)$","args":[{"Named":"i"}],"vars":[]},"variables":[{"expr":"i","value":"1"}]}
+{"logRef":{"details":{}},"srcRef":{"sourcePath":"{java_dir}/Basic.java","language":"Java","lineNumber":25,"endLineNumber":25,"column":20,"name":"foo","text":"\"Hello from foo i=\\{i}\"","quality":14,"pattern":"(?s)^Hello from foo i=(.+)$","args":[{"Named":"i"}],"vars":[]},"variables":[{"expr":"i","value":"2"}]}
 
 ----- stderr -----
diff --git a/tests/snapshots/test_java__basic_slf4j.snap b/tests/snapshots/test_java__basic_slf4j.snap
@@ -8,12 +8,12 @@ info:
     - "-l"
     - tests/resources/java/basic-slf4j.log
     - "-f"
-    - "^(?<timestamp>\\d{4}-\\d{2}-\\d{2} \\d{2}:\\d{2}:\\d{2}) (?<body>.*)$"
+    - "^(?<timestamp>\\d{4}-\\d{2}-\\d{2} \\d{2}:\\d{2}:\\d{2}) (?<thread>\\d+) (?<body>.*)$"
 ---
 success: true
 exit_code: 0
 ----- stdout -----
-{"srcRef":{"sourcePath":"{java_dir}/BasicSlf4j.java","language":"Java","lineNumber":10,"endLineNumber":10,"column":20,"name":"main","text":"\"Application starting\"","quality":19,"pattern":"(?s)^Application starting$","args":[],"vars":[]},"variables":[]}
-{"srcRef":{"sourcePath":"{java_dir}/BasicSlf4j.java","language":"Java","lineNumber":12,"endLineNumber":13,"column":21,"name":"main","text":"\"Debug message: args length = {}\"","quality":24,"pattern":"(?s)^Debug message: args length = (.+)$","args":["Placeholder"],"vars":["args.length"]},"variables":[{"expr":"args.length","value":"0"}]}
+{"logRef":{"details":{"thread":"123"}},"srcRef":{"sourcePath":"{java_dir}/BasicSlf4j.java","language":"Java","lineNumber":10,"endLineNumber":10,"column":20,"name":"main","text":"\"Application starting\"","quality":19,"pattern":"(?s)^Application starting$","args":[],"vars":[]},"variables":[]}
+{"logRef":{"details":{"thread":"123"}},"srcRef":{"sourcePath":"{java_dir}/BasicSlf4j.java","language":"Java","lineNumber":12,"endLineNumber":13,"column":21,"name":"main","text":"\"Debug message: args length = {}\"","quality":24,"pattern":"(?s)^Debug message: args length = (.+)$","args":["Placeholder"],"vars":["args.length"]},"variables":[{"expr":"args.length","value":"0"}]}
 
 ----- stderr -----
diff --git a/tests/snapshots/test_java__basic_with_log.snap b/tests/snapshots/test_java__basic_with_log.snap
@@ -13,9 +13,9 @@ info:
 success: true
 exit_code: 0
 ----- stdout -----
-{"srcRef":{"sourcePath":"{java_dir}/BasicWithLog.java","language":"Java","lineNumber":18,"endLineNumber":18,"column":13,"name":"main","text":"\"Hello from main\"","quality":13,"pattern":"(?s)^Hello from main$","args":[],"vars":[]},"variables":[]}
-{"srcRef":{"sourcePath":"{java_dir}/BasicWithLog.java","language":"Java","lineNumber":25,"endLineNumber":25,"column":17,"name":"foo","text":"\"Hello from foo i=\\{i}\"","quality":14,"pattern":"(?s)^Hello from foo i=(.+)$","args":[{"Named":"i"}],"vars":[]},"variables":[{"expr":"i","value":"0"}]}
-{"srcRef":{"sourcePath":"{java_dir}/BasicWithLog.java","language":"Java","lineNumber":25,"endLineNumber":25,"column":17,"name":"foo","text":"\"Hello from foo i=\\{i}\"","quality":14,"pattern":"(?s)^Hello from foo i=(.+)$","args":[{"Named":"i"}],"vars":[]},"variables":[{"expr":"i","value":"1"}]}
-{"srcRef":{"sourcePath":"{java_dir}/BasicWithLog.java","language":"Java","lineNumber":25,"endLineNumber":25,"column":17,"name":"foo","text":"\"Hello from foo i=\\{i}\"","quality":14,"pattern":"(?s)^Hello from foo i=(.+)$","args":[{"Named":"i"}],"vars":[]},"variables":[{"expr":"i","value":"2"}]}
+{"logRef":{"details":{}},"srcRef":{"sourcePath":"{java_dir}/BasicWithLog.java","language":"Java","lineNumber":18,"endLineNumber":18,"column":13,"name":"main","text":"\"Hello from main\"","quality":13,"pattern":"(?s)^Hello from main$","args":[],"vars":[]},"variables":[]}
+{"logRef":{"details":{}},"srcRef":{"sourcePath":"{java_dir}/BasicWithLog.java","language":"Java","lineNumber":25,"endLineNumber":25,"column":17,"name":"foo","text":"\"Hello from foo i=\\{i}\"","quality":14,"pattern":"(?s)^Hello from foo i=(.+)$","args":[{"Named":"i"}],"vars":[]},"variables":[{"expr":"i","value":"0"}]}
+{"logRef":{"details":{}},"srcRef":{"sourcePath":"{java_dir}/BasicWithLog.java","language":"Java","lineNumber":25,"endLineNumber":25,"column":17,"name":"foo","text":"\"Hello from foo i=\\{i}\"","quality":14,"pattern":"(?s)^Hello from foo i=(.+)$","args":[{"Named":"i"}],"vars":[]},"variables":[{"expr":"i","value":"1"}]}
+{"logRef":{"details":{}},"srcRef":{"sourcePath":"{java_dir}/BasicWithLog.java","language":"Java","lineNumber":25,"endLineNumber":25,"column":17,"name":"foo","text":"\"Hello from foo i=\\{i}\"","quality":14,"pattern":"(?s)^Hello from foo i=(.+)$","args":[{"Named":"i"}],"vars":[]},"variables":[{"expr":"i","value":"2"}]}
 
 ----- stderr -----