src/init.rs

@@ -1,15 +1,12 @@
 //
 use crate::model::Config;
 use crate::patterns::{issue_pattern, marker_pattern};
-use crate::printer::Printer;
 use anyhow::{Context, Result};
-use kxio::fs;
-use kxio::network::Network;
 
-pub fn init_config(printer: &impl Printer, net: Network) -> Result<Config> {
+pub fn init_config(net: kxio::network::Network) -> Result<Config, anyhow::Error> {
     let config = Config::builder()
         .net(net)
-        .fs(fs::new(
+        .fs(kxio::fs::new(
             std::env::var("GITHUB_WORKSPACE")
                 .context("GITHUB_WORKSPACE")?
                 .into(),
@@ -21,9 +18,9 @@ pub fn init_config(printer: &impl Printer, net: Network) -> Result<Config> {
         .maybe_auth_token(std::env::var("REPO_TOKEN").ok())
         .build();
 
-    printer.println(format!("Repo: {}", config.repo()));
-    printer.println(format!("Prefix: {}", config.prefix_pattern()));
-    printer.println(format!("Issues: {}", config.issue_pattern()));
+    println!("Repo: {}", config.repo());
+    println!("Prefix: {}", config.prefix_pattern());
+    println!("Issues: {}", config.issue_pattern());
 
     Ok(config)
 }

src/main.rs

@@ -2,15 +2,12 @@
 use anyhow::{bail, Result};
 use init::init_config;
 use issues::fetch_open_issues;
-use kxio::network::Network;
-use printer::Printer;
 use scanner::{find_markers, DefaultFileScanner};
 
 mod init;
 mod issues;
 mod model;
 mod patterns;
-mod printer;
 mod scanner;
 
 #[cfg(test)]
@@ -19,19 +16,28 @@ mod tests;
 #[tokio::main]
 #[cfg(not(tarpaulin_include))]
 async fn main() -> std::result::Result<(), Box<dyn std::error::Error>> {
-    use printer::StandardPrinter;
-
-    Ok(run(&StandardPrinter, Network::new_real()).await?)
+    Ok(run(kxio::network::Network::new_real()).await?)
 }
 
-async fn run(printer: &impl Printer, net: Network) -> Result<()> {
-    printer.println("Forgejo TODO Checker!");
+async fn run(net: kxio::network::Network) -> Result<()> {
+    println!("Forgejo TODO Checker!");
 
-    let config = init_config(printer, net)?;
+    let config = init_config(net)?;
     let issues = fetch_open_issues(&config).await?;
-    let errors = find_markers(printer, &config, issues, &DefaultFileScanner)?;
+    let markers = find_markers(&config, issues, &DefaultFileScanner)?;
 
-    if errors > 0 {
+    let mut errors = false;
+    for marker in (*markers).iter() {
+        match marker {
+            model::Marker::Closed(_, _) | model::Marker::Invalid(_) => {
+                println!("{marker}");
+                errors = true;
+            }
+            model::Marker::Unmarked | model::Marker::Valid(_, _) => {}
+        }
+    }
+
+    if errors {
         bail!("Invalid or closed TODO/FIXMEs found")
     }
     Ok(())

src/model/config.rs

@@ -1,4 +1,6 @@
 //
+#![allow(dead_code)]
+
 use bon::Builder;
 use regex::Regex;
 

src/model/line.rs

@@ -1,4 +1,6 @@
 //
+#![allow(dead_code)]
+
 use std::path::PathBuf;
 
 use anyhow::{Context, Result};
@@ -13,11 +15,22 @@ use super::Marker;
 
 #[derive(Debug, Builder)]
 pub struct Line {
+    file: PathBuf,
     relative_path: PathBuf,
     num: usize,
     value: String,
 }
 impl Line {
+    // pub fn file(&self) -> &Path {
+    //     &self.file
+    // }
+    // pub fn num(&self) -> usize {
+    //     self.num
+    // }
+    // pub fn value(&self) -> &str {
+    //     &self.value
+    // }
+
     pub fn into_marker(self) -> Result<Marker> {
         if marker_pattern()?.find(&self.value).is_some() {
             match issue_pattern()?.captures(&self.value) {

src/model/markers.rs

@@ -1,29 +0,0 @@
-//
-use crate::{issues::Issue, model::Line};
-
-#[derive(Debug)]
-pub enum Marker {
-    Unmarked,
-    Invalid(Line),
-    Valid(Line, Issue),
-    Closed(Line, Issue),
-}
-impl Marker {
-    pub fn into_closed(self) -> Self {
-        match self {
-            Self::Valid(line, issue) => Self::Closed(line, issue),
-            #[cfg(not(tarpaulin_include))] // only ever called when is a Valid
-            _ => self,
-        }
-    }
-}
-
-impl std::fmt::Display for Marker {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        match self {
-            Self::Invalid(line) => write!(f, "- Invalid: {}", line),
-            Self::Closed(line, issue) => write!(f, "- Closed : ({issue}) {line}"),
-            Self::Valid(_, _) | Self::Unmarked => Ok(()), // tarpaulin uncovered okay
-        }
-    }
-}

src/model/markers/mod.rs

@@ -0,0 +1,62 @@
+//
+
+#[cfg(test)]
+mod tests;
+
+use std::ops::Deref;
+
+use crate::{issues::Issue, model::Line};
+
+#[derive(Debug)]
+pub enum Marker {
+    Unmarked,
+    Invalid(Line),
+    #[allow(dead_code)]
+    Valid(Line, Issue),
+    Closed(Line, Issue),
+}
+impl Marker {
+    pub fn into_closed(self) -> Self {
+        match self {
+            Self::Valid(line, issue) => Self::Closed(line, issue),
+            #[cfg(not(tarpaulin_include))] // only ever called when is a Valid
+            _ => self,
+        }
+    }
+}
+
+impl std::fmt::Display for Marker {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            Self::Unmarked => Ok(()),
+            Self::Invalid(line) => write!(f, "- Invalid: {line}"),
+            Self::Valid(line, issue) => write!(f, "- Valid  : ({issue}) {line}"),
+            Self::Closed(line, issue) => write!(f, "- Closed : ({issue}) {line}"),
+        }
+    }
+}
+
+#[derive(Debug, Default)]
+pub struct Markers {
+    markers: Vec<Marker>,
+}
+impl Markers {
+    pub fn add_marker(&mut self, marker: Marker) {
+        self.markers.push(marker);
+    }
+}
+impl Deref for Markers {
+    type Target = Vec<Marker>;
+
+    fn deref(&self) -> &Self::Target {
+        &self.markers
+    }
+}
+impl std::fmt::Display for Markers {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        for marker in self.markers.iter() {
+            write!(f, "{marker}")?;
+        }
+        Ok(())
+    }
+}

src/model/markers/tests.rs

@@ -0,0 +1,55 @@
+//
+use crate::model::{markers::Markers, Line};
+
+#[test]
+fn found_when_displayed() -> anyhow::Result<()> {
+    //given
+    let fs = kxio::fs::temp()?;
+    let file = fs.base().join("file-name");
+    let relative = file.strip_prefix(fs.base())?.to_path_buf();
+
+    let mut found = Markers::default();
+
+    let marker_unmarked = Line::builder()
+        .file(file.clone())
+        .relative_path(relative.clone())
+        .num(10)
+        .value("line with no comment".to_owned())
+        .build()
+        .into_marker()?;
+    let marker_invalid = Line::builder()
+        .file(file.clone())
+        .relative_path(relative.clone())
+        .num(10)
+        .value(format!("line // {}: comment", "TODO"))
+        .build()
+        .into_marker()?;
+    let marker_valid = Line::builder()
+        .file(file)
+        .relative_path(relative)
+        .num(11)
+        .value(format!("line // {}: (#13) do this", "TODO"))
+        .build()
+        .into_marker()?;
+
+    found.add_marker(marker_unmarked);
+    found.add_marker(marker_invalid);
+    found.add_marker(marker_valid);
+
+    //when
+    let markers_as_string = found.to_string();
+    let result = markers_as_string.lines().collect::<Vec<&str>>();
+
+    //then
+    assert_eq!(
+        result,
+        vec![
+            "- Invalid: file-name#10:",
+            format!(" line // {}: comment", "TODO").as_str(),
+            "- Valid  : (13) file-name#11:",
+            format!(" line // {}: (#13) do this", "TODO").as_str()
+        ]
+    );
+
+    Ok(())
+}

src/model/mod.rs

@@ -9,3 +9,4 @@ mod tests;
 pub use config::Config;
 pub use line::Line;
 pub use markers::Marker;
+pub use markers::Markers;

src/printer.rs

@@ -1,12 +0,0 @@
-//
-pub trait Printer {
-    fn println(&self, message: impl Into<String>);
-}
-
-pub struct StandardPrinter;
-impl Printer for StandardPrinter {
-    #[cfg(not(tarpaulin_include))]
-    fn println(&self, message: impl Into<String>) {
-        println!("{}", message.into());
-    }
-}

src/scanner.rs

@@ -3,37 +3,36 @@ use std::{collections::HashSet, path::Path};
 
 use crate::{
     issues::Issue,
-    model::{Config, Line, Marker},
-    printer::Printer,
+    model::{Config, Line, Marker, Markers},
 };
 use anyhow::Result;
 use file_format::FileFormat;
 use ignore::Walk;
 
+//<'a> = dyn Fn(&'a Path, &'a Config, &'a mut Markers, &'a HashSet<Issue>, Output=Result<()>);
 pub trait FileScanner {
     fn scan_file(
         &self,
         path: &Path,
         config: &Config,
-        printer: &impl Printer,
+        markers: &mut Markers,
         issues: &HashSet<Issue>,
-    ) -> Result<u32>;
+    ) -> Result<()>;
 }
 
 pub fn find_markers(
-    printer: &impl Printer,
     config: &Config,
     issues: HashSet<Issue>,
     file_scanner: &impl FileScanner,
-) -> Result<u32, anyhow::Error> {
-    let mut errors = 0;
+) -> Result<Markers, anyhow::Error> {
+    let mut markers = Markers::default();
     for file in Walk::new(config.fs().base()).flatten() {
         let path = file.path();
         if is_text_file(config, path)? {
-            errors += file_scanner.scan_file(path, config, printer, &issues)?
+            file_scanner.scan_file(path, config, &mut markers, &issues)?
         }
     }
-    Ok(errors)
+    Ok(markers)
 }
 
 fn is_text_file(config: &Config, path: &Path) -> Result<bool> {
@@ -49,11 +48,10 @@ impl FileScanner for DefaultFileScanner {
         &self,
         file: &Path,
         config: &Config,
-        printer: &impl Printer,
+        markers: &mut Markers,
         issues: &HashSet<Issue>,
-    ) -> Result<u32> {
+    ) -> Result<()> {
         let relative_path = file.strip_prefix(config.fs().base())?.to_path_buf();
-        let mut errors = 0;
         config
             .fs()
             .file_read_to_string(file)? // tarpaulin uncovered okay
@@ -61,6 +59,7 @@ impl FileScanner for DefaultFileScanner {
             .enumerate()
             .map(|(n, line)| {
                 Line::builder()
+                    .file(file.to_path_buf())
                     .relative_path(relative_path.clone())
                     .num(n + 1) // line numbers are not 0-based, but enumerate is
                     .value(line.to_owned())
@@ -69,24 +68,9 @@ impl FileScanner for DefaultFileScanner {
             .filter_map(|line| line.into_marker().ok())
             .filter(|marker| !matches!(marker, Marker::Unmarked))
             .map(|marker| has_open_issue(marker, issues))
-            .for_each(|marker| match marker {
-                Marker::Invalid(_) => {
-                    errors += 1;
-                    // TODO: (#6) Better error message
-                    printer.println(marker.to_string());
-                }
-                Marker::Closed(_, _) => {
-                    errors += 1;
-                    // TODO: (#6) Better error message
-                    printer.println(marker.to_string());
-                }
-                _ => {}
-            });
-        if errors > 0 {
-            printer.println(format!(">> {errors} errors in {}", file.to_string_lossy()));
-        }
+            .for_each(|marker| markers.add_marker(marker));
 
-        Ok(errors)
+        Ok(())
     }
 }
 

src/tests/init.rs

@@ -2,10 +2,8 @@
 use super::*;
 
 use assert2::let_assert;
-use kxio::network::Network;
 use model::Config;
 use patterns::{issue_pattern, marker_pattern};
-use printer::TestPrinter;
 
 #[test]
 fn init_when_all_valid() -> anyhow::Result<()> {
@@ -15,8 +13,7 @@ fn init_when_all_valid() -> anyhow::Result<()> {
     std::env::set_var("GITHUB_WORKSPACE", fs.base());
     std::env::set_var("GITHUB_REPOSITORY", "repo");
     std::env::set_var("GITHUB_SERVER_URL", "server");
-    let net = Network::new_mock();
-    let printer = TestPrinter::default();
+    let net = kxio::network::Network::new_mock();
     let expected = Config::builder()
         .net(net.clone())
         .fs(kxio::fs::new(fs.base().to_path_buf()))
@@ -28,7 +25,7 @@ fn init_when_all_valid() -> anyhow::Result<()> {
         .build();
 
     //when
-    let result = init_config(&printer, net)?;
+    let result = init_config(net)?;
 
     //then
     assert_eq!(result.fs().base(), expected.fs().base());
@@ -53,10 +50,9 @@ fn init_when_no_workspace() -> anyhow::Result<()> {
     std::env::remove_var("GITHUB_WORKSPACE");
     std::env::set_var("GITHUB_REPOSITORY", "repo");
     std::env::set_var("GITHUB_SERVER_URL", "server");
-    let printer = TestPrinter::default();
 
     //when
-    let result = init_config(&printer, Network::new_mock());
+    let result = init_config(kxio::network::Network::new_mock());
 
     //then
     let_assert!(Err(e) = result);
@@ -73,10 +69,9 @@ fn init_when_no_repository() -> anyhow::Result<()> {
     std::env::set_var("GITHUB_WORKSPACE", fs.base());
     std::env::remove_var("GITHUB_REPOSITORY");
     std::env::set_var("GITHUB_SERVER_URL", "server");
-    let printer = TestPrinter::default();
 
     //when
-    let result = init_config(&printer, Network::new_mock());
+    let result = init_config(kxio::network::Network::new_mock());
 
     //then
     let_assert!(Err(e) = result);
@@ -93,10 +88,9 @@ fn init_when_no_server_url() -> anyhow::Result<()> {
     std::env::set_var("GITHUB_WORKSPACE", fs.base());
     std::env::set_var("GITHUB_REPOSITORY", "repo");
     std::env::remove_var("GITHUB_SERVER_URL");
-    let printer = TestPrinter::default();
 
     //when
-    let result = init_config(&printer, Network::new_mock());
+    let result = init_config(kxio::network::Network::new_mock());
 
     //then
     let_assert!(Err(e) = result);

src/tests/mod.rs

@@ -1,13 +1,12 @@
-//
-use super::*;
-
 use std::sync::{LazyLock, Mutex};
 
 use model::Config;
 use patterns::{issue_pattern, marker_pattern};
 
+//
+use super::*;
+
 mod init;
-mod printer;
 mod run;
 mod scanner;
 

src/tests/printer.rs

@@ -1,14 +0,0 @@
-//
-use std::cell::RefCell;
-
-use crate::printer::Printer;
-
-#[derive(Default)]
-pub struct TestPrinter {
-    pub messages: RefCell<Vec<String>>,
-}
-impl Printer for TestPrinter {
-    fn println(&self, message: impl Into<String>) {
-        self.messages.borrow_mut().push(message.into());
-    }
-}

src/tests/run.rs

@@ -4,7 +4,6 @@ use super::*;
 use anyhow::Result;
 use kxio::network::{RequestBody, RequestMethod, SavedRequest, StatusCode};
 use pretty_assertions::assert_eq;
-use printer::TestPrinter;
 
 #[tokio::test]
 async fn run_with_some_invalids() -> Result<()> {
@@ -30,7 +29,7 @@ async fn run_with_some_invalids() -> Result<()> {
     std::env::set_var("GITHUB_SERVER_URL", "https://git.kemitix.net");
 
     //when
-    let result = run(&TestPrinter::default(), net.clone().into()).await;
+    let result = run(net.clone().into()).await;
 
     //then
     assert!(result.is_err()); // there is an invalid file
@@ -67,7 +66,7 @@ async fn run_with_no_invalids() -> Result<()> {
     std::env::set_var("GITHUB_SERVER_URL", "https://git.kemitix.net");
 
     //when
-    let result = run(&TestPrinter::default(), net.clone().into()).await;
+    let result = run(net.clone().into()).await;
 
     //then
     assert!(result.is_ok()); // there is an invalid file

src/tests/scanner.rs

@@ -9,15 +9,13 @@ use issues::Issue;
 use model::Config;
 use patterns::{issue_pattern, marker_pattern};
 use pretty_assertions::assert_eq;
-use printer::TestPrinter;
 
 #[test]
 fn find_markers_in_dir() -> anyhow::Result<()> {
     //given
     let fs = kxio::fs::temp()?;
-    let file_with_invalids = fs.base().join("file_with_invalids.txt");
     fs.file_write(
-        &file_with_invalids,
+        &fs.base().join("file_with_invalids.txt"),
         include_str!("data/file_with_invalids.txt"),
     )?;
     fs.file_write(
@@ -34,22 +32,26 @@ fn find_markers_in_dir() -> anyhow::Result<()> {
         .issue_pattern(issue_pattern()?)
         .build();
     let issues = HashSet::from_iter(vec![Issue::new(23), Issue::new(43)]);
-    let printer = TestPrinter::default();
 
     //when
-    let errors = find_markers(&printer, &config, issues, &DefaultFileScanner)?;
+    let markers = find_markers(&config, issues, &DefaultFileScanner)?;
 
     //then
     assert_eq!(
-        printer.messages.take(),
+        markers.to_string().lines().collect::<Vec<_>>(),
         vec![
-            "- Invalid: file_with_invalids.txt#3:\n It contains a todo comment: // TODO: this is it\n",
-            "- Invalid: file_with_invalids.txt#5:\n It also contains a fix-me comment: // FIXME: and this is it\n",
-            "- Closed : (3) file_with_invalids.txt#9:\n We also have a todo comment: // TODO: (#3) and it has an issue number, but it is closed\n",
-            format!(">> 3 errors in {}", file_with_invalids.to_string_lossy()).as_str() 
+            "- Invalid: file_with_invalids.txt#3:",
+            " It contains a todo comment: // TODO: this is it",
+            "- Invalid: file_with_invalids.txt#5:",
+            " It also contains a fix-me comment: // FIXME: and this is it",
+            "- Closed : (3) file_with_invalids.txt#9:",
+            " We also have a todo comment: // TODO: (#3) and it has an issue number, but it is closed",
+            "- Valid  : (23) file_with_valids.txt#3:",
+            " It also has a todo comment: // TODO: (#23) and it has an issue number",
+            "- Valid  : (43) file_with_valids.txt#5:",
+            " Here is a fix-me comment: // FIXME: (#43) and is also has an issue number"
         ]
     );
-    assert_eq!(errors, 3);
 
     Ok(())
 }
@@ -68,10 +70,9 @@ fn skips_binary_files() -> Result<()> {
     let config = a_config(net, fs)?;
     let issues = HashSet::new();
     let file_scanner = TestFileScanner::default();
-    let printer = TestPrinter::default();
 
     //when
-    find_markers(&printer, &config, issues, &file_scanner)?;
+    find_markers(&config, issues, &file_scanner)?;
 
     //then
     assert_eq!(file_scanner.scanned.take(), vec![text_path]);
@@ -88,10 +89,10 @@ impl FileScanner for TestFileScanner {
         &self,
         path: &std::path::Path,
         _config: &Config,
-        _printer: &impl Printer,
+        _markers: &mut model::Markers,
         _issues: &HashSet<Issue>,
-    ) -> Result<u32> {
+    ) -> Result<()> {
         self.scanned.borrow_mut().push(path.to_path_buf());
-        Ok(0)
+        Ok(())
     }
 }