diff --git a/crates/engine/src/cmd/variablecmds.rs b/crates/engine/src/cmd/variablecmds.rs index fb82f1f4b..78e080283 100644 --- a/crates/engine/src/cmd/variablecmds.rs +++ b/crates/engine/src/cmd/variablecmds.rs @@ -5,8 +5,9 @@ //! variable, or a graph root to a `graph` variable (CSV data), put a row of values in //! each dataset, and `dataset.select` swaps the whole document to that row in one undo //! step. `dataset.import` / `dataset.export` move those rows as CSV (header: `name` -//! then variable names); import also reads tab-separated files and files without the -//! `name` column. Agents reach every command through `run_command`, like all engine +//! then variable names); import also reads tab-separated files, files without the +//! `name` column, and header cells that define new variables (`#` for visibility, `@` for +//! image, no prefix for text). Agents reach every command through `run_command`, like all engine //! commands. use std::collections::{BTreeMap, BTreeSet}; @@ -29,8 +30,9 @@ const MAX_CSV_BYTES: usize = 1_048_576; const MAX_CSV_ROWS: usize = 1_000; const MAX_CSV_COLS: usize = 256; -/// Lock Variables refuses New Variable, Delete Variable, Rename Variable (Variable Options) and -/// Make … Dynamic; binding, unbinding and the dataset commands still run. +/// Lock Variables refuses New Variable, Delete Variable, Rename Variable (Variable Options), +/// Make … Dynamic and an import whose header would define a variable; binding, unbinding and the +/// other dataset commands still run. fn variables_unlocked(s: &Session) -> std::result::Result<(), String> { has_doc(s)?; if s.active().is_some_and(|st| st.doc.variables.locked) { Err("variables are locked".into()) } else { Ok(()) } @@ -710,10 +712,52 @@ fn datasets_to_csv(vars: &Variables) -> String { out } +/// The variable that a datasets CSV header cell (trimmed, not empty) in 1-based `column` names, +/// and its kind. A cell that is the name of a variable in `kinds` or in `defined` (the variables +/// that earlier columns define) names that variable. Otherwise `#` before the name makes it a +/// visibility variable, `@` makes it an image variable, and a cell without a prefix names a text +/// variable. A prefixed name that is already a variable must be one of the prefix's kind; a new +/// name is added to `defined`. A `%` cell that names no variable (graph data files) is refused. +fn header_variable( + cell: &str, + column: usize, + kinds: &BTreeMap, + defined: &mut Vec, + cmd: &str, +) -> Result<(String, VariableKind)> { + let known = |name: &str, defined: &[Variable]| kinds.get(name).copied().or_else(|| defined.iter().find(|v| v.name == name).map(|v| v.kind)); + if let Some(kind) = known(cell, defined) { + return Ok((cell.to_string(), kind)); + } + let (name, kind) = if let Some(rest) = cell.strip_prefix('#') { + (rest.trim(), VariableKind::Visibility) + } else if let Some(rest) = cell.strip_prefix('@') { + (rest.trim(), VariableKind::Image) + } else if cell.starts_with('%') { + return Err(bad(cmd, format!("column {column}: `{cell}` is a `%` column of graph data files, which are not read"))); + } else { + (cell, VariableKind::Text) + }; + if name.is_empty() { + return Err(bad(cmd, format!("column {column}: `{cell}` names no variable"))); + } + match known(name, defined) { + Some(k) if k == kind => Ok((name.to_string(), kind)), + Some(k) => Err(bad(cmd, format!("column {column}: `{cell}` names `{name}`, which is a {} variable", k.label()))), + None if name.chars().count() > MAX_NAME => Err(bad(cmd, format!("column {column}: the variable name is longer than {MAX_NAME} characters"))), + None => { + defined.push(Variable { name: name.to_string(), kind }); + Ok((name.to_string(), kind)) + } + } +} + /// Import datasets from CSV. A first header cell `name` makes the first column the dataset -/// names; the other headers are variable names. Without it, every header is a variable name -/// and the rows take the first free names "Data Set 1", "Data Set 2" and so on. Existing rows -/// of the same name are replaced; new names are appended. One undo step. +/// names; the other headers name variables, as [`header_variable`] reads them, and the +/// variables the document doesn't define yet are defined, unbound. Without the `name` column, +/// every header names a variable and the rows take the first free names "Data Set 1", "Data +/// Set 2" and so on. Existing rows of the same name are replaced; new names are appended. One +/// undo step. fn dataset_import(s: &mut Session, p: &Value) -> Result { const C: &str = "dataset.import"; let bytes = datasets_csv_bytes(p, C)?; @@ -730,13 +774,18 @@ fn dataset_import(s: &mut Session, p: &Value) -> Result { (kinds, existing) }; let mut columns: Vec<(usize, String, VariableKind)> = Vec::new(); + let mut defined: Vec = Vec::new(); for (i, h) in header.iter().enumerate().skip(usize::from(name_column)) { - let name = h.trim(); - if name.is_empty() { + let cell = h.trim(); + if cell.is_empty() { return Err(bad(C, format!("empty header in column {}", i + 1))); } - let kind = kinds.get(name).copied().ok_or_else(|| bad(C, format!("no variable named `{name}`")))?; - columns.push((i, name.to_string(), kind)); + let (name, kind) = header_variable(cell, i + 1, &kinds, &mut defined, C)?; + columns.push((i, name, kind)); + } + // Lock Variables refuses New Variable, so the import is refused when a header would define one. + if !defined.is_empty() { + variables_unlocked(s).map_err(|why| EngineError::Disabled(C.into(), why))?; } if rows.len() == 1 { return Err(bad(C, "CSV has a header but no data rows")); @@ -777,7 +826,9 @@ fn dataset_import(s: &mut Session, p: &Value) -> Result { parsed.push(DataSet { name, values }); } let names: Vec = parsed.iter().map(|d| d.name.clone()).collect(); + let new_variables: Vec = defined.iter().map(|v| v.name.clone()).collect(); let (imported, updated) = s.edit("Import Data Sets", |d, _| { + d.variables.variables.extend(defined); let mut imported = 0usize; let mut updated = 0usize; for row in parsed { @@ -791,7 +842,7 @@ fn dataset_import(s: &mut Session, p: &Value) -> Result { } Ok((imported, updated)) })?; - Ok(json!({"imported": imported, "updated": updated, "names": names})) + Ok(json!({"imported": imported, "updated": updated, "names": names, "defined": new_variables})) } /// Export every dataset as CSV (`name` then variables). With `path`, write the file; else return `{csv}`. @@ -1003,7 +1054,7 @@ pub fn specs() -> Vec { "Import Data Sets…", ["Window", "Variables"], None, - "{path? | data? | dataBase64? | csv?} load datasets from CSV, comma- or tab-separated (header: name, then variable names; one row per dataset; without the name column every header is a variable and the rows take the first free names Data Set 1, 2…). Rows whose name already exists are replaced → {imported, updated, names}", + "{path? | data? | dataBase64? | csv?} load datasets from CSV, comma- or tab-separated (header: name, then variable names; one row per dataset; without the name column every header is a variable and the rows take the first free names Data Set 1, 2…). A header that names no variable defines one, unbound: `#Name` defines a visibility variable, `@Name` defines an image variable, and a header without a prefix defines a text variable. A prefixed header whose name is a variable of another kind, and a `%` header that isn't a variable's exact name (graph data files), are refused, and so is a header that would define a variable while the variables are locked. Rows whose name already exists are replaced → {imported, updated, names, defined}", has_doc, dataset_import ), diff --git a/crates/engine/src/tests_variables.rs b/crates/engine/src/tests_variables.rs index b1d8d28d2..0b5698abc 100644 --- a/crates/engine/src/tests_variables.rs +++ b/crates/engine/src/tests_variables.rs @@ -1149,6 +1149,114 @@ fn datasets_csv_row_errors_name_the_file_line() { assert!(long.contains("line 3:") && long.contains("longer than 255"), "{long}"); } +/// A header cell that names no variable defines one, unbound, in the import's undo step: `#` +/// defines a visibility variable, `@` defines an image variable, and a cell without a prefix +/// defines a text variable. Importing the file again finds them; a prefix of the other kind, a +/// bare prefix and a long name fail and define nothing. +#[test] +fn datasets_csv_header_prefixes_define_variables() { + let mut s = session(); + let csv = "name,#Shown,@Photo,Title\nA,true,a.png,x"; + let imp = s.execute("dataset.import", &json!({"csv": csv})).unwrap(); + assert_eq!(imp["defined"], json!(["Shown", "Photo", "Title"])); + assert_eq!((imp["imported"].as_u64(), imp["names"].clone()), (Some(1), json!(["A"]))); + let listed = s.execute("variable.list", &json!({})).unwrap(); + assert_eq!( + listed["variables"], + json!([ + {"name": "Shown", "kind": "visibility", "bindings": 0}, + {"name": "Photo", "kind": "image", "bindings": 0}, + {"name": "Title", "kind": "text", "bindings": 0}, + ]) + ); + let value = |s: &Session, var: &str| s.doc().unwrap().doc.variables.dataset("A").and_then(|d| d.values.get(var)).cloned(); + assert_eq!(value(&s, "Shown"), Some(vectorcraft_doc::DataValue::Visible(true))); + assert_eq!(value(&s, "Photo"), Some(vectorcraft_doc::DataValue::Image("a.png".into()))); + assert_eq!(value(&s, "Title"), Some(vectorcraft_doc::DataValue::Text("x".into()))); + + let again = s.execute("dataset.import", &json!({"csv": "name\t#Shown\t@Photo\tTitle\nA\tfalse\tb.png\ty"})).unwrap(); + assert_eq!((again["updated"].as_u64(), again["defined"].clone()), (Some(1), json!([]))); + assert_eq!(value(&s, "Shown"), Some(vectorcraft_doc::DataValue::Visible(false))); + + let error = |s: &mut Session, csv: &str| s.execute("dataset.import", &json!({"csv": csv})).unwrap_err().to_string(); + let other_kind = error(&mut s, "name,New,#Title\nB,n,true"); + assert!(other_kind.contains("column 3:") && other_kind.contains("text variable"), "{other_kind}"); + let twice = error(&mut s, "Fresh,@Fresh\nn,f.png"); + assert!(twice.contains("column 2:") && twice.contains("text variable"), "{twice}"); + let bare = error(&mut s, "name,#\nB,true"); + assert!(bare.contains("column 2:") && bare.contains("names no variable"), "{bare}"); + let long = error(&mut s, &format!("name,@{}\nB,b.png", "n".repeat(256))); + assert!(long.contains("column 2:") && long.contains("longer than 255"), "{long}"); + assert_eq!(s.doc().unwrap().doc.variables.variables.len(), 3, "a failed import defines nothing"); + + // Each import is one undo step, which takes back the variables it defined with its rows. + s.execute("edit.undo", &json!({})).unwrap(); + assert_eq!(value(&s, "Shown"), Some(vectorcraft_doc::DataValue::Visible(true))); + assert_eq!(s.doc().unwrap().doc.variables.variables.len(), 3); + s.execute("edit.undo", &json!({})).unwrap(); + let vars = &s.doc().unwrap().doc.variables; + assert!(vars.variables.is_empty() && vars.datasets.is_empty()); +} + +/// A header cell that is a variable's exact name names it, even when the name starts with `#`, +/// `@` or `%`, so a file `dataset.export` wrote imports back without defining anything. +#[test] +fn datasets_csv_exact_names_win_over_prefixes() { + let mut s = session(); + for (name, kind) in [("#Shown", "text"), ("@Photo", "visibility"), ("%Chart", "graph"), ("Shown", "visibility")] { + s.execute("variable.define", &json!({"name": name, "kind": kind})).unwrap(); + } + let values = json!({"#Shown": "hello", "@Photo": true, "%Chart": ",Q\nX,3", "Shown": false}); + s.execute("dataset.new", &json!({"name": "A", "values": values})).unwrap(); + let csv = s.execute("dataset.export", &json!({})).unwrap()["csv"].as_str().unwrap().to_string(); + assert!(csv.starts_with("name,#Shown,@Photo,%Chart,Shown\n"), "{csv}"); + s.execute("dataset.delete", &json!({"names": ["A"]})).unwrap(); + let imp = s.execute("dataset.import", &json!({"csv": csv})).unwrap(); + assert_eq!((imp["imported"].as_u64(), imp["defined"].clone()), (Some(1), json!([]))); + let value = |var: &str| s.doc().unwrap().doc.variables.dataset("A").and_then(|d| d.values.get(var)).cloned(); + assert_eq!(value("#Shown"), Some(vectorcraft_doc::DataValue::Text("hello".into()))); + assert_eq!(value("@Photo"), Some(vectorcraft_doc::DataValue::Visible(true))); + assert_eq!(value("%Chart"), Some(vectorcraft_doc::DataValue::Graph(",Q\nX,3".into()))); + assert_eq!(value("Shown"), Some(vectorcraft_doc::DataValue::Visible(false))); + assert_eq!(s.doc().unwrap().doc.variables.variables.len(), 4); +} + +/// A `%` header cell that isn't a variable's name (graph data files) is refused with its column, +/// and the import defines nothing. +#[test] +fn datasets_csv_refuses_graph_data_file_columns() { + let mut s = session(); + let error = |s: &mut Session, csv: &str| s.execute("dataset.import", &json!({"csv": csv})).unwrap_err().to_string(); + let named = error(&mut s, "name,Title,%Sales\nA,x,sales.txt"); + assert!(named.contains("column 3:") && named.contains("`%Sales`"), "{named}"); + let unnamed = error(&mut s, "%Sales\tTitle\nsales.txt\tx"); + assert!(unnamed.contains("column 1:") && unnamed.contains("`%Sales`"), "{unnamed}"); + let vars = &s.doc().unwrap().doc.variables; + assert!(vars.variables.is_empty() && vars.datasets.is_empty()); +} + +/// While the variables are locked, an import is refused when a header cell would define a +/// variable, as New Variable is, and a file whose headers all name variables imports. +#[test] +fn locked_variables_refuse_header_cells_that_define_variables() { + let mut s = session(); + s.execute("variable.define", &json!({"name": "Shown", "kind": "visibility"})).unwrap(); + s.execute("variable.define", &json!({"name": "Title", "kind": "text"})).unwrap(); + s.execute("variable.lock", &json!({"locked": true})).unwrap(); + for csv in ["name,#Shown,Title,New\nA,true,x,n", "@Photo\nb.png", "name,#Fresh\nA,true"] { + let e = s.execute("dataset.import", &json!({"csv": csv})).unwrap_err(); + assert!(matches!(&e, EngineError::Disabled(cmd, why) if cmd == "dataset.import" && why == "variables are locked"), "{csv}: {e}"); + } + let vars = &s.doc().unwrap().doc.variables; + assert!(vars.variables.len() == 2 && vars.datasets.is_empty(), "a refused import defines and imports nothing"); + + let imp = s.execute("dataset.import", &json!({"csv": "name,#Shown,Title\nA,true,x"})).unwrap(); + assert_eq!((imp["imported"].as_u64(), imp["defined"].clone()), (Some(1), json!([]))); + s.execute("variable.lock", &json!({"locked": false})).unwrap(); + let imp = s.execute("dataset.import", &json!({"csv": "name,New\nB,n"})).unwrap(); + assert_eq!(imp["defined"], json!(["New"])); +} + /// Import / export datasets as CSV: header `name` then variables; round-trip keeps values. #[test] fn datasets_csv_round_trips_and_rejects_bad_input() { @@ -1185,12 +1293,11 @@ fn datasets_csv_round_trips_and_rejects_bad_input() { assert!(!s.doc().unwrap().doc.node(r).unwrap().visible); assert_eq!(image(&s, img).link.as_ref().map(|l| l.path.as_str()), Some(blue_path.as_str())); - // Re-import updates in place; unknown variable and bad visibility are errors. + // Re-import updates in place and defines nothing; a bad visibility value is an error. let again = s.execute("dataset.import", &json!({"csv": csv})).unwrap(); assert_eq!((again["imported"].as_u64(), again["updated"].as_u64()), (Some(0), Some(2))); - assert!(s.execute("dataset.import", &json!({"csv": "name,Ghost\nX,y"})).is_err()); + assert_eq!(again["defined"], json!([])); assert!(s.execute("dataset.import", &json!({"csv": "name,Show\nX,maybe"})).is_err()); - assert!(s.execute("dataset.import", &json!({"csv": "Title,Name\nX,y"})).is_err(), "no variable named Title"); let out = dir.file("sets.csv"); s.execute("dataset.export", &json!({"path": out})).unwrap(); diff --git a/crates/engine/tests/import_fuzz.rs b/crates/engine/tests/import_fuzz.rs index fe2a7340d..1e752bf30 100644 --- a/crates/engine/tests/import_fuzz.rs +++ b/crates/engine/tests/import_fuzz.rs @@ -1212,10 +1212,21 @@ proptest! { #![proptest_config(config())] #[test] - fn dataset_csv_garbage_never_panics(s in ".{0,300}", head in prop::sample::select(vec!["", "name,Title,Show,Chart\n", "Title\tShow\tChart\n", "name\tTitle\n", "Show,Chart\n"])) { + fn dataset_csv_garbage_never_panics(s in ".{0,300}", head in prop::sample::select(vec!["", "name,Title,Show,Chart\n", "Title\tShow\tChart\n", "name\tTitle\n", "Show,Chart\n", "name,#Show,@Photo,New\n", "#Shown\t@Title\tChart\n", "name,%Chart,#\n"])) { datasets("data set CSV garbage", json!({"csv": format!("{head}{s}")}))?; } + #[test] + fn prefixed_dataset_headers_never_panic( + cells in prop::collection::vec((prop::sample::select(vec!["", "#", "@", "%", "#@", " # "]), "[A-Za-z #@%\"]{0,8}"), 1..8), + tabs in any::(), + body in ".{0,200}", + ) { + let sep = if tabs { "\t" } else { "," }; + let header: Vec = cells.iter().map(|(prefix, name)| format!("{prefix}{name}")).collect(); + datasets("prefixed data set CSV header", json!({"csv": format!("{}\n{body}", header.join(sep))}))?; + } + #[test] fn dataset_csv_bytes_never_panic(bytes in prop::collection::vec(any::(), 0..300)) { datasets("data set CSV bytes", json!({"dataBase64": vectorcraft_format::base64_encode(&bytes)}))?; diff --git a/docs/mcp.md b/docs/mcp.md index e0e962bf9..7f674398e 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -1130,15 +1130,20 @@ already walks Image Trace. Short agent entry points: values CSV as `graph.setData`). `variable.lock {locked?}` (Lock Variables) locks or unlocks the variables; without `locked` it toggles, and the lock is saved with the document. While the variables are locked, `variable.define`, `variable.delete`, `variable.rename` and the four `variable.make…Dynamic` commands fail with - "variables are locked"; `variable.bind`, `variable.unbind` and the `dataset.*` commands still run, and - `variable.list` reports `locked`. + "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`, `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 files, and files whose first header cell isn't `name`: every header cell is then a variable, and the rows take the first free names `Data Set 1`, `Data Set 2` and so on. An import error about a row's values or name gives the file - line the row starts on. + line the row starts on. A header cell that names no variable defines one, unbound, in the same undo step as the + rows. `#Shown` defines a visibility variable `Shown`, `@Photo` defines an image variable `Photo`, and a cell without + a prefix defines a text variable. A prefixed cell whose name is already a variable of the prefix's kind names it; + one whose name is a variable of another kind fails the import. A `%` cell that isn't a variable's exact name (graph + data files) is refused. The reply's `defined` lists the variables the import defined. While the variables are + locked, an import whose header would define a variable fails with "variables are locked". - Batch: `action.batch {commands, source?: folder|datasets, folder?, destination?: none|saveAndClose|folder, destinationFolder?, overrideOpen?, overrideSave?, overrideExport?, includeSubdirs?, stopOnError?}` plays an action on every file in a folder or every data set (the Actions panel menu's Batch…). Override Save and Override diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index e8112c0ee..a450a5ab6 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -433,6 +433,7 @@ Moved from the ROADMAP's "Shipped so far" on 2026-10-10. Update it when a featur - Export for Screens, Save for Web (slice images, and files named after the document's title) and the Libraries panel name files with the characters Windows does not allow written as `-` and with a `-` after a Windows device name such as `CON`, on every platform (`document.exportForScreens`, `document.exportForWeb`, `library.create`). - Variables (data merge): text, visibility and image variables bound to objects (an object may carry several), dataset rows applied in one undo step (`variable.*`, `dataset.*`) from a Variables panel and Window › Variables, with dialogs to define, bind and edit, Make Text/Visibility/Image Dynamic, Capture/Update Data Set, CSV import/export (`dataset.import` / `dataset.export`), and a row in italics when an edit leaves the art not matching it; an image dataset value is a file path and apply keeps the object's bounds; bindings let go of deleted art and locked art is left alone; every command, like the rest, is drivable over MCP and the CLI. - Variables: Import Data Sets… (`dataset.import`) reads tab-separated files as well as comma-separated ones, and files without a `name` column, whose rows take the first free names Data Set 1, Data Set 2 and so on. An import error about a row's values or name gives the file line the row starts on. + - Variables: Import Data Sets… (`dataset.import`) defines the variables a file's header names that the document lacks, unbound. `#Shown` defines a visibility variable, `@Photo` defines an image variable, and a header without a prefix defines a text variable. A header that is a variable's exact name keeps naming it, and any other `%` header (graph data files) is refused. - JSON-lines control channel with real egui pointer and keyboard injection. - Synthetic input validates complete gestures before queue mutation: at most 256 drag steps, 16 clicks and 1,024 pending events; up to 64 control requests per frame, with later requests retained for the next frame (#1709). - The control server writes each reply through a 64 KiB buffer. On Windows 11 it sends the 17 MB `document.inspect` reply of a document with 50,000 objects on one layer to a client over loopback in 0.09 s instead of 34 s.