From 0019c2c5523bde83615f07451988065593cca48d Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Mon, 12 Oct 2026 08:31:39 +0200 Subject: [PATCH 01/15] Make check --json list warning messages instead of a count (#526) --- apps/gridcraft-cli/src/check.rs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/apps/gridcraft-cli/src/check.rs b/apps/gridcraft-cli/src/check.rs index 9fc81e30a..4b1c65991 100644 --- a/apps/gridcraft-cli/src/check.rs +++ b/apps/gridcraft-cli/src/check.rs @@ -134,7 +134,8 @@ pub struct FileReport { pub path: String, /// GridCraft couldn't open the file: its message. pub error: Option, - pub warnings: usize, + /// Import warnings, in the order the importer produced them. + pub warnings: Vec, pub calc: CalcReport, pub round_trip: RoundTrip, } @@ -280,7 +281,7 @@ fn check_inner(path: &str, bytes: &[u8], opts: &Options) -> FileReport { return report; } }; - report.warnings = warnings.len(); + report.warnings = warnings; let mut fresh = cached.clone(); let mut calc = Calc::new(); calc.recalc_all(&mut fresh); @@ -1342,6 +1343,15 @@ mod tests { assert_eq!(validate_package(&own).unwrap(), Vec::::new()); } + #[test] + fn json_lists_warning_messages() { + let with = FileReport { path: "a.ods".into(), warnings: vec!["ODS data-only import".into()], ..Default::default() }; + let without = FileReport { path: "b.xlsx".into(), ..Default::default() }; + let v = report_json(&[with, without], &Options::default()); + assert_eq!(v["files"][0]["warnings"], json!(["ODS data-only import"])); + assert_eq!(v["files"][1]["warnings"], json!([])); + } + #[test] fn unopenable_files_are_reported() { let r = check_bytes("broken.xlsx", b"PK\x03\x04 not really", &Options::default()); From da1d9d011673bc5e85944d6fcf3917ec44ad4167 Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Mon, 12 Oct 2026 08:31:04 +0200 Subject: [PATCH 02/15] Cap queued pointer events from ui.click/ui.drag control requests (#525) --- crates/ui-egui/src/control.rs | 90 +++++++++++++++++++++++++++-------- docs/control-protocol.md | 4 +- 2 files changed, 72 insertions(+), 22 deletions(-) diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index 6f6280ef7..2ec09b875 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -133,24 +133,42 @@ fn cell_center(app: &SheetApp, c: CellRef) -> Option { Some(geo.screen_cell_rect(sh, c).center()) } -fn push_click(app: &mut SheetApp, pos: egui::Pos2, button: egui::PointerButton, count: u32, m: egui::Modifiers) { +/// Most synthetic pointer events a control client may keep queued at once (the queue drains one batch per frame). +const MAX_QUEUED_POINTER_EVENTS: usize = 10_000; + +/// Refuses a request that would push the queued pointer events past [`MAX_QUEUED_POINTER_EVENTS`]; the queue is left unchanged. +fn pointer_budget(app: &SheetApp, extra: usize) -> Result<(), String> { + let queued: usize = app.synthetic.iter().map(Vec::len).sum(); + if queued.saturating_add(extra) > MAX_QUEUED_POINTER_EVENTS { + return Err(format!("too many pointer events (limit {MAX_QUEUED_POINTER_EVENTS} queued at once)")); + } + Ok(()) +} + +fn push_click(app: &mut SheetApp, pos: egui::Pos2, button: egui::PointerButton, count: u32, m: egui::Modifiers) -> Result<(), String> { + let count = count.max(1) as usize; + pointer_budget(app, count.saturating_mul(2).saturating_add(1))?; app.synthetic.push_back(vec![egui::Event::PointerMoved(pos)]); - let mut batch = Vec::new(); - for _ in 0..count.max(1) { + let mut batch = Vec::with_capacity(count * 2); + for _ in 0..count { batch.push(egui::Event::PointerButton { pos, button, pressed: true, modifiers: m }); batch.push(egui::Event::PointerButton { pos, button, pressed: false, modifiers: m }); } app.synthetic.push_back(batch); + Ok(()) } -fn push_drag(app: &mut SheetApp, a: egui::Pos2, b: egui::Pos2, steps: usize, m: egui::Modifiers) { +fn push_drag(app: &mut SheetApp, a: egui::Pos2, b: egui::Pos2, steps: usize, m: egui::Modifiers) -> Result<(), String> { + let steps = steps.max(1); + pointer_budget(app, steps.saturating_add(3))?; app.synthetic.push_back(vec![egui::Event::PointerMoved(a)]); app.synthetic.push_back(vec![egui::Event::PointerButton { pos: a, button: egui::PointerButton::Primary, pressed: true, modifiers: m }]); - for i in 1..=steps.max(1) { - let t = i as f32 / steps.max(1) as f32; + for i in 1..=steps { + let t = i as f32 / steps as f32; app.synthetic.push_back(vec![egui::Event::PointerMoved(a + (b - a) * t)]); } app.synthetic.push_back(vec![egui::Event::PointerButton { pos: b, button: egui::PointerButton::Primary, pressed: false, modifiers: m }]); + Ok(()) } /// Keeps the commands whose id, label or ribbon path contains `search` (case-insensitive); `None` keeps all. @@ -312,33 +330,38 @@ pub fn handle(app: &mut SheetApp, ctx: &egui::Context, method: &str, p: &Json) - } "ui.click" => { let button = if s("button") == Some("right") { egui::PointerButton::Secondary } else { egui::PointerButton::Primary }; - push_click(app, egui::pos2(f("x"), f("y")), button, p.get("count").and_then(Json::as_u64).unwrap_or(1) as u32, modifiers(p)); - ok(Json::Null) + let count = p.get("count").and_then(Json::as_u64).unwrap_or(1).min(u64::from(u32::MAX)) as u32; + match push_click(app, egui::pos2(f("x"), f("y")), button, count, modifiers(p)) { + Ok(()) => ok(Json::Null), + Err(e) => err(e), + } } "ui.drag" => { - push_drag( - app, - egui::pos2(f("x"), f("y")), - egui::pos2(f("toX"), f("toY")), - p.get("steps").and_then(Json::as_u64).unwrap_or(8) as usize, - modifiers(p), - ); - ok(Json::Null) + let steps = p.get("steps").and_then(Json::as_u64).unwrap_or(8).min(u64::from(u32::MAX)) as usize; + match push_drag(app, egui::pos2(f("x"), f("y")), egui::pos2(f("toX"), f("toY")), steps, modifiers(p)) { + Ok(()) => ok(Json::Null), + Err(e) => err(e), + } } "ui.cell.click" => { let Some(c) = s("cell").and_then(CellRef::parse) else { return err("missing `cell`") }; let Some(pos) = cell_center(app, c) else { return err("no grid yet") }; let button = if s("button") == Some("right") { egui::PointerButton::Secondary } else { egui::PointerButton::Primary }; - push_click(app, pos, button, p.get("count").and_then(Json::as_u64).unwrap_or(1) as u32, modifiers(p)); - ok(json!({"x": pos.x, "y": pos.y})) + let count = p.get("count").and_then(Json::as_u64).unwrap_or(1).min(u64::from(u32::MAX)) as u32; + match push_click(app, pos, button, count, modifiers(p)) { + Ok(()) => ok(json!({"x": pos.x, "y": pos.y})), + Err(e) => err(e), + } } "ui.cell.drag" => { let (Some(a), Some(b)) = (s("from").and_then(CellRef::parse), s("to").and_then(CellRef::parse)) else { return err("missing `from`/`to`"); }; let (Some(pa), Some(pb)) = (cell_center(app, a), cell_center(app, b)) else { return err("no grid yet") }; - push_drag(app, pa, pb, 8, modifiers(p)); - ok(Json::Null) + match push_drag(app, pa, pb, 8, modifiers(p)) { + Ok(()) => ok(Json::Null), + Err(e) => err(e), + } } "ui.resize" => { ctx.send_viewport_cmd(egui::ViewportCommand::InnerSize(egui::vec2(f("width").max(400.0), f("height").max(300.0)))); @@ -494,4 +517,31 @@ mod tests { assert!(filter_commands(sample(), Some("catalog_no_match_7391")).is_empty()); assert_eq!(filter_commands(sample(), None).len(), 3); } + + fn test_app() -> SheetApp { + let mut session = gridcraft_engine::Session::new(); + session.new_workbook(); + SheetApp::new(session, crate::Services::default()) + } + + #[test] + fn oversized_pointer_requests_are_refused_and_leave_the_queue_alone() { + let mut app = test_app(); + let p = egui::pos2(20.0, 20.0); + let m = egui::Modifiers::NONE; + let btn = egui::PointerButton::Primary; + assert!(push_click(&mut app, p, btn, 2, m).is_ok()); + assert_eq!(app.synthetic.iter().map(Vec::len).sum::(), 5); + assert!(push_click(&mut app, p, btn, u32::MAX, m).is_err()); + assert!(push_drag(&mut app, p, p, usize::MAX, m).is_err()); + assert!(push_drag(&mut app, p, p, MAX_QUEUED_POINTER_EVENTS, m).is_err()); + assert_eq!(app.synthetic.len(), 2); + assert_eq!(app.synthetic.iter().map(Vec::len).sum::(), 5); + // Already queued input counts toward the budget. + assert!(push_click(&mut app, p, btn, MAX_QUEUED_POINTER_EVENTS / 2, m).is_err()); + assert_eq!(app.synthetic.len(), 2); + // Normal requests still queue in full. + assert!(push_drag(&mut app, p, p + egui::vec2(40.0, 40.0), 3, m).is_ok()); + assert_eq!(app.synthetic.len(), 8); + } } diff --git a/docs/control-protocol.md b/docs/control-protocol.md index a16208a38..d434fa996 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -26,8 +26,8 @@ Errors come back as `{"id": …, "ok": false, "error": "…"}`. | `sheet.read` | `{range?, sheet?, formulas?, formatted?}` | Values (or formulas / displayed text) of a range | | `cell.get` | `{cell?}` | One cell: value, formula, displayed text, style, merge, comment, link | | `ui.inspect` | | UI state: ribbon tab, editor, open dialog, Backstage page, message, grid rect, scroll, perf | -| `ui.click` / `ui.drag` / `ui.move` | `{x, y, toX?, toY?, button?, count?, shift?, cmd?, alt?}` | Real pointer input in window points | -| `ui.cell.click` / `ui.cell.drag` | `{cell}` / `{from, to}` | Pointer input at a cell's centre (works in point mode while editing) | +| `ui.click` / `ui.drag` / `ui.move` | `{x, y, toX?, toY?, button?, count?, shift?, cmd?, alt?}` | Real pointer input in window points; refused with an error if it would leave more than 10,000 pointer events queued | +| `ui.cell.click` / `ui.cell.drag` | `{cell}` / `{from, to}` | Pointer input at a cell's centre (works in point mode while editing); same 10,000-event queue limit | | `ui.key` / `ui.text` | `{key, shift?, cmd?, alt?, ctrl?}` / `{text}` | Keyboard input (typing in the grid starts editing) | | `ui.edit.begin` / `ui.edit.text` / `ui.edit.commit` / `ui.edit.cancel` | `{text?}` / `{move?: down\|right\|up\|left\|none}` | Drive the cell editor | | `ui.ribbon` | `{tab}` | Switch ribbon tab | From 2d0a7a1e42c04f1c772e8294ec701ca0dfe6bde6 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Tue, 13 Oct 2026 14:48:32 +0200 Subject: [PATCH 03/15] Ribbon: more currency symbols in the Accounting Number Format menu (fixes #1063) --- crates/ui-egui/src/ribbon.rs | 71 ++++++++++++++++++++++++------------ 1 file changed, 47 insertions(+), 24 deletions(-) diff --git a/crates/ui-egui/src/ribbon.rs b/crates/ui-egui/src/ribbon.rs index 9c6d89eeb..e4a2cbe57 100644 --- a/crates/ui-egui/src/ribbon.rs +++ b/crates/ui-egui/src/ribbon.rs @@ -478,6 +478,21 @@ pub(crate) fn editable_combo( result } +/// The Accounting Number Format dropdown: label and the code Excel applies for that symbol. +const ACCOUNTING_SYMBOLS: &[(&str, &str)] = &[ + ("$ English (United States)", "_(\"$\"* #,##0.00_);_(\"$\"* \\(#,##0.00\\);_(\"$\"* \"-\"??_);_(@_)"), + ("£ English (United Kingdom)", "_-[$£-809]* #,##0.00_-;-[$£-809]* #,##0.00_-;_-[$£-809]* \"-\"??_-;_-@_-"), + ("€ Euro", "_-[$€-x-euro2] * #,##0.00_-;-[$€-x-euro2] * #,##0.00_-;_-[$€-x-euro2] * \"-\"??_-;_-@_-"), + ("¥ Japanese", "_-[$¥-411]* #,##0_-;-[$¥-411]* #,##0_-;_-[$¥-411]* \"-\"_-;_-@_-"), + ("¥ Chinese (PRC)", "_-[$¥-804]* #,##0.00_-;-[$¥-804]* #,##0.00_-;_-[$¥-804]* \"-\"??_-;_-@_-"), + ("₹ English (India)", "_-[$₹-4009]* #,##0.00_-;-[$₹-4009]* #,##0.00_-;_-[$₹-4009]* \"-\"??_-;_-@_-"), + ("CHF German (Switzerland)", "_-[$CHF-807]* #,##0.00_-;-[$CHF-807]* #,##0.00_-;_-[$CHF-807]* \"-\"??_-;_-@_-"), + ("Rp Indonesian", "_-[$Rp-421]* #,##0.00_-;-[$Rp-421]* #,##0.00_-;_-[$Rp-421]* \"-\"??_-;_-@_-"), + ("₩ Korean", "_-[$₩-412]* #,##0_-;-[$₩-412]* #,##0_-;_-[$₩-412]* \"-\"_-;_-@_-"), + ("R$ Portuguese (Brazil)", "_-[$R$-416]* #,##0.00_-;-[$R$-416]* #,##0.00_-;_-[$R$-416]* \"-\"??_-;_-@_-"), + ("₽ Russian", "_-[$₽-419]* #,##0.00_-;-[$₽-419]* #,##0.00_-;_-[$₽-419]* \"-\"??_-;_-@_-"), +]; + fn menu_items(app: &mut SheetApp, ui: &mut Ui, items: &[(&str, &str, serde_json::Value)]) { for (label, id, p) in items { if *label == "-" { @@ -793,30 +808,10 @@ fn home(app: &mut SheetApp, ui: &mut Ui) { act(app, "home.accounting", json!({})); } egui::Popup::menu(&ca).show(|ui| { - menu_items( - app, - ui, - &[ - ( - "$ English (United States)", - "home.numberFormat", - json!({"code": "_(\"$\"* #,##0.00_);_(\"$\"* \\(#,##0.00\\);_(\"$\"* \"-\"??_);_(@_)"}), - ), - ( - "£ English (United Kingdom)", - "home.numberFormat", - json!({"code": "_-[$£-809]* #,##0.00_-;-[$£-809]* #,##0.00_-;_-[$£-809]* \"-\"??_-;_-@_-"}), - ), - ( - "€ Euro", - "home.numberFormat", - json!({"code": "_-[$€-x-euro2] * #,##0.00_-;-[$€-x-euro2] * #,##0.00_-;_-[$€-x-euro2] * \"-\"??_-;_-@_-"}), - ), - ("¥ Japanese", "home.numberFormat", json!({"code": "_-[$¥-411]* #,##0_-;-[$¥-411]* #,##0_-;_-[$¥-411]* \"-\"_-;_-@_-"})), - ("-", "", json!(null)), - ("More Accounting Formats…", "dialog:formatCells", json!({"tab": "Number"})), - ], - ); + let mut items: Vec<_> = ACCOUNTING_SYMBOLS.iter().map(|(label, code)| (*label, "home.numberFormat", json!({"code": code}))).collect(); + items.push(("-", "", json!(null))); + items.push(("More Accounting Formats…", "dialog:formatCells", json!({"tab": "Number"}))); + menu_items(app, ui, &items); }); let __k = icon_button(ui, Icon::Percent, t.text, "Percent Style", vec2(24.0, 23.0)); control_keytip(app, ui, &__k, "home.percent"); @@ -2514,3 +2509,31 @@ mod checkbox_on_tests { assert!(!checkbox_on(false, true)); } } + +#[cfg(test)] +mod accounting_symbol_tests { + use super::*; + use gridcraft_engine::core::{DateSystem, Value}; + use gridcraft_engine::numfmt::{NumberFormat, format_value}; + + #[test] + fn accounting_dropdown_offers_the_issue_currencies_with_excel_codes() { + // #1063: label, Excel's [$symbol-LCID] tag, and 1234.5 as Excel shows it (fill and the + // `_-` padding aside). The yen and won have no minor unit, so their codes show no decimals. + for (label, tag, shown) in [ + ("Rp Indonesian", "[$Rp-421]", "Rp1,234.50"), + ("₹ English (India)", "[$₹-4009]", "₹1,234.50"), + ("₩ Korean", "[$₩-412]", "₩1,235"), + ("¥ Chinese (PRC)", "[$¥-804]", "¥1,234.50"), + ("¥ Japanese", "[$¥-411]", "¥1,235"), + ] { + let code = ACCOUNTING_SYMBOLS.iter().find(|(l, _)| *l == label).map(|(_, c)| *c); + let Some(code) = code else { panic!("{label} is not in the Accounting dropdown") }; + assert!(code.split(';').take(3).all(|section| section.contains(tag)), "{label}: {code}"); + let out = format_value(&Value::Number(1234.5), &NumberFormat::parse(code), DateSystem::D1900); + assert_eq!(out.text.trim(), shown, "{label}"); + let symbol = tag.trim_start_matches("[$").split('-').next().unwrap_or_default(); + assert_eq!(out.fill.map(|(c, at)| (c, out.text[..at].trim_start())), Some((' ', symbol)), "{label}"); + } + } +} From 3f520a3420ec075ccc45047f31cfadf7c9065188 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Wed, 14 Oct 2026 03:03:33 +0200 Subject: [PATCH 04/15] File: Save a Copy asks where to save a bare name instead of writing it to the working directory (fixes #1212) --- crates/ui-egui/src/dialogs.rs | 12 ++++++++- crates/ui-egui/tests/file_menu.rs | 43 +++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index fb899c702..9b42a70ff 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -1170,7 +1170,17 @@ pub fn show(app: &mut SheetApp, ctx: &egui::Context) { d.values.insert("path".into(), json!(path.clone())); ok_cancel(ui, &mut confirm, &mut open); if confirm { - let target = if path.is_empty() { app.services.pick_save.as_ref().and_then(|f| f(&format!("Copy.{fmt}"))) } else { Some(path) }; + // A bare name has no folder: written as typed it would land in the + // process's working directory (`/` from the Finder, the install folder on + // Windows), never where the user looks. The picker chooses the folder. + let pick = app.services.pick_save.as_ref(); + let target = if path.is_empty() { + pick.and_then(|f| f(&format!("Copy.{fmt}"))) + } else if let Some(f) = pick.filter(|_| std::path::Path::new(&path).is_relative()) { + f(&if path.ends_with(&format!(".{fmt}")) { path } else { format!("{path}.{fmt}") }) + } else { + Some(path) + }; if let Some(t) = target { let id = match fmt.as_str() { "csv" => "file.exportCsv", diff --git a/crates/ui-egui/tests/file_menu.rs b/crates/ui-egui/tests/file_menu.rs index 691eef367..c1b531e96 100644 --- a/crates/ui-egui/tests/file_menu.rs +++ b/crates/ui-egui/tests/file_menu.rs @@ -217,3 +217,46 @@ fn a_cancelled_export_pdf_picker_writes_nothing() { assert!(h.state().message.is_none()); assert!(h.state_mut().session.take_ui_requests().is_empty()); } + +/// Share › Export as Web Page (and the native File menu's Save as Web Page) asks for a name in its +/// own dialog. A bare name has no folder: written as typed it lands in the process's working +/// directory, where nobody looks for it (#1212). The save picker chooses the folder instead. +#[test] +fn save_a_copy_with_a_bare_name_asks_where_to_save_it() { + let dir = std::env::temp_dir().join(format!("gridcraft-save-copy-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let asked = Arc::new(Mutex::new(Vec::::new())); + let (capture, target) = (asked.clone(), dir.clone()); + let mut h = harness(Services { + pick_save: Some(Box::new(move |suggested| { + capture.lock().unwrap().push(suggested.to_string()); + Some(target.join("Chosen.html").to_string_lossy().into_owned()) + })), + ..Default::default() + }); + h.state_mut().run("cell.set", serde_json::json!({"cell": "A1", "input": "42"})).unwrap(); + let save_copy = |h: &mut egui_kittest::Harness<'static, SheetApp>, path: &str| { + h.state_mut().open_dialog("saveCopy", serde_json::json!({"format": "html"})); + h.run_steps(3); + h.state_mut().dialog.as_mut().unwrap().values.insert("path".into(), serde_json::json!(path)); + h.run_steps(2); + h.get_by_label(" OK ").click(); + h.run_steps(3); + }; + let name = format!("gridcraft-save-copy-{}", std::process::id()); + save_copy(&mut h, &name); + let stray = std::path::PathBuf::from(format!("{name}.html")); + let written_in_cwd = stray.exists(); + let _ = std::fs::remove_file(&stray); + assert!(!written_in_cwd, "a bare name is not written to the working directory"); + assert_eq!(*asked.lock().unwrap(), vec![format!("{name}.html")], "the picker is offered the typed name"); + assert!(std::fs::read_to_string(dir.join("Chosen.html")).unwrap().contains("42"), "the page is written where the picker says"); + assert!(h.state().message.is_none()); + // A full path skips the picker; a write that fails is reported, not swallowed. + save_copy(&mut h, &dir.join("Named.html").to_string_lossy()); + assert!(dir.join("Named.html").exists()); + save_copy(&mut h, &dir.join("missing").join("Lost.html").to_string_lossy()); + assert!(h.state().message.is_some(), "a failed write is reported"); + assert_eq!(asked.lock().unwrap().len(), 1); + let _ = std::fs::remove_dir_all(&dir); +} From 9f11e26e787d1525102c2ffc865a3e9a71e8aca7 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Wed, 14 Oct 2026 05:10:19 +0200 Subject: [PATCH 05/15] Journal: cap by serialized size and keep a recording's start through trims (part of #1228) --- crates/engine/src/lib.rs | 47 ++++++++++++++++++++++++++++++++++---- crates/engine/src/tests.rs | 35 ++++++++++++++++++++++++++++ docs/control-protocol.md | 2 +- 3 files changed, 79 insertions(+), 5 deletions(-) diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 258a1cb72..3efca7ab4 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -116,6 +116,13 @@ pub struct DocState { static NEXT_UID: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(1); const UNDO_LIMIT: usize = 100; +/// Past this many entries the journal drops its oldest half. +const JOURNAL_MAX_ENTRIES: usize = 10_000; +/// Past this many bytes (entries serialized as JSON) the journal drops its oldest entries until +/// it is under half. Entries keep their full params so they replay (recorded scripts, +/// `engine.journal`), and a `file.open` holds the whole file as base64: without a size cap a +/// few opens of a large workbook keep tens of MB alive. +pub(crate) const JOURNAL_MAX_BYTES: usize = 64 * 1024 * 1024; impl DocState { pub fn new(mut wb: Workbook, path: Option, title: String) -> DocState { @@ -236,6 +243,9 @@ pub struct Session { pub mode: Mode, /// Commands run so far, without their passwords ([`cmd::journal_params`]). pub journal: Vec<(String, Json)>, + /// Serialized size of each journal entry, measured once when it is pushed, and their sum. + journal_sizes: Vec, + journal_bytes: usize, pub ui_requests: Vec, /// Format painter source style (sticky when double-clicked). pub format_painter: Option<(Arc, usize, RangeRef, bool)>, @@ -309,6 +319,38 @@ impl Session { self.add_document(DocState::new(wb, None, format!("Book{n}"))) } + /// Journals a command, then trims the oldest entries past either cap. A recording in progress + /// keeps pointing at the same entry (the steps the trim dropped are lost). + fn push_journal(&mut self, id: &str, params: Json) { + let size = |e: &(String, Json)| serde_json::to_vec(e).map_or(0, |v| v.len()); + // `journal` is public: if it was changed from outside, measure it again. + if self.journal_sizes.len() != self.journal.len() { + self.journal_sizes = self.journal.iter().map(size).collect(); + self.journal_bytes = self.journal_sizes.iter().sum(); + } + let entry = (id.to_string(), params); + let entry_size = size(&entry); + self.journal.push(entry); + self.journal_sizes.push(entry_size); + self.journal_bytes = self.journal_bytes.saturating_add(entry_size); + let len = self.journal.len(); + let mut n = if len > JOURNAL_MAX_ENTRIES { len - JOURNAL_MAX_ENTRIES / 2 } else { 0 }; + let mut kept = self.journal_bytes.saturating_sub(self.journal_sizes.iter().take(n).sum()); + if self.journal_bytes > JOURNAL_MAX_BYTES { + // The newest entry stays even when it is larger than the cap on its own. + while kept > JOURNAL_MAX_BYTES / 2 && n + 1 < len { + kept = kept.saturating_sub(self.journal_sizes.get(n).copied().unwrap_or(0)); + n += 1; + } + } + if n > 0 { + self.journal.drain(..n); + self.journal_sizes.drain(..n); + self.journal_bytes = kept; + self.recording = self.recording.map(|r| r.saturating_sub(n)); + } + } + pub fn commands(&self) -> Vec { command_specs().iter().map(|c| c.info(self)).collect() } @@ -340,10 +382,7 @@ impl Session { } }; if spec.journal && result.is_ok() { - self.journal.push((id.to_string(), cmd::journal_params(¶ms))); - if self.journal.len() > 10_000 { - self.journal.drain(..5_000); - } + self.push_journal(id, cmd::journal_params(¶ms)); } if spec.undoable && let Some((uid, wb, sel)) = before diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index ae499a189..2458670bb 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -3282,3 +3282,38 @@ fn paste_special_validation_moves_relative_references() { assert!(!ok(&mut s, "C3", "5"), "the pasted rule checks C3, not A1"); assert!(ok(&mut s, "C3", "20")); } + +#[test] +fn journal_is_capped_by_serialized_size() { + let mut s = s(); + s.execute("cell.set", json!({"cell": "A1", "input": "5"})).unwrap(); + // Large entries (like a `file.open` with its base64): the oldest go once the cap is passed. + let pad = "x".repeat(crate::JOURNAL_MAX_BYTES / 8); + for i in 0..12 { + s.execute("selection.set", json!({"range": "B2", "pad": pad, "n": i})).unwrap(); + } + let bytes: usize = s.journal.iter().map(|e| serde_json::to_vec(e).unwrap().len()).sum(); + assert!(bytes <= crate::JOURNAL_MAX_BYTES, "{bytes}"); + assert!(s.journal.len() < 12, "{}", s.journal.len()); + assert_eq!(s.journal.last().unwrap().1["n"], 11, "the newest entries stay"); + // Small entries are kept as they were. + s.execute("cell.set", json!({"cell": "A2", "input": "=SUM(A1)"})).unwrap(); + assert_eq!(s.journal.last().unwrap(), &("cell.set".to_string(), json!({"cell": "A2", "input": "=SUM(A1)"}))); +} + +#[test] +fn recording_keeps_its_steps_when_the_journal_is_trimmed() { + let mut s = s(); + for _ in 0..9_990 { + s.execute("selection.set", json!({"range": "A1"})).unwrap(); + } + s.execute("automate.recordActions", json!({})).unwrap(); + // The 10th entry past the start overflows the 10,000-entry cap and drops the oldest half. + for i in 0..20 { + s.execute("cell.set", json!({"cell": "B1", "input": format!("step{i}")})).unwrap(); + } + let r = s.execute("automate.recordActions", json!({"stop": true, "name": "Steps"})).unwrap(); + assert_eq!(r["steps"], 20, "{r}"); + let script = s.doc().unwrap().wb.names.iter().find(|n| n.name == "_sc_scripts").map(|n| n.formula.clone()).unwrap(); + assert!(script.contains("step0") && script.contains("step19") && !script.contains("selection.set"), "{script}"); +} diff --git a/docs/control-protocol.md b/docs/control-protocol.md index 7cc6ec230..94e31ddab 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -21,7 +21,7 @@ Errors come back as `{"id": …, "ok": false, "error": "…"}`. |---|---|---| | `engine.execute` | `{command, params}` | Runs any engine command (see `engine.commands` or `gridcraft-cli commands`) | | `engine.commands` | `{search?}` | Every command: id, label, ribbon path, shortcut, params doc, enabled; `search` keeps those whose id, label or ribbon path contains it (case-insensitive) | -| `engine.journal` | | Commands run so far (replayable). Passwords (`password`, `…Password` params) are left out, so a replayed Protect Sheet has no password | +| `engine.journal` | | Commands run so far (replayable). Passwords (`password`, `…Password` params) are left out, so a replayed Protect Sheet has no password. Capped at 10,000 entries and 64 MB (serialized); past either cap the oldest entries go | | `document.inspect` | | Workbook summary: sheets, used ranges, tables, charts, names, selection, undo labels | | `sheet.read` | `{range?, sheet?, formulas?, formatted?}` | Values (or formulas / displayed text) of a range | | `cell.get` | `{cell?}` | One cell: value, formula, displayed text, style, merge, comment, link | From 72db972de306a165af5296cb2e5f932a19c0cc02 Mon Sep 17 00:00:00 2001 From: derechopiano <257026466+derechopiano@users.noreply.github.com> Date: Wed, 14 Oct 2026 08:23:57 +0100 Subject: [PATCH 06/15] fix(engine): bound Undo history by retained cell storage (#1227) --- crates/engine/src/cmd/edit.rs | 10 ++++--- crates/engine/src/lib.rs | 44 ++++++++++++++++++++++++++++--- crates/engine/src/tests.rs | 44 +++++++++++++++++++++++++++++++ crates/model/src/store.rs | 49 +++++++++++++++++++++++++++++++++++ docs/architecture.md | 6 ++++- 5 files changed, 144 insertions(+), 9 deletions(-) diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 82dbe0be7..30effd9fa 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -601,11 +601,12 @@ fn undo(s: &mut Session, p: &Json) -> Result { let now = d.wb.clone(); for _ in 0..steps { let Some(e) = d.undo.pop() else { break }; - let cur = crate::HistoryEntry { label: e.label.clone(), wb: d.wb.clone(), selection: d.selection.clone() }; - d.redo.push(cur); + let cur_wb = d.wb.clone(); + let cur_selection = d.selection.clone(); label = e.label.clone(); d.wb = e.wb; d.selection = e.selection; + crate::push_history(&mut d.redo, label.clone(), cur_wb, &d.wb, cur_selection); } super::extra::keep_watch_list(&now, &mut d.wb); d.calc.rebuild(&d.wb); @@ -620,11 +621,12 @@ fn redo(s: &mut Session, p: &Json) -> Result { let now = d.wb.clone(); for _ in 0..steps { let Some(e) = d.redo.pop() else { break }; - let cur = crate::HistoryEntry { label: e.label.clone(), wb: d.wb.clone(), selection: d.selection.clone() }; - d.undo.push(cur); + let cur_wb = d.wb.clone(); + let cur_selection = d.selection.clone(); label = e.label.clone(); d.wb = e.wb; d.selection = e.selection; + crate::push_history(&mut d.undo, label.clone(), cur_wb, &d.wb, cur_selection); } super::extra::keep_watch_list(&now, &mut d.wb); d.calc.rebuild(&d.wb); diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 258a1cb72..b97dd23c6 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -61,6 +61,8 @@ pub struct HistoryEntry { pub label: String, pub wb: Arc, pub selection: Selection, + /// Estimate of storage this snapshot retains beyond the next workbook version. + pub retained_bytes: usize, } /// The clipboard: a block of cells (with formulas and styles) or marching-ants reference. @@ -116,6 +118,43 @@ pub struct DocState { static NEXT_UID: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(1); const UNDO_LIMIT: usize = 100; +const UNDO_BYTES_LIMIT: usize = 128 * 1024 * 1024; + +fn history_retained_bytes(older: &Workbook, newer: &Workbook) -> usize { + let mut bytes = std::mem::size_of::(); + for (index, sheet) in older.sheets.iter().enumerate() { + // Moving or inserting sheets must not charge unchanged sheets at a new index. + if newer.sheets.iter().any(|next| Arc::ptr_eq(sheet, next)) { + continue; + } + bytes = bytes.saturating_add(std::mem::size_of::()); + let empty = gridcraft_model::CellStore::new(); + let next = newer.sheets.get(index).map_or(&empty, |next| &next.cells); + bytes = bytes.saturating_add(sheet.cells.retained_bytes_against(next)); + } + bytes +} + +fn push_history(history: &mut Vec, label: String, wb: Arc, next: &Workbook, selection: Selection) { + push_history_with_budget(history, label, wb, next, selection, UNDO_BYTES_LIMIT); +} + +fn push_history_with_budget( + history: &mut Vec, + label: String, + wb: Arc, + next: &Workbook, + selection: Selection, + byte_budget: usize, +) { + let retained_bytes = history_retained_bytes(&wb, next); + history.push(HistoryEntry { label, wb, selection, retained_bytes }); + let mut total = history.iter().fold(0usize, |sum, entry| sum.saturating_add(entry.retained_bytes)); + // Always preserve the latest action, even if that one edit exceeds the budget. + while history.len() > 1 && (history.len() > UNDO_LIMIT || total > byte_budget) { + total = total.saturating_sub(history.remove(0).retained_bytes); + } +} impl DocState { pub fn new(mut wb: Workbook, path: Option, title: String) -> DocState { @@ -350,10 +389,7 @@ impl Session { && let Some(d) = self.docs.iter_mut().find(|d| d.uid == uid) && !Arc::ptr_eq(&d.wb, &wb) { - d.undo.push(HistoryEntry { label: spec.label.trim_end_matches('…').to_string(), wb, selection: sel }); - if d.undo.len() > UNDO_LIMIT { - d.undo.remove(0); - } + push_history(&mut d.undo, spec.label.trim_end_matches('…').to_string(), wb, &d.wb, sel); d.redo.clear(); d.revision += 1; } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index ae499a189..c4fb1cc8e 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -253,6 +253,50 @@ fn undo_redo() { assert!(s.doc().unwrap().is_dirty()); } +#[test] +fn bulk_formula_history_is_bounded_and_latest_edit_still_undoes() { + use std::sync::Arc; + + use gridcraft_model::{Cell, Formula, Workbook}; + + let mut current = Arc::new(Workbook::new()); + let mut history = Vec::new(); + let budget = 64 * 1024; + for version in 0..5 { + let previous = current.clone(); + let mut next = (*previous).clone(); + let sheet = next.sheet_mut(0).unwrap(); + for row in 0..128 { + sheet.cells.set(CellRef::new(row, 0), Cell::formula(Formula::new(&format!("=B{}+{}", row + 1, version)))); + } + current = Arc::new(next); + crate::push_history_with_budget(&mut history, format!("edit {version}"), previous, ¤t, crate::Selection::default(), budget); + } + assert!(history.len() < 5, "bulk edits should evict old steps before reaching the count limit"); + assert_eq!(history.last().unwrap().label, "edit 4"); + assert!(history.len() == 1 || history.iter().map(|e| e.retained_bytes).sum::() <= budget); + + let mut session = s(); + let doc = session.doc_mut().unwrap(); + doc.wb = current; + doc.undo = history; + session.execute("edit.undo", json!({})).unwrap(); + assert_eq!(session.doc().unwrap().wb.active().unwrap().cells.get(CellRef::new(0, 0)).unwrap().formula.as_ref().unwrap().text, "B1+3"); + session.execute("edit.redo", json!({})).unwrap(); + assert_eq!(session.doc().unwrap().wb.active().unwrap().cells.get(CellRef::new(0, 0)).unwrap().formula.as_ref().unwrap().text, "B1+4"); +} + +#[test] +fn small_edits_keep_the_full_undo_depth() { + let mut session = s(); + for number in 0..100 { + session.execute("cell.set", json!({"cell": "A1", "input": number.to_string()})).unwrap(); + } + assert_eq!(session.doc().unwrap().undo.len(), 100); + session.execute("edit.undo", json!({"steps": 100})).unwrap(); + assert_eq!(v(&session, "A1"), Value::Empty); +} + #[test] fn shape_text_edit_undo_redo_and_noop() { let mut s = s(); diff --git a/crates/model/src/store.rs b/crates/model/src/store.rs index d6a9c4561..939358764 100644 --- a/crates/model/src/store.rs +++ b/crates/model/src/store.rs @@ -154,6 +154,55 @@ impl CellStore { } out } + /// Approximate heap storage kept alive by this snapshot's bands that `newer` no longer + /// shares. This is for bounding Undo history, not for reporting process memory: BTreeMap + /// nodes and allocator overhead vary by platform. Shared formulas and values are counted + /// only when the newer band no longer holds the same Arc. + pub fn retained_bytes_against(&self, newer: &CellStore) -> usize { + use std::mem::size_of; + + let mut bytes = 0usize; + for (key, band) in &self.bands { + let next = newer.bands.get(key); + if next.is_some_and(|b| Arc::ptr_eq(b, band)) { + continue; + } + bytes = bytes.saturating_add(size_of::()); + for (row, cols) in &band.rows { + bytes = bytes.saturating_add(size_of::() + 3 * size_of::()); + for (col, cell) in cols { + // Cells and BTreeMap nodes are duplicated with a cloned band even when + // their formula/value Arcs are still shared with the next snapshot. + bytes = bytes.saturating_add(size_of::() + size_of::() + 3 * size_of::()); + let next_cell = next.and_then(|b| b.rows.get(row)).and_then(|r| r.get(col)); + if let Some(formula) = &cell.formula + && !next_cell.and_then(|c| c.formula.as_ref()).is_some_and(|f| Arc::ptr_eq(f, formula)) + { + bytes = bytes.saturating_add(size_of::() + formula.text.len()); + if let Some(expr) = &formula.expr { + expr.walk(&mut |_| bytes = bytes.saturating_add(size_of::())); + } + } + match &cell.value { + gridcraft_core::Value::Text(text) + if !next_cell.and_then(|c| c.value.as_text()).is_some_and(|t| std::ptr::eq(t, text.as_ref())) => + { + bytes = bytes.saturating_add(text.len()); + } + gridcraft_core::Value::Array(array) + if !next_cell.is_some_and(|c| matches!(&c.value, gridcraft_core::Value::Array(a) if Arc::ptr_eq(a, array))) => + { + bytes = bytes.saturating_add( + size_of::() + array.data.len().saturating_mul(size_of::()), + ); + } + _ => {} + } + } + } + } + bytes + } /// Removes every cell in `r` and returns them. pub fn take_range(&mut self, r: RangeRef) -> Vec<(CellRef, Cell)> { let keys: Vec = self.iter_range(r).map(|(c, _)| c).collect(); diff --git a/docs/architecture.md b/docs/architecture.md index 5695036b2..80d1062d5 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -1,6 +1,6 @@ # Architecture -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-10 · **Change:** major (created from the code on main; supersedes the local-only plan/architecture.md for committed docs) · **Target:** Microsoft Excel (Microsoft 365) +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-14 · **Change:** minor (bound Undo's retained cell storage) · **Target:** Microsoft Excel (Microsoft 365) How GridCraft is built today. Rules for contributors are in [`AGENTS.md`](../AGENTS.md). @@ -35,6 +35,9 @@ including tests, 2026-10-10. `data.sortAscending`) plus primitives (`cell.set`, `selection.set`, `range.setValues`). `Session::execute` catches escaped panics and restores the workbook (never-crash guard). 2. **Edits** change the model through copy-on-write bands; undo keeps the previous snapshot. + Undo retains at most 100 steps and trims older steps when its estimated retained cell storage + exceeds 128 MiB. The newest step remains undoable even if that step alone exceeds the budget. + The estimate is a history policy, not a measure or hard cap of process memory. 3. **Recalc** marks dependents of changed cells dirty through the dependency graph (with range nodes for range dependents), evaluates in topological order, spills dynamic arrays and reports `#SPILL!`/cycles. Volatile functions are always dirty. @@ -68,4 +71,5 @@ rpm, tarball), FreeBSD, and web (WASM; WebGPU or WebGL2). Release workflows in | Date | Change | Summary | |---|---|---| +| 2026-10-14 | minor | Document Undo's retained-cell-storage budget and latest-step guarantee | | 2026-10-10 | major | Created from the code on main: crates, layers, data flow, agent control, known gaps | From 22157433ce1810c96b51dea337d3b07453bbebd5 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Thu, 15 Oct 2026 06:48:41 +0200 Subject: [PATCH 07/15] Fix fraction numerators saturating at u64::MAX for large values Found while testing crates/numfmt/src/render.rs:470 Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Opus 5.5 --- crates/numfmt/src/render.rs | 25 ++++++++++++++++--------- crates/numfmt/src/tests.rs | 12 ++++++++++++ 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/crates/numfmt/src/render.rs b/crates/numfmt/src/render.rs index 4408d383f..d9f78314e 100644 --- a/crates/numfmt/src/render.rs +++ b/crates/numfmt/src/render.rs @@ -466,20 +466,26 @@ fn render_fraction(sec: &Section, x: f64, pieces: &mut [String], lits: &mut [Opt let has_q = |idx: &[usize]| idx.iter().any(|&i| toks.get(i) == Some(&Tok::Digit('?'))); let has_whole = !whole_idx.is_empty(); let (mut whole, f) = if has_whole { (x.floor(), x - x.floor()) } else { (0.0, x) }; - let (mut num, den) = match fixed_den { - Some(d) => ((f * d as f64).round() as u64, d), + // Without a whole part the numerator carries the whole value and can pass u64::MAX, so it + // stays an f64 (a float-to-u64 cast saturates). + let (mut num, den): (f64, u64) = match fixed_den { + Some(d) => ((f * d as f64).round(), d), + None if f >= 1e15 => (f.round(), 1), None => { let digits = den_idx.len().clamp(1, 7) as u32; - approximate(f, 10u64.pow(digits) - 1) + let (n, d) = approximate(f, 10u64.pow(digits) - 1); + (n as f64, d) } }; - if has_whole && num >= den && den > 0 { - whole += (num / den) as f64; - num %= den; + if has_whole && num >= den as f64 && den > 0 { + // The fractional part is below 1, so the numerator is at most the denominator here. + let n = num as u64; + whole += (n / den) as f64; + num = (n % den) as f64; } let whole_s = if whole >= 1.0 { fixed(whole, 0).0 } else { String::new() }; let num_zero_ph = num_idx.iter().any(|&i| toks.get(i) == Some(&Tok::Digit('0'))); - if has_whole && num == 0 && !num_zero_ph { + if has_whole && num == 0.0 && !num_zero_ph { fill_int(toks, &whole_idx, &whole_s, sec.thousands, true, pieces); let spaces = has_q(&whole_idx) || has_q(&num_idx) || has_q(&den_idx); blank_range(toks, sep_start..den_end, spaces, pieces, lits); @@ -488,7 +494,7 @@ fn render_fraction(sec: &Section, x: f64, pieces: &mut [String], lits: &mut [Opt if has_whole { // A zero value shows `0` in a `?` or `0` units place, nothing in a `#` one. let units_hash = whole_idx.last().is_some_and(|&i| toks.get(i) == Some(&Tok::Digit('#'))); - let force = whole_s.is_empty() && num == 0 && !units_hash; + let force = whole_s.is_empty() && num == 0.0 && !units_hash; fill_int(toks, &whole_idx, &whole_s, sec.thousands, force, pieces); let whole_shown = whole_idx.iter().any(|&i| pieces.get(i).is_some_and(|p| p.chars().any(|c| c.is_ascii_digit()))); if !whole_shown { @@ -496,7 +502,8 @@ fn render_fraction(sec: &Section, x: f64, pieces: &mut [String], lits: &mut [Opt blank_range(toks, sep_start..num_start, spaces, pieces, lits); } } - fill_int(toks, &num_idx, &num.to_string(), false, true, pieces); + let num_s = if num < 1e15 { (num as u64).to_string() } else { fixed(num, 0).0 }; + fill_int(toks, &num_idx, &num_s, false, true, pieces); if let Some(p) = pieces.get_mut(slash) { *p = "/".into(); } diff --git a/crates/numfmt/src/tests.rs b/crates/numfmt/src/tests.rs index e5687f5bd..c92553271 100644 --- a/crates/numfmt/src/tests.rs +++ b/crates/numfmt/src/tests.rs @@ -145,6 +145,18 @@ fn fractions() { assert_eq!(f("# ???/???", 0.5), " 1/2 "); } +/// A fraction without a whole part puts the whole value in the numerator; large values must +/// not saturate at u64::MAX. +#[test] +fn fraction_numerator_beyond_u64() { + assert_eq!(f("?/1000000000", 2e10), "20000000000000000000/1000000000"); + assert_eq!(f("?/?", 1e20), "100000000000000000000/1"); + assert_eq!(f("?/100", 1e18), "100000000000000000000/100"); + assert_eq!(f("??/??", 1e25), "10000000000000000000000000/1 "); + assert!(!f("?/10", 1e300).contains("18446744073709551615")); + assert_eq!(f("?/1000000000", 2.5), "2500000000/1000000000"); +} + /// Fraction sections with literal text, as Excel renders them (observed `TEXT()` results). #[test] fn fractions_with_literals() { From edcc76d0a85d3af238727a32cc211ec81ef6172a Mon Sep 17 00:00:00 2001 From: TheRenegadeDev Date: Fri, 16 Oct 2026 20:39:34 -0500 Subject: [PATCH 08/15] Fail CLI checks on incomplete or empty input discovery (#1051) --- apps/gridcraft-cli/src/check.rs | 137 ++++++++++++++++++++++++-------- apps/gridcraft-cli/tests/cli.rs | 32 ++++++++ docs/cli.md | 6 +- 3 files changed, 140 insertions(+), 35 deletions(-) diff --git a/apps/gridcraft-cli/src/check.rs b/apps/gridcraft-cli/src/check.rs index 2b8529876..6341bb068 100644 --- a/apps/gridcraft-cli/src/check.rs +++ b/apps/gridcraft-cli/src/check.rs @@ -139,7 +139,7 @@ impl RoundTrip { #[derive(Clone, Debug, Default, PartialEq)] pub struct FileReport { pub path: String, - /// GridCraft couldn't open the file: its message. + /// Opening the file or discovering the requested inputs failed: its message. pub error: Option, pub warnings: usize, pub calc: CalcReport, @@ -204,67 +204,112 @@ pub fn main(args: &[String]) -> ExitCode { Err(_) => return usage(format!("--max-report needs a whole number, not `{n}`")), } } - let files = collect_inputs(&a.positional); - let reports: Vec = files - .iter() - .map(|p| { - let path = p.to_string_lossy().into_owned(); - match gridcraft_engine::io::read_file(&path) { - Ok(bytes) => check_bytes(&path, &bytes, &opts), - Err(e) => FileReport { path, error: Some(e.to_string()), ..Default::default() }, - } - }) - .collect(); + let Discovery { files, failures: mut reports } = collect_inputs(&a.positional); + reports.extend(files.iter().map(|p| { + let path = p.to_string_lossy().into_owned(); + match gridcraft_engine::io::read_file(&path) { + Ok(bytes) => check_bytes(&path, &bytes, &opts), + Err(e) => FileReport { path, error: Some(e.to_string()), ..Default::default() }, + } + })); if a.has("json") { outln!("{}", pretty(&report_json(&reports, &opts))); } else { print_human(&reports, &opts); } - if reports.iter().all(FileReport::is_clean) { ExitCode::SUCCESS } else { ExitCode::FAILURE } + if !files.is_empty() && reports.iter().all(FileReport::is_clean) { ExitCode::SUCCESS } else { ExitCode::FAILURE } +} + +#[derive(Default)] +struct Discovery { + files: Vec, + failures: Vec, +} + +impl Discovery { + fn fail(&mut self, path: &Path, error: String) { + self.failures.push(FileReport { path: path.to_string_lossy().into_owned(), error: Some(error), ..Default::default() }); + } + + /// False means the file budget is exhausted: the remaining scan is incomplete. + fn add_file(&mut self, path: PathBuf) -> bool { + if self.files.len() >= MAX_FILES { + self.fail(&path, format!("file limit of {MAX_FILES} reached; this and remaining inputs were not checked")); + return false; + } + self.files.push(path); + true + } } /// Files named on the command line are checked whatever their extension; directories contribute /// their `.xlsx`/`.xlsm` files, recursively, in name order. Hidden files and Excel lock files /// (`~$…`) are skipped. -fn collect_inputs(inputs: &[String]) -> Vec { - let mut out = Vec::new(); +fn collect_inputs(inputs: &[String]) -> Discovery { + let mut out = Discovery::default(); for i in inputs { let p = PathBuf::from(i); if p.is_dir() { - walk(&p, 0, &mut out); - } else { - // A missing path is reported as a file that failed to open. - out.push(p); - } - if out.len() >= MAX_FILES { - out.truncate(MAX_FILES); + if !walk(&p, 0, &mut out) { + break; + } + } else if !out.add_file(p) { break; } } + if out.files.is_empty() + && out.failures.is_empty() + && let Some(input) = inputs.first() + { + out.fail(Path::new(input), "no workbook files were found in the requested inputs".into()); + } out } -fn walk(dir: &Path, depth: usize, out: &mut Vec) { - if depth > MAX_DEPTH || out.len() >= MAX_FILES { - return; +/// Returns false only when the global file budget stops traversal. +fn walk(dir: &Path, depth: usize, out: &mut Discovery) -> bool { + if depth > MAX_DEPTH { + out.fail(dir, format!("directory depth limit of {MAX_DEPTH} exceeded; this subtree was not checked")); + return true; + } + let rd = match std::fs::read_dir(dir) { + Ok(rd) => rd, + Err(e) => { + out.fail(dir, format!("cannot read directory: {e}")); + return true; + } + }; + let mut entries = Vec::new(); + for entry in rd { + match entry { + Ok(entry) => entries.push(entry.path()), + Err(e) => out.fail(dir, format!("cannot read directory entry: {e}")), + } } - let Ok(rd) = std::fs::read_dir(dir) else { return }; - let mut entries: Vec = rd.flatten().map(|e| e.path()).collect(); entries.sort(); for p in entries { let name = p.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default(); if name.starts_with('.') || name.starts_with("~$") { continue; } - if p.is_dir() { - walk(&p, depth + 1, out); - } else if p.extension().and_then(|e| e.to_str()).is_some_and(|e| e.eq_ignore_ascii_case("xlsx") || e.eq_ignore_ascii_case("xlsm")) { - out.push(p); - } - if out.len() >= MAX_FILES { - return; + let metadata = match p.metadata() { + Ok(metadata) => metadata, + Err(e) => { + out.fail(&p, format!("cannot inspect input: {e}")); + continue; + } + }; + if metadata.is_dir() { + if !walk(&p, depth + 1, out) { + return false; + } + } else if p.extension().and_then(|e| e.to_str()).is_some_and(|e| e.eq_ignore_ascii_case("xlsx") || e.eq_ignore_ascii_case("xlsm")) + && !out.add_file(p) + { + return false; } } + true } /// Checks one file's bytes. Never fails: problems are recorded in the report. A panic escaping @@ -1270,6 +1315,30 @@ mod tests { use super::*; + #[test] + fn discovery_reports_a_directory_read_error() { + let path = std::env::temp_dir().join(format!("gridcraft-check-not-a-directory-{}", std::process::id())); + std::fs::write(&path, "not a directory").unwrap(); + let mut discovery = Discovery::default(); + assert!(walk(&path, 0, &mut discovery)); + assert!(discovery.files.is_empty()); + assert_eq!(discovery.failures.len(), 1); + assert_eq!(discovery.failures[0].path, path.to_string_lossy()); + assert!(discovery.failures[0].error.as_ref().unwrap().contains("cannot read directory")); + let _ = std::fs::remove_file(path); + } + + #[test] + fn discovery_reports_omissions_only_after_the_file_limit() { + let mut discovery = Discovery { files: vec![PathBuf::from("book.xlsx"); MAX_FILES - 1], ..Default::default() }; + assert!(discovery.add_file(PathBuf::from("last.xlsx"))); + assert!(discovery.failures.is_empty()); + assert!(!discovery.add_file(PathBuf::from("omitted.xlsx"))); + assert_eq!(discovery.files.len(), MAX_FILES); + assert_eq!(discovery.failures[0].path, "omitted.xlsx"); + assert!(discovery.failures[0].error.as_ref().unwrap().contains("file limit")); + } + fn at(a1: &str) -> CellRef { CellRef::parse(a1).unwrap() } diff --git a/apps/gridcraft-cli/tests/cli.rs b/apps/gridcraft-cli/tests/cli.rs index fb25b6320..3105819ec 100644 --- a/apps/gridcraft-cli/tests/cli.rs +++ b/apps/gridcraft-cli/tests/cli.rs @@ -174,3 +174,35 @@ fn check_exit_codes_and_json() { assert_eq!(run(&["check", book_s, "--tolerance", "x"]).status.code(), Some(2)); assert_eq!(run(&["check", book_s, "--bogus"]).status.code(), Some(2)); } + +#[test] +fn check_rejects_an_empty_scan() { + let dir = tmpdir("check-empty"); + std::fs::write(dir.join("notes.txt"), "not a workbook").unwrap(); + std::fs::write(dir.join(".hidden.xlsx"), "ignored").unwrap(); + std::fs::write(dir.join("~$locked.xlsx"), "ignored").unwrap(); + let out = run(&["check", dir.to_str().unwrap(), "--json"]); + assert_eq!(out.status.code(), Some(1)); + let report: serde_json::Value = serde_json::from_slice(&out.stdout).unwrap(); + assert!(report["files"][0]["error"].as_str().unwrap().contains("no workbook files")); + let _ = std::fs::remove_dir_all(dir); +} + +#[test] +fn check_reports_a_depth_limited_scan_while_checking_readable_files() { + let dir = tmpdir("check-depth"); + let book = dir.join("readable.xlsx"); + ok(&["run", "--out", book.to_str().unwrap(), "--quiet"]); + let mut deep = dir.clone(); + for _ in 0..34 { + deep.push("d"); + } + std::fs::create_dir_all(&deep).unwrap(); + std::fs::write(deep.join("skipped.xlsx"), "not reached").unwrap(); + let out = run(&["check", dir.to_str().unwrap(), "--json"]); + assert_eq!(out.status.code(), Some(1)); + let report: serde_json::Value = serde_json::from_slice(&out.stdout).unwrap(); + assert_eq!(report["totals"]["opened"], 1); + assert!(report["files"].as_array().unwrap().iter().any(|r| r["error"].as_str().is_some_and(|e| e.contains("depth limit")))); + let _ = std::fs::remove_dir_all(dir); +} diff --git a/docs/cli.md b/docs/cli.md index ec9578d60..321c6827d 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -102,7 +102,11 @@ sheet's rules in priority order, priority numbers aside), `validations`, `charts `hyperlinks`, `pivots`. `#SPILL!`/`#CALC!` compare equal to `#VALUE!` and `#CIRC!` to 0, as that's what the file format stores. -Files that fail to open are reported with GridCraft's error and don't stop the run. Exit code: 0 when every file +Files that fail to open are reported with GridCraft's error and don't stop the run. Unreadable directories, +directory-entry/metadata errors, and subtrees or files omitted by the depth (32) and file (100,000) limits appear +as failed path reports too, in both human and JSON output. Intentional hidden/lock-file exclusions remain ignored. +JSON `totals.files` includes failed discovery paths; `totals.opened` counts only workbooks that opened successfully. +An empty scan fails with a diagnostic. Exit code: 0 when at least one file was checked, discovery was complete, and every file opened with no mismatch and no round-trip difference, 1 otherwise, 2 on a usage error. `--max-report` caps the examples listed per file and category (counts are always complete). From 7776cfcc5caa66923f42b00438ca5279a2694163 Mon Sep 17 00:00:00 2001 From: zyzabruh Date: Mon, 19 Oct 2026 13:16:13 +0200 Subject: [PATCH 09/15] Accounting format: the documented currency `symbol` is honoured `home.accounting` documents `{range?, symbol?}` but ignored the symbol: the command always applied the `$` format, so a script or MCP client asking for lempiras (#1893) or any other currency silently got dollars. The symbol is now part of the code it writes, and a symbol that would break the format string (a quote or a backslash) is refused instead of being pasted in. The default is unchanged, character for character. Tests: `accounting_format_takes_the_symbol_it_is_given` (displayed text for `L` and the default, and the two refused symbols) and `the_default_symbol_reproduces_the_accounting_format` (the built code still equals the named `Accounting` format). `cargo xtask ci`: 8/8 steps, 0 failures. --- crates/engine/src/cmd/format.rs | 23 ++++++++++++++++++++--- crates/engine/src/tests.rs | 21 +++++++++++++++++++++ 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/cmd/format.rs b/crates/engine/src/cmd/format.rs index 4712351ca..7561b6ddc 100644 --- a/crates/engine/src/cmd/format.rs +++ b/crates/engine/src/cmd/format.rs @@ -133,9 +133,7 @@ pub fn specs() -> Vec { has_doc, number_format ), - cmd!("home.accounting", "Accounting Number Format", ["Home", "Number"], None, "{range?, symbol?: \"$\"}", has_doc, |s, p| set_fmt( - s, p, ACCOUNTING - )), + cmd!("home.accounting", "Accounting Number Format", ["Home", "Number"], None, "{range?, symbol?: \"$\"}", has_doc, accounting), cmd!("home.percent", "Percent Style", ["Home", "Number"], Some("Ctrl+Shift+%"), "{range?}", has_doc, |s, p| set_fmt(s, p, "0%")), cmd!("home.comma", "Comma Style", ["Home", "Number"], None, "{range?}", has_doc, |s, p| set_fmt(s, p, COMMA)), cmd!("home.increaseDecimal", "Increase Decimal", ["Home", "Number"], None, "{range?}", has_doc, |s, p| decimals(s, p, 1)), @@ -202,6 +200,20 @@ pub fn specs() -> Vec { const ACCOUNTING: &str = "_(\"$\"* #,##0.00_);_(\"$\"* \\(#,##0.00\\);_(\"$\"* \"-\"??_);_(@_)"; const COMMA: &str = "_(* #,##0.00_);_(* \\(#,##0.00\\);_(* \"-\"??_);_(@_)"; +/// Accounting Number Format with the currency symbol to use (Excel's own button takes the system +/// currency; `symbol` is how the control channel picks another one, e.g. `"L"` for lempiras). +fn accounting(s: &mut Session, p: &Json) -> Result { + let symbol = str_param(p, "symbol").unwrap_or("$"); + if symbol.contains(['"', '\\']) { + return Err(bad("home.accounting", "the symbol can't contain a quote or a backslash")); + } + set_fmt(s, p, &accounting_format(symbol)) +} + +fn accounting_format(symbol: &str) -> String { + format!("_({symbol:?}* #,##0.00_);_({symbol:?}* \\(#,##0.00\\);_({symbol:?}* \"-\"??_);_(@_)") +} + fn with(p: &Json, key: &str, v: Json) -> Json { let mut m = p.as_object().cloned().unwrap_or_default(); m.insert(key.into(), v); @@ -1137,6 +1149,11 @@ mod tests { assert_eq!(v(&mut s, "D1"), json!(4.0)); } + #[test] + fn the_default_symbol_reproduces_the_accounting_format() { + assert_eq!(accounting_format("$"), ACCOUNTING); + } + #[test] fn decimal_steps() { let v = gridcraft_core::Value::Number(1.5); diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index ae499a189..f4d17bd7e 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -1259,6 +1259,27 @@ fn slicer_filters_table() { assert!(s.execute("slicer.toggle", json!({"value": "Nowhere"})).is_err()); } +#[test] +fn accounting_format_takes_the_symbol_it_is_given() { + // The command documents `symbol`; it used to be dropped, so only `$` was ever reachable + // from the control channel (asking for lempiras, #1893, changed nothing). + let text = |s: &Session, a: &str| { + let d = s.doc().unwrap(); + crate::display::cell_text(&d.wb, d.wb.active().unwrap(), CellRef::parse(a).unwrap()) + }; + let mut s = s(); + s.execute("cell.set", json!({"cell": "A1", "input": "1234.5"})).unwrap(); + s.execute("home.accounting", json!({"range": "A1", "symbol": "L"})).unwrap(); + assert_eq!(text(&s, "A1"), " L 1,234.50 "); + // Without a symbol it stays the default `$` format, character for character. + s.execute("home.accounting", json!({"range": "A1"})).unwrap(); + assert_eq!(text(&s, "A1"), " $ 1,234.50 "); + // A quote or a backslash would break the format string, so the symbol is refused. + for bad in ["\"", "\\"] { + assert!(s.execute("home.accounting", json!({"range": "A1", "symbol": bad})).is_err(), "{bad}"); + } +} + #[test] fn table_header_formatted_as_accounting_keeps_its_text_as_the_column_name() { // Found by the command-sequence fuzzer: the accounting format pads text (` 172.4 `), and that From 041bee21688a082fde64de4aef85539c480b7aab Mon Sep 17 00:00:00 2001 From: zyzabruh Date: Mon, 19 Oct 2026 14:03:14 +0200 Subject: [PATCH 10/15] Accounting format: quote the symbol as format text, refuse control characters (review #1930) The symbol was interpolated with Rust's Debug formatting, which escapes a control character into literal `\n`-style text. It is now written into the format's quoted literal directly, and a symbol holding a control character is refused along with a quote or a backslash, so nothing is escaped behind the user's back. Test: `accounting_format_takes_the_symbol_it_is_given` covers `\n` and a tab. `cargo xtask ci`: 8/8 steps, 0 failures. --- crates/engine/src/cmd/format.rs | 9 ++++++--- crates/engine/src/tests.rs | 7 ++++--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/crates/engine/src/cmd/format.rs b/crates/engine/src/cmd/format.rs index 7561b6ddc..a935a6d01 100644 --- a/crates/engine/src/cmd/format.rs +++ b/crates/engine/src/cmd/format.rs @@ -204,14 +204,17 @@ const COMMA: &str = "_(* #,##0.00_);_(* \\(#,##0.00\\);_(* \"-\"??_);_(@_)"; /// currency; `symbol` is how the control channel picks another one, e.g. `"L"` for lempiras). fn accounting(s: &mut Session, p: &Json) -> Result { let symbol = str_param(p, "symbol").unwrap_or("$"); - if symbol.contains(['"', '\\']) { - return Err(bad("home.accounting", "the symbol can't contain a quote or a backslash")); + // The symbol goes into a quoted literal: a quote or a backslash would end it, and a control + // character would reach the format's text as-is. + if symbol.contains(['"', '\\']) || symbol.chars().any(char::is_control) { + return Err(bad("home.accounting", "the symbol can't contain a quote, a backslash or a control character")); } set_fmt(s, p, &accounting_format(symbol)) } fn accounting_format(symbol: &str) -> String { - format!("_({symbol:?}* #,##0.00_);_({symbol:?}* \\(#,##0.00\\);_({symbol:?}* \"-\"??_);_(@_)") + let symbol = format!("\"{symbol}\""); + format!("_({symbol}* #,##0.00_);_({symbol}* \\(#,##0.00\\);_({symbol}* \"-\"??_);_(@_)") } fn with(p: &Json, key: &str, v: Json) -> Json { diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index f4d17bd7e..97e460cf9 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -1274,9 +1274,10 @@ fn accounting_format_takes_the_symbol_it_is_given() { // Without a symbol it stays the default `$` format, character for character. s.execute("home.accounting", json!({"range": "A1"})).unwrap(); assert_eq!(text(&s, "A1"), " $ 1,234.50 "); - // A quote or a backslash would break the format string, so the symbol is refused. - for bad in ["\"", "\\"] { - assert!(s.execute("home.accounting", json!({"range": "A1", "symbol": bad})).is_err(), "{bad}"); + // A quote or a backslash would end the format's quoted text, and a control character would + // reach it as-is, so those symbols are refused. + for bad in ["\"", "\\", "\n", "L\t"] { + assert!(s.execute("home.accounting", json!({"range": "A1", "symbol": bad})).is_err(), "{bad:?}"); } } From ea00ca6b1fb69bda1607f4275fb34b30cf9bc924 Mon Sep 17 00:00:00 2001 From: Hamza Date: Mon, 19 Oct 2026 20:57:25 +0500 Subject: [PATCH 11/15] Control channel: a request its sender gave up on is not run later A request that waited more than 60 s was answered with a timeout and left in the queue, so `control::poll` ran it on the next UI pass and dropped the reply. The client was told its command had failed while the command was applied: an agent that retried after the error ran it twice, and one that gave up left an unexpected change in the workbook. `ControlRequest` now carries who has it -- pending, taken by the app, or abandoned by its sender -- as the report suggests. `poll` claims each request and skips one it cannot claim; the connection thread abandons the request when its reply times out, and says which happened: still queued timed out waiting for the UI (is the window minimised?) already started timed out waiting for the UI, which had already started the request, so it may still finish; its outcome is unknown The second case cannot be called a failure without guessing. This is the shape DeckCraft settled on for the same bug (its #719), so the two apps now agree. Fix: #1914 --- apps/gridcraft/src/control_server.rs | 15 ++++-- crates/ui-egui/src/control.rs | 30 +++++++++++- crates/ui-egui/tests/control_timeout.rs | 61 +++++++++++++++++++++++++ 3 files changed, 102 insertions(+), 4 deletions(-) create mode 100644 crates/ui-egui/tests/control_timeout.rs diff --git a/apps/gridcraft/src/control_server.rs b/apps/gridcraft/src/control_server.rs index 4c57ccf41..a82541c10 100644 --- a/apps/gridcraft/src/control_server.rs +++ b/apps/gridcraft/src/control_server.rs @@ -119,14 +119,23 @@ fn serve(stream: TcpStream, tx: Sender, ctx: egui::Context) { Line::Reject(e) => e, Line::Request { id, method, params } => { let (req, rx) = ControlRequest::new(method, params); + let state = std::sync::Arc::clone(&req.state); if tx.send(req).is_err() { break; } // Wake the UI thread: egui only runs frames when something happens. ctx.request_repaint(); - let mut reply = rx - .recv_timeout(Duration::from_secs(60)) - .unwrap_or_else(|_| json!({"ok": false, "error": "timed out waiting for the UI (is the window minimised?)"})); + let mut reply = rx.recv_timeout(Duration::from_secs(60)).unwrap_or_else(|_| { + // Give up on the request as well as on its reply. While it is still + // queued this keeps it from running late -- the client was told it had + // failed, and an agent that retried after the error ran it twice (#1914); + // if the app had already started it, its outcome is unknown instead. + if ControlRequest::abandon(&state) { + json!({"ok": false, "error": "timed out waiting for the UI (is the window minimised?)"}) + } else { + json!({"ok": false, "error": "timed out waiting for the UI, which had already started the request, so it may still finish; its outcome is unknown"}) + } + }); if let Some(o) = reply.as_object_mut() { o.insert("id".into(), id); } diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index 2ea5cfae0..ea5620edc 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -29,12 +29,35 @@ pub struct ControlRequest { pub method: String, pub params: Json, pub reply: Sender, + /// Who has the request: [`PENDING`], [`TAKEN`] (the app runs it) or [`ABANDONED`] (the sender + /// gave up waiting, so the app must not run it late: a client told its request timed out had + /// it applied anyway on the next UI pass, and an agent that retried ran it twice (#1914). + pub state: std::sync::Arc, } +/// Queued, not yet run. +pub const PENDING: u8 = 0; +/// The app has started it: it will answer. +pub const TAKEN: u8 = 1; +/// Its sender gave up waiting (the reply timeout expired): it is dropped unrun. +pub const ABANDONED: u8 = 2; + impl ControlRequest { pub fn new(method: impl Into, params: Json) -> (Self, std::sync::mpsc::Receiver) { let (tx, rx) = std::sync::mpsc::channel(); - (Self { method: method.into(), params, reply: tx }, rx) + (Self { method: method.into(), params, reply: tx, state: Default::default() }, rx) + } + + /// Claim the request for running; false when its sender already gave up on it. + pub fn take(&self) -> bool { + use std::sync::atomic::Ordering; + self.state.compare_exchange(PENDING, TAKEN, Ordering::SeqCst, Ordering::SeqCst).is_ok() || self.state.load(Ordering::SeqCst) == TAKEN + } + + /// Give up on the request; false when the app has already started it (it will answer). + pub fn abandon(state: &std::sync::atomic::AtomicU8) -> bool { + use std::sync::atomic::Ordering; + state.compare_exchange(PENDING, ABANDONED, Ordering::SeqCst, Ordering::SeqCst).is_ok() || state.load(Ordering::SeqCst) == ABANDONED } } @@ -391,6 +414,11 @@ pub fn handle(app: &mut SheetApp, ctx: &egui::Context, method: &str, p: &Json) - pub fn poll(app: &mut SheetApp, ctx: &egui::Context) { let Some(rx) = app.control_rx.take() else { return }; while let Ok(req) = rx.try_recv() { + // A request whose sender stopped waiting is dropped unrun: running it would apply a + // command the client was told had failed, and its reply would go nowhere (#1914). + if !req.take() { + continue; + } let outcome = handle(app, ctx, &req.method, &req.params); match outcome { Outcome::Done(v) => { diff --git a/crates/ui-egui/tests/control_timeout.rs b/crates/ui-egui/tests/control_timeout.rs new file mode 100644 index 000000000..f4e3c947d --- /dev/null +++ b/crates/ui-egui/tests/control_timeout.rs @@ -0,0 +1,61 @@ +//! A control request whose sender stopped waiting is dropped unrun (#1914). + +use gridcraft_engine::Session; +use gridcraft_engine::core::CellRef; +use gridcraft_ui_egui::{ControlRequest, SheetApp}; +use serde_json::json; + +fn app() -> (SheetApp, egui::Context) { + let mut s = Session::new(); + s.new_workbook(); + (SheetApp::new(s, Default::default()), egui::Context::default()) +} + +fn a1(app: &SheetApp) -> String { + let d = app.session.active().unwrap(); + d.wb.active().unwrap().cell(CellRef::parse("A1").unwrap()).map(|c| format!("{:?}", c.value)).unwrap_or_else(|| "empty".into()) +} + +/// `set` writes A1, so a request that did run is visible in the workbook. +fn set_a1(text: &str) -> serde_json::Value { + json!({"command": "cell.set", "params": {"cell": "A1", "input": text}}) +} + +#[test] +fn a_request_its_sender_gave_up_on_does_not_run_later() { + let (mut app, ctx) = app(); + let (tx, rx) = std::sync::mpsc::channel(); + app.control_rx = Some(rx); + + // One request the sender is still waiting for, and one it gave up on after its reply timed + // out. Before the fix both ran on the next UI pass: the client was told its command had + // failed and the command was applied anyway, so an agent that retried ran it twice. + let (kept, kept_rx) = ControlRequest::new("engine.execute", set_a1("kept")); + tx.send(kept).unwrap(); + gridcraft_ui_egui::control::poll(&mut app, &ctx); + assert!(kept_rx.try_recv().is_ok(), "a request still waited for is answered"); + assert_eq!(a1(&app), "Text(\"kept\")"); + + let (dropped, dropped_rx) = ControlRequest::new("engine.execute", set_a1("gave up")); + assert!(ControlRequest::abandon(&dropped.state), "the sender gives up while it is queued"); + tx.send(dropped).unwrap(); + gridcraft_ui_egui::control::poll(&mut app, &ctx); + assert_eq!(a1(&app), "Text(\"kept\")", "the abandoned request did not run"); + assert!(dropped_rx.try_recv().is_err(), "and nothing was replied to it"); +} + +/// Which of the two sides wins the race is settled once: a request the app has started is not +/// abandoned (the app will answer, so the sender reports an unknown outcome instead of a failure), +/// and one already abandoned is never started. +#[test] +fn taking_and_abandoning_a_request_settle_once() { + let (taken, _rx) = ControlRequest::new("engine.execute", json!({})); + assert!(taken.take(), "a pending request can be taken"); + assert!(taken.take(), "and taking it again is the same answer"); + assert!(!ControlRequest::abandon(&taken.state), "it cannot then be abandoned"); + + let (given_up, _rx) = ControlRequest::new("engine.execute", json!({})); + assert!(ControlRequest::abandon(&given_up.state), "a pending request can be abandoned"); + assert!(ControlRequest::abandon(&given_up.state), "and again is the same answer"); + assert!(!given_up.take(), "it cannot then be taken"); +} From 4faca498365a0ece04e5f120eddaac2bd24c7166 Mon Sep 17 00:00:00 2001 From: Mouhand-Kaddo Date: Mon, 19 Oct 2026 16:37:09 +0400 Subject: [PATCH 12/15] M12.AR13: read Windows-1256 Arabic CSV and text files as Arabic --- crates/xlsx/src/csv.rs | 122 +++++++++++++++++++++++++++-- crates/xlsx/src/tests/csv_tests.rs | 95 ++++++++++++++++++++++ docs/file-format-parity.md | 11 +-- docs/localization-parity.md | 3 +- 4 files changed, 217 insertions(+), 14 deletions(-) diff --git a/crates/xlsx/src/csv.rs b/crates/xlsx/src/csv.rs index 47486bfa2..0e0e54c28 100644 --- a/crates/xlsx/src/csv.rs +++ b/crates/xlsx/src/csv.rs @@ -17,7 +17,107 @@ const CP1252: [u32; 32] = [ 0x201C, 0x201D, 0x2022, 0x2013, 0x2014, 0x02DC, 0x2122, 0x0161, 0x203A, 0x0153, 0x9D, 0x017E, 0x0178, ]; -/// Decodes CSV bytes: UTF-8/UTF-16 BOMs, else UTF-8, else Windows-1252. +/// Windows-1256 code points for bytes 0x80–0xFF (unicode.org `CP1256.TXT`). +pub(crate) const CP1256: [u32; 128] = [ + 0x20AC, 0x067E, 0x201A, 0x0192, 0x201E, 0x2026, 0x2020, 0x2021, 0x02C6, 0x2030, 0x0679, 0x2039, 0x0152, 0x0686, 0x0698, 0x0688, 0x06AF, 0x2018, + 0x2019, 0x201C, 0x201D, 0x2022, 0x2013, 0x2014, 0x06A9, 0x2122, 0x0691, 0x203A, 0x0153, 0x200C, 0x200D, 0x06BA, 0x00A0, 0x060C, 0x00A2, 0x00A3, + 0x00A4, 0x00A5, 0x00A6, 0x00A7, 0x00A8, 0x00A9, 0x06BE, 0x00AB, 0x00AC, 0x00AD, 0x00AE, 0x00AF, 0x00B0, 0x00B1, 0x00B2, 0x00B3, 0x00B4, 0x00B5, + 0x00B6, 0x00B7, 0x00B8, 0x00B9, 0x061B, 0x00BB, 0x00BC, 0x00BD, 0x00BE, 0x061F, 0x06C1, 0x0621, 0x0622, 0x0623, 0x0624, 0x0625, 0x0626, 0x0627, + 0x0628, 0x0629, 0x062A, 0x062B, 0x062C, 0x062D, 0x062E, 0x062F, 0x0630, 0x0631, 0x0632, 0x0633, 0x0634, 0x0635, 0x0636, 0x00D7, 0x0637, 0x0638, + 0x0639, 0x063A, 0x0640, 0x0641, 0x0642, 0x0643, 0x00E0, 0x0644, 0x00E2, 0x0645, 0x0646, 0x0647, 0x0648, 0x00E7, 0x00E8, 0x00E9, 0x00EA, 0x00EB, + 0x0649, 0x064A, 0x00EE, 0x00EF, 0x064B, 0x064C, 0x064D, 0x064E, 0x00F4, 0x064F, 0x0650, 0x00F7, 0x0651, 0x00F9, 0x0652, 0x00FB, 0x00FC, 0x200E, + 0x200F, 0x06D2, +]; + +/// Evidence that bytes which aren't UTF-8 are Windows-1256 Arabic rather than Windows-1252. +#[derive(Default)] +struct Cp1256Evidence { + /// High bytes that decode to different characters in the two code pages, apart from invisible + /// marks and ordinals after a digit. + differing: usize, + /// Of those, Arabic letters and Arabic punctuation (، ؛ ؟) in Windows-1256. + arabic: usize, + /// Arabic bytes next to an ASCII letter: Western accents sit inside Latin words, Arabic doesn't. + arabic_touching_latin: usize, + /// Accented Latin bytes the two code pages share, next to another high byte: Hebrew + /// Windows-1255 letters land there, Arabic and Western text rarely do. + shared_letters_in_runs: usize, +} + +impl Cp1256Evidence { + fn of(bytes: &[u8]) -> Self { + let mut e = Self::default(); + for (i, &b) in bytes.iter().enumerate() { + if b < 0x80 { + continue; + } + let before = i.checked_sub(1).and_then(|j| bytes.get(j)).copied(); + let after = bytes.get(i + 1).copied(); + let next_to = |pred: fn(u8) -> bool| before.is_some_and(pred) || after.is_some_and(pred); + if same_in_cp1252_and_cp1256(b) { + if is_shared_latin_letter(b) && next_to(|x| x >= 0x80) { + e.shared_letters_in_runs += 1; + } + continue; + } + if is_invisible_mark(b) || is_ordinal_after_digit(b, before) { + continue; + } + e.differing += 1; + if is_cp1256_arabic(b) { + e.arabic += 1; + if next_to(|x| x.is_ascii_alphabetic()) { + e.arabic_touching_latin += 1; + } + } + } + e + } + + /// Thresholds chosen against month, day and language names from 95 glibc locales (Western in + /// Windows-1252, Arabic, Persian and Urdu in 1256, Hebrew and Yiddish in 1255) plus CSV samples. + fn is_arabic(&self) -> bool { + self.arabic >= 2 + && self.arabic * 10 >= self.differing * 9 + && self.arabic_touching_latin * 10 <= self.arabic * 3 + && self.shared_letters_in_runs * 20 <= self.arabic + } +} + +fn same_in_cp1252_and_cp1256(b: u8) -> bool { + matches!( + b, + 0x80 | 0x82..=0x89 | 0x8B..=0x8C | 0x91..=0x97 | 0x99 | 0x9B..=0x9C | 0xA0 | 0xA2..=0xA9 | 0xAB..=0xB9 | 0xBB..=0xBE | 0xD7 | 0xE0 | 0xE2 | 0xE7..=0xEB + | 0xEE..=0xEF | 0xF4 | 0xF7 | 0xF9 | 0xFB..=0xFC + ) +} + +/// ZWNJ, ZWJ, LRM and RLM: common inside Persian words and around Arabic cells, but no evidence +/// either way. +fn is_invisible_mark(b: u8) -> bool { + matches!(b, 0x9D | 0x9E | 0xFD | 0xFE) +} + +/// `º` and `ª` after a digit (`1º`, `2ª`) are Western ordinals, though Windows-1256 reads the bytes +/// as `؛` and `ھ`; either way they are no evidence. +fn is_ordinal_after_digit(b: u8, before: Option) -> bool { + matches!(b, 0xAA | 0xBA) && before.is_some_and(|x| x.is_ascii_digit()) +} + +fn is_shared_latin_letter(b: u8) -> bool { + matches!(b, 0xE0 | 0xE2 | 0xE7..=0xEB | 0xEE..=0xEF | 0xF4 | 0xF9 | 0xFB..=0xFC) +} + +fn is_cp1256_arabic(b: u8) -> bool { + matches!( + b, + 0x81 | 0x8A | 0x8D..=0x90 | 0x98 | 0x9A | 0x9F | 0xA1 | 0xAA | 0xBA | 0xBF..=0xD6 | 0xD8..=0xDB | 0xDD..=0xDF | 0xE1 | 0xE3..=0xE6 | 0xEC..=0xED | 0xFF + ) +} + +/// Decodes CSV bytes with BOM-aware UTF-8 and UTF-16 handling. Without a BOM, valid UTF-8 is kept. +/// Other bytes decode as Windows-1256 when they read as Arabic (see [`Cp1256Evidence`]), else as +/// Windows-1252. pub fn decode_text(bytes: &[u8]) -> String { if let Some(rest) = bytes.strip_prefix(&[0xEF, 0xBB, 0xBF]) { return String::from_utf8_lossy(rest).into_owned(); @@ -34,13 +134,19 @@ pub fn decode_text(bytes: &[u8]) -> String { } match std::str::from_utf8(bytes) { Ok(s) => s.to_string(), - Err(_) => bytes - .iter() - .map(|&b| match b { - 0x80..=0x9F => char::from_u32(CP1252.get((b - 0x80) as usize).copied().unwrap_or(0xFFFD)).unwrap_or('\u{FFFD}'), - _ => b as char, - }) - .collect(), + Err(_) => { + let use_windows_1256 = Cp1256Evidence::of(bytes).is_arabic(); + bytes + .iter() + .map(|&b| match b { + 0x80..=0xFF if use_windows_1256 => { + char::from_u32(CP1256.get((b - 0x80) as usize).copied().unwrap_or(0xFFFD)).unwrap_or('\u{FFFD}') + } + 0x80..=0x9F => char::from_u32(CP1252.get((b - 0x80) as usize).copied().unwrap_or(0xFFFD)).unwrap_or('\u{FFFD}'), + _ => b as char, + }) + .collect() + } } } diff --git a/crates/xlsx/src/tests/csv_tests.rs b/crates/xlsx/src/tests/csv_tests.rs index cc52072e7..daaa81b80 100644 --- a/crates/xlsx/src/tests/csv_tests.rs +++ b/crates/xlsx/src/tests/csv_tests.rs @@ -68,6 +68,101 @@ fn encodings() { assert_eq!(v(&wb, 0, 1), Value::Number(1.0)); } +#[test] +fn windows_1256_table_matches_the_unicode_mapping() { + let at = |b: u8| crate::csv::CP1256.get(usize::from(b - 0x80)).and_then(|&c| char::from_u32(c)); + let expected = [ + (0x80, '€'), + (0x81, 'پ'), + (0x8D, 'چ'), + (0x98, 'ک'), + (0x9D, '\u{200C}'), + (0xA1, '،'), + (0xBA, '؛'), + (0xBF, '؟'), + (0xC7, 'ا'), + (0xC8, 'ب'), + (0xDC, 'ـ'), + (0xE1, 'ل'), + (0xE3, 'م'), + (0xE9, 'é'), + (0xFD, '\u{200E}'), + (0xFE, '\u{200F}'), + (0xFF, 'ے'), + ]; + for (b, c) in expected { + assert_eq!(at(b), Some(c), "0x{b:02X}"); + } +} + +#[test] +fn arabic_csv_in_windows_1256_reads_as_arabic() { + let bytes = [ + 0xC7, 0xE1, 0xC7, 0xD3, 0xE3, b',', 0xC7, 0xE1, 0xDE, 0xED, 0xE3, 0xC9, b'\n', 0xE3, 0xD1, 0xCD, 0xC8, 0xC7, b' ', b'1', b'2', b'3', 0xA1, + b' ', b'4', b'2', 0xBF, b',', 0xC8, 0xED, 0xC7, 0xE4, 0xC7, 0xCA, b' ', b'7', b'\n', + ]; + let wb = read_csv(&bytes, &CsvOptions::default()).unwrap(); + assert_eq!(v(&wb, 0, 0), Value::text("الاسم")); + assert_eq!(v(&wb, 0, 1), Value::text("القيمة")); + assert_eq!(v(&wb, 1, 0), Value::text("مرحبا 123، 42؟")); + assert_eq!(v(&wb, 1, 1), Value::text("بيانات 7")); +} + +fn reads_as_arabic(bytes: &[u8]) -> bool { + decode_text(bytes).chars().any(|c| ('\u{0600}'..='\u{06FF}').contains(&c)) +} + +#[test] +fn western_1252_text_stays_latin() { + let french = [ + b'C', b'r', 0xE8, b'm', b'e', b' ', b'b', b'r', 0xFB, b'l', 0xE9, b'e', b',', b' ', 0xE0, b' ', b'c', 0xF4, b't', 0xE9, b',', b' ', 0xE9, + b't', 0xE9, + ]; + let german = [ + b'G', b'r', 0xF6, 0xDF, b'e', b',', b' ', 0xDC, b'b', b'e', b'r', b'm', b'a', 0xDF, b',', b' ', b'F', b'u', 0xDF, b'g', 0xE4, b'n', b'g', + b'e', b'r', + ]; + assert_eq!(decode_text(&french), "Crème brûlée, à côté, été"); + assert_eq!(decode_text(&german), "Größe, Übermaß, Fußgänger"); + let portuguese = [ + b'A', 0xC7, 0xC3, b'O', b',', b' ', b'i', b'n', b'f', b'o', b'r', b'm', b'a', 0xE7, 0xE3, b'o', b',', b' ', b's', b'i', b't', b'u', b'a', + 0xE7, 0xE3, b'o', b',', b' ', b'P', 0xE3, b'o', + ]; + assert_eq!(decode_text(&portuguese), "AÇÃO, informação, situação, Pão"); + let nordic = [ + 0xC6, b'r', 0xF8, b',', 0xD8, b's', b't', b',', 0xC5, b's', b'e', b',', 0xDE, 0xF3, b'r', 0xF0, b'u', b'r', b',', b'G', b'u', 0xF0, b'r', + 0xFA, b'n', b',', 0xC4, 0xD6, 0xDC, + ]; + assert_eq!(decode_text(&nordic), "Ærø,Øst,Åse,Þórður,Guðrún,ÄÖÜ"); + let ordinals = [b'1', 0xBA, b',', b'2', 0xBA, b',', b'3', 0xBA, b'\n', b'1', 0xAA, b',', b'2', 0xAA]; + assert_eq!(decode_text(&ordinals), "1º,2º,3º\n1ª,2ª"); + let inside_words = [b'K', 0xE4, b's', b'e', b',', b'B', 0xE4, b'c', b'k', b'e', b'r']; + assert_eq!(decode_text(&inside_words), "Käse,Bäcker"); +} + +#[test] +fn short_arabic_words_read_as_arabic() { + assert_eq!(decode_text(&[0xDD, 0xED, b',', 0xE3, 0xE4, b',', 0xDA, 0xE4, b'\n']), "في,من,عن\n"); + assert_eq!(decode_text(&[0xE4, 0xDA, 0xE3, b',', 0xE1, 0xC7, b'\n', 0xE1, 0xC7, b',', 0xE4, 0xDA, 0xE3, b'\n']), "نعم,لا\nلا,نعم\n"); + assert_eq!(decode_text(&[0x98, 0xCA, 0xC7, 0xC8, b' ', 0x81, 0xC7, 0xD1, 0xD3, 0xED]), "کتاب پارسي"); + let with_marks = [ + 0xDF, 0xCA, 0xC7, 0xC8, 0x9D, 0xE5, 0xC7, b',', 0xCE, 0xC7, 0xE4, 0xE5, 0x9D, 0xE5, 0xC7, b',', 0xE3, 0xCD, 0xE3, 0xCF, 0xFE, b',', 0xDA, + 0xE1, 0xED, 0xFE, + ]; + assert_eq!(decode_text(&with_marks), "كتاب\u{200C}ها,خانه\u{200C}ها,محمد\u{200F},علي\u{200F}"); +} + +#[test] +fn hebrew_1255_bytes_are_not_read_as_arabic() { + let plain = [0xF9, 0xEC, 0xE5, 0xED, b',', 0xF2, 0xE5, 0xEC, 0xED]; + let pointed = [0xF9, 0xC8, 0xD1, 0xEC, 0xE5, 0xC9, 0xED, b' ', 0xF2, 0xE5, 0xC9, 0xEC, 0xC8, 0xED]; + let maqaf = [0xFA, 0xEC, 0xCE, 0xE0, 0xE1, 0xE9, 0xE1, b',', 0xE1, 0xEF, 0xCE, 0xE2, 0xE5, 0xF8, 0xE9, 0xE5, 0xEF]; + let gershayim = [0xF6, 0xE4, 0xD8, 0xEC, b',', 0xE3, 0xD8, 0xF8]; + for bytes in [&plain[..], &pointed, &maqaf, &gershayim] { + assert!(!reads_as_arabic(bytes), "{:?}", decode_text(bytes)); + } +} + #[test] fn delimiter_sniffing() { assert_eq!(sniff_delimiter("a;b;c\n1;2;3\n", '"'), b';'); diff --git a/docs/file-format-parity.md b/docs/file-format-parity.md index 5a7588986..804d81441 100644 --- a/docs/file-format-parity.md +++ b/docs/file-format-parity.md @@ -1,6 +1,6 @@ # File-format parity -> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-19 · **Change:** minor (.xls dialog sheets are skipped instead of read as worksheets; `.xls` import reads each sheet's display options from WINDOW2: right-to-left, gridlines, headings, zeros, formulas; XLSX text boxes and shapes keep their paragraphs' right-to-left direction on save; CSV/TSV/TXT can open with every field kept as text via `file.open {csvAsText}`; formulas over Excel's 8,192 characters, and `%` or call chains past the nesting limit, are parse errors instead of stack overflows when a file is opened; XLSX PivotTables read and write `rowGrandTotals`/`colGrandTotals` the way Excel does, so a table saved with only one grand total opens with the right one; HTML export keeps right-to-left) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-19 · **Change:** minor (.xls dialog sheets are skipped instead of read as worksheets; `.xls` import reads each sheet's display options from WINDOW2: right-to-left, gridlines, headings, zeros, formulas; XLSX text boxes and shapes keep their paragraphs' right-to-left direction on save; CSV/TSV/TXT can open with every field kept as text via `file.open {csvAsText}`; formulas over Excel's 8,192 characters, and `%` or call chains past the nesting limit, are parse errors instead of stack overflows when a file is opened; XLSX PivotTables read and write `rowGrandTotals`/`colGrandTotals` the way Excel does, so a table saved with only one grand total opens with the right one; HTML export keeps right-to-left; CSV and text import detects likely Windows-1256 Arabic when invalid UTF-8 has Arabic byte runs; other invalid UTF-8 remains Windows-1252) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 Every format Excel opens or saves, with GridCraft's read and write support, fidelity and tests. Part of [target-app-parity.md](target-app-parity.md). **File formats: ~50% ready, 80–120 h** @@ -41,10 +41,10 @@ and repair-free opening remain unverified. | Format | Excel | GridCraft read | GridCraft write | Notes | |---|---|---|---|---| -| `.csv` UTF-8 / Windows / Mac / MS-DOS | R/W | yes (UTF-8 and UTF-16 BOMs, UTF-8, Windows-1252 fallback; delimiter detection) | yes (UTF-8 without BOM, CRLF; a file opened as CSV is saved with the separator it was read with, so a `;` file stays `;`; otherwise `,`) | Excel writes the **displayed** values; we write raw values. No encoding choice on save. Re-import of a single-column export splits text (#127) A used range that would write more than 512 MiB (two far-apart cells span the whole rectangle) is refused with an error instead of being expanded (#528). | -| `.txt` tab-delimited, Unicode text | R/W | yes (`.txt`, `.tsv`, `.tab`) | yes | Text Import Wizard options partly via Text to Columns | -| `.csv` UTF-8 / Windows / Mac / MS-DOS | R/W | yes (UTF-8 and UTF-16 BOMs, UTF-8, Windows-1252 fallback; delimiter detection) | yes (UTF-8 without BOM, CRLF) | Writes the **displayed** values like Excel (number formats applied, formula results, never column-width `#####`). No encoding choice on save. Re-import of a single-column export splits text (#127) A used range that would write more than 512 MiB (two far-apart cells span the whole rectangle) is refused with an error instead of being expanded (#528). | -| `.txt` tab-delimited, Unicode text | R/W | yes (`.txt`, `.tsv`, `.tab`) | yes (displayed values) | Text Import Wizard options partly via Text to Columns | +| `.csv` UTF-8 / Windows / Mac / MS-DOS | R/W | yes (UTF-8 and UTF-16 BOMs, UTF-8, likely Windows-1256 Arabic detection, otherwise Windows-1252 fallback; delimiter detection) | yes (UTF-8 without BOM, CRLF; a file opened as CSV is saved with the separator it was read with, so a `;` file stays `;`; otherwise `,`) | Excel writes the **displayed** values; we write raw values. No encoding choice on save. Re-import of a single-column export splits text (#127) A used range that would write more than 512 MiB (two far-apart cells span the whole rectangle) is refused with an error instead of being expanded (#528). | +| `.txt` tab-delimited, Unicode text | R/W | yes (`.txt`, `.tsv`, `.tab`; likely Windows-1256 Arabic detection for invalid UTF-8) | yes | Text Import Wizard options partly via Text to Columns | +| `.csv` UTF-8 / Windows / Mac / MS-DOS | R/W | yes (UTF-8 and UTF-16 BOMs, UTF-8, likely Windows-1256 Arabic detection, otherwise Windows-1252 fallback; delimiter detection) | yes (UTF-8 without BOM, CRLF) | Writes the **displayed** values like Excel (number formats applied, formula results, never column-width `#####`). No encoding choice on save. Re-import of a single-column export splits text (#127) A used range that would write more than 512 MiB (two far-apart cells span the whole rectangle) is refused with an error instead of being expanded (#528). | +| `.txt` tab-delimited, Unicode text | R/W | yes (`.txt`, `.tsv`, `.tab`; likely Windows-1256 Arabic detection for invalid UTF-8) | yes (displayed values) | Text Import Wizard options partly via Text to Columns | | `.prn` formatted text (space-delimited) | R/W | no | no | | | `.dif`, `.slk` (SYLK) | R/W | no | no | Small, legacy | | `.htm` / `.html` web page | R/W (Windows) | yes, via Get Data ▸ From HTML | yes (one sheet as a table; a used range taller than 100,001 rows fails with an error instead of being cut short) | Formatting not exported | @@ -85,6 +85,7 @@ refresh. Counted under features (Get & Transform, ~10% ready), not here. | Date | Change | Summary | |---|---|---| +| 2026-10-19 | minor | CSV and text import decodes likely Windows-1256 Arabic; other invalid UTF-8 retains the Windows-1252 fallback | | 2026-10-19 | minor | .xls import skips dialog sheets (WsBool `fDialog`), which share the worksheet BOF type and were read as empty worksheets | | 2026-10-19 | minor | `.xls` import reads WINDOW2: right-to-left sheets (common in older Arabic and Hebrew files) open right-to-left, and hidden gridlines, headings and zero values and Show Formulas carry over | | 2026-10-19 | trivial | XLSX text boxes and shapes keep right-to-left paragraph direction (`a:pPr rtl`, ECMA-376 Part 1 §21.1.2.2.7): read from the first paragraph and written on every paragraph; output without it is unchanged | diff --git a/docs/localization-parity.md b/docs/localization-parity.md index 64876633d..e89c4d2b8 100644 --- a/docs/localization-parity.md +++ b/docs/localization-parity.md @@ -1,6 +1,6 @@ # Localization parity -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-19 · **Change:** minor (defined-name length limit counts characters, so long Arabic names are accepted; Table names are limited to 255 characters; right-to-left sheets in `.xls` files open right-to-left; text boxes and shapes keep right-to-left paragraph direction through XLSX save; Save as Web Page and copied HTML keep right-to-left; French and Serbian `context|label` translations no longer show the context) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-19 · **Change:** minor (defined-name length limit counts characters, so long Arabic names are accepted; Table names are limited to 255 characters; right-to-left sheets in `.xls` files open right-to-left; text boxes and shapes keep right-to-left paragraph direction through XLSX save; Save as Web Page and copied HTML keep right-to-left; French and Serbian `context|label` translations no longer show the context; CSV and text import decodes likely Windows-1256 Arabic; Arabic shaping and bidirectional UI remain unsupported) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 Per-language status against Excel, which ships its UI in dozens of languages (Windows language packs cover 100+), with localized function names, argument separators, number and date formats, @@ -83,6 +83,7 @@ language (none so far) and of localized function names. | Date | Change | Summary | |---|---|---| +| 2026-10-19 | minor | Arabic CSV and text imports decode Windows-1256 text; Arabic shaping and bidirectional UI remain unsupported | | 2026-10-19 | minor | Defined-name length counts characters, not bytes: Arabic names up to 255 letters are accepted, and Create from Selection keeps long Arabic headers | | 2026-10-19 | minor | Table names are limited to 255 Unicode characters | | 2026-10-19 | minor | Right-to-left sheets in legacy `.xls` files (common in older Arabic workbooks) open right-to-left | From 37f7b43bcaaab42a81b4e893a3300b6acf5a6f66 Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Mon, 19 Oct 2026 21:06:55 +0200 Subject: [PATCH 13/15] Fix pointer budget test: pass push_click a u32 click count (#525) --- crates/ui-egui/src/control.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index 53530ad8a..f80d1500a 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -549,7 +549,7 @@ mod tests { assert_eq!(app.synthetic.len(), 2); assert_eq!(app.synthetic.iter().map(Vec::len).sum::(), 5); // Already queued input counts toward the budget. - assert!(push_click(&mut app, p, btn, MAX_QUEUED_POINTER_EVENTS / 2, m).is_err()); + assert!(push_click(&mut app, p, btn, (MAX_QUEUED_POINTER_EVENTS / 2) as u32, m).is_err()); assert_eq!(app.synthetic.len(), 2); // Normal requests still queue in full. assert!(push_drag(&mut app, p, p + egui::vec2(40.0, 40.0), 3, m).is_ok()); From f29ac7d9ef04332fbd7a1fe626d88aad44db8b70 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Tue, 20 Oct 2026 18:16:31 +0000 Subject: [PATCH 14/15] Use scientific notation when number_to_text rounds up to 1e15 Found while testing crates/core/src/value.rs:354-367 Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/core/src/value.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/crates/core/src/value.rs b/crates/core/src/value.rs index cfef7ac57..9a5c09bdf 100644 --- a/crates/core/src/value.rs +++ b/crates/core/src/value.rs @@ -359,6 +359,10 @@ pub fn number_to_text(n: f64) -> String { if !(1e-9..1e15).contains(&a) && a.round() != a || a >= 1e15 { return sci_text(n, 15); } + // A value just below 1e15 can round up to 1e15 at 15 significant digits. + if format!("{:.14e}", a).parse::().is_ok_and(|r| r >= 1e15) { + return sci_text(n, 15); + } // 15 significant digits, trailing zeros trimmed. let mag = a.log10().floor() as i32; let decimals = (14 - mag).clamp(0, 30) as usize; @@ -402,6 +406,13 @@ mod tests { assert_eq!(number_to_text(1.0 / 3.0), "0.333333333333333"); } + #[test] + fn number_text_rounds_up_to_scientific_boundary() { + assert_eq!(number_to_text(999999999999999.9), "1E+15"); + assert_eq!(number_to_text(-999999999999999.9), "-1E+15"); + assert_eq!(number_to_text(999999999999999.0), "999999999999999"); + } + #[test] fn coercion() { assert_eq!(Value::from("12").to_number(), Ok(12.0)); From feac1679a1910d1af1cf756ad831c7722a0c48de Mon Sep 17 00:00:00 2001 From: Mirza Bilal Date: Wed, 21 Oct 2026 01:35:58 +0500 Subject: [PATCH 15/15] HTML export: place pictures and charts at their anchors Save as Web Page wrote only the cell table, so pictures and charts on a sheet were dropped. A sheet that has either now keeps its own geometry and draws them: - The table gets the sheet's column widths and row heights (`table-layout:fixed`, a ``, a height per row); hidden columns and rows take no room and merged-cell spans count only visible lines. - A picture is an `` with a `data:` URI, a chart is inline SVG built from the chart crate's drawing primitives (titles, axes, legends and data labels included), each absolutely positioned at its anchor cell plus offset. - Sheets without pictures or charts write byte-identical HTML. Adds contributors/people.toml entry for bilalmughal. --- contributors/people.toml | 3 + crates/engine/src/chart_svg.rs | 201 +++++++++++++++++++++++++++++++++ crates/engine/src/io.rs | 163 ++++++++++++++++++++++++-- crates/engine/src/lib.rs | 1 + crates/engine/src/tests.rs | 31 +++++ docs/file-format-parity.md | 5 +- 6 files changed, 393 insertions(+), 11 deletions(-) create mode 100644 crates/engine/src/chart_svg.rs diff --git a/contributors/people.toml b/contributors/people.toml index be84b5b01..d5ea71883 100644 --- a/contributors/people.toml +++ b/contributors/people.toml @@ -68,3 +68,6 @@ sync_github_name = false [people.Shnrqpdr] sync_github_name = true + +[people.bilalmughal] +sync_github_name = true diff --git a/crates/engine/src/chart_svg.rs b/crates/engine/src/chart_svg.rs new file mode 100644 index 000000000..be9e6d227 --- /dev/null +++ b/crates/engine/src/chart_svg.rs @@ -0,0 +1,201 @@ +//! Charts as inline SVG for Save as Web Page: the chart crate's drawing primitives, text included. + +use std::fmt::Write as _; + +use gridcraft_chart::{HAlign, Prim, Rgba, VAlign}; +use gridcraft_model::{Chart, Workbook}; + +use crate::io::esc; + +/// Widest or tallest chart written; larger anchors are clamped. +const MAX_SIDE: f32 = 16_384.0; + +struct HelveticaMeasure; + +impl gridcraft_chart::Measure for HelveticaMeasure { + fn text_width(&self, text: &str, size: f32, bold: bool) -> f32 { + gridcraft_pdf::text_width(if bold { gridcraft_pdf::Font::HelveticaBold } else { gridcraft_pdf::Font::Helvetica }, size, text) + } +} + +/// `chart` of sheet `sheet` as an `` element placed by `style`, or `None` for a degenerate size. +pub(crate) fn chart_svg(wb: &Workbook, sheet: usize, chart: &Chart, style: &str) -> Option { + let (w, h) = (chart.anchor.width.min(MAX_SIDE), chart.anchor.height.min(MAX_SIDE)); + if !(w >= 1.0 && h >= 1.0) { + return None; + } + let data = gridcraft_chart::resolve(wb, sheet, chart); + let prims = gridcraft_chart::render(chart, &data, w, h, &HelveticaMeasure); + Some(prims_svg(&prims, w, h, style)) +} + +fn prims_svg(prims: &[Prim], w: f32, h: f32, style: &str) -> String { + let mut out = String::new(); + let _ = write!( + out, + "" + ); + for p in prims { + match p { + Prim::Rect { x, y, w, h, fill, stroke, radius } => { + let _ = write!(out, " 0.0 { + let _ = write!(out, " rx=\"{}\"", num(*radius)); + } + paint(&mut out, *fill, *stroke); + out.push_str("/>"); + } + Prim::Line { pts, color, width, dash } => { + let _ = write!(out, ""); + } + Prim::Polygon { pts, fill, stroke } => { + let _ = write!(out, ""); + } + Prim::Wedge { cx, cy, r_outer, r_inner, a0, a1, fill, stroke } => { + let _ = write!(out, ""); + } + Prim::Circle { cx, cy, r, fill, stroke } => { + let _ = write!(out, ""); + } + Prim::Text { x, y, text, size, color, bold, align, valign, rotation } => { + // Baseline offsets as in the PDF export. + let dy = match valign { + VAlign::Top => size * 0.78, + VAlign::Middle => size * 0.35, + VAlign::Bottom => -size * 0.21, + }; + let anchor = match align { + HAlign::Left => "start", + HAlign::Center => "middle", + HAlign::Right => "end", + }; + let _ = write!(out, "{}", hex(*color), opacity("fill", *color), esc(text)); + } + } + } + out.push_str(""); + out +} + +/// Fill and stroke attributes of an open shape element; the caller closes it. +fn paint(out: &mut String, fill: Option, stroke: Option<(Rgba, f32)>) { + match fill { + Some(c) => { + let _ = write!(out, " fill=\"{}\"{}", hex(c), opacity("fill", c)); + } + None => out.push_str(" fill=\"none\""), + } + if let Some((c, w)) = stroke { + let _ = write!(out, " stroke=\"{}\" stroke-width=\"{}\"{}", hex(c), num(w), opacity("stroke", c)); + } +} + +/// A pie or doughnut sector. Angles run clockwise from 12 o'clock, like the chart crate's. +fn wedge_path(cx: f32, cy: f32, r_outer: f32, r_inner: f32, a0: f32, a1: f32) -> String { + let span = (a1 - a0).clamp(-std::f32::consts::TAU + 0.001, std::f32::consts::TAU - 0.001); + let a1 = a0 + span; + let at = |a: f32, r: f32| format!("{} {}", num(cx + r * a.sin()), num(cy - r * a.cos())); + let large = u8::from(span.abs() > std::f32::consts::PI); + let sweep = u8::from(span >= 0.0); + let mut d = format!("M{} A{r} {r} 0 {large} {sweep} {}", at(a0, r_outer), at(a1, r_outer), r = num(r_outer)); + if r_inner > 0.0 { + let _ = write!(d, " L{} A{r} {r} 0 {large} {} {} Z", at(a1, r_inner), 1 - sweep, at(a0, r_inner), r = num(r_inner)); + } else { + let _ = write!(d, " L{} {} Z", num(cx), num(cy)); + } + d +} + +fn points(pts: &[[f32; 2]]) -> String { + pts.iter().map(|p| format!("{},{}", num(p[0]), num(p[1]))).collect::>().join(" ") +} + +fn hex(c: Rgba) -> String { + format!("#{:02x}{:02x}{:02x}", c[0], c[1], c[2]) +} + +fn opacity(attr: &str, c: Rgba) -> String { + if c[3] == 255 { String::new() } else { format!(" {attr}-opacity=\"{:.2}\"", f32::from(c[3]) / 255.0) } +} + +/// A coordinate with two decimals; a non-finite value is 0 so the SVG stays well-formed. +fn num(v: f32) -> String { + if !v.is_finite() { + return "0".into(); + } + let s = format!("{v:.2}"); + s.trim_end_matches('0').trim_end_matches('.').to_string() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn every_primitive_becomes_an_svg_element() { + let red = [255, 0, 0, 255]; + let prims = vec![ + Prim::Rect { x: 1.0, y: 2.0, w: 3.0, h: 4.0, fill: Some(red), stroke: None, radius: 0.0 }, + Prim::Line { pts: vec![[0.0, 0.0], [5.0, 5.5]], color: red, width: 1.0, dash: true }, + Prim::Polygon { pts: vec![[0.0, 0.0], [1.0, 0.0], [0.0, 1.0]], fill: red, stroke: Some(([0, 0, 0, 255], 1.0)) }, + Prim::Wedge { cx: 10.0, cy: 10.0, r_outer: 5.0, r_inner: 2.0, a0: 0.0, a1: 1.0, fill: red, stroke: None }, + Prim::Circle { cx: 1.0, cy: 1.0, r: 2.0, fill: Some([0, 0, 255, 128]), stroke: None }, + Prim::Text { + x: 5.0, + y: 6.0, + text: "A", + "", + "A<B", + ] { + assert!(svg.contains(part), "{part} in {svg}"); + } + assert!(svg.starts_with("") && svg.contains("style=\"left:1px\""), "{svg}"); + } + + #[test] + fn non_finite_numbers_do_not_break_the_markup() { + let prims = [Prim::Circle { cx: f32::NAN, cy: f32::INFINITY, r: 1.0, fill: None, stroke: None }]; + assert!(prims_svg(&prims, 10.0, 10.0, "").contains("")); + } + + #[test] + fn a_full_circle_wedge_keeps_a_drawable_arc() { + let d = wedge_path(0.0, 0.0, 10.0, 0.0, 0.0, std::f32::consts::TAU); + assert!(d.contains("A10 10 0 1 1 ") && d.ends_with("L0 0 Z"), "{d}"); + } +} diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index 14717a3df..287722547 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -254,7 +254,7 @@ fn csv(wb: &Workbook, sh: &Sheet, delimiter: u8) -> Result> { .map_err(|e| EngineError::Other(e.to_string())) } -fn esc(s: &str) -> String { +pub(crate) fn esc(s: &str) -> String { s.replace('&', "&").replace('<', "<").replace('>', ">").replace('"', """) } @@ -408,31 +408,56 @@ impl std::fmt::Write for ClipboardHtml { /// Most rows Save as Web Page writes. A sheet whose used range is taller fails instead of losing cells. const HTML_EXPORT_MAX_ROWS: u32 = 100_001; -/// A sheet as a standalone HTML table (Save as Web Page). +/// A sheet as a standalone HTML table (Save as Web Page). A sheet with pictures gets the sheet's +/// own column widths and row heights so each picture and chart (inline SVG) can sit at its anchor. pub fn to_html(wb: &Workbook, sheet: usize) -> Result { let Some(sh) = wb.sheet(sheet) else { return Ok(String::new()) }; + let fixed = !(sh.images.is_empty() && sh.charts.is_empty()); + let used = sh.used_range(); let mut out = String::new(); let _ = write!( out, - "{}", + "{}", esc(&sh.name), - table_dir(sh) + if fixed { ";table-layout:fixed" } else { "" }, + if fixed { "0 4px" } else { "2px 4px" }, + if fixed { ";overflow:hidden;line-height:1" } else { "" } ); - if let Some(r) = sh.used_range() { + if fixed { + out.push_str("
"); + } + let _ = write!(out, "'); + if let Some(r) = used { if r.height() > HTML_EXPORT_MAX_ROWS { return Err(EngineError::Other(format!( "the sheet is too tall for a web page ({} rows, at most {HTML_EXPORT_MAX_ROWS}). Save as .xlsx or CSV instead.", r.height() ))); } + if fixed { + out.push_str(""); + for col in (r.start.col..=r.end.col).filter(|&c| !sh.is_col_hidden(c)) { + let _ = write!(out, "", sh.col_width(col)); + } + out.push_str(""); + } for row in r.start.row..=r.end.row { if sh.is_row_hidden(row) { continue; } - out.push_str(""); + if fixed { + let _ = write!(out, "", sh.row_height(row)); + } else { + out.push_str(""); + } for col in r.start.col..=r.end.col { let c = CellRef::new(row, col); - if sh.merges.iter().any(|m| m.contains(c) && m.start != c) { + if sh.merges.iter().any(|m| m.contains(c) && m.start != c) || (fixed && sh.is_col_hidden(col)) { continue; } let text = crate::display::cell_text(wb, sh, c); @@ -442,14 +467,52 @@ pub fn to_html(wb: &Workbook, sheet: usize) -> Result { .merges .iter() .find(|m| m.start == c) - .map(|m| format!(" rowspan=\"{}\" colspan=\"{}\"", m.height(), m.width())) + .map(|m| { + if fixed { + let rows = (m.start.row..=m.end.row).filter(|&y| !sh.is_row_hidden(y)).count(); + let cols = (m.start.col..=m.end.col).filter(|&x| !sh.is_col_hidden(x)).count(); + format!(" rowspan=\"{rows}\" colspan=\"{cols}\"") + } else { + format!(" rowspan=\"{}\" colspan=\"{}\"", m.height(), m.width()) + } + }) .unwrap_or_default(); let _ = write!(out, "{}", dir.attr(), esc(&text)); } out.push_str(""); } } - out.push_str(""); + out.push_str(""); + if fixed { + // Sheet units are CSS pixels; the table starts at the used range, not at A1. + let (left, top) = used.map_or((0.0, 0.0), |r| (sh.col_left(r.start.col), sh.row_top(r.start.row))); + let place = |a: &gridcraft_model::Anchor| { + format!( + "position:absolute;left:{}px;top:{}px;width:{}px;height:{}px", + sh.col_left(a.cell.col) - left + f64::from(a.dx), + sh.row_top(a.cell.row) - top + f64::from(a.dy), + a.width.max(0.0), + a.height.max(0.0) + ) + }; + for im in &sh.images { + let _ = write!( + out, + "\"{}\"", + esc(&im.mime), + base64_encode(&im.data), + esc(&im.alt), + place(&im.anchor) + ); + } + for ch in &sh.charts { + if let Some(svg) = crate::chart_svg::chart_svg(wb, sheet, ch, &place(&ch.anchor)) { + out.push_str(&svg); + } + } + out.push_str("
"); + } + out.push_str(""); Ok(out) } @@ -572,6 +635,88 @@ mod tests { range_html(wb, 0, r, &crate::display::range_text(wb, 0, r)).unwrap() } + #[test] + fn html_export_embeds_images_as_data_uris() { + let mut wb = Workbook::new(); + wb.sheet_mut(0).unwrap().images.push(gridcraft_model::Image { + id: 1, + anchor: gridcraft_model::Anchor { cell: CellRef::new(0, 0), dx: 0.0, dy: 0.0, width: 72.0, height: 36.0, mode: Default::default() }, + data: vec![1, 2, 3], + mime: "image/png".into(), + alt: "a \"logo\"".into(), + }); + let html = to_html(&wb, 0).unwrap(); + assert!( + html.contains("\"a"), + "{html}" + ); + assert!(html.contains("
"); + } + + #[test] + fn html_export_places_images_at_their_anchor_using_sheet_geometry() { + let mut wb = Workbook::new(); + let sh = wb.sheet_mut(0).unwrap(); + sh.cols.insert(1, gridcraft_model::LineInfo { size: Some(100.0), ..Default::default() }); + sh.rows.insert(1, gridcraft_model::LineInfo { size: Some(30.0), ..Default::default() }); + sh.images.push(gridcraft_model::Image { + id: 1, + anchor: gridcraft_model::Anchor { cell: CellRef::new(3, 2), dx: 5.0, dy: 7.0, width: 10.0, height: 20.0, mode: Default::default() }, + data: vec![1], + mime: "image/png".into(), + alt: String::new(), + }); + sh.set_value(CellRef::new(1, 1), gridcraft_core::Value::Number(1.0)); + let html = to_html(&wb, 0).unwrap(); + // The table starts at B2; the picture is at C4: one 100 px column and two rows (30 + 20) further on. + assert!(html.contains(""), "{html}"); + assert!(html.contains(""), "{html}"); + assert!(html.contains("left:105px;top:57px;width:10px;height:20px"), "{html}"); + } + + fn book_with_picture(anchor: &str, cell_at: (u32, u32)) -> Workbook { + let mut wb = Workbook::new(); + let sh = wb.sheet_mut(0).unwrap(); + sh.set_value(CellRef::new(cell_at.0, cell_at.1), gridcraft_core::Value::Number(1.0)); + sh.images.push(gridcraft_model::Image { + id: 1, + anchor: gridcraft_model::Anchor { + cell: CellRef::parse(anchor).unwrap(), + dx: 0.0, + dy: 0.0, + width: 10.0, + height: 10.0, + mode: Default::default(), + }, + data: vec![1], + mime: "image/png".into(), + alt: String::new(), + }); + wb + } + + #[test] + fn html_export_places_images_past_hidden_columns_and_rows() { + let mut wb = book_with_picture("C3", (0, 0)); + let sh = wb.sheet_mut(0).unwrap(); + sh.set_value(CellRef::new(2, 2), gridcraft_core::Value::Number(2.0)); + sh.cols.insert(1, gridcraft_model::LineInfo { hidden: true, ..Default::default() }); + sh.rows.insert(1, gridcraft_model::LineInfo { hidden: true, ..Default::default() }); + let html = to_html(&wb, 0).unwrap(); + // Column B and row 2 take no room: C3 is one default column and row from A1. + assert_eq!(html.matches("", "at": "D2", "width": 300, "height": 200})) + .unwrap(); + let html = crate::io::to_html(&s.doc().unwrap().wb, 0).unwrap(); + assert!(html.contains("Quarterly <Sales>"), "the title is drawn as escaped text: {html}"); + assert!(html.contains("position:absolute;left:"), "{html}"); + assert!(html.contains("= 2, "one wedge per slice: {html}"); + assert!(html.contains(">North") && html.contains(">South"), "the legend names the slices: {html}"); +} + +#[test] +fn html_export_without_pictures_or_charts_has_no_fixed_geometry() { + let mut s = s(); + s.execute("range.setValues", json!({"range": "A1", "values": [["a", "b"]]})).unwrap(); + let html = crate::io::to_html(&s.doc().unwrap().wb, 0).unwrap(); + assert!(!html.contains("") && !html.contains("table-layout") && !html.contains(" **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-19 · **Change:** minor (.xls dialog sheets are skipped instead of read as worksheets; `.xls` import reads each sheet's display options from WINDOW2: right-to-left, gridlines, headings, zeros, formulas; XLSX text boxes and shapes keep their paragraphs' right-to-left direction on save; CSV/TSV/TXT can open with every field kept as text via `file.open {csvAsText}`; formulas over Excel's 8,192 characters, and `%` or call chains past the nesting limit, are parse errors instead of stack overflows when a file is opened; XLSX PivotTables read and write `rowGrandTotals`/`colGrandTotals` the way Excel does, so a table saved with only one grand total opens with the right one; HTML export keeps right-to-left) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-21 · **Change:** minor (.xls dialog sheets are skipped instead of read as worksheets; `.xls` import reads each sheet's display options from WINDOW2: right-to-left, gridlines, headings, zeros, formulas; XLSX text boxes and shapes keep their paragraphs' right-to-left direction on save; CSV/TSV/TXT can open with every field kept as text via `file.open {csvAsText}`; formulas over Excel's 8,192 characters, and `%` or call chains past the nesting limit, are parse errors instead of stack overflows when a file is opened; XLSX PivotTables read and write `rowGrandTotals`/`colGrandTotals` the way Excel does, so a table saved with only one grand total opens with the right one; HTML export keeps right-to-left; HTML export places pictures and charts, as inline SVG, at their anchors) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 Every format Excel opens or saves, with GridCraft's read and write support, fidelity and tests. Part of [target-app-parity.md](target-app-parity.md). **File formats: ~50% ready, 80–120 h** @@ -47,7 +47,7 @@ and repair-free opening remain unverified. | `.txt` tab-delimited, Unicode text | R/W | yes (`.txt`, `.tsv`, `.tab`) | yes (displayed values) | Text Import Wizard options partly via Text to Columns | | `.prn` formatted text (space-delimited) | R/W | no | no | | | `.dif`, `.slk` (SYLK) | R/W | no | no | Small, legacy | -| `.htm` / `.html` web page | R/W (Windows) | yes, via Get Data ▸ From HTML | yes (one sheet as a table; a used range taller than 100,001 rows fails with an error instead of being cut short) | Formatting not exported | +| `.htm` / `.html` web page | R/W (Windows) | yes, via Get Data ▸ From HTML | yes (one sheet as a table; a used range taller than 100,001 rows fails with an error instead of being cut short) | Formatting not exported. A sheet with pictures or charts is written with the sheet's own column widths and row heights, a picture as a `data:` URI `` and a chart as inline SVG, each placed at its anchor; shapes, text boxes and comments are not exported | | `.mht` / `.mhtml` single-file web page | R/W (Windows) | no | no | | | `.json` | via Power Query | GridCraft's own workbook JSON (debug) and JSON import | GridCraft JSON | Not Power Query's JSON shaping; notes and hyperlinks break export (#126) | | `.xml` with XML maps | Developer ▸ Import/Export | no | no | | @@ -85,6 +85,7 @@ refresh. Counted under features (Get & Transform, ~10% ready), not here. | Date | Change | Summary | |---|---|---| +| 2026-10-21 | minor | HTML (Save as Web Page) writes pictures (`data:` URI ``) and charts (inline SVG from the chart crate's drawing primitives, text included) at their anchors; the table uses the sheet's column widths and row heights when the sheet has either, and output for sheets without pictures or charts is unchanged | | 2026-10-19 | minor | .xls import skips dialog sheets (WsBool `fDialog`), which share the worksheet BOF type and were read as empty worksheets | | 2026-10-19 | minor | `.xls` import reads WINDOW2: right-to-left sheets (common in older Arabic and Hebrew files) open right-to-left, and hidden gridlines, headings and zero values and Show Formulas carry over | | 2026-10-19 | trivial | XLSX text boxes and shapes keep right-to-left paragraph direction (`a:pPr rtl`, ECMA-376 Part 1 §21.1.2.2.7): read from the first paragraph and written on every paragraph; output without it is unchanged |