From 01032e9010cc68fa684cacc9140a354a0683fa4f Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Tue, 13 Oct 2026 18:02:01 +0200 Subject: [PATCH 01/16] Cap CSV import fields per record at the worksheet column limit (#1037) --- crates/xlsx/src/csv.rs | 19 ++++++++++++++----- crates/xlsx/src/tests/csv_tests.rs | 25 +++++++++++++++++++++++-- 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/crates/xlsx/src/csv.rs b/crates/xlsx/src/csv.rs index 47486bfa2..946fafe18 100644 --- a/crates/xlsx/src/csv.rs +++ b/crates/xlsx/src/csv.rs @@ -87,8 +87,9 @@ pub fn sniff_delimiter(text: &str, quote: char) -> u8 { best.0 } -/// Splits CSV text into records of fields. -fn parse_records(text: &str, delim: char, quote: char, mut on_record: impl FnMut(Vec) -> bool) { +/// Splits CSV text into records of fields. A record keeps at most [`MAX_COLS`] fields; the rest are +/// dropped as they are read, so a row of endless delimiters cannot allocate a string per field. +pub(crate) fn parse_records(text: &str, delim: char, quote: char, mut on_record: impl FnMut(Vec) -> bool) { let mut field = String::new(); let mut record: Vec = vec![]; let mut in_q = false; @@ -112,13 +113,19 @@ fn parse_records(text: &str, delim: char, quote: char, mut on_record: impl FnMut in_q = true; field_started_quoted = true; } else if c == delim { - record.push(std::mem::take(&mut field)); + let f = std::mem::take(&mut field); + if record.len() < MAX_COLS as usize { + record.push(f); + } field_started_quoted = false; } else if c == '\r' || c == '\n' { if c == '\r' && chars.peek() == Some(&'\n') { chars.next(); } - record.push(std::mem::take(&mut field)); + let f = std::mem::take(&mut field); + if record.len() < MAX_COLS as usize { + record.push(f); + } field_started_quoted = false; if !on_record(std::mem::take(&mut record)) { return; @@ -128,7 +135,9 @@ fn parse_records(text: &str, delim: char, quote: char, mut on_record: impl FnMut } } if !field.is_empty() || !record.is_empty() || field_started_quoted { - record.push(field); + if record.len() < MAX_COLS as usize { + record.push(field); + } on_record(record); } } diff --git a/crates/xlsx/src/tests/csv_tests.rs b/crates/xlsx/src/tests/csv_tests.rs index cc52072e7..c0a774b0d 100644 --- a/crates/xlsx/src/tests/csv_tests.rs +++ b/crates/xlsx/src/tests/csv_tests.rs @@ -1,7 +1,7 @@ -use gridcraft_core::{CellRef, DateSystem, Value}; +use gridcraft_core::{CellRef, DateSystem, MAX_COLS, Value}; use gridcraft_model::{Sheet, Workbook}; -use crate::csv::{decode_text, sniff_delimiter}; +use crate::csv::{decode_text, parse_records, sniff_delimiter}; use crate::{CsvOptions, Format, IoError, read_csv, sniff, write_csv, write_csv_limited}; fn v(wb: &Workbook, r: u32, c: u32) -> Value { @@ -190,3 +190,24 @@ fn the_cap_counts_the_bytes_the_cells_hold_and_accepted_exports_keep_every_posit let tabs = write_csv_limited(&s, b'\t', 14, text_of(&s)).unwrap(); assert_eq!(tabs.len(), 14); } + +#[test] +fn parser_stops_collecting_fields_at_max_cols() { + let extra = 50; + // MAX_COLS + extra fields on the first row (the last one filled), then a short second row. + let mut text = ",".repeat((MAX_COLS + extra) as usize - 1); + text.push_str("end\nx,y\n"); + let mut lens = vec![]; + parse_records(&text, ',', '"', |rec| { + lens.push(rec.len()); + true + }); + assert_eq!(lens, vec![MAX_COLS as usize, 2]); + // An unterminated last row is capped the same way. + let mut lens = vec![]; + parse_records(&",".repeat(MAX_COLS as usize + extra as usize), ',', '"', |rec| { + lens.push(rec.len()); + true + }); + assert_eq!(lens, vec![MAX_COLS as usize]); +} From 8939e7817f8d81f42472a50551984bcbdfdc05ef Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Tue, 13 Oct 2026 18:04:09 +0200 Subject: [PATCH 02/16] Fix xlsx: combine escaped UTF-16 surrogate pairs when decoding strings (#1025) --- crates/xlsx/src/xml.rs | 37 +++++++++++++++++++++++++++++++++++-- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/crates/xlsx/src/xml.rs b/crates/xlsx/src/xml.rs index 43bb5b53e..f27927638 100644 --- a/crates/xlsx/src/xml.rs +++ b/crates/xlsx/src/xml.rs @@ -304,6 +304,20 @@ fn looks_like_escape(chars: &[char], i: usize) -> bool { chars.get(i + 1) == Some(&'x') && chars.get(i + 6) == Some(&'_') && (i + 2..i + 6).all(|k| chars.get(k).is_some_and(|c| c.is_ascii_hexdigit())) } +fn escape_unit(chars: &[char], i: usize) -> Option { + let hex: String = chars.get(i + 2..i + 6)?.iter().collect(); + u32::from_str_radix(&hex, 16).ok() +} + +/// The character for a high surrogate escape at `i` (value `high`) followed directly by a low surrogate escape. +fn surrogate_pair(chars: &[char], i: usize, high: u32) -> Option { + if !(0xD800..=0xDBFF).contains(&high) || chars.get(i + 7) != Some(&'_') || !looks_like_escape(chars, i + 7) { + return None; + } + let low = escape_unit(chars, i + 7).filter(|l| (0xDC00..=0xDFFF).contains(l))?; + char::from_u32(0x10000 + ((high - 0xD800) << 10) + (low - 0xDC00)) +} + /// Inverse of [`encode_xstring`]. pub fn decode_xstring(s: &str) -> String { if !s.contains("_x") { @@ -314,12 +328,18 @@ pub fn decode_xstring(s: &str) -> String { let mut i = 0; while i < chars.len() { if chars[i] == '_' && looks_like_escape(&chars, i) { - let hex: String = chars[i + 2..i + 6].iter().collect(); - if let Some(ch) = u32::from_str_radix(&hex, 16).ok().and_then(char::from_u32) { + let unit = escape_unit(&chars, i); + if let Some(ch) = unit.and_then(char::from_u32) { o.push(ch); i += 7; continue; } + // A high surrogate escape followed by a low surrogate escape is one UTF-16 pair. + if let Some(ch) = unit.and_then(|high| surrogate_pair(&chars, i, high)) { + o.push(ch); + i += 14; + continue; + } } o.push(chars[i]); i += 1; @@ -402,6 +422,19 @@ mod tests { assert_eq!(decode_xstring("_x00"), "_x00"); } + #[test] + fn xstring_surrogate_pairs() { + assert_eq!(decode_xstring("a_xD83D__xDE00_b"), "a\u{1F600}b"); + assert_eq!(decode_xstring("_xd83d__xde00_"), "\u{1F600}"); + // Lone surrogates stay as literal escape text. + assert_eq!(decode_xstring("_xD83D_"), "_xD83D_"); + assert_eq!(decode_xstring("_xDE00_"), "_xDE00_"); + assert_eq!(decode_xstring("_xD83D_x"), "_xD83D_x"); + assert_eq!(decode_xstring("_xD83D__x0041_"), "_xD83D_A"); + // A protected literal escape is not a low surrogate. + assert_eq!(decode_xstring("_xD83D__x005F_xDE00_"), "_xD83D__xDE00_"); + } + #[test] fn numbers() { assert_eq!(num(1.0), "1"); From 66be0a864637ffec667787e877db6634d5a579a4 Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Tue, 13 Oct 2026 18:02:36 +0200 Subject: [PATCH 03/16] Open notice: do not promise to keep macros that were dropped (#1024) --- crates/ui-egui/src/lib.rs | 9 ++++++++- crates/ui-egui/tests/interaction.rs | 16 ++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index f96cdac3c..06396f42b 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -800,8 +800,15 @@ impl SheetApp { } let vba: Vec<&str> = msgs.iter().copied().filter(|m| m.to_lowercase().contains("macro") || m.contains("VBA")).collect(); if !vba.is_empty() { + // The reader says "dropped" when the VBA project was missing or unreadable; never promise to keep it then. + let dropped = vba.iter().any(|m| m.contains("dropped")); + let text = if dropped { + "This workbook's VBA macros could not be loaded and were dropped, so they will not be in the saved file. Keep the original file." + } else { + "This workbook contains VBA macros. GridCraft keeps them when you save as .xlsm, but does not run them." + }; self.notice = Some(Notice { - text: "This workbook contains VBA macros. GridCraft keeps them intact when you save, but does not run them.".into(), + text: text.into(), kind: NoticeKind::Warning, action: Some(("What this means".into(), "macros".into())), }); diff --git a/crates/ui-egui/tests/interaction.rs b/crates/ui-egui/tests/interaction.rs index b40769cf8..13f4aa3c5 100644 --- a/crates/ui-egui/tests/interaction.rs +++ b/crates/ui-egui/tests/interaction.rs @@ -781,6 +781,22 @@ fn opening_a_macro_workbook_shows_a_notice() { assert!(h.state().notice.is_none()); } +#[test] +fn dropped_macro_warning_never_promises_to_keep_macros() { + let mut h = harness(blank()); + for w in [ + "macros (VBA project) reference a missing part and were dropped", + "macros (VBA project) could not be read and were dropped: bad zip", + ] { + h.state_mut().show_open_warnings(&json!({"warnings": [w]})); + let n = h.state().notice.clone().unwrap(); + assert_eq!(n.kind, gridcraft_ui_egui::NoticeKind::Warning); + assert!(n.text.contains("dropped") && !n.text.contains("keeps them"), "{}", n.text); + } + h.state_mut().show_open_warnings(&json!({"warnings": ["macros (VBA project) were kept but not executed"]})); + assert!(h.state().notice.clone().unwrap().text.contains("keeps them when you save as .xlsm")); +} + #[test] fn plain_open_warning_becomes_an_info_notice() { let mut h = harness(blank()); From a091fd9b509e317308a12c84bce0a50acd3af61f Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Tue, 13 Oct 2026 18:04:20 +0200 Subject: [PATCH 04/16] CSV export: check the size cap before each field, not after the row (#1023) --- crates/xlsx/src/csv.rs | 22 +++++++++++++++++----- crates/xlsx/src/tests/csv_tests.rs | 19 +++++++++++++++++++ 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/crates/xlsx/src/csv.rs b/crates/xlsx/src/csv.rs index 47486bfa2..485de7a37 100644 --- a/crates/xlsx/src/csv.rs +++ b/crates/xlsx/src/csv.rs @@ -193,9 +193,9 @@ pub fn read_csv(bytes: &[u8], opts: &CsvOptions) -> Result { /// /// The output is capped at [`MAX_CSV_BYTES`]: two cells far apart make the used range, and so the /// file, as big as the whole rectangle between them (`A1` and `XFD1048576` are 16 GiB of commas). -/// An export that would pass the cap is refused before anything is allocated, or as soon as the -/// text written so far passes it, with [`IoError::TooLarge`]; accepted exports keep every field in -/// its place. +/// An export that would pass the cap is refused before anything is allocated, or at the first +/// field or line end that takes it past the cap (a field is measured before it is written), with +/// [`IoError::TooLarge`]; accepted exports keep every field in its place. pub fn write_csv(sheet: &Sheet, delimiter: u8, cell_text: impl FnMut(CellRef) -> String) -> Result, IoError> { write_csv_limited(sheet, delimiter, MAX_CSV_BYTES, cell_text) } @@ -221,9 +221,21 @@ pub fn write_csv_limited(sheet: &Sheet, delimiter: u8, limit: usize, mut cell_te out.push(delim); } let t = cell_text(CellRef::new(row, col)); - if t.contains(delim) || t.contains('"') || t.contains('\n') || t.contains('\r') { + let quoted = t.contains(delim) || t.contains('"') || t.contains('\n') || t.contains('\r'); + // The exact size of the field as written (the delimiter was pushed above), so a row + // never grows past the cap unseen and no doubled-quote copy of the text is built. + let field_len = if quoted { t.len() + t.matches('"').count() + 2 } else { t.len() }; + if out.len().saturating_add(field_len) > limit { + return Err(too_large(format!("more than {} MiB (stopped at row {})", limit >> 20, row + 1))); + } + if quoted { out.push('"'); - out.push_str(&t.replace('"', "\"\"")); + for ch in t.chars() { + if ch == '"' { + out.push('"'); + } + out.push(ch); + } out.push('"'); } else { out.push_str(&t); diff --git a/crates/xlsx/src/tests/csv_tests.rs b/crates/xlsx/src/tests/csv_tests.rs index cc52072e7..3716a162d 100644 --- a/crates/xlsx/src/tests/csv_tests.rs +++ b/crates/xlsx/src/tests/csv_tests.rs @@ -186,6 +186,25 @@ fn the_cap_counts_the_bytes_the_cells_hold_and_accepted_exports_keep_every_posit let err = write_csv_limited(&long, b',', 1000, text_of(&long)).unwrap_err(); assert!(err.to_string().contains("stopped at row"), "{err}"); assert!(write_csv_limited(&long, b',', 50 * 102, text_of(&long)).is_ok()); + // Within one row the cap is checked before each field is written: a wide row of long and + // quote-heavy fields is refused at the field that crosses it, without asking for the rest. + let mut wide = Sheet::new("W"); + for c in 0..10u32 { + wide.set_value(CellRef::new(0, c), Value::text("\"".repeat(100))); + } + let mut asked = 0; + let err = write_csv_limited(&wide, b',', 500, |c| { + asked += 1; + text_of(&wide)(c) + }) + .unwrap_err(); + assert!(err.to_string().contains("stopped at row 1"), "{err}"); + assert_eq!(asked, 3, "each field is 202 bytes; the third passes 500"); + // Exact boundary: a quoted field of 202 bytes plus CRLF fits in 204 and not in 203. + let mut one = Sheet::new("O"); + one.set_value(CellRef::new(0, 0), Value::text("\"".repeat(100))); + assert_eq!(write_csv_limited(&one, b',', 204, text_of(&one)).unwrap().len(), 204); + assert!(write_csv_limited(&one, b',', 203, text_of(&one)).is_err()); // A tab is one byte, like a comma. let tabs = write_csv_limited(&s, b'\t', 14, text_of(&s)).unwrap(); assert_eq!(tabs.len(), 14); From 48696f9bcf860b15170b4e43d2bcd4d95bca6c46 Mon Sep 17 00:00:00 2001 From: vangel76 Date: Tue, 13 Oct 2026 21:44:20 +0200 Subject: [PATCH 05/16] XLSX: [Content_Types].xml is the first zip entry, workbook parts precede document properties (#1090) Co-Authored-By: Claude Sonnet 5.5 --- ROADMAP.md | 1 + crates/xlsx/src/tests/mod.rs | 1 + crates/xlsx/src/tests/zip_order.rs | 33 ++++++++++++++++++++++++++++++ crates/xlsx/src/write.rs | 10 +++++---- 4 files changed, 41 insertions(+), 4 deletions(-) create mode 100644 crates/xlsx/src/tests/zip_order.rs diff --git a/ROADMAP.md b/ROADMAP.md index 070552d07..549962d1a 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -132,6 +132,7 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in | Date | What landed | |---|---| +| 2026-10-13 | XLSX: `[Content_Types].xml`, `_rels/.rels`, the workbook rels and `xl/workbook.xml` are the first zip entries (document properties last), so libmagic and PHP `finfo` detect the file type even with large properties (#1090) | | 2026-10-12 | Rows auto-fit oversized fonts: opening a file and whole-row/column formatting grow non-custom rows to their tallest font (#833) | | 2026-10-12 | Number formats: parsing a format code is linear in its length (long runs of `[`, `,` or `/` no longer take quadratic time), and an unclosed `[` is a literal when splitting sections as well as when tokenizing (#810) | | 2026-10-12 | Number formats: a fraction denominator with 65,536 zeros no longer crashes; format codes are capped at 255 characters like Excel (longer codes display as General, `TEXT` gives `#VALUE!`) (#807) | diff --git a/crates/xlsx/src/tests/mod.rs b/crates/xlsx/src/tests/mod.rs index 517113d0f..409e27935 100644 --- a/crates/xlsx/src/tests/mod.rs +++ b/crates/xlsx/src/tests/mod.rs @@ -18,6 +18,7 @@ mod smooth_roundtrip; mod trendline_roundtrip; mod xls_tests; mod xlsb_tests; +mod zip_order; use std::io::Write; diff --git a/crates/xlsx/src/tests/zip_order.rs b/crates/xlsx/src/tests/zip_order.rs new file mode 100644 index 000000000..939b268d7 --- /dev/null +++ b/crates/xlsx/src/tests/zip_order.rs @@ -0,0 +1,33 @@ +//! libmagic (PHP `finfo`, `file`) detects an OOXML package from the first zip entries (#1090). + +use gridcraft_model::Workbook; + +use crate::write_xlsx; + +fn find(hay: &[u8], needle: &[u8]) -> Option { + hay.windows(needle.len()).position(|w| w == needle) +} + +#[test] +fn package_starts_with_content_types_rels_and_workbook_before_any_large_part() { + let mut wb = Workbook::new(); + // A long, hard-to-compress description makes docProps/core.xml big. + wb.props.description = + (0..20_000).map(|i| char::from(b'a' + (i * 7 % 26) as u8)).chain((0..20_000).map(|i| char::from(b'0' + (i * i % 10) as u8))).collect(); + let bytes = write_xlsx(&wb).unwrap(); + + let mut zip = zip::ZipArchive::new(std::io::Cursor::new(&bytes)).unwrap(); + let names: Vec = (0..zip.len()).filter_map(|i| zip.by_index(i).ok().map(|f| f.name().to_string())).collect(); + assert_eq!(names.first().map(String::as_str), Some("[Content_Types].xml"), "{names:?}"); + assert_eq!(names.get(1).map(String::as_str), Some("_rels/.rels"), "{names:?}"); + assert_eq!(names.get(2).map(String::as_str), Some("xl/_rels/workbook.xml.rels"), "{names:?}"); + assert_eq!(names.get(3).map(String::as_str), Some("xl/workbook.xml"), "{names:?}"); + + // No data descriptor (general-purpose bit 3) on the first local header, entry name at offset 30. + assert_eq!(bytes.get(..4), Some(&b"PK\x03\x04"[..])); + assert_eq!(bytes.get(6).map(|b| b & 0x08), Some(0)); + assert_eq!(bytes.get(30..49), Some(&b"[Content_Types].xml"[..])); + // libmagic looks for an `xl/` entry name within the first few KB: keep it well inside that. + let at = find(&bytes, b"xl/_rels/workbook.xml.rels"); + assert!(at.is_some_and(|p| p < 2048), "xl/ entry at {at:?}"); +} diff --git a/crates/xlsx/src/write.rs b/crates/xlsx/src/write.rs index 06761f01d..c580f0a32 100644 --- a/crates/xlsx/src/write.rs +++ b/crates/xlsx/src/write.rs @@ -452,13 +452,15 @@ pub fn write_xlsx(wb: &Workbook, kind: XlsxKind) -> Result, IoError> { ct.push_str(""); let mut zip = ZipOut::new(); zip.add("[Content_Types].xml", ct.as_bytes())?; - // Conventional order: rels, docProps, workbook, the rest. + // Excel's order: rels, workbook rels, workbook, the rest, document properties last. libmagic (PHP + // `finfo`, `file`) recognises an OOXML package only when an `xl/` entry name appears within the + // first few KB, so no user-sized part (document properties, a long description) may precede it. let rank = |n: &str| match n { "_rels/.rels" => 0, - n if n.starts_with("docProps/") => 1, + "xl/_rels/workbook.xml.rels" => 1, "xl/workbook.xml" => 2, - "xl/_rels/workbook.xml.rels" => 3, - _ => 4, + n if n.starts_with("docProps/") => 4, + _ => 3, }; let mut parts = std::mem::take(&mut out.parts); parts.sort_by_key(|(n, _)| rank(n)); From 736cc89b2b788712b9b22e8b2194a91312fd9758 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Wed, 14 Oct 2026 07:44:41 +0200 Subject: [PATCH 06/16] Opening an XLSX with a part over 512 MB says how to get the data in (fixes #1242) --- crates/engine/src/io.rs | 74 +++++++++++++++++++++++++++++++++++++- crates/xlsx/src/lib.rs | 7 ++++ crates/xlsx/src/package.rs | 9 +++++ 3 files changed, 89 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index 3e84036cb..e03bba1a0 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -140,7 +140,12 @@ fn open_plain(name: &str, bytes: &[u8]) -> Result<(Workbook, Vec)> { } // Text named `.xlsx` is read as text. if sniffed == gridcraft_xlsx::Format::Xlsx || (kind == Some(FileKind::Xlsx) && sniffed != gridcraft_xlsx::Format::Csv) { - let (wb, report) = gridcraft_xlsx::read_xlsx(bytes).map_err(|e| EngineError::Other(format!("We can't open '{name}': {e}")))?; + let (wb, report) = gridcraft_xlsx::read_xlsx(bytes).map_err(|e| { + EngineError::Other(match (&e, gridcraft_xlsx::oversized_part(bytes)) { + (gridcraft_xlsx::IoError::TooLarge(_), Some((part, size))) => part_too_large_message(name, &part, size), + _ => format!("We can't open '{name}': {e}"), + }) + })?; return Ok((wb, report.warnings)); } match kind { @@ -169,6 +174,25 @@ fn open_plain(name: &str, bytes: &[u8]) -> Result<(Workbook, Vec)> { } } +/// What to tell the user when one part of an XLSX package unpacks to more than the reader's +/// per-part limit: the sizes, why the limit is there, and what gets the data in. The limit is +/// per part, so smaller sheets fit; the shared strings are one part for every sheet, so only a +/// CSV, which has no parts, helps there. +fn part_too_large_message(name: &str, part: &str, size: u64) -> String { + let limit = gridcraft_xlsx::MAX_PART >> 20; + let mb = size >> 20; + let size = if mb > limit { format!("{mb} MB") } else { format!("more than {limit} MB") }; + let remedy = if part.contains("worksheets/") { + "To open it, split the data across several sheets, since the limit applies to each part, or save the sheet as CSV, which keeps only its values." + } else { + "To open it, save the data as CSV, which keeps only its values." + }; + format!( + "We can't open '{name}': its part {part} unpacks to {size}, over GridCraft's limit of {limit} MB for one part \ + (a guard against damaged or malicious files). {remedy}" + ) +} + /// A CSV opens as a sheet named after the file (like Excel). fn rename_first_sheet(wb: &mut Workbook, path: &str) { let stem = std::path::Path::new(path).file_stem().and_then(|s| s.to_str()).unwrap_or("Sheet1"); @@ -725,6 +749,54 @@ mod tests { ); } + /// Where `part`'s entry starts in the central directory of `package`. + fn central_entry(package: &[u8], part: &str) -> usize { + let mut at = 0; + while let Some(i) = package.get(at..).and_then(|rest| rest.windows(4).position(|w| w == b"PK\x01\x02")) { + let entry = at + i; + let len = u16::from_le_bytes([package[entry + 28], package[entry + 29]]) as usize; + if &package[entry + 46..entry + 46 + len] == part.as_bytes() { + return entry; + } + at = entry + 46; + } + panic!("{part} is not in the package"); + } + + /// `package` with `part` declaring `size` unpacked bytes: the reader refuses the part on the + /// declared size, so no real 512 MB part has to be built. + fn declaring(mut package: Vec, part: &str, size: u32) -> Vec { + let at = central_entry(&package, part); + package[at + 24..at + 28].copy_from_slice(&size.to_le_bytes()); + package + } + + #[test] + fn a_part_over_the_size_limit_says_how_to_get_the_data_in() { + let mut wb = Workbook::new(); + wb.sheet_mut(0).unwrap().set_value(CellRef::new(0, 0), gridcraft_core::Value::text("big")); + let package = save_bytes(&wb, "big.xlsx").unwrap(); + // The size reported in storytold/gridcraft#1242. + let msg = open_bytes("big.xlsx", &declaring(package.clone(), "xl/worksheets/sheet1.xml", 540_278_731)).unwrap_err().to_string(); + assert!(msg.starts_with("We can't open 'big.xlsx': "), "{msg}"); + assert!(msg.contains("xl/worksheets/sheet1.xml") && msg.contains("515 MB") && msg.contains("512 MB"), "{msg}"); + assert!(msg.contains("several sheets") && msg.contains("CSV"), "{msg}"); + assert!(!msg.contains("bytes"), "{msg}"); + // Splitting sheets doesn't shrink the shared strings, which every sheet uses: only CSV helps. + let msg = open_bytes("big.xlsx", &declaring(package.clone(), "xl/sharedStrings.xml", 540_278_731)).unwrap_err().to_string(); + assert!(msg.contains("xl/sharedStrings.xml") && msg.contains("515 MB") && msg.contains("CSV"), "{msg}"); + assert!(!msg.contains("several sheets"), "{msg}"); + // Just over the limit never reads as a size equal to it. + let msg = open_bytes("big.xlsx", &declaring(package, "xl/worksheets/sheet1.xml", 512 * 1024 * 1024 + 1)).unwrap_err().to_string(); + assert!(msg.contains("more than 512 MB"), "{msg}"); + // A package that fails for another reason keeps its own error, oversized part or not. + let mut broken = declaring(save_bytes(&wb, "big.xlsx").unwrap(), "xl/worksheets/sheet1.xml", 540_278_731); + let at = central_entry(&broken, "xl/workbook.xml"); + broken[at + 46 + "xl/workbook.xml".len() - 1] = b'x'; + let msg = open_bytes("big.xlsx", &broken).unwrap_err().to_string(); + assert!(msg.starts_with("We can't open 'big.xlsx': ") && !msg.contains("unpacks"), "{msg}"); + } + #[test] fn content_wins_over_a_mismatched_extension() { let mut wb = Workbook::new(); diff --git a/crates/xlsx/src/lib.rs b/crates/xlsx/src/lib.rs index 3300f97fe..9d070a38d 100644 --- a/crates/xlsx/src/lib.rs +++ b/crates/xlsx/src/lib.rs @@ -47,6 +47,7 @@ use gridcraft_model::Workbook; pub use crypto::{DecryptError, check_encryption, decrypt}; pub use csv::{MAX_CSV_BYTES, read_csv, write_csv, write_csv_limited}; pub use ods::read_ods; +pub use package::MAX_PART; pub use passthrough::rebase_links; pub use xls::read_xls; pub use xlsb::read_xlsb; @@ -85,6 +86,12 @@ pub fn read_xlsx(bytes: &[u8]) -> Result<(Workbook, ReadReport), IoError> { read::read_xlsx(bytes) } +/// The part of a zip package that declares more than [`MAX_PART`] unpacked bytes, with that size: +/// what an [`IoError::TooLarge`] from reading the package is about when one part is too large. +pub fn oversized_part(bytes: &[u8]) -> Option<(String, u64)> { + package::oversized_part(bytes) +} + /// Which member of the SpreadsheetML family a package is saved as. Only the content type of the /// workbook part differs (ECMA-376 Part 1 Workbook part; MS-OI29500 adds the macro-enabled types). #[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] diff --git a/crates/xlsx/src/package.rs b/crates/xlsx/src/package.rs index 9a2899906..e5bde555f 100644 --- a/crates/xlsx/src/package.rs +++ b/crates/xlsx/src/package.rs @@ -120,6 +120,15 @@ impl<'a> Package<'a> { } } +/// The first part whose declared unpacked size is over [`MAX_PART`], with that size. +pub fn oversized_part(bytes: &[u8]) -> Option<(String, u64)> { + let mut zip = zip::ZipArchive::new(Cursor::new(bytes)).ok()?; + (0..zip.len().min(MAX_ENTRIES)).find_map(|i| { + let f = zip.by_index_raw(i).ok()?; + (f.size() > MAX_PART).then(|| (f.name().to_string(), f.size())) + }) +} + fn prefix(part: &str, e: IoError) -> IoError { match e { IoError::Xml(m) => IoError::Xml(format!("{part}: {m}")), From 9097170b08d3205c87e93777ad28c3982161c04a Mon Sep 17 00:00:00 2001 From: derechopiano <257026466+derechopiano@users.noreply.github.com> Date: Thu, 15 Oct 2026 10:33:57 +0100 Subject: [PATCH 07/16] XLSX: preserve carriage returns in saved text --- crates/xlsx/src/tests/roundtrip.rs | 41 ++++++++++++++++++++++++++++++ crates/xlsx/src/xml.rs | 16 +++++++++--- docs/file-format-parity.md | 3 ++- 3 files changed, 56 insertions(+), 4 deletions(-) diff --git a/crates/xlsx/src/tests/roundtrip.rs b/crates/xlsx/src/tests/roundtrip.rs index 901802905..a9425fc74 100644 --- a/crates/xlsx/src/tests/roundtrip.rs +++ b/crates/xlsx/src/tests/roundtrip.rs @@ -693,6 +693,47 @@ fn empty_workbook_roundtrip() { assert!(crate::sniff(&bytes) == crate::Format::Xlsx); } +#[test] +fn cell_text_carriage_returns_survive_xlsx_roundtrip() { + let mut wb = Workbook::new(); + let values = [("A1", "Line 1\r\nLine 2"), ("A2", "Part A\rPart B"), ("A3", "Item 3\nItem 4"), ("A4", "literal _x000D_ marker")]; + let sheet = wb.sheet_mut(0).unwrap(); + for (address, text) in values { + sheet.set_value(at(address), Value::text(text)); + } + let mut formula = Cell::formula(Formula::new("A1")); + formula.value = Value::text("Cached\r\ntext"); + sheet.cells.set(at("B1"), formula); + sheet.comments.insert( + at("A1"), + Comment { + author: "Tester".into(), + text: "Note\r\ntext".into(), + replies: vec![], + threaded: false, + resolved: false, + visible: false, + dates: vec![], + }, + ); + + let bytes = write_xlsx(&wb).unwrap(); + let mut package = Package::open(&bytes).unwrap(); + let shared_strings = String::from_utf8(package.read("xl/sharedStrings.xml").unwrap().unwrap()).unwrap(); + assert!(shared_strings.contains("Line 1_x000D_\nLine 2"), "CR must be encoded before XML parsing"); + assert!(shared_strings.contains("Part A_x000D_Part B")); + assert!(!shared_strings.contains('\r'), "raw CR is normalized to LF by XML parsers"); + let sheet_xml = String::from_utf8(package.read("xl/worksheets/sheet1.xml").unwrap().unwrap()).unwrap(); + assert!(sheet_xml.contains("Cached_x000D_\ntext"), "formula caches also use ST_Xstring"); + + let (back, _) = read_xlsx(&bytes).unwrap(); + for (address, text) in values { + assert_eq!(back.sheets[0].value(at(address)), Value::text(text), "{address} changed on save/reopen"); + } + assert_eq!(back.sheets[0].value(at("B1")), Value::text("Cached\r\ntext")); + assert_eq!(back.sheets[0].comments.get(&at("A1")).map(|c| c.text.as_str()), Some("Note\r\ntext")); +} + #[test] fn writing_repairs_invalid_models() { let mut wb = Workbook::new(); diff --git a/crates/xlsx/src/xml.rs b/crates/xlsx/src/xml.rs index 43bb5b53e..95c2ce0e8 100644 --- a/crates/xlsx/src/xml.rs +++ b/crates/xlsx/src/xml.rs @@ -253,6 +253,7 @@ pub fn esc(s: &str) -> String { '&' => o.push_str("&"), '<' => o.push_str("<"), '>' => o.push_str(">"), + '\r' => o.push_str(" "), c if is_xml_char(c) => o.push(c), _ => {} } @@ -283,15 +284,16 @@ fn is_xml_char(c: char) -> bool { matches!(c, '\t' | '\n' | '\r' | '\u{20}'..='\u{D7FF}' | '\u{E000}'..='\u{FFFD}' | '\u{10000}'..='\u{10FFFF}') } -/// ST_Xstring encoding for cell text: characters XML can't carry become `_xHHHH_`, and literal -/// `_xHHHH_` sequences are protected with `_x005F_`. The result still needs [`esc`]. +/// ST_Xstring encoding for cell text: characters XML can't carry and carriage returns become +/// `_xHHHH_`, and literal `_xHHHH_` sequences are protected with `_x005F_`. The result still needs +/// [`esc`]. pub fn encode_xstring(s: &str) -> String { let mut o = String::with_capacity(s.len()); let chars: Vec = s.chars().collect(); for (i, &c) in chars.iter().enumerate() { if c == '_' && looks_like_escape(&chars, i) { o.push_str("_x005F_"); - } else if !is_xml_char(c) || (c.is_control() && !matches!(c, '\t' | '\n' | '\r')) { + } else if !is_xml_char(c) || (c.is_control() && !matches!(c, '\t' | '\n')) { o.push_str(&format!("_x{:04X}_", c as u32 & 0xFFFF)); } else { o.push(c); @@ -397,11 +399,19 @@ mod tests { #[test] fn xstring() { assert_eq!(decode_xstring("a_x000D_b"), "a\rb"); + assert_eq!(encode_xstring("a\r\nb\rc"), "a_x000D_\nb_x000D_c"); + assert_eq!(decode_xstring(&encode_xstring("a\r\nb\rc")), "a\r\nb\rc"); assert_eq!(encode_xstring("a\u{1}b"), "a_x0001_b"); assert_eq!(decode_xstring(&encode_xstring("_x0041_")), "_x0041_"); assert_eq!(decode_xstring("_x00"), "_x00"); } + #[test] + fn text_escaping_preserves_carriage_returns() { + assert_eq!(esc("a\rb"), "a b"); + assert_eq!(esc_attr("a\rb"), "a b"); + } + #[test] fn numbers() { assert_eq!(num(1.0), "1"); diff --git a/docs/file-format-parity.md b/docs/file-format-parity.md index 988472414..f66ae2b40 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-12 · **Change:** trivial (XLSX chart sheets are kept on save (#727); previously minor: CSV save keeps the separator it was opened with) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-15 · **Change:** minor (XLSX text retains carriage returns on save/reopen) · **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** @@ -85,6 +85,7 @@ refresh. Counted under features (Get & Transform, ~10% ready), not here. | Date | Change | Summary | |---|---|---| +| 2026-10-15 | minor | XLSX text preserves CR and CRLF across save/reopen; shared strings use `_x000D_` and other XML text uses ` ` (#1488) | | 2026-10-12 | trivial | XLSX chart sheets are kept on save (#727) | | 2026-10-12 | trivial | CSV and TSV export is capped at 512 MiB and refuses a larger used range with an error (#528) | | 2026-10-12 | minor | `.xls` import reads the calculation settings (CalcMode, CalcIter, CalcCount, CalcDelta, CalcPrecision), so circular models calculate instead of showing `#CIRC!` (#724) | From b930d60bc3918477b311c0e0a23b9d3047bac682 Mon Sep 17 00:00:00 2001 From: justinjohn0306 Date: Thu, 15 Oct 2026 18:32:00 +0530 Subject: [PATCH 08/16] #1505: keep scalar INDEX MATCH formulas scalar --- crates/xlsx/src/array_formula.rs | 9 +++++- crates/xlsx/src/tests/array_export.rs | 46 +++++++++++++++++++++++++++ docs/file-format-parity.md | 6 +++- 3 files changed, 59 insertions(+), 2 deletions(-) diff --git a/crates/xlsx/src/array_formula.rs b/crates/xlsx/src/array_formula.rs index caa5e8e9f..2001b0492 100644 --- a/crates/xlsx/src/array_formula.rs +++ b/crates/xlsx/src/array_formula.rs @@ -154,7 +154,9 @@ impl Scan<'_> { "INDEX" => { let row = args.get(1); let col = args.get(2); - !row.is_some_and(positive) || col.is_some_and(|e| !positive(e)) || (col.is_none() && !args.first().is_some_and(single_column)) + let row_positive = row.is_some_and(|e| positive_index(e, shapes.get(1).copied())); + let col_positive = col.is_some_and(|e| positive_index(e, shapes.get(2).copied())); + !row_positive || col.is_some_and(|_| !col_positive) || (col.is_none() && !args.first().is_some_and(single_column)) } "ROW" | "COLUMN" => { affected |= first_array && !array_context; @@ -187,6 +189,11 @@ fn positive(e: &Expr) -> bool { matches!(e, Expr::Number(n) if *n >= 1.0) } +fn positive_index(e: &Expr, shape: Option) -> bool { + // MATCH returns a one-based position or an error, never INDEX's zero "whole row" selector. + positive(e) || (matches!(e, Expr::Call(name, _) if matches!(name.as_str(), "MATCH" | "XMATCH")) && shape.is_some_and(|s| !s.array)) +} + fn one(e: &Expr) -> bool { matches!(e, Expr::Number(n) if *n == 1.0) } diff --git a/crates/xlsx/src/tests/array_export.rs b/crates/xlsx/src/tests/array_export.rs index b90b98668..7042e14a3 100644 --- a/crates/xlsx/src/tests/array_export.rs +++ b/crates/xlsx/src/tests/array_export.rs @@ -161,6 +161,47 @@ fn scalar_formulas_constants_and_legacy_cse_do_not_require_dynamic_metadata() { } } +#[test] +fn legacy_index_match_formulas_remain_scalar_after_xlsx_roundtrip() { + let source = super::minimal( + r#"a0.1cxINDEX($B$1:$B$3,MATCH(D1,$A$1:$A$3,0))0.7IF(E1="","",INDEX($B$1:$B$3,MATCH(D1,$A$1:$A$3,0)))0.7VLOOKUP(D1,$A$1:$B$3,2,FALSE)0.7SUM($B$1:$B$3)1.2MATCH(D1,$A$1:$A$3,0)3b0.4IFERROR(INDEX($B$1:$B$3,MATCH(D1,$A$1:$A$3,0)),0)0.7c0.7"#, + &[], + "", + "", + ); + let (opened, _) = read_xlsx(&source).unwrap(); + let saved = write_xlsx(&opened).unwrap(); + let mut pkg = Package::open(&saved).unwrap(); + let sheet = pkg.read_xml("xl/worksheets/sheet1.xml").unwrap().unwrap(); + + for address in ["F1", "F2", "F3", "F5", "F6", "F7"] { + assert_scalar(&sheet, address); + } +} + +#[test] +fn shared_legacy_index_match_formulas_remain_scalar_after_xlsx_roundtrip() { + let mut body = String::from(""); + for row in 16..=39 { + let formula = if row == 16 { + format!(r#"IF(E{row}="","",IFERROR(INDEX($B$1:$B$3,MATCH(D{row},$A$1:$A$3,0)),0))"#) + } else { + r#""#.to_string() + }; + body.push_str(&format!(r#"{formula}0.7"#)); + } + body.push_str(""); + let source = super::minimal(&body, &[], "", ""); + let (opened, _) = read_xlsx(&source).unwrap(); + let saved = write_xlsx(&opened).unwrap(); + let mut pkg = Package::open(&saved).unwrap(); + let sheet = pkg.read_xml("xl/worksheets/sheet1.xml").unwrap().unwrap(); + + for row in 16..=39 { + assert_scalar(&sheet, &format!("F{row}")); + } +} + #[test] fn calculated_table_column_keeps_formula_and_cell_value() { let mut wb = Workbook::new(); @@ -304,6 +345,11 @@ fn implicit_intersection_metadata_respects_function_argument_context() { ("SQRT(INDIRECT(A1))", true), ("INDEX(A1:B3,1,1)", false), ("INDEX(A1:A3,1)", false), + ("INDEX(A1:A3,MATCH(D1,A1:A3,0))", false), + ("IFERROR(INDEX(A1:A3,MATCH(D1,A1:A3,0)),0)", false), + ("IF(E1=\"\",\"\",INDEX(A1:A3,MATCH(D1,A1:A3,0)))", false), + ("INDEX(A1:B3,MATCH(D1,A1:A3,0),MATCH(E1,A1:B1,0))", false), + ("INDEX(A1:B3,MATCH(D1,A1:A3,0))", true), ("SUM(INDEX(A1:B3,0,1))", false), ("SQRT(INDEX(A1:B3,0,1))", true), ("SUM(ROW(A1:A3))", true), diff --git a/docs/file-format-parity.md b/docs/file-format-parity.md index 988472414..ebb6ad612 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-12 · **Change:** trivial (XLSX chart sheets are kept on save (#727); previously minor: CSV save keeps the separator it was opened with) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 +> **Last reviewed:** 2026-10-15 · **Last updated:** 2026-10-15 · **Change:** minor (legacy XLSX `INDEX`/`MATCH` formulas stay scalar on save (#1505)) · **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** @@ -11,6 +11,9 @@ documentation (the Office bundle is off limits under this repo's clean-room rule support comes from `crates/engine/src/io.rs` (`FileKind`), `crates/xlsx/src` and the tests in `crates/xlsx/src/tests/` (round-trip, fixtures, malformed input, CSV, ODS, XLSB, XLS, PivotTables). +Legacy scalar `INDEX(range, MATCH(...))` formulas remain ordinary formulas on save, including when +shared formulas are expanded during import (`array_export.rs`, #1505). + ## Workbook formats Ink created with Draw is written as standard, unfilled DrawingML polylines, without visible point @@ -85,6 +88,7 @@ refresh. Counted under features (Get & Transform, ~10% ready), not here. | Date | Change | Summary | |---|---|---| +| 2026-10-15 | minor | XLSX save keeps legacy `INDEX`/`MATCH` formulas scalar instead of marking them as dynamic arrays (#1505) | | 2026-10-12 | trivial | XLSX chart sheets are kept on save (#727) | | 2026-10-12 | trivial | CSV and TSV export is capped at 512 MiB and refuses a larger used range with an error (#528) | | 2026-10-12 | minor | `.xls` import reads the calculation settings (CalcMode, CalcIter, CalcCount, CalcDelta, CalcPrecision), so circular models calculate instead of showing `#CIRC!` (#724) | From 67be35564c23fcfd13759336e864e83692e8438a Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Thu, 15 Oct 2026 16:01:47 +0200 Subject: [PATCH 09/16] Keep leading empty columns and rows when saving CSV/text Found while testing crates/xlsx/src/csv.rs:204 (write_csv_limited, starts at sheet.used_range().start instead of A1) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/tests.rs | 7 +++++++ crates/xlsx/src/csv.rs | 15 ++++++++------- crates/xlsx/src/tests/csv_tests.rs | 9 +++++---- 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index ae499a189..3dab83cd5 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -3282,3 +3282,10 @@ 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 saving_csv_keeps_leading_empty_columns_and_rows() { + let wb = gridcraft_xlsx::read_csv(b",x\r\n\r\n,,y\r\n", &gridcraft_xlsx::CsvOptions::default()).unwrap(); + let out = crate::io::save_bytes(&wb, "a.csv").unwrap(); + assert_eq!(String::from_utf8(out).unwrap(), ",x,\r\n,,\r\n,,y\r\n"); +} diff --git a/crates/xlsx/src/csv.rs b/crates/xlsx/src/csv.rs index 47486bfa2..6e707229b 100644 --- a/crates/xlsx/src/csv.rs +++ b/crates/xlsx/src/csv.rs @@ -187,12 +187,13 @@ pub fn read_csv(bytes: &[u8], opts: &CsvOptions) -> Result { Ok(wb) } -/// Writes the sheet's used range as CSV (CRLF line ends, UTF-8 without BOM). `cell_text` gives +/// Writes the sheet as CSV from A1 to the end of its used range, so leading empty columns and rows +/// stay as empty fields and every cell keeps its position (CRLF line ends, UTF-8 without BOM). `cell_text` gives /// each cell's field text: the caller decides how values look (Save As CSV writes them as /// displayed, number formats applied); quoting is done here. /// -/// The output is capped at [`MAX_CSV_BYTES`]: two cells far apart make the used range, and so the -/// file, as big as the whole rectangle between them (`A1` and `XFD1048576` are 16 GiB of commas). +/// The output is capped at [`MAX_CSV_BYTES`]: a cell far from A1 makes the file as big as the whole +/// rectangle between them (`A1` and `XFD1048576` are 16 GiB of commas). /// An export that would pass the cap is refused before anything is allocated, or as soon as the /// text written so far passes it, with [`IoError::TooLarge`]; accepted exports keep every field in /// its place. @@ -209,15 +210,15 @@ pub fn write_csv_limited(sheet: &Sheet, delimiter: u8, limit: usize, mut cell_te |what: String| IoError::TooLarge(format!("the CSV would be {what}, over the limit of {} MiB; export a smaller range", limit >> 20)); // Every row has a delimiter between its fields and a CRLF, whatever the cells hold: a lower // bound on the output that needs no cell text. - let (rows, cols) = (u64::from(r.end.row - r.start.row) + 1, u64::from(r.end.col - r.start.col) + 1); + let (rows, cols) = (u64::from(r.end.row) + 1, u64::from(r.end.col) + 1); let per_row = (cols - 1).saturating_mul(delim.len_utf8() as u64).saturating_add(2); let least = rows.saturating_mul(per_row); if least > limit as u64 { return Err(too_large(format!("at least {} MiB ({rows} rows by {cols} columns)", least >> 20))); } - for row in r.start.row..=r.end.row { - for col in r.start.col..=r.end.col { - if col > r.start.col { + for row in 0..=r.end.row { + for col in 0..=r.end.col { + if col > 0 { out.push(delim); } let t = cell_text(CellRef::new(row, col)); diff --git a/crates/xlsx/src/tests/csv_tests.rs b/crates/xlsx/src/tests/csv_tests.rs index cc52072e7..a833c75be 100644 --- a/crates/xlsx/src/tests/csv_tests.rs +++ b/crates/xlsx/src/tests/csv_tests.rs @@ -110,14 +110,15 @@ fn writing() { Value::Array(_) => String::new(), }; let out = String::from_utf8(write_csv(sh, b',', raw).unwrap()).unwrap(); - assert_eq!(out, "\"a,b\",\"q\"\"t\",\r\n0.1,FALSE,\r\n#DIV/0!,,\"x\ny\"\r\n"); + // The first cell is B2: the file starts at A1, so the leading empty row and column stay. + assert_eq!(out, ",,,\r\n,\"a,b\",\"q\"\"t\",\r\n,0.1,FALSE,\r\n,#DIV/0!,,\"x\ny\"\r\n"); let semi = String::from_utf8(write_csv(sh, b';', raw).unwrap()).unwrap(); - assert!(semi.starts_with("a,b;")); + assert!(semi.starts_with(";;;\r\n;a,b;")); assert!(write_csv(&Sheet::new("E"), b',', raw).unwrap().is_empty()); // Round trip. let back = read_csv(out.as_bytes(), &CsvOptions::default()).unwrap(); - assert_eq!(v(&back, 0, 0), Value::text("a,b")); - assert_eq!(v(&back, 2, 2), Value::text("x\ny")); + assert_eq!(v(&back, 1, 1), Value::text("a,b")); + assert_eq!(v(&back, 3, 3), Value::text("x\ny")); } #[test] From 706127402a58a20b2e5f8de09a99afdeb786d0bc Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Fri, 16 Oct 2026 02:41:59 +0200 Subject: [PATCH 10/16] XLSX: keep header and footer pictures on save (part of #1257) --- crates/xlsx/src/passthrough.rs | 71 +++++++++++++--- crates/xlsx/src/sheet_write.rs | 2 +- crates/xlsx/src/tests/passthrough_tests.rs | 94 ++++++++++++++++++++++ crates/xlsx/src/write.rs | 1 + docs/file-format-parity.md | 5 +- 5 files changed, 160 insertions(+), 13 deletions(-) diff --git a/crates/xlsx/src/passthrough.rs b/crates/xlsx/src/passthrough.rs index dd881c1e9..03bde2bb5 100644 --- a/crates/xlsx/src/passthrough.rs +++ b/crates/xlsx/src/passthrough.rs @@ -6,7 +6,8 @@ //! their ``, data connections, XML maps) and each sheet (query tables, cells //! mapped to XML elements, form controls, ActiveX controls, OLE objects, the sheet background and //! printer settings, with the sheet's ``, `` and `` XML, the `r:id` -//! of `` and the legacy VML shapes those controls need). The workbook also keeps its +//! of ``, the legacy VML shapes those controls need and the pictures of the headers and +//! footers, `&G`, with their ``). The workbook also keeps its //! `` and the calcFeatures extension, which Excel needs to trust the saved values. //! //! On write, kept parts get fresh part names where needed and new relationship ids; the `r:id`s in @@ -92,7 +93,7 @@ const FILE_VERSION_ATTRS: &[&str] = &["appName", "lastEdited", "lowestEdited", " const CALC_FEATURES_URI: &str = "{B58B0392-4F1F-4190-BB64-5DF3571DCE5F}"; const NS_CALC_FEATURES: &str = "http://schemas.microsoft.com/office/spreadsheetml/2018/calcfeatures"; /// Kept sheet elements, in schema order (all after ``, before ``). -const SHEET_SLOTS: &[&str] = &["picture", "oleObjects", "controls"]; +const SHEET_SLOTS: &[&str] = &["legacyDrawingHF", "picture", "oleObjects", "controls"]; /// Relationship kinds GridCraft reads and rewrites, or that depend on content it rewrites: /// kept content whose graph reaches one of them is dropped. @@ -479,7 +480,10 @@ impl KeptReader { cx.warn(format!("sheet \"{name}\": {} was dropped: {why}", kind(&r.rel_type))); } } - if rels.iter().any(|r| is_kind(r, SHEET_ELEMENT_KINDS)) { + // Notes have one VML drawing per sheet; another one holds the header and footer pictures. + let vml_drawings = rels.iter().filter(|r| is_kind(r, &["vmlDrawing"])).count(); + let notes = rels.iter().filter(|r| is_kind(r, &["comments"])).count(); + if rels.iter().any(|r| is_kind(r, SHEET_ELEMENT_KINDS)) || vml_drawings > notes { self.sheet_elements(cx, part, name, &rels, &mut p, &mut seen); } (!p.is_empty()).then(|| Arc::new(p)) @@ -503,6 +507,21 @@ impl KeptReader { Err(why) => cx.warn(format!("sheet \"{name}\": printer settings were dropped: {why}")), } } + // Header and footer pictures (`&G`): a VML drawing of their own and the images it shows, + // kept byte for byte (GridCraft writes the header text, not the pictures). + if let Some(id) = scan.legacy_drawing_hf.as_deref() + && scan.legacy_drawing.as_deref() != Some(id) + && let Some(r) = rels.iter().find(|r| r.id == id && is_kind(r, &["vmlDrawing"])) + { + match self.keep_through(cx, p, std::slice::from_ref(r), seen, &["vmlDrawing"]) { + Ok(()) => { + let xml = format!("", esc_attr(&r.id)); + p.elements.push(PreservedXml { slot: "legacyDrawingHF".into(), xml }); + cx.warn(format!("sheet \"{name}\": pictures in the header and footer are not shown; they are kept for saving")); + } + Err(why) => cx.warn(format!("sheet \"{name}\": header and footer pictures were dropped: {why}")), + } + } let mut frags = scan.fragments; // Controls and OLE objects are drawn from shapes in the legacy VML drawing. if frags.iter().any(|(slot, _)| slot != "picture") { @@ -581,18 +600,20 @@ struct SheetScan { /// (slot, XML with the namespace declarations it needs). fragments: Vec<(String, String)>, legacy_drawing: Option, + /// The relationship id on `` (header and footer pictures). + legacy_drawing_hf: Option, /// The relationship id on `` (printer settings). page_setup: Option, } /// Finds the sheet's ``, ``, `` (also inside -/// ``) and the `` id. `None` when the XML is damaged. +/// ``) and the `` and `` ids. `None` when the XML is damaged. fn scan_sheet(bytes: &[u8]) -> Option { let data = xml::to_utf8(bytes); let text = std::str::from_utf8(&data).ok()?; let mut reader = Reader::from_str(text); let mut decls: Vec<(String, String)> = vec![]; - let mut out = SheetScan { fragments: vec![], legacy_drawing: None, page_setup: None }; + let mut out = SheetScan { fragments: vec![], legacy_drawing: None, legacy_drawing_hf: None, page_setup: None }; let mut in_root = false; loop { let start = usize::try_from(reader.buffer_position()).ok()?; @@ -614,6 +635,7 @@ fn scan_sheet(bytes: &[u8]) -> Option { match local.as_slice() { b"pageSetup" => out.page_setup = rel_id_attr(&e), b"legacyDrawing" => out.legacy_drawing = rel_id_attr(&e), + b"legacyDrawingHF" => out.legacy_drawing_hf = rel_id_attr(&e), _ => {} } reader.read_to_end(e.name()).ok()?; @@ -626,12 +648,12 @@ fn scan_sheet(bytes: &[u8]) -> Option { } Event::Empty(e) if in_root => { let local = e.local_name().as_ref().to_vec(); - if local == b"legacyDrawing" || local == b"pageSetup" { + if local == b"legacyDrawing" || local == b"legacyDrawingHF" || local == b"pageSetup" { let id = rel_id_attr(&e); - if local == b"pageSetup" { - out.page_setup = id; - } else { - out.legacy_drawing = id; + match local.as_slice() { + b"pageSetup" => out.page_setup = id, + b"legacyDrawing" => out.legacy_drawing = id, + _ => out.legacy_drawing_hf = id, } } else if local == b"picture" && let Some(frag) = text.get(start..usize::try_from(reader.buffer_position()).ok()?) @@ -738,6 +760,19 @@ fn shape_numbers(vml: &str) -> Vec { .collect() } +/// The highest shape block VML text claims in `` or uses in a shape id. +fn vml_block_max(vml: &str) -> u64 { + let used = shape_numbers(vml).into_iter().max().unwrap_or(0) / 1024; + let claimed = vml + .find("().ok()).max().unwrap_or(0)) + .unwrap_or(0); + used.max(claimed) +} + /// Kept VML (controls, OLE objects) plus GridCraft's note shapes: the notes get ids in a fresh /// shape block so they can't collide with the kept shapes the sheet XML refers to. fn merge_vml(kept: &str, ours: &str) -> String { @@ -837,11 +872,27 @@ fn map_start(e: &BytesStart<'_>, prefixes: &mut HashSet>, map: &mut dyn pub struct KeptOut { used: HashSet, n: u32, + /// The highest VML shape block a kept header and footer drawing uses: notes go after it. + vml_blocks: u32, /// Package-root relationships (type, target, external), added when `_rels/.rels` is written. root: Vec<(String, String, bool)>, } impl KeptOut { + /// Reserves the VML shape blocks the kept header and footer drawings of `wb` use, so that the + /// notes drawings GridCraft writes don't reuse their shape ids. + pub fn reserve_vml_blocks(&mut self, wb: &Workbook) { + let vmls = + wb.sheets.iter().filter_map(|s| s.preserved.as_deref()).flat_map(|p| &p.parts).filter(|q| q.content_type.eq_ignore_ascii_case(CT_VML)); + let max = vmls.map(|q| vml_block_max(&String::from_utf8_lossy(&q.data))).max().unwrap_or(0); + self.vml_blocks = u32::try_from(max).unwrap_or(u32::MAX); + } + + /// The shape block of the `n`th notes drawing. + pub fn notes_block(&self, n: u32) -> u32 { + n.saturating_add(self.vml_blocks) + } + /// A part name for a kept part: its original name when that can't collide with anything /// written, otherwise a fresh one in the same folder. fn claim(&mut self, original: &str) -> String { diff --git a/crates/xlsx/src/sheet_write.rs b/crates/xlsx/src/sheet_write.rs index 78e1afaae..2d6836b6d 100644 --- a/crates/xlsx/src/sheet_write.rs +++ b/crates/xlsx/src/sheet_write.rs @@ -551,7 +551,7 @@ pub fn write_sheet(wb: &Workbook, si: usize, selected: bool, out: &mut Out) -> ( } out.default_type("vml", "application/vnd.openxmlformats-officedocument.vmlDrawing"); let vml_name = format!("xl/drawings/vmlDrawing{n}.vml"); - let vml = kept.notes_vml(vml_xml(sheet, n), &vml_name, out); + let vml = kept.notes_vml(vml_xml(sheet, out.kept.notes_block(n)), &vml_name, out); out.part(&vml_name, None, vml.into_bytes()); rels.add("comments", &format!("../comments{n}.xml")); let rid = rels.add("vmlDrawing", &format!("../drawings/vmlDrawing{n}.vml")); diff --git a/crates/xlsx/src/tests/passthrough_tests.rs b/crates/xlsx/src/tests/passthrough_tests.rs index d844daa05..23a7cdea5 100644 --- a/crates/xlsx/src/tests/passthrough_tests.rs +++ b/crates/xlsx/src/tests/passthrough_tests.rs @@ -823,3 +823,97 @@ fn file_version_and_calc_features_survive_a_save() { let plain = Unpacked::new(&write_xlsx(&gridcraft_model::Workbook::new()).unwrap()).text("xl/workbook.xml"); assert!(!plain.contains("fileVersion") && !plain.contains("extLst"), "{plain}"); } + +const LOGO: &[u8] = b"\x89PNG\r\n\x1a\n letterhead logo"; +const SIGNATURE: &[u8] = b"\x89PNG\r\n\x1a\n footer signature"; +const HF_VML: &str = r##""##; + +/// #1257: a letterhead in the header and a picture in the footer (`&G`), drawn from a VML part +/// that `` names, next to a note whose VML drawing GridCraft rewrites. +fn header_picture_package() -> Vec { + let types = r#""#; + let workbook = format!( + r#""# + ); + let sheet = format!( + r#"1&L&G&CInvoice&C&G"# + ); + let root_rels = rels(&[("rId1", "officeDocument", "xl/workbook.xml")]); + let wb_rels = rels(&[("rId1", "worksheet", "worksheets/sheet1.xml")]); + let sheet_rels = rels(&[ + ("rId1", "comments", "../comments1.xml"), + ("rId2", "vmlDrawing", "../drawings/vmlDrawing1.vml"), + ("rId3", "vmlDrawing", "../drawings/vmlDrawing2.vml"), + ]); + let hf_rels = rels(&[("rId1", "image", "../media/image1.png"), ("rId2", "image", "../media/image2.png")]); + make_zip(&[ + ("[Content_Types].xml", types.as_bytes()), + ("_rels/.rels", root_rels.as_bytes()), + ("xl/workbook.xml", workbook.as_bytes()), + ("xl/_rels/workbook.xml.rels", wb_rels.as_bytes()), + ("xl/worksheets/sheet1.xml", sheet.as_bytes()), + ("xl/worksheets/_rels/sheet1.xml.rels", sheet_rels.as_bytes()), + ("xl/comments1.xml", COMMENTS.as_bytes()), + ("xl/drawings/vmlDrawing1.vml", VML.as_bytes()), + ("xl/drawings/vmlDrawing2.vml", HF_VML.as_bytes()), + ("xl/drawings/_rels/vmlDrawing2.vml.rels", hf_rels.as_bytes()), + ("xl/media/image1.png", LOGO), + ("xl/media/image2.png", SIGNATURE), + ]) +} + +#[test] +fn header_and_footer_pictures_survive_a_save() { + let (wb, rep) = read_xlsx(&header_picture_package()).unwrap(); + assert!(!rep.warnings.iter().any(|w| w.contains("dropped")), "{:?}", rep.warnings); + assert_eq!(wb.sheet(0).unwrap().comments.len(), 1, "the note is modelled"); + // Twice: what was kept on the first save is read back and kept again. + let (again, _) = read_xlsx(&write_xlsx(&wb).unwrap()).unwrap(); + for saved in [write_xlsx(&wb).unwrap(), write_xlsx(&again).unwrap()] { + let u = Unpacked::new(&saved); + u.assert_valid(); + let sheet = sheet_part(&u, "Invoice"); + let sx = u.text(&sheet); + assert!(sx.contains("&L&G&CInvoice"), "{sx}"); + let (ty, vml) = u.target(&sheet, &attr_of(&sx, "legacyDrawingHF", "r:id")); + assert!(ty.ends_with("/vmlDrawing"), "{ty}"); + assert_eq!(u.text(&vml), HF_VML); + let pictures: Vec<(String, Vec)> = u.rels(&vml).into_iter().map(|r| (r.0, u.parts.get(&r.2).unwrap().clone())).collect(); + assert_eq!(pictures, [("rId1".to_string(), LOGO.to_vec()), ("rId2".to_string(), SIGNATURE.to_vec())]); + // The notes keep their own drawing; CT_Worksheet puts legacyDrawingHF right after it. + let notes = u.target(&sheet, &attr_of(&sx, "legacyDrawing", "r:id")).1; + assert_ne!(notes, vml); + assert!(u.text(¬es).contains("ObjectType=\"Note\"")); + assert!(sx.find("`) and the blocks of its shape ids +/// (`_x0000_s1025` is in block 1). +fn vml_blocks(vml: &str) -> (Vec, Vec) { + let i = vml.find("().parse::().unwrap() / 1024) + .collect(); + (idmap, spids) +} + +#[test] +fn notes_take_shape_blocks_the_header_pictures_dont_use() { + let (wb, _) = read_xlsx(&header_picture_package()).unwrap(); + let u = Unpacked::new(&write_xlsx(&wb).unwrap()); + let sheet = sheet_part(&u, "Invoice"); + let sx = u.text(&sheet); + let (hf_map, hf_ids) = vml_blocks(&u.text(&u.target(&sheet, &attr_of(&sx, "legacyDrawingHF", "r:id")).1)); + let (notes_map, notes_ids) = vml_blocks(&u.text(&u.target(&sheet, &attr_of(&sx, "legacyDrawing", "r:id")).1)); + assert_eq!((hf_map.as_slice(), hf_ids.as_slice()), ([1].as_slice(), [1, 1].as_slice()), "the header drawing is kept as it was"); + assert!(!notes_ids.is_empty()); + for b in notes_map.iter().chain(¬es_ids) { + assert!(!hf_map.contains(b) && !hf_ids.contains(b), "notes reuse shape block {b}: {notes_map:?} {notes_ids:?}"); + } + assert!(notes_ids.iter().all(|b| notes_map.contains(b)), "note ids outside their drawing's blocks"); +} diff --git a/crates/xlsx/src/write.rs b/crates/xlsx/src/write.rs index 06761f01d..c7e21e30e 100644 --- a/crates/xlsx/src/write.rs +++ b/crates/xlsx/src/write.rs @@ -212,6 +212,7 @@ pub fn write_xlsx(wb: &Workbook, kind: XlsxKind) -> Result, IoError> { cell_pictures: Default::default(), kept: Default::default(), }; + out.kept.reserve_vml_blocks(wb); out.default_type("rels", "application/vnd.openxmlformats-package.relationships+xml"); out.default_type("xml", "application/xml"); diff --git a/docs/file-format-parity.md b/docs/file-format-parity.md index 158149fb3..2c95c870e 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-14 · **Change:** trivial (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; previously trivial: XLSX chart sheets are kept on save (#727)) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-16 · **Change:** trivial (pictures in XLSX headers and footers are kept on save (#1257); previously trivial: 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) · **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** @@ -22,7 +22,7 @@ and repair-free opening remain unverified. | Format | Excel | GridCraft read | GridCraft write | Fidelity / notes | Tests | |---|---|---|---|---|---| -| `.xlsx` Excel Workbook (transitional) | R/W, native | yes | yes | Cells, shared/array/dynamic formulas, styles, themes, merges, CF, validation, tables, comments and notes, hyperlinks, names, freeze panes, outline, print settings, pictures, shapes, charts (simplified, see [chart-parity.md](chart-parity.md)), sparklines, PivotTables (caches and tables), protection, calc settings. **Kept unchanged on save (pass-through, `passthrough.rs`):** custom document properties, ribbon customisation and add-in task panes, custom XML, links to external workbooks (never followed or refreshed), data connections, sheet query tables, printer settings (#324), form controls, ActiveX controls, OLE objects and sheet backgrounds (with their VML shapes), and chart sheets (#727: the sheet, drawing, chart, style and colour parts are written back unchanged in their place, and left out when a sheet they plot was deleted or renamed or a worksheet took their name; they can't be edited or created); dropped when their sheet is deleted or their parts point at content GridCraft rewrites. **Not preserved:** slicers and slicer caches, timelines, data model, query tables attached to tables, sheet and workbook `extLst` entries we don't model, chart details; Strict Open XML files keep nothing. Never checked in Excel across a corpus: repair prompts unknown. 100 MB files fail (#175) | `roundtrip.rs`, `fixtures.rs`, `malformed.rs`, `pivot_tests.rs` | +| `.xlsx` Excel Workbook (transitional) | R/W, native | yes | yes | Cells, shared/array/dynamic formulas, styles, themes, merges, CF, validation, tables, comments and notes, hyperlinks, names, freeze panes, outline, print settings, pictures, shapes, charts (simplified, see [chart-parity.md](chart-parity.md)), sparklines, PivotTables (caches and tables), protection, calc settings. **Kept unchanged on save (pass-through, `passthrough.rs`):** custom document properties, ribbon customisation and add-in task panes, custom XML, links to external workbooks (never followed or refreshed), data connections, sheet query tables, printer settings (#324), form controls, ActiveX controls, OLE objects and sheet backgrounds (with their VML shapes), pictures in headers and footers (`&G`, #1257: their VML drawing and images are written back unchanged; they are not shown, and can't be inserted), and chart sheets (#727: the sheet, drawing, chart, style and colour parts are written back unchanged in their place, and left out when a sheet they plot was deleted or renamed or a worksheet took their name; they can't be edited or created); dropped when their sheet is deleted or their parts point at content GridCraft rewrites. **Not preserved:** slicers and slicer caches, timelines, data model, query tables attached to tables, sheet and workbook `extLst` entries we don't model, chart details; Strict Open XML files keep nothing. Never checked in Excel across a corpus: repair prompts unknown. 100 MB files fail (#175) | `roundtrip.rs`, `fixtures.rs`, `malformed.rs`, `pivot_tests.rs` | | `.xlsx` Excel Workbook (transitional) | R/W, native | yes | yes | Cells, shared/array/dynamic formulas, styles, themes, merges, CF, validation, tables, notes (with their shown/hidden state) and threaded comments with replies, resolved state and timestamps (`threadedComments` and `persons` parts, #300), hyperlinks, names, freeze panes, outline, print settings, pictures, shapes, charts (simplified, see [chart-parity.md](chart-parity.md)), sparklines, PivotTables (caches and tables), protection, calc settings. **Not preserved:** unmodelled parts and extensions (form controls, slicer caches, timelines, external links, OLE objects, custom XML, data model, query tables, chart details). Never checked in Excel across a corpus: repair prompts unknown. 100 MB files fail (#175) | `roundtrip.rs`, `fixtures.rs`, `malformed.rs`, `pivot_tests.rs` | | `.xlsx` Excel Workbook (transitional) | R/W, native | yes | yes | Cells, shared/array/dynamic formulas, styles, themes, merges, CF, validation, tables, comments and notes, hyperlinks, names, freeze panes, outline, print settings, pictures, shapes, charts (simplified, see [chart-parity.md](chart-parity.md)), sparklines, PivotTables (caches and tables), protection, Allow Edit Ranges (`protectedRanges` with their password hashes, #307), calc settings. **Not preserved:** unmodelled parts and extensions (form controls, slicer caches, timelines, external links, OLE objects, custom XML, data model, query tables, chart details). Never checked in Excel across a corpus: repair prompts unknown. 100 MB files fail (#175) | `roundtrip.rs`, `fixtures.rs`, `malformed.rs`, `pivot_tests.rs` | | `.xlsx` Strict Open XML | R/W | yes | no (writes transitional) | Excel also saves transitional by default | fixtures | @@ -85,6 +85,7 @@ refresh. Counted under features (Get & Transform, ~10% ready), not here. | Date | Change | Summary | |---|---|---| +| 2026-10-16 | trivial | XLSX: pictures in headers and footers (`&G`, the VML drawing `` names and its images) are kept on save; they are still not shown (#1257) | | 2026-10-14 | trivial | Formula parser: text longer than Excel's 8,192 characters, and postfix chains (`1%%%…`, `f(1)(1)…`) past the 256-level nesting limit, are parse errors (the cell keeps its formula text and cached value) instead of trees deep enough to overflow the stack when a hostile file is opened, saved or dropped | | 2026-10-12 | trivial | XLSX chart sheets are kept on save (#727) | | 2026-10-12 | trivial | CSV and TSV export is capped at 512 MiB and refuses a larger used range with an error (#528) | From e8c6fcb8b0fc65deedf245000bef859657bd4d77 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Fri, 16 Oct 2026 05:11:07 +0200 Subject: [PATCH 11/16] XLSX: keep custom column widths that round to the default width (fixes #1607) --- crates/xlsx/src/sheet_read.rs | 4 ++-- crates/xlsx/src/tests/fixtures.rs | 22 ++++++++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/crates/xlsx/src/sheet_read.rs b/crates/xlsx/src/sheet_read.rs index 0194d2268..83f8d0d91 100644 --- a/crates/xlsx/src/sheet_read.rs +++ b/crates/xlsx/src/sheet_read.rs @@ -324,8 +324,8 @@ fn read_cols(cx: &Ctx<'_>, e: &El, sheet: &mut Sheet) { info.style = Some(id); } } - // A width equal to the default with nothing else set is noise. - if info == LineInfo::default() || (info.size == Some(sheet.default_col_width) && LineInfo { size: None, ..info } == LineInfo::default()) { + // A custom width equal to the default still pins the column (Standard Width leaves it alone). + if info == LineInfo::default() { continue; } for col in (min - 1)..max { diff --git a/crates/xlsx/src/tests/fixtures.rs b/crates/xlsx/src/tests/fixtures.rs index 39ea30951..4ca580a5b 100644 --- a/crates/xlsx/src/tests/fixtures.rs +++ b/crates/xlsx/src/tests/fixtures.rs @@ -577,3 +577,25 @@ fn user_defined_function_prefix_round_trips() { assert_eq!(cell.child("f").unwrap().text, f, "written {a}"); } } + +/// `customWidth="1"` pins a column even when its width rounds to the default: Standard Width does not +/// move it, and a save must keep its `` (openpyxl writes 9.08 for Excel's 8.43, #1607). +#[test] +fn custom_widths_equal_to_the_default_round_trip() { + let body = r#" + + "#; + let (wb, _) = read_xlsx(&minimal(body, &[], "", "")).unwrap(); + let s = wb.sheet(0).unwrap(); + assert_eq!(s.default_col_width, 64.0); + for c in 3..=6 { + assert_eq!(s.cols.get(&c).and_then(|i| i.size), Some(64.0), "read col {c}"); + } + assert_eq!(s.cols.get(&7), None, "a width without customWidth equal to the default stays the default"); + let bytes = crate::write_xlsx(&wb).unwrap(); + let mut pkg = crate::package::Package::open(&bytes).unwrap(); + let sheet = pkg.read_xml("xl/worksheets/sheet1.xml").unwrap().unwrap(); + let cols: Vec<_> = + sheet.child("cols").unwrap().kids("col").map(|c| (c.attr_u32("min"), c.attr_u32("max"), c.flag("customWidth", false))).collect(); + assert_eq!(cols, [(Some(4), Some(7), true)]); +} From ad4422f5f67138692a72828c53fdd980afb66533 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Fri, 16 Oct 2026 05:59:22 +0200 Subject: [PATCH 12/16] Reject unsupported flip param in arrange.rotate Found while testing crates/engine/src/cmd/extra.rs:664 (command doc at extra.rs:114) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/cmd/extra.rs | 5 ++++- crates/engine/src/tests.rs | 21 +++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/cmd/extra.rs b/crates/engine/src/cmd/extra.rs index 01e2ac6e6..5f8126d1f 100644 --- a/crates/engine/src/cmd/extra.rs +++ b/crates/engine/src/cmd/extra.rs @@ -111,7 +111,7 @@ pub fn specs() -> Vec { "Rotate", ["Page Layout", "Arrange"], None, - "{kind: shape, id, flip?: horizontal|vertical} (swaps width/height for 90° turns)", + "{kind: shape, id} (swaps width/height for a 90° turn; `flip` is rejected: shapes have no flip state)", has_doc, rotate ), @@ -662,6 +662,9 @@ fn align(s: &mut Session, p: &Json) -> Result { } fn rotate(s: &mut Session, p: &Json) -> Result { + if p.get("flip").is_some_and(|f| !f.is_null()) { + return Err(bad("arrange.rotate", "`flip` is not supported: shapes have no flip state")); + } with_shape(s, p, |sh| std::mem::swap(&mut sh.anchor.width, &mut sh.anchor.height)) } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index ae499a189..0e200ffe6 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -3282,3 +3282,24 @@ 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 rotate_rejects_flip_param_instead_of_swapping_size() { + let mut s = s(); + let id = s.execute("insert.shape", json!({"kind": "rectangle"})).unwrap()["shape"].clone(); + let size = |s: &Session| { + let a = &s.doc().unwrap().wb.active().unwrap().shapes[0].anchor; + (a.width, a.height) + }; + let (w, h) = size(&s); + assert_ne!(w, h); + let before = s.doc().unwrap().wb.clone(); + for flip in ["horizontal", "vertical", "sideways"] { + let r = s.execute("arrange.rotate", json!({"kind": "shape", "id": id, "flip": flip})); + assert!(matches!(r, Err(crate::EngineError::BadParams { .. })), "flip {flip}: {r:?}"); + assert_eq!(size(&s), (w, h)); + assert!(std::sync::Arc::ptr_eq(&s.doc().unwrap().wb, &before)); + } + s.execute("arrange.rotate", json!({"kind": "shape", "id": id})).unwrap(); + assert_eq!(size(&s), (h, w)); +} From 4548c6f7fc3fe6e758dc4feeb5a54668a9d31103 Mon Sep 17 00:00:00 2001 From: harrisonoconnorhover <130776285+harrisonoconnorhover@users.noreply.github.com> Date: Fri, 16 Oct 2026 07:44:44 -0400 Subject: [PATCH 13/16] fix(xlsx): bound repeated column expansion (#1022) --- crates/xlsx/src/sheet_read.rs | 12 +++++- crates/xlsx/src/tests/malformed.rs | 65 ++++++++++++++++++++++++++++++ docs/decisions.md | 9 +++++ docs/file-format-parity.md | 9 ++++- 4 files changed, 92 insertions(+), 3 deletions(-) diff --git a/crates/xlsx/src/sheet_read.rs b/crates/xlsx/src/sheet_read.rs index 0194d2268..a1094ef91 100644 --- a/crates/xlsx/src/sheet_read.rs +++ b/crates/xlsx/src/sheet_read.rs @@ -38,6 +38,7 @@ struct RowState { } const MAX_HYPERLINK_CELLS: u64 = 10_000; +const MAX_COLUMN_ASSIGNMENTS: u64 = 1_000_000; pub fn read_sheet(cx: &mut Ctx<'_>, part: &str, name: &str) -> Result { let bytes = cx.pkg.read(part)?.ok_or_else(|| IoError::Format(format!("missing part {part}")))?; @@ -61,6 +62,7 @@ pub fn read_sheet(cx: &mut Ctx<'_>, part: &str, name: &str) -> Result, part: &str, name: &str) -> Result read_cols(cx, e, &mut sheet), + "cols" => read_cols(cx, e, &mut sheet, &mut column_assignments_left)?, "mergeCells" => { for m in e.kids("mergeCell") { if let Some(r) = m.attr("ref").and_then(RangeRef::parse) @@ -300,7 +302,7 @@ fn read_views(e: &El, sheet: &mut Sheet) { } } -fn read_cols(cx: &Ctx<'_>, e: &El, sheet: &mut Sheet) { +fn read_cols(cx: &Ctx<'_>, e: &El, sheet: &mut Sheet, remaining: &mut u64) -> Result<(), IoError> { for c in e.kids("col") { let (Some(min), Some(max)) = (c.attr_u32("min"), c.attr_u32("max")) else { continue }; if min == 0 || min > MAX_COLS || max < min { @@ -328,10 +330,16 @@ fn read_cols(cx: &Ctx<'_>, e: &El, sheet: &mut Sheet) { if info == LineInfo::default() || (info.size == Some(sheet.default_col_width) && LineInfo { size: None, ..info } == LineInfo::default()) { continue; } + // Overlapping declarations repeat work even when the final map stays small. + // Charge the clipped span before expansion, sharing the budget across blocks. + *remaining = remaining + .checked_sub(u64::from(max - min + 1)) + .ok_or_else(|| IoError::TooLarge(format!("sheet \"{}\": more than {MAX_COLUMN_ASSIGNMENTS} expanded column assignments", sheet.name)))?; for col in (min - 1)..max { sheet.cols.insert(col, info); } } + Ok(()) } fn read_row(cx: &mut Ctx<'_>, sheet: &mut Sheet, st: &mut RowState, row: &El) { diff --git a/crates/xlsx/src/tests/malformed.rs b/crates/xlsx/src/tests/malformed.rs index d029acaa4..6d9d4379f 100644 --- a/crates/xlsx/src/tests/malformed.rs +++ b/crates/xlsx/src/tests/malformed.rs @@ -256,3 +256,68 @@ fn hostile_formula_text_does_not_overflow_the_stack() { assert_eq!(a_len, 1_000_001); assert_eq!(a_value, gridcraft_core::Value::Number(3.0)); } + +#[test] +fn repeated_column_ranges_have_one_worksheet_work_budget() { + // 62 * 16,384 = 1,015,808 assignments, while the map holds at most 16,384 columns. + // This stays small even on the unbounded reader, but exceeds the one-million work cap. + let col = r#""#; + for (first, second) in [(62, 0), (31, 31)] { + let body = format!("{}{}", col.repeat(first), col.repeat(second)); + assert!(matches!(read_xlsx(&minimal(&body, &[], "", "")), Err(IoError::TooLarge(_))), "blocks: {first}, {second}"); + } +} + +#[test] +fn column_work_budget_counts_only_clipped_nondefault_assignments() { + let full = r#""#; + // Default-only records and invalid coordinates do not expand into column assignments. + let noise = format!( + "{}{}", + r#""#.repeat(62), + r#" + + + "# + ); + // The last declaration clips to 576 columns: 61 * 16,384 + 576 = 1,000,000. + let cols = format!("{noise}{}", full.repeat(61)); + let body = format!("{cols}"); + let (wb, _) = read_xlsx(&minimal(&body, &[], "", "")).unwrap(); + let sheet = wb.sheet(0).unwrap(); + assert_eq!(sheet.cols.len(), 16_384); + assert_eq!(sheet.cols.get(&15_807).unwrap().size, Some(crate::tables::col_width_to_px(10.0))); + assert_eq!(sheet.cols.get(&15_808).unwrap().size, Some(crate::tables::col_width_to_px(20.0))); + assert_eq!(sheet.cols.get(&16_383).unwrap().size, Some(crate::tables::col_width_to_px(20.0))); + + let body = format!("{cols}"); + assert!(matches!(read_xlsx(&minimal(&body, &[], "", "")), Err(IoError::TooLarge(_)))); +} + +#[test] +fn ordinary_overlapping_column_ranges_keep_last_declaration() { + let styles = r#" + "#; + let rels = r#""#; + let body = r#" + + + "#; + let (wb, _) = read_xlsx(&minimal(body, &[("xl/styles.xml", styles)], rels, "")).unwrap(); + let sheet = wb.sheet(0).unwrap(); + assert_eq!(sheet.cols.len(), 5); + let first = sheet.cols.get(&0).unwrap(); + assert_eq!(first.size, Some(crate::tables::col_width_to_px(10.0))); + assert!(first.hidden); + assert_eq!(wb.styles.get(first.style.unwrap()).num_fmt.as_str(), "0.00"); + for col in 1..4 { + let info = sheet.cols.get(&col).unwrap(); + assert_eq!(info.size, Some(crate::tables::col_width_to_px(20.0))); + assert!(!info.hidden); + assert_eq!(info.style, None); + } + let last = sheet.cols.get(&4).unwrap(); + assert_eq!(last.size, None); + assert!(last.hidden); + assert_eq!(wb.styles.get(last.style.unwrap()).num_fmt.as_str(), "0.00"); +} diff --git a/docs/decisions.md b/docs/decisions.md index 7b0a99a38..2aa18069f 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -1,5 +1,14 @@ # Decisions +## Bound repeated XLSX column expansion (#1022) + +Limit each worksheet to 1,000,000 nondefault column assignments across all `` blocks. +This allows more than 60 full-grid passes while bounding repeated work on overlapping spans; +counting only the final map size would not limit that work. Charge each clipped span before +inserting it, after the existing default-record skip. Exceeding the limit returns the existing +`IoError::TooLarge`, which aborts the import instead of silently dropping a partially read sheet. +Keep the current last-wins semantics rather than introduce interval storage or normalization. + ## Formula localization uses the existing interface preference PR #98 builds on the persisted `UiState.language` introduced by #31 and extended by #104. diff --git a/docs/file-format-parity.md b/docs/file-format-parity.md index 158149fb3..760109480 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-14 · **Change:** trivial (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; previously trivial: XLSX chart sheets are kept on save (#727)) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-16 · **Change:** minor (XLSX column declarations share a per-sheet expansion-work limit, #1022) · **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** @@ -13,6 +13,12 @@ support comes from `crates/engine/src/io.rs` (`FileKind`), `crates/xlsx/src` and ## Workbook formats +XLSX column declarations are limited to 1,000,000 expanded column assignments per worksheet, +including repeated overlaps across `` blocks. Spans are clipped to the grid and records +that would be skipped as defaults are excluded before charging the limit. Excess work returns +the existing file-too-large error; accepted overlaps retain their existing last-wins behavior +(#1022). + Ink created with Draw is written as standard, unfilled DrawingML polylines, without visible point captions or private metadata. Supported numeric polylines reopen as editable ink, retaining points, colour, integer width and anchor. Curves, filled paths, transforms and unsupported line styles are @@ -85,6 +91,7 @@ refresh. Counted under features (Get & Transform, ~10% ready), not here. | Date | Change | Summary | |---|---|---| +| 2026-10-16 | minor | XLSX column imports refuse more than 1,000,000 cumulative expanded assignments per sheet before expanding the next span; repeated overlaps cannot bypass the work limit (#1022) | | 2026-10-14 | trivial | Formula parser: text longer than Excel's 8,192 characters, and postfix chains (`1%%%…`, `f(1)(1)…`) past the 256-level nesting limit, are parse errors (the cell keeps its formula text and cached value) instead of trees deep enough to overflow the stack when a hostile file is opened, saved or dropped | | 2026-10-12 | trivial | XLSX chart sheets are kept on save (#727) | | 2026-10-12 | trivial | CSV and TSV export is capped at 512 MiB and refuses a larger used range with an error (#528) | From 2c6ffe489c98184e1022f802dadc84adfe5b1dca Mon Sep 17 00:00:00 2001 From: midasdf <205497066+midasdf@users.noreply.github.com> Date: Sat, 17 Oct 2026 12:37:15 +0900 Subject: [PATCH 14/16] XLSX theme: keep the major/minor ea and cs typefaces through open + save MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A theme's carries latin, ea (East Asian) and cs (complex script) typefaces for each of the major and minor fonts. The model held only the latin ones and the writer emitted and unconditionally, so a Japanese workbook's theme fonts (e.g. 游ゴシック, ＭＳ Ｐゴシック) were silently deleted on open + save. Theme gains major_font_ea, minor_font_ea, major_font_cs and minor_font_cs (#[serde(default)], so older GridCraft JSON still loads; empty means the file did not set one). The XLSX reader and writer round-trip them escaped, and a theme without them writes exactly the empty attributes it wrote before. Rendering and font resolution are unchanged: the UI does not consume ea yet, and wiring it in is a follow-up. --- crates/model/src/style.rs | 42 +++++++++++++++++- crates/xlsx/src/theme.rs | 86 +++++++++++++++++++++++++++++++++++-- docs/file-format-parity.md | 8 ++-- docs/localization-parity.md | 5 ++- 4 files changed, 129 insertions(+), 12 deletions(-) diff --git a/crates/model/src/style.rs b/crates/model/src/style.rs index d3976876e..77edb1fd3 100644 --- a/crates/model/src/style.rs +++ b/crates/model/src/style.rs @@ -119,16 +119,33 @@ pub struct Theme { pub colors: [u32; 12], pub major_font: String, pub minor_font: String, + /// The heading font's East Asian typeface (``); empty when the file did not set one. + #[serde(default)] + pub major_font_ea: String, + /// The body font's East Asian typeface (``); empty when the file did not set one. + #[serde(default)] + pub minor_font_ea: String, + /// The heading font's complex-script typeface (``); empty when the file did not set one. + #[serde(default)] + pub major_font_cs: String, + /// The body font's complex-script typeface (``); empty when the file did not set one. + #[serde(default)] + pub minor_font_cs: String, } impl Default for Theme { - /// GridCraft's own default theme ("Craft"): original palette. + /// GridCraft's own default theme ("Craft"): original palette. The East Asian and + /// complex-script typefaces stay empty: the file did not set one. fn default() -> Self { Theme { name: "Craft".into(), colors: [0xFFFFFF, 0x000000, 0xE8E8E8, 0x0E2841, 0x156082, 0xE97132, 0x196B24, 0x0F9ED5, 0xA02B93, 0x4EA72E, 0x467886, 0x96607D], major_font: crate::DEFAULT_FONT.into(), minor_font: crate::DEFAULT_FONT.into(), + major_font_ea: String::new(), + minor_font_ea: String::new(), + major_font_cs: String::new(), + minor_font_cs: String::new(), } } } @@ -444,4 +461,27 @@ mod tests { assert_eq!(Color::Theme(1, 500).resolve(&th), Some([128, 128, 128])); assert_eq!(Color::Auto.resolve(&th), None); } + + #[test] + fn theme_json_keeps_ea_and_cs_and_reads_old_json() { + // JSON written before the ea/cs fields existed still deserializes (`serde(default)`). + let mut old = serde_json::to_value(Theme::default()).unwrap(); + let obj = old.as_object_mut().unwrap(); + for key in ["major_font_ea", "minor_font_ea", "major_font_cs", "minor_font_cs"] { + assert!(obj.remove(key).is_some(), "the default theme serializes {key}"); + } + assert_eq!(serde_json::from_value::(old).unwrap(), Theme::default()); + + // Set fields serialize and come back. + let t = Theme { + major_font_ea: "游ゴシック".into(), + minor_font_ea: "ＭＳ Ｐゴシック".into(), + major_font_cs: "Arial".into(), + minor_font_cs: "Times New Roman".into(), + ..Theme::default() + }; + let json = serde_json::to_string(&t).unwrap(); + assert!(json.contains("major_font_ea") && json.contains("游ゴシック"), "{json}"); + assert_eq!(serde_json::from_str::(&json).unwrap(), t); + } } diff --git a/crates/xlsx/src/theme.rs b/crates/xlsx/src/theme.rs index 68e25bd69..2080fa49b 100644 --- a/crates/xlsx/src/theme.rs +++ b/crates/xlsx/src/theme.rs @@ -41,6 +41,12 @@ fn scheme_rgb(e: &El) -> Option { None } +/// The typeface of `//` (`latin`, `ea` or `cs`), `None` when absent or +/// empty (an empty typeface means "not set" and must not overwrite the default). +fn typeface<'a>(fs: &'a El, font: &str, kind: &str) -> Option<&'a str> { + fs.path(&[font, kind]).and_then(|e| e.attr("typeface")).filter(|f| !f.is_empty()) +} + pub fn read_theme(root: &El) -> Theme { let mut t = Theme::default(); if let Some(n) = root.attr("name") { @@ -57,12 +63,24 @@ pub fn read_theme(root: &El) -> Theme { } } if let Some(fs) = el.child("fontScheme") { - if let Some(f) = fs.path(&["majorFont", "latin"]).and_then(|l| l.attr("typeface")).filter(|f| !f.is_empty()) { + if let Some(f) = typeface(fs, "majorFont", "latin") { t.major_font = f.to_string(); } - if let Some(f) = fs.path(&["minorFont", "latin"]).and_then(|l| l.attr("typeface")).filter(|f| !f.is_empty()) { + if let Some(f) = typeface(fs, "minorFont", "latin") { t.minor_font = f.to_string(); } + if let Some(f) = typeface(fs, "majorFont", "ea") { + t.major_font_ea = f.to_string(); + } + if let Some(f) = typeface(fs, "minorFont", "ea") { + t.minor_font_ea = f.to_string(); + } + if let Some(f) = typeface(fs, "majorFont", "cs") { + t.major_font_cs = f.to_string(); + } + if let Some(f) = typeface(fs, "minorFont", "cs") { + t.minor_font_cs = f.to_string(); + } } t } @@ -80,10 +98,14 @@ pub fn write_theme(t: &Theme) -> String { s.push_str(""); let _ = write!( s, - "", + "", esc_attr(name), esc_attr(&t.major_font), - esc_attr(&t.minor_font) + esc_attr(&t.major_font_ea), + esc_attr(&t.major_font_cs), + esc_attr(&t.minor_font), + esc_attr(&t.minor_font_ea), + esc_attr(&t.minor_font_cs) ); // Format scheme: three entries per list, as the schema requires. s.push_str(""); @@ -113,6 +135,9 @@ pub fn write_theme(t: &Theme) -> String { mod tests { use super::*; + /// A Japanese Office theme: the heading font sets latin, ea and cs; the body font only ea. + const EA_THEME: &str = r#""#; + #[test] fn roundtrip() { let mut t = Theme::default(); @@ -125,4 +150,57 @@ mod tests { let back = read_theme(&crate::xml::parse(x.as_bytes()).unwrap()); assert_eq!(back, t); } + + #[test] + fn ea_and_cs_typefaces_roundtrip() { + let t = read_theme(&crate::xml::parse(EA_THEME.as_bytes()).unwrap()); + assert_eq!(t.major_font, "Calibri"); + assert_eq!(t.major_font_ea, "游ゴシック"); + assert_eq!(t.major_font_cs, "Arial"); + // No `` under the minor font: the default stays. + assert_eq!(t.minor_font, Theme::default().minor_font); + assert_eq!(t.minor_font_ea, "ＭＳ Ｐゴシック"); + assert!(t.minor_font_cs.is_empty()); + + let x = write_theme(&t); + assert!( + x.contains(r#""#), + "{x}" + ); + assert!( + x.contains(r#""#), + "{x}" + ); + assert_eq!(read_theme(&crate::xml::parse(x.as_bytes()).unwrap()), t); + } + + #[test] + fn theme_without_ea_or_cs_writes_empty_attributes() { + let t = Theme::default(); + let x = write_theme(&t); + assert!(x.contains(r#""#), "{x}"); + assert!(x.contains(r#""#), "{x}"); + } + + #[test] + fn explicit_empty_ea_and_cs_read_as_unset() { + let xml = EA_THEME + .replace(r#""#, r#""#) + .replace(r#""#, r#""#); + let t = read_theme(&crate::xml::parse(xml.as_bytes()).unwrap()); + assert!(t.major_font_ea.is_empty() && t.minor_font_ea.is_empty()); + assert_eq!(t.major_font_cs, "Arial"); + let x = write_theme(&t); + assert!(x.contains(r#""#), "{x}"); + assert_eq!(read_theme(&crate::xml::parse(x.as_bytes()).unwrap()), t); + } + + #[test] + fn ea_and_cs_typefaces_are_escaped() { + let t = Theme { major_font_ea: "游&ゴシック".into(), minor_font_cs: "A\"C\"".into(), ..Theme::default() }; + let x = write_theme(&t); + assert!(x.contains(r#""#), "{x}"); + assert!(x.contains(r#""#), "{x}"); + assert_eq!(read_theme(&crate::xml::parse(x.as_bytes()).unwrap()), t); + } } diff --git a/docs/file-format-parity.md b/docs/file-format-parity.md index 5a7588986..07989d5a5 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-17 · **Change:** trivial (a workbook theme's major/minor East Asian (``) and complex-script (``) typefaces are kept through a save; previously trivial: 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) · **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** @@ -22,10 +22,7 @@ and repair-free opening remain unverified. | Format | Excel | GridCraft read | GridCraft write | Fidelity / notes | Tests | |---|---|---|---|---|---| -| `.xlsx` Excel Workbook (transitional) | R/W, native | yes | yes | Cells, shared/array/dynamic formulas, styles, themes, merges, CF, validation, tables, comments and notes, hyperlinks, names, freeze panes, outline, print settings, pictures, shapes, charts (simplified, see [chart-parity.md](chart-parity.md)), sparklines, PivotTables (caches and tables), protection, calc settings. **Kept unchanged on save (pass-through, `passthrough.rs`):** custom document properties, ribbon customisation and add-in task panes, custom XML, links to external workbooks (never followed or refreshed), data connections, sheet query tables, printer settings (#324), form controls, ActiveX controls, OLE objects and sheet backgrounds (with their VML shapes), and chart sheets (#727: the sheet, drawing, chart, style and colour parts are written back unchanged in their place, and left out when a sheet they plot was deleted or renamed or a worksheet took their name; they can't be edited or created); dropped when their sheet is deleted or their parts point at content GridCraft rewrites. **Not preserved:** slicers and slicer caches, timelines, data model, query tables attached to tables, sheet and workbook `extLst` entries we don't model, chart details; Strict Open XML files keep nothing. Never checked in Excel across a corpus: repair prompts unknown. 100 MB files fail (#175) | `roundtrip.rs`, `fixtures.rs`, `malformed.rs`, `pivot_tests.rs` | -| `.xlsx` Excel Workbook (transitional) | R/W, native | yes | yes | Cells, shared/array/dynamic formulas, styles, themes, merges, CF, validation, tables, notes (with their shown/hidden state) and threaded comments with replies, resolved state and timestamps (`threadedComments` and `persons` parts, #300), hyperlinks, names, freeze panes, outline, print settings, pictures, shapes, charts (simplified, see [chart-parity.md](chart-parity.md)), sparklines, PivotTables (caches and tables), protection, calc settings. **Not preserved:** unmodelled parts and extensions (form controls, slicer caches, timelines, external links, OLE objects, custom XML, data model, query tables, chart details). Never checked in Excel across a corpus: repair prompts unknown. 100 MB files fail (#175) | `roundtrip.rs`, `fixtures.rs`, `malformed.rs`, `pivot_tests.rs` | -| `.xlsx` Excel Workbook (transitional) | R/W, native | yes | yes | Cells, shared/array/dynamic formulas, styles, themes, merges, CF, validation, tables, comments and notes, hyperlinks, names, freeze panes, outline, print settings, pictures, shapes, charts (simplified, see [chart-parity.md](chart-parity.md)), sparklines, PivotTables (caches and tables), protection, Allow Edit Ranges (`protectedRanges` with their password hashes, #307), calc settings. **Not preserved:** unmodelled parts and extensions (form controls, slicer caches, timelines, external links, OLE objects, custom XML, data model, query tables, chart details). Never checked in Excel across a corpus: repair prompts unknown. 100 MB files fail (#175) | `roundtrip.rs`, `fixtures.rs`, `malformed.rs`, `pivot_tests.rs` | -| `.xlsx` Strict Open XML | R/W | yes | no (writes transitional) | Excel also saves transitional by default | fixtures | +| `.xlsx` Excel Workbook (transitional) | R/W, native | yes | yes | Cells, shared/array/dynamic formulas, styles, themes (the major/minor `latin`, East Asian `ea` and complex-script `cs` typefaces are kept through a save), merges, CF, validation, tables, comments and notes, hyperlinks, names, freeze panes, outline, print settings, pictures, shapes, charts (simplified, see [chart-parity.md](chart-parity.md)), sparklines, PivotTables (caches and tables), protection, calc settings. **Kept unchanged on save (pass-through, `passthrough.rs`):** custom document properties, ribbon customisation and add-in task panes, custom XML, links to external workbooks (never followed or refreshed), data connections, sheet query tables, printer settings (#324), form controls, ActiveX controls, OLE objects and sheet backgrounds (with their VML shapes), and chart sheets (#727: the sheet, drawing, chart, style and colour parts are written back unchanged in their place, and left out when a sheet they plot was deleted or renamed or a worksheet took their name; they can't be edited or created); dropped when their sheet is deleted or their parts point at content GridCraft rewrites. **Not preserved:** slicers and slicer caches, timelines, data model, query tables attached to tables, sheet and workbook `extLst` entries we don't model, chart details; Strict Open XML files keep nothing. Never checked in Excel across a corpus: repair prompts unknown. 100 MB files fail (#175) | `roundtrip.rs`, `fixtures.rs`, `malformed.rs`, `pivot_tests.rs` | | `.xlsm` Macro-Enabled Workbook | R/W | yes | yes, macros kept, not run | The VBA project is kept byte-for-byte and written back only into `.xlsm`/`.xltm` (#6); macros never run. Saved with the macro-enabled workbook content type | `roundtrip.rs` (`vba_project_roundtrip`, `package_kind_sets_workbook_content_type`), engine `io.rs` (`macros_are_saved_only_into_xlsm`) | | `.xltx` / `.xltm` templates | R/W | yes | partial | Saved with the template (and macro-enabled template) content type, chosen by extension; `.xltm` keeps a VBA project, `.xltx` drops it; "new from template" behaviour missing | `roundtrip.rs` (`package_kind_sets_workbook_content_type`), engine `io.rs` | | `.xlsb` Binary Workbook | R/W | **values only** (#101) | no | Names, visibility, numbers, text, booleans, errors, merges, cached formula results; no formulas, formatting, charts, names | `xlsb_tests.rs` | @@ -85,6 +82,7 @@ refresh. Counted under features (Get & Transform, ~10% ready), not here. | Date | Change | Summary | |---|---|---| +| 2026-10-20 | trivial | A workbook theme's major/minor East Asian (``) and complex-script (``) typefaces are read and written back, so a Japanese theme's fonts (e.g. 游ゴシック) survive open + save; they were deleted before because the model had nowhere to keep them. Rendering still resolves fonts without them (follow-up) | | 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..2bad6d4d0 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-17 · **Change:** trivial (a Japanese workbook keeps its theme's East Asian typeface (``) through open + save; the UI does not use it for rendering yet) · **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, @@ -56,7 +56,7 @@ separators, number/date formats, machine translation with agent review, and scri | French | fr | 0 (0%) | no | Latin: yes | no | none | 6–10 h | | Portuguese | pt-BR | 923 (~42%) | dialogs and tooltips (dialogs take effect with #669) | Latin: yes | no | menus, File view, dialogs | 2–4 h | | Indonesian | id | 0 (0%) | no | Latin: yes | no | none | 5–8 h | -| Japanese | ja | 268 (~12%) | no | CJK system-font fallback; IME unverified; no furigana display | no | menus only | 8–12 h | +| Japanese | ja | 268 (~12%) | no | CJK system-font fallback; the theme's East Asian typeface (``) survives open + save (rendering does not use it yet); IME unverified; no furigana display | no | menus only | 8–12 h | | German | de | 266 (~12%) | no | Latin: yes | no | menus only | 4–8 h | | Korean | ko | 266 (~12%) | no | Hangul via the CJK fallback; IME unverified | no | menus only | 6–10 h | | Vietnamese | vi | 0 (0%) | no | Latin with stacked diacritics: unverified | no | none | 6–10 h | @@ -83,6 +83,7 @@ language (none so far) and of localized function names. | Date | Change | Summary | |---|---|---| +| 2026-10-20 | trivial | XLSX: a workbook theme's major/minor East Asian (``) and complex-script (``) typefaces are kept through open + save, so a Japanese workbook keeps its theme's East Asian font; rendering does not resolve them yet (follow-up) | | 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 189eb7ae525c17b240b4bd350bc03b6e3d7e6ba7 Mon Sep 17 00:00:00 2001 From: Mouhand-Kaddo Date: Tue, 20 Oct 2026 09:21:42 +0400 Subject: [PATCH 15/16] M12.AR14: keep the theme's per-script typefaces () on save --- crates/model/src/style.rs | 17 ++++++++++++ crates/xlsx/src/theme.rs | 52 +++++++++++++++++++++++++++++++++++-- docs/file-format-parity.md | 5 ++-- docs/localization-parity.md | 3 ++- 4 files changed, 72 insertions(+), 5 deletions(-) diff --git a/crates/model/src/style.rs b/crates/model/src/style.rs index 77edb1fd3..186e5efb3 100644 --- a/crates/model/src/style.rs +++ b/crates/model/src/style.rs @@ -131,6 +131,13 @@ pub struct Theme { /// The body font's complex-script typeface (``); empty when the file did not set one. #[serde(default)] pub minor_font_cs: String, + /// The heading font's per-script typefaces (``), as + /// (script, typeface) in file order. + #[serde(default)] + pub major_font_scripts: Vec<(String, String)>, + /// The body font's per-script typefaces, as (script, typeface) in file order. + #[serde(default)] + pub minor_font_scripts: Vec<(String, String)>, } impl Default for Theme { @@ -146,6 +153,8 @@ impl Default for Theme { minor_font_ea: String::new(), major_font_cs: String::new(), minor_font_cs: String::new(), + major_font_scripts: Vec::new(), + minor_font_scripts: Vec::new(), } } } @@ -452,6 +461,14 @@ mod tests { assert!(!t.get(StyleId(999)).font.bold); } + #[test] + fn a_theme_saved_before_per_script_fonts_still_opens() { + let mut json = serde_json::to_value(Theme::default()).unwrap(); + let fields = json.as_object_mut().unwrap(); + assert!(fields.remove("major_font_scripts").is_some() && fields.remove("minor_font_scripts").is_some()); + assert_eq!(serde_json::from_value::(json).unwrap(), Theme::default()); + } + #[test] fn colors() { let th = Theme::default(); diff --git a/crates/xlsx/src/theme.rs b/crates/xlsx/src/theme.rs index 2080fa49b..818e5137a 100644 --- a/crates/xlsx/src/theme.rs +++ b/crates/xlsx/src/theme.rs @@ -9,6 +9,9 @@ use gridcraft_model::Theme; use crate::xml::{El, esc_attr}; +const MAX_SCRIPT_FONTS: usize = 64; +const MAX_SCRIPT_FONT_CHARS: usize = 256; + const SLOTS: [(&str, usize); 12] = [ ("dk1", 1), ("lt1", 0), @@ -81,10 +84,23 @@ pub fn read_theme(root: &El) -> Theme { if let Some(f) = typeface(fs, "minorFont", "cs") { t.minor_font_cs = f.to_string(); } + t.major_font_scripts = fs.child("majorFont").map(script_fonts).unwrap_or_default(); + t.minor_font_scripts = fs.child("minorFont").map(script_fonts).unwrap_or_default(); } t } +/// The `` entries of a font collection, in file order. A hostile file is +/// held to [`MAX_SCRIPT_FONTS`] entries of at most [`MAX_SCRIPT_FONT_CHARS`] characters each. +fn script_fonts(collection: &El) -> Vec<(String, String)> { + let capped = |v: Option<&str>| v.unwrap_or_default().chars().take(MAX_SCRIPT_FONT_CHARS).collect::(); + collection.kids("font").take(MAX_SCRIPT_FONTS).map(|e| (capped(e.attr("script")), capped(e.attr("typeface")))).collect() +} + +fn script_fonts_xml(fonts: &[(String, String)]) -> String { + fonts.iter().map(|(script, face)| format!("", esc_attr(script), esc_attr(face))).collect() +} + /// A minimal, schema-complete theme part for `t`. pub fn write_theme(t: &Theme) -> String { let mut s = String::from("\n"); @@ -98,14 +114,16 @@ pub fn write_theme(t: &Theme) -> String { s.push_str(""); let _ = write!( s, - "", + "{}{}", esc_attr(name), esc_attr(&t.major_font), esc_attr(&t.major_font_ea), esc_attr(&t.major_font_cs), + script_fonts_xml(&t.major_font_scripts), esc_attr(&t.minor_font), esc_attr(&t.minor_font_ea), - esc_attr(&t.minor_font_cs) + esc_attr(&t.minor_font_cs), + script_fonts_xml(&t.minor_font_scripts) ); // Format scheme: three entries per list, as the schema requires. s.push_str(""); @@ -203,4 +221,34 @@ mod tests { assert!(x.contains(r#""#), "{x}"); assert_eq!(read_theme(&crate::xml::parse(x.as_bytes()).unwrap()), t); } + + /// An Arabic Office theme: Arabic, Hebrew and Japanese faces in the per-script lists. + const SCRIPT_THEME: &str = r#""#; + + #[test] + fn per_script_typefaces_roundtrip_in_file_order() { + let t = read_theme(&crate::xml::parse(SCRIPT_THEME.as_bytes()).unwrap()); + let pairs = |v: &[(String, String)]| v.iter().map(|(s, f)| (s.clone(), f.clone())).collect::>(); + let expect = + |a: &str, j: &str| vec![("Arab".to_string(), a.to_string()), ("Hebr".to_string(), a.to_string()), ("Jpan".to_string(), j.to_string())]; + assert_eq!(pairs(&t.major_font_scripts), expect("Times New Roman", "游ゴシック Light")); + assert_eq!(pairs(&t.minor_font_scripts), expect("Arial", "游ゴシック")); + let x = write_theme(&t); + assert!( + x.contains(r#""#), + "{x}" + ); + assert_eq!(read_theme(&crate::xml::parse(x.as_bytes()).unwrap()), t); + } + + #[test] + fn hostile_per_script_lists_are_capped() { + let entries: String = (0..100).map(|i| format!(r#""#, "x".repeat(300))).collect(); + let xml = format!( + r#"{entries}"# + ); + let t = read_theme(&crate::xml::parse(xml.as_bytes()).unwrap()); + assert_eq!(t.major_font_scripts.len(), 64); + assert!(t.major_font_scripts.iter().all(|(script, face)| script.chars().count() <= 256 && face.chars().count() <= 256)); + } } diff --git a/docs/file-format-parity.md b/docs/file-format-parity.md index 07989d5a5..e25880b85 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-17 · **Change:** trivial (a workbook theme's major/minor East Asian (``) and complex-script (``) typefaces are kept through a save; previously trivial: 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) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-20 · **Change:** minor (XLSX theme fonts keep their per-script typefaces (`` and the rest) through a save; a workbook theme's major/minor East Asian (``) and complex-script (``) typefaces are kept through a save; previously trivial: 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) · **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** @@ -22,7 +22,7 @@ and repair-free opening remain unverified. | Format | Excel | GridCraft read | GridCraft write | Fidelity / notes | Tests | |---|---|---|---|---|---| -| `.xlsx` Excel Workbook (transitional) | R/W, native | yes | yes | Cells, shared/array/dynamic formulas, styles, themes (the major/minor `latin`, East Asian `ea` and complex-script `cs` typefaces are kept through a save), merges, CF, validation, tables, comments and notes, hyperlinks, names, freeze panes, outline, print settings, pictures, shapes, charts (simplified, see [chart-parity.md](chart-parity.md)), sparklines, PivotTables (caches and tables), protection, calc settings. **Kept unchanged on save (pass-through, `passthrough.rs`):** custom document properties, ribbon customisation and add-in task panes, custom XML, links to external workbooks (never followed or refreshed), data connections, sheet query tables, printer settings (#324), form controls, ActiveX controls, OLE objects and sheet backgrounds (with their VML shapes), and chart sheets (#727: the sheet, drawing, chart, style and colour parts are written back unchanged in their place, and left out when a sheet they plot was deleted or renamed or a worksheet took their name; they can't be edited or created); dropped when their sheet is deleted or their parts point at content GridCraft rewrites. **Not preserved:** slicers and slicer caches, timelines, data model, query tables attached to tables, sheet and workbook `extLst` entries we don't model, chart details; Strict Open XML files keep nothing. Never checked in Excel across a corpus: repair prompts unknown. 100 MB files fail (#175) | `roundtrip.rs`, `fixtures.rs`, `malformed.rs`, `pivot_tests.rs` | +| `.xlsx` Excel Workbook (transitional) | R/W, native | yes | yes | Cells, shared/array/dynamic formulas, styles, themes (the major/minor `latin`, East Asian `ea` and complex-script `cs` typefaces and the per-script `` lists are kept through a save), merges, CF, validation, tables, comments and notes, hyperlinks, names, freeze panes, outline, print settings, pictures, shapes, charts (simplified, see [chart-parity.md](chart-parity.md)), sparklines, PivotTables (caches and tables), protection, calc settings. **Kept unchanged on save (pass-through, `passthrough.rs`):** custom document properties, ribbon customisation and add-in task panes, custom XML, links to external workbooks (never followed or refreshed), data connections, sheet query tables, printer settings (#324), form controls, ActiveX controls, OLE objects and sheet backgrounds (with their VML shapes), and chart sheets (#727: the sheet, drawing, chart, style and colour parts are written back unchanged in their place, and left out when a sheet they plot was deleted or renamed or a worksheet took their name; they can't be edited or created); dropped when their sheet is deleted or their parts point at content GridCraft rewrites. **Not preserved:** slicers and slicer caches, timelines, data model, query tables attached to tables, sheet and workbook `extLst` entries we don't model, chart details; Strict Open XML files keep nothing. Never checked in Excel across a corpus: repair prompts unknown. 100 MB files fail (#175) | `roundtrip.rs`, `fixtures.rs`, `malformed.rs`, `pivot_tests.rs` | | `.xlsm` Macro-Enabled Workbook | R/W | yes | yes, macros kept, not run | The VBA project is kept byte-for-byte and written back only into `.xlsm`/`.xltm` (#6); macros never run. Saved with the macro-enabled workbook content type | `roundtrip.rs` (`vba_project_roundtrip`, `package_kind_sets_workbook_content_type`), engine `io.rs` (`macros_are_saved_only_into_xlsm`) | | `.xltx` / `.xltm` templates | R/W | yes | partial | Saved with the template (and macro-enabled template) content type, chosen by extension; `.xltm` keeps a VBA project, `.xltx` drops it; "new from template" behaviour missing | `roundtrip.rs` (`package_kind_sets_workbook_content_type`), engine `io.rs` | | `.xlsb` Binary Workbook | R/W | **values only** (#101) | no | Names, visibility, numbers, text, booleans, errors, merges, cached formula results; no formulas, formatting, charts, names | `xlsb_tests.rs` | @@ -82,6 +82,7 @@ refresh. Counted under features (Get & Transform, ~10% ready), not here. | Date | Change | Summary | |---|---|---| +| 2026-10-20 | minor | A workbook theme's major/minor per-script typefaces (``, also Hebr, Jpan and the rest) are read and written back in file order after ``; they were dropped before. A hostile file is held to 64 entries of 256 characters | | 2026-10-20 | trivial | A workbook theme's major/minor East Asian (``) and complex-script (``) typefaces are read and written back, so a Japanese theme's fonts (e.g. 游ゴシック) survive open + save; they were deleted before because the model had nowhere to keep them. Rendering still resolves fonts without them (follow-up) | | 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 | diff --git a/docs/localization-parity.md b/docs/localization-parity.md index 2bad6d4d0..0226c395a 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-17 · **Change:** trivial (a Japanese workbook keeps its theme's East Asian typeface (``) through open + save; the UI does not use it for rendering yet) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-20 · **Change:** minor (an Arabic workbook keeps its theme's per-script Arabic typeface (``) through open + save; a Japanese workbook keeps its theme's East Asian typeface (``) through open + save; the UI does not use it for rendering yet) · **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-20 | minor | XLSX: a theme's per-script typefaces (``, Hebr, Jpan…) are kept through open + save, so Arabic Excel's theme fonts for shapes, charts and theme-font cells survive a GridCraft save | | 2026-10-20 | trivial | XLSX: a workbook theme's major/minor East Asian (``) and complex-script (``) typefaces are kept through open + save, so a Japanese workbook keeps its theme's East Asian font; rendering does not resolve them yet (follow-up) | | 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 | From d056570d97b69009120626fd30a7b2fd46c6eff6 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 02:57:51 +0000 Subject: [PATCH 16/16] Integration: cargo fmt --- crates/ui-egui/src/lib.rs | 6 +----- crates/ui-egui/tests/interaction.rs | 5 +---- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index ba54404ab..59e418dfd 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -833,11 +833,7 @@ impl SheetApp { } else { "This workbook contains VBA macros. GridCraft keeps them when you save as .xlsm, but does not run them." }; - self.notice = Some(Notice { - text: text.into(), - kind: NoticeKind::Warning, - action: Some(("What this means".into(), "macros".into())), - }); + self.notice = Some(Notice { text: text.into(), kind: NoticeKind::Warning, action: Some(("What this means".into(), "macros".into())) }); } else { let text = if msgs.len() == 1 { msgs[0].to_string() diff --git a/crates/ui-egui/tests/interaction.rs b/crates/ui-egui/tests/interaction.rs index 0b1d5c42e..5d0907c39 100644 --- a/crates/ui-egui/tests/interaction.rs +++ b/crates/ui-egui/tests/interaction.rs @@ -784,10 +784,7 @@ fn opening_a_macro_workbook_shows_a_notice() { #[test] fn dropped_macro_warning_never_promises_to_keep_macros() { let mut h = harness(blank()); - for w in [ - "macros (VBA project) reference a missing part and were dropped", - "macros (VBA project) could not be read and were dropped: bad zip", - ] { + for w in ["macros (VBA project) reference a missing part and were dropped", "macros (VBA project) could not be read and were dropped: bad zip"] { h.state_mut().show_open_warnings(&json!({"warnings": [w]})); let n = h.state().notice.clone().unwrap(); assert_eq!(n.kind, gridcraft_ui_egui::NoticeKind::Warning);