Merge pull request #4429 from dotCooCoo/fix/layer-commands-refuse-text-booleans

Layers: the Layers panel's commands refuse a boolean given as text

crates/engine/src/cmd/layer.rs

@@ -15,7 +15,7 @@ use vectorcraft_geom::{Affine, Rect};
 
 use super::{
     CommandSpec, Result, Session, bad, bool_or, check_numbers, check_switches, checked_id, checked_ids_param, cmd, color_value, f64_or, has_doc,
-    has_selection, id_param, ids_param, index, ok, str_param,
+    has_selection, id_param, ids_param, index, ok, str_param, switch_or,
 };
 use crate::EngineError;
 
@@ -405,7 +405,7 @@ fn check_options(cmd: &str, p: &Value, xml_id: bool) -> Result<()> {
 /// Layers panel → Paste Remembers Layers.
 fn paste_remembers_layers(s: &mut Session, p: &Value) -> Result<Value> {
     let cur = s.doc()?.doc.paste_remembers_layers;
-    let on = bool_or(p, "on", !cur);
+    let on = switch_or(p, "on", !cur, "layer.pasteRemembersLayers")?;
     if on != cur {
         s.edit("Paste Remembers Layers", |d, _| {
             d.paste_remembers_layers = on;
@@ -429,7 +429,7 @@ fn target(s: &mut Session, p: &Value) -> Result<Value> {
     }
     let layer = n.is_layer();
     let rows = st.layer_rows.clone();
-    if p.get("add").and_then(Value::as_bool).unwrap_or(false) {
+    if switch_or(p, "add", false, C)? {
         s.select(|d, sel| sel.toggle_target(d, id))?;
     } else {
         s.select(|d, sel| sel.set_target(d, id))?;
@@ -480,6 +480,7 @@ fn clip_toggle(s: &mut Session, p: &Value) -> Result<Value> {
 
 /// A new layer with the options in `p`, at `parent`/`index`; it becomes current and highlighted.
 fn add_layer(s: &mut Session, p: &Value, cmd: &str, label: &str, place: impl FnOnce(&Document) -> Result<(Option<NodeId>, usize)>) -> Result<Value> {
+    check_switches(p, &["top"], cmd)?;
     let xml_id = s.prefs.identify_objects_by == "xmlId";
     check_options(cmd, p, xml_id)?;
     let name = str_param(p, "name").filter(|n| !n.trim().is_empty()).map(str::to_string);
@@ -669,7 +670,7 @@ fn in_isolation(doc: &Document, isolation: Option<NodeId>, id: NodeId) -> bool {
 fn select_art(s: &mut Session, p: &Value) -> Result<Value> {
     const C: &str = "layer.selectAll";
     let id = id_param(p, "id").ok_or_else(|| bad(C, "missing id"))?;
-    let add = bool_or(p, "add", false);
+    let add = switch_or(p, "add", false, C)?;
     let st = s.doc()?;
     let n = st.doc.node(id).ok_or(EngineError::NoNode(id))?;
     let ids: Vec<NodeId> = if !st.doc.is_editable(id) || !in_isolation(&st.doc, st.isolation, id) {
@@ -719,7 +720,7 @@ fn move_rows(s: &mut Session, p: &Value) -> Result<Value> {
         "inside" | "into" => Place::Inside,
         other => return Err(bad(C, format!("place is above, below or inside, not `{other}`"))),
     };
-    let copy = bool_or(p, "copy", false);
+    let copy = switch_or(p, "copy", false, C)?;
     let st = s.doc()?;
     let d = &st.doc;
     d.node(target).ok_or(EngineError::NoNode(target))?;

crates/engine/src/cmd/layerpanel.rs

@@ -10,7 +10,7 @@ use serde_json::{Value, json};
 use vectorcraft_doc::{Document, Node, NodeId, NodeKind};
 
 use super::layer::{NewLayers, row_roots, rows_param};
-use super::{CommandSpec, Result, Session, bad, bool_or, checked_id, cmd, has_doc, id_param, ok};
+use super::{CommandSpec, Result, Session, bad, checked_id, cmd, has_doc, id_param, ok, switch_or};
 use crate::EngineError;
 
 pub fn specs() -> Vec<CommandSpec> {
@@ -225,7 +225,7 @@ fn template(s: &mut Session, p: &Value) -> Result<Value> {
     let d = &s.doc()?.doc;
     let ids: Vec<NodeId> = ids.into_iter().filter(|id| d.node(*id).is_some_and(Node::is_layer)).collect();
     let Some(first) = ids.first().and_then(|id| d.node(*id)) else { return Err(bad("layer.template", "select a layer")) };
-    let on = bool_or(p, "on", !first.is_template());
+    let on = switch_or(p, "on", !first.is_template(), "layer.template")?;
     s.execute("layer.setProps", &json!({"ids": ids.iter().map(|i| i.0).collect::<Vec<_>>(), "template": on}))?;
     Ok(json!({ "on": on }))
 }
@@ -334,7 +334,7 @@ fn flatten(s: &mut Session, p: &Value) -> Result<Value> {
 
 fn release(s: &mut Session, p: &Value) -> Result<Value> {
     const C: &str = "layer.releaseToLayers";
-    let build = bool_or(p, "build", false);
+    let build = switch_or(p, "build", false, C)?;
     let st = s.doc()?;
     let id = match id_param(p, "id") {
         Some(id) => id,

crates/engine/src/tests/layers.rs

@@ -472,6 +472,60 @@ fn moves_are_validated() {
     assert_eq!(parent(&s, sub), None);
 }
 
+/// The Layers panel's commands refuse a switch given as text or as another value that is not true
+/// or false, and leave the document, the selection, the highlighted rows and the undo history as
+/// they were; true, false and null still work.
+#[test]
+fn layer_commands_refuse_a_switch_given_as_text() {
+    let fixture = || {
+        let mut s = session();
+        let (layer, sub, a, g, _, c) = tree(&mut s);
+        run(&mut s, "layer.setCurrent", json!({"id": sub.0}));
+        (s, layer, sub, a, g, c)
+    };
+    let (_, layer, sub, a, g, _) = fixture();
+    let mut unexpected = vec![];
+    for (cmd, p, key) in [
+        ("layer.new", json!({"top": "true"}), "top"),
+        ("layer.new", json!({"top": 1, "name": "Top"}), "top"),
+        ("layer.newSublayer", json!({"top": "false"}), "top"),
+        ("layer.selectAll", json!({"id": layer.0, "add": "true"}), "add"),
+        ("layer.target", json!({"id": layer.0, "add": 1}), "add"),
+        ("layer.move", json!({"ids": [a.0], "target": layer.0, "copy": "true"}), "copy"),
+        ("layer.move", json!({"ids": [a.0], "target": g.0, "copy": {}}), "copy"),
+        ("layer.template", json!({"ids": [layer.0], "on": "false"}), "on"),
+        ("layer.pasteRemembersLayers", json!({"on": "false"}), "on"),
+        ("layer.pasteRemembersLayers", json!({"on": 0}), "on"),
+        ("layer.releaseToLayers", json!({"id": sub.0, "build": [true]}), "build"),
+    ] {
+        // Each call on a fresh copy of the same tree, so one that goes through does not change the next.
+        let (mut s, l, sl, o, gr, _) = fixture();
+        assert_eq!((l, sl, o, gr), (layer, sub, a, g), "the same ids");
+        let before = layer_target_snapshot(&s);
+        let r = s.execute(cmd, &p);
+        let want = format!("`{key}` must be true or false");
+        if !matches!(&r, Err(EngineError::BadParams { cmd: got, msg }) if got == cmd && *msg == want) {
+            unexpected.push(format!("{cmd} {p}: {r:?}"));
+        } else if layer_target_snapshot(&s) != before {
+            unexpected.push(format!("{cmd} {p}: refused, but the document, selection, rows or history changed"));
+        }
+    }
+    assert!(unexpected.is_empty(), "not refused: {}", unexpected.join("; "));
+    let (mut s, layer, sub, a, g, c) = fixture();
+    // A null reads as the key left out.
+    let n = id(&run(&mut s, "layer.new", json!({"top": null})));
+    assert_eq!(parent(&s, n), Some(layer), "beside the current sublayer");
+    run(&mut s, "layer.selectAll", json!({"id": sub.0, "add": null}));
+    run(&mut s, "layer.selectAll", json!({"id": c.0, "add": true}));
+    assert_eq!(s.doc().unwrap().selection.objects, vec![a, g, c]);
+    run(&mut s, "layer.move", json!({"ids": [a.0], "target": g.0, "copy": false}));
+    assert_eq!(parent(&s, a), Some(g));
+    assert_eq!(run(&mut s, "layer.template", json!({"ids": [layer.0], "on": null}))["on"], true);
+    assert_eq!(run(&mut s, "layer.template", json!({"ids": [layer.0], "on": false}))["on"], false);
+    assert_eq!(run(&mut s, "layer.pasteRemembersLayers", json!({"on": null}))["on"], true, "null toggles");
+    assert_eq!(ids(&run(&mut s, "layer.releaseToLayers", json!({"id": g.0, "build": null}))).len(), 2);
+}
+
 /// `node.move` into a hidden or locked layer deselects the object it moves, here the key object.
 /// Undo puts the object back and selects it again as the key object.
 #[test]

docs/mcp.md

@@ -2154,6 +2154,12 @@ and `layerRows`, the rows highlighted in the panel.
   stays. A key left out keeps its list. The filter lasts the session and does not change the document, its open rows
   or the selection. `ui.layersFilter` answers `{objects, layers}`.
 
+These Layers panel commands refuse a string, a number or another value that is not true or false with
+`` `copy` must be true or false ``, naming the key, and leave the document, the selection, the highlighted rows and
+the undo history as they were: `top` in `layer.new` and `layer.newSublayer`, `add` in `layer.selectAll` and
+`layer.target`, `copy` in `layer.move`, `on` in `layer.template` and `layer.pasteRemembersLayers`, and `build` in
+`layer.releaseToLayers`. A null is read as a key left out.
+
 ```json
 {"name":"run_command","arguments":{"command":"layer.newSublayer","params":{"name":"Shadows"}}}
 {"name":"run_command","arguments":{"command":"layer.move","params":{"ids":[12,15],"target":7,"place":"inside"}}}
@@ -3187,8 +3193,9 @@ Every artboard command that takes an `index` fails with `no such artboard` when
 clipboard stay as they were.
 These artboard commands refuse a string, a number or another value that is not true or false with
 `` `art` must be true or false ``, naming the key, and leave the document, its undo history and the clipboard as they
-were: `moveArt`, `scaleArt`, `strokes`, `corners`, `patterns`, `lockedAndHidden` and an artboard's `locked` in
-`artboard.setProps`; `moveArt`, `copy` and `patterns` in `artboard.move`; `art` and `patterns` in `artboard.duplicate`;
+were: `moveArt`, `scaleArt`, `strokes`, `corners`, `patterns`, `lockedAndHidden` and an artboard's `locked`,
+`showCenterMark`, `showCrossHairs` and `showVideoSafeAreas` in `artboard.setProps`; `showCenterMark`, `showCrossHairs`
+and `showVideoSafeAreas` in `artboard.new`; `moveArt`, `copy` and `patterns` in `artboard.move`; `art` and `patterns` in `artboard.duplicate`;
 and `art` in `artboard.copy` and `artboard.cut`. `artboard.setProps` refuses an artboard's `x`, `y`, `width` or
 `height` that is not a number, and `artboard.move` refuses such a `dx` or `dy`, with
 `` `dx` must be a number, not "10" ``. A null is read as a key left out.

docs/target-app-parity.md

@@ -896,6 +896,7 @@ Moved from the ROADMAP's "Shipped so far" on 2026-10-10. Update it when a featur
   Transform Each (Reflect X, Reflect Y, Random, Copy, Scale Strokes & Effects, Scale Corners, Transform Objects and Transform Patterns), Create Object Mosaic (gray tiles and deleting the image), Crop Image (its trim), Create Trim Marks (all artboards) and Rearrange All Artboards (by column, Move Artwork with Artboard and Transform Patterns) refuse a value other than true, false or null, such as "false", and change nothing. Create Trim Marks refuses a style that is not text, such as 5.
   New Artboard refuses a position or size other than a number or null, such as "10", and New Artboard and Artboard Options refuse a name other than text or null, such as 5; each leaves the artboards and the undo history as they were.
   Artboard Options (Move Artwork, Scale Artwork, Scale Strokes & Effects, Scale Corners, Transform Patterns, locked and hidden art and the lock), Move Artboard (Move Artwork, the copy and Transform Patterns), Duplicate Artboards (Transform Patterns) and Duplicate, Copy and Cut Artboard (the art) refuse a value other than true, false or null, such as "true", and Artboard Options and Move Artboard refuse a position, size or move given as text, such as "10"; each leaves the document, its undo history and the clipboard as they were.
+  The Layers panel's New Layer (on top), Select Art and Target (Shift-click), Move Rows (the copy), Template, Paste Remembers Layers and Release to Layers (Build) refuse a value other than true, false or null, such as "true", and leave the document, the selection, the highlighted rows and the undo history as they were; New Sublayer refuses such a `top` too.
   Change Case, Smart Punctuation, Find and Replace, Convert To Area Type, Convert To Point Type, Type on a Path Options, the Type on a Path effects and Fill with Placeholder Text record no undo step when they change nothing.
   The commands that take `keepModel` (fills and strokes, Appearance rows, Live Paint, Shape Builder fills, gradient stops and freeform points, gradient mesh points, type colors, the Symbol Stainer and the swatch commands) refuse a `keepModel` other than true, false or null, such as "true". Toggle Fill/Stroke Focus (`fill`), Invert, Complement, Apply Last Color and Apply Last Gradient (`stroke`), `paint.setFill` and `paint.setStroke` (`focus`, `none`), and `appearance.setItem`, `livePaint.fill`, `livePaint.strokeEdge`, a `shapeBuilder.merge` `fill`, `swatch.new` and `swatch.edit`'s `paint` (`none`) refuse such a value too. Each leaves the document, its undo history and the defaults for new art as they were.
   The shape tools' commands (Rectangle, Polygon, Star, Flare, Spiral, Arc, Rectangular Grid and Polar Grid), `path.create` (with `anchors` or `d`), the type commands (new type, type in a path, character formats, the byte range a text command takes, Touch Type, font size steps, `text.getRange`, Optical Margin Alignment, System Layout, Fractional Widths, Ruby and Area Type Options), the symbol commands (Place, Delete, the Symbol Sprayer and the symbolism tools), the swatch commands, the Appearance panel's commands, the gradient and freeform gradient commands, Shape Builder, Live Paint and Image Trace refuse an option other than true, false or null and a number given as text, such as "true" or "40", and leave the document and its undo history as they were.