From 8ee59d04d396480e121cf420a20a7f96d75c0b84 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 4 Nov 2026 01:09:40 +0000 Subject: [PATCH] address review: cache the chart-sheet usability verdict per relationship id instead of every reference --- crates/xlsx/src/passthrough/write.rs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/crates/xlsx/src/passthrough/write.rs b/crates/xlsx/src/passthrough/write.rs index 5b294a500..f582d7c21 100644 --- a/crates/xlsx/src/passthrough/write.rs +++ b/crates/xlsx/src/passthrough/write.rs @@ -255,15 +255,16 @@ pub(crate) fn sheet_names_in(formula: &str) -> Vec { /// 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. /// -/// `refs` caches, by relationship id, the sheet names a chart sheet's graph refers to (`None` -/// when the relationship is missing): entries sharing one graph are analysed once. -fn chart_sheet_usable<'p>(wb: &Workbook, p: &'p Preserved, s: &'p PreservedSheet, refs: &mut HashMap<&'p str, Option>>) -> bool { +/// `usable` caches, by relationship id, whether that chart sheet's graph refers only to sheets +/// that exist (`false` when the relationship is missing): entries sharing one graph are analysed +/// once, and only the verdict is kept, not the references it came from. +fn chart_sheet_usable<'p>(wb: &Workbook, p: &'p Preserved, s: &'p PreservedSheet, usable: &mut HashMap<&'p str, bool>) -> 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 names = refs.entry(s.rel_id.as_str()).or_insert_with(|| { - let root = p.rels.iter().find(|r| r.id == s.rel_id)?; + *usable.entry(s.rel_id.as_str()).or_insert_with(|| { + 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")) { @@ -271,10 +272,8 @@ fn chart_sheet_usable<'p>(wb: &Workbook, p: &'p Preserved, s: &'p PreservedSheet chart_formulas(&x, &mut formulas); } } - Some(formulas.iter().flat_map(|f| sheet_names_in(f)).collect()) - }); - let Some(names) = names else { return false }; - names.iter().all(|n| wb.sheets.iter().any(|w| same(&w.name, n))) + 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