From 41cb19fd6d001cb886cab2558809158cec66da7f Mon Sep 17 00:00:00 2001 From: Eduardo Garcia Date: Fri, 16 Oct 2026 12:58:42 -0300 Subject: [PATCH] Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/cmd/file.rs | 3 +- crates/engine/src/io.rs | 5 +++- crates/engine/tests/pivotchart_link.rs | 41 ++++++++++++++++++++++++++ crates/xlsx/src/chart.rs | 15 +++++++--- crates/xlsx/src/lib.rs | 6 ++++ crates/xlsx/src/sheet_write.rs | 2 +- crates/xlsx/src/write.rs | 12 ++++++++ docs/file-format-parity.md | 3 +- 8 files changed, 79 insertions(+), 8 deletions(-) create mode 100644 crates/engine/tests/pivotchart_link.rs diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index 9ddc05611..291c06268 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -139,8 +139,9 @@ fn save_as(s: &mut Session, p: &Json) -> Result { fn save_bytes(s: &mut Session, p: &Json) -> Result { let fmt = str_param(p, "format").unwrap_or("xlsx"); let d = s.doc()?; - let bytes = crate::io::save_bytes(&d.wb, &format!("x.{fmt}"))?; let name = format!("{}.{fmt}", std::path::Path::new(&d.display_title()).file_stem().and_then(|x| x.to_str()).unwrap_or("Book")); + // The file name goes into a PivotChart's link to its PivotTable. + let bytes = crate::io::save_bytes(&d.wb, &name)?; Ok(json!({"name": name, "base64": crate::io::base64_encode(&bytes), "bytes": bytes.len()})) } diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index 14717a3df..24da81fac 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -207,7 +207,10 @@ pub fn save_bytes_as(wb: &Workbook, path: &str, kind: FileKind) -> Result gridcraft_xlsx::write_xlsx_as(wb, gridcraft_xlsx::XlsxKind::from_path(path)).map_err(|e| EngineError::Other(e.to_string())), + FileKind::Xlsx => { + let book = std::path::Path::new(path).file_name().and_then(|n| n.to_str()).unwrap_or_default(); + gridcraft_xlsx::write_xlsx_named(wb, gridcraft_xlsx::XlsxKind::from_path(path), book).map_err(|e| EngineError::Other(e.to_string())) + } FileKind::Xls => { Err(EngineError::Other("Excel 97-2003 (.xls) is supported for import only. Save as .xlsx or another supported export format.".into())) } diff --git a/crates/engine/tests/pivotchart_link.rs b/crates/engine/tests/pivotchart_link.rs new file mode 100644 index 000000000..04c5d830d --- /dev/null +++ b/crates/engine/tests/pivotchart_link.rs @@ -0,0 +1,41 @@ +//! #1447: a PivotChart is bound to its PivotTable behind the workbook's name. + +use std::io::Read; + +use gridcraft_engine::Session; +use serde_json::{Value as Json, json}; + +fn run(s: &mut Session, command: &str, params: Json) -> Json { + s.execute(command, params).unwrap() +} + +fn source() -> Session { + let mut s = Session::new(); + s.new_workbook(); + run( + &mut s, + "range.setValues", + json!({"range": "A1", "values": [["Region ", " Sales ", "Qty"], ["East", 10, 1], ["West", 20, 2], ["East", 5, 3]]}), + ); + s +} + +#[test] +fn pivot_chart_link_names_the_workbook_in_brackets() { + let mut s = source(); + run(&mut s, "selection.set", json!({"range": "A1"})); + run(&mut s, "insert.pivotTable", json!({"rows": ["Region"], "values": ["Sales"]})); + run(&mut s, "insert.pivotChart", json!({})); + let bytes = gridcraft_engine::io::save_bytes(&s.doc().unwrap().wb, "Q1 Report.xlsx").unwrap(); + let mut zip = zip::ZipArchive::new(std::io::Cursor::new(bytes)).unwrap(); + let mut chart = String::new(); + zip.by_name("xl/charts/chart1.xml").unwrap().read_to_string(&mut chart).unwrap(); + assert!(chart.contains("[Q1 Report.xlsx]Sheet2!PivotTable1"), "{chart}"); + + // Without a file name, a placeholder workbook is named; reading drops the brackets again. + let saved = run(&mut s, "file.saveBytes", json!({"format": "xlsx"})); + let mut reopened = Session::new(); + run(&mut reopened, "file.open", json!({"name": "q.xlsx", "base64": saved["base64"]})); + let link = reopened.doc().unwrap().wb.sheets.iter().flat_map(|sh| sh.charts.iter()).find_map(|c| c.pivot.clone()); + assert_eq!(link.as_deref(), Some("Sheet2!PivotTable1")); +} diff --git a/crates/xlsx/src/chart.rs b/crates/xlsx/src/chart.rs index dee33c83d..da64e0f44 100644 --- a/crates/xlsx/src/chart.rs +++ b/crates/xlsx/src/chart.rs @@ -236,7 +236,7 @@ pub fn read_chart(root: &El) -> Option { y_title: y_ax.and_then(|a| a.child("title")).and_then(rich_title), source: None, by_rows: false, - pivot: root.path(&["pivotSource", "name"]).map(|n| n.text.trim().to_string()).filter(|n| !n.is_empty()), + pivot: root.path(&["pivotSource", "name"]).map(|n| strip_book(n.text.trim()).to_string()).filter(|n| !n.is_empty()), }) } @@ -610,17 +610,24 @@ fn axis_xml(a: &Ax<'_>) -> String { s } +/// A PivotChart link without the workbook in brackets: `[Book.xlsx]Sheet2!PT` → `Sheet2!PT`. +fn strip_book(link: &str) -> &str { + link.strip_prefix('[').and_then(|r| r.split_once(']')).map_or(link, |(_, rest)| rest) +} + /// Chart part XML for a model chart on sheet `si`. -pub fn write_chart(wb: &Workbook, si: usize, ch: &Chart) -> String { +pub fn write_chart(wb: &Workbook, si: usize, ch: &Chart, book_name: &str) -> String { let mut s = String::from( "\n", ); if (1..=48).contains(&ch.style) { let _ = write!(s, "", ch.style); } - // A PivotChart names its PivotTable (`Sheet2!PivotTable1`). + // A PivotChart names its PivotTable behind the workbook in brackets (`[Book.xlsx]Sheet2!PivotTable1`); + // without the brackets Excel shows an ordinary chart. if let Some(link) = ch.pivot.as_deref().map(str::trim).filter(|l| !l.is_empty()) { - let _ = write!(s, "{}", esc(link)); + let link = strip_book(link); + let _ = write!(s, "[{}]{}", esc(book_name), esc(link)); } s.push_str(""); match &ch.title { diff --git a/crates/xlsx/src/lib.rs b/crates/xlsx/src/lib.rs index 3300f97fe..1197cea4f 100644 --- a/crates/xlsx/src/lib.rs +++ b/crates/xlsx/src/lib.rs @@ -138,6 +138,12 @@ pub fn write_xlsx_as(wb: &Workbook, kind: XlsxKind) -> Result, IoError> write::write_xlsx(wb, kind) } +/// Writes a workbook as the given member of the `.xlsx` family for the file `book_name` (a bare +/// file name, used for the workbook in a PivotChart's link to its PivotTable). +pub fn write_xlsx_named(wb: &Workbook, kind: XlsxKind, book_name: &str) -> Result, IoError> { + write::write_xlsx_named(wb, kind, Some(book_name)) +} + /// CSV import options. #[derive(Clone, Debug, PartialEq)] pub struct CsvOptions { diff --git a/crates/xlsx/src/sheet_write.rs b/crates/xlsx/src/sheet_write.rs index 78e1afaae..1d55569bc 100644 --- a/crates/xlsx/src/sheet_write.rs +++ b/crates/xlsx/src/sheet_write.rs @@ -522,7 +522,7 @@ pub fn write_sheet(wb: &Workbook, si: usize, selected: bool, out: &mut Out) -> ( drels.add_uri(crate::chartex::REL_CHARTEX, &format!("../charts/chartEx{n}.xml")) } else { let name = format!("xl/charts/chart{}.xml", out.charts); - out.part(&name, Some(CT_CHART), crate::chart::write_chart(wb, si, ch).into_bytes()); + out.part(&name, Some(CT_CHART), crate::chart::write_chart(wb, si, ch, &out.book_name).into_bytes()); drels.add("chart", &format!("../charts/chart{}.xml", out.charts)) }; objs.push(Obj::Chart(ch, rid)); diff --git a/crates/xlsx/src/write.rs b/crates/xlsx/src/write.rs index 06761f01d..a900722f6 100644 --- a/crates/xlsx/src/write.rs +++ b/crates/xlsx/src/write.rs @@ -113,6 +113,8 @@ pub struct Out { pub sst: Sst, pub images: u32, pub charts: u32, + /// The workbook's file name, which a PivotChart's link to its PivotTable names in brackets. + pub book_name: String, /// References of chartex parts, defined as hidden `_xlchart.v1.N` names. pub chart_names: Vec, pub drawings: u32, @@ -185,7 +187,16 @@ fn check_sheet_names(wb: &Workbook) -> Result<(), IoError> { Ok(()) } +/// Workbook name written in a PivotChart's link when the file name isn't known. +const DEFAULT_BOOK_NAME: &str = "Workbook.xlsx"; + pub fn write_xlsx(wb: &Workbook, kind: XlsxKind) -> Result, IoError> { + write_xlsx_named(wb, kind, None) +} + +/// Writes the workbook for the file `book_name` (a bare file name). A PivotChart is bound to its +/// PivotTable in Excel only when its link starts with the workbook in brackets. +pub fn write_xlsx_named(wb: &Workbook, kind: XlsxKind, book_name: Option<&str>) -> Result, IoError> { if wb.sheets.is_empty() { return Err(IoError::Format("a workbook needs at least one sheet".into())); } @@ -201,6 +212,7 @@ pub fn write_xlsx(wb: &Workbook, kind: XlsxKind) -> Result, IoError> { sst: Sst::default(), images: 0, charts: 0, + book_name: book_name.filter(|n| !n.is_empty() && !n.contains(['[', ']'])).unwrap_or(DEFAULT_BOOK_NAME).to_string(), chart_names: vec![], drawings: 0, tables: 0, diff --git a/docs/file-format-parity.md b/docs/file-format-parity.md index 5a7588986..c84236748 100644 --- a/docs/file-format-parity.md +++ b/docs/file-format-parity.md @@ -1,6 +1,6 @@ # File-format parity -> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-19 · **Change:** minor (.xls dialog sheets are skipped instead of read as worksheets; `.xls` import reads each sheet's display options from WINDOW2: right-to-left, gridlines, headings, zeros, formulas; XLSX text boxes and shapes keep their paragraphs' right-to-left direction on save; CSV/TSV/TXT can open with every field kept as text via `file.open {csvAsText}`; formulas over Excel's 8,192 characters, and `%` or call chains past the nesting limit, are parse errors instead of stack overflows when a file is opened; XLSX PivotTables read and write `rowGrandTotals`/`colGrandTotals` the way Excel does, so a table saved with only one grand total opens with the right one; HTML export keeps right-to-left) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-19 · **Change:** minor (XLSX PivotCharts stay bound to their PivotTable in Excel (#1447); .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 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** @@ -91,6 +91,7 @@ refresh. Counted under features (Get & Transform, ~10% ready), not here. | 2026-10-18 | trivial | `file.open {csvAsText: true}` keeps every CSV/TSV/TXT field as the text the file holds, so identifiers (GTINs, SKUs, ZIP codes) keep their digits and leading zeros (#1808; a preference and a CLI flag are separate work) | | 2026-10-18 | trivial | XLSX PivotTables: `rowGrandTotals` is the Grand Total row and `colGrandTotals` the Grand Total column. Both were read and written the other way round, which a GridCraft round trip hid, so an Excel file with only one grand total opened showing the other | | 2026-10-19 | minor | HTML (Save as Web Page and clipboard) marks right-to-left sheets and right-to-left cell text with `dir` | +| 2026-10-16 | minor | XLSX PivotTables: a PivotChart's link to its PivotTable is written behind the workbook in brackets (`[Q1 Report.xlsx]Sheet2!PivotTable1`; `Workbook.xlsx` when the file name is unknown), so Excel binds the chart to the PivotTable instead of showing an ordinary chart; the reader drops the prefix (#1447, item 7) | | 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) |