From e90c4e6fb2b70de28298c0d36a91aae6ec918d60 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sat, 14 Nov 2026 13:07:03 -0800 Subject: [PATCH] Variables: Load and Save Variable Library hold visibility, image and graph variables Load Variable Library defines each variable with the kind of its trait (textcontent, visibility, fileref, graphdata; any other or none is the unknown kind) and reads visibility values from true or false, image values from a file's absolute path or file: URL, and graph values from a data element of series names and rows. A document variable declared with the same name and kind keeps its bindings. Save Variable Library writes every kind in the same forms and is enabled with any variable. Image values that aren't absolute paths and graph data past what a graph holds are refused, so every saved file loads back. --- crates/engine/src/cmd/graph.rs | 4 +- crates/engine/src/cmd/graph/data.rs | 2 +- crates/engine/src/cmd/variablecmds.rs | 8 +- crates/engine/src/cmd/variablecmds/library.rs | 546 ++++++++++++++---- crates/engine/src/tests/variables.rs | 352 ++++++++--- crates/ui-egui/src/panels/variables.rs | 17 +- crates/ui-egui/src/tests/picks.rs | 16 +- docs/mcp.md | 60 +- docs/target-app-parity.md | 4 +- 9 files changed, 773 insertions(+), 236 deletions(-) diff --git a/crates/engine/src/cmd/graph.rs b/crates/engine/src/cmd/graph.rs index da23bd4f04..1147e9ce9b 100644 --- a/crates/engine/src/cmd/graph.rs +++ b/crates/engine/src/cmd/graph.rs @@ -37,9 +37,9 @@ use axis::{nice_axis, tick_values}; #[cfg(test)] use columns::{fill_totals, total_text}; pub use commands::specs; -pub(crate) use data::apply_csv; +pub(crate) use data::{apply_csv, parse_csv}; #[cfg(test)] -use data::{fmt_value, parse_csv, to_csv}; +use data::{fmt_value, to_csv}; #[cfg(test)] use generate::generate; #[cfg(test)] diff --git a/crates/engine/src/cmd/graph/data.rs b/crates/engine/src/cmd/graph/data.rs index f7f672325e..68a83fd80a 100644 --- a/crates/engine/src/cmd/graph/data.rs +++ b/crates/engine/src/cmd/graph/data.rs @@ -42,7 +42,7 @@ fn cell(c: &str) -> Cell { } } -pub(super) fn parse_csv(csv: &str) -> (Vec, Vec, Vec>>) { +pub(crate) fn parse_csv(csv: &str) -> (Vec, Vec, Vec>>) { let csv = csv.strip_prefix('\u{feff}').unwrap_or(csv); // Records end at line breaks outside quotes; a record is tab-separated if it has a tab outside quotes. let rows: Vec> = split_unquoted(csv, '\n') diff --git a/crates/engine/src/cmd/variablecmds.rs b/crates/engine/src/cmd/variablecmds.rs index 5f8d6605b3..a73c16448b 100644 --- a/crates/engine/src/cmd/variablecmds.rs +++ b/crates/engine/src/cmd/variablecmds.rs @@ -9,8 +9,8 @@ //! before an image variable); import replaces the datasets, names the rows Data Set 1, 2…, //! also reads tab-separated files, and reads header cells that define new variables (`#` for //! visibility, `@` for image, no prefix for text). `variable.loadLibrary` replaces the variables and data sets with the -//! text variables and data sets of a variable library, the XML file of the reference app's Save -//! Variable Library, and `variable.saveLibrary` writes the text variables and data sets as one. +//! variables and data sets of a variable library, the XML file of the reference app's Save +//! Variable Library, and `variable.saveLibrary` writes the variables and data sets as one. //! Agents reach every command through `run_command`, like all engine commands. use std::collections::{BTreeMap, BTreeSet}; @@ -886,7 +886,7 @@ pub fn specs() -> Vec { "Load Variable Library…", ["Window", "Variables"], None, - "{path? | xml? | dataBase64?} replace the document's variables and data sets with a variable library's (XML: `variableSets` › `variableSet` › `variables` of `variable` elements with `varName` and `trait`, and `sampleDataSets` of `sampleDataSet` elements with `dataSetName`) in one undo step (none when the document already holds what the library gives). Every data set is deleted, and every variable of the document is deleted with its bindings and values except a text variable the library also declares, which keeps its bindings; then the library's other variables whose `trait` is `textcontent` are defined as text variables, unbound, and each data set takes a value for a variable from the `p` elements of the element named after it, one paragraph each. Variables of other kinds are left out and named in `skipped` and `warning`. Refused while the variables are locked when it would delete or define a variable → {variables, deleted, deletedDataSets, names, skipped, warning?}", + "{path? | xml? | dataBase64?} replace the document's variables and data sets with a variable library's (XML: `variableSets` › `variableSet` › `variables` of `variable` elements with `varName` and `trait`, and `sampleDataSets` of `sampleDataSet` elements with `dataSetName`) in one undo step (none when the document already holds what the library gives). Every data set is deleted, and every variable of the document is deleted with its bindings and values except a variable the library also declares with the same kind, which keeps its bindings; then the library's other variables are defined, unbound, with the kind of their `trait`: `textcontent` text, `visibility` visibility, `fileref` image, `graphdata` graph, and any other or none unknown (no values). Each data set takes a variable's value from the element named after it: a text value from its `p` elements, one paragraph each; a visibility value from `true` or `false`; an image value from a file's absolute path or a `file:` URL (the file isn't read); a graph value from a `data` element (`propertyRow` series names, `values` › `row` with a first `value` key=\"name\" category), read as the CSV graph.setData takes. A value that doesn't fit its kind is refused. Refused while the variables are locked when it would delete or define a variable → {variables, deleted, deletedDataSets, names}", has_doc, load_library ), @@ -895,7 +895,7 @@ pub fn specs() -> Vec { "Save Variable Library…", ["Window", "Variables"], None, - "{path?} write the document's text variables and every data set as a variable library, the XML that variable.loadLibrary reads: a `variable` element with `varName` and `trait=\"textcontent\"` per text variable, and a `sampleDataSet` element with `dataSetName` per data set, holding an element named after each text variable it has a value for, with one `p` element per line of the value. Variables of other kinds are left out with their values and named in `skipped` and `warning`. Refused without a text variable, for a text variable whose name isn't an XML name (letters, digits, `-`, `_` and `.`, starting with a letter or `_`), for a data set name or value that holds a control character, for a data set name with white space at either end, for a name longer than 255 characters or used twice, and past variable.loadLibrary's caps (256 variables, 1,000 data sets, 8 MiB). Without path → {xml, variables, datasets, skipped, warning?}; with it → {path, variables, datasets, skipped, warning?}", + "{path?} write the document's variables and every data set as a variable library, the XML that variable.loadLibrary reads: a `variable` element with `varName` per variable, with the `trait` of its kind (`textcontent`, `visibility`, `fileref` for image, `graphdata` for graph; none for an unknown kind), and a `sampleDataSet` element with `dataSetName` per data set, holding an element named after each variable it has a value for: one `p` element per line of a text value, `true` or `false`, an image's path (a `file:///` URL for a path from a Unix root), or graph data as a `data` element. Refused without a variable, for a variable whose name isn't an XML name (letters, digits, `-`, `_` and `.`, starting with a letter or `_`; not asked of an unknown kind), for a name or value that holds a control character, for a name with white space at either end, for a name longer than 255 characters or used twice, for an image value that isn't a file's absolute path, and past variable.loadLibrary's caps (256 variables, 1,000 data sets, 8 MiB). Without path → {xml, variables, datasets}; with it → {path, variables, datasets}", has_doc, save_library ), diff --git a/crates/engine/src/cmd/variablecmds/library.rs b/crates/engine/src/cmd/variablecmds/library.rs index e1f4ae0bd5..1c20fc77f7 100644 --- a/crates/engine/src/cmd/variablecmds/library.rs +++ b/crates/engine/src/cmd/variablecmds/library.rs @@ -1,18 +1,29 @@ use std::collections::HashMap; +use std::fmt::Write as _; + +use vectorcraft_doc::{GraphSpec, MAX_GRAPH_CATEGORIES, MAX_GRAPH_SERIES}; + use super::{ - BTreeMap, BTreeSet, DataSet, DataValue, Document, EngineError, MAX_NAME, NodeId, NodeKind, Result, Session, Value, Variable, VariableKind, - Variables, absolute_path, bad, json, named, read_file_head, str_param, variables_unlocked, write_file, + BTreeMap, BTreeSet, DataSet, DataValue, Document, EngineError, MAX_GRAPH_CSV, MAX_NAME, NodeId, NodeKind, Result, Session, Value, Variable, + VariableKind, Variables, absolute_path, bad, json, named, read_file_head, str_param, variables_unlocked, write_file, }; /// The namespace of a variable library's elements, the default namespace of its data sets' values, -/// and the `category` of its text variables. +/// and the `category` of its visibility and image variables. const NS_VARIABLES: &str = "http://ns.adobe.com/Variables/1.0/"; // brand-ok: the namespace variable libraries use const NS_CUSTOM: &str = "http://ns.adobe.com/GenericCustomNamespace/1.0/"; // brand-ok: the namespace variable libraries use +/// The `category` of a library's text variables. const NS_FLOWS: &str = "http://ns.adobe.com/Flows/1.0/"; // brand-ok: the namespace variable libraries use +/// The `category` of a library's graph variables. +const NS_GRAPHS: &str = "http://ns.adobe.com/Graphs/1.0/"; // brand-ok: the namespace variable libraries use + +/// The white space XML puts round a value: a value's text is read without it at either end. +const XML_SPACE: [char; 4] = [' ', '\t', '\n', '\r']; + /// Caps on a variable library (paths and MCP payloads are untrusted): its size, its XML nodes, how /// deep its elements nest, and the entities, variables and data sets it declares. const MAX_LIBRARY_BYTES: usize = 8 << 20; @@ -170,16 +181,47 @@ fn without_doctype(xml: &str, cmd: &str) -> Result { Ok(out) } -/// A variable library as Load Variable Library reads it: the names of its text variables in the -/// order it declares them, the name and `trait` of each variable of another kind, its data sets -/// with their text values, and the element id each text variable's `docRef` names. +/// One variable a library declares: its name, its kind, and its `trait` as the library spells it +/// (`None` when it has none). +struct LibraryVariable { + name: String, + kind: VariableKind, + trait_name: Option, +} + +/// A variable library as Load Variable Library reads it: its variables in the order it declares +/// them, its data sets with their values, and the element id each text variable's `docRef` names. pub(crate) struct Library { - text: Vec, - other: Vec<(String, String)>, + variables: Vec, datasets: Vec, refs: Vec<(String, String)>, } +/// The kind of a library variable whose `trait` is `t`: `textcontent` is text, `visibility` is +/// visibility, `fileref` (a linked file) is image and `graphdata` is graph. A variable without a +/// `trait`, or with one VectorCraft doesn't know, is of unknown kind. +fn trait_kind(t: Option<&str>) -> VariableKind { + match t { + Some("textcontent") => VariableKind::Text, + Some("visibility") => VariableKind::Visibility, + Some("fileref") => VariableKind::Image, + Some("graphdata") => VariableKind::Graph, + _ => VariableKind::Unknown, + } +} + +/// The `trait` and the `category` reference a library declares a variable of `kind` with, or `None` +/// for a variable of unknown kind, which is declared with neither. +fn kind_trait(kind: VariableKind) -> Option<(&'static str, &'static str)> { + match kind { + VariableKind::Text => Some(("textcontent", "&ns_flows;")), + VariableKind::Visibility => Some(("visibility", "&ns_vars;")), + VariableKind::Image => Some(("fileref", "&ns_vars;")), + VariableKind::Graph => Some(("graphdata", "&ns_graphs;")), + VariableKind::Unknown => None, + } +} + impl Library { /// The element ids the text variables' `docRef`s name. pub(crate) fn elements(&self) -> std::collections::HashSet { @@ -190,12 +232,11 @@ impl Library { /// Read a variable library: XML whose `variableSets` element holds `variableSet` elements, each /// with a `variables` element of `variable` elements (`varName`, `trait`) and a `sampleDataSets` /// element of `sampleDataSet` elements (`dataSetName`). Elements are matched by their local names. -/// A variable whose `trait` is `textcontent` is a text variable. In a data set, an element named -/// after a text variable gives its value: the text of its `p` elements, one paragraph each, and -/// none when it has no `p` element. Values for variables of other kinds are left out. A variable -/// or data set declared twice, a value for a variable the library doesn't declare, and a library -/// past the caps are refused. Entities that its document type declaration defines are read as -/// [`without_doctype`] says. +/// A variable's kind comes from its `trait` ([`trait_kind`]). In a data set, an element named after +/// a variable gives its value, as [`library_value`] reads it. A variable or data set declared +/// twice, a value for a variable the library doesn't declare, a value that doesn't fit its +/// variable's kind, and a library past the caps are refused. Entities that its document type +/// declaration defines are read as [`without_doctype`] says. fn parse_library(xml: &str, cmd: &str) -> Result { let xml = without_doctype(xml, cmd)?; // roxmltree parses nested elements recursively. @@ -210,32 +251,29 @@ fn parse_library(xml: &str, cmd: &str) -> Result { return Err(bad(cmd, "the file holds no `variableSets` element")); }; let sets: Vec = root.children().filter(|n| is(n, "variableSet")).collect(); - let (mut text, mut other, mut declared, mut refs) = (Vec::new(), Vec::new(), BTreeSet::new(), Vec::new()); + let (mut variables, mut kinds, mut refs) = (Vec::new(), BTreeMap::new(), Vec::new()); for var in sets.iter().flat_map(|s| s.children()).filter(|n| is(n, "variables")).flat_map(|v| v.children()).filter(|n| is(n, "variable")) { let name = named(var.attribute("varName"), "varName", cmd)?; - if !declared.insert(name.clone()) { + let trait_name = var.attribute("trait").map(|t| t.chars().take(MAX_NAME).collect::()); + let kind = trait_kind(trait_name.as_deref()); + if kinds.insert(name.clone(), kind).is_some() { return Err(bad(cmd, format!("variable `{name}` is declared twice"))); } - if declared.len() > MAX_LIBRARY_VARIABLES { + if kinds.len() > MAX_LIBRARY_VARIABLES { return Err(bad(cmd, format!("the variable library declares more than {MAX_LIBRARY_VARIABLES} variables"))); } - match var.attribute("trait") { - Some("textcontent") => { - // `docRef="id('…')"` names the element of the variable's bound object. - let element = var.attribute("docRef").and_then(|r| r.trim().strip_prefix("id('")?.strip_suffix("')")).filter(|e| !e.is_empty()); - if let Some(element) = element { - refs.push((name.clone(), element.to_string())); - } - text.push(name); + if kind == VariableKind::Text { + // `docRef="id('…')"` names the element of the variable's bound object. + let element = var.attribute("docRef").and_then(|r| r.trim().strip_prefix("id('")?.strip_suffix("')")).filter(|e| !e.is_empty()); + if let Some(element) = element { + refs.push((name.clone(), element.to_string())); } - Some(t) => other.push((name, t.chars().take(MAX_NAME).collect())), - None => return Err(bad(cmd, format!("variable `{name}` has no `trait`"))), } + variables.push(LibraryVariable { name, kind, trait_name }); } - if declared.is_empty() { + if variables.is_empty() { return Err(bad(cmd, "the variable library declares no variables")); } - let text_names: BTreeSet<&str> = text.iter().map(String::as_str).collect(); let (mut datasets, mut seen) = (Vec::new(), BTreeSet::new()); for row in sets.iter().flat_map(|s| s.children()).filter(|n| is(n, "sampleDataSets")).flat_map(|d| d.children()).filter(|n| is(n, "sampleDataSet")) @@ -250,27 +288,194 @@ fn parse_library(xml: &str, cmd: &str) -> Result { let (mut values, mut held) = (BTreeMap::new(), BTreeSet::new()); for el in row.children().filter(roxmltree::Node::is_element) { let var = el.tag_name().name(); - if !declared.contains(var) { + let Some(kind) = kinds.get(var) else { return Err(bad(cmd, format!("data set `{name}` holds a value for `{var}`, which the library doesn't declare"))); - } + }; if !held.insert(var) { return Err(bad(cmd, format!("data set `{name}` holds two values for `{var}`"))); } - if !text_names.contains(var) { - continue; - } - let paras: Vec = el - .children() - .filter(|n| is(n, "p")) - .map(|p| p.descendants().filter(roxmltree::Node::is_text).filter_map(|t| t.text()).collect()) - .collect(); - if !paras.is_empty() { - values.insert(var.to_string(), DataValue::Text(paras.join("\n"))); + if let Some(value) = library_value(el, *kind, &name, cmd)? { + values.insert(var.to_string(), value); } } datasets.push(DataSet { name, values }); } - Ok(Library { text, other, datasets, refs }) + Ok(Library { variables, datasets, refs }) +} + +/// The value a data set's element `el` gives a variable of `kind`, or `None` when it gives none. +/// A text value is the text of its `p` elements, one paragraph each, and none without a `p` +/// element. A visibility value is `true` or `false`, in any case. An image value is the absolute +/// path of a file, or a `file:` URL of one ([`library_path`]); the file isn't read. A graph value is +/// a `data` element ([`graph_value`]). Visibility, image and graph values are read without the +/// white space round them, and an element holding only white space gives none. A variable of +/// unknown kind takes no value, so what its element holds is passed over. `set` names the data set +/// in the errors. +fn library_value(el: roxmltree::Node, kind: VariableKind, set: &str, cmd: &str) -> Result> { + let refused = |what: &str| bad(cmd, format!("data set `{set}` holds a value for `{}` that isn't {what}", el.tag_name().name())); + let text = || if elements(el).next().is_some() { None } else { Some(deep_text(el)) }; + match kind { + VariableKind::Text => { + let paras: Vec = elements(el).filter(|n| n.tag_name().name() == "p").map(deep_text).collect(); + Ok((!paras.is_empty()).then(|| DataValue::Text(paras.join("\n")))) + } + VariableKind::Visibility => { + let what = "`true` or `false`"; + let text = text().ok_or_else(|| refused(what))?; + match text.trim_matches(XML_SPACE) { + "" => Ok(None), + t if t.eq_ignore_ascii_case("true") => Ok(Some(DataValue::Visible(true))), + t if t.eq_ignore_ascii_case("false") => Ok(Some(DataValue::Visible(false))), + _ => Err(refused(what)), + } + } + VariableKind::Image => { + let what = "a file's absolute path or `file:` URL"; + let text = text().ok_or_else(|| refused(what))?; + match text.trim_matches(XML_SPACE) { + "" => Ok(None), + t => library_path(t).map(|p| Some(DataValue::Image(p))).ok_or_else(|| refused(what)), + } + } + VariableKind::Graph => graph_value(el, set, cmd), + VariableKind::Unknown => Ok(None), + } +} + +/// The elements among `n`'s children. +fn elements<'a, 'i>(n: roxmltree::Node<'a, 'i>) -> impl Iterator> { + n.children().filter(roxmltree::Node::is_element) +} + +/// The text of every text node inside `n`, in order. +fn deep_text(n: roxmltree::Node) -> String { + n.descendants().filter(roxmltree::Node::is_text).filter_map(|t| t.text()).collect() +} + +/// The absolute path a library's image value `text` names: a `file:` URL's path (`file:///…`, +/// `file://localhost/…` or `file:/…`), its `%XX` escapes decoded (a URL with a host names a share, +/// `//host/…`), or `text` itself. `None` unless that is an absolute path: one starting with `/` or +/// `\\`, or a drive's path from its root (`C:\…`, `C:/…`). +fn library_path(text: &str) -> Option { + let drive = |p: &str| matches!(p.as_bytes(), [l, b':', b'\\' | b'/', ..] if l.is_ascii_alphabetic()); + let url = match (text.strip_prefix("file://"), text.strip_prefix("file:")) { + (Some(rest), _) => Some(rest.strip_prefix("localhost").filter(|r| r.starts_with('/')).unwrap_or(rest)), + // `file:/…` names a path without a host. + (None, Some(rest)) if rest.starts_with('/') => Some(rest), + _ => None, + }; + let path = match url { + Some(rest) => { + if rest.starts_with('/') { + let p = percent_decoded(rest.trim_start_matches('/')); + if drive(&p) { p } else { format!("/{p}") } + } else { + format!("//{}", percent_decoded(rest)) + } + } + None => text.to_string(), + }; + (path.len() > 1 && (path.starts_with('/') || path.starts_with("\\\\") || drive(&path))).then_some(path) +} + +/// `s` with its `%XX` escapes decoded, or as it is when the decoded bytes aren't UTF-8. +fn percent_decoded(s: &str) -> String { + let hex = |b: u8| char::from(b).to_digit(16); + let (mut out, mut rest) = (Vec::with_capacity(s.len()), s.as_bytes()); + while let Some((&b, tail)) = rest.split_first() { + match tail { + [h, l, more @ ..] if b == b'%' && hex(*h).is_some() && hex(*l).is_some() => { + out.push(((hex(*h).unwrap_or(0) << 4) | hex(*l).unwrap_or(0)) as u8); + rest = more; + } + _ => { + out.push(b); + rest = tail; + } + } + } + String::from_utf8(out).unwrap_or_else(|_| s.to_string()) +} + +/// How a library writes the image value `path`: an absolute path from the root of a Unix file +/// system as a `file:` URL (`file:///` and the path, with `%`, `#`, `?`, spaces, controls and +/// bytes past ASCII escaped), any other path as it is. [`library_path`] reads it back. +fn library_file(path: &str) -> String { + if !path.starts_with('/') || path.starts_with("//") { + return path.to_string(); + } + let mut url = String::from("file:///"); + for &b in path.as_bytes() { + if matches!(b, b'%' | b'#' | b'?') || !(0x21..=0x7e).contains(&b) { + let _ = write!(url, "%{b:02X}"); + } else { + url.push(char::from(b)); + } + } + url +} + +/// A library's graph value: the `data` element that `el` holds, read as CSV of the kind Graph +/// Data takes. Its `propertyRow` holds a corner cell and then the names of the series, and each +/// `row` of its `values` holds the row's category in a first `value` whose `key` is `name`, then +/// the row's values. The graph has categories when a row's name isn't blank or is marked +/// `type="string"`. Labels are read as they are; a value is read without the white space round it, +/// and one that is empty, marked `type="string"` or not a finite number is a blank. Without rows or +/// series, or when `el` holds only white space, there is no value. Anything else in `el`, more than +/// [`MAX_GRAPH_CATEGORIES`] rows or [`MAX_GRAPH_SERIES`] series (what a graph holds), and CSV longer +/// than graph values hold, are refused. +fn graph_value(el: roxmltree::Node, set: &str, cmd: &str) -> Result> { + let var = el.tag_name().name(); + let refused = || bad(cmd, format!("data set `{set}` holds a value for `{var}` that isn't a graph's `data` element")); + let named = |name: &'static str| move |n: &roxmltree::Node| n.tag_name().name() == name; + let mut children = elements(el); + let data = match (children.next(), children.next()) { + (None, _) if deep_text(el).trim_matches(XML_SPACE).is_empty() => return Ok(None), + (Some(data), None) if data.tag_name().name() == "data" => data, + _ => return Err(refused()), + }; + // Labels keep their white space; numbers are read without it. + let series: Vec = + elements(data).find(named("propertyRow")).map(|r| elements(r).filter(named("value")).skip(1).map(deep_text).collect()).unwrap_or_default(); + let rows: Vec = + elements(data).filter(named("values")).flat_map(elements).filter(named("row")).take(MAX_GRAPH_CATEGORIES + 1).collect(); + if rows.is_empty() && series.is_empty() { + return Ok(None); + } + let too_many = || bad(cmd, format!("data set `{set}` holds a graph value for `{var}` with {}", graph_limits())); + // A row's values after its name. + let wide = |row: &roxmltree::Node| { + let mut values = elements(*row).filter(named("value")).peekable(); + let _name = values.next_if(|v| v.attribute("key") == Some("name")); + values.count() > MAX_GRAPH_SERIES + }; + if rows.len() > MAX_GRAPH_CATEGORIES || series.len() > MAX_GRAPH_SERIES || rows.iter().any(wide) { + return Err(too_many()); + } + // Each row's name and the values after it. A blank name marked `type="string"` is a blank + // category, so a graph whose categories are all blank keeps them. + let mut labelled = false; + let read: Vec<(Option, Vec>)> = rows + .iter() + .map(|row| { + let mut values = elements(*row).filter(named("value")).peekable(); + let name = values.next_if(|v| v.attribute("key") == Some("name")); + labelled |= name.is_some_and(|v| v.attribute("type") == Some("string") || !deep_text(v).is_empty()); + let name = name.map(deep_text); + let number = |v: roxmltree::Node| deep_text(v).trim_matches(XML_SPACE).parse::().ok().filter(|n| n.is_finite()); + let numbers = values.map(|v| if v.attribute("type") == Some("string") { None } else { number(v) }); + (name, numbers.collect()) + }) + .collect(); + let categories = if labelled { read.iter().map(|(name, _)| name.clone().unwrap_or_default()).collect() } else { vec![] }; + let mut spec = GraphSpec { series, categories, ..GraphSpec::default() }; + spec.set_cells(read.into_iter().map(|(_, numbers)| numbers).collect()); + // Data set values hold graph data without the line break at its end. + let csv = spec.to_csv().trim_end().to_string(); + if csv.len() > MAX_GRAPH_CSV { + return Err(bad(cmd, format!("data set `{set}` holds a graph value for `{var}` longer than {MAX_GRAPH_CSV} bytes"))); + } + Ok(Some(DataValue::Graph(csv))) } /// The variable library an SVG carries (SVG Options › Include Variable Data), read as @@ -389,12 +594,16 @@ fn variable_sets_range(svg: &str) -> Option<(usize, Option)> { /// the type object made of the element its `docRef` names, or the one type object among the /// objects a group made of it holds (`found`: the object made of each element, by id). A `docRef` /// that names no type object, or one another variable is bound to already, binds nothing. The art -/// stays as the file draws it. The warning about the variables of other kinds that were left out, -/// if any. +/// stays as the file draws it. Variables of other kinds are left out with their values. The +/// warning about those variables, if any. pub(crate) fn give_svg_library(doc: &mut Document, lib: Library, found: &HashMap) -> Option { + let (text, other): (Vec, Vec) = lib.variables.into_iter().partition(|v| v.kind == VariableKind::Text); let mut vars = - Variables { variables: lib.text.iter().map(|n| Variable { name: n.clone(), kind: VariableKind::Text }).collect(), ..Variables::default() }; + Variables { variables: text.iter().map(|v| Variable { name: v.name.clone(), kind: VariableKind::Text }).collect(), ..Variables::default() }; vars.datasets = lib.datasets; + for ds in &mut vars.datasets { + ds.values.retain(|_, v| matches!(v, DataValue::Text(_))); + } // A type object written with a paint of its own, or with fills and strokes of its appearance, // is a group around its text (and the appearance's paths). let text_of = |id: NodeId| match doc.node(id).map(|n| &n.kind) { @@ -415,32 +624,32 @@ pub(crate) fn give_svg_library(doc: &mut Document, lib: Library, found: &HashMap } } doc.variables = vars.into(); - left_out_warning(lib.other.iter().map(|(_, t)| t.as_str())) + left_out_warning(other.iter().map(|v| v.trait_name.as_deref().unwrap_or_else(|| v.kind.label()))) } /// Load Variable Library: replace the document's variables and data sets with a variable library's /// ([`parse_library`]) in one undo step, or none when the document already holds what the library /// gives. Every data set is deleted, and every variable of the document is deleted with its -/// bindings and values, except a text variable the library also declares: it stays bound to its -/// objects, so the library's data sets change them (#4136; a template's bound variables take a -/// library of the same names: +/// bindings and values, except a variable the library also declares with the same kind: it stays +/// bound to its objects, so the library's data sets change them (#4136; a template's bound +/// variables take a library of the same names: /// ). The -/// library's other text variables are then defined, unbound, and its data sets are added. The -/// library's variables of other kinds are left out, named in `skipped` and in a warning. +/// library's other variables are then defined with their kinds, unbound, and its data sets are +/// added. pub(super) fn load_library(s: &mut Session, p: &Value) -> Result { const C: &str = "variable.loadLibrary"; let bytes = library_bytes(p, C)?; let xml = std::str::from_utf8(bytes.strip_prefix(b"\xEF\xBB\xBF").unwrap_or(&bytes)).map_err(|_| bad(C, "the variable library is not UTF-8"))?; let lib = parse_library(xml, C)?; let vars = &s.doc()?.doc.variables; - let kept = |v: &Variable| v.kind == VariableKind::Text && lib.text.contains(&v.name); + let kept = |v: &Variable| lib.variables.iter().any(|l| l.name == v.name && l.kind == v.kind); let deleted: Vec = vars.variables.iter().filter(|v| !kept(v)).map(|v| v.name.clone()).collect(); let deleted_sets: Vec = vars.datasets.iter().map(|d| d.name.clone()).collect(); let added: Vec = lib - .text + .variables .iter() - .filter(|n| !vars.variables.iter().any(|v| kept(v) && &v.name == *n)) - .map(|n| Variable { name: n.clone(), kind: VariableKind::Text }) + .filter(|l| !vars.variables.iter().any(|v| kept(v) && v.name == l.name)) + .map(|l| Variable { name: l.name.clone(), kind: l.kind }) .collect(); // Lock Variables refuses New Variable and Delete Variable, so the load is refused when it would // delete or define a variable. @@ -460,14 +669,8 @@ pub(super) fn load_library(s: &mut Session, p: &Value) -> Result { let st = s.doc_mut()?; st.drop_stale_variables_highlight(); st.revision += 1; - let skipped: Vec = lib.other.iter().map(|(name, t)| json!({"name": name, "trait": t})).collect(); - let mut out = json!({ - "variables": lib.text, "deleted": deleted, "deletedDataSets": deleted_sets, "names": names, "skipped": skipped, - }); - if let Some(warning) = left_out_warning(lib.other.iter().map(|(_, t)| t.as_str())) { - out["warning"] = json!(warning); - } - Ok(out) + let declared: Vec<&str> = lib.variables.iter().map(|l| l.name.as_str()).collect(); + Ok(json!({"variables": declared, "deleted": deleted, "deletedDataSets": deleted_sets, "names": names})) } /// The warning about variables that aren't text and were left out, given the kind of each: every @@ -533,26 +736,95 @@ fn push_escaped(out: &mut String, c: char, attribute: bool) { } } -/// Save Variable Library: the document's text variables and its data sets as a variable library in -/// the layout [`parse_library`] reads, so Load Variable Library reads back the same text variables, -/// data set names and text values. A text value is written as one `p` element per line. Variables -/// of other kinds are left out with their values, named in `skipped` and in a warning. A text -/// variable whose name can't name an element ([`element_name`]), a data set name or text value that -/// holds a character XML can't hold, a name that Load Variable Library would read differently or -/// refuse (white space at either end of a data set name, a name longer than `MAX_NAME` characters, -/// a name used twice), and a library past the caps Load Variable Library reads are refused. With `path` the library is written to the file; without it the reply carries it as -/// `xml`. +/// Add the graph value `csv` of the variable `var` to `xml` as the `data` element [`graph_value`] +/// reads: the CSV read as Graph Data reads it, a `propertyRow` with a blank corner cell and the +/// series names when it names series, and a `row` per category, its name in a first `value` with +/// `key="name"` (blank without categories) and then a `value` per series, blank where the row has +/// none. A label that isn't a number is marked `type="string"`, and so is a blank category. CSV +/// that names series and has no rows is written as the `propertyRow` alone. Refused for CSV with +/// neither rows nor series, which reads back as no value, for more than [`MAX_GRAPH_CATEGORIES`] +/// rows or [`MAX_GRAPH_SERIES`] series (what a graph holds), when a label holds a character XML +/// can't hold, and when `xml` grows past `room` bytes. +fn push_graph(xml: &mut String, var: &str, csv: &str, room: usize) -> std::result::Result<(), GraphRefusal> { + let (series, categories, rows) = super::super::graph::parse_csv(csv); + if rows.is_empty() && series.is_empty() { + return Err(GraphRefusal::Empty); + } + let width = rows.iter().map(Vec::len).max().unwrap_or(0).max(series.len()); + if rows.len() > MAX_GRAPH_CATEGORIES || width > MAX_GRAPH_SERIES { + return Err(GraphRefusal::TooMany); + } + if !series.iter().chain(&categories).all(|l| l.chars().all(xml_char)) { + return Err(GraphRefusal::Control); + } + let labelled = !categories.is_empty(); + let label = |l: &str, category: bool| { + let text = !l.is_empty() && l.trim().parse::().is_err(); + let blank_category = category && labelled && l.is_empty(); + let key = if category { " key=\"name\"" } else { "" }; + let string = if text || blank_category { " type=\"string\"" } else { "" }; + format!("{}", escaped(l, false)) + }; + let _ = writeln!(xml, "\t\t\t\t<{var}>\n\t\t\t\t\t", width + usize::from(labelled)); + if !series.is_empty() { + let names: String = (0..width).map(|s| label(series.get(s).map(String::as_str).unwrap_or_default(), false)).collect(); + let _ = writeln!(xml, "\t\t\t\t\t\t{names}"); + } + xml.push_str("\t\t\t\t\t\t\n"); + for (i, row) in rows.iter().enumerate() { + let name = label(categories.get(i).map(String::as_str).unwrap_or_default(), true); + let values: String = (0..width) + .map(|s| match row.get(s).copied().flatten() { + Some(v) => format!("{v}"), + None => "".to_string(), + }) + .collect(); + let _ = writeln!(xml, "\t\t\t\t\t\t\t{name}{values}"); + if xml.len() > room { + return Err(GraphRefusal::TooLarge); + } + } + let _ = writeln!(xml, "\t\t\t\t\t\t\n\t\t\t\t\t\n\t\t\t\t"); + Ok(()) +} + +/// What a graph holds at most, for the errors about graph data past it. +fn graph_limits() -> String { + format!("more than {MAX_GRAPH_CATEGORIES} rows or {MAX_GRAPH_SERIES} series, which a graph doesn't hold") +} + +/// Why [`push_graph`] refused a graph value. +enum GraphRefusal { + /// A label holds a character XML can't hold. + Control, + /// The data has more rows or series than a graph holds. + TooMany, + /// The data has neither rows nor series. + Empty, + /// The library grew past the room it has. + TooLarge, +} + +/// Save Variable Library: the document's variables and its data sets as a variable library in the +/// layout [`parse_library`] reads, so Load Variable Library reads back the same variables with their +/// kinds, data set names and values. Each variable is declared with the `trait` and `category` of +/// its kind ([`kind_trait`]), and a variable of unknown kind with neither. A text value is written +/// as one `p` element per line, a visibility value as `true` or `false`, an image value as +/// [`library_file`] writes its path, and a graph value as a `data` element ([`push_graph`]). A +/// variable whose name can't name an element ([`element_name`]; a variable of unknown kind has no +/// values, so no element is named after it), a name or value that holds a character XML can't +/// hold, a name that Load Variable Library would read differently or refuse (white space at either +/// end, a name longer than `MAX_NAME` characters, a name used twice), an image value that isn't a +/// file's absolute path, and a library past the caps Load Variable Library reads are refused. With +/// `path` the library is written to the file; without it the reply carries it as `xml`. pub(super) fn save_library(s: &mut Session, p: &Value) -> Result { const C: &str = "variable.saveLibrary"; let vars = &s.doc()?.doc.variables; if vars.variables.is_empty() { return Err(bad(C, "define a variable first")); } - let (text, other): (Vec<&Variable>, Vec<&Variable>) = vars.variables.iter().partition(|v| v.kind == VariableKind::Text); - if text.is_empty() { - return Err(bad(C, "a variable library holds text variables, and the document has none")); - } - if let Some(name) = text.iter().map(|v| &v.name).find(|n| !element_name(n)) { + let all: Vec<&Variable> = vars.variables.iter().collect(); + if let Some(name) = all.iter().filter(|v| v.kind != VariableKind::Unknown).map(|v| &v.name).find(|n| !element_name(n)) { return Err(bad( C, format!( @@ -560,48 +832,62 @@ pub(super) fn save_library(s: &mut Session, p: &Value) -> Result { ), )); } - // Load Variable Library refuses a name longer than `MAX_NAME` characters and a name declared - // twice. + // Load Variable Library reads a name trimmed and refuses one longer than `MAX_NAME` characters + // or declared twice. let mut declared = BTreeSet::new(); - for v in &text { - named(Some(&v.name), "varName", C)?; + for v in &all { + if named(Some(&v.name), "varName", C)? != v.name || !v.name.chars().all(xml_char) { + return Err(bad( + C, + format!("variable `{}` starts or ends with white space or holds a control character, which a variable library can't hold", v.name), + )); + } if !declared.insert(v.name.as_str()) { return Err(bad(C, format!("variable `{}` is already defined", v.name))); } } - if text.len() > MAX_LIBRARY_VARIABLES { + if all.len() > MAX_LIBRARY_VARIABLES { return Err(bad(C, format!("the variable library declares more than {MAX_LIBRARY_VARIABLES} variables"))); } if vars.datasets.len() > MAX_LIBRARY_DATASETS { return Err(bad(C, format!("the variable library declares more than {MAX_LIBRARY_DATASETS} data sets"))); } - fn text_value<'a>(ds: &'a DataSet, var: &str) -> Option<&'a str> { - match ds.values.get(var) { - Some(DataValue::Text(t)) => Some(t.as_str()), - _ => None, - } - } let mut xml = format!( "\n\ \n\ \t\n\ \t\n\ + \t\n\ ]>\n\ \n\ \n\ \t\n\ \t\t\n" ); - for v in &text { - xml += &format!("\t\t\t\n", v.name); + for v in &all { + let name = escaped(&v.name, true); + match kind_trait(v.kind) { + Some((t, category)) => xml += &format!("\t\t\t\n"), + None => xml += &format!("\t\t\t\n"), + } } xml += "\t\t\n\t\t\n"; - // Load Variable Library caps the file and the text its entity references expand to: one - // `&ns_flows;` per variable, two `&ns_vars;` and one `&ns_custom;`. The library stays within - // the cap less what the references add. + // Load Variable Library caps the file and the text its entity references expand to: each + // variable's `category`, two `&ns_vars;` and one `&ns_custom;`. The library stays within the + // cap less what the references add. let grows = |value: &str, reference: &str| value.len().saturating_sub(reference.len()); - let growth = text.len() * grows(NS_FLOWS, "&ns_flows;") + 2 * grows(NS_VARIABLES, "&ns_vars;") + grows(NS_CUSTOM, "&ns_custom;"); + let growth = all + .iter() + .filter_map(|v| kind_trait(v.kind)) + .map(|(_, category)| match category { + "&ns_flows;" => grows(NS_FLOWS, category), + "&ns_graphs;" => grows(NS_GRAPHS, category), + _ => grows(NS_VARIABLES, category), + }) + .sum::() + + 2 * grows(NS_VARIABLES, "&ns_vars;") + + grows(NS_CUSTOM, "&ns_custom;"); let room = MAX_LIBRARY_BYTES.saturating_sub(growth); let too_large = || bad(C, format!("the variable library is larger than {MAX_LIBRARY_BYTES} bytes")); let mut seen = BTreeSet::new(); @@ -615,25 +901,61 @@ pub(super) fn save_library(s: &mut Session, p: &Value) -> Result { if !seen.insert(name.as_str()) { return Err(bad(C, format!("dataset `{name}` already exists"))); } - if !name.chars().all(xml_char) || text.iter().filter_map(|v| text_value(ds, &v.name)).any(|t| !t.chars().all(xml_char)) { - return Err(bad(C, format!("data set `{name}` holds a control character, which a variable library can't hold"))); + let control = || bad(C, format!("data set `{name}` holds a control character, which a variable library can't hold")); + let mut text_values = all.iter().filter_map(|v| match ds.values.get(&v.name) { + Some(DataValue::Text(t)) if v.kind == VariableKind::Text => Some(t), + _ => None, + }); + if !name.chars().all(xml_char) || text_values.any(|t| !t.chars().all(xml_char)) { + return Err(control()); } xml += &format!("\t\t\t\n", escaped(name, true)); - for v in &text { - let Some(t) = text_value(ds, &v.name) else { continue }; - xml += &format!("\t\t\t\t<{}>\n", v.name); - for line in t.split('\n') { - xml += "\t\t\t\t\t

"; - // A value can be larger than the cap, so the text stops growing once it passes it. - for c in line.chars() { - push_escaped(&mut xml, c, false); - if xml.len() > room { - return Err(too_large()); + for v in &all { + let Some(value) = ds.values.get(&v.name).filter(|value| value.fits(v.kind)) else { continue }; + let var = &v.name; + match value { + DataValue::Text(t) => { + xml += &format!("\t\t\t\t<{var}>\n"); + for line in t.split('\n') { + xml += "\t\t\t\t\t

"; + // A value can be larger than the cap, so the text stops growing once it passes it. + for c in line.chars() { + push_escaped(&mut xml, c, false); + if xml.len() > room { + return Err(too_large()); + } + } + xml += "

\n"; } + xml += &format!("\t\t\t\t\n"); } - xml += "

\n"; + DataValue::Visible(shown) => xml += &format!("\t\t\t\t<{var}>{shown}\n"), + DataValue::Image(path) => { + // Load Variable Library reads back only a file's absolute path. + let file = library_file(path); + if library_path(file.trim_matches(XML_SPACE)).as_deref() != Some(path.as_str()) { + return Err(bad(C, format!("data set `{name}` holds an image value for `{var}` that isn't a file's absolute path"))); + } + if !file.chars().all(xml_char) { + return Err(control()); + } + xml += &format!("\t\t\t\t<{var}>{}\n", escaped(&file, false)); + } + DataValue::Graph(csv) => match push_graph(&mut xml, var, csv, room) { + Ok(()) => {} + Err(GraphRefusal::Control) => return Err(control()), + Err(GraphRefusal::TooMany) => { + return Err(bad(C, format!("data set `{name}` holds a graph value for `{var}` with {}", graph_limits()))); + } + Err(GraphRefusal::Empty) => { + return Err(bad(C, format!("data set `{name}` holds a graph value for `{var}` without rows or series"))); + } + Err(GraphRefusal::TooLarge) => return Err(too_large()), + }, + } + if xml.len() > room { + return Err(too_large()); } - xml += &format!("\t\t\t\t\n", v.name); } xml += "\t\t\t\n"; if xml.len() > room { @@ -644,13 +966,7 @@ pub(super) fn save_library(s: &mut Session, p: &Value) -> Result { if xml.len() > room { return Err(too_large()); } - let skipped: Vec = other.iter().map(|v| json!({"name": v.name, "kind": v.kind.label()})).collect(); - let mut out = json!({ - "variables": text.iter().map(|v| v.name.as_str()).collect::>(), "datasets": vars.datasets.len(), "skipped": skipped, - }); - if let Some(warning) = left_out_warning(other.iter().map(|v| v.kind.label())) { - out["warning"] = json!(warning); - } + let mut out = json!({"variables": all.iter().map(|v| v.name.as_str()).collect::>(), "datasets": vars.datasets.len()}); match str_param(p, "path") { Some(path) => { let path = absolute_path(path); diff --git a/crates/engine/src/tests/variables.rs b/crates/engine/src/tests/variables.rs index 6256088842..64d5923b9c 100644 --- a/crates/engine/src/tests/variables.rs +++ b/crates/engine/src/tests/variables.rs @@ -2284,7 +2284,7 @@ fn a_variable_library_defines_text_variables_and_data_sets() { assert_eq!(r["variables"], json!(["Name", "Note"])); assert_eq!((r["deleted"].clone(), r["deletedDataSets"].clone()), (json!([]), json!([]))); assert_eq!(r["names"], json!(["Row 1", "Row 2"])); - assert_eq!((r["skipped"].clone(), r.get("warning")), (json!([]), None)); + assert_eq!((r.get("skipped"), r.get("warning")), (None, None)); let listed = s.execute("variable.list", &json!({})).unwrap(); assert_eq!(listed["variables"], json!([{"name": "Name", "kind": "text", "bindings": 0}, {"name": "Note", "kind": "text", "bindings": 0}])); let sets = s.execute("dataset.list", &json!({})).unwrap(); @@ -2405,13 +2405,14 @@ fn an_import_or_library_load_that_changes_nothing_records_no_undo_step() { s.execute("variable.loadLibrary", &json!({"xml": other})).unwrap(); assert_eq!(super::history(&s), (undo + 1, 0, true)); - // While the variables are locked, a library without text variables still loads its data sets, - // and loading it again changes nothing. + // While the variables are locked, a library of the variables the document has, with their + // kinds, still loads its data sets, and loading it again changes nothing. let mut s = session(); let t = text(&mut s, "Alice"); - s.execute("variable.lock", &json!({"locked": true})).unwrap(); let xml = library(&[("Shown", "other")], &[("B", &[])]); s.execute("variable.loadLibrary", &json!({"xml": xml})).unwrap(); + s.execute("variable.lock", &json!({"locked": true})).unwrap(); + s.execute("variable.loadLibrary", &json!({"xml": xml})).unwrap(); super::select_with_a_step_to_redo(&mut s, &[t]); let before = super::history(&s); s.execute("variable.loadLibrary", &json!({"xml": xml})).unwrap(); @@ -2432,22 +2433,89 @@ fn a_refused_variable_library_deletes_nothing() { assert_eq!(s.doc().unwrap().doc.variables, before); } -/// A library's variables of other kinds are left out with their values, named in `skipped` and -/// in a warning that gives each kind once with its count; its text variables load. +/// A library of every kind, in the layout the public grammar gives: visibility (`visibility`), +/// image (`fileref`) and graph (`graphdata`) variables beside a text variable, a variable without a +/// `trait` and one whose `trait` VectorCraft doesn't know, with values in the forms +/// [`a_variable_library_defines_every_kind_with_its_values`] reads. +fn every_kind_library() -> String { + "\n\ + ]>\n\ + \ + \ + \ + \ + \ +

Bravo

true\ + \n\t\tfile:////Users/me/Art%20Work/logo%23one.png\n\t\t\ + NorthSouth\ + Q1102.5\ + 2024-4\ + ignored\ + FALSE C:\\Art\\logo.png\ + 78\ + \n\ + \n" + .to_string() +} + +/// Load Variable Library defines a library's variables of every kind, unbound, with their values: +/// a visibility value from `true` or `false` in any case, an image value from a file's absolute +/// path or a `file:` URL (escapes decoded, the white space round it left out, the file not read), +/// and a graph value from a `data` element, as the CSV Graph Data takes (series from its +/// `propertyRow`, a category from each row's `key="name"` value, blanks where a row has no +/// number). A variable without a `trait`, or with one VectorCraft doesn't know, is defined of +/// unknown kind, without values. An element holding only white space gives no value. #[test] -fn a_variable_library_leaves_out_variables_that_are_not_text() { +fn a_variable_library_defines_every_kind_with_its_values() { let mut s = session(); - let xml = library( - &[("Shown", "kindB"), ("Name", "textcontent"), ("Photo", "kindC"), ("Other", "kindB")], - &[("A", &[("Shown", &["true"]), ("Name", &["Bravo"]), ("Photo", &["a.png"])])], - ); - let r = s.execute("variable.loadLibrary", &json!({"xml": xml})).unwrap(); - assert_eq!(r["variables"], json!(["Name"])); - assert_eq!(r["skipped"], json!([{"name": "Shown", "trait": "kindB"}, {"name": "Photo", "trait": "kindC"}, {"name": "Other", "trait": "kindB"}])); - assert_eq!(r["warning"], "variables that aren't text were left out: kindB (2), kindC (1)"); + let r = s.execute("variable.loadLibrary", &json!({"xml": every_kind_library()})).unwrap(); + assert_eq!(r["variables"], json!(["Name", "Shown", "Photo", "Chart", "Loose", "Odd"])); let vars = &s.doc().unwrap().doc.variables; - assert_eq!(vars.variables.len(), 1); - assert_eq!(vars.dataset("A").map(|d| d.values.len()), Some(1)); + let kinds: Vec<(&str, &str)> = vars.variables.iter().map(|v| (v.name.as_str(), v.kind.label())).collect(); + assert_eq!(kinds, [("Name", "text"), ("Shown", "visibility"), ("Photo", "image"), ("Chart", "graph"), ("Loose", "unknown"), ("Odd", "unknown")]); + assert!(vars.bindings.is_empty()); + use vectorcraft_doc::DataValue::{Graph, Image, Text, Visible}; + let a = &vars.dataset("A").unwrap().values; + assert_eq!(a.get("Name"), Some(&Text("Bravo".into()))); + assert_eq!(a.get("Shown"), Some(&Visible(true))); + assert_eq!(a.get("Photo"), Some(&Image("/Users/me/Art Work/logo#one.png".into()))); + assert_eq!(a.get("Chart"), Some(&Graph(",North,South\nQ1,10,2.5\n\"2024\",,-4".into()))); + assert_eq!((a.get("Loose"), a.get("Odd")), (None, None)); + let b = &vars.dataset("B").unwrap().values; + assert_eq!((b.get("Shown"), b.get("Photo")), (Some(&Visible(false)), Some(&Image("C:\\Art\\logo.png".into())))); + assert_eq!(b.get("Chart"), Some(&Graph(",,\n7,8".into()))); + assert!(vars.dataset("C").unwrap().values.is_empty(), "{:?}", vars.dataset("C")); +} + +/// A library's visibility, image and graph variables drive the art they are bound to: a variable +/// the document already has with the same name and kind stays bound (#4136), so selecting a +/// library's data set shows or hides the object, swaps the linked image's path and replaces the +/// graph's data. A variable of the same name and another kind is replaced, and its bindings go. +#[test] +fn a_variable_library_keeps_bound_variables_of_every_kind() { + let mut s = session(); + let r = rect(&mut s); + let graph = s.execute("graph.create", &json!({"type": "column", "x": 10, "y": 10, "width": 120, "height": 80, "csv": ",a\nX,1"})).unwrap()["id"] + .as_u64() + .unwrap(); + for (name, kind, id) in [("Shown", "visibility", r.0), ("Chart", "graph", graph), ("Photo", "text", r.0)] { + s.execute("variable.define", &json!({"name": name, "kind": kind})).unwrap(); + if kind != "text" { + s.execute("variable.bind", &json!({"variable": name, "ids": [id]})).unwrap(); + } + } + let loaded = s.execute("variable.loadLibrary", &json!({"xml": every_kind_library()})).unwrap(); + assert_eq!(loaded["deleted"], json!(["Photo"]), "Photo is an image variable in the library"); + let listed = s.execute("variable.list", &json!({})).unwrap(); + let bindings: Vec<(String, u64)> = + listed["variables"].as_array().unwrap().iter().map(|v| (v["name"].as_str().unwrap().to_string(), v["bindings"].as_u64().unwrap())).collect(); + assert_eq!(bindings.iter().find(|(n, _)| n == "Shown").map(|b| b.1), Some(1)); + assert_eq!(bindings.iter().find(|(n, _)| n == "Chart").map(|b| b.1), Some(1)); + s.execute("dataset.select", &json!({"name": "B"})).unwrap(); + let doc = &s.doc().unwrap().doc; + assert!(!doc.node(r).unwrap().visible, "Shown is false in B"); + let csv = doc.node(NodeId(graph)).and_then(|n| n.extras().graph.as_ref()).map(|g| g.to_csv()); + assert_eq!(csv.as_deref(), Some(",,\n7,8\n"), "B's graph data"); } /// A library that isn't one, or that breaks its grammar or the caps, is refused, and the document @@ -2477,12 +2545,36 @@ fn malformed_variable_libraries_are_refused() { "&big;".repeat(8000), " ".repeat(300_000) ); + // A library of one variable `Shown` of the trait given, and a data set holding `value`. + let kind_value = |t: &str, value: &str| { + format!( + "\ + {value}" + ) + }; + // A library graph value of more rows or series than a graph holds. + let rows_past = format!("{}", "1".repeat(10_001)); + let series_past = + format!("Q{}", "1".repeat(257)); + // 2,000 rows of 20 values of eight digits are more CSV than a graph value holds. + let row = format!("{}", "12345678".repeat(20)); + let long_graph = format!("{}", row.repeat(2_000)); let cases = [ ("not well-formed XML", "".to_string()), ("no `variableSets` element", "".to_string()), ("declares no variables", "".to_string()), ("is declared twice", library(&[("Name", "textcontent"), ("Name", "textcontent")], &[])), - ("has no `trait`", ok.replace(" trait=\"textcontent\"", "")), + ("for `Shown` that isn't `true` or `false`", kind_value("visibility", "maybe")), + ("for `Shown` that isn't `true` or `false`", kind_value("visibility", "

true

")), + ("for `Shown` that isn't a file's absolute path or `file:` URL", kind_value("fileref", "logo.png")), + ("for `Shown` that isn't a file's absolute path or `file:` URL", kind_value("fileref", "C:logo.png")), + ("for `Shown` that isn't a file's absolute path or `file:` URL", kind_value("fileref", "file:logo.png")), + ("for `Shown` that isn't a graph's `data` element", kind_value("graphdata", "1,2")), + ("for `Shown` that isn't a graph's `data` element", kind_value("graphdata", "")), + ("for `Shown` that isn't a graph's `data` element", kind_value("graphdata", "")), + ("graph value for `Shown` longer than 262144 bytes", kind_value("graphdata", &long_graph)), + ("graph value for `Shown` with more than 10000 rows or 256 series", kind_value("graphdata", &rows_past)), + ("graph value for `Shown` with more than 10000 rows or 256 series", kind_value("graphdata", &series_past)), ("missing `varName`", ok.replace("varName=\"Name\"", "varName=\" \"")), ("missing `dataSetName`", ok.replace(" dataSetName=\"A\"", "")), ("data set `A` is declared twice", library(&[("Name", "textcontent")], &[("A", &[]), ("A", &[])])), @@ -2531,51 +2623,50 @@ fn a_variable_library_expands_entities_of_plain_text() { assert_eq!(value, Some(vectorcraft_doc::DataValue::Text("x-y \"z\"-x&a;".into()))); } -/// Save Variable Library writes what Load Variable Library reads back: the text variables in their -/// order, every data set by name and in order, and each text value as it was, with several lines, -/// markup characters, quotes, a carriage return and an empty value. Variables of other kinds are -/// left out with their values, named in `skipped` and in the warning. The library is written while -/// the variables are locked too, to a file with `path`. +/// Save Variable Library writes what Load Variable Library reads back: the variables of every kind +/// in their order, every data set by name and in order, and each value as it was: text with several +/// lines, markup characters, quotes, a carriage return and an empty value; visibility values; +/// image paths with spaces, markup characters, `%`, `#` and letters past ASCII, from a Unix root, +/// a drive and a share; and graph data with series, categories, a numeric category and blanks. A +/// variable of unknown kind is declared without a `trait` and loads back as one. The library is +/// written while the variables are locked too, to a file with `path`. #[test] -fn a_saved_variable_library_loads_back_the_text_variables_and_data_sets() { +fn a_saved_variable_library_loads_back_every_variable_and_data_set() { let mut s = session(); let t = text(&mut s, "Alice"); - for (name, kind) in [("Name", "text"), ("Shown", "visibility"), ("Note", "text"), ("Photo", "image"), ("Badge", "visibility")] { + for (name, kind) in [("Name", "text"), ("Shown", "visibility"), ("Note", "text"), ("Photo", "image"), ("Chart", "graph"), ("Badge", "visibility")] + { s.execute("variable.define", &json!({"name": name, "kind": kind})).unwrap(); } s.execute("variable.bind", &json!({"variable": "Name", "ids": [t.0]})).unwrap(); - s.execute("dataset.new", &json!({"name": "First", "values": {"Name": "Bravo", "Note": "One\nTwo & \r\n'four' \"five\"", "Shown": true}})) - .unwrap(); - s.execute("dataset.new", &json!({"name": "Say \"hi\" & \tnow", "values": {"Name": "", "Photo": "a.png"}})).unwrap(); + let chart = ",North,\"S, \"\"q\"\"\"\nQ1,10,2.5\n\"2024\",,-4"; + let first = json!({"Name": "Bravo", "Note": "One\nTwo & \r\n'four' \"five\"", "Shown": true, "Photo": "/Users/me/Art & 100%/#1 ü.png", "Chart": chart}); + s.execute("dataset.new", &json!({"name": "First", "values": first})).unwrap(); + let second = json!({"Name": "", "Photo": "C:\\Art\\logo.png", "Badge": false, "Chart": ",,\n7,8"}); + s.execute("dataset.new", &json!({"name": "Say \"hi\" & \tnow", "values": second})).unwrap(); + s.execute("dataset.new", &json!({"name": "Share", "values": {"Photo": "\\\\server\\share\\a.png"}})).unwrap(); s.execute("dataset.new", &json!({"name": "Empty"})).unwrap(); + // A file can declare a variable of unknown kind, which no command defines. + std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().doc) + .variables + .variables + .push(vectorcraft_doc::Variable { name: "Loose name".into(), kind: vectorcraft_doc::VariableKind::Unknown }); s.execute("variable.lock", &json!({"locked": true})).unwrap(); let before = s.doc().unwrap().doc.variables.clone(); let saved = s.execute("variable.saveLibrary", &json!({})).unwrap(); - assert_eq!(saved["variables"], json!(["Name", "Note"])); - assert_eq!(saved["datasets"], json!(3)); - assert_eq!( - saved["skipped"], - json!([{"name": "Shown", "kind": "visibility"}, {"name": "Photo", "kind": "image"}, {"name": "Badge", "kind": "visibility"}]) - ); - assert_eq!(saved["warning"], "variables that aren't text were left out: visibility (2), image (1)"); + assert_eq!(saved["variables"], json!(["Name", "Shown", "Note", "Photo", "Chart", "Badge", "Loose name"])); + assert_eq!(saved["datasets"], json!(4)); + assert_eq!((saved.get("skipped"), saved.get("warning")), (None, None)); assert_eq!(s.doc().unwrap().doc.variables, before, "saving changes nothing"); let mut other = session(); - let loaded = other.execute("variable.loadLibrary", &json!({"xml": saved["xml"]})).unwrap(); - assert_eq!((loaded["skipped"].clone(), loaded.get("warning")), (json!([]), None)); + other.execute("variable.loadLibrary", &json!({"xml": saved["xml"]})).unwrap(); let vars = &other.doc().unwrap().doc.variables; - assert_eq!(vars.variables.iter().map(|v| (v.name.as_str(), v.kind.label())).collect::>(), [("Name", "text"), ("Note", "text")]); - let text_only: Vec = before - .datasets - .iter() - .map(|d| { - let mut d = d.clone(); - d.values.retain(|k, _| k == "Name" || k == "Note"); - d - }) - .collect(); - assert_eq!(vars.datasets, text_only); + assert_eq!(vars.variables, before.variables); + assert_eq!(vars.datasets, before.datasets); + // Saving the loaded library again writes the same file. + assert_eq!(other.execute("variable.saveLibrary", &json!({})).unwrap()["xml"], saved["xml"]); let dir = Folder::new("vars-save-library"); let path = dir.file("library.xml"); @@ -2585,19 +2676,78 @@ fn a_saved_variable_library_loads_back_the_text_variables_and_data_sets() { assert_eq!(std::fs::read_to_string(&path).unwrap(), saved["xml"].as_str().unwrap()); let mut from_file = session(); from_file.execute("variable.loadLibrary", &json!({"path": path})).unwrap(); - assert_eq!(from_file.doc().unwrap().doc.variables.datasets, text_only); + assert_eq!(from_file.doc().unwrap().doc.variables.datasets, before.datasets); +} + +/// Graph data whose categories are all blank keeps them through a save and a load, and so does +/// graph data of series names without rows; graph data with neither is refused. An image value +/// written as a `file:/…` URL without a host loads as its path, graph data with more series than a +/// graph holds is refused, and wide graph data is checked against the size cap as it is written. +#[test] +fn saved_graph_values_and_file_urls_load_back_at_their_edges() { + let mut s = session(); + s.execute("variable.define", &json!({"name": "Chart", "kind": "graph"})).unwrap(); + s.execute("dataset.new", &json!({"name": "A", "values": {"Chart": ",S\n\"\",1\n\"\",2"}})).unwrap(); + let xml = s.execute("variable.saveLibrary", &json!({})).unwrap()["xml"].as_str().unwrap().to_string(); + assert!(xml.contains("1"), "{xml}"); + let mut other = session(); + other.execute("variable.loadLibrary", &json!({"xml": xml})).unwrap(); + assert_eq!(other.doc().unwrap().doc.variables.datasets, s.doc().unwrap().doc.variables.datasets); + // Series names without rows load back, and graph data with neither is refused. + s.execute("dataset.set", &json!({"name": "A", "values": {"Chart": ",North,South"}})).unwrap(); + let xml = s.execute("variable.saveLibrary", &json!({})).unwrap()["xml"].clone(); + let mut other = session(); + other.execute("variable.loadLibrary", &json!({"xml": xml})).unwrap(); + assert_eq!(other.doc().unwrap().doc.variables.datasets, s.doc().unwrap().doc.variables.datasets); + s.execute("dataset.set", &json!({"name": "A", "values": {"Chart": ","}})).unwrap(); + let e = s.execute("variable.saveLibrary", &json!({})).unwrap_err().to_string(); + assert!(e.contains("graph value for `Chart` without rows or series"), "{e}"); + + let url = "\ + file:/tmp/logo%20one.png\ + file:/C:/Art/logo.png"; + let mut s = session(); + s.execute("variable.loadLibrary", &json!({"xml": url})).unwrap(); + let image = |set: &str| s.doc().unwrap().doc.variables.dataset(set).and_then(|d| d.values.get("Photo")).cloned(); + assert_eq!(image("A"), Some(vectorcraft_doc::DataValue::Image("/tmp/logo one.png".into()))); + assert_eq!(image("B"), Some(vectorcraft_doc::DataValue::Image("C:/Art/logo.png".into()))); + + // More series than a graph holds are refused. + let mut s = session(); + s.execute("variable.define", &json!({"name": "Chart", "kind": "graph"})).unwrap(); + let wide = (0..257).map(|i| i.to_string()).collect::>().join(","); + s.execute("dataset.new", &json!({"name": "A", "values": {"Chart": wide}})).unwrap(); + let e = s.execute("variable.saveLibrary", &json!({})).unwrap_err().to_string(); + assert!(e.contains("graph value for `Chart` with more than 10000 rows or 256 series"), "{e}"); + // One row of 256 values and 1,000 rows of one: every row is written 256 values wide. + let mut csv = (0..256).map(|i| i.to_string()).collect::>().join(","); + for i in 0..1_000 { + csv += &format!("\n{i}"); + } + s.execute("dataset.set", &json!({"name": "A", "values": {"Chart": csv.clone()}})).unwrap(); + let xml = s.execute("variable.saveLibrary", &json!({})).unwrap()["xml"].as_str().unwrap().to_string(); + assert_eq!(xml.matches("").count(), 1_001); + assert!(xml.contains("")); + // Rows that make the library larger than 8 MiB are refused as each is written. + for i in 1_000..9_000 { + csv += &format!("\n{i}"); + } + s.execute("dataset.set", &json!({"name": "A", "values": {"Chart": csv}})).unwrap(); + let e = s.execute("variable.saveLibrary", &json!({})).unwrap_err().to_string(); + assert!(e.contains("larger than 8388608 bytes"), "{e}"); } /// The library is the XML of the public grammar: a document type declaration with the namespace -/// entities, `variableSets` › `variableSet` › `variables` with a `textcontent` variable per text -/// variable, and `sampleDataSets` with a `sampleDataSet` per data set, holding an element per text -/// variable it has a value for, with a `p` element per line. Markup characters, a carriage return, -/// and a tab or quote in a data set name are escaped. A document with text variables and no data -/// sets writes an empty `sampleDataSets`. +/// entities, `variableSets` › `variableSet` › `variables` with a variable per variable, its `trait` +/// and `category` those of its kind, and `sampleDataSets` with a `sampleDataSet` per data set, +/// holding an element per variable it has a value for: a `p` element per line of a text value, +/// `true` or `false`, a `file:` URL for an image's path from a Unix root, and a `data` element for +/// graph data. Markup characters, a carriage return, and a tab or quote in a data set name are +/// escaped. A document with variables and no data sets writes an empty `sampleDataSets`. #[test] fn a_saved_variable_library_has_the_library_layout() { let mut s = session(); - for (name, kind) in [("Name", "text"), ("Shown", "visibility"), ("Note", "text")] { + for (name, kind) in [("Name", "text"), ("Shown", "visibility"), ("Note", "text"), ("Photo", "image"), ("Chart", "graph")] { s.execute("variable.define", &json!({"name": name, "kind": kind})).unwrap(); } let head = "\n\ @@ -2605,13 +2755,17 @@ fn a_saved_variable_library_has_the_library_layout() { \t\n\ \t\n\ \t\n\ + \t\n\ ]>\n\ \n\ \n\ \t\n\ \t\t\n\ \t\t\t\n\ + \t\t\t\n\ \t\t\t\n\ + \t\t\t\n\ + \t\t\t\n\ \t\t\n\ \t\t\n"; let tail = "\t\t\n\t\n\n\n"; @@ -2619,7 +2773,8 @@ fn a_saved_variable_library_has_the_library_layout() { assert_eq!(empty["xml"].as_str().unwrap(), format!("{head}{tail}")); assert_eq!(empty["datasets"], json!(0)); - s.execute("dataset.new", &json!({"name": "A &\t\"B\"", "values": {"Name": "x\r\ny < z", "Shown": false}})).unwrap(); + let values = json!({"Name": "x\r\ny < z", "Shown": false, "Photo": "/art/a b.png", "Chart": ",S\nQ1,1\n2,"}); + s.execute("dataset.new", &json!({"name": "A &\t\"B\"", "values": values})).unwrap(); s.execute("dataset.new", &json!({"name": "C", "values": {"Note": ""}})).unwrap(); let xml = s.execute("variable.saveLibrary", &json!({})).unwrap()["xml"].as_str().unwrap().to_string(); let sets = "\t\t\t\n\ @@ -2627,6 +2782,17 @@ fn a_saved_variable_library_has_the_library_layout() { \t\t\t\t\t

x

\n\ \t\t\t\t\t

y < z

\n\ \t\t\t\t\n\ + \t\t\t\tfalse\n\ + \t\t\t\tfile:////art/a%20b.png\n\ + \t\t\t\t\n\ + \t\t\t\t\t\n\ + \t\t\t\t\t\tS\n\ + \t\t\t\t\t\t\n\ + \t\t\t\t\t\t\tQ11\n\ + \t\t\t\t\t\t\t2\n\ + \t\t\t\t\t\t\n\ + \t\t\t\t\t\n\ + \t\t\t\t\n\ \t\t\t\n\ \t\t\t\n\ \t\t\t\t\n\ @@ -2636,10 +2802,11 @@ fn a_saved_variable_library_has_the_library_layout() { assert_eq!(xml, format!("{head}{sets}{tail}")); } -/// Save Variable Library refuses a document without variables or without text variables, a text -/// variable whose name can't name an element, a data set name or text value with a character XML -/// can't hold, a name that Load Variable Library would read differently or refuse, and a library -/// past the caps that Load Variable Library reads. A refused save writes no file. +/// Save Variable Library refuses a document without variables, a variable whose name can't name an +/// element (but for a variable of unknown kind), a data set name, text value or graph label with a +/// character XML can't hold, an image value that isn't a file's absolute path, a name that Load +/// Variable Library would read differently or refuse, and a library past the caps that Load +/// Variable Library reads. A refused save writes no file. #[test] fn save_variable_library_refuses_what_a_library_cant_hold() { let dir = Folder::new("vars-save-refused"); @@ -2652,14 +2819,41 @@ fn save_variable_library_refuses_what_a_library_cant_hold() { let mut s = session(); refused(&mut s, "define a variable first"); + // A library without text variables is saved. s.execute("variable.define", &json!({"name": "Shown", "kind": "visibility"})).unwrap(); - refused(&mut s, "holds text variables, and the document has none"); + assert_eq!(s.execute("variable.saveLibrary", &json!({})).unwrap()["variables"], json!(["Shown"])); - for name in ["First Name", "1st", "a:b", "x/y", "-dash"] { + for (name, kind) in [("First Name", "text"), ("1st", "visibility"), ("a:b", "image"), ("x/y", "graph"), ("-dash", "text")] { let mut s = session(); - s.execute("variable.define", &json!({"name": name, "kind": "text"})).unwrap(); + s.execute("variable.define", &json!({"name": name, "kind": kind})).unwrap(); refused(&mut s, &format!("variable `{name}` can't be saved in a variable library")); } + // No element is named after a variable of unknown kind, so its name can be any name. + let mut s = session(); + std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().doc) + .variables + .variables + .push(vectorcraft_doc::Variable { name: "First \"Name\" & ".into(), kind: vectorcraft_doc::VariableKind::Unknown }); + let xml = s.execute("variable.saveLibrary", &json!({})).unwrap()["xml"].clone(); + let mut other = session(); + other.execute("variable.loadLibrary", &json!({"xml": xml})).unwrap(); + assert_eq!(other.doc().unwrap().doc.variables.variables, s.doc().unwrap().doc.variables.variables); + + // An image value that isn't a file's absolute path, which Load Variable Library refuses. + for path in ["a.png", "C:a.png", "C:\\a.png ", "/"] { + let mut s = session(); + s.execute("variable.define", &json!({"name": "Photo", "kind": "image"})).unwrap(); + std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().doc).variables.datasets.push(vectorcraft_doc::DataSet { + name: "A".into(), + values: [("Photo".to_string(), vectorcraft_doc::DataValue::Image(path.into()))].into(), + }); + refused(&mut s, "data set `A` holds an image value for `Photo` that isn't a file's absolute path"); + } + // A graph label with a character XML can't hold. + let mut s = session(); + s.execute("variable.define", &json!({"name": "Chart", "kind": "graph"})).unwrap(); + s.execute("dataset.new", &json!({"name": "A", "values": {"Chart": ",S\u{1}\nQ1,1"}})).unwrap(); + refused(&mut s, "data set `A` holds a control character"); for name in ["Ünïcode", "_x", "a.b-c_d", "名前"] { let mut s = session(); s.execute("variable.define", &json!({"name": name, "kind": "text"})).unwrap(); @@ -2757,10 +2951,10 @@ fn a_saved_variable_library_near_the_size_cap_loads_back() { /// While the variables are locked, a library that would delete or define a variable is refused, as /// Delete Variable and New Variable are, and the document keeps its variables and data sets. That -/// covers a library without text variables loaded into a document with variables, and one that -/// adds a text variable to the document's. A library of the document's own text variables, and -/// one without text variables loaded into a document without variables, delete and define none: -/// they load, replacing the data sets. +/// covers a library of other variables loaded into a document with variables, one that adds a +/// variable to the document's, one that declares a document variable's name with another kind, and +/// any library loaded into a document without variables. A library of the document's own variables +/// with their kinds deletes and defines none: it loads, replacing the data sets. #[test] fn locked_variables_refuse_a_library_that_deletes_or_defines_variables() { let mut s = session(); @@ -2768,7 +2962,7 @@ fn locked_variables_refuse_a_library_that_deletes_or_defines_variables() { s.execute("dataset.new", &json!({"name": "A", "values": {"Name": "Old"}})).unwrap(); s.execute("variable.lock", &json!({"locked": true})).unwrap(); let before = s.doc().unwrap().doc.variables.clone(); - for vars in [&[("Shown", "other")][..], &[("Name", "textcontent"), ("Note", "textcontent")][..]] { + for vars in [&[("Shown", "other")][..], &[("Name", "textcontent"), ("Note", "textcontent")][..], &[("Name", "visibility")][..]] { let e = s.execute("variable.loadLibrary", &json!({"xml": library(vars, &[("B", &[])])})).unwrap_err(); assert!(matches!(&e, EngineError::Disabled(cmd, why) if cmd == "variable.loadLibrary" && why == "variables are locked"), "{e}"); assert_eq!(s.doc().unwrap().doc.variables, before); @@ -2780,13 +2974,13 @@ fn locked_variables_refuse_a_library_that_deletes_or_defines_variables() { let mut s = session(); s.execute("dataset.new", &json!({"name": "A"})).unwrap(); s.execute("variable.lock", &json!({"locked": true})).unwrap(); - let e = s.execute("variable.loadLibrary", &json!({"xml": library(&[("Name", "textcontent")], &[])})).unwrap_err(); - assert!(matches!(&e, EngineError::Disabled(_, why) if why == "variables are locked"), "{e}"); - let r = s.execute("variable.loadLibrary", &json!({"xml": library(&[("Shown", "other")], &[("B", &[])])})).unwrap(); - assert_eq!((r["deletedDataSets"].clone(), r["names"].clone()), (json!(["A"]), json!(["B"]))); + for vars in [&[("Name", "textcontent")][..], &[("Shown", "other")][..]] { + let e = s.execute("variable.loadLibrary", &json!({"xml": library(vars, &[("B", &[])])})).unwrap_err(); + assert!(matches!(&e, EngineError::Disabled(_, why) if why == "variables are locked"), "{e}"); + } let vars = &s.doc().unwrap().doc.variables; assert!(vars.variables.is_empty() && vars.locked); - assert_eq!(vars.datasets.iter().map(|d| d.name.as_str()).collect::>(), ["B"]); + assert_eq!(vars.datasets.iter().map(|d| d.name.as_str()).collect::>(), ["A"]); } /// A graph variable's data set applies its CSV as `graph.setData` does: a right-axis series the row's data no longer @@ -3109,6 +3303,10 @@ fn an_svg_without_readable_variable_data_opens_its_art() { let head = ""; let art = ""; let untraited = format!("{head}{art}"); + let relative = format!( + "{head}\ + a.png{art}" + ); let refs = "&ns_big;".repeat(40_000); let big = "x".repeat(250); let growing = format!( @@ -3164,7 +3362,13 @@ fn an_svg_without_readable_variable_data_opens_its_art() { assert!(warnings.is_empty(), "{warnings:?}"); assert_eq!(o.doc().unwrap().doc.variables.variables.len(), 1); - for (svg, why) in [(untraited, "variable `A` has no `trait`"), (growing, "entities expand to more than")] { + // A variable without a `trait` is of unknown kind: the SVG leaves it out like the other kinds + // that aren't text. + let (o, warnings) = svg_opened(&untraited, "untraited.svg"); + assert!(warnings.contains(&json!("variables that aren't text were left out: unknown (1)")), "{warnings:?}"); + assert!(o.doc().unwrap().doc.variables.variables.is_empty()); + + for (svg, why) in [(relative, "that isn't a file's absolute path or `file:` URL"), (growing, "entities expand to more than")] { let (o, warnings) = svg_opened(&svg, "broken.svg"); assert!(warnings.iter().any(|w| w.as_str().is_some_and(|w| w.contains("variable data wasn't read") && w.contains(why))), "{warnings:?}"); let d = &o.doc().unwrap().doc; diff --git a/crates/ui-egui/src/panels/variables.rs b/crates/ui-egui/src/panels/variables.rs index a8987855eb..3ab5fc2484 100644 --- a/crates/ui-egui/src/panels/variables.rs +++ b/crates/ui-egui/src/panels/variables.rs @@ -268,8 +268,6 @@ pub fn menu(app: &mut VectorcraftApp, ui: &mut Ui) { let highlighted_bound: Vec = highlighted.as_ref().map(|h| bound_objects(st, h)).unwrap_or_default(); let Picked { selected, has_selection, selection_bound, single_type, single_image, selection_graph } = picked(st); let locked = st.doc.variables.locked; - // Save Variable Library writes text variables only. - let has_text = st.doc.variables.variables.iter().any(|v| v.kind == VariableKind::Text); if widgets::menu_item(ui, tl!("New Variable…"), !locked, false) { crate::menus::invoke(app, "variable.define", json!({})); @@ -333,7 +331,7 @@ pub fn menu(app: &mut VectorcraftApp, ui: &mut Ui) { if widgets::menu_item(ui, tl!("Load Variable Library…"), true, false) { crate::picks::button(app, load_library); } - if widgets::menu_item(ui, tl!("Save Variable Library…"), has_text, false) { + if widgets::menu_item(ui, tl!("Save Variable Library…"), !vars.is_empty(), false) { crate::picks::button(app, save_library); } ui.separator(); @@ -494,19 +492,22 @@ mod tests { assert!(text.contains("Name") && text.contains("Data Set 1"), "{text}"); } - /// Load Variable Library's warning about variables that aren't text shows in the status bar, - /// translated in every language but English. + /// Load Variable Library defines a library's variables of every kind and gives no warning. The + /// warning an SVG's variable data gives about variables that aren't text is translated in every + /// language but English. #[test] - fn the_variable_library_warning_shows_in_the_status_bar() { + fn the_svg_variable_data_warning_is_translated() { use crate::i18n::{Lang, message}; let mut app = app_with_data(); + let status = app.ui.status.clone(); let xml = r#""#; app.run("variable.loadLibrary", json!({"xml": xml})).unwrap(); - assert_eq!(app.ui.status, "variables that aren't text were left out: other (1)"); + assert_eq!(app.ui.status, status, "the load gives no warning"); + let warning = "variables that aren't text were left out: other (1)"; let langs: Vec = Lang::all().filter(|l| l.code() != "en").collect(); assert!(langs.len() >= 23, "{} languages", langs.len()); for l in langs { - let shown = message(l, &app.ui.status); + let shown = message(l, warning); assert!(!shown.contains("left out") && shown.contains("other") && shown.contains('1'), "{}: {shown}", l.code()); } } diff --git a/crates/ui-egui/src/tests/picks.rs b/crates/ui-egui/src/tests/picks.rs index ff290952fb..d6a06960bb 100644 --- a/crates/ui-egui/src/tests/picks.rs +++ b/crates/ui-egui/src/tests/picks.rs @@ -366,18 +366,17 @@ fn load_variable_library_asks_off_the_ui_thread() { let _ = std::fs::remove_dir_all(dir); } -/// The Variables panel menu's Save Variable Library… is enabled once the document has a text -/// variable, and then asks off the UI thread: the library is written -/// to the path the dialog answers, Load Variable Library reads it back, and the warning about the -/// visibility variable it left out shows in the status bar. +/// The Variables panel menu's Save Variable Library… is enabled once the document has a variable, +/// of any kind, and then asks off the UI thread: the library is written to the path the dialog +/// answers, and Load Variable Library reads back its visibility and text variables. #[test] fn save_variable_library_asks_off_the_ui_thread() { let (mut app, shown, _) = app(); app.run("file.new", json!({"width": 100, "height": 80})).unwrap(); - app.run("variable.define", json!({"name": "Shown", "kind": "visibility"})).unwrap(); - // Without a text variable the item is disabled and asks for no file. + // Without a variable the item is disabled and asks for no file. click(&mut app, "Save Variable Library…", crate::panels::variables::menu); assert!(shown.requests().is_empty(), "{:?}", shown.requests()); + app.run("variable.define", json!({"name": "Shown", "kind": "visibility"})).unwrap(); app.run("variable.define", json!({"name": "Name", "kind": "text"})).unwrap(); app.run("dataset.new", json!({"name": "A", "values": {"Name": "x", "Shown": true}})).unwrap(); let dir = vectorcraft_testkit::temp_dir("picks-save-variable-library"); @@ -392,10 +391,11 @@ fn save_variable_library_asks_off_the_ui_thread() { assert!(!library.exists(), "nothing is written before the dialog answers"); shown.answer(&[&library.to_string_lossy()]); poll(&mut app); - assert_eq!(app.ui.status, "variables that aren't text were left out: visibility (1)"); let xml = std::fs::read_to_string(&library).unwrap_or_else(|e| panic!("{e}: {}", app.ui.status)); let loaded = app.run("variable.loadLibrary", json!({"xml": xml})).unwrap(); - assert_eq!((loaded["variables"].clone(), loaded["names"].clone()), (json!(["Name"]), json!(["A"]))); + assert_eq!((loaded["variables"].clone(), loaded["names"].clone()), (json!(["Shown", "Name"]), json!(["A"]))); + let values = &app.session.doc().unwrap().doc.variables.datasets[0].values; + assert_eq!(values.get("Shown"), Some(&vectorcraft_doc::DataValue::Visible(true))); let _ = std::fs::remove_dir_all(dir); } diff --git a/docs/mcp.md b/docs/mcp.md index 2e43c14d0e..d464e888d3 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -3281,32 +3281,48 @@ already walks Image Trace. Short agent entry points: `dataset.import`, adds no undo step when it leaves the variables and data sets as they were. The library is XML whose `variableSets` element holds `variableSet` elements, each with a `variables` element of `variable` elements (`varName`, `trait`) and a `sampleDataSets` element of `sampleDataSet` elements (`dataSetName`). Every - data set is deleted, and every variable of the document is deleted with its bindings and values, except a text - variable the library also declares: it keeps its bindings, so selecting one of the library's data sets changes the - objects bound to it. Then the library's other variables whose `trait` is `textcontent` are defined as text - variables, unbound. In each data set, the element named - after a text variable gives its value: the text of its `p` elements, one paragraph each. An element without a `p` - element gives no value. Variables of other kinds are left out with their values: the reply lists them in `skipped` - (`name`, `trait`), and its `warning` names each `trait` with its count. The reply's `deleted` and `deletedDataSets` + data set is deleted, and every variable of the document is deleted with its bindings and values, except a variable + the library also declares with the same kind: it keeps its bindings, so selecting one of the library's data sets + changes the objects bound to it. Then the library's other variables are defined, unbound, with the kind their + `trait` gives: `textcontent` is text, `visibility` is visibility, `fileref` (a linked file) is image and + `graphdata` is graph. A variable without a `trait`, or with one VectorCraft doesn't know, is defined of unknown + kind and takes no values. In each data set, the element named after a variable gives its value: + - text: the text of its `p` elements, one paragraph each; an element without a `p` element gives no value; + - visibility: `true` or `false`, in any case; + - image: a file's absolute path (from `/`, `\\` or a drive's root) or a `file:` URL of one, whose `%XX` escapes + are decoded; the file isn't read, so a library can name files that aren't there; + - graph: a `data` element. Its optional `propertyRow` holds a corner cell and then the series names, and each `row` + of its `values` holds the row's category in a first `value` whose `key` is `name`, then a `value` per series. A + value that is empty, marked `type="string"` or not a number is a blank. The data is read as the CSV + `graph.setData` takes, at most 256 KiB of it. Graph data with more than 10,000 rows or 256 series, which a + graph doesn't hold, is refused, by `variable.saveLibrary` too. `variable.saveLibrary` also refuses graph data + with neither rows nor series, which loads back as no value. + + Visibility, image and graph values are read without the white space round them, and an element holding only white + space gives no value. The reply's `variables` lists the library's variables, and `deleted` and `deletedDataSets` list the variables and data sets the load deleted. While the variables are locked, a library that would delete or - define a variable fails with "variables are locked". The reply is `{variables, deleted, deletedDataSets, names, - skipped, warning?}`. A library larger than 8 MiB, one whose elements nest more than 256 deep, and one with more + define a variable fails with "variables are locked". The reply is `{variables, deleted, deletedDataSets, names}`. A + library larger than 8 MiB, one whose elements nest more than 256 deep, and one with more than 256 variables or 1,000 data sets are refused. Its document type declaration may hold an external identifier, comments, processing instructions and up to 64 entity declarations whose values are plain text (no `<`, `&` or `%`). A declaration that holds anything else, and entity references that expand the library past 8 MiB, are - refused. So are a variable or data set declared twice and a value for a variable the library doesn't declare. - `variable.saveLibrary {path?}` (the panel menu's Save Variable Library…) writes the document's text variables and - every data set as a variable library in the layout `variable.loadLibrary` reads: a `variable` element with - `varName` and `trait="textcontent"` for each text variable, and a `sampleDataSet` element with `dataSetName` for - each data set, holding an element named after each text variable it has a value for, with one `p` element per line - of the value. Loading the file gives back the same text variables, data set names and text values. Variables of - other kinds are left out with their values: the reply lists them in `skipped` (`name`, `kind`), and its `warning` - names each kind with its count. Saving is refused for a document without a text variable, for a text variable whose - name isn't an XML name (it starts with a letter or `_` and holds only letters, digits, `-`, `_` and `.`), for a - data set whose name or text value holds a control character, for a data set name with white space at either end, - for a name longer than 255 characters or used twice, and past `variable.loadLibrary`'s caps (256 variables, 1,000 - data sets, 8 MiB). Without `path` the reply is `{xml, variables, datasets, skipped, warning?}`; - with it, the file is written and the reply is `{path, variables, datasets, skipped, warning?}`. + refused. So are a variable or data set declared twice, a value for a variable the library doesn't declare, and a + value that doesn't fit its variable's kind. + `variable.saveLibrary {path?}` (the panel menu's Save Variable Library…) writes the document's variables and every + data set as a variable library in the layout `variable.loadLibrary` reads: a `variable` element with `varName` for + each variable, with the `trait` and `category` of its kind (`textcontent` and `&ns_flows;`, `visibility` and + `&ns_vars;`, `fileref` and `&ns_vars;`, `graphdata` and `&ns_graphs;`; neither for a variable of unknown kind), and + a `sampleDataSet` element with `dataSetName` for each data set, holding an element named after each variable it has + a value for. A text value is written with one `p` element per line, a visibility value as `true` or `false`, an + image value as its path (a path from the root of a Unix file system as a `file:///` URL), and a graph value as a + `data` element, with a blank category marked `type="string"`. Loading the file gives back the same variables with their kinds, data set names and values; graph + data comes back in the CSV form Graph Data writes. Saving is refused for a document without a variable, for a + variable whose name isn't an XML name (it starts with a letter or `_` and holds only letters, digits, `-`, `_` and + `.`; a variable of unknown kind has no values, so its name isn't checked), for a name or value that holds a control + character, for a name with white space at either end, for a name longer than 255 characters or used twice, for an + image value that isn't a file's absolute path, and past `variable.loadLibrary`'s caps (256 variables, 1,000 data + sets, 8 MiB). Without `path` the reply is `{xml, variables, datasets}`; with it, the file is written and the reply + is `{path, variables, datasets}`. - 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 c6375f3aa3..d198ee96a2 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -808,8 +808,8 @@ Moved from the ROADMAP's "Shipped so far" on 2026-10-10. Update it when a featur - Variables: Import Data Sets… (`dataset.import`) reads a UTF-16 file without a byte-order mark, instead of refusing it, when at least half of its two-byte units hold a NUL byte; it uses the byte order that reads its line breaks. - Graphs: the Graph Data window's Import Data reads a UTF-16 text file without a byte-order mark by the same rule as Import Data Sets…, instead of filling the cells with NUL characters. It reads a file cut short by one byte, or with an unpaired surrogate, as it reads one after a byte-order mark. - 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. - - Variables: Load Variable Library… in the Variables panel menu (`variable.loadLibrary`) replaces the document's variables and data sets with a variable library's, read from its XML file, in one undo step: every data set is deleted, and every variable is deleted with its bindings and values, except a text variable the library also declares, which stays bound so the library's data sets change its objects (#4136); then the library's other text variables are defined, unbound, and its data sets take the text of each value's `p` elements, one paragraph each. Variables of other kinds are left out with a warning that names their kinds. - - Variables: Save Variable Library… in the Variables panel menu (`variable.saveLibrary`) writes the document's text variables and data sets as a variable library's XML file, which Load Variable Library reads back with the same text variables, data set names and text values. Variables of other kinds are left out with a warning that names their kinds. + - Variables: Load Variable Library… in the Variables panel menu (`variable.loadLibrary`) replaces the document's variables and data sets with a variable library's, read from its XML file, in one undo step: every data set is deleted, and every variable is deleted with its bindings and values, except a variable the library also declares with the same kind, which stays bound so the library's data sets change its objects (#4136); then the library's other variables are defined, unbound: text, visibility, image (linked file) and graph variables, and variables without a known `trait` as unknown. Its data sets take text values from each value's `p` elements, one paragraph each, visibility values from `true` or `false`, image values from a file's absolute path or `file:` URL, and graph values from a `data` element of series names and rows. + - Variables: Save Variable Library… in the Variables panel menu (`variable.saveLibrary`) writes the document's variables of every kind and its data sets as a variable library's XML file, which Load Variable Library reads back with the same variables, kinds, data set names and values. - JSON-lines control channel with real egui pointer and keyboard injection. - Variables: a type object holds one text variable. `variable.bind` and Make Text Dynamic refuse a second one and name the text variable the object has; a visibility variable can still share the object, and a document saved with two text variables on one object opens with both. - 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).