Merge pull request #1071 from googlefonts/show-tags

crater html display tweaks
googlefonts · Oct 28, 2024 · 184a37c · 184a37c
2 parents ddd569c + 40dac14
commit 184a37c
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 17 deletions.
diff --git a/fontc_crater/resources/style.css b/fontc_crater/resources/style.css
@@ -64,7 +64,8 @@ td.rev {
   font-family: monospace;
 }
 
-.font_path .diff_result {
+.diff_result {
+  width: 10em;
   display: inline-block;
 }
 
@@ -73,6 +74,13 @@ td.rev {
   display: inline-block;
 }
 
+.changed_tag_list {
+  /*display: inline-block;*/
+  font-family: monospace;
+  font-size: 0.8em;
+  color: #888;
+}
+
 .diff_info {
   padding: 10px 20px;
 }
diff --git a/fontc_crater/src/ci/html.rs b/fontc_crater/src/ci/html.rs
@@ -318,7 +318,8 @@ fn make_diff_report(
             make_repro_cmd(repo_url, path)
         );
         let decoration = make_delta_decoration(*ratio, prev_ratio, More::IsBetter);
-        let details = format_diff_report_details(diff_details, prev_details);
+        let changed_tag_list = list_different_tables(diff_details).unwrap_or_default();
+        let details = format_diff_report_details(diff_details, prev_details, onclick);
         // avoid .9995 printing as 100%
         let ratio_fmt = format!("{:.3}%", (ratio * 1000.0).floor() / 1000.0);
 
@@ -327,11 +328,9 @@ fn make_diff_report(
                 summary {
                     span.font_path {
                         a href = (repo_url) { (path) }
-                        " ("
-                        a href = "" onclick = (onclick) { "copy repro" }
-                        ")"
                     }
                     span.diff_result { (ratio_fmt) " " (decoration) }
+                    span.changed_tag_list { (changed_tag_list) }
                 }
                 (details)
             }
@@ -483,8 +482,24 @@ fn format_compiler_error(err: &CompilerFailure) -> Markup {
     }
 }
 
+fn list_different_tables(current: &DiffOutput) -> Option<String> {
+    let changed = current
+        .iter_tables()
+        .filter(|x| *x != "total")
+        .collect::<Vec<_>>()
+        .join(", ");
+    if changed.is_empty() {
+        return None;
+    }
+    Some(format!("({changed})"))
+}
+
 /// for a given diff, the detailed information on per-table changes
-fn format_diff_report_details(current: &DiffOutput, prev: Option<&DiffOutput>) -> Markup {
+fn format_diff_report_details(
+    current: &DiffOutput,
+    prev: Option<&DiffOutput>,
+    repro_cmd: String,
+) -> Markup {
     let value_decoration = |table, value: DiffValue| -> Markup {
         let prev_value = match prev {
             None => None,
@@ -533,21 +548,32 @@ fn format_diff_report_details(current: &DiffOutput, prev: Option<&DiffOutput>) -
         })
         .collect::<Vec<_>>();
 
-    html! {
-        table.diff_info {
-            thead {
-                tr {
-                    th.diff_table scope = "col" { "table" }
-                    th.diff_value scope = "col" { "value" }
+    let diff_table = if all_items.is_empty() {
+        html!()
+    } else {
+        html! {
+            table {
+                thead {
+                    tr {
+                        th.diff_table scope = "col" { "table" }
+                        th.diff_value scope = "col" { "value" }
+                    }
                 }
-            }
-            @for (table, value) in all_items {
-                tr.table_diff_row {
-                    td.table_diff_name { (table) }
-                    td.table_diff_value { (value) " " ( {value_decoration(table, value) }) }
+                @for (table, value) in all_items {
+                    tr.table_diff_row {
+                        td.table_diff_name { (table) }
+                        td.table_diff_value { (value) " " ( {value_decoration(table, value) }) }
+                    }
                 }
             }
         }
+    };
+
+    html! {
+        div.diff_info {
+            (diff_table)
+            a href = "" onclick = (repro_cmd) { "copy reproduction command" }
+        }
     }
 }