Variables: Edit Data Set's Name field renames the data set

Squashed from #5121 onto main.

crates/engine/src/cmd/variablecmds.rs

@@ -400,18 +400,34 @@ fn dataset_new(s: &mut Session, p: &Value) -> Result<Value> {
 }
 
 /// 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<Value> {
     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<String> = 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<Value> {
@@ -736,7 +752,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "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
         ),

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();

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(&current.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<Value, String> {
         "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();

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