From 07282d95e0f9d5f24a2a6aebae51f76a093b6084 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sun, 1 Nov 2026 08:38:15 -0800 Subject: [PATCH 1/2] Variables: a graph variable's data set applies its CSV as graph.setData does Choosing a data set gave a graph variable's CSV to the graph without the two steps graph.setData takes after new data: rows with neither a label nor a value were kept and drawn as empty slots, and a right-axis series the new data no longer had stayed assigned, so a series added by a later row started on the right axis. The data set now goes through the same steps. --- crates/engine/src/cmd/graph/data.rs | 20 +++++++++++--------- crates/engine/src/tests/variables.rs | 28 ++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 9 deletions(-) diff --git a/crates/engine/src/cmd/graph/data.rs b/crates/engine/src/cmd/graph/data.rs index e81f24feb..362f988f0 100644 --- a/crates/engine/src/cmd/graph/data.rs +++ b/crates/engine/src/cmd/graph/data.rs @@ -86,7 +86,8 @@ pub(super) fn to_csv(g: &GraphSpec) -> String { g.to_csv() } -/// Apply CSV graph data to the graph root `id` and rebuild its art (Variables data merge). +/// Apply CSV graph data to the graph root `id` and rebuild its art (Variables data merge), as `graph.setData` with +/// `csv` does. pub(crate) fn apply_csv(d: &mut Document, id: NodeId, csv: &str) -> Result<()> { let Some(node) = d.node(id) else { return Err(EngineError::NoNode(id)); @@ -94,13 +95,17 @@ pub(crate) fn apply_csv(d: &mut Document, id: NodeId, csv: &str) -> Result<()> { let Some(mut spec) = node.extras().graph.as_deref().cloned() else { return Err(bad("graph.applyCsv", format!("node {} is not a graph", id.0))); }; - let (series, categories, rows) = parse_csv(csv); - spec.series = series; - spec.categories = categories; - spec.set_cells(rows); + apply_data(&mut spec, &json!({ "csv": csv })); + keep_right_series(&mut spec); regenerate(d, id, spec) } +/// Drop right-axis series the data no longer has, so a series added later starts on the left. +fn keep_right_series(g: &mut GraphSpec) { + let count = g.rows.iter().map(Vec::len).max().unwrap_or(0); + g.right_series.retain(|i| *i < count); +} + pub(super) fn apply_data(g: &mut GraphSpec, p: &Value) -> bool { let mut changed = false; if let Some(csv) = str_param(p, "csv") { @@ -185,10 +190,7 @@ pub(super) fn set_data(s: &mut Session, p: &Value) -> Result { // Blank cells come back as null, so the rows can be edited and sent back as they are. return Ok(json!({ "csv": to_csv(&spec), "series": spec.series, "categories": spec.categories, "rows": spec.cells() })); } - // Series assigned to the right axis that the new data no longer has are dropped, so a series added later starts - // on the left. - let count = spec.rows.iter().map(Vec::len).max().unwrap_or(0); - spec.right_series.retain(|i| *i < count); + keep_right_series(&mut spec); s.edit("Graph Data", |d, _| regenerate(d, id, spec))?; Ok(json!({ "id": id.0 })) } diff --git a/crates/engine/src/tests/variables.rs b/crates/engine/src/tests/variables.rs index bdacd0e1b..fb252d854 100644 --- a/crates/engine/src/tests/variables.rs +++ b/crates/engine/src/tests/variables.rs @@ -1632,6 +1632,34 @@ fn locked_variables_refuse_a_library_that_deletes_or_defines_variables() { assert_eq!(vars.datasets.iter().map(|d| d.name.as_str()).collect::>(), ["B"]); } +/// A graph variable's data set applies its CSV as `graph.setData` does: a right-axis series the row's data no longer +/// has is dropped, and a row with neither a label nor a value is not drawn as an empty slot. +#[test] +fn graph_variable_applies_csv_as_set_data() { + let mut s = session(); + let create = json!({"type": "column", "x": 10, "y": 10, "width": 120, "height": 80, "csv": ",a,b\nQ1,10,1000\nQ2,5,500"}); + let id = s.execute("graph.create", &create).unwrap()["id"].as_u64().unwrap(); + let other = s.execute("graph.create", &create).unwrap()["id"].as_u64().unwrap(); + for g in [id, other] { + s.execute("graph.setType", &json!({"id": g, "seriesIndexes": [1], "valueAxis": "right"})).unwrap(); + } + s.execute("select.set", &json!({"ids": [id]})).unwrap(); + s.execute("variable.makeGraphDynamic", &json!({})).unwrap(); + let name = s.execute("variable.list", &json!({})).unwrap()["variables"][0]["name"].as_str().unwrap().to_string(); + let spec = |s: &Session, g: u64| s.doc().unwrap().doc.node(NodeId(g)).unwrap().extras().graph.as_deref().unwrap().clone(); + for (row, csv) in [("One", ",a\nQ1,10\nQ2,5"), ("Blank", ",a\nQ1,10\n,\nQ2,5")] { + s.execute("dataset.new", &json!({"name": row, "values": {name.clone(): csv}})).unwrap(); + s.execute("dataset.select", &json!({"name": row})).unwrap(); + s.execute("graph.setData", &json!({"id": other, "csv": csv})).unwrap(); + let (merged, set) = (spec(&s, id), spec(&s, other)); + assert_eq!(merged.right_series, set.right_series, "{row}"); + assert_eq!(merged.categories, set.categories, "{row}"); + assert_eq!(merged.cells(), set.cells(), "{row}"); + } + assert!(spec(&s, id).right_series.is_empty()); + assert_eq!(spec(&s, id).categories, ["Q1", "Q2"]); +} + #[test] fn graph_variable_applies_csv_and_captures() { let mut s = session(); From b104efc53c28d418a04dd05101d3d0ec97b5e8d1 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sun, 1 Nov 2026 10:19:45 -0800 Subject: [PATCH 2/2] Graph data: a dropped empty row adds no blank series Dropping rows with neither a label nor a value read the data padded to the widest row, so a dropped row wider than the rest (`,,`) left every kept row one blank cell wider. The graph then had an extra blank series, and a right-axis assignment to that index was kept. Each kept row now keeps its own width. This applies to graph.setData and to a graph variable's data set. --- crates/engine/src/cmd/graph/data.rs | 12 ++++++++++-- crates/engine/src/tests/variables.rs | 5 ++++- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/cmd/graph/data.rs b/crates/engine/src/cmd/graph/data.rs index 362f988f0..0e69e0240 100644 --- a/crates/engine/src/cmd/graph/data.rs +++ b/crates/engine/src/cmd/graph/data.rs @@ -136,9 +136,17 @@ pub(super) fn apply_data(g: &mut GraphSpec, p: &Value) -> bool { changed } -/// Drop rows with neither a value nor a label; a label-only row stays, its values blank. +/// Drop rows with neither a value nor a label; a label-only row stays, its values blank. Each kept row keeps its own +/// width, so a dropped row wider than the rest adds no blank series. fn drop_empty_rows(g: &mut GraphSpec) { - let mut cells = g.cells(); + let blank: std::collections::HashSet<[usize; 2]> = g.blanks.iter().copied().collect(); + let mut cells: Vec>> = g + .rows + .iter() + .take(vectorcraft_doc::MAX_GRAPH_CATEGORIES) + .enumerate() + .map(|(c, r)| r.iter().take(vectorcraft_doc::MAX_GRAPH_SERIES).enumerate().map(|(s, v)| (!blank.contains(&[c, s])).then_some(*v)).collect()) + .collect(); let keep: Vec = (0..cells.len()) .map(|c| cells.get(c).is_some_and(|r| r.iter().any(Option::is_some)) || g.categories.get(c).is_some_and(|l| !l.is_empty())) .collect(); diff --git a/crates/engine/src/tests/variables.rs b/crates/engine/src/tests/variables.rs index fb252d854..07ac4aed0 100644 --- a/crates/engine/src/tests/variables.rs +++ b/crates/engine/src/tests/variables.rs @@ -1647,7 +1647,8 @@ fn graph_variable_applies_csv_as_set_data() { s.execute("variable.makeGraphDynamic", &json!({})).unwrap(); let name = s.execute("variable.list", &json!({})).unwrap()["variables"][0]["name"].as_str().unwrap().to_string(); let spec = |s: &Session, g: u64| s.doc().unwrap().doc.node(NodeId(g)).unwrap().extras().graph.as_deref().unwrap().clone(); - for (row, csv) in [("One", ",a\nQ1,10\nQ2,5"), ("Blank", ",a\nQ1,10\n,\nQ2,5")] { + // A dropped empty row wider than the others (`,,`) adds no blank series, so the right axis keeps nothing. + for (row, csv) in [("Wide", ",a\nQ1,10\n,,\nQ2,5"), ("One", ",a\nQ1,10\nQ2,5"), ("Blank", ",a\nQ1,10\n,\nQ2,5")] { s.execute("dataset.new", &json!({"name": row, "values": {name.clone(): csv}})).unwrap(); s.execute("dataset.select", &json!({"name": row})).unwrap(); s.execute("graph.setData", &json!({"id": other, "csv": csv})).unwrap(); @@ -1655,6 +1656,8 @@ fn graph_variable_applies_csv_as_set_data() { assert_eq!(merged.right_series, set.right_series, "{row}"); assert_eq!(merged.categories, set.categories, "{row}"); assert_eq!(merged.cells(), set.cells(), "{row}"); + assert!(merged.right_series.is_empty(), "{row}"); + assert_eq!(merged.cells(), [[Some(10.0)], [Some(5.0)]], "{row}"); } assert!(spec(&s, id).right_series.is_empty()); assert_eq!(spec(&s, id).categories, ["Q1", "Q2"]);