diff --git a/crates/engine/src/cmd/css.rs b/crates/engine/src/cmd/css.rs index ed83a01410..dd6af8e6eb 100644 --- a/crates/engine/src/cmd/css.rs +++ b/crates/engine/src/cmd/css.rs @@ -8,7 +8,7 @@ use vectorcraft_doc::NodeId; use vectorcraft_svg::{CssOptions, CssSheet, css_rules}; use super::fileio::{default_name, encode_with_warnings, isolated, write_file}; -use super::{CommandSpec, Result, Session, bad, cmd, has_doc, str_param, targets}; +use super::{CommandSpec, Result, Session, bad, cmd, has_doc, named_ids, targets}; pub fn specs() -> Vec { vec![ @@ -21,13 +21,13 @@ pub fn specs() -> Vec { has_doc, selection ), - cmd!(query "css.generate", "Generate CSS", [], None, "{…options as css.selection} css.selection of every object in the document", has_doc, generate), + cmd!(query "css.generate", "Generate CSS", [], None, "{…options as css.selection} css.selection of every object in the document; ids and id are not used, and a malformed one is an error", has_doc, generate), cmd!( "css.export", "Export CSS", [], None, - "{path?, scope?: selection|all (default selection; ids? picks the objects), …options as css.selection, id?: object id (default: the selection)} write the CSS (css.selection, or css.generate for all) to path and, with rasterize, each picture its rules refer to next to it (PNG of what the object draws, with the clips, masks and effects of the groups and layers that hold it, 1 px per point) → {path, bytes, rules: n, images: [path…], warnings}; no path → {data: the CSS, name: .css, rules, images: [{name, dataBase64}], warnings}; warnings are css.selection's and the pictures' as document.export gives them (an image that couldn't be decoded, type in the fallback font or that doesn't fit its frame), each once", + "{path?: string, scope?: selection|all (default selection when left out; ids? picks the objects), …options as css.selection, id?: object id (default: the selection)} a path or scope given as anything but a string, null included, is an error, and so is a malformed ids or id with scope all, which does not use them; write the CSS (css.selection, or css.generate for all) to path and, with rasterize, each picture its rules refer to next to it (PNG of what the object draws, with the clips, masks and effects of the groups and layers that hold it, 1 px per point) → {path, bytes, rules: n, images: [path…], warnings}; no path → {data: the CSS, name: .css, rules, images: [{name, dataBase64}], warnings}; warnings are css.selection's and the pictures' as document.export gives them (an image that couldn't be decoded, type in the fallback font or that doesn't fit its frame), each once", has_doc, export ), @@ -77,16 +77,34 @@ fn selection(s: &mut Session, p: &Value) -> Result { Ok(result(&sheet(s, p, ids, "css.selection")?)) } +/// Checks the form of the `ids` or `id` that `p` gives a command that exports every object and does +/// not use them: a malformed one is an error, as it is for the selection. +fn unused_ids(p: &Value) -> Result<()> { + named_ids(p).map(|_| ()) +} + fn generate(s: &mut Session, p: &Value) -> Result { + unused_ids(p)?; let ids = everything(s)?; Ok(result(&sheet(s, p, ids, "css.generate")?)) } +/// The string `p` gives for `key`, or `None` when `p` leaves it out. Any value given that is not a +/// string, null included, is an error: "`{key}` must be a string". +fn given_text<'a>(p: &'a Value, key: &str, cmd: &str) -> Result> { + p.get(key).map(|v| v.as_str().ok_or_else(|| bad(cmd, format!("`{key}` must be a string")))).transpose() +} + fn export(s: &mut Session, p: &Value) -> Result { const C: &str = "css.export"; - let ids = match str_param(p, "scope").unwrap_or("selection") { + let scope = given_text(p, "scope", C)?; + let path = given_text(p, "path", C)?; + let ids = match scope.unwrap_or("selection") { "selection" => targets(s, p)?, - "all" => everything(s)?, + "all" => { + unused_ids(p)?; + everything(s)? + } other => return Err(bad(C, format!("scope must be selection or all (got {other})"))), }; let sheet = sheet(s, p, ids, C)?; @@ -115,7 +133,7 @@ fn export(s: &mut Session, p: &Value) -> Result { } } let n = sheet.rules.len(); - match str_param(p, "path") { + match path { Some(path) => { write_file(path, css.as_bytes())?; let folder = std::path::Path::new(path).parent(); diff --git a/crates/engine/src/tests/css.rs b/crates/engine/src/tests/css.rs index 764990b100..1ce5b09e41 100644 --- a/crates/engine/src/tests/css.rs +++ b/crates/engine/src/tests/css.rs @@ -98,6 +98,69 @@ fn export_writes_the_sheet_and_rasterized_art() { assert!(s.execute("css.export", &json!({})).is_err(), "nothing selected: nothing to export"); } +#[test] +fn export_refuses_a_scope_or_path_that_is_not_a_string() { + let mut s = session(); + run(&mut s, "shape.rectangle", json!({"x": 0, "y": 0, "width": 40, "height": 40})); + run(&mut s, "shape.ellipse", json!({"x": 60, "y": 0, "width": 40, "height": 40})); + let refused = |s: &mut Session, p: Value, key: &str| { + let e = s.execute("css.export", &p).expect_err(&p.to_string()).to_string(); + assert_eq!(e, format!("invalid parameters for `css.export`: `{key}` must be a string"), "{p}"); + }; + for scope in [json!(5), json!(true), json!(["all"]), Value::Null] { + refused(&mut s, json!({"scope": scope}), "scope"); + } + for path in [json!(5), Value::Null] { + refused(&mut s, json!({"path": path}), "path"); + refused(&mut s, json!({"path": path, "scope": "all"}), "path"); + } + // Left out, each keeps its default: the selection (the ellipse), and the CSS as data. + let r = run(&mut s, "css.export", json!({})); + assert_eq!((r["rules"].clone(), r["data"].is_string()), (json!(1), true), "{r}"); + assert_eq!(run(&mut s, "css.export", json!({"scope": "all"}))["rules"], json!(2)); + let e = s.execute("css.export", &json!({"scope": "page"})).unwrap_err().to_string(); + assert!(e.contains("scope must be selection or all (got page)"), "{e}"); +} + +#[test] +fn every_object_refuses_malformed_ids_it_does_not_use() { + let mut s = session(); + let rect = run(&mut s, "shape.rectangle", json!({"x": 0, "y": 0, "width": 40, "height": 40}))["id"].as_u64().unwrap(); + run(&mut s, "shape.ellipse", json!({"x": 60, "y": 0, "width": 40, "height": 40})); + for (cmd, base) in [("css.generate", json!({})), ("css.export", json!({"scope": "all"}))] { + for (key, value, says) in [ + ("ids", json!("abc"), "`ids` must be an array of object ids, not \"abc\""), + ("ids", json!([1.5]), "object ids are non-negative integers, not 1.5"), + ("id", json!(-1), "object ids are non-negative integers, not -1"), + ("id", json!("$step"), "`id` is the step reference \"$step\""), + ] { + let mut p = base.clone(); + p[key] = value; + let e = s.execute(cmd, &p).expect_err(&format!("{cmd} {p}")).to_string(); + assert!(e.contains(says), "{cmd} {p}: {e}"); + } + // Well-formed ids are still not used: every object. + let mut p = base.clone(); + p["ids"] = json!([rect]); + let rules = run(&mut s, cmd, p)["rules"].clone(); + let count = rules.as_array().map_or(rules.as_u64(), |r| Some(r.len() as u64)); + assert_eq!(count, Some(2), "{cmd}: {rules}"); + } +} + +#[cfg(not(target_arch = "wasm32"))] +#[test] +fn export_with_a_scope_that_is_not_a_string_writes_nothing() { + let mut s = session(); + run(&mut s, "shape.rectangle", json!({"x": 0, "y": 0, "width": 40, "height": 40})); + let dir = std::env::temp_dir().join(format!("vc-css-scope-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join("site.css"); + assert!(s.execute("css.export", &json!({"path": path.to_string_lossy(), "scope": 5})).is_err()); + assert!(!path.exists(), "no file for a refused scope"); + std::fs::remove_dir_all(&dir).ok(); +} + #[cfg(not(target_arch = "wasm32"))] #[test] fn export_to_a_path_puts_the_pictures_next_to_the_sheet() { diff --git a/docs/mcp.md b/docs/mcp.md index 26515616ab..9191ab8bd7 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -6947,9 +6947,11 @@ says when CSS can't describe the art exactly (`unsupported`: other shapes, image written as its box, or with `rasterize: true` as `background-image: url(.png)`. `units` is `px` (1 px per point, as in SVG export), `pt`, `mm`, `cm` or `in`; `unnamed: false` leaves out unnamed objects (counted in `skipped`). `css.generate` does the same for the whole document, and `css.export {path?, scope?: selection|all}` -writes the `.css` file with the PNGs of rasterized art next to it (no path: `data` and `images` as base64). Its -`warnings` add the PNGs' notes to the CSS's, each once (an image that couldn't be decoded, type written in the -fallback font…). In the app, `css.copy` copies the CSS and `css.exportFile` writes it through a save panel (the web +writes the `.css` file with the PNGs of rasterized art next to it (no path: `data` and `images` as base64). A left-out +`scope` exports the selection; a `path` or `scope` given as anything but a string, `null` included, is an error, and +nothing is written. `css.generate` and `css.export {scope: "all"}` do not use `ids` or `id`, but a malformed one is still +an error. Its `warnings` add the PNGs' notes to the CSS's, each once (an image that couldn't be decoded, +type written in the fallback font…). In the app, `css.copy` copies the CSS and `css.exportFile` writes it through a save panel (the web downloads it) and shows the first note in the status bar, with how many there are; resting the pointer on the message lists up to 30 of them, fewer when the list would not fit in the window, with how many were left out. diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index 02beac2428..a981a008a1 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -966,6 +966,7 @@ Moved from the ROADMAP's "Shipped so far" on 2026-10-10. Update it when a featur The command palette lists `select.key` as Cancel Key Object, and a pick cancels the key object (#541). Agent selection commands validate every object id before changing the selection and report the resulting count and ids (#784). Commands that act on `id` or else the selection, the highlighted layer row or a new object (`css.export`, `object.isolate`, `layer.locate`, `layer.releaseToLayers`, the graphic, character and paragraph style commands, `image.info`, the `links.*` and `shaper.*` commands that take an `id`, opacity mask editing and viewing, `symbol.edit`, the blend spine commands and `path.curvature`) refuse an `id` that is not an object id (`"abc"`, `"5"`, `true`) and change nothing; `effect.apply` with the effect given as `id` still applies it to the selection. + `css.export` refuses a `scope` or `path` that is not a string (`5`, `true`, `["all"]`, `null`) before writing anything, instead of exporting the selection or returning the CSS as data; a left-out `scope` still exports the selection, and a left-out `path` still returns the CSS. `css.generate` and `css.export {scope: "all"}` refuse a malformed `ids` or `id`, which they do not use, as the selection's export does. Agents read the key object: `select.key` returns it, and `document.inspect` and the `ui.pointer` and `pointer_gesture` replies report it as `key`. `select.key` with an id that isn't a selected top-level object returns an error and leaves the key unchanged. The Select menu's Same commands, its Object commands that select art, Next Object Above and Below, saved selections and the Magic Wand (#2655) select only objects that Select All selects and objects inside them: nothing hidden or locked, nothing in a hidden or locked group, layer or sublayer, nothing on a template layer, and in isolation mode nothing outside the isolated group or layer. In isolation mode Select All, All on Active Artboard and Inverse select nothing while the isolated group or layer, or a group or layer it is in, is hidden, locked or a template layer. Select ▸ Object ▸ Bristle Brush Strokes selects the objects whose topmost stroke has a bristle brush, whatever the brush is named, and leaves out objects whose brush is of another type, also when its name has "Bristle" in it.