Improve terminal reporting, add ghmarkdown

Author: simoncozens

fontspector-checkapi/src/checkresult.rs

@@ -1,8 +1,8 @@
-use serde::Serialize;
+use serde::{ser::SerializeStruct, Serialize};
 
 use crate::{Check, CheckId, Status, StatusCode};
 
-#[derive(Debug, Clone, Serialize)]
+#[derive(Debug, Clone)]
 pub struct CheckResult {
     pub check_id: CheckId,
     pub check_name: String,
@@ -12,6 +12,20 @@ pub struct CheckResult {
     pub subresults: Vec<Status>,
 }
 
+impl Serialize for CheckResult {
+    fn serialize<S: serde::Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
+        let mut s = serializer.serialize_struct("CheckResult", 7)?;
+        s.serialize_field("check_id", &self.check_id)?;
+        s.serialize_field("check_name", &self.check_name)?;
+        s.serialize_field("check_rationale", &self.check_rationale)?;
+        s.serialize_field("filename", &self.filename)?;
+        s.serialize_field("section", &self.section)?;
+        s.serialize_field("subresults", &self.subresults)?;
+        s.serialize_field("worst_status", &self.worst_status())?;
+        s.end()
+    }
+}
+
 impl CheckResult {
     pub fn new(
         check: &Check,

fontspector-checkapi/src/status.rs

@@ -19,6 +19,19 @@ pub enum StatusCode {
     Error,
 }
 
+impl StatusCode {
+    pub fn all() -> impl Iterator<Item = StatusCode> {
+        vec![
+            StatusCode::Error,
+            StatusCode::Fail,
+            StatusCode::Warn,
+            StatusCode::Info,
+            StatusCode::Skip,
+            StatusCode::Pass,
+        ]
+        .into_iter()
+    }
+}
 impl std::fmt::Display for StatusCode {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match *self {

fontspector-cli/Cargo.toml

@@ -23,6 +23,7 @@ serde =  {version = "1.0.130", features=["derive"] }
 
 # Terminal reporter
 termimad = "0.14"
+colored = "2.1.0"
 
 # JSON reporter
 serde_json = "1.0"

fontspector-cli/src/args.rs

@@ -62,9 +62,13 @@ pub struct Args {
     pub skip_network: bool,
 
     /// Write a JSON formatted report to the given filename
-    #[clap(short, long, help_heading = "Reports")]
+    #[clap(long, help_heading = "Reports")]
     pub json: Option<String>,
 
+    /// Write a GitHub-Markdown formatted report to the given filename
+    #[clap(long, help_heading = "Reports")]
+    pub ghmarkdown: Option<String>,
+
     /// Input files
     pub inputs: Vec<String>,
 }

fontspector-cli/src/main.rs

@@ -11,7 +11,10 @@ use indicatif::ParallelProgressIterator;
 use profile_googlefonts::GoogleFonts;
 use profile_universal::Universal;
 use rayon::iter::{IntoParallelRefIterator, ParallelIterator};
-use reporters::{json::JsonReporter, terminal::TerminalReporter, Reporter, RunResults};
+use reporters::{
+    json::JsonReporter, markdown::MarkdownReporter, terminal::TerminalReporter, Reporter,
+    RunResults,
+};
 use serde_json::Map;
 
 /// Filter out checks that don't apply
@@ -150,19 +153,16 @@ fn main() {
     if let Some(jsonfile) = args.json.as_ref() {
         reporters.push(Box::new(JsonReporter::new(jsonfile)));
     }
+    if let Some(mdfile) = args.ghmarkdown.as_ref() {
+        reporters.push(Box::new(MarkdownReporter::new(mdfile)));
+    }
 
     for reporter in reporters {
         reporter.report(&results, &args, &registry);
     }
 
     if !args.quiet {
-        // Summary report
-        let summary = results.summary();
-        print!("\nSummary:\n  ");
-        for (status, count) in summary.iter() {
-            print!("{:}: {:}  ", status, count);
-        }
-        println!();
+        TerminalReporter::summary_report(results.summary());
     }
     if worst_status >= args.error_code_on {
         std::process::exit(1);

fontspector-cli/src/reporters/markdown.rs

@@ -0,0 +1,127 @@
+use std::collections::HashMap;
+
+use crate::reporters::{Reporter, RunResults};
+use crate::Args;
+use fontspector_checkapi::Registry;
+use serde_json::json;
+use tera::{Context, Tera, Value};
+
+pub(crate) struct MarkdownReporter {
+    filename: String,
+    tera: Tera,
+}
+
+fn percent_of(v: &Value, options: &HashMap<String, Value>) -> tera::Result<Value> {
+    let v = v.as_f64().unwrap_or(0.0);
+    let total = options.get("total").unwrap().as_f64().unwrap_or(100.0);
+    Ok(format!("{:.0}%", v / total * 100.0).into())
+}
+
+fn unindent(v: &Value, _options: &HashMap<String, Value>) -> tera::Result<Value> {
+    let v = v.as_str().unwrap_or("");
+    let v = v.trim_start();
+    Ok(v.into())
+}
+
+fn emoticon(v: &Value, _options: &HashMap<String, Value>) -> tera::Result<Value> {
+    let v = v.as_str().unwrap_or("");
+    let v = match v {
+        "ERROR" => "💥",
+        "FATAL" => "☠",
+        "FAIL" => "🔥",
+        "WARN" => "⚠️",
+        "INFO" => "ℹ️",
+        "SKIP" => "⏩",
+        "PASS" => "✅",
+        "DEBUG" => "🔎",
+        _ => "❓",
+    };
+    Ok(v.into())
+}
+
+impl MarkdownReporter {
+    pub fn new(filename: &str) -> Self {
+        let mut tera = Tera::new("templates/markdown/*").unwrap_or_else(|e| {
+            log::error!("Error parsing Markdown templates: {:?}", e);
+            std::process::exit(1);
+        });
+        tera.register_filter("percent", percent_of);
+        tera.register_filter("unindent", unindent);
+        tera.register_filter("emoticon", emoticon);
+
+        tera.register_tester("omitted", |_value: Option<&Value>, _params: &[Value]| {
+            // XXX
+            Ok(false)
+        });
+        Self {
+            tera,
+            filename: filename.to_string(),
+        }
+    }
+}
+impl Reporter for MarkdownReporter {
+    fn report(&self, results: &RunResults, args: &Args, registry: &Registry) {
+        let mut fatal_checks = HashMap::new();
+        let mut experimental_checks = HashMap::new();
+        let mut other_checks = HashMap::new();
+        let all_fonts = "All fonts".to_string();
+        for result in results.iter() {
+            let filename = result.filename.as_ref().unwrap_or(&all_fonts).as_str();
+            if registry.is_experimental(&result.check_id) {
+                experimental_checks
+                    .entry(filename)
+                    .or_insert_with(Vec::new)
+                    .push(result);
+            } else if result.is_error() {
+                fatal_checks
+                    .entry(filename)
+                    .or_insert_with(Vec::new)
+                    .push(result);
+            } else {
+                other_checks
+                    .entry(filename)
+                    .or_insert_with(Vec::new)
+                    .push(result);
+            }
+        }
+        let summary = results.summary();
+
+        let proposals: HashMap<String, String> = registry
+            .checks
+            .iter()
+            .map(|(k, v)| (k.clone(), v.proposal.to_string()))
+            .collect();
+
+        let val: serde_json::Value = json!({
+            "version": env!("CARGO_PKG_VERSION"),
+            "summary": &summary,
+            "summary_keys": summary.keys().collect::<Vec<_>>(),
+            // "omitted": vec![],
+            "fatal_checks": fatal_checks,
+            "other_checks": other_checks,
+            "experimental_checks": experimental_checks,
+            "succinct": args.succinct,
+            "total": results.len(),
+            "proposal": proposals,
+        });
+        let context = &Context::from_serialize(val).unwrap_or_else(|e| {
+            log::error!("Error creating Markdown context: {:}", e);
+            std::process::exit(1);
+        });
+
+        let rendered = self
+            .tera
+            .render("main.markdown", context)
+            .unwrap_or_else(|e| {
+                log::error!("Error rendering Markdown report: {:?}", e);
+                std::process::exit(1);
+            });
+        std::fs::write(&self.filename, rendered).unwrap_or_else(|e| {
+            eprintln!(
+                "Error writing Markdown report to {:}: {:}",
+                self.filename, e
+            );
+            std::process::exit(1);
+        });
+    }
+}

fontspector-cli/src/reporters/mod.rs

@@ -57,7 +57,7 @@ impl RunResults {
         organised_results
     }
 
-    fn len(&self) -> usize {
+    pub fn len(&self) -> usize {
         self.results.len()
     }
 }

fontspector-cli/src/reporters/terminal.rs

@@ -1,9 +1,10 @@
+use super::RunResults;
+use crate::{reporters::Reporter, Args};
+use colored::{ColoredString, Colorize};
 use fontspector_checkapi::{Registry, StatusCode, Testable};
 use itertools::Itertools;
+use std::{collections::HashMap, path::Path};
 
-use crate::{reporters::Reporter, Args};
-
-use super::RunResults;
 pub(crate) struct TerminalReporter {
     succinct: bool,
 }
@@ -14,6 +15,21 @@ impl TerminalReporter {
     }
 }
 
+fn colored_status(c: StatusCode, s: Option<&str>) -> ColoredString {
+    let string = match s {
+        Some(s) => s.to_string(),
+        None => c.to_string(),
+    };
+    match c {
+        StatusCode::Error => string.on_red(),
+        StatusCode::Fail => string.red(),
+        StatusCode::Warn => string.yellow(),
+        StatusCode::Info => string.cyan(),
+        StatusCode::Skip => string.blue(),
+        StatusCode::Pass => string.green(),
+    }
+}
+
 impl Reporter for TerminalReporter {
     fn report(&self, results: &RunResults, args: &Args, registry: &Registry) {
         let organised_results = results.organize();
@@ -25,33 +41,41 @@ impl Reporter for TerminalReporter {
             for (sectionname, results) in sectionresults.iter() {
                 let mut sectionheading_done = false;
                 for result in results.iter() {
+                    let subresults = result
+                        .subresults
+                        .iter()
+                        .filter(|c| c.severity >= args.loglevel)
+                        .collect::<Vec<_>>();
+                    if subresults.is_empty() {
+                        continue;
+                    }
+
                     if self.succinct {
                         println!(
-                            "{:}: {:} {:}",
-                            filename,
-                            result.check_id,
-                            result
-                                .subresults
+                            "{:}: {:} {:} [{}]",
+                            Path::new(filename).file_name().unwrap().to_string_lossy(),
+                            result.check_id.bright_cyan(),
+                            colored_status(result.worst_status(), None),
+                            subresults
                                 .iter()
-                                .flat_map(|r| r.code.as_ref())
-                                .join(", ")
+                                .map(|r| colored_status(
+                                    r.severity,
+                                    r.code.as_ref().map(|x| x.as_str())
+                                ))
+                                .join(" ")
                         );
                         continue;
                     }
 
-                    for subresult in result
-                        .subresults
-                        .iter()
-                        .filter(|c| c.severity >= args.loglevel)
-                    {
-                        if !fileheading_done {
-                            println!("Testing: {:}", filename);
-                            fileheading_done = true;
-                        }
-                        if !sectionheading_done {
-                            println!("  Section: {:}\n", sectionname);
-                            sectionheading_done = true;
-                        }
+                    if !fileheading_done {
+                        println!("Testing: {:}", filename);
+                        fileheading_done = true;
+                    }
+                    if !sectionheading_done {
+                        println!("  Section: {:}\n", sectionname);
+                        sectionheading_done = true;
+                    }
+                    for subresult in subresults {
                         println!(">> {:}", result.check_id);
                         if args.verbose > 1 {
                             println!("   {:}", result.check_name);
@@ -61,10 +85,6 @@ impl Reporter for TerminalReporter {
                             ));
                         }
                         termimad::print_inline(&format!("{:}\n", subresult));
-                        if subresult.severity != StatusCode::Fail {
-                            println!();
-                            continue;
-                        }
                         #[allow(clippy::unwrap_used)]
                         // This is a genuine can't-happen. We put it in the hashmap earlier!
                         let check = registry.checks.get(&result.check_id).unwrap();
@@ -86,3 +106,17 @@ impl Reporter for TerminalReporter {
         }
     }
 }
+
+impl TerminalReporter {
+    pub fn summary_report(summary: HashMap<StatusCode, i32>) {
+        print!("\nSummary:\n  ");
+        for code in StatusCode::all() {
+            print!(
+                "{:}: {:} ",
+                colored_status(code, None),
+                summary.get(&code).unwrap_or(&0)
+            );
+        }
+        println!();
+    }
+}

templates/markdown/check.markdown

@@ -0,0 +1,22 @@
+<details>
+    <summary>{{check.worst_status | emoticon}} <b>{{check.worst_status}}</b> {{check.check_name}} ({{check.check_id}})</summary>
+    <div>
+
+{% if not succinct %}
+{% for line in check.check_rationale | split(pat="\n") %}> {{line | unindent | replace(from="\n", to="") }}
+{% endfor %}
+{% endif %}
+
+{% if not succinct and proposals[check.check_id]%}
+Original proposal: {{proposals[check.check_id]}}
+{% endif %}
+
+{% for result in check.subresults |sort(attribute="severity") %}
+{% if not result is omitted %}
+
+- {{result.severity | emoticon }} **{{result.severity}}** {% if result is containing("message") %}{{result.message}}{% endif %} {%if result.code%}[code: {{result.code}}]{%endif%}
+  {% endif %}
+  {% endfor %}
+
+</div>
+</details>