From 925bf22e242c1eea0c0d6dfb5f9e10179c654221 Mon Sep 17 00:00:00 2001 From: nihilist-narwhal <240798405+nihilist-narwhal@users.noreply.github.com> Date: Fri, 9 Oct 2026 23:31:18 -0500 Subject: [PATCH] Charts: combo and stock charts keep their kind in XLSX A combo chart whose series have no kind of their own (as insert.chart makes them) was written as one column group, and a stock chart as clustered columns, so both reopened as clustered columns. Combo series are now written as drawn (the first as columns, the others as lines), and stock charts as a stockChart group (as lines when they don't have the 3 or 4 series a stock chart needs). --- crates/engine/src/tests.rs | 41 ++++++++++++++++++++++++++++++++++++++ crates/xlsx/src/chart.rs | 23 ++++++++++++++------- 2 files changed, 57 insertions(+), 7 deletions(-) diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index f3e36cdf4..538d27167 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -167,6 +167,47 @@ fn every_command_survives_empty_params() { } } +fn reopen(s: &mut Session) { + let r = s.execute("file.saveBytes", json!({"format": "xlsx"})).unwrap(); + let b64 = r["base64"].as_str().unwrap().to_string(); + s.execute("file.open", json!({"name": "x.xlsx", "base64": b64})).unwrap(); +} + +fn chart_data(s: &mut Session) { + s.execute( + "range.setValues", + json!({"range": "A1", "values": [["", "Open", "High", "Low", "Close"], ["Mon", 10, 14, 9, 12], ["Tue", 12, 15, 11, 14], ["Wed", 14, 16, 10, 11], ["Thu", 11, 13, 8, 9], ["Fri", 9, 12, 7, 11], ["Sat", 11, 15, 10, 14]]}), + ) + .unwrap(); +} + +#[test] +fn every_chart_kind_survives_xlsx() { + use gridcraft_model::ChartKind; + let kinds = [ + ("combo", ChartKind::Combo), + ("stock", ChartKind::Stock), + ("column", ChartKind::ColumnClustered), + ("line", ChartKind::Line), + ("scatter", ChartKind::Scatter), + ]; + for (t, kind) in kinds { + let mut s = s(); + chart_data(&mut s); + s.execute("insert.chart", json!({"type": t, "range": "A1:E7", "title": "Week"})).unwrap(); + let before = s.doc().unwrap().wb.active().unwrap().charts[0].clone(); + assert_eq!(before.kind, kind); + reopen(&mut s); + let after = &s.doc().unwrap().wb.active().unwrap().charts[0]; + assert_eq!(after.kind, kind, "{t}"); + assert_eq!(after.title.as_deref(), Some("Week"), "{t}"); + assert_eq!(after.series.len(), before.series.len(), "{t}"); + for (a, b) in after.series.iter().zip(&before.series) { + assert_eq!((&a.name, &a.categories, &a.values), (&b.name, &b.categories, &b.values), "{t}"); + } + } +} + #[test] fn parity_counts() { let (done, total) = crate::catalog::parity(); diff --git a/crates/xlsx/src/chart.rs b/crates/xlsx/src/chart.rs index eac9ef2dd..f325a6c26 100644 --- a/crates/xlsx/src/chart.rs +++ b/crates/xlsx/src/chart.rs @@ -198,6 +198,7 @@ enum G { Scatter { lines: bool }, Bubble, Radar, + Stock, } fn group_for(k: ChartKind) -> G { @@ -218,7 +219,8 @@ fn group_for(k: ChartKind) -> G { ChartKind::ScatterLines => G::Scatter { lines: true }, ChartKind::Bubble => G::Bubble, ChartKind::Radar => G::Radar, - // Kinds without a classic chart element are written as clustered columns. + ChartKind::Stock => G::Stock, + // Combo charts start from clustered columns; chartex kinds have their own part. _ => G::Bar { bar: false, grouping: "clustered" }, } } @@ -226,7 +228,7 @@ fn group_for(k: ChartKind) -> G { const MAX_CACHE: u64 = 100_000; /// Values of a same-workbook reference (`Sheet1!$B$2:$B$9`), row-major. -fn ref_values(wb: &Workbook, sheet_idx: usize, f: &str) -> Option> { +pub(crate) fn ref_values(wb: &Workbook, sheet_idx: usize, f: &str) -> Option> { let e = gridcraft_formula::parse(f).ok()?; let Expr::Ref(r) = e else { return None }; let si = match &r.sheet { @@ -330,12 +332,12 @@ fn series_xml(s: &Series, idx: usize, g: G, wb: &Workbook, si: usize) -> String (Some(c), false) => { let _ = write!(x, "{c}"); } - (None, _) if g == (G::Scatter { lines: false }) => x.push_str(""), + (None, _) if g == (G::Scatter { lines: false }) || g == G::Stock => x.push_str(""), _ => {} } match g { G::Bar { .. } => x.push_str(""), - G::Line { markers: false, .. } | G::Scatter { lines: true } => x.push_str(""), + G::Line { markers: false, .. } | G::Scatter { lines: true } | G::Stock => x.push_str(""), G::Bubble => x.push_str(""), _ => {} } @@ -402,6 +404,7 @@ fn group_xml(g: G, series: &[(usize, &Series)], ch: &Chart, axes: (u32, u32), wb format!("{sers}{dl}{ax}") } G::Radar => format!("{sers}{dl}{ax}"), + G::Stock => format!("{sers}{dl}{ax}"), } } @@ -474,9 +477,11 @@ pub fn write_chart(wb: &Workbook, si: usize, ch: &Chart) -> String { let primary_g; let mut secondary: Option = None; if ch.kind == ChartKind::Combo { + // Series without a kind of their own: the first is a column, the others lines (as drawn). + let kind = |i: usize, x: &Series| x.kind.filter(|k| *k != ChartKind::Combo).or((i > 0).then_some(ChartKind::Line)); let is_line = |k: Option| matches!(k.map(group_for), Some(G::Line { .. } | G::Area { .. } | G::Radar | G::Scatter { .. })); - let bars: Vec<(usize, &Series)> = all.iter().filter(|(_, x)| !x.secondary && !is_line(x.kind)).copied().collect(); - let lines: Vec<(usize, &Series)> = all.iter().filter(|(_, x)| !x.secondary && is_line(x.kind)).copied().collect(); + let bars: Vec<(usize, &Series)> = all.iter().filter(|(i, x)| !x.secondary && !is_line(kind(*i, x))).copied().collect(); + let lines: Vec<(usize, &Series)> = all.iter().filter(|(i, x)| !x.secondary && is_line(kind(*i, x))).copied().collect(); let sec: Vec<(usize, &Series)> = all.iter().filter(|(_, x)| x.secondary).copied().collect(); primary_g = G::Bar { bar: false, grouping: "clustered" }; if !bars.is_empty() || (lines.is_empty() && sec.is_empty()) { @@ -494,7 +499,11 @@ pub fn write_chart(wb: &Workbook, si: usize, ch: &Chart) -> String { secondary = Some(g); } } else { - primary_g = group_for(ch.kind); + primary_g = match group_for(ch.kind) { + // A stock chart has 3 or 4 series (high-low-close, open-high-low-close). + G::Stock if !(3..=4).contains(&ch.series.len()) => G::Line { stacked: false, markers: false }, + g => g, + }; s.push_str(&group_xml(primary_g, &all, ch, (1, 2), wb, si)); } match primary_g {