From 32d0a866f951102ee1c33826e9bf7ba239324c03 Mon Sep 17 00:00:00 2001
From: Alexander Alber <90604107+alexanderalber@users.noreply.github.com>
Date: Sun, 11 Oct 2026 09:26:00 +0200
Subject: [PATCH 1/2] XLSX: write one cfvo per icon in icon-set rules (#235)
An icon-set rule without stored thresholds (every rule made by
home.conditionalFormat, and the sales sample) was written as an empty
, which CT_IconSet forbids (cfvo minOccurs 2); openpyxl drops it.
The writer now always emits one cfvo per icon: the stored thresholds when
there is one per icon, the lower bound prepended when it is missing, else
Excel's defaults (percent 0/33/67, 0/25/50/75, 0/20/40/60/80).
Evaluation read the thresholds as one fewer than the icons, but files
carry one per icon (the first is the lower bound), so thresholds read from
a file were ignored. Both now share icon_count and default_icon_thresholds
in gridcraft-model; icon_count is clamped to 3..=5 so a set name like
"0Foo" can't underflow k - 1.
generated with Claude Code
---
crates/engine/src/cf.rs | 20 ++++++++-----
crates/engine/src/tests.rs | 29 ++++++++++++++++++
crates/model/src/features.rs | 12 ++++++++
crates/xlsx/src/sheet_write.rs | 12 ++++++--
crates/xlsx/src/tests/roundtrip.rs | 48 ++++++++++++++++++++++++++++++
5 files changed, 111 insertions(+), 10 deletions(-)
diff --git a/crates/engine/src/cf.rs b/crates/engine/src/cf.rs
index a1028632e..25fa49f83 100644
--- a/crates/engine/src/cf.rs
+++ b/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 = 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 = 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);
diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs
index 6bd03d55e..c2cce25fa 100644
--- a/crates/engine/src/tests.rs
+++ b/crates/engine/src/tests.rs
@@ -1633,3 +1633,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)]);
+}
diff --git a/crates/model/src/features.rs b/crates/model/src/features.rs
index b9fe2ce74..2d2518d16 100644
--- a/crates/model/src/features.rs
+++ b/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 {
+ 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,
diff --git a/crates/xlsx/src/sheet_write.rs b/crates/xlsx/src/sheet_write.rs
index b1e3fca2c..313839f91 100644
--- a/crates/xlsx/src/sheet_write.rs
+++ b/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("");
diff --git a/crates/xlsx/src/tests/roundtrip.rs b/crates/xlsx/src/tests/roundtrip.rs
index 12bba62ed..b5bba89d8 100644
--- a/crates/xlsx/src/tests/roundtrip.rs
+++ b/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(
+ ""
+ ),
+ "{xml}"
+ );
+ assert!(
+ xml.contains(""),
+ "{xml}"
+ );
+ assert!(xml.contains(""), "{xml}");
+ let (back, _) = read_xlsx(&bytes).unwrap();
+ let rules: Vec = 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]);
+}
From 71c61f7eaa88e0ebff32be89be72279213c63133 Mon Sep 17 00:00:00 2001
From: Alexander Alber <90604107+alexanderalber@users.noreply.github.com>
Date: Sun, 11 Oct 2026 10:12:00 +0200
Subject: [PATCH 2/2] Add alexanderalber to the contributor names
generated with Claude Code
---
contributors/people.toml | 3 +++
1 file changed, 3 insertions(+)
diff --git a/contributors/people.toml b/contributors/people.toml
index 78683ae55..a268022cd 100644
--- a/contributors/people.toml
+++ b/contributors/people.toml
@@ -28,3 +28,6 @@ sync_github_name = false
[people.edurbs]
sync_github_name = true
+
+[people.alexanderalber]
+real_name = "Alexander Alber"