diff --git a/crates/engine/src/cmd/graph.rs b/crates/engine/src/cmd/graph.rs index f4b6c90c1..2c4249416 100644 --- a/crates/engine/src/cmd/graph.rs +++ b/crates/engine/src/cmd/graph.rs @@ -17,7 +17,9 @@ use vectorcraft_doc::{ use vectorcraft_geom::{Affine, BezPath, FillRule, PathData, Point, Rect, shapes}; use super::typecmd::refresh_bounds; -use super::{CommandSpec, EngineError, Result, Session, bad, bool_or, cmd, f64_or, f64_req, has_doc, has_selection, named_id, ok, str_param}; +use super::{ + CommandSpec, EngineError, Result, Session, bad, cmd, f64_req, has_doc, has_selection, named_id, number_or, ok, str_param, switch_or, text_param, +}; mod axis; mod columns; diff --git a/crates/engine/src/cmd/graph/axis.rs b/crates/engine/src/cmd/graph/axis.rs index 47858d395..b805efb43 100644 --- a/crates/engine/src/cmd/graph/axis.rs +++ b/crates/engine/src/cmd/graph/axis.rs @@ -1,4 +1,4 @@ -use super::{GraphSpec, Value}; +use super::{GraphSpec, Result, Value, bad}; pub(super) const LABEL_SIZE: f64 = 9.0; @@ -22,6 +22,16 @@ pub(super) fn count_param(p: &Value, key: &str, max: u64) -> Option { usize::try_from(n.min(max)).ok() } +/// [`count_param`], or `None` when `p` leaves `key` out or gives null. A negative number is an error, and so is any +/// other value, such as `"5"`. +pub(super) fn count_given(p: &Value, key: &str, max: u64, cmd: &str) -> Result> { + match p.get(key) { + None | Some(Value::Null) => Ok(None), + Some(v) if !v.is_number() => Err(bad(cmd, format!("`{key}` must be a number, not {v}"))), + Some(_) => count_param(p, key, max).map(Some).ok_or_else(|| bad(cmd, format!("`{key}` must be 0 or more"))), + } +} + /// A "nice" axis: (min, max, step). pub(super) fn nice_axis(lo: f64, hi: f64, ticks: usize) -> (f64, f64, f64) { let (lo, hi) = if (hi - lo).abs() < 1e-12 { (lo.min(0.0), lo.max(0.0) + 1.0) } else { (lo, hi) }; diff --git a/crates/engine/src/cmd/graph/commands.rs b/crates/engine/src/cmd/graph/commands.rs index 5df961ee3..7084fe19f 100644 --- a/crates/engine/src/cmd/graph/commands.rs +++ b/crates/engine/src/cmd/graph/commands.rs @@ -12,7 +12,7 @@ pub fn specs() -> Vec { "Create Graph", [], None, - "{type: column|stackedColumn|bar|stackedBar|line|area|scatter|pie|radar, x, y, width, height, series?: [..], categories?: [..], rows?: [[number|null]], csv?} → {id}; an empty CSV cell or a null is a blank value, a number in straight quotes is a label", + "{type: column|stackedColumn|bar|stackedBar|line|area|scatter|pie|radar, x, y, width, height, series?: [..], categories?: [..], rows?: [[number|null]], csv?} → {id}; an empty CSV cell or a null is a blank value, a number in straight quotes is a label; a `type` that is not a string, a `width` or `height` that is not a number, and data of the wrong type (as `graph.setData` reads it) are refused", has_doc, create ), @@ -21,7 +21,7 @@ pub fn specs() -> Vec { "Data…", ["Object", "Graph"], None, - "{id?, series?, categories?, rows?, csv?: first row = series labels (first cell empty); then one row per category: label; values…; an empty cell or a null is a blank value; a quoted number a label; a `|` in a label a line break, transpose?: true swaps rows and columns (Transpose row/column), switchXY?: true swaps each scatter series' y and x columns (Switch x/y)} replace the graph's data; no data → the current {csv, series, categories, rows}", + "{id?, series?, categories?, rows?, csv?: first row = series labels (first cell empty); then one row per category: label; values…; an empty cell or a null is a blank value; a quoted number a label; a `|` in a label a line break, transpose?: true swaps rows and columns (Transpose row/column), switchXY?: true swaps each scatter series' y and x columns (Switch x/y)} replace the graph's data; a `csv` that is not a string, `series` or `categories` that are not lists, `rows` that are not lists of numbers and nulls, or a `transpose` or `switchXY` that is not true or false is refused and the graph stays as it was; no data → the current {csv, series, categories, rows}", has_selection, set_data ), @@ -30,7 +30,7 @@ pub fn specs() -> Vec { "Type…", ["Object", "Graph"], None, - "{id?, type?, seriesIndexes?: [index], columnWidth?: % (1–1000; past 100 columns overlap), clusterWidth?: % (1–1000), legend?: bool, markPoints?: bool, connectPoints?: bool, edgeToEdge?: bool (line graphs: true runs the lines across the whole plot, false puts the points at the centres of their categories), ticks?: n, axisMin?, axisMax?, valueAxis?: left|right|both (the bar graphs' value axis stays along the bottom), separateScales?: bool, rightTicks?: n, rightAxisMin?, rightAxisMax?, tickLength?: none|short|full, tickMarks?: n per division, rightTickLength?, rightTickMarks?, categoryTickLength?: none|short|full, categoryTickMarks?: n, ticksBetweenLabels?: bool, prefix?, suffix?, rightPrefix?, rightSuffix?: text around the value axis numbers, firstRowInFront?: bool, firstColumnInFront?: bool (which overlapping columns are drawn on top: the first category over the next within each series, the first series over the others), filledLines?: bool (Draw Filled Lines: connecting lines as 3 pt bands filled with the series colour), dropShadow?: bool (Add Drop Shadow: a translucent black shadow down and right of the columns, bars, lines and pie wedges), legendAcrossTop?: bool (Add Legend Across Top: the legend in rows above the plot), pieLegend?: none|standard|wedges (Legends in Wedges: the series labels inside their wedges), piePosition?: ratio|even|stacked (several pies sized by their totals, at one size (the default), or stacked), pieSort?: all|first|none (wedges largest first in each pie, in the first pie's order, or in data order (the default)); the query lists the pie options for pie graphs} change the graph type and options; with `seriesIndexes`, or (no `id`) with only series selected with Group Selection, `type` goes to those series only and `valueAxis` (left|right; both is refused) puts them on that value axis, while the other options still apply to the whole graph (Combine different graph types: column, stacked column, line and area mix, and so do bar and stacked bar; a series given the graph's type follows the graph again) (axisMin and axisMax together override the calculated value axis: exactly that range in `ticks` divisions, 5 when 0; with the value axis on both sides, separateScales gives the series on the right axis a scale of their own, set the same way with rightTicks, rightAxisMin and rightAxisMax); no options → the current ones", + "{id?, type?, seriesIndexes?: [index], columnWidth?: % (1–1000; past 100 columns overlap), clusterWidth?: % (1–1000), legend?: bool, markPoints?: bool, connectPoints?: bool, edgeToEdge?: bool (line graphs: true runs the lines across the whole plot, false puts the points at the centres of their categories), ticks?: n, axisMin?: number|null (null = calculated), axisMax?: number|null, valueAxis?: left|right|both (the bar graphs' value axis stays along the bottom), separateScales?: bool, rightTicks?: n, rightAxisMin?, rightAxisMax?, tickLength?: none|short|full, tickMarks?: n per division, rightTickLength?, rightTickMarks?, categoryTickLength?: none|short|full, categoryTickMarks?: n, ticksBetweenLabels?: bool, prefix?, suffix?, rightPrefix?, rightSuffix?: text around the value axis numbers, firstRowInFront?: bool, firstColumnInFront?: bool (which overlapping columns are drawn on top: the first category over the next within each series, the first series over the others), filledLines?: bool (Draw Filled Lines: connecting lines as 3 pt bands filled with the series colour), dropShadow?: bool (Add Drop Shadow: a translucent black shadow down and right of the columns, bars, lines and pie wedges), legendAcrossTop?: bool (Add Legend Across Top: the legend in rows above the plot), pieLegend?: none|standard|wedges (Legends in Wedges: the series labels inside their wedges), piePosition?: ratio|even|stacked (several pies sized by their totals, at one size (the default), or stacked), pieSort?: all|first|none (wedges largest first in each pie, in the first pie's order, or in data order (the default)); the query lists the pie options for pie graphs} change the graph type and options; with `seriesIndexes`, or (no `id`) with only series selected with Group Selection, `type` goes to those series only and `valueAxis` (left|right; both is refused) puts them on that value axis, while the other options still apply to the whole graph (Combine different graph types: column, stacked column, line and area mix, and so do bar and stacked bar; a series given the graph's type follows the graph again) (axisMin and axisMax together override the calculated value axis: exactly that range in `ticks` divisions, 5 when 0; with the value axis on both sides, separateScales gives the series on the right axis a scale of their own, set the same way with rightTicks, rightAxisMin and rightAxisMax); a value of the wrong type, such as \"10\" for a number or \"true\" for a bool, or a negative count, is refused and the graph stays as it was; a null is read as a key left out, except that a null axis bound goes back to the calculated value; no options → the current ones", has_selection, set_type ), diff --git a/crates/engine/src/cmd/graph/data.rs b/crates/engine/src/cmd/graph/data.rs index f4eb3ef83..f7f672325 100644 --- a/crates/engine/src/cmd/graph/data.rs +++ b/crates/engine/src/cmd/graph/data.rs @@ -1,6 +1,6 @@ use super::edit::{regenerate, spec_of, target}; use super::series::{switch_xy, transpose}; -use super::{Document, EngineError, GraphKind, GraphSpec, NodeId, Result, Session, Value, bad, bool_or, json, str_param}; +use super::{Document, EngineError, GraphKind, GraphSpec, NodeId, Result, Session, Value, bad, json, switch_or, text_param}; // ---------- data ---------- @@ -101,7 +101,7 @@ pub(crate) fn apply_csv(d: &mut Document, id: NodeId, csv: &str) -> Result<()> { return Err(bad("graph.applyCsv", format!("node {} is not a graph", id.0))); }; let mut spec = current.clone(); - apply_data(&mut spec, &json!({ "csv": csv })); + apply_data(&mut spec, &json!({ "csv": csv }), "graph.applyCsv")?; keep_right_series(&mut spec); if spec == *current { return Ok(()); @@ -115,34 +115,58 @@ fn keep_right_series(g: &mut GraphSpec) { g.right_series.retain(|i| *i < count); } -pub(super) fn apply_data(g: &mut GraphSpec, p: &Value) -> bool { +/// Apply the data `p` gives (`csv`, `series`, `categories`, `rows`) to `g`; true when it gave any. A null is read as a +/// key left out. A `csv` that is not a string, a `series` or `categories` that is not a list, and `rows` that are not +/// lists of numbers and nulls are errors, and `g` is left as it was. +pub(super) fn apply_data(g: &mut GraphSpec, p: &Value, cmd: &str) -> Result { + let list = |key: &str| -> Result>> { + match p.get(key) { + None | Some(Value::Null) => Ok(None), + Some(Value::Array(a)) => Ok(Some(a)), + Some(v) => Err(bad(cmd, format!("`{key}` must be a list, not {v}"))), + } + }; + let csv = text_param(p, "csv", cmd)?; + let strings = |a: &Vec| a.iter().map(|x| x.as_str().map(str::to_string).unwrap_or_else(|| x.to_string())).collect::>(); + let series = list("series")?.map(strings); + let categories = list("categories")?.map(strings); + // A null is a blank cell. + let rows = list("rows")? + .map(|rows| { + rows.iter() + .map(|r| { + let bad_row = || bad(cmd, format!("`rows` must be lists of numbers or nulls, not {r}")); + r.as_array() + .ok_or_else(bad_row)? + .iter() + .map(|v| if v.is_null() { Ok(None) } else { v.as_f64().map(|v| Some(v).filter(|v| v.is_finite())).ok_or_else(bad_row) }) + .collect::>>() + }) + .collect::>>() + }) + .transpose()?; let mut changed = false; - if let Some(csv) = str_param(p, "csv") { + if let Some(csv) = csv { let (s, c, r) = parse_csv(csv); g.series = s; g.categories = c; g.set_cells(r); changed = true; } - let strings = - |v: &Value| v.as_array().map(|a| a.iter().map(|x| x.as_str().map(str::to_string).unwrap_or_else(|| x.to_string())).collect::>()); - if let Some(s) = p.get("series").and_then(strings) { + if let Some(s) = series { g.series = s; changed = true; } - if let Some(c) = p.get("categories").and_then(strings) { + if let Some(c) = categories { g.categories = c; changed = true; } - if let Some(rows) = p.get("rows").and_then(Value::as_array) { - // `null` (or anything that isn't a finite number) is a blank cell. - g.set_cells( - rows.iter().map(|r| r.as_array().map(|a| a.iter().map(|v| v.as_f64().filter(|v| v.is_finite())).collect()).unwrap_or_default()).collect(), - ); + if let Some(rows) = rows { + g.set_cells(rows); changed = true; } drop_empty_rows(g); - changed + Ok(changed) } /// Drop rows with neither a value nor a label; a label-only row stays, its values blank. Each kept row keeps its own @@ -184,13 +208,13 @@ pub(super) fn set_data(s: &mut Session, p: &Value) -> Result { const C: &str = "graph.setData"; let id = target(s, p, C)?; let mut spec = spec_of(s, id)?; - let flip = bool_or(p, "transpose", false); - let swap = bool_or(p, "switchXY", false); + let flip = switch_or(p, "transpose", false, C)?; + let swap = switch_or(p, "switchXY", false, C)?; if swap && spec.kind != GraphKind::Scatter { return Err(bad(C, "Switch x/y is for scatter graphs")); } // Transpose and Switch x/y act after any new data in the same call. - let changed = apply_data(&mut spec, p); + let changed = apply_data(&mut spec, p, C)?; if flip { // The categories become series: at most as many as a graph has series. if spec.cells().len() > vectorcraft_doc::MAX_GRAPH_SERIES { diff --git a/crates/engine/src/cmd/graph/edit.rs b/crates/engine/src/cmd/graph/edit.rs index c33be78d5..c7f6d52ea 100644 --- a/crates/engine/src/cmd/graph/edit.rs +++ b/crates/engine/src/cmd/graph/edit.rs @@ -1,12 +1,22 @@ -use super::axis::{MAX_TICK_MARKS, count_param, one_line}; +use super::axis::{MAX_TICK_MARKS, count_given, one_line}; use super::data::apply_data; use super::generate::{children_bounds, generate}; use super::series::{picked_series, reselect_series}; use super::{ - Affine, Document, EngineError, GraphKind, GraphSpec, Node, NodeId, Rect, Result, Session, TickLength, Value, ValueAxisSide, bad, bool_or, f64_or, - f64_req, json, named_id, str_param, + Affine, Document, EngineError, GraphKind, GraphSpec, Node, NodeId, Rect, Result, Session, TickLength, Value, ValueAxisSide, bad, f64_req, json, + named_id, number_or, switch_or, text_param, }; +/// An axis bound `p` gives for `key`: `None` when left out, `Some(None)` for null (calculated from the data), or the +/// number. Any other value, such as `"10"`, is an error. +fn bound_param(p: &Value, key: &str, cmd: &str) -> Result>> { + match p.get(key) { + None => Ok(None), + Some(Value::Null) => Ok(Some(None)), + Some(v) => v.as_f64().map(|x| Some(Some(x))).ok_or_else(|| bad(cmd, format!("`{key}` must be a number or null, not {v}"))), + } +} + /// Regenerate graph `id` from `spec` (adjusting the plot rect for any move/scale since the last /// generation, which is recorded in the spec's `placed` bounds). pub(super) fn regenerate(d: &mut Document, id: NodeId, mut spec: GraphSpec) -> Result<()> { @@ -33,12 +43,14 @@ pub(super) fn regenerate(d: &mut Document, id: NodeId, mut spec: GraphSpec) -> R pub(super) fn create(s: &mut Session, p: &Value) -> Result { const C: &str = "graph.create"; - let kind = - str_param(p, "type").map(|t| GraphKind::parse(t).ok_or_else(|| bad(C, format!("unknown graph type `{t}`")))).transpose()?.unwrap_or_default(); + let kind = text_param(p, "type", C)? + .map(|t| GraphKind::parse(t).ok_or_else(|| bad(C, format!("unknown graph type `{t}`")))) + .transpose()? + .unwrap_or_default(); let (x, y) = (f64_req(p, "x", C)?, f64_req(p, "y", C)?); - let (w, h) = (f64_or(p, "width", 200.0).abs().max(1.0), f64_or(p, "height", 150.0).abs().max(1.0)); + let (w, h) = (number_or(p, "width", 200.0, C)?.abs().max(1.0), number_or(p, "height", 150.0, C)?.abs().max(1.0)); let mut spec = GraphSpec { kind, rect: Rect::new(x, y, x + w, y + h), ..GraphSpec::default() }; - if !apply_data(&mut spec, p) { + if !apply_data(&mut spec, p, C)? { // Starter data so the new graph shows something (edit it with Object → Graph → Data…). spec.series = vec!["Series 1".into(), "Series 2".into()]; spec.categories = vec!["A".into(), "B".into(), "C".into(), "D".into()]; @@ -150,7 +162,8 @@ pub(super) fn set_type(s: &mut Session, p: &Value) -> Result { spec.right_series.retain(|i| *i < count); spec.right_series.sort_unstable(); spec.right_series.dedup(); - if p.get("seriesIndexes").is_some() && str_param(p, "type").is_none() && str_param(p, "valueAxis").is_none() { + let (new_type, new_axis) = (text_param(p, "type", C)?, text_param(p, "valueAxis", C)?); + if p.get("seriesIndexes").is_some() && new_type.is_none() && new_axis.is_none() { return Err(bad(C, "`seriesIndexes` needs a `type` or a `valueAxis`")); } let picked = picked_series(s, p, id, count, C)?; @@ -196,7 +209,7 @@ pub(super) fn set_type(s: &mut Session, p: &Value) -> Result { } // Series picked with Group Selection stay selected through the regeneration (their groups are new nodes). let reselect: Vec = if p.get("seriesIndexes").is_none() { picked.iter().filter_map(|i| u32::try_from(*i).ok()).collect() } else { vec![] }; - if let Some(t) = str_param(p, "type") { + if let Some(t) = new_type { let kind = GraphKind::parse(t).ok_or_else(|| bad(C, format!("unknown graph type `{t}`")))?; if picked.is_empty() { spec.kind = kind; @@ -215,7 +228,7 @@ pub(super) fn set_type(s: &mut Session, p: &Value) -> Result { } } } - if let Some(v) = str_param(p, "valueAxis") { + if let Some(v) = new_axis { let side = ValueAxisSide::parse(v).ok_or_else(|| bad(C, format!("unknown value axis `{v}` (left, right or both)")))?; if picked.is_empty() { spec.value_axis = side; @@ -231,54 +244,58 @@ pub(super) fn set_type(s: &mut Session, p: &Value) -> Result { } } } - spec.separate_scales = bool_or(p, "separateScales", spec.separate_scales); + spec.separate_scales = switch_or(p, "separateScales", spec.separate_scales, C)?; for (key, slot) in [ ("tickLength", &mut spec.tick_length), ("rightTickLength", &mut spec.right_tick_length), ("categoryTickLength", &mut spec.category_tick_length), ] { - if let Some(v) = str_param(p, key) { + if let Some(v) = text_param(p, key, C)? { *slot = TickLength::parse(v).ok_or_else(|| bad(C, format!("unknown {key} `{v}` (none, short or full)")))?; } } for (key, slot) in [("tickMarks", &mut spec.tick_marks), ("rightTickMarks", &mut spec.right_tick_marks), ("categoryTickMarks", &mut spec.category_tick_marks)] { - *slot = count_param(p, key, MAX_TICK_MARKS as u64).map_or(*slot, |n| n.max(1)); + *slot = count_given(p, key, MAX_TICK_MARKS as u64, C)?.map_or(*slot, |n| n.max(1)); } - spec.ticks_between_labels = bool_or(p, "ticksBetweenLabels", spec.ticks_between_labels); - if let Some(v) = str_param(p, "piePosition") { + spec.ticks_between_labels = switch_or(p, "ticksBetweenLabels", spec.ticks_between_labels, C)?; + if let Some(v) = text_param(p, "piePosition", C)? { spec.pie_position = vectorcraft_doc::PiePosition::parse(v).ok_or_else(|| bad(C, format!("unknown piePosition `{v}` (even, ratio or stacked)")))?; } - if let Some(v) = str_param(p, "pieSort") { + if let Some(v) = text_param(p, "pieSort", C)? { spec.pie_sort = vectorcraft_doc::PieSort::parse(v).ok_or_else(|| bad(C, format!("unknown pieSort `{v}` (none, all or first)")))?; } for (key, slot) in [("prefix", &mut spec.prefix), ("suffix", &mut spec.suffix), ("rightPrefix", &mut spec.right_prefix), ("rightSuffix", &mut spec.right_suffix)] { - if let Some(v) = str_param(p, key) { + if let Some(v) = text_param(p, key, C)? { // A label, not a paragraph: one line, at most MAX_AFFIX characters. *slot = one_line(v); } } - spec.right_ticks = count_param(p, "rightTicks", 100).unwrap_or(spec.right_ticks); - if let Some(v) = p.get("rightAxisMin") { - spec.right_axis_min = v.as_f64(); + spec.right_ticks = count_given(p, "rightTicks", 100, C)?.unwrap_or(spec.right_ticks); + for (key, slot) in [ + ("rightAxisMin", &mut spec.right_axis_min), + ("rightAxisMax", &mut spec.right_axis_max), + ("axisMin", &mut spec.axis_min), + ("axisMax", &mut spec.axis_max), + ] { + if let Some(v) = bound_param(p, key, C)? { + *slot = v; + } } - if let Some(v) = p.get("rightAxisMax") { - spec.right_axis_max = v.as_f64(); - } - spec.column_width = f64_or(p, "columnWidth", spec.column_width).clamp(1.0, 1000.0); - spec.cluster_width = f64_or(p, "clusterWidth", spec.cluster_width).clamp(1.0, 1000.0); - spec.legend = bool_or(p, "legend", spec.legend); - spec.drop_shadow = bool_or(p, "dropShadow", spec.drop_shadow); - spec.first_row_in_front = bool_or(p, "firstRowInFront", spec.first_row_in_front); - spec.first_column_in_front = bool_or(p, "firstColumnInFront", spec.first_column_in_front); - spec.filled_lines = bool_or(p, "filledLines", spec.filled_lines); - spec.legend_across_top = bool_or(p, "legendAcrossTop", spec.legend_across_top); + spec.column_width = number_or(p, "columnWidth", spec.column_width, C)?.clamp(1.0, 1000.0); + spec.cluster_width = number_or(p, "clusterWidth", spec.cluster_width, C)?.clamp(1.0, 1000.0); + spec.legend = switch_or(p, "legend", spec.legend, C)?; + spec.drop_shadow = switch_or(p, "dropShadow", spec.drop_shadow, C)?; + spec.first_row_in_front = switch_or(p, "firstRowInFront", spec.first_row_in_front, C)?; + spec.first_column_in_front = switch_or(p, "firstColumnInFront", spec.first_column_in_front, C)?; + spec.filled_lines = switch_or(p, "filledLines", spec.filled_lines, C)?; + spec.legend_across_top = switch_or(p, "legendAcrossTop", spec.legend_across_top, C)?; // After `legend`, which the Graph Type dialog sends too. - if let Some(v) = str_param(p, "pieLegend") { + if let Some(v) = text_param(p, "pieLegend", C)? { (spec.legend, spec.pie_legend_in_wedges) = match v.to_ascii_lowercase().as_str() { "none" => (false, false), "standard" => (true, false), @@ -286,16 +303,10 @@ pub(super) fn set_type(s: &mut Session, p: &Value) -> Result { _ => return Err(bad(C, format!("unknown pieLegend `{v}` (none, standard or wedges)"))), }; } - spec.mark_points = bool_or(p, "markPoints", spec.mark_points); - spec.connect_points = bool_or(p, "connectPoints", spec.connect_points); - spec.edge_to_edge = bool_or(p, "edgeToEdge", spec.edge_to_edge); - spec.ticks = count_param(p, "ticks", 100).unwrap_or(spec.ticks); - if let Some(v) = p.get("axisMin") { - spec.axis_min = v.as_f64(); - } - if let Some(v) = p.get("axisMax") { - spec.axis_max = v.as_f64(); - } + spec.mark_points = switch_or(p, "markPoints", spec.mark_points, C)?; + spec.connect_points = switch_or(p, "connectPoints", spec.connect_points, C)?; + spec.edge_to_edge = switch_or(p, "edgeToEdge", spec.edge_to_edge, C)?; + spec.ticks = count_given(p, "ticks", 100, C)?.unwrap_or(spec.ticks); let label = format!("{} Graph", spec.kind.label()); s.edit("Graph Type", |d, sel| { regenerate(d, id, spec)?; diff --git a/crates/engine/src/cmd/graph/tests.rs b/crates/engine/src/cmd/graph/tests.rs index efcd04320..aef1918a6 100644 --- a/crates/engine/src/cmd/graph/tests.rs +++ b/crates/engine/src/cmd/graph/tests.rs @@ -2638,9 +2638,12 @@ fn counts_come_back_from_the_dialog_as_floats_and_are_capped() { s.execute("graph.setType", &json!({"tickMarks": 3.0, "ticks": 4.0, "categoryTickMarks": 2.6})).unwrap(); let g = spec(&s, id); assert_eq!((g.tick_marks, g.ticks, g.category_tick_marks), (3, 4, 3)); - s.execute("graph.setType", &json!({"categoryTickMarks": 0, "rightTickMarks": 1e300, "rightTicks": -2.0})).unwrap(); + s.execute("graph.setType", &json!({"categoryTickMarks": 0, "rightTickMarks": 1e300})).unwrap(); let g = spec(&s, id); assert_eq!((g.category_tick_marks, g.right_tick_marks, g.right_ticks), (1, 20, 0)); + // A negative count is refused. + s.execute("graph.setType", &json!({"rightTicks": -2.0})).unwrap_err(); + assert_eq!(spec(&s, id).right_ticks, 0); // A file's values come back as drawn. let mut fields = s.execute("graph.setType", &json!({})).unwrap(); assert_eq!(fields["rightTickMarks"], 20); @@ -4498,3 +4501,93 @@ fn a_legend_label_with_kept_negative_space_before_stays_below_the_entry_above_it let (first, next) = (bounds("2024"), bounds("20\n25")); assert!(next.y0 >= first.y1, "the second legend label starts below the first one: {first:?} {next:?}"); } + +/// A graph command given a value of the wrong type refuses it, naming the key, and leaves the graph and the undo +/// history as they were; a null axis bound still goes back to the calculated value. +#[test] +fn graph_commands_refuse_params_of_the_wrong_type() { + let mut s = Session::new(); + s.execute("file.new", &json!({})).unwrap(); + let id = graph(&mut s, "line"); + s.execute("graph.setType", &json!({"id": id.0, "axisMin": 0, "axisMax": 10})).unwrap(); + let spec = |s: &Session| s.doc().unwrap().doc.node(id).unwrap().extras().graph.as_deref().cloned().unwrap(); + let undo_len = |s: &Session| s.doc().unwrap().history.undo.len(); + let (before, steps) = (spec(&s), undo_len(&s)); + let refused = |s: &mut Session, cmd: &str, p: serde_json::Value, msg: &str| { + let e = s.execute(cmd, &p).unwrap_err().to_string(); + assert!(e.contains(msg), "{cmd} {p}: {e}"); + }; + for (key, v, msg) in [ + ("axisMin", json!("10"), "`axisMin` must be a number or null, not \"10\""), + ("axisMax", json!(true), "`axisMax` must be a number or null"), + ("rightAxisMin", json!([1]), "`rightAxisMin` must be a number or null"), + ("rightAxisMax", json!({}), "`rightAxisMax` must be a number or null"), + ("columnWidth", json!("50"), "`columnWidth` must be a number, not \"50\""), + ("clusterWidth", json!(false), "`clusterWidth` must be a number"), + ("ticks", json!("5"), "`ticks` must be a number, not \"5\""), + ("ticks", json!(-2), "`ticks` must be 0 or more"), + ("rightTicks", json!("5"), "`rightTicks` must be a number"), + ("tickMarks", json!("2"), "`tickMarks` must be a number"), + ("rightTickMarks", json!(-1), "`rightTickMarks` must be 0 or more"), + ("categoryTickMarks", json!(true), "`categoryTickMarks` must be a number"), + ("legend", json!("false"), "`legend` must be true or false"), + ("markPoints", json!(0), "`markPoints` must be true or false"), + ("connectPoints", json!("no"), "`connectPoints` must be true or false"), + ("edgeToEdge", json!(1), "`edgeToEdge` must be true or false"), + ("separateScales", json!("true"), "`separateScales` must be true or false"), + ("ticksBetweenLabels", json!(1), "`ticksBetweenLabels` must be true or false"), + ("firstRowInFront", json!("yes"), "`firstRowInFront` must be true or false"), + ("firstColumnInFront", json!(0), "`firstColumnInFront` must be true or false"), + ("filledLines", json!("on"), "`filledLines` must be true or false"), + ("dropShadow", json!(1), "`dropShadow` must be true or false"), + ("legendAcrossTop", json!("x"), "`legendAcrossTop` must be true or false"), + ("type", json!(5), "`type` must be a string"), + ("valueAxis", json!(true), "`valueAxis` must be a string"), + ("tickLength", json!(1), "`tickLength` must be a string"), + ("rightTickLength", json!(1), "`rightTickLength` must be a string"), + ("categoryTickLength", json!(1), "`categoryTickLength` must be a string"), + ("prefix", json!(1), "`prefix` must be a string"), + ("suffix", json!(true), "`suffix` must be a string"), + ("rightPrefix", json!([]), "`rightPrefix` must be a string"), + ("rightSuffix", json!(2), "`rightSuffix` must be a string"), + ("pieLegend", json!(1), "`pieLegend` must be a string"), + ("piePosition", json!(1), "`piePosition` must be a string"), + ("pieSort", json!(1), "`pieSort` must be a string"), + ] { + refused(&mut s, "graph.setType", json!({"id": id.0, key: v}), msg); + } + for (key, v, msg) in [ + ("csv", json!(5), "`csv` must be a string"), + ("series", json!("a"), "`series` must be a list, not \"a\""), + ("categories", json!({}), "`categories` must be a list"), + ("rows", json!("1,2"), "`rows` must be a list"), + ("rows", json!([1, 2]), "`rows` must be lists of numbers or nulls, not 1"), + ("rows", json!([[1, "2"]]), "`rows` must be lists of numbers or nulls"), + ("transpose", json!("true"), "`transpose` must be true or false"), + ("switchXY", json!(1), "`switchXY` must be true or false"), + ] { + refused(&mut s, "graph.setData", json!({"id": id.0, key: v}), msg); + } + assert_eq!(spec(&s), before); + assert_eq!(undo_len(&s), steps); + let count = |s: &Session| s.doc().unwrap().doc.node_count(); + let objects = count(&s); + for (key, v, msg) in [ + ("type", json!(1), "`type` must be a string"), + ("width", json!("300"), "`width` must be a number, not \"300\""), + ("height", json!(true), "`height` must be a number"), + ("csv", json!(1), "`csv` must be a string"), + ("rows", json!([[true]]), "`rows` must be lists of numbers or nulls"), + ] { + let mut p = json!({"type": "column", "x": 0, "y": 0, "width": 100, "height": 100}); + p[key] = v; + refused(&mut s, "graph.create", p, msg); + } + assert_eq!((count(&s), undo_len(&s)), (objects, steps)); + // A null is a key left out, except for an axis bound, where it goes back to the calculated value. + s.execute("graph.setType", &json!({"id": id.0, "legend": null, "ticks": null, "prefix": null, "axisMin": null})).unwrap(); + assert_eq!(spec(&s).axis_min, None); + assert_eq!(spec(&s).axis_max, Some(10.0)); + s.execute("graph.setData", &json!({"id": id.0, "rows": [[1, null], [2, 3]], "series": null, "transpose": null})).unwrap(); + assert_eq!(spec(&s).cells(), vec![vec![Some(1.0), None], vec![Some(2.0), Some(3.0)]]); +} diff --git a/docs/mcp.md b/docs/mcp.md index 5826c5c8c..dfafcd9c3 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -1353,6 +1353,20 @@ message, as in `` `steps` must be a number, not "8" ``, and change nothing: `ste `highlight` in `object.mesh.create`; and `opacity` in `object.mesh.setPointColor`. A null is read as a key left out. `object.mesh.create` refuses an `at` that is not two numbers with `` `at` must be [x, y] ``. +The graph commands refuse a value of the wrong type, naming the key, and leave the graph and the undo history as they +were. `graph.setType` refuses `` `columnWidth` must be a number, not "50" `` for `columnWidth` and `clusterWidth`; +`` `legend` must be true or false `` for `legend`, `markPoints`, `connectPoints`, `edgeToEdge`, `separateScales`, +`ticksBetweenLabels`, `firstRowInFront`, `firstColumnInFront`, `filledLines`, `dropShadow` and `legendAcrossTop`; +`` `type` must be a string `` for `type`, `valueAxis`, the three tick lengths, `prefix`, `suffix`, `rightPrefix`, +`rightSuffix`, `pieLegend`, `piePosition` and `pieSort`; and `` `ticks` must be a number, not "5" `` (or +`` `ticks` must be 0 or more `` for a negative number) for `ticks`, `rightTicks` and the three tick mark counts. +`axisMin`, `axisMax`, `rightAxisMin` and `rightAxisMax` take a number, or null to go back to the value calculated from +the data; any other value is refused with `` `axisMin` must be a number or null, not "10" ``. `graph.setData` and +`graph.create` refuse a `csv` that is not a string, `series` or `categories` that are not lists, and `rows` that are not +lists of numbers and nulls; `graph.setData` also refuses a `transpose` or `switchXY` that is not true or false, and +`graph.create` a `type` that is not a string or a `width` or `height` that is not a number. A null is read as a key +left out. + The transform, Align, Object Properties and Live Shape Properties commands refuse a string, a number or another value that is not true or false with `` `strokes` must be true or false ``, naming the key, and leave the document and its undo history as they were: the options `copy`, `strokes`, `corners`, `patterns`, `objects` and `typeAreas` in