From ab0072a39edd1605bb07c24723f174b1ffdc8593 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Mon, 2 Nov 2026 11:21:16 +0700 Subject: [PATCH] Variables: Edit Data Set's Name field renames the data set Squashed from #5121 onto main. --- crates/engine/src/cmd/variablecmds.rs | 24 ++++++-- crates/engine/src/tests/variables.rs | 31 +++++++++++ crates/ui-egui/src/dialogs/variables.rs | 74 ++++++++++++++++++++++++- docs/mcp.md | 3 +- 4 files changed, 125 insertions(+), 7 deletions(-) diff --git a/crates/engine/src/cmd/variablecmds.rs b/crates/engine/src/cmd/variablecmds.rs index f31477a85..040d8c802 100644 --- a/crates/engine/src/cmd/variablecmds.rs +++ b/crates/engine/src/cmd/variablecmds.rs @@ -400,18 +400,34 @@ fn dataset_new(s: &mut Session, p: &Value) -> Result { } /// Replace a dataset's values. A variable left out of `values` loses its value in this row, -/// so the dataset ends up holding exactly what the call asked for. +/// so the dataset ends up holding exactly what the call asked for. With `newName` the row is +/// also renamed, in the same undo step, as `dataset.rename` would rename it. fn dataset_set(s: &mut Session, p: &Value) -> Result { const C: &str = "dataset.set"; let name = var_name(p, C)?; - let values = values_param(&s.doc()?.doc.variables, p, C)?; + let to = match p.get("newName") { + None | Some(Value::Null) => name.clone(), + Some(v) => named(v.as_str(), "newName", C)?, + }; + let st = s.doc()?; + if st.doc.variables.dataset(&name).is_none() { + return Err(bad(C, format!("no dataset named `{name}`"))); + } + if to != name && st.doc.variables.dataset(&to).is_some() { + return Err(bad(C, format!("dataset `{to}` already exists"))); + } + let values = values_param(&st.doc.variables, p, C)?; let (set, removed) = edit_variables(s, "Edit Data Set", |v| { let ds = v.datasets.iter_mut().find(|x| x.name == name).ok_or_else(|| bad(C, format!("no dataset named `{name}`")))?; let removed: Vec = ds.values.keys().filter(|k| !values.contains_key(*k)).cloned().collect(); ds.values = values.clone(); + ds.name = to.clone(); + if v.active_dataset.as_deref() == Some(name.as_str()) { + v.active_dataset = Some(to.clone()); + } Ok((values.len(), removed)) })?; - Ok(json!({"name": name, "values": set, "removed": removed})) + Ok(json!({"name": to, "values": set, "removed": removed})) } fn dataset_rename(s: &mut Session, p: &Value) -> Result { @@ -736,7 +752,7 @@ pub fn specs() -> Vec { "Edit Data Set…", ["Window", "Variables"], None, - "{name, values?: {variable: value}} replace a dataset's values; a variable left out loses its value in this row → {values, removed}", + "{name, newName?, values?: {variable: value}} replace a dataset's values; a variable left out loses its value in this row. `newName` also renames the dataset in the same undo step, refused when another dataset has that name → {name, values, removed}", has_doc, dataset_set ), diff --git a/crates/engine/src/tests/variables.rs b/crates/engine/src/tests/variables.rs index a7dcd9dec..6ba092b02 100644 --- a/crates/engine/src/tests/variables.rs +++ b/crates/engine/src/tests/variables.rs @@ -851,6 +851,37 @@ fn dataset_set_replaces_the_row_and_reports_what_it_dropped() { assert_eq!(plain(&s, t), "z"); } +/// `newName` renames the row in the same step as its values change, and keeps it active. +#[test] +fn dataset_set_with_a_new_name_renames_the_row() { + let mut s = session(); + s.execute("variable.define", &json!({"name": "Name", "kind": "text"})).unwrap(); + s.execute("dataset.new", &json!({"name": "D", "values": {"Name": "y"}})).unwrap(); + s.execute("dataset.new", &json!({"name": "E", "values": {"Name": "e"}})).unwrap(); + s.execute("dataset.select", &json!({"name": "D"})).unwrap(); + let undo = s.doc().unwrap().history.undo.len(); + + let v = s.execute("dataset.set", &json!({"name": "D", "newName": "Spring", "values": {"Name": "z"}})).unwrap(); + assert_eq!(v["name"], "Spring"); + let sets = s.execute("dataset.list", &json!({})).unwrap(); + assert_eq!((sets["datasets"][0]["name"].clone(), sets["active"].clone()), (json!("Spring"), json!("Spring"))); + assert_eq!(sets["datasets"][0]["values"], json!({"Name": {"text": "z"}})); + assert_eq!(s.doc().unwrap().history.undo.len(), undo + 1, "one undo step"); + + // Its own name is no rename; another row's name, or an empty one, is refused and changes nothing. + s.execute("dataset.set", &json!({"name": "Spring", "newName": "Spring", "values": {"Name": "z"}})).unwrap(); + for bad in [json!("E"), json!(""), json!(3)] { + assert!(s.execute("dataset.set", &json!({"name": "Spring", "newName": bad, "values": {"Name": "x"}})).is_err(), "{bad}"); + } + let sets = s.execute("dataset.list", &json!({})).unwrap(); + assert_eq!(sets["datasets"][0]["values"], json!({"Name": {"text": "z"}})); + assert_eq!(sets["datasets"][1]["values"], json!({"Name": {"text": "e"}})); + + s.execute("edit.undo", &json!({})).unwrap(); + s.execute("edit.undo", &json!({})).unwrap(); + assert_eq!(s.execute("dataset.list", &json!({})).unwrap()["active"], "D", "undo puts the name back"); +} + #[test] fn bad_variables_requests_are_errors_not_panics() { let mut s = session(); diff --git a/crates/ui-egui/src/dialogs/variables.rs b/crates/ui-egui/src/dialogs/variables.rs index 7fd8639ab..61c82f4ad 100644 --- a/crates/ui-egui/src/dialogs/variables.rs +++ b/crates/ui-egui/src/dialogs/variables.rs @@ -5,7 +5,8 @@ //! name and a kind; a dataset is a name and one field per variable — a text field for a //! `text`, `image` (a path) or `graph` (CSV) variable, a checkbox for a `visibility` one. //! `dataset.set` opens with the row's current values, so editing a row is editing what it -//! already says. +//! already says, and its Name field renames the row. The data set dialogs stay open when OK +//! fails, keeping what was typed. use serde_json::{Value, json}; use vectorcraft_doc::{DataValue, VariableKind, Variables}; @@ -19,6 +20,10 @@ use crate::widgets; /// The dialog kind of every Variables dialog. pub const KIND: &str = "variables"; +/// The field of Edit Data Set that holds the name of the row it edits, which its Name field +/// renames. +const EDITED: &str = "__dataset"; + /// The kinds a variable can have, as `variable.define` names them. const KINDS: [&str; 4] = ["text", "visibility", "image", "graph"]; @@ -85,6 +90,8 @@ pub fn open(app: &mut VectorcraftApp, id: &str, preselect: Option<&str>) -> bool }; let Some(current) = vars.dataset(&name) else { return true }; fields.insert("name".into(), json!(name)); + // The row being edited; the Name field renames it. + fields.insert(EDITED.into(), json!(name)); value_fields(&vars, Some(¤t.values), &mut fields); // An empty text value is a value, but its field looks like a field with no value: // mark it so OK keeps it until the field is edited. @@ -322,7 +329,17 @@ fn confirm(app: &mut VectorcraftApp, d: &Dialog) -> Result { "dataset.rename" => json!({"name": name, "newName": d.str("newName")}), "dataset.new" | "dataset.set" => { let values = app.session.active().map(|st| values_of(d, &st.doc.variables)).unwrap_or_else(|| json!({})); - json!({"name": name, "values": values}) + // A data set dialog keeps its typed values when OK fails (a taken or empty name). + let p = if cmd == "dataset.set" { + json!({"name": d.str(EDITED), "newName": name, "values": values}) + } else { + json!({"name": name, "values": values}) + }; + let r = app.run(&cmd, p); + if r.is_ok() { + app.ui.dialog = None; + } + return r; } "dataset.select" => json!({"name": name}), _ => json!({}), @@ -442,6 +459,59 @@ mod tests { assert_eq!(sets["datasets"][0]["values"]["Badge"], json!({"visible": true})); } + #[test] + fn edit_data_sets_name_field_renames_the_row_it_edits() { + let mut app = app_with_variables(); + app.run("dataset.new", json!({"name": "Row 1", "values": {"Title": "Hello"}})).unwrap(); + app.run("dataset.new", json!({"name": "Row 2", "values": {"Title": "Other"}})).unwrap(); + app.run("dataset.select", json!({"name": "Row 1"})).unwrap(); + let undo = app.session.active().unwrap().history.undo.len(); + open(&mut app, "dataset.set", None); + { + let d = app.ui.dialog.as_mut().unwrap(); + d.fields.insert("name".into(), json!("Spring")); + d.fields.insert(value_key("Title"), json!("Changed")); + } + crate::dialogs::confirm(&mut app).unwrap(); + assert!(app.ui.dialog.is_none()); + let sets = app.run("dataset.list", json!({})).unwrap(); + assert_eq!((sets["datasets"][0]["name"].clone(), sets["active"].clone()), (json!("Spring"), json!("Spring"))); + assert_eq!(sets["datasets"][0]["values"]["Title"], json!({"text": "Changed"})); + assert_eq!(sets["datasets"][1]["values"]["Title"], json!({"text": "Other"}), "the other row is untouched"); + assert_eq!(app.session.active().unwrap().history.undo.len(), undo + 1, "one undo step"); + + // Another row's name, or no name, is refused: neither row changes and the dialog keeps + // what was typed. + for taken in ["Row 2", " "] { + open(&mut app, "dataset.set", None); + { + let d = app.ui.dialog.as_mut().unwrap(); + d.fields.insert("name".into(), json!(taken)); + d.fields.insert(value_key("Title"), json!("Meant for Spring")); + } + assert!(crate::dialogs::confirm(&mut app).is_err(), "{taken:?}"); + assert_eq!(app.ui.dialog.as_ref().map(|d| d.str(&value_key("Title"))).as_deref(), Some("Meant for Spring")); + let sets = app.run("dataset.list", json!({})).unwrap(); + assert_eq!(sets["datasets"][0]["values"]["Title"], json!({"text": "Changed"})); + assert_eq!(sets["datasets"][1]["values"]["Title"], json!({"text": "Other"})); + } + } + + #[test] + fn new_data_set_keeps_its_values_when_the_name_is_taken() { + let mut app = app_with_variables(); + app.run("dataset.new", json!({"name": "Row 1", "values": {"Title": "Hello"}})).unwrap(); + open(&mut app, "dataset.new", None); + { + let d = app.ui.dialog.as_mut().unwrap(); + d.fields.insert("name".into(), json!("Row 1")); + d.fields.insert(value_key("Title"), json!("Typed")); + } + assert!(crate::dialogs::confirm(&mut app).is_err()); + assert_eq!(app.ui.dialog.as_ref().map(|d| d.str(&value_key("Title"))).as_deref(), Some("Typed")); + assert_eq!(app.run("dataset.list", json!({})).unwrap()["datasets"].as_array().map(Vec::len), Some(1)); + } + #[test] fn selecting_a_dataset_from_the_dialog_applies_it() { let mut app = app_with_variables(); diff --git a/docs/mcp.md b/docs/mcp.md index a69fe660b..5a7948614 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -1639,7 +1639,8 @@ already walks Image Trace. Short agent entry points: `variable.loadLibrary` that would delete or define a variable (below) fail with "variables are locked"; `variable.bind`, `variable.unbind` and the `dataset.*` commands still run (except a `dataset.import` whose header would define a variable, below), and `variable.list` reports `locked`. -- Data sets: `dataset.list`, `dataset.new {name, values?}`, `dataset.set {name, values?}`, `dataset.rename`, +- Data sets: `dataset.list`, `dataset.new {name, values?}`, `dataset.set {name, newName?, values?}` (`newName` also + renames the row, in the same undo step; Edit Data Set's Name field sends it), `dataset.rename`, `dataset.delete`, `dataset.capture`, `dataset.update`, `dataset.select {name}`, `dataset.next` / `dataset.prev` (each `dataset.select` / next / prev is one undo step over every bound object), `dataset.import {path|data|dataBase64|csv}` / `dataset.export {path?}` (CSV: `name`, then one column per variable). `dataset.import` also reads tab-separated