chore(deps): update kemitix/rust action to v2.4.1

Renovate isn't smart enough to realise that this is hosted on
code.forgejo.org, and the whole point of having it here is to verify
that the shorthand format works.

.forgejo/workflows/push-next.yml

@@ -4,16 +4,56 @@ on:
   push:
     branches:
       - next
+  pull_request:
+    branches:
+      - main
   workflow_dispatch:
 
+env:
+  CARGO_TERM_COLOR: always
+
 jobs:
 
-  test:
+  checks:
     runs-on: docker
 
+    strategy:
+      matrix:
+        toolchain:
+          - name: stable
+          - name: nightly
+
     steps:
       - name: Checkout
         uses: actions/checkout@v4
 
-      - name: Check TODOs
-        uses: https://git.kemitix.net/kemitix/forgejo-todo-checker@v1
+      - name: Check TODOs (Origin)
+        uses: https://git.kemitix.net/kemitix/forgejo-todo-checker@v1.1.0
+
+      - name: Check TODOs (Mirror)
+        uses: kemitix/todo-checker@v1.0.0
+
+      - name: Machete
+        uses: https://git.kemitix.net/kemitix/rust@v2.4.1
+        with:
+          args: ${{ matrix.toolchain.name }} cargo machete
+
+      - name: Format
+        uses: https://git.kemitix.net/kemitix/rust@v2.4.1
+        with:
+          args: ${{ matrix.toolchain.name }} cargo fmt --all -- --check
+
+      - name: Clippy
+        uses: https://git.kemitix.net/kemitix/rust@v2.4.1
+        with:
+          args: ${{ matrix.toolchain.name }} cargo hack --feature-powerset clippy
+
+      - name: Build
+        uses: https://git.kemitix.net/kemitix/rust@v2.4.1
+        with:
+          args: ${{ matrix.toolchain.name }} cargo hack --feature-powerset build
+
+      - name: Test
+        uses: https://git.kemitix.net/kemitix/rust@v2.4.1
+        with:
+          args: ${{ matrix.toolchain.name }} cargo hack --feature-powerset test

CHANGELOG.md

@@ -0,0 +1,89 @@
+## [1.1.0] - 2024-09-22
+
+### Build
+
+- Add justfile for self-testing
+- Add test and build checks to workflows
+- Use codeberg mirror to self-test todo action
+- Configure release-plz
+- Customise release-plz
+- Drop release-plz
+- Add prep-release recipie to justfile
+
+### Chore
+
+- Clean up output
+
+### Feat
+
+- Detect and ignore non-text files
+- Log errors as they are found
+- Improve error messageso
+
+### Fix
+
+- Only look for issue number within the comment
+
+### Refactor
+
+- Abstract printer via Printer trait
+
+## [1.0.0] - 2024-09-20
+
+### Build
+
+- Don't check README for TODO/FIXME comments
+- Use v1.0.0 of the todo checker
+
+### Chore
+
+- *(deps)* Update actions/checkout action to v4
+
+### Docs
+
+- Add instructions in README
+
+### Feat
+
+- Add skeleton action
+- Use Dockerfile (hello world)
+- List contents of current directory
+- Check env vars are all set
+- Collect useful environment vars
+- Collect env into Config and note planned operations
+- Scan for TODO and FIXME markers
+- Log progress ignoring files listed in .gitignore, .ignore and .rgignore
+- Pretty-print found markers
+- Fetch open issues
+- Flag markers where issue is closed
+- Log any invalid/closed markers and exit if any found
+
+### Fix
+
+- Specify url correctly for action
+- Specify valid value for runs.using
+- Recheck tests
+
+### Refactor
+
+- Prepare for adding tests
+- Markers as enum parsed from lines
+- Comment out unused code in line
+- Split up main
+- Collapse empty modules
+- Clean up issue regex
+
+### Test
+
+- Add skeleton self-test
+- Allow workflow to be run manually
+- Add first tests for pattern matching
+- Add tests for markers
+
+### Tests
+
+- Add tests for Config
+- Add tests for init module
+- Add tests for scanner module
+- Add tests for main
+

Cargo.toml

@@ -1,20 +1,23 @@
 [package]
 name = "forgejo-todo-checker"
-version = "0.1.0"
+version = "1.1.0"
 edition = "2021"
+publish = false # NOTE: Not a CLI tool or a library, so don't release to crates.io
 
 [dependencies]
 anyhow = "1.0"
-regex = "1.10"
-ureq = "2.10"
-kxio = "1.2"
+bon = "3.0"
 ignore = "0.4"
-bon = "2.3"
-tokio = { version = "1.37", features = [ "full" ] }
-serde = { version = "1.0", features = [ "derive" ] }
-serde_json = "1.0"
+file-format = { version = "0.26", features = ["reader-txt"] }
+kxio = "1.2"
+regex = "1.10"
+serde = { version = "1.0", features = ["derive"] }
+tokio = { version = "1.37", features = ["full"] }
 
 [dev-dependencies]
 assert2 = "0.3"
 pretty_assertions = "1.4"
-rstest = "0.22"
+rstest = "0.23"
+
+[lints.rust]
+unexpected_cfgs = { level = "warn", check-cfg = ['cfg(tarpaulin_include)'] }

Dockerfile

@@ -1,4 +1,4 @@
-FROM docker.io/rust:1.81.0-bookworm
+FROM docker.io/rust:1.82.0-bookworm
 
 WORKDIR /app
 COPY Cargo.toml ./

README.md

@@ -1,14 +1,42 @@
 # forgejo-todo-checker
 
-Checks your source files for TODO and FIXME comments, where they don't have an open issue number.
+Checks your source files for TODO and FIXME comments, failing your build where they don't have an open issue number.
 
-A ForgeJo Action.
+- [A ForgeJo Action](https://forgejo.org/docs/next/user/actions/).
 
 (Inspired by https://woodpecker-ci.org/plugins/TODO-Checker)
 
+## LATEST version
+
+See [Releases](https://git.kemitix.net/kemitix/forgejo-todo-checker/releases) for the latest version. Replace `${LATEST}` in the examples below with the tag version (include any leading `v`).
+
+## code.forgejo.org Mirror
+
+Main development takes place on [git.kemitix.net](https://git.kemitix.net/kemitix/forgejo-todo-checker).
+
+There is a mirror on code.forgejo.org as [kemitix/todo-checker](https://code.forgejo.org/kemitix/todo-checker).
+
+This mirror allows you to refer to the action as simply `kemitix/todo-checker@${LATEST}`.
+
+## Usage
+
+```yaml
+jobs:
+  tests:
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+
+      - name: Check TODOs
+        # Original:
+        # uses: https://git.kemitix.net/kemitix/forgejo-todo-checker@${LATEST}
+        # Codeberg mirror:
+        uses: kemitix/todo-checker@${LATEST}
+```
+
 ## Comments Format
 
-This Action only pays attention to comments in a particular format. e.g:
+This Action looks for comments in the following formats:
 
 ```
 // TODO: (#19) This is the comment
@@ -19,43 +47,38 @@ This Action only pays attention to comments in a particular format. e.g:
 
 These are all considered valid comments. Pick the format that fits your language or file best.
 
-Comments are found by matching them against this regular expression: `(#|//)\s*(TODO|FIXME)`
+Comments are found by matching them against this regular expression: `(#|//)\s*(TODO|FIXME):?`
 
-i.e.: be a comment by starting with either '#' or '//', then the word `TODO` or `FIXME` in all caps.
+i.e.: be a comment by starting with either '#' or '//', then the word `TODO` or `FIXME` in all caps, with or without a trailing '`:`'.
 
 Once we have a line with such a comment we look for the Issue Number with: `\(#?(?P<ISSUE_NUMBER>\d+)\)`
 
-i.e.: a number in `()`, with or without a leading `#` (inside the braces).
+i.e.: a number in '`()`', with or without a leading '`#`' (inside the braces) immediately after the '`TODO`' or '`FIXME`'.
 
 The `ISSUE_NUMBER` must correspond to an **OPEN** Issue in the repo that the Action is running against.
 
 If the issue has been closed or can't be found then the comment is marked as an error and the Check with fail.
 
-## Example Use as a ForgeJo Action Step
-
-```yaml
-jobs:
-  tests:
-    steps:
-      - name: Checkout
-        uses: actions/checkout@v4
-
-      - name: Check TODOs
-        uses: https://git.kemitix.net/kemitix/forgejo-todo-checker@v1.0.0
-```
+## Example Output
 
 The output will be similar to the following if there are any errors:
 
 ```
 Forgejo TODO Checker!
-Repo: kemitix/my-projext
-Prefix: (#|//)\s*(TODO|FIXME)
-Issues: ( |)(\(|\(#)(?P<ISSUE_NUMBER>\d+)(\))
-- Invalid: src/main.rs#38:
+
+Repo : kemitix/my-project
+Regex: (#|//)\s*(TODO|FIXME):?\s*\(#?(?P<ISSUE_NUMBER>\d+)\)
+
+- Issue number missing: src/main.rs#38:
  // TODO: implement this cool feature and get rich!
-- Closed : (19) README.md#12:
+
+>> 1 error in src/main.rs
+
+- Closed/Invalid Issue: (19) src/model/line.rs#12:
  // TODO: (#19) This is the comment
 
+>> 1 error in src/model/line.rs
+
 Error: Invalid or closed TODO/FIXMEs found
 ```
 

justfile

@@ -0,0 +1,22 @@
+self-test:
+  just test $PWD forgejo-todo-checker
+ 
+test path repo:
+  GITHUB_WORKSPACE={{ path }} \
+  GITHUB_REPOSITORY=kemitix/{{ repo }} \
+  GITHUB_SERVER_URL=https://git.kemitix.net \
+  cargo run
+
+next_version := `git-cliff --bumped-version | cut -b 2-`
+
+@next:
+  echo "Next version: {{ next_version }}"
+
+prep-release:
+  jj new -m"chore: release v{{ next_version }}"
+  cargo set-version "{{ next_version }}"
+  git-cliff -o CHANGELOG.md --bump
+  echo "Check CHANGELOG.md for next version"
+  jj diff
+  jj status
+  

renovate.json

@@ -2,5 +2,6 @@
   "$schema": "https://docs.renovatebot.com/renovate-schema.json",
   "extends": [
     "config:recommended"
-  ]
+  ],
+  "ignoreDeps": ["kemitix/todo-checker"]
 }

src/init.rs

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

src/issues/fetch.rs

@@ -23,7 +23,7 @@ pub async fn fetch_open_issues(config: &Config) -> Result<HashSet<Issue>> {
     let issues: HashSet<Issue> = config
         .net()
         .get::<Vec<Issue>>(request)
-        .await?
+        .await? // tarpaulin uncovered okay
         .response_body()
         .unwrap_or_default()
         .into_iter()

src/main.rs

@@ -2,41 +2,36 @@
 use anyhow::{bail, Result};
 use init::init_config;
 use issues::fetch_open_issues;
-use scanner::find_markers;
+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)]
 mod tests;
 
 #[tokio::main]
+#[cfg(not(tarpaulin_include))]
 async fn main() -> std::result::Result<(), Box<dyn std::error::Error>> {
-    Ok(run(kxio::network::Network::new_real()).await?)
+    use printer::StandardPrinter;
+
+    Ok(run(&StandardPrinter, Network::new_real()).await?)
 }
 
-async fn run(net: kxio::network::Network) -> Result<()> {
-    println!("Forgejo TODO Checker!");
+async fn run(printer: &impl Printer, net: Network) -> Result<()> {
+    printer.println("Forgejo TODO Checker!");
 
-    let config = init_config(net)?;
+    let config = init_config(printer, net)?;
     let issues = fetch_open_issues(&config).await?;
-    let markers = find_markers(&config, issues)?;
+    let errors = find_markers(printer, &config, issues, &DefaultFileScanner)?;
 
-    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 {
+    if errors > 0 {
         bail!("Invalid or closed TODO/FIXMEs found")
     }
     Ok(())

src/model/config.rs

@@ -1,6 +1,4 @@
 //
-#![allow(dead_code)]
-
 use bon::Builder;
 use regex::Regex;
 
@@ -11,7 +9,6 @@ pub struct Config {
     repo: String,
     server: String,
     auth_token: Option<String>,
-    prefix_pattern: Regex,
     issue_pattern: Regex,
 }
 impl Config {
@@ -30,9 +27,6 @@ impl Config {
     pub fn auth_token(&self) -> Option<&str> {
         self.auth_token.as_deref()
     }
-    pub fn prefix_pattern(&self) -> &Regex {
-        &self.prefix_pattern
-    }
     pub fn issue_pattern(&self) -> &Regex {
         &self.issue_pattern
     }

src/model/line.rs

@@ -1,6 +1,4 @@
 //
-#![allow(dead_code)]
-
 use std::path::PathBuf;
 
 use anyhow::{Context, Result};
@@ -15,22 +13,11 @@ 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

@@ -0,0 +1,29 @@
+//
+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, "- Issue number missing: {}", line),
+            Self::Closed(line, issue) => write!(f, "- Closed/Invalid Issue: ({issue}) {line}"),
+            Self::Valid(_, _) | Self::Unmarked => Ok(()), // tarpaulin uncovered okay
+        }
+    }
+}

src/model/markers/mod.rs

@@ -1,61 +0,0 @@
-//
-
-#[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),
-            _ => 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

@@ -1,55 +0,0 @@
-//
-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,4 +9,3 @@ mod tests;
 pub use config::Config;
 pub use line::Line;
 pub use markers::Marker;
-pub use markers::Markers;

src/model/tests/config.rs

@@ -1,10 +1,7 @@
 //
 use anyhow::Result;
 
-use crate::{
-    patterns::{issue_pattern, marker_pattern},
-    tests::a_config,
-};
+use crate::{patterns::issue_pattern, tests::a_config};
 
 #[tokio::test]
 async fn with_config_get_net() -> Result<()> {
@@ -37,22 +34,6 @@ fn with_config_get_fs() -> Result<()> {
     Ok(())
 }
 
-#[test]
-fn with_config_get_prefix_pattern() -> Result<()> {
-    //given
-    let net = kxio::network::Network::new_mock();
-    let fs = kxio::fs::temp()?;
-    let config = a_config(net, fs)?;
-
-    //when
-    let result = config.prefix_pattern();
-
-    //then
-    assert_eq!(result.to_string(), marker_pattern()?.to_string());
-
-    Ok(())
-}
-
 #[test]
 fn with_config_get_issue_pattern() -> Result<()> {
     //given

src/patterns/mod.rs

@@ -5,12 +5,15 @@ mod tests;
 use anyhow::{Context as _, Result};
 use regex::Regex;
 
+const MARKER_RE: &str = r"(#|//)\s*(TODO|FIXME):?";
+const ISSUE_RE: &str = r"\(#?(?P<ISSUE_NUMBER>\d+)\)";
+
 /// The pattern to find a TODO or FIXME comment
 pub fn marker_pattern() -> Result<Regex> {
-    regex::Regex::new(r"(#|//)\s*(TODO|FIXME)").context("prefix regex")
+    regex::Regex::new(MARKER_RE).context("prefix regex")
 }
 
 /// The pattern to find an issue number on an already found TODO or FIXME comment
 pub fn issue_pattern() -> Result<Regex> {
-    regex::Regex::new(r"\(#?(?P<ISSUE_NUMBER>\d+)\)").context("issue regex")
+    regex::Regex::new(format!(r"{MARKER_RE}\s*{ISSUE_RE}").as_str()).context("issue regex")
 }

src/patterns/tests/issue.rs

@@ -21,8 +21,8 @@ fn when_issue_should_find_number() -> Result<()> {
 }
 
 #[rstest::rstest]
-#[case("(#13)")]
-#[case("(13)")]
+#[case("(41) // TODO: (#13)")] // should ignore the 41
+#[case("(#52) // FIXME (13)")] // should ignore the 52
 fn find_issue_thirteen(#[case] input: &str) -> Result<()> {
     assert_eq!(
         issue_pattern()?

src/printer.rs

@@ -0,0 +1,12 @@
+//
+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,64 @@ use std::{collections::HashSet, path::Path};
 
 use crate::{
     issues::Issue,
-    model::{Config, Line, Marker, Markers},
+    model::{Config, Line, Marker},
+    printer::Printer,
 };
 use anyhow::Result;
+use file_format::FileFormat;
 use ignore::Walk;
 
-pub fn find_markers(config: &Config, issues: HashSet<Issue>) -> Result<Markers, anyhow::Error> {
-    let mut markers = Markers::default();
-    for file in Walk::new(config.fs().base()).flatten() {
-        let path = file.path();
-        if config.fs().path_is_file(path)? {
-            scan_file(path, config, &mut markers, &issues)?;
-        }
-    }
-    Ok(markers)
+pub trait FileScanner {
+    fn scan_file(
+        &self,
+        path: &Path,
+        config: &Config,
+        printer: &impl Printer,
+        issues: &HashSet<Issue>,
+    ) -> Result<u32>;
 }
 
-fn scan_file(
+pub fn find_markers(
+    printer: &impl Printer,
+    config: &Config,
+    issues: HashSet<Issue>,
+    file_scanner: &impl FileScanner,
+) -> Result<u32, anyhow::Error> {
+    let mut errors = 0;
+    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)?
+        }
+    }
+    Ok(errors)
+}
+
+fn is_text_file(config: &Config, path: &Path) -> Result<bool> {
+    Ok(config.fs().path_is_file(path)?
+        && FileFormat::from_file(path)?
+            .media_type()
+            .starts_with("text/"))
+}
+
+pub struct DefaultFileScanner;
+impl FileScanner for DefaultFileScanner {
+    fn scan_file(
+        &self,
         file: &Path,
         config: &Config,
-    found_markers: &mut Markers,
+        printer: &impl Printer,
         issues: &HashSet<Issue>,
-) -> Result<()> {
+    ) -> Result<u32> {
         let relative_path = file.strip_prefix(config.fs().base())?.to_path_buf();
+        let mut errors = 0;
         config
             .fs()
-        .file_read_to_string(file)?
+            .file_read_to_string(file)? // tarpaulin uncovered okay
             .lines()
             .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())
@@ -42,9 +69,26 @@ fn scan_file(
             .filter_map(|line| line.into_marker().ok())
             .filter(|marker| !matches!(marker, Marker::Unmarked))
             .map(|marker| has_open_issue(marker, issues))
-        .for_each(|marker| found_markers.add_marker(marker));
+            .for_each(|marker| match marker {
+                Marker::Invalid(_) => {
+                    errors += 1;
+                    printer.println(marker.to_string());
+                }
+                Marker::Closed(_, _) => {
+                    errors += 1;
+                    printer.println(marker.to_string());
+                }
+                _ => {}
+            });
+        if errors > 0 {
+            printer.println(format!(
+                ">> {errors} errors in {}\n",
+                relative_path.to_string_lossy()
+            ));
+        }
 
-    Ok(())
+        Ok(errors)
+    }
 }
 
 fn has_open_issue(marker: Marker, issues: &HashSet<Issue>) -> Marker {

src/tests/init.rs

@@ -2,8 +2,10 @@
 use super::*;
 
 use assert2::let_assert;
+use kxio::network::Network;
 use model::Config;
-use patterns::{issue_pattern, marker_pattern};
+use patterns::issue_pattern;
+use printer::TestPrinter;
 
 #[test]
 fn init_when_all_valid() -> anyhow::Result<()> {
@@ -13,28 +15,24 @@ 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 = kxio::network::Network::new_mock();
+    let net = Network::new_mock();
+    let printer = TestPrinter::default();
     let expected = Config::builder()
         .net(net.clone())
         .fs(kxio::fs::new(fs.base().to_path_buf()))
         .repo("repo".to_string())
         .server("server".to_string())
-        .prefix_pattern(marker_pattern()?)
         .issue_pattern(issue_pattern()?)
         .maybe_auth_token(Some("auth".to_string()))
         .build();
 
     //when
-    let result = init_config(net)?;
+    let result = init_config(&printer, net)?;
 
     //then
     assert_eq!(result.fs().base(), expected.fs().base());
     assert_eq!(result.repo(), expected.repo());
     // assert_eq!(result.server(), expected.server());
-    assert_eq!(
-        result.prefix_pattern().to_string(),
-        expected.prefix_pattern().to_string()
-    );
     assert_eq!(
         result.issue_pattern().to_string(),
         expected.issue_pattern().to_string()
@@ -50,9 +48,10 @@ 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(kxio::network::Network::new_mock());
+    let result = init_config(&printer, Network::new_mock());
 
     //then
     let_assert!(Err(e) = result);
@@ -69,9 +68,10 @@ 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(kxio::network::Network::new_mock());
+    let result = init_config(&printer, Network::new_mock());
 
     //then
     let_assert!(Err(e) = result);
@@ -88,9 +88,10 @@ 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(kxio::network::Network::new_mock());
+    let result = init_config(&printer, Network::new_mock());
 
     //then
     let_assert!(Err(e) = result);

src/tests/mod.rs

@@ -1,12 +1,13 @@
-use std::sync::{LazyLock, Mutex};
-
-use model::Config;
-use patterns::{issue_pattern, marker_pattern};
-
 //
 use super::*;
 
+use std::sync::{LazyLock, Mutex};
+
+use model::Config;
+use patterns::issue_pattern;
+
 mod init;
+mod printer;
 mod run;
 mod scanner;
 
@@ -19,7 +20,6 @@ pub fn a_config(net: kxio::network::Network, fs: kxio::fs::FileSystem) -> Result
         .server("https://git.kemitix.net".to_string())
         .repo("kemitix/test".to_string())
         .auth_token("secret".to_string())
-        .prefix_pattern(marker_pattern()?)
         .issue_pattern(issue_pattern()?)
         .build())
 }

src/tests/printer.rs

@@ -0,0 +1,14 @@
+//
+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,6 +4,7 @@ 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<()> {
@@ -29,7 +30,7 @@ async fn run_with_some_invalids() -> Result<()> {
     std::env::set_var("GITHUB_SERVER_URL", "https://git.kemitix.net");
 
     //when
-    let result = run(net.clone().into()).await;
+    let result = run(&TestPrinter::default(), net.clone().into()).await;
 
     //then
     assert!(result.is_err()); // there is an invalid file
@@ -66,7 +67,7 @@ async fn run_with_no_invalids() -> Result<()> {
     std::env::set_var("GITHUB_SERVER_URL", "https://git.kemitix.net");
 
     //when
-    let result = run(net.clone().into()).await;
+    let result = run(&TestPrinter::default(), net.clone().into()).await;
 
     //then
     assert!(result.is_ok()); // there is an invalid file

src/tests/scanner.rs

@@ -1,19 +1,23 @@
+use crate::scanner::FileScanner;
+
 //
 use super::*;
 
-use std::collections::HashSet;
+use std::{cell::RefCell, collections::HashSet, fs::File, io::Write, path::PathBuf};
 
 use issues::Issue;
 use model::Config;
-use patterns::{issue_pattern, marker_pattern};
+use patterns::issue_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(
-        &fs.base().join("file_with_invalids.txt"),
+        &file_with_invalids,
         include_str!("data/file_with_invalids.txt"),
     )?;
     fs.file_write(
@@ -26,30 +30,67 @@ fn find_markers_in_dir() -> anyhow::Result<()> {
         .fs(fs.clone())
         .server("".to_string())
         .repo("".to_string())
-        .prefix_pattern(marker_pattern()?)
         .issue_pattern(issue_pattern()?)
         .build();
     let issues = HashSet::from_iter(vec![Issue::new(23), Issue::new(43)]);
+    let printer = TestPrinter::default();
 
     //when
-    let markers = find_markers(&config, issues)?;
+    let errors = find_markers(&printer, &config, issues, &DefaultFileScanner)?;
 
     //then
     assert_eq!(
-        markers.to_string().lines().collect::<Vec<_>>(),
+        printer.messages.take(),
         vec![
-            "- 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"
+            "- Issue number missing: file_with_invalids.txt#3:\n It contains a todo comment: // TODO: this is it\n",
+            "- Issue number missing: file_with_invalids.txt#5:\n It also contains a fix-me comment: // FIXME: and this is it\n",
+            "- Closed/Invalid Issue: (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 {}\n", file_with_invalids.strip_prefix(fs.base())?.to_string_lossy()).as_str() 
         ]
     );
+    assert_eq!(errors, 3);
 
     Ok(())
 }
+
+#[test]
+fn skips_binary_files() -> Result<()> {
+    //given
+    let fs = kxio::fs::temp()?;
+    let binary_path = fs.base().join("binary_file.bin");
+    let mut binary_file = File::create(binary_path)?;
+    binary_file.write_all(&[0, 0, 0, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9])?;
+    let text_path = fs.base().join("text_file.txt");
+    fs.file_write(&text_path, "text contents")?;
+
+    let net = kxio::network::Network::new_mock();
+    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)?;
+
+    //then
+    assert_eq!(file_scanner.scanned.take(), vec![text_path]);
+
+    Ok(())
+}
+
+#[derive(Default)]
+struct TestFileScanner {
+    scanned: RefCell<Vec<PathBuf>>,
+}
+impl FileScanner for TestFileScanner {
+    fn scan_file(
+        &self,
+        path: &std::path::Path,
+        _config: &Config,
+        _printer: &impl Printer,
+        _issues: &HashSet<Issue>,
+    ) -> Result<u32> {
+        self.scanned.borrow_mut().push(path.to_path_buf());
+        Ok(0)
+    }
+}