From 706fe7b10c4fa2ab4a168cfd9a11646fb03d1bcf Mon Sep 17 00:00:00 2001 From: Zarvinx Date: Sun, 11 Oct 2026 01:20:03 +0300 Subject: [PATCH 01/17] macOS: use the padded app icon in the Dock --- apps/gridcraft/src/main.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/apps/gridcraft/src/main.rs b/apps/gridcraft/src/main.rs index 3c73a95..dc70a09 100644 --- a/apps/gridcraft/src/main.rs +++ b/apps/gridcraft/src/main.rs @@ -165,6 +165,9 @@ fn services() -> Services { } fn icon() -> Option { + #[cfg(target_os = "macos")] + let bytes = include_bytes!("../../../assets/app-icon/gridcraft-macos-512.png"); + #[cfg(not(target_os = "macos"))] let bytes = include_bytes!("../../../assets/app-icon/hicolor/256x256/apps/ai.storyteller.gridcraft.png"); let img = image::load_from_memory(bytes).ok()?.to_rgba8(); Some(egui::IconData { width: img.width(), height: img.height(), rgba: img.into_raw() }) From a0dc1abce1f5e8fab69430ddbf727ba4d1bd9f84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dejan=20Koci=C4=87?= Date: Sun, 11 Oct 2026 00:35:45 +0200 Subject: [PATCH 02/17] fix: Save toolbar button and saveChanges dialog discard an in-progress cell edit Clicking the Save icon in the toolbar, or the Save button in the "save changes?" dialog, called file.save directly without first committing a cell edit still in progress (e.g. double-click a cell, type a new value, then click Save with the mouse instead of pressing Enter). The pending text in EditState was never flushed into the document, so the saved file still held the pre-edit value, silently. The File > Save menu item already guards against this (app.commit_edit(...) before running the command); this applies the same guard to the toolbar icon and the saveChanges dialog's Save button. --- crates/ui-egui/src/dialogs.rs | 3 ++- crates/ui-egui/src/ribbon.rs | 5 ++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 7dd3acb..8d6c8ad 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -810,7 +810,8 @@ pub fn show(app: &mut SheetApp, ctx: &egui::Context) { if ui.button("Cancel").clicked() { open = false; } - if ui.button("Save").clicked() { + // File operations must include the cell the user is still editing (as the File > Save menu item already does). + if ui.button("Save").clicked() && app.commit_edit(0, 0, false, false) { app.run_or_alert("file.save", json!({})); if app.session.active().is_some_and(|x| !x.is_dirty()) { app.run_or_alert("file.close", json!({"force": true})); diff --git a/crates/ui-egui/src/ribbon.rs b/crates/ui-egui/src/ribbon.rs index 264479a..7838316 100644 --- a/crates/ui-egui/src/ribbon.rs +++ b/crates/ui-egui/src/ribbon.rs @@ -44,7 +44,10 @@ pub fn title_bar(app: &mut SheetApp, ui: &mut Ui) { if icon_button(ui, Icon::Home, t.text_dim, "Home", vec2(26.0, 26.0)).clicked() { app.open_dialog("start", json!({})); } - if icon_button(ui, Icon::Save, t.text_dim, "Save (⌘S)", vec2(26.0, 26.0)).clicked() { + if icon_button(ui, Icon::Save, t.text_dim, "Save (⌘S)", vec2(26.0, 26.0)).clicked() + // File operations must include the cell the user is still editing (as the File > Save menu item already does). + && app.commit_edit(0, 0, false, false) + { app.run_or_alert("file.save", json!({})); } let can_undo = app.session.active().is_some_and(|d| !d.undo.is_empty()); From 2f078962a0aa2d4692d04f6485d4a1aa6327c85e Mon Sep 17 00:00:00 2001 From: pkibiego Date: Sun, 11 Oct 2026 03:39:32 +0300 Subject: [PATCH 03/17] xlsx: bump quick-xml to 0.41 for the namespace-declaration DoS (RUSTSEC-2026-0195) The ODS and XLSB readers parse untrusted package metadata and content through `quick_xml::NsReader`. In 0.38.4 that is RUSTSEC-2026-0195 (CVSS 7.5): the namespace resolver pushes a binding for every `xmlns` / `xmlns:*` declaration on a start tag, with no cap on how many there are, and `NsReader::read_event` reaches that path - not only `read_resolved_event`. None of our caps bound it. MAX_PART (512 MiB), MAX_TOTAL (1 GiB), MAX_ENTRIES, MAX_CELLS, MAX_TEXT and MAX_DEPTH bound part size, entry count, cells, text and nesting, never declarations per tag. Measured with `gridcraft-cli info` on crafted .ods files, before this change: 754 B, 0 declarations -> 8.6 MB, exit 0 24 KB, 1,000,000 -> 52.6 MB, exit 0 187 KB, 8,000,000 -> 367.8 MB, exit 0 770 KB, 33,000,000 -> 1.46 GB, exit 0 About 45 bytes of resident memory per declaration, roughly 1,900x the file size, and no error at all. At MAX_PART a ~1 MB file reaches ~1.9 GB, and every part is held in memory while the package is read. Both readers share the path, so a downloaded .ods or .xlsb was enough. Nothing panicked, so `Session::execute` never saw it. 0.41.0 caps declarations per start tag at 256 (DEFAULT_MAX_DECLARATIONS_PER_ELEMENT, raisable through NamespaceResolver::set_max_declarations_per_element) and returns `NamespaceError::TooManyDeclarations` beyond it. The same fixtures after: 754 B -> 8.6 MB, ok 24 KB -> 19 MB, error 187 KB -> 101.8 MB, error 770 KB -> 397 MB, error with "start tag declares more than 256 namespace bindings". The residual peak is the part itself: MAX_PART still lets one part decompress to 256 MiB, the existing intended bound. API changes for 0.41 (0.x bumps may break; all four are in the xlsx crate): - `resolve_element` and `resolve_attribute` moved off `NsReader` on to the resolver: `reader.resolver_mut().resolve_element(..)`. `attr` and `tag` in ods.rs take `&mut NsReader` for that. - `Attribute::unescape_value` is deprecated in favour of `normalized_value`, which also applies the attribute-value normalization the XML specification requires. - `BytesText::xml_content` now takes the document's XML version. This also clears RUSTSEC-2026-0194, the quadratic duplicate-attribute check. xml.rs already passed `with_checks(false)`, but ods.rs and xlsb.rs used the checked iterator, so they were on the quadratic path as well. Regression test: `ods_tests::ods_namespace_declaration_bomb_is_rejected` asserts that a tag with thousands of declarations is an error and that a document with a handful still reads. It fails on 0.38.4 (the tag is accepted) and passes on 0.41.0. Verified: `cargo xtask ci`; the full xlsx suite (80 tests) plus the malformed-input tests; and the four measurements above, before and after, with no panics either way. --- Cargo.lock | 13 ++----------- Cargo.toml | 2 +- crates/xlsx/src/ods.rs | 23 ++++++++++++----------- crates/xlsx/src/tests/ods_tests.rs | 16 ++++++++++++++++ crates/xlsx/src/xlsb.rs | 7 ++++--- crates/xlsx/src/xml.rs | 5 +++-- 6 files changed, 38 insertions(+), 28 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f772f83..16e2221 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1584,7 +1584,7 @@ dependencies = [ "gridcraft-core", "gridcraft-formula", "gridcraft-model", - "quick-xml 0.38.4", + "quick-xml", "thiserror 2.0.21", "zip", ] @@ -2739,15 +2739,6 @@ version = "2.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a993555f31e5a609f617c12db6250dedcac1b0a85076912c436e6fc9b2c8e6a3" -[[package]] -name = "quick-xml" -version = "0.38.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b66c2058c55a409d601666cffe35f04333cf1013010882cec174a7467cd4e21c" -dependencies = [ - "memchr", -] - [[package]] name = "quick-xml" version = "0.41.0" @@ -3725,7 +3716,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "338e30461b3a2b67d70eb30a6d89f8e0c93a833e07d2ae89085cd070c4a00ac0" dependencies = [ "proc-macro2", - "quick-xml 0.41.0", + "quick-xml", "quote", ] diff --git a/Cargo.toml b/Cargo.toml index d7fad0f..d826390 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,7 +32,7 @@ serde_json = { version = "1", features = ["float_roundtrip"] } log = "0.4" rayon = "1" regex = { version = "1", default-features = false, features = ["std", "unicode", "perf"] } -quick-xml = "0.38" +quick-xml = "0.41" zip = { version = "3", default-features = false, features = ["deflate"] } image = { version = "0.25", default-features = false, features = ["png", "jpeg"] } diff --git a/crates/xlsx/src/ods.rs b/crates/xlsx/src/ods.rs index 4f63fd1..8415ce3 100644 --- a/crates/xlsx/src/ods.rs +++ b/crates/xlsx/src/ods.rs @@ -10,6 +10,7 @@ use gridcraft_core::date::{days_in_month, serial_from_ymd}; use gridcraft_core::{CellRef, DateSystem, MAX_COLS, MAX_ROWS, RangeRef, Value}; use gridcraft_model::{Cell, NumFmt, Sheet, StyleId, Workbook}; use quick_xml::NsReader; +use quick_xml::XmlVersion; use quick_xml::events::{BytesStart, Event}; use quick_xml::name::ResolveResult; @@ -67,13 +68,13 @@ fn is_ns(ns: ResolveResult<'_>, expected: &[u8]) -> bool { matches!(ns, ResolveResult::Bound(n) if n.as_ref() == expected) } -fn attr(reader: &NsReader<&[u8]>, e: &BytesStart<'_>, ns: &[u8], name: &[u8]) -> Result, IoError> { +fn attr(reader: &mut NsReader<&[u8]>, e: &BytesStart<'_>, ns: &[u8], name: &[u8]) -> Result, IoError> { let mut found = None; for a in e.attributes() { let a = a.map_err(|e| IoError::Xml(e.to_string()))?; - let (resolved, local) = reader.resolve_attribute(a.key); + let (resolved, local) = reader.resolver_mut().resolve_attribute(a.key); if is_ns(resolved, ns) && local.as_ref() == name { - let value = a.unescape_value().map_err(|e| IoError::Xml(e.to_string()))?; + let value = a.normalized_value(XmlVersion::Explicit1_0).map_err(|e| IoError::Xml(e.to_string()))?; if value.len() > MAX_CELL_TEXT { return Err(IoError::TooLarge("ODS attribute exceeds 1 MiB".into())); } @@ -106,7 +107,7 @@ fn check_manifest(bytes: &[u8]) -> Result<(), IoError> { let event = reader.read_event().map_err(xml_error)?; match &event { Event::Start(e) | Event::Empty(e) => { - let (ns, name) = reader.resolve_element(e.name()); + let (ns, name) = reader.resolver_mut().resolve_element(e.name()); if is_ns(ns.clone(), MANIFEST) && name.as_ref() == b"encryption-data" { return Err(IoError::Format("encrypted ODS files are not supported".into())); } @@ -153,8 +154,8 @@ enum Tag { Other, } -fn tag(reader: &NsReader<&[u8]>, e: &BytesStart<'_>) -> Tag { - let (ns, name) = reader.resolve_element(e.name()); +fn tag(reader: &mut NsReader<&[u8]>, e: &BytesStart<'_>) -> Tag { + let (ns, name) = reader.resolver_mut().resolve_element(e.name()); let local = name.as_ref(); if is_ns(ns.clone(), OFFICE) { return match local { @@ -253,23 +254,23 @@ impl Import { return Err(xml_error("XML nested too deeply")); } if self.stack.is_empty() { - if root_seen || tag(&reader, &e) != Tag::Document { + if root_seen || tag(&mut reader, &e) != Tag::Document { return Err(xml_error("expected one office:document-content root")); } root_seen = true; } - self.start(&reader, &e)?; + self.start(&mut reader, &e)?; } Event::Empty(e) => { if self.stack.is_empty() { return Err(xml_error("expected office:document-content with a spreadsheet")); } - self.start(&reader, &e)?; + self.start(&mut reader, &e)?; self.end()?; } Event::End(_) => self.end()?, Event::Text(t) => { - let text = t.xml_content().map_err(xml_error)?; + let text = t.xml_content(XmlVersion::Explicit1_0).map_err(xml_error)?; if self.stack.is_empty() && !text.trim().is_empty() { return Err(xml_error("text outside the document")); } @@ -301,7 +302,7 @@ impl Import { Ok(()) } - fn start(&mut self, reader: &NsReader<&[u8]>, e: &BytesStart<'_>) -> Result<(), IoError> { + fn start(&mut self, reader: &mut NsReader<&[u8]>, e: &BytesStart<'_>) -> Result<(), IoError> { let t = tag(reader, e); let depth = self.stack.len(); match t { diff --git a/crates/xlsx/src/tests/ods_tests.rs b/crates/xlsx/src/tests/ods_tests.rs index c0f2e0e..47c2586 100644 --- a/crates/xlsx/src/tests/ods_tests.rs +++ b/crates/xlsx/src/tests/ods_tests.rs @@ -23,6 +23,22 @@ fn text(s: &str) -> Value { Value::Text(s.into()) } +/// A start tag with thousands of `xmlns:` declarations is refused instead of allocated +/// (RUSTSEC-2026-0195: the resolver kept a binding per declaration, with no cap). +#[test] +fn ods_namespace_declaration_bomb_is_rejected() { + let bomb = |n: usize| -> String { (0..n).map(|i| format!(" xmlns:p{i}=\"urn:p{i}\"")).collect() }; + let doc = |decls: &str| { + format!( + r#"1"# + ) + }; + let build = |decls: &str| make_zip(&[("mimetype", MIME), ("META-INF/manifest.xml", MANIFEST), ("content.xml", doc(decls).as_bytes())]); + let ok = read_ods(&build(&bomb(8))); + assert!(ok.is_ok(), "a handful of namespaces still reads: {:?}", ok.err()); + assert!(read_ods(&build(&bomb(10_000))).is_err(), "a tag with thousands of declarations is refused"); +} + #[test] fn ods_typed_values_and_formula_caches_are_constants_and_survive_xlsx() { let bytes = row(r#" diff --git a/crates/xlsx/src/xlsb.rs b/crates/xlsx/src/xlsb.rs index 897a40b..3c84547 100644 --- a/crates/xlsx/src/xlsb.rs +++ b/crates/xlsx/src/xlsb.rs @@ -7,6 +7,7 @@ use std::sync::Arc; use gridcraft_core::{CellError, CellRef, DateSystem, MAX_COLS, MAX_ROWS, RangeRef, Value}; use gridcraft_model::{Cell, Sheet, Visibility, Workbook}; use quick_xml::NsReader; +use quick_xml::XmlVersion; use quick_xml::events::Event; use quick_xml::name::ResolveResult; @@ -160,7 +161,7 @@ fn metadata(bytes: &[u8], ns: &[u8], root_name: &[u8]) -> Result, IoE let event = reader.read_event().map_err(|e| IoError::Xml(e.to_string()))?; match &event { Event::Start(e) | Event::Empty(e) => { - let (namespace, local) = reader.resolve_element(e.name()); + let (namespace, local) = reader.resolver_mut().resolve_element(e.name()); let valid_ns = matches!(namespace, ResolveResult::Bound(n) if n.as_ref() == ns); if depth == 0 { if seen || !valid_ns || local.as_ref() != root_name { @@ -174,7 +175,7 @@ fn metadata(bytes: &[u8], ns: &[u8], root_name: &[u8]) -> Result, IoE if !a.key.as_ref().contains(&b':') { attrs.push(( String::from_utf8_lossy(a.key.as_ref()).into_owned(), - a.unescape_value().map_err(|e| IoError::Xml(e.to_string()))?.into_owned(), + a.normalized_value(XmlVersion::Explicit1_0).map_err(|e| IoError::Xml(e.to_string()))?.into_owned(), )); } } @@ -188,7 +189,7 @@ fn metadata(bytes: &[u8], ns: &[u8], root_name: &[u8]) -> Result, IoE } } Event::End(_) => depth = depth.checked_sub(1).ok_or_else(|| invalid("unexpected metadata end"))?, - Event::Text(t) if depth == 0 && !t.xml_content().map_err(|e| IoError::Xml(e.to_string()))?.trim().is_empty() => { + Event::Text(t) if depth == 0 && !t.xml_content(XmlVersion::Explicit1_0).map_err(|e| IoError::Xml(e.to_string()))?.trim().is_empty() => { return Err(invalid("text outside package metadata")); } Event::DocType(_) => return Err(invalid("DOCTYPE in package metadata is unsupported")), diff --git a/crates/xlsx/src/xml.rs b/crates/xlsx/src/xml.rs index efa5cdc..9e03f5f 100644 --- a/crates/xlsx/src/xml.rs +++ b/crates/xlsx/src/xml.rs @@ -2,6 +2,7 @@ //! shared strings) so large parts never need a full tree, plus escaping helpers for writing. use quick_xml::Reader; +use quick_xml::XmlVersion; use quick_xml::events::{BytesStart, Event}; use crate::IoError; @@ -124,7 +125,7 @@ fn start_el(e: &BytesStart<'_>) -> El { if key == b"xmlns" || key.starts_with(b"xmlns:") { continue; } - let v = match a.unescape_value() { + let v = match a.normalized_value(XmlVersion::Explicit1_0) { Ok(v) => v.into_owned(), Err(_) => String::from_utf8_lossy(&a.value).into_owned(), }; @@ -203,7 +204,7 @@ pub fn parse_streaming(xml: &[u8], split: &[&str], cb: &mut dyn FnMut(El) -> Res } Event::Text(t) => { if let Some(top) = stack.last_mut() { - let s = t.xml_content().map_err(|e| IoError::Xml(e.to_string()))?; + let s = t.xml_content(XmlVersion::Explicit1_0).map_err(|e| IoError::Xml(e.to_string()))?; top.text.push_str(&s); } } From fae90c8e4aece9de93547c22c2a8364a7b6e11eb Mon Sep 17 00:00:00 2001 From: webdevtodayjason <115739170+webdevtodayjason@users.noreply.github.com> Date: Sun, 11 Oct 2026 01:37:50 +0000 Subject: [PATCH 04/17] selection.move: clamp extreme dr/dc to the sheet edge instead of overflowing --- crates/core/src/addr.rs | 23 +++++++++-- crates/engine/src/cmd/edit.rs | 6 +-- .../engine/tests/extreme_selection_moves.rs | 40 +++++++++++++++++++ 3 files changed, 62 insertions(+), 7 deletions(-) create mode 100644 crates/engine/tests/extreme_selection_moves.rs diff --git a/crates/core/src/addr.rs b/crates/core/src/addr.rs index 8a64910..2bdfe79 100644 --- a/crates/core/src/addr.rs +++ b/crates/core/src/addr.rs @@ -30,14 +30,14 @@ impl CellRef { } /// Moved by `(dr, dc)`, `None` when the result leaves the sheet. pub fn offset(&self, dr: i64, dc: i64) -> Option { - let r = self.row as i64 + dr; - let c = self.col as i64 + dc; + let r = (self.row as i64).checked_add(dr)?; + let c = (self.col as i64).checked_add(dc)?; if (0..MAX_ROWS as i64).contains(&r) && (0..MAX_COLS as i64).contains(&c) { Some(CellRef::new(r as u32, c as u32)) } else { None } } /// Moved by `(dr, dc)` and clamped to the sheet. pub fn offset_clamped(&self, dr: i64, dc: i64) -> CellRef { - let r = (self.row as i64 + dr).clamp(0, MAX_ROWS as i64 - 1); - let c = (self.col as i64 + dc).clamp(0, MAX_COLS as i64 - 1); + let r = (self.row as i64).saturating_add(dr).clamp(0, MAX_ROWS as i64 - 1); + let c = (self.col as i64).saturating_add(dc).clamp(0, MAX_COLS as i64 - 1); CellRef::new(r as u32, c as u32) } /// Parses `A1`, `$A$1`, `xfd1048576` (case-insensitive). `None` if malformed or out of range. @@ -272,6 +272,21 @@ mod tests { assert_eq!(a.iter().count(), 9); } + #[test] + fn extreme_offsets() { + let cell = CellRef::new(1, 1); + for delta in [i64::MIN, i64::MAX] { + assert_eq!(cell.offset(delta, 0), None); + assert_eq!(cell.offset(0, delta), None); + assert_eq!(cell.offset(delta, delta), None); + } + assert_eq!(cell.offset_clamped(i64::MAX, i64::MAX), CellRef::new(MAX_ROWS - 1, MAX_COLS - 1)); + assert_eq!(cell.offset_clamped(i64::MIN, i64::MIN), CellRef::new(0, 0)); + assert_eq!(cell.offset_clamped(i64::MAX, i64::MIN), CellRef::new(MAX_ROWS - 1, 0)); + assert_eq!(cell.offset_clamped(i64::MIN, i64::MAX), CellRef::new(0, MAX_COLS - 1)); + assert_eq!(cell.offset(2, -1), Some(CellRef::new(3, 0))); + } + #[test] fn offsets() { assert_eq!(CellRef::new(0, 0).offset(-1, 0), None); diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index bb54fe0..2dba66f 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -1,6 +1,6 @@ //! Cell entry, selection, undo/redo, clipboard, clear, fill, find & replace. -use gridcraft_core::{CellRef, MAX_COLS, MAX_ROWS, RangeRef, Value}; +use gridcraft_core::{CellRef, MAX_ROWS, RangeRef, Value}; use gridcraft_model::{Cell, Formula, StyleId}; use serde_json::{Value as Json, json}; @@ -408,9 +408,9 @@ fn selection_move(s: &mut Session, p: &Json) -> Result { if !extend && let Some(m) = sheet.merge_at(from) { // Leaving a merged cell moves from its edge. if dr > 0 { - t = CellRef::new((m.end.row + dr as u32).min(MAX_ROWS - 1), from.col); + t = CellRef::new(m.end.offset_clamped(dr, 0).row, from.col); } else if dc > 0 { - t = CellRef::new(from.row, (m.end.col + dc as u32).min(MAX_COLS - 1)); + t = CellRef::new(from.row, m.end.offset_clamped(0, dc).col); } } t diff --git a/crates/engine/tests/extreme_selection_moves.rs b/crates/engine/tests/extreme_selection_moves.rs new file mode 100644 index 0000000..f6f5737 --- /dev/null +++ b/crates/engine/tests/extreme_selection_moves.rs @@ -0,0 +1,40 @@ +use gridcraft_core::{CellRef, MAX_COLS, MAX_ROWS}; +use gridcraft_engine::Session; +use serde_json::json; + +#[test] +fn selection_move_clamps_extreme_displacements() { + let mut session = Session::new(); + session.new_workbook(); + for (dr, dc, expected) in [ + (i64::MAX, 0, CellRef::new(MAX_ROWS - 1, 1)), + (0, i64::MAX, CellRef::new(1, MAX_COLS - 1)), + (i64::MAX, i64::MAX, CellRef::new(MAX_ROWS - 1, MAX_COLS - 1)), + (i64::MIN, i64::MIN, CellRef::new(0, 0)), + (i64::MAX, i64::MIN, CellRef::new(MAX_ROWS - 1, 0)), + (i64::MIN, i64::MAX, CellRef::new(0, MAX_COLS - 1)), + (2, -1, CellRef::new(3, 0)), + ] { + session.execute("selection.set", json!({"cell": "B2"})).unwrap(); + session.execute("selection.move", json!({"dr": dr, "dc": dc})).unwrap(); + assert_eq!(session.doc().unwrap().selection.active, expected); + } +} + +#[test] +fn selection_move_from_a_merged_cell_clamps_extreme_displacements() { + let mut session = Session::new(); + session.new_workbook(); + session.execute("selection.set", json!({"range": "B2:C3"})).unwrap(); + session.execute("home.mergeCells", json!({})).unwrap(); + for (dr, dc, expected) in [ + (i64::MAX, 0, CellRef::new(MAX_ROWS - 1, 1)), + (0, i64::MAX, CellRef::new(1, MAX_COLS - 1)), + (4_294_967_297, 0, CellRef::new(MAX_ROWS - 1, 1)), + (1, 0, CellRef::new(3, 1)), + ] { + session.execute("selection.set", json!({"cell": "B2"})).unwrap(); + session.execute("selection.move", json!({"dr": dr, "dc": dc})).unwrap(); + assert_eq!(session.doc().unwrap().selection.active, expected); + } +} From aee898595f49426ffdcae86708171a3dfbc4a7e8 Mon Sep 17 00:00:00 2001 From: Sebastian Whincop Date: Sun, 11 Oct 2026 16:22:36 +1300 Subject: [PATCH 05/17] Accuracy: round an exact tie at the 16th significant digit toward zero MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Excel shows 15 significant digits. When a number is an exact tie at the 16th (its exact decimal expansion is 15 digits, a 5, then only zeros), Excel rounds it toward zero: TEXT(1234567890123455,"0") is 1234567890123450. GridCraft took Rust's `{:.14e}`, which rounds that tie to even, so any tie with an odd 15th digit came out one unit too high (…460). decimal_digits now formats 16 digits (correct unless the 16th is a 5), and only on a 5 checks the exact expansion (a 30-digit screen, then the full one) and truncates a true tie. Non-ties, including values just above or below a tie, round as before. One format serves almost every number. Tests: ties of every 15th-digit parity, negatives, fractional ties, the neighbours just above and below a tie, the carry after fifteen 9s (999999999999999.5 shows 999999999999999), and unchanged ordinary cases. Evidence: Windows Excel 16.0.20430 TEXT() over a 174-row probe and a 24-row carry probe agree on every row except those under a percent format and General over negatives (the two bugs named below). The suite's number-format golden goes from 183,843 to 184,900 of 187,810 passing with no row that passed before failing now. modified: crates/numfmt/src/decimal.rs modified: crates/numfmt/src/tests.rs --- crates/numfmt/src/decimal.rs | 63 +++++++++++++++++++++++++++++--- crates/numfmt/src/tests.rs | 70 ++++++++++++++++++++++++++++++++++++ 2 files changed, 128 insertions(+), 5 deletions(-) diff --git a/crates/numfmt/src/decimal.rs b/crates/numfmt/src/decimal.rs index 15cfcd3..67071d4 100644 --- a/crates/numfmt/src/decimal.rs +++ b/crates/numfmt/src/decimal.rs @@ -9,17 +9,70 @@ pub(crate) const MAX_DECIMALS: usize = 400; /// The 15 significant digits of a positive finite number and the power of ten of the first /// digit: `a ≈ d0.d1d2… × 10^exp`. +/// +/// The value is rounded to the nearest 15-digit decimal. An exact tie at the 16th digit (the exact +/// decimal expansion is 15 digits, a 5, then only zeros, as for 1234567890123455 or +/// 123456789012345.5) goes toward zero, as Excel does: `1234567890123455` shows `1234567890123450`, +/// not `…460`. Excel observed (Windows 16.0.20430, TEXT()). pub(crate) fn decimal_digits(a: f64) -> ([u8; 15], i32) { let mut digits = [0u8; 15]; if !a.is_finite() || a <= 0.0 { return (digits, 0); } - let s = format!("{:.14e}", a); - let (mant, exp) = s.split_once('e').unwrap_or((s.as_str(), "0")); - for (slot, b) in digits.iter_mut().zip(mant.bytes().filter(u8::is_ascii_digit)) { - *slot = b - b'0'; + // 16 digits, rounded. Unless the 16th is a 5 it decides the 15th exactly (4 or less rounds + // down, 6 or more up), so one format serves almost every number. + let (d16, exp) = significant_digits(a, 16); + for (slot, d) in digits.iter_mut().zip(&d16) { + *slot = *d; } - (digits, exp.parse().unwrap_or(0)) + match d16.get(15).copied().unwrap_or(0) { + 0..=4 => (digits, exp), + 5 if is_exact_tie(a) => (digits, exp), + 5 => { + // Not a tie: round the exact value (the 16-digit rounding above may be a double rounding). + let (d15, exp15) = significant_digits(a, 15); + let mut out = [0u8; 15]; + for (slot, d) in out.iter_mut().zip(&d15) { + *slot = *d; + } + (out, exp15) + } + _ => { + let mut exp = exp; + let mut carry = true; + for d in digits.iter_mut().rev() { + if *d == 9 { + *d = 0; + } else { + *d += 1; + carry = false; + break; + } + } + if carry { + digits = [0; 15]; + digits[0] = 1; + exp += 1; + } + (digits, exp) + } + } +} + +/// `n` significant digits of `a` (rounded half to even on the exact binary value) and the power of +/// ten of the first. +fn significant_digits(a: f64, n: usize) -> (Vec, i32) { + let s = format!("{:.*e}", n.saturating_sub(1), a); + let (mant, exp) = s.split_once('e').unwrap_or((s.as_str(), "0")); + (mant.bytes().filter(u8::is_ascii_digit).map(|b| b - b'0').collect(), exp.parse().unwrap_or(0)) +} + +/// True when `a`'s exact decimal expansion has a 5 as its 16th significant digit and only zeros after. +fn is_exact_tie(a: f64) -> bool { + let is_tie = |ds: &[u8]| ds.get(15) == Some(&5) && ds.get(16..).is_some_and(|t| t.iter().all(|&d| d == 0)); + // A cheap screen first (30 digits), then the exact expansion: a double has at most 767 + // significant digits and Rust prints every one of them. + is_tie(&significant_digits(a, 30).0) && is_tie(&significant_digits(a, 800).0) } /// Rounds the number `digits × 10^exp` (see [`decimal_digits`]) to `decimals` places, half away diff --git a/crates/numfmt/src/tests.rs b/crates/numfmt/src/tests.rs index ea40753..0742b26 100644 --- a/crates/numfmt/src/tests.rs +++ b/crates/numfmt/src/tests.rs @@ -421,3 +421,73 @@ fn fuzz_no_panic() { } } } + +// ---- 15 significant digits: an exact tie at the 16th digit goes toward zero ---- +// Windows Excel 16.0.20430 TEXT() over the exact doubles below. An odd 15th digit is where +// round-half-even and toward-zero differ; an even one is where they agree. + +#[test] +fn an_exact_tie_at_the_sixteenth_digit_goes_toward_zero() { + for (v, whole) in [ + (1234567890123405.0, "1234567890123400"), + (1234567890123415.0, "1234567890123410"), + (1234567890123425.0, "1234567890123420"), + (1234567890123435.0, "1234567890123430"), + (1234567890123455.0, "1234567890123450"), + (1234567890123475.0, "1234567890123470"), + (1234567890123495.0, "1234567890123490"), + (6631679258820775.0, "6631679258820770"), + (8052991635850695.0, "8052991635850690"), + ] { + assert_eq!(f("0", v), whole, "0 over {v}"); + assert_eq!(f("0.00", v), format!("{whole}.00"), "0.00 over {v}"); + assert_eq!(f("0", -v), format!("-{whole}"), "0 over -{v}"); + } + assert_eq!(f("#,##0", 1234567890123455.0), "1,234,567,890,123,450"); + assert_eq!(f("#,##0", -3267527853107975.0), "-3,267,527,853,107,970"); + // A fraction is a tie as well, when it is exact in binary. + assert_eq!(f("0", 123456789012345.5), "123456789012345"); + assert_eq!(f("0.00", 123456789012345.5), "123456789012345.00"); + // Excel observed: no carry after fifteen 9s. + assert_eq!(f("0", 999999999999999.5), "999999999999999"); + assert_eq!(f("0", -999999999999999.5), "-999999999999999"); + assert_eq!(f("0", 4999999999999995.0), "4999999999999990"); + assert_eq!(f("#,##0", -4999999999999995.0), "-4,999,999,999,999,990"); +} + +#[test] +fn only_exact_ties_go_toward_zero() { + // Just above and just below a tie round to the nearer 15-digit value (Excel observed). + assert_eq!(f("0", 1234567890123455.0 + 0.25), "1234567890123460"); + assert_eq!(f("0", 1234567890123455.5), "1234567890123460"); + assert_eq!(f("0", 1234567890123455.0 - 0.25), "1234567890123450"); + // 16 digits that are not a tie, and the ordinary decimal cases, are untouched. + assert_eq!(f("0", 1234567890123456.0), "1234567890123460"); + assert_eq!(f("0", 1234567890123454.0), "1234567890123450"); + assert_eq!(f("0.00", 2.675), "2.68"); + assert_eq!(f("0.00", 1.005), "1.01"); + assert_eq!(f("0.0", 0.285), "0.3"); + assert_eq!(f("0.0", 99999999999999.95), "100000000000000.0"); + // Ten digits and a half are not a 16th-digit tie. + assert_eq!(f("0", 0.5), "1"); + assert_eq!(f("0", 2.5), "3"); +} + +#[test] +fn decimal_digits_keeps_fifteen_digits_and_truncates_a_tie() { + use crate::decimal::decimal_digits; + let (d, e) = decimal_digits(1234567890123455.0); + assert_eq!((d, e), ([1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5], 15)); + let (d, e) = decimal_digits(1234567890123456.0); + assert_eq!((d, e), ([1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 6], 15)); + let (d, e) = decimal_digits(123456789012345.5); + assert_eq!((d, e), ([1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5], 14)); + // Excel observed: a tie after fifteen 9s does not carry (999999999999999.5 shows 999999999999999). + let (d, e) = decimal_digits(999999999999999.5); + assert_eq!((d, e), ([9; 15], 14)); + let (d, e) = decimal_digits(4999999999999995.0); + assert_eq!((d, e), ([4, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9], 15)); + // A 16th digit above 5 carries as usual. + let (d, e) = decimal_digits(999999999999999.6); + assert_eq!((d, e), ([1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0], 15)); +} From d75b6b73b43804de83a5baa3ac9030fff00707fd Mon Sep 17 00:00:00 2001 From: nihilist-narwhal <240798405+nihilist-narwhal@users.noreply.github.com> Date: Sat, 10 Oct 2026 00:56:12 -0500 Subject: [PATCH 06/17] Criteria: text patterns don't match blank cells COUNTIF(range,"*") counted empty cells and COUNTIF(range,"<>*") skipped them. Excel matches text patterns against text only: an empty string is text, a blank cell is not. --- crates/functions/src/criteria.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/crates/functions/src/criteria.rs b/crates/functions/src/criteria.rs index 49d84a3..20410c8 100644 --- a/crates/functions/src/criteria.rs +++ b/crates/functions/src/criteria.rs @@ -127,9 +127,9 @@ impl Criterion { }, Operand::Text(pat) => match self.op { Op::Eq | Op::Ne => { + // Blank cells hold no text: `"*"` skips them and `"<>*"` counts them. let cell_text = match &v { Value::Text(t) => Some(t.to_string()), - Value::Empty => Some(String::new()), _ => None, }; let m = cell_text.is_some_and(|t| wildcard_match(pat, &t)); @@ -298,6 +298,13 @@ mod tests { assert!(lookup_equal(&Value::Number(0.3), &Value::Number(0.1 + 0.2), false)); assert!(m("<>a*", Value::from("bcd"))); assert!(m(">1/1/2020", Value::Number(44000.0))); + // Text patterns never match blank cells, but an empty string is text. + assert!(!m("*", Value::Empty)); + assert!(m("*", Value::from(""))); + assert!(m("<>*", Value::Empty)); + assert!(!m("<>*", Value::from(""))); + assert!(!m("?*", Value::Empty)); + assert!(m("<>a*", Value::Empty)); } #[test] From 060889afb21b3aaaab139c6ed51b1ba87bb9828b Mon Sep 17 00:00:00 2001 From: nihilist-narwhal <240798405+nihilist-narwhal@users.noreply.github.com> Date: Sat, 10 Oct 2026 00:57:28 -0500 Subject: [PATCH 07/17] CHOOSE: pick element by element when the index is an array The evaluator took a single index, so CHOOSE({1,2},"a","b") gave "a" and VLOOKUP(x,CHOOSE({1,2},B1:B3,A1:A3),2,FALSE) gave #REF!. An array index now goes through the library CHOOSE, which lifts over its arguments as Excel does. --- crates/calc/src/eval.rs | 15 ++++++++++++++- crates/calc/src/tests.rs | 19 +++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/crates/calc/src/eval.rs b/crates/calc/src/eval.rs index eff659d..2c0dbeb 100644 --- a/crates/calc/src/eval.rs +++ b/crates/calc/src/eval.rs @@ -554,7 +554,20 @@ impl<'h> Evaluator<'h> { if args.len() < 2 { return err(CellError::Value); } - let i = match self.arg_num(args, 0, 0.0) { + let index = self.value(&args[0]); + if let Value::Array(_) = index { + // An array of indexes picks element by element: `CHOOSE({1,2},A1:A3,B1:B3)` + // is a two-column array. The library CHOOSE lifts over its arguments. + let Some(spec) = gridcraft_functions::lookup(name) else { return err(CellError::Name) }; + let mut evaluated = vec![Arg { value: index, from_ref: false }]; + for a in &args[1..] { + let value = self.value(a); + evaluated.push(Arg { value, from_ref: false }); + } + let mut ctx = FnCtx { ev: self }; + return gridcraft_functions::call(spec, &evaluated, &mut ctx).pipe(Ev::V); + } + let i = match index.to_number() { Ok(n) => n.trunc() as i64, Err(e) => return err(e), }; diff --git a/crates/calc/src/tests.rs b/crates/calc/src/tests.rs index d1e8576..a7f1faa 100644 --- a/crates/calc/src/tests.rs +++ b/crates/calc/src/tests.rs @@ -219,6 +219,25 @@ fn ifs_and_lazy() { assert_eq!(t.get("B4"), Value::from("five")); } +#[test] +fn choose_with_an_array_index() { + let mut t = T::new(); + for (i, (k, v)) in [(1, 10), (2, 20), (3, 30)].iter().enumerate() { + t.set(&format!("A{}", i + 1), &k.to_string()); + t.set(&format!("B{}", i + 1), &v.to_string()); + } + t.set("D1", "=CHOOSE({1,2},\"a\",\"b\")"); + assert_eq!(t.get("D1"), Value::from("a")); + assert_eq!(t.get("E1"), Value::from("b")); + // The usual "look left" idiom: a two-column array from two ranges. + t.set("D3", "=VLOOKUP(20,CHOOSE({1,2},B1:B3,A1:A3),2,FALSE)"); + assert_eq!(t.num("D3"), 2.0); + t.set("D4", "=SUM(CHOOSE({1,2},A1:A3,B1:B3))"); + assert_eq!(t.num("D4"), 66.0); + t.set("D5", "=CHOOSE(2,A1:A3,B1:B3)"); + assert_eq!(t.num("D7"), 30.0); +} + #[test] fn references_functions() { let mut t = T::new(); From 5e22f9b373f32c3af7cee5f0f339482c207ed6a6 Mon Sep 17 00:00:00 2001 From: nihilist-narwhal <240798405+nihilist-narwhal@users.noreply.github.com> Date: Sat, 10 Oct 2026 03:40:56 -0500 Subject: [PATCH 08/17] Sort: case-sensitive order puts lowercase before uppercase With Case sensitive checked, Sort compared text by code point, so every uppercase word came before every lowercase one (A, B, a, b). Excel compares ignoring case first and only then puts lowercase before uppercase: a, A, b, B. --- crates/engine/src/cmd/data.rs | 8 +++++++- crates/engine/src/tests.rs | 9 +++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/cmd/data.rs b/crates/engine/src/cmd/data.rs index fd50afb..9473307 100644 --- a/crates/engine/src/cmd/data.rs +++ b/crates/engine/src/cmd/data.rs @@ -248,7 +248,7 @@ fn do_sort(s: &mut Session, r: RangeRef, header: bool, keys: Vec, by_co let pos = |v: &Value| v.as_text().and_then(|t| list.iter().position(|x| x.eq_ignore_ascii_case(t))).unwrap_or(usize::MAX); pos(va).cmp(&pos(vb)) } else if case && va.is_text() && vb.is_text() { - va.as_text().unwrap_or("").cmp(vb.as_text().unwrap_or("")) + sort_compare(va, vb).then_with(|| case_order(va.as_text().unwrap_or(""), vb.as_text().unwrap_or(""))) } else { sort_compare(va, vb) }; @@ -653,6 +653,12 @@ pub fn filter_values(wb: &Workbook, sheet: usize, col: u32) -> Vec<(String, bool seen.into_iter().map(|((_, t), on)| (t, on)).collect() } +/// Case-sensitive tie-break for text that is equal ignoring case: as in Excel, lowercase sorts +/// before uppercase at the first letter that differs in case. +fn case_order(a: &str, b: &str) -> Ordering { + a.chars().zip(b.chars()).find(|(x, y)| x != y).map_or(Ordering::Equal, |(x, y)| y.is_lowercase().cmp(&x.is_lowercase())) +} + fn sort_key(v: &Value) -> (u8, i64, String) { match v { Value::Number(n) => (0, (n * 1e6) as i64, String::new()), diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 6bd03d5..92ca386 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -606,6 +606,15 @@ fn hidden_formulas_stay_hidden_on_protected_sheets() { assert_eq!(r["results"][0]["value"], "7"); } +#[test] +fn case_sensitive_sort_puts_lowercase_first() { + let mut s = s(); + s.execute("range.setValues", json!({"range": "A1", "values": [["b"], ["B"], ["Ab"], ["a"], ["aB"], ["A"]]})).unwrap(); + s.execute("data.sort", json!({"range": "A1:A6", "header": false, "matchCase": true, "keys": [{"column": "A"}]})).unwrap(); + let col: Vec = (1..=6).map(|r| v(&s, &format!("A{r}"))).collect(); + assert_eq!(col, ["a", "A", "aB", "Ab", "b", "B"].map(Value::from)); +} + #[test] fn formatting_and_styles() { let mut s = s(); From ce83871f2b8903472541ef7095bc80e6222e7fcb Mon Sep 17 00:00:00 2001 From: nihilist-narwhal <240798405+nihilist-narwhal@users.noreply.github.com> Date: Sat, 10 Oct 2026 03:40:56 -0500 Subject: [PATCH 09/17] Remove Duplicates: compare the text cells show Excel decides duplicates by what a cell displays, not the value stored: the same date shown as 3/8/2006 and as Mar 8, 2006 is two values, while 1.01 and 1.02 both shown as 1.0 are duplicates. Remove Duplicates compared stored values, so it did the opposite in both cases. --- crates/engine/src/cmd/data.rs | 5 ++++- crates/engine/src/tests.rs | 12 ++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/cmd/data.rs b/crates/engine/src/cmd/data.rs index 9473307..7372380 100644 --- a/crates/engine/src/cmd/data.rs +++ b/crates/engine/src/cmd/data.rs @@ -691,8 +691,11 @@ fn remove_duplicates(s: &mut Session, p: &Json) -> Result { let Some(sh) = cx.wb.sheet(sheet) else { return Ok(()) }; let mut seen = std::collections::HashSet::new(); let mut keep: Vec = Vec::new(); + // Cells are compared by the text they show, as in Excel: the same date in two formats + // isn't a duplicate, while 1.01 and 1.02 shown as 1.0 are. + let shown = |c: CellRef| crate::display::format(&sh.value(c), cx.wb.styles.get(sh.style_id(c)).num_fmt.as_str(), &cx.wb).text; for row in start..=r.end.row { - let key: Vec = cols.iter().map(|c| sh.value(CellRef::new(row, *c)).display().to_lowercase()).collect(); + let key: Vec = cols.iter().map(|c| shown(CellRef::new(row, *c)).to_lowercase()).collect(); if seen.insert(key) { keep.push(row); } else { diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 92ca386..139a249 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -615,6 +615,18 @@ fn case_sensitive_sort_puts_lowercase_first() { assert_eq!(col, ["a", "A", "aB", "Ab", "b", "B"].map(Value::from)); } +#[test] +fn remove_duplicates_compares_shown_text() { + let mut s = s(); + s.execute("range.setValues", json!({"range": "A1", "values": [["Date", "Price"], ["2006-03-08", 1.01], ["2006-03-08", 1.02]]})).unwrap(); + s.execute("home.numberFormat", json!({"range": "A3", "code": "mmm d, yyyy"})).unwrap(); + s.execute("home.numberFormat", json!({"range": "B2:B3", "code": "0.0"})).unwrap(); + let r = s.execute("data.removeDuplicates", json!({"range": "A1:A3", "header": true})).unwrap(); + assert_eq!(r["removed"], 0); + let r = s.execute("data.removeDuplicates", json!({"range": "B1:B3", "header": true})).unwrap(); + assert_eq!(r["removed"], 1); +} + #[test] fn formatting_and_styles() { let mut s = s(); From cf1696aff38c25010d4578bcbec3c01bc53dd1ed Mon Sep 17 00:00:00 2001 From: Sebastian Whincop Date: Sun, 11 Oct 2026 17:36:25 +1300 Subject: [PATCH 10/17] Add MacJedi42 to the contributor names Adds my own entry to contributors/people.toml, as docs/contributors.md asks: my GitHub username with `sync_github_name = true`. --- contributors/people.toml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/contributors/people.toml b/contributors/people.toml index 78683ae..c7f86da 100644 --- a/contributors/people.toml +++ b/contributors/people.toml @@ -28,3 +28,6 @@ sync_github_name = false [people.edurbs] sync_github_name = true + +[people.MacJedi42] +sync_github_name = true 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 11/17] 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 a102863..25fa49f 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 6bd03d5..c2cce25 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 b9fe2ce..2d2518d 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 b1e3fca..313839f 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 12bba62..b5bba89 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 92adfea27ce388f668f227f473288f8a6597dee2 Mon Sep 17 00:00:00 2001 From: Alexander Alber <90604107+alexanderalber@users.noreply.github.com> Date: Sun, 11 Oct 2026 10:04:09 +0200 Subject: [PATCH 12/17] XLSX: write `@` as `_xlfn.SINGLE` in a dynamic-array formula (#64) The file format has no `@` operator, but formulas with an explicit implicit intersection were written with a literal `@` (`@A1:A3*2`). Excel writes `@x` as `_xlfn.SINGLE(x)`, which only dynamic-array formulas carry. The writer now does the same and marks such formulas cm="1"; the parser reads `SINGLE(x)` back as `@x`, as it already does for `ANCHORARRAY` and `#`. Two cases in the #83 metadata test expected a redundant `@` (`SQRT(@A1:A3)`, `@A1:A3*2`) to be written as an ordinary formula. Dropping the `@` there is only lossless if the reader puts it back on legacy formulas, which it doesn't yet (the read side of #64), so they now expect cm. generated with Claude Code --- crates/formula/src/parser.rs | 7 +++++++ crates/xlsx/src/array_formula.rs | 5 ++++- crates/xlsx/src/fmla.rs | 11 +++++++++++ crates/xlsx/src/tests/array_export.rs | 6 ++++-- crates/xlsx/src/tests/roundtrip.rs | 23 +++++++++++++++++++++++ 5 files changed, 49 insertions(+), 3 deletions(-) diff --git a/crates/formula/src/parser.rs b/crates/formula/src/parser.rs index 754e986..ee2354e 100644 --- a/crates/formula/src/parser.rs +++ b/crates/formula/src/parser.rs @@ -236,6 +236,13 @@ impl Parser { { return Ok(Expr::Unary(UnOp::Spill, Box::new(r))); } + // Files write `@x` as `_xlfn.SINGLE(x)`. + if name == "SINGLE" + && args.len() == 1 + && let Some(x) = args.pop() + { + return Ok(Expr::Unary(UnOp::At, Box::new(x))); + } Ok(Expr::Call(name, args)) } Tok::Sheet(a, b) => { diff --git a/crates/xlsx/src/array_formula.rs b/crates/xlsx/src/array_formula.rs index 2512cc0..01c9d92 100644 --- a/crates/xlsx/src/array_formula.rs +++ b/crates/xlsx/src/array_formula.rs @@ -24,7 +24,10 @@ pub(crate) fn needs_array(wb: &Workbook, sheet: usize, at: CellRef, formula: &Fo let Some(expr) = formula.expr_arc() else { return false }; let mut scan = Scan { wb, at, remaining: 4096, locals: Vec::new() }; let shape = scan.expr(&expr, sheet, false, 0); - shape.array || shape.affected + // An explicit `@` is written as `_xlfn.SINGLE`, which only dynamic-array formulas carry. + let mut at = false; + expr.walk(&mut |e| at |= matches!(e, Expr::Unary(UnOp::At, _))); + shape.array || shape.affected || at } struct Scan<'a> { diff --git a/crates/xlsx/src/fmla.rs b/crates/xlsx/src/fmla.rs index 018cbf2..9ac4ccf 100644 --- a/crates/xlsx/src/fmla.rs +++ b/crates/xlsx/src/fmla.rs @@ -78,6 +78,8 @@ fn expr_to_file(e: Expr) -> String { }, // Files have no `#` operator: Excel writes `A1#` as `_xlfn.ANCHORARRAY(A1)`. Expr::Unary(gridcraft_formula::UnOp::Spill, r) => Expr::Call("_xlfn.ANCHORARRAY".into(), vec![*r]), + // Nor an `@` operator: Excel writes `@x` as `_xlfn.SINGLE(x)`. + Expr::Unary(gridcraft_formula::UnOp::At, x) => Expr::Call("_xlfn.SINGLE".into(), vec![*x]), other => other, }); gridcraft_formula::print(&e) @@ -115,6 +117,15 @@ mod tests { assert_eq!(from_file("SUM(_xlfn.ANCHORARRAY(Sheet1!$D$1))+_xlfn.ANCHORARRAY(B2)"), "SUM(Sheet1!$D$1#)+B2#"); } + #[test] + fn implicit_intersection() { + assert_eq!(to_file(&Formula::new("SUM(@A1:A3*B1:B3)")), "SUM(_xlfn.SINGLE(A1:A3)*B1:B3)"); + assert_eq!(to_file(&Formula::new("@A1:A3*2")), "_xlfn.SINGLE(A1:A3)*2"); + assert_eq!(text_to_file("=@Sheet1!$A$1:$A$3"), "_xlfn.SINGLE(Sheet1!$A$1:$A$3)"); + assert_eq!(from_file("_xlfn.SINGLE(A1:A3)*2"), "@A1:A3*2"); + assert_eq!(from_file("SUM(_xlfn.SINGLE(A1:A3)*B1:B3)"), "SUM(@A1:A3*B1:B3)"); + } + #[test] fn prefixes() { assert_eq!(to_file(&Formula::new("XLOOKUP(1,A:A,B:B)+SUM(A1)")), "_xlfn.XLOOKUP(1,A:A,B:B)+SUM(A1)"); diff --git a/crates/xlsx/src/tests/array_export.rs b/crates/xlsx/src/tests/array_export.rs index d92f473..4bed1e5 100644 --- a/crates/xlsx/src/tests/array_export.rs +++ b/crates/xlsx/src/tests/array_export.rs @@ -236,8 +236,10 @@ fn implicit_intersection_metadata_respects_function_argument_context() { ("SUM(A1:A3)", false), ("SUMPRODUCT(A1:A3*B1:B3)", false), ("SUMPRODUCT(SQRT(A1:A3))", false), - ("SQRT(@A1:A3)", false), - ("@A1:A3*2", false), + // An explicit `@` is written as `_xlfn.SINGLE`, a dynamic-array formula. Dropping it + // instead would need the reader to put it back on legacy formulas. + ("SQRT(@A1:A3)", true), + ("@A1:A3*2", true), ("@(A1:A3*2)", true), ("SQRT(INDIRECT(\"A1\"))", false), ("SUM(INDIRECT(\"A1:A3\"))", false), diff --git a/crates/xlsx/src/tests/roundtrip.rs b/crates/xlsx/src/tests/roundtrip.rs index 12bba62..7360820 100644 --- a/crates/xlsx/src/tests/roundtrip.rs +++ b/crates/xlsx/src/tests/roundtrip.rs @@ -600,3 +600,26 @@ fn spill_and_calc_errors_are_cached_as_value_errors() { assert_eq!(cell.child("v").unwrap().text, value); } } + +/// Files have no `@` operator: Excel writes `@x` as `_xlfn.SINGLE(x)` in a dynamic-array +/// formula, and reads it back as `@x`. +#[test] +fn implicit_intersection_is_written_as_single() { + let mut wb = Workbook::new(); + let s = wb.sheet_mut(0).unwrap(); + for (r, n) in [(0, 1.0), (1, 3.0), (2, 5.0)] { + s.set_value(CellRef::new(r, 0), Value::Number(n)); + } + s.cells.set(at("D3"), Cell { value: Value::Number(10.0), ..Cell::formula(Formula::new("@A1:A3*2")) }); + s.cells.set(at("D4"), Cell { value: Value::Number(2.0), ..Cell::formula(Formula::new("A1*2")) }); + let bytes = write_xlsx(&wb).unwrap(); + let (xml, metadata) = sheet1_xml(&bytes); + assert!(xml.contains("_xlfn.SINGLE(A1:A3)*210"), "{xml}"); + assert!(xml.contains("A1*2"), "{xml}"); + assert!(!xml.contains('@'), "{xml}"); + assert!(metadata); + let (back, _) = read_xlsx(&bytes).unwrap(); + let sh = back.sheet(0).unwrap(); + assert_eq!(sh.cell(at("D3")).and_then(|c| c.formula.as_ref()).map(|f| f.text.as_str()), Some("@A1:A3*2")); + assert_eq!(sh.cell(at("D3")).and_then(|c| c.formula.as_ref()).and_then(|f| f.array), None); +} 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 13/17] 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 78683ae..a268022 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" From 17211dcb65e8aa9542eb1cec5aceb4f3fd70bcea Mon Sep 17 00:00:00 2001 From: mbi Date: Sun, 11 Oct 2026 12:00:07 +0300 Subject: [PATCH 14/17] Fix Paste button deadlock in ribbon popup --- crates/ui-egui/src/ribbon.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/src/ribbon.rs b/crates/ui-egui/src/ribbon.rs index 264479a..8dc1071 100644 --- a/crates/ui-egui/src/ribbon.rs +++ b/crates/ui-egui/src/ribbon.rs @@ -332,10 +332,13 @@ fn home(app: &mut SheetApp, ui: &mut Ui) { // Keep a keyboard-opened menu anchored to the button when a pointer click // cancels keytips, so its items remain in place and can still be clicked. let anchor_id = paste.id.with("keytip_anchor"); + // Query response state before taking the egui context write lock. + // Calling secondary_clicked() inside data_mut can deadlock. + let paste_secondary_clicked = paste.secondary_clicked(); let anchored = ui.ctx().data_mut(|data| { let anchored = if paste_keys { true - } else if paste.secondary_clicked() { + } else if paste_secondary_clicked { false } else { data.get_temp::(anchor_id).unwrap_or(false) From a06de3357a00a9b45389aac976845aa28a66f2d2 Mon Sep 17 00:00:00 2001 From: vangel76 Date: Sun, 11 Oct 2026 11:43:50 +0200 Subject: [PATCH 15/17] Add vangel76 to the contributor names Co-Authored-By: Claude Opus 5.5 --- contributors/people.toml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/contributors/people.toml b/contributors/people.toml index 78683ae..35a56f1 100644 --- a/contributors/people.toml +++ b/contributors/people.toml @@ -28,3 +28,7 @@ sync_github_name = false [people.edurbs] sync_github_name = true + +[people.vangel76] +real_name = "Valentin Klein" +sync_github_name = true From 573cf927452e541f040f99476303b3d4950c9edc Mon Sep 17 00:00:00 2001 From: vangel76 Date: Sun, 11 Oct 2026 11:55:32 +0200 Subject: [PATCH 16/17] UI: skip system fonts egui can't parse instead of panicking at startup (#226) epaint panics in set_fonts on a face skrifa can't open. Every system font file is now checked with the same skrifa call (plus a readable cmap) before it reaches FontDefinitions; a bad candidate is skipped with a warning and the next one is tried, and a final pass drops any remaining unparseable face from the definitions and every family. Co-Authored-By: Claude Opus 5.5 --- Cargo.lock | 1 + Cargo.toml | 3 ++ ROADMAP.md | 1 + crates/ui-egui/Cargo.toml | 1 + crates/ui-egui/src/theme.rs | 81 +++++++++++++++++++++++++++++++++++++ 5 files changed, 87 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index f772f83..398c656 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1555,6 +1555,7 @@ dependencies = [ "log", "serde", "serde_json", + "skrifa", "sys-locale", "toml", "web-sys", diff --git a/Cargo.toml b/Cargo.toml index d7fad0f..15aa9ef 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -45,6 +45,9 @@ rfd = "0.17" arboard = { version = "3.6", default-features = false } # The desktop's preferred language, for the first-run interface language (native only). sys-locale = "0.3" +# Font parser egui's epaint uses; the UI validates system font files with it before handing them +# to egui, which panics on a file it can't parse. Same version as epaint's, so no second copy. +skrifa = { version = "0.44", default-features = false, features = ["std"] } [workspace.lints.rust] unsafe_code = "forbid" diff --git a/ROADMAP.md b/ROADMAP.md index 1189fd7..39fa740 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -131,6 +131,7 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in | Date | What landed | |---|---| +| 2026-10-11 | Startup no longer panics on a system font egui can't parse: fonts are validated and skipped (#226) | | 2026-10-10 | Ribbon translated into Japanese, Chinese, Korean, Brazilian Portuguese and Russian with system-language default (#31, #104, #82); CJK/Thai/Hebrew font fallback (#38); clear messages for encrypted and `.xls` files (#17); smooth line charts (#62). Progress docs restructured to the craftrules standard; full re-measure (this page). Initial ribbon key tips (#43), in-place text box editing (#84), copy as HTML table (#36). ODS and XLSB data import (#100, #101); recalc linear in the number of range formulas (#66); RISC-V Linux build; Flatpak repository on Pages; web cell-commit trap fixed | | 2026-10-09 | Community fixes: spill range operator `A1#`, sheet-scoped names, protection refuses locked-cell edits, Insert/Delete Cells adjust references, `DATE(1900,2,29)`, SUMIF sizing, `ROUND` at 15 digits, `_xlpm.` LAMBDA parameters in XLSX, vertical scrollbar, system appearance, border previews; v0.4.0 | | 2026-10-08 | v0.2.0 and v0.3.0 releases; Flatpak, AppImage zsync, branded DMG; desktop log file; DirectX 12 default on Windows | diff --git a/crates/ui-egui/Cargo.toml b/crates/ui-egui/Cargo.toml index a6a2da2..4f44fd9 100644 --- a/crates/ui-egui/Cargo.toml +++ b/crates/ui-egui/Cargo.toml @@ -16,6 +16,7 @@ serde = { workspace = true } serde_json = { workspace = true } log = { workspace = true } image = { workspace = true } +skrifa = { workspace = true } [dev-dependencies] egui_kittest = { workspace = true, features = ["wgpu", "snapshot"] } diff --git a/crates/ui-egui/src/theme.rs b/crates/ui-egui/src/theme.rs index 219e2d2..a414139 100644 --- a/crates/ui-egui/src/theme.rs +++ b/crates/ui-egui/src/theme.rs @@ -166,10 +166,28 @@ pub fn cell_family(name: &str, bold: bool, italic: bool) -> FontFamily { ) } +/// Whether epaint can open face `index` of this font file and it maps characters to glyphs. +/// epaint panics (in `Context::set_fonts`, i.e. on startup) on a face it can't open, so every font +/// file we did not ship is checked first with the same parser and the same call epaint makes +/// (`skrifa::FontRef::from_index`): a corrupt file, something that is not an sfnt at all, or a +/// collection index past the end is rejected instead of crashing the app (#226). A face whose +/// character map is unreadable (a truncated file) would open but draw nothing, so it is refused +/// too and the next candidate gets its turn. +pub(crate) fn font_parses(bytes: &[u8], index: u32) -> bool { + use skrifa::MetadataProvider; + skrifa::FontRef::from_index(bytes, index).is_ok_and(|font| font.charmap().has_map()) +} + +/// Loads the first candidate that exists and parses. A file that exists but can't be parsed is +/// skipped with a warning and the next candidate is tried. #[cfg(not(target_arch = "wasm32"))] fn try_load(paths: &[(&str, u32)]) -> Option { for (p, index) in paths { if let Ok(bytes) = std::fs::read(p) { + if !font_parses(&bytes, *index) { + log::warn!("skipping font {p} (face {index}): not a font file egui can read"); + continue; + } let mut fd = FontData::from_owned(bytes); fd.index = *index; return Some(fd); @@ -178,6 +196,21 @@ fn try_load(paths: &[(&str, u32)]) -> Option { None } +/// Last line of defence before [`FontDefinitions`] reach egui: drops every face epaint could not +/// parse (and its key from every family), so a bad font costs a fallback, never a crash. +pub(crate) fn drop_unparseable_fonts(fonts: &mut FontDefinitions) { + let bad: Vec = fonts.font_data.iter().filter(|(_, fd)| !font_parses(&fd.font, fd.index)).map(|(k, _)| k.clone()).collect(); + for key in &bad { + log::warn!("dropping font {key:?}: egui can't parse it"); + fonts.font_data.remove(key); + } + if !bad.is_empty() { + for chain in fonts.families.values_mut() { + chain.retain(|k| !bad.contains(k)); + } + } +} + #[cfg(target_arch = "wasm32")] fn try_load(_paths: &[(&str, u32)]) -> Option { None @@ -340,6 +373,7 @@ pub fn font_definitions_for_language(language: crate::i18n::Language) -> FontDef if let Some(ui) = fonts.families.get(&FontFamily::Name(UI.into())).cloned() { fonts.families.insert(FontFamily::Proportional, ui); } + drop_unparseable_fonts(&mut fonts); fonts } @@ -650,4 +684,51 @@ mod tests { } } } + + #[test] + fn unparseable_font_files_are_rejected_not_fatal() { + // #226: epaint panics in `set_fonts` on a face it can't parse ("An offset was out of + // bounds"). Garbage, a truncated real font and a bad collection index must all be refused. + let real = FontDefinitions::default().font_data.values().next().map(|fd| fd.font.to_vec()).unwrap_or_default(); + assert!(font_parses(&real, 0), "egui's own font must parse"); + assert!(!font_parses(b"", 0)); + assert!(!font_parses(&[0xAB; 64], 0)); + assert!(!font_parses(real.get(..20).unwrap_or_default(), 0), "a truncated font must be refused"); + assert!(!font_parses(&real, 3), "a single-face file has no face 3"); + + // A bad face that slipped into the definitions is dropped (key and family entries), and the + // context builds its fonts without panicking. + let mut fonts = font_definitions(); + fonts.font_data.insert("sys-hebrew".into(), Arc::new(FontData::from_owned(vec![0, 1, 0, 0, 0, 0xFF, 0, 0]))); + for chain in fonts.families.values_mut() { + chain.push("sys-hebrew".into()); + } + drop_unparseable_fonts(&mut fonts); + assert!(!fonts.font_data.contains_key("sys-hebrew")); + assert!(fonts.families.values().all(|c| !c.iter().any(|k| k == "sys-hebrew"))); + let ctx = egui::Context::default(); + ctx.set_fonts(fonts); + let mut out = ctx.run_ui(egui::RawInput::default(), |ui| { + ui.label("Hebrew עברית"); + }); + out.textures_delta.clear(); // nothing consumes the atlas in a headless test + } + + #[cfg(not(target_arch = "wasm32"))] + #[test] + fn try_load_skips_a_broken_candidate() { + // A candidate that exists but is not a readable font must not win over a good one behind it. + let dir = std::env::temp_dir().join(format!("gridcraft-font-test-{}", std::process::id())); + let _ = std::fs::create_dir_all(&dir); + let bad = dir.join("broken.ttf"); + let good = dir.join("good.ttf"); + let real = FontDefinitions::default().font_data.values().next().map(|fd| fd.font.to_vec()).unwrap_or_default(); + let _ = std::fs::write(&bad, real.get(..40).unwrap_or_default()); + let _ = std::fs::write(&good, &real); + let (bad_s, good_s) = (bad.to_string_lossy().into_owned(), good.to_string_lossy().into_owned()); + assert!(try_load(&[(bad_s.as_str(), 0)]).is_none()); + let loaded = try_load(&[(bad_s.as_str(), 0), (good_s.as_str(), 0)]); + assert_eq!(loaded.map(|fd| fd.font.len()), Some(real.len())); + let _ = std::fs::remove_dir_all(&dir); + } } From 5c614fca48584b7358dfba2a06fbcb27da1953b3 Mon Sep 17 00:00:00 2001 From: vangel76 Date: Sun, 11 Oct 2026 11:57:26 +0200 Subject: [PATCH 17/17] UI: Name Box Enter goes to the typed cell, not the one below (#247) The Enter that commits the Name Box stayed in the frame's input; the grid, drawn after the formula bar and focused again, read it as its own Enter and moved the active cell down. The Name Box now consumes it. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/formula_bar.rs | 4 ++++ crates/ui-egui/tests/interaction.rs | 20 ++++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/crates/ui-egui/src/formula_bar.rs b/crates/ui-egui/src/formula_bar.rs index 6cca110..a693ee9 100644 --- a/crates/ui-egui/src/formula_bar.rs +++ b/crates/ui-egui/src/formula_bar.rs @@ -112,6 +112,10 @@ fn name_box(app: &mut SheetApp, ui: &mut egui::Ui, t: &Tokens) { } if resp.lost_focus() { if ui.input(|i| i.key_pressed(Key::Enter)) { + // The Enter that commits the Name Box is used up here. Left in the frame's input, the + // grid (drawn after the formula bar, and focused again now the box let go) would read + // it as its own Enter and move the active cell one row down from the jump target (#247). + ui.input_mut(|i| i.events.retain(|e| !matches!(e, egui::Event::Key { key: Key::Enter, .. }))); let target = text.trim().to_string(); if !target.is_empty() && target != shown { // A new name for the selection, or a place to go. diff --git a/crates/ui-egui/tests/interaction.rs b/crates/ui-egui/tests/interaction.rs index 84f20a6..b0ab7ae 100644 --- a/crates/ui-egui/tests/interaction.rs +++ b/crates/ui-egui/tests/interaction.rs @@ -125,6 +125,26 @@ fn space_toggles_the_active_checkbox_without_clicking_the_hovered_cell() { assert!(h.state().editor.is_none()); } +#[test] +fn name_box_enter_goes_to_the_typed_cell_not_the_one_below() { + // #247: typing A1 in the Name Box and pressing Enter selected A2; the grid replayed the Enter. + let mut h = harness(blank()); + h.state_mut().session.run("selection.set", json!({"cell": "C5"})).unwrap(); + h.run_steps(2); + h.ctx.memory_mut(|m| m.request_focus(egui::Id::new("gridcraft.name_box"))); + h.run_steps(2); + h.input_mut().events.push(Event::Key { key: Key::A, physical_key: None, pressed: true, repeat: false, modifiers: Modifiers::COMMAND }); + h.input_mut().events.push(Event::Text("A1".into())); + h.run_steps(2); + key(&mut h, Key::Enter, Modifiers::NONE); + h.run_steps(2); + assert_eq!(h.state().session.active().unwrap().selection.active, CellRef::new(0, 0)); + assert!(h.state().name_box.is_none()); + // The grid has the focus back: a plain Enter there still moves down. + key(&mut h, Key::Enter, Modifiers::NONE); + assert_eq!(h.state().session.active().unwrap().selection.active, CellRef::new(1, 0)); +} + #[test] fn typing_enters_values_and_moves_down() { let mut h = harness(blank());