Merge pull request #262 from alexanderalber/fix/235-icon-set-cfvo

XLSX: write one cfvo per icon in icon-set rules (#235)

# Conflicts:
#	contributors/people.toml

contributors/people.toml

@@ -35,3 +35,6 @@ sync_github_name = true
 
 [people.MacJedi42]
 sync_github_name = true
+
+[people.alexanderalber]
+real_name = "Alexander Alber"

crates/engine/src/cf.rs

@@ -90,10 +90,7 @@ fn lerp(a: [u8; 3], b: [u8; 3], t: f64) -> [u8; 3] {
     ]
 }
 
-/// Number of icons in a set name like `3TrafficLights1`, `5Quarters`.
-pub fn icon_count(set: &str) -> usize {
-    set.chars().next().and_then(|c| c.to_digit(10)).unwrap_or(3) as usize
-}
+pub use gridcraft_model::icon_count;
 
 impl CfCache {
     pub fn new() -> Self {
@@ -261,11 +258,18 @@ impl CfCache {
                 CfRule::IconSet { set, thresholds, reverse, show_value } => {
                     if let Some(n) = v.as_f64() {
                         let k = icon_count(set);
-                        let cuts: Vec<f64> = if thresholds.len() + 1 == k {
-                            thresholds.iter().map(|t| threshold(wb, sheet, c, &st, t)).collect()
-                        } else {
-                            (1..k).map(|i| percentile(&st.sorted, i as f64 * 100.0 / k as f64)).collect()
+                        // One threshold per icon, the first being the lower bound (as in the file);
+                        // also accept the lower bound left out, else Excel's defaults.
+                        let defaults;
+                        let lower = match thresholds.len() {
+                            n if n == k => thresholds.get(1..).unwrap_or_default(),
+                            n if n + 1 == k => thresholds.as_slice(),
+                            _ => {
+                                defaults = default_icon_thresholds(set);
+                                defaults.get(1..).unwrap_or_default()
+                            }
                         };
+                        let cuts: Vec<f64> = lower.iter().map(|t| threshold(wb, sheet, c, &st, t)).collect();
                         let mut idx = cuts.iter().filter(|cut| n >= **cut).count();
                         if *reverse {
                             idx = k - 1 - idx.min(k - 1);

crates/engine/src/tests.rs

@@ -1654,3 +1654,32 @@ fn notes_comments_and_links_survive_a_json_save() {
     assert_eq!(after.hyperlinks, before.hyperlinks);
     assert_eq!(after.cells, before.cells);
 }
+
+/// Icon-set thresholds as read from a file (one per icon, the first the lower bound) decide the
+/// icon; a rule without thresholds uses Excel's defaults (percent 33 and 67) (#235).
+#[test]
+fn icon_set_thresholds_from_files_pick_the_icon() {
+    use gridcraft_model::{CfRule, CfValueKind, CondFormat};
+    let mut s = s();
+    s.execute("range.setValues", json!({"range": "A1", "values": [[0], [10], [20], [30], [100]]})).unwrap();
+    s.execute("home.conditionalFormat", json!({"range": "A1:A5", "rule": {"type": "iconSet", "set": "3Arrows"}})).unwrap();
+    let icon = |s: &Session, a: &str| {
+        let wb = &s.doc().unwrap().wb;
+        crate::cf::CfCache::new().look(wb, 0, CellRef::parse(a).unwrap()).and_then(|l| l.icon).map(|(_, i)| i)
+    };
+    // Defaults: percent of the 0..100 span, so 30 is still the lowest icon.
+    assert_eq!([icon(&s, "A1"), icon(&s, "A4"), icon(&s, "A5")], [Some(0), Some(0), Some(2)]);
+    let sh = std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().wb).sheet_mut(0).unwrap();
+    sh.cond_formats = vec![CondFormat {
+        ranges: vec![gridcraft_core::RangeRef::parse("A1:A5").unwrap()],
+        rule: CfRule::IconSet {
+            set: "3Arrows".into(),
+            thresholds: vec![CfValueKind::Percent(0.0), CfValueKind::Number(10.0), CfValueKind::Number(25.0)],
+            reverse: false,
+            show_value: true,
+        },
+        priority: 1,
+        stop_if_true: false,
+    }];
+    assert_eq!([icon(&s, "A1"), icon(&s, "A2"), icon(&s, "A3"), icon(&s, "A4")], [Some(0), Some(1), Some(1), Some(2)]);
+}

crates/model/src/features.rs

@@ -196,6 +196,18 @@ pub enum CfRule {
     },
 }
 
+/// Number of icons in an icon set (`3Arrows` → 3, `5Quarters` → 5), always 3 to 5.
+pub fn icon_count(set: &str) -> usize {
+    set.chars().next().and_then(|c| c.to_digit(10)).unwrap_or(3).clamp(3, 5) as usize
+}
+
+/// Excel's default icon-set thresholds: one per icon, the first being the lower bound,
+/// then even percent steps (`0, 33, 67` for three icons).
+pub fn default_icon_thresholds(set: &str) -> Vec<CfValueKind> {
+    let k = icon_count(set);
+    (0..k).map(|i| CfValueKind::Percent((i as f64 * 100.0 / k as f64).round())).collect()
+}
+
 #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
 pub struct CondFormat {
     pub ranges: Vec<RangeRef>,

crates/xlsx/src/sheet_write.rs

@@ -6,7 +6,7 @@ use std::fmt::Write as _;
 use gridcraft_core::{CellError, CellRef, RangeRef, Value};
 use gridcraft_model::{
     AutoFilter, Cell, CfOperator, CfRule, CfValueKind, Color, CondFormat, ErrorStyle, FilterCriterion, LineInfo, Orientation, Sheet, SparklineKind,
-    Style, Table, TotalsFn, Validation, ValidationKind, Workbook,
+    Style, Table, TotalsFn, Validation, ValidationKind, Workbook, default_icon_thresholds, icon_count,
 };
 
 use crate::drawing::{Obj, image_content_type, image_ext};
@@ -778,7 +778,15 @@ fn cf_rule_xml(cf: &CondFormat, priority: u32, out: &mut Out) -> String {
                 b.push_str(" showValue=\"0\"");
             }
             b.push('>');
-            for t in thresholds {
+            // CT_IconSet needs one cfvo per icon (minOccurs 2); the first is the lower bound.
+            let k = icon_count(set);
+            let defaults = default_icon_thresholds(set);
+            let cfvos: Vec<&CfValueKind> = match thresholds.len() {
+                n if n == k => thresholds.iter().collect(),
+                n if n + 1 == k => defaults.iter().take(1).chain(thresholds).collect(),
+                _ => defaults.iter().collect(),
+            };
+            for t in cfvos {
                 b.push_str(&cfvo_xml(t));
             }
             b.push_str("</iconSet>");

crates/xlsx/src/tests/roundtrip.rs

@@ -600,3 +600,51 @@ fn spill_and_calc_errors_are_cached_as_value_errors() {
         assert_eq!(cell.child("v").unwrap().text, value);
     }
 }
+
+/// `CT_IconSet` needs one `cfvo` per icon (at least two); a rule without stored thresholds
+/// gets Excel's defaults, one without the lower bound gets it prepended (#235).
+#[test]
+fn icon_sets_always_write_one_cfvo_per_icon() {
+    let mut wb = Workbook::new();
+    let s = wb.sheet_mut(0).unwrap();
+    for (i, (set, thresholds)) in [
+        ("3Arrows", vec![]),
+        ("4Rating", vec![CfValueKind::Number(2.0), CfValueKind::Number(5.0), CfValueKind::Number(8.0)]),
+        ("9Bogus", vec![CfValueKind::Percent(50.0)]),
+    ]
+    .into_iter()
+    .enumerate()
+    {
+        s.cond_formats.push(CondFormat {
+            ranges: vec![rr(&format!("A{}", i + 1))],
+            rule: CfRule::IconSet { set: set.into(), thresholds, reverse: false, show_value: true },
+            priority: i as u32 + 1,
+            stop_if_true: false,
+        });
+    }
+    let bytes = write_xlsx(&wb).unwrap();
+    let (xml, _) = sheet1_xml(&bytes);
+    assert!(
+        xml.contains(
+            "<iconSet iconSet=\"3Arrows\"><cfvo type=\"percent\" val=\"0\"/><cfvo type=\"percent\" val=\"33\"/><cfvo type=\"percent\" val=\"67\"/></iconSet>"
+        ),
+        "{xml}"
+    );
+    assert!(
+        xml.contains("<iconSet iconSet=\"4Rating\"><cfvo type=\"percent\" val=\"0\"/><cfvo type=\"num\" val=\"2\"/><cfvo type=\"num\" val=\"5\"/><cfvo type=\"num\" val=\"8\"/></iconSet>"),
+        "{xml}"
+    );
+    assert!(xml.contains("<iconSet iconSet=\"9Bogus\"><cfvo type=\"percent\" val=\"0\"/>"), "{xml}");
+    let (back, _) = read_xlsx(&bytes).unwrap();
+    let rules: Vec<usize> = back
+        .sheet(0)
+        .unwrap()
+        .cond_formats
+        .iter()
+        .map(|cf| match &cf.rule {
+            CfRule::IconSet { thresholds, .. } => thresholds.len(),
+            _ => 0,
+        })
+        .collect();
+    assert_eq!(rules, [3, 4, 5]);
+}