diff --git a/ROADMAP.md b/ROADMAP.md index 90aabd4..325c3a4 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-12 | XLSX chart sheets are kept on open and save: the sheet, its drawing and chart (with style and colours) are written back unchanged in their place among the worksheets, and left out when the sheet they plot is deleted or renamed or a worksheet took their name (#727) | | 2026-10-12 | CSV and tab-separated export is capped at 512 MiB: a sheet whose used range would expand to more (two cells far apart, such as A1 and XFD1048576, are 16 GiB of commas) is refused with the rows, columns and limit, before anything is allocated or written (#528) | | 2026-10-12 | PDF writer hardening: text width cannot overflow, all-zero dash arrays and negative or NaN stroke widths are sanitised, non-finite sizes and positions draw nothing (#812) | | 2026-10-12 | PDF export writes the document title and author as UTF-16BE when they are not ASCII, so viewers show accents, currency signs and non-Latin text correctly (#813) | @@ -187,6 +188,7 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in | Date | Change | Summary | |---|---|---| +| 2026-10-12 | trivial | Progress log: XLSX chart sheets kept on save (#727) | | 2026-10-12 | trivial | Progress log: CSV export size cap (#528) | | 2026-10-12 | minor | Progress log: margins dragged on the Page Layout rulers (#792) | | 2026-10-12 | minor | Ink preservation across XLSX save/reopen (ink portion of #551) | diff --git a/crates/model/src/lib.rs b/crates/model/src/lib.rs index fa64aad..a76f30f 100644 --- a/crates/model/src/lib.rs +++ b/crates/model/src/lib.rs @@ -18,7 +18,7 @@ pub mod workbook; pub use cell::{Cell, CellPicture, Formula}; pub use features::*; pub use password::PasswordHash; -pub use preserved::{Preserved, PreservedPart, PreservedRel, PreservedXml}; +pub use preserved::{Preserved, PreservedPart, PreservedRel, PreservedSheet, PreservedXml}; pub use sheet::{DEFAULT_COL_WIDTH, DEFAULT_ROW_HEIGHT, LineIndex, Sheet, SheetViewMode, Visibility, points_to_chars}; pub use store::CellStore; pub use style::{ diff --git a/crates/model/src/preserved.rs b/crates/model/src/preserved.rs index eb8e2bb..eab8501 100644 --- a/crates/model/src/preserved.rs +++ b/crates/model/src/preserved.rs @@ -7,6 +7,8 @@ use std::sync::Arc; +use crate::sheet::Visibility; + /// A relationship from an owning part (the package root, the workbook or a sheet, or a kept part) /// to a kept part or to an external target. #[derive(Clone, Debug, Default, PartialEq, Eq)] @@ -43,6 +45,19 @@ pub struct PreservedXml { pub xml: String, } +/// A chart sheet (a sheet that holds one chart and no cells) kept as it was in the source file. Its +/// part, drawing, chart, style and colour parts are in [`Preserved::parts`]; the workbook +/// relationship to it is in [`Preserved::rels`]. +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct PreservedSheet { + pub name: String, + pub visibility: Visibility, + /// The id of the workbook relationship to the chart sheet part, in the source file. + pub rel_id: String, + /// How many worksheets come before it: its position among the model's sheets. + pub before: usize, +} + /// Unmodelled content kept for one owner (the workbook, which also holds package-root content, /// or one sheet). #[derive(Clone, Debug, Default, PartialEq, Eq)] @@ -58,11 +73,18 @@ pub struct Preserved { /// The sheet's legacy VML drawing when kept elements need its shapes (form controls, OLE /// objects). Its own relationships point into `parts`. pub vml: Option, + /// Chart sheets, in file order (workbook owner only). + pub sheets: Vec, } impl Preserved { pub fn is_empty(&self) -> bool { - self.package_rels.is_empty() && self.rels.is_empty() && self.parts.is_empty() && self.elements.is_empty() && self.vml.is_none() + self.package_rels.is_empty() + && self.rels.is_empty() + && self.parts.is_empty() + && self.elements.is_empty() + && self.vml.is_none() + && self.sheets.is_empty() } /// Total bytes held. pub fn byte_len(&self) -> usize { diff --git a/crates/xlsx/src/passthrough.rs b/crates/xlsx/src/passthrough.rs index 7db21fd..edce309 100644 --- a/crates/xlsx/src/passthrough.rs +++ b/crates/xlsx/src/passthrough.rs @@ -27,7 +27,7 @@ use std::collections::{HashMap, HashSet, VecDeque}; use std::fmt::Write as _; use std::sync::Arc; -use gridcraft_model::{Preserved, PreservedPart, PreservedRel, PreservedXml, Sheet, Workbook}; +use gridcraft_model::{Preserved, PreservedPart, PreservedRel, PreservedSheet, PreservedXml, Sheet, Visibility, Workbook}; use quick_xml::events::attributes::Attribute; use quick_xml::events::{BytesStart, Event}; use quick_xml::name::QName; @@ -137,6 +137,10 @@ const REWRITTEN: &[&str] = &[ "powerpivotdata", ]; +/// Relationship kinds a chart sheet reaches that are otherwise rewritten: its drawing, chart and +/// their style, colour and shape parts are kept byte for byte (the chart sheet is not modelled). +const CHART_SHEET_GRAPH: &[&str] = &["chartsheet", "drawing", "chart", "chartex", "chartstyle", "chartcolorstyle", "chartusershapes"]; + /// Part-name prefixes (lower case) GridCraft's writer never produces: kept parts there keep their /// original names. Anything else is renamed so it can't collide with written parts. const OWN_DIRS: &[&str] = &[ @@ -201,6 +205,21 @@ fn raw_rels(cx: &mut Ctx<'_>, part: &str) -> Result, String> { // ---------------------------------------------------------------- reading +/// The `` entries of the workbook that are chart sheets, each with the number of worksheets +/// (model sheets) before it. +fn chart_sheet_entries<'a>(wb_xml: &'a xml::El, wb_rels: &[PreservedRel], file_to_model: &[Option]) -> Vec<(usize, &'a xml::El)> { + let mut out = vec![]; + let mut worksheets = 0usize; + for (i, se) in wb_xml.child("sheets").into_iter().flat_map(|s| s.kids("sheet")).enumerate() { + if file_to_model.get(i).copied().flatten().is_some() { + worksheets += 1; + } else if se.attr("id").and_then(|id| wb_rels.iter().find(|r| r.id == id)).is_some_and(|r| is_kind(r, &["chartsheet"])) { + out.push((worksheets, se)); + } + } + out +} + /// Collects kept content while a package is read. pub struct KeptReader { on: bool, @@ -257,11 +276,12 @@ impl KeptReader { /// The parts reachable from `root` that aren't in `seen` yet. Fails (and keeps nothing) when the /// graph reaches rewritten content, a missing or damaged part, or a size limit. - fn closure(&mut self, cx: &mut Ctx<'_>, root: &PreservedRel, seen: &HashSet) -> Result, String> { + fn closure(&mut self, cx: &mut Ctx<'_>, root: &PreservedRel, seen: &HashSet, through: &[&str]) -> Result, String> { if root.external { return Ok(vec![]); } - if rewritten(root) { + let blocked = |r: &PreservedRel| rewritten(r) && !through.iter().any(|k| k.eq_ignore_ascii_case(kind(&r.rel_type))); + if blocked(root) { return Err(format!("it refers to {}", kind(&root.rel_type))); } let mut parts = vec![]; @@ -291,7 +311,7 @@ impl KeptReader { } let rels = raw_rels(cx, &name)?; for r in rels.iter().filter(|r| !r.external) { - if rewritten(r) { + if blocked(r) { return Err(format!("it refers to {}", kind(&r.rel_type))); } queue.push_back((r.target.clone(), depth + 1)); @@ -305,10 +325,22 @@ impl KeptReader { /// Keeps a set of relationships (all or nothing) and the parts behind them. fn keep(&mut self, cx: &mut Ctx<'_>, p: &mut Preserved, roots: &[PreservedRel], seen: &mut HashSet) -> Result<(), String> { + self.keep_through(cx, p, roots, seen, &[]) + } + + /// [`Self::keep`] for roots whose graph may reach the rewritten kinds in `through`. + fn keep_through( + &mut self, + cx: &mut Ctx<'_>, + p: &mut Preserved, + roots: &[PreservedRel], + seen: &mut HashSet, + through: &[&str], + ) -> Result<(), String> { let mut new_parts: Vec = vec![]; let mut scratch = seen.clone(); for r in roots { - let parts = self.closure(cx, r, &scratch)?; + let parts = self.closure(cx, r, &scratch, through)?; scratch.extend(parts.iter().map(|q| normalize(&q.name))); new_parts.extend(parts); } @@ -323,8 +355,12 @@ impl KeptReader { } /// Package-root and workbook-level content. - pub fn workbook(&mut self, cx: &mut Ctx<'_>, wb_part: &str, wb_xml: &xml::El) -> Option> { + pub fn workbook(&mut self, cx: &mut Ctx<'_>, wb_part: &str, wb_xml: &xml::El, file_to_model: &[Option]) -> Option> { if !self.on { + let wb_rels = raw_rels(cx, wb_part).unwrap_or_default(); + for (_, se) in chart_sheet_entries(wb_xml, &wb_rels, file_to_model) { + cx.warn(format!("chart sheet \"{}\" was skipped and is not kept on save", se.attr("name").unwrap_or(""))); + } if self.strict { cx.warn("unmodelled content of a Strict Open XML workbook (custom XML, links, controls…) is not kept on save"); } @@ -351,6 +387,25 @@ impl KeptReader { cx.warn(format!("{} part {} was dropped: {why}", kind(&r.rel_type), r.target)); } } + // Chart sheets: the sheet entry (name, state, position) plus the chart sheet part and the + // drawing, chart, style and colour parts behind it. + for (worksheets, se) in chart_sheet_entries(wb_xml, &wb_rels, file_to_model) { + let name = se.attr("name").unwrap_or("").to_string(); + let Some(r) = se.attr("id").and_then(|id| wb_rels.iter().find(|r| r.id == id)) else { continue }; + match self.keep_through(cx, &mut p, std::slice::from_ref(r), &mut seen, CHART_SHEET_GRAPH) { + Ok(()) => p.sheets.push(PreservedSheet { + name, + visibility: match se.attr("state") { + Some("hidden") => Visibility::Hidden, + Some("veryHidden") => Visibility::VeryHidden, + _ => Visibility::Visible, + }, + rel_id: r.id.clone(), + before: worksheets, + }), + Err(why) => cx.warn(format!("chart sheet \"{name}\" was dropped: {why}")), + } + } // Links to external workbooks: `[1]` in formulas is the position in , // so they are kept all together or not at all. let ids: Vec<&str> = @@ -869,18 +924,165 @@ fn remap(x: &str, ids: &HashMap) -> Option { map_rel_ids(x, &mut |id| ids.get(id).cloned()) } -/// Writes the workbook's kept parts and relationships; returns the workbook XML elements that -/// go right after `` (``). -pub fn write_workbook(wb: &Workbook, wb_rels: &mut Rels, out: &mut Out) -> String { - let Some(p) = wb.preserved.as_deref() else { return String::new() }; - let names = write_parts(out, &p.parts); - let ids = attach(wb_rels, &p.rels, &names); +/// A kept chart sheet being written: its `` entry's fields and its new relationship id. +pub struct KeptSheet { + before: usize, + name: String, + visibility: Visibility, + rid: String, +} + +/// The workbook's kept content as written. +#[derive(Default)] +pub struct WorkbookKept { + /// Workbook XML elements that go right after `` (``). + pub elements: String, + sheets: Vec, +} + +impl WorkbookKept { + /// Where the model's sheet `i` is in the file's sheet list: chart sheets come between worksheets. + pub fn file_index(&self, i: usize) -> usize { + i + self.sheets.iter().filter(|s| s.before <= i).count() + } + + /// The `` entries of the chart sheets that go right before the model's sheet `i` + /// (`i` = the number of sheets for those that come last); `id` numbers them from `first_id`. + pub fn sheet_entries(&self, i: usize, last: bool, mut id: usize) -> (String, usize) { + let mut x = String::new(); + for s in self.sheets.iter().filter(|s| if last { s.before >= i } else { s.before == i }) { + let state = match s.visibility { + Visibility::Visible => "", + Visibility::Hidden => " state=\"hidden\"", + Visibility::VeryHidden => " state=\"veryHidden\"", + }; + let name: String = s.name.chars().take(31).collect(); + let _ = write!(x, "", esc_attr(&name), esc_attr(&s.rid)); + id += 1; + } + (x, id) + } +} + +/// The part names reachable from `root` through kept parts' relationships (normalized). +fn reachable(p: &Preserved, root: &PreservedRel) -> HashSet { + let by_name: HashMap = p.parts.iter().map(|q| (normalize(&q.name), q)).collect(); + let mut seen = HashSet::new(); + let mut queue = VecDeque::from([normalize(&root.target)]); + while let Some(n) = queue.pop_front() { + if !seen.insert(n.clone()) { + continue; + } + if let Some(q) = by_name.get(&n) { + queue.extend(q.rels.iter().filter(|r| !r.external).map(|r| normalize(&r.target))); + } + } + seen +} + +/// Every formula (``) of a chart part, depth first. +fn chart_formulas(el: &xml::El, out: &mut Vec) { + if el.name == "f" && !el.text.trim().is_empty() { + out.push(el.text.trim().to_string()); + } + for c in &el.children { + chart_formulas(c, out); + } +} + +/// The sheet names a formula such as `'My Sheet'!$A$1:$A$9,Data!$B$1` refers to. References to +/// other workbooks (`[1]Sheet!A1`) are not sheets of this one and are left out. +pub(crate) fn sheet_names_in(formula: &str) -> Vec { + let mut names = vec![]; + let chars: Vec = formula.chars().collect(); + let at = |i: usize| chars.get(i).copied(); + let mut i = 0; + while let Some(c) = at(i) { + let mut name = String::new(); + let start = i; + if c == '\'' { + i += 1; + while let Some(c) = at(i) { + if c == '\'' { + if at(i + 1) == Some('\'') { + name.push('\''); + i += 2; + continue; + } + break; + } + name.push(c); + i += 1; + } + i += 1; + } else { + while let Some(c) = at(i).filter(|c| !matches!(c, '!' | ',' | '(' | ')' | '\'' | ' ')) { + name.push(c); + i += 1; + } + } + if at(i) == Some('!') { + if !name.is_empty() && !name.starts_with('[') { + names.push(name); + } + i += 1; + } else if i == start { + i += 1; + } + } + names +} + +/// Whether a kept chart sheet can be written back: its chart's series must still refer to sheets +/// that exist under the same names (the chart is kept as it was, so a series on a deleted or +/// renamed sheet would be left pointing at nothing), and no worksheet may have taken its name. +fn chart_sheet_usable(wb: &Workbook, p: &Preserved, s: &PreservedSheet) -> bool { + let same = |a: &str, b: &str| a.chars().take(31).collect::().eq_ignore_ascii_case(&b.chars().take(31).collect::()); + if wb.sheets.iter().any(|w| same(&w.name, &s.name)) { + return false; + } + let Some(root) = p.rels.iter().find(|r| r.id == s.rel_id) else { return false }; + let parts = reachable(p, root); + let mut formulas = vec![]; + for q in p.parts.iter().filter(|q| parts.contains(&normalize(&q.name)) && q.content_type.contains("chart")) { + if let Ok(x) = xml::parse(&q.data) { + chart_formulas(&x, &mut formulas); + } + } + formulas.iter().flat_map(|f| sheet_names_in(f)).all(|n| wb.sheets.iter().any(|w| same(&w.name, &n))) +} + +/// Writes the workbook's kept parts and relationships. Chart sheets that can't be written back +/// (see [`chart_sheet_usable`]) are left out with the parts only they reach. +pub fn write_workbook(wb: &Workbook, wb_rels: &mut Rels, out: &mut Out) -> WorkbookKept { + let Some(p) = wb.preserved.as_deref() else { return WorkbookKept::default() }; + let (live, dropped): (Vec<&PreservedSheet>, Vec<&PreservedSheet>) = p.sheets.iter().partition(|s| chart_sheet_usable(wb, p, s)); + let root_of = |s: &PreservedSheet| p.rels.iter().find(|r| r.id == s.rel_id); + let dropped_ids: HashSet<&str> = dropped.iter().map(|s| s.rel_id.as_str()).collect(); + // Parts reached only by dropped chart sheets go with them. + let mut gone: HashSet = dropped.iter().filter_map(|s| root_of(s)).flat_map(|r| reachable(p, r)).collect(); + if !gone.is_empty() { + let others = p.rels.iter().filter(|r| !dropped_ids.contains(r.id.as_str())).flat_map(|r| reachable(p, r)); + for n in others { + gone.remove(&n); + } + } + let parts: Vec = p.parts.iter().filter(|q| !gone.contains(&normalize(&q.name))).cloned().collect(); + let rels: Vec = p.rels.iter().filter(|r| !dropped_ids.contains(r.id.as_str())).cloned().collect(); + let names = write_parts(out, &parts); + let ids = attach(wb_rels, &rels, &names); for r in &p.package_rels { if let Some(target) = rel_target(r, &names) { out.kept.root.push((r.rel_type.clone(), target, r.external)); } } - p.elements.iter().filter(|e| e.slot == "externalReferences").filter_map(|e| remap(&e.xml, &ids)).collect() + WorkbookKept { + elements: p.elements.iter().filter(|e| e.slot == "externalReferences").filter_map(|e| remap(&e.xml, &ids)).collect(), + sheets: live + .iter() + .filter_map(|s| Some(KeptSheet { before: s.before, name: s.name.clone(), visibility: s.visibility, rid: ids.get(&s.rel_id)?.clone() })) + .collect(), + } } /// The workbook's kept elements of `slot` that refer to no part (`fileVersion`, `ext`). diff --git a/crates/xlsx/src/read.rs b/crates/xlsx/src/read.rs index ed7996a..1a47908 100644 --- a/crates/xlsx/src/read.rs +++ b/crates/xlsx/src/read.rs @@ -208,6 +208,11 @@ pub fn read_xlsx(bytes: &[u8]) -> Result<(Workbook, ReadReport), IoError> { file_to_model.push(None); continue; }; + if rel.kind == "chartsheet" { + // Kept as it is and written back in place (see `passthrough`); nothing to model. + file_to_model.push(None); + continue; + } if rel.kind != "worksheet" { cx.warn(format!("sheet \"{name}\" is a {} and was skipped", rel.kind)); file_to_model.push(None); @@ -334,7 +339,7 @@ pub fn read_xlsx(bytes: &[u8]) -> Result<(Workbook, ReadReport), IoError> { Err(e) => cx.warn(format!("macros (VBA project) could not be read and were dropped: {e}")), } } - wb.preserved = kept.workbook(&mut cx, &wb_part, &wb_xml); + wb.preserved = kept.workbook(&mut cx, &wb_part, &wb_xml, &file_to_model); // Calls of LAMBDA names are spelled like the names (the parser upper-cases them). wb.apply_name_call_case(); wb.styles = cx.styles; diff --git a/crates/xlsx/src/tests/passthrough_tests.rs b/crates/xlsx/src/tests/passthrough_tests.rs index aac9b6b..59e2e8d 100644 --- a/crates/xlsx/src/tests/passthrough_tests.rs +++ b/crates/xlsx/src/tests/passthrough_tests.rs @@ -484,6 +484,184 @@ fn kept_vml_drawings_are_charged_to_the_kept_total() { assert!(wb.sheets.iter().all(|s| s.preserved.as_ref().is_some_and(|p| p.vml.is_some()))); } +// ------------------------------------------------------------------ chart sheets (#727) + +const NS_C: &str = "http://schemas.openxmlformats.org/drawingml/2006/chart"; +const CT_PREFIX: &str = "application/vnd.openxmlformats-officedocument"; + +fn chart_xml(formula: &str) -> String { + format!( + r#"{formula}"# + ) +} + +fn chartsheet_xml(note: &str) -> String { + format!( + r#""# + ) +} + +fn drawing_xml(note: &str) -> String { + format!( + r#""# + ) +} + +fn worksheet_xml(value: &str) -> String { + format!( + r#"{value}"# + ) +} + +/// Sheets in file order: Data, Chart1 (a chart sheet on Data), More, Spare, Last (a hidden chart +/// sheet on More, last in the list). "More" has a print area; the active tab is "More". +fn chartsheet_package() -> Vec { + let workbook = format!( + r#"More!$A$1:$B$2"# + ); + let content_types = format!( + r#""# + ); + let (cs1, cs2) = (chartsheet_xml("first"), chartsheet_xml("last")); + let (d1, d2) = (drawing_xml("first"), drawing_xml("last")); + let (c1, c2) = (chart_xml("Data!$A$1:$A$3"), chart_xml("'More'!$A$1")); + let (s1, s2, s3) = (worksheet_xml("data"), worksheet_xml("more"), worksheet_xml("spare")); + make_zip(&[ + ("[Content_Types].xml", content_types.as_bytes()), + ("_rels/.rels", rels(&[("rId1", "officeDocument", "xl/workbook.xml")]).as_bytes()), + ("xl/workbook.xml", workbook.as_bytes()), + ( + "xl/_rels/workbook.xml.rels", + rels(&[ + ("rId1", "worksheet", "worksheets/sheet1.xml"), + ("rId2", "chartsheet", "chartsheets/sheet1.xml"), + ("rId3", "worksheet", "worksheets/sheet2.xml"), + ("rId4", "worksheet", "worksheets/sheet3.xml"), + ("rId5", "chartsheet", "chartsheets/sheet2.xml"), + ]) + .as_bytes(), + ), + ("xl/worksheets/sheet1.xml", s1.as_bytes()), + ("xl/worksheets/sheet2.xml", s2.as_bytes()), + ("xl/worksheets/sheet3.xml", s3.as_bytes()), + ("xl/chartsheets/sheet1.xml", cs1.as_bytes()), + ("xl/chartsheets/sheet2.xml", cs2.as_bytes()), + ( + "xl/chartsheets/_rels/sheet1.xml.rels", + rels(&[("rId1", "drawing", "../drawings/drawing1.xml"), ("rId2", "printerSettings", "../printerSettings/printerSettings1.bin")]) + .as_bytes(), + ), + ("xl/chartsheets/_rels/sheet2.xml.rels", rels(&[("rId1", "drawing", "../drawings/drawing2.xml")]).as_bytes()), + ("xl/printerSettings/printerSettings1.bin", PRINTER), + ("xl/drawings/drawing1.xml", d1.as_bytes()), + ("xl/drawings/drawing2.xml", d2.as_bytes()), + ("xl/drawings/_rels/drawing1.xml.rels", rels(&[("rId1", "chart", "../charts/chart1.xml")]).as_bytes()), + ("xl/drawings/_rels/drawing2.xml.rels", rels(&[("rId1", "chart", "../charts/chart2.xml")]).as_bytes()), + ("xl/charts/chart1.xml", c1.as_bytes()), + ("xl/charts/chart2.xml", c2.as_bytes()), + ]) +} + +/// The sheets of a written workbook: (name, state) in file order. +fn written_sheets(u: &Unpacked) -> Vec<(String, String)> { + let wb = xml::parse(u.parts.get("xl/workbook.xml").unwrap()).unwrap(); + wb.child("sheets").unwrap().kids("sheet").map(|s| (s.attr("name").unwrap().to_string(), s.attr("state").unwrap_or("").to_string())).collect() +} + +/// The part a written chart sheet is in, found through the workbook relationship; its drawing and +/// that drawing's chart, followed through relationships. +fn chart_sheet_chain(u: &Unpacked, name: &str) -> (String, String, String) { + let wb = xml::parse(u.parts.get("xl/workbook.xml").unwrap()).unwrap(); + let id = wb.child("sheets").unwrap().kids("sheet").find(|s| s.attr("name") == Some(name)).unwrap().attr("id").unwrap().to_string(); + let (ty, part) = u.target("xl/workbook.xml", &id); + assert!(ty.ends_with("/chartsheet"), "{ty}"); + let drawing = u.rels(&part).into_iter().find(|r| r.1.ends_with("/drawing")).unwrap().2; + let chart = u.rels(&drawing).into_iter().find(|r| r.1.ends_with("/chart")).unwrap().2; + (part, drawing, chart) +} + +#[test] +fn chart_sheets_survive_a_round_trip_in_place() { + let (wb, rep) = read_xlsx(&chartsheet_package()).unwrap(); + assert!(!rep.warnings.iter().any(|w| w.contains("skipped") || w.contains("dropped")), "{:?}", rep.warnings); + let names: Vec<&str> = wb.sheets.iter().map(|s| s.name.as_str()).collect(); + assert_eq!(names, ["Data", "More", "Spare"], "chart sheets are not worksheets"); + + let out = write_xlsx(&wb).unwrap(); + let u = Unpacked::new(&out); + u.assert_valid(); + // Same order, same visibility, the way Excel shows the tabs. + assert_eq!( + written_sheets(&u), + [("Data", ""), ("Chart1", ""), ("More", ""), ("Spare", ""), ("Last", "hidden")].map(|(a, b)| (a.to_string(), b.to_string())), + ); + // Sheet ids are unique. + let wbx = u.text("xl/workbook.xml"); + let mut ids: Vec = wbx.match_indices("`. + let (cs_part, _, _) = chart_sheet_chain(&u, "Chart1"); + let ps = u.target(&cs_part, &attr_of(&u.text(&cs_part), "pageSetup", "r:id")); + assert!(ps.0.ends_with("/printerSettings")); + assert_eq!(u.parts.get(&ps.1).unwrap(), PRINTER); + + // Reading the output again finds the same two chart sheets, and a second save is stable. + let (wb2, rep2) = read_xlsx(&out).unwrap(); + assert!(!rep2.warnings.iter().any(|w| w.contains("skipped") || w.contains("dropped")), "{:?}", rep2.warnings); + let again = Unpacked::new(&write_xlsx(&wb2).unwrap()); + again.assert_valid(); + assert_eq!(written_sheets(&again), written_sheets(&u)); +} + +#[test] +fn a_chart_sheet_whose_data_sheet_was_renamed_is_dropped_with_its_parts() { + let (mut wb, _) = read_xlsx(&chartsheet_package()).unwrap(); + wb.sheet_mut(0).unwrap().name = "Renamed".into(); + let u = Unpacked::new(&write_xlsx(&wb).unwrap()); + u.assert_valid(); + // "Chart1" plots Data, which is gone under that name; "Last" plots More, which is not. + assert_eq!(written_sheets(&u).iter().map(|s| s.0.as_str()).collect::>(), ["Renamed", "More", "Spare", "Last"]); + assert!(u.find(chart_xml("Data!$A$1:$A$3").as_bytes()).is_empty()); + assert!(u.find(drawing_xml("first").as_bytes()).is_empty()); + assert!(u.find(PRINTER).is_empty()); + chart_sheet_chain(&u, "Last"); + let wbx = u.text("xl/workbook.xml"); + assert_eq!(attr_of(&wbx, "workbookView", "activeTab"), "1"); + assert_eq!(attr_of(&wbx, "definedName", "localSheetId"), "1"); +} + +#[test] +fn a_chart_sheet_whose_name_a_worksheet_took_is_dropped() { + let (mut wb, _) = read_xlsx(&chartsheet_package()).unwrap(); + wb.sheet_mut(2).unwrap().name = "LAST".into(); + let u = Unpacked::new(&write_xlsx(&wb).unwrap()); + u.assert_valid(); + assert_eq!(written_sheets(&u).iter().map(|s| s.0.as_str()).collect::>(), ["Data", "Chart1", "More", "LAST"]); + assert!(u.find(chart_xml("'More'!$A$1").as_bytes()).is_empty()); + chart_sheet_chain(&u, "Chart1"); +} + +#[test] +fn formulas_name_the_sheets_a_chart_plots() { + use crate::passthrough::sheet_names_in; + assert_eq!(sheet_names_in("Data!$A$1:$A$3"), ["Data"]); + assert_eq!(sheet_names_in("'My ''Sheet'!$A$1,Data!$B$1"), ["My 'Sheet", "Data"]); + assert_eq!(sheet_names_in("SUM(Data!A1:A3,'Other sheet'!B2)"), ["Data", "Other sheet"]); + assert!(sheet_names_in("[1]Ext!A1").is_empty() && sheet_names_in("[1]Ext!A1:A3").is_empty()); + assert!(sheet_names_in("Name").is_empty() && sheet_names_in("").is_empty()); #[test] fn file_version_and_calc_features_survive_a_save() { // Excel trusts the saved cell values only when workbook.xml carries both and the diff --git a/crates/xlsx/src/write.rs b/crates/xlsx/src/write.rs index dddba98..06761f0 100644 --- a/crates/xlsx/src/write.rs +++ b/crates/xlsx/src/write.rs @@ -224,9 +224,15 @@ pub fn write_xlsx(wb: &Workbook, kind: XlsxKind) -> Result, IoError> { }; let mut wb_rels = Rels::default(); + // Kept content first: the chart sheets among the worksheets need their relationship ids. + let kept = crate::passthrough::write_workbook(wb, &mut wb_rels, &mut out); let mut pivots = crate::pivot::PivotWriter::default(); let mut sheets_xml = String::new(); + let mut sheet_id = 1usize; for (i, sheet) in wb.sheets.iter().enumerate() { + let (before, next) = kept.sheet_entries(i, false, sheet_id); + sheets_xml.push_str(&before); + sheet_id = next; let (xml, mut rels) = crate::sheet_write::write_sheet(wb, i, i == active, &mut out); pivots.add_sheet(wb, i, &mut out, &mut rels); let name = format!("xl/worksheets/sheet{}.xml", i + 1); @@ -242,8 +248,10 @@ pub fn write_xlsx(wb: &Workbook, kind: XlsxKind) -> Result, IoError> { Visibility::VeryHidden => " state=\"veryHidden\"", }; let name: String = sheet.name.chars().take(31).collect(); - let _ = write!(sheets_xml, "", esc_attr(&name), i + 1); + let _ = write!(sheets_xml, "", esc_attr(&name)); + sheet_id += 1; } + sheets_xml.push_str(&kept.sheet_entries(wb.sheets.len(), true, sheet_id).0); // Defined names, then print areas and titles. let mut names = String::new(); @@ -259,7 +267,7 @@ pub fn write_xlsx(wb: &Workbook, kind: XlsxKind) -> Result, IoError> { } let _ = write!(names, "