From b5da39d7b54c7bcbe2f61eab900b99ea0ec39305 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Thu, 5 Nov 2026 21:55:29 -0800 Subject: [PATCH] Commands: true/false flags reject other values everywhere The remaining flags under crates/engine/src/cmd that read a boolean with get(..).and_then(Value::as_bool).unwrap_or(..) now go through switch_or, so a value such as "false" or 1 is an error instead of the default. Omitting a flag or giving null keeps the old behavior. - effect.move: copy - aligned on the appearance.*, stroke.set, transparency.set and effect.* commands, checked before any early return - transparency.viewOpacityMask: on - view.printTiling: on - plugin.install: replace and allowHeavy, checked for every plug-in - edit.clear: guides, and path.deleteAnchors' read of it - text.create and text.createInPath: vertical (already refused by check_switches, so their behavior is unchanged) --- crates/engine/src/cmd/appearance.rs | 20 +++++++++------- crates/engine/src/cmd/create.rs | 2 +- crates/engine/src/cmd/edit.rs | 5 ++-- crates/engine/src/cmd/effectcmd.rs | 10 ++++---- crates/engine/src/cmd/maskedit.rs | 4 ++-- crates/engine/src/cmd/path.rs | 2 +- crates/engine/src/cmd/plugin.rs | 7 +++--- crates/engine/src/cmd/printtiling.rs | 4 ++-- crates/engine/src/cmd/textedit.rs | 2 +- crates/engine/src/tests/appearance.rs | 33 ++++++++++++++++++++++++++ crates/engine/src/tests/effectedit.rs | 23 ++++++++++++++++++ crates/engine/src/tests/maskview.rs | 20 ++++++++++++++++ crates/engine/src/tests/menucmds.rs | 24 +++++++++++++++++++ crates/engine/src/tests/plugins.rs | 30 +++++++++++++++++++++++ crates/engine/src/tests/printtiling.rs | 15 ++++++++++++ docs/mcp.md | 7 ++++++ 16 files changed, 183 insertions(+), 25 deletions(-) diff --git a/crates/engine/src/cmd/appearance.rs b/crates/engine/src/cmd/appearance.rs index 6507001c7..c8c479bc0 100644 --- a/crates/engine/src/cmd/appearance.rs +++ b/crates/engine/src/cmd/appearance.rs @@ -190,7 +190,7 @@ impl Session { let ids: Vec = ids.into_iter().map(|id| id.0).collect(); let aligned = match item { ItemTarget::Item { aligned, .. } => aligned, - ItemTarget::Top => aligned(params), + ItemTarget::Top => aligned(params, "effect.apply").ok()?, }; Some((json!({"ids": ids, "item": at, "aligned": aligned}), effects.as_slice())) } @@ -241,11 +241,12 @@ pub(crate) fn item_target(s: &Session, p: &Value, cmd: &str) -> Result Result { + let aligned = aligned(p, cmd)?; match p.get(key) { Some(Value::Null) => Ok(ItemTarget::Top), Some(v) => v .as_u64() - .map(|i| ItemTarget::Item { index: index(i), explicit: true, aligned: aligned(p) }) + .map(|i| ItemTarget::Item { index: index(i), explicit: true, aligned }) .ok_or_else(|| bad(cmd, format!("`{key}` must be an appearance item index (paint order) or null"))), // The active item is a row of the first selected object's own stack. None if p.get("ids").is_some() || p.get("id").is_some() || targets_contents(p)? => Ok(ItemTarget::Top), @@ -337,8 +338,8 @@ impl ItemTarget { /// Does `p` say its item and effect indices are rows of the first target's stack (`aligned`), /// which each other target edits where it has them (see [`ItemTarget::per_target`])? -pub(crate) fn aligned(p: &Value) -> bool { - p.get("aligned").and_then(Value::as_bool).unwrap_or(false) +pub(crate) fn aligned(p: &Value, cmd: &str) -> Result { + switch_or(p, "aligned", false, cmd) } /// What the appearances of `ids` share, when there are several of them and they all exist. @@ -745,7 +746,7 @@ fn set_item(s: &mut Session, p: &Value) -> Result { // When every object comes out as it was (for example, when none has item `index`), the document // and its undo history stay as they are. let before = s.doc()?.doc.clone(); - let per = items_per_target(&before, &ids, &[idx], aligned(p)); + let per = items_per_target(&before, &ids, &[idx], aligned(p, "appearance.setItem")?); super::overprint::edit_counted(s, "Appearance", |d| { let mut edited = Vec::new(); for (id, at) in ids.iter().zip(&per) { @@ -818,7 +819,7 @@ fn remove_item(s: &mut Session, p: &Value) -> Result { // When no object has an item at any of the indices, the document and its undo history stay as // they are. let before = s.doc()?.doc.clone(); - let per = items_per_target(&before, &ids, &idx, aligned(p)); + let per = items_per_target(&before, &ids, &idx, aligned(p, "appearance.removeItem")?); super::overprint::edit_counted(s, "Remove Item", |d| { for (id, idx) in ids.iter().zip(&per) { if let Some(n) = d.node_mut(*id) { @@ -854,11 +855,11 @@ fn duplicate_item(s: &mut Session, p: &Value) -> Result { let active = s.active_appearance_item.as_ref().and_then(|a| a.objects.first().copied()); let mut active_at = None; // Aligned rows of a mixed selection are copied right above themselves in each object. - if aligned(p) && to.is_some() && shared_stack(&s.doc()?.doc, &ids).is_some() { + if aligned(p, C)? && to.is_some() && shared_stack(&s.doc()?.doc, &ids).is_some() { return Err(bad(C, "`to` takes the rows of one object's stack, not aligned rows")); } let before = s.doc()?.doc.clone(); - let per = items_per_target(&before, &ids, &idx, aligned(p)); + let per = items_per_target(&before, &ids, &idx, aligned(p, C)?); s.edit("Duplicate Item", |d, _| { let mut any = false; for (id, idx) in ids.iter().zip(&per) { @@ -932,11 +933,12 @@ fn move_item(s: &mut Session, p: &Value) -> Result { const C: &str = "appearance.moveItem"; let to = index_param(p, "to", C)?; let ids = require_targets(s, p, C)?; + let aligned = aligned(p, C)?; if p.get("from").and_then(Value::as_str) == Some("contents") { return move_contents(s, &ids, to); } // Where an aligned row would go in each of several different stacks isn't one place. - if aligned(p) && appearances_differ(&s.doc()?.doc, &ids) { + if aligned && appearances_differ(&s.doc()?.doc, &ids) { return Err(bad(C, "rows of objects whose appearances differ can't be reordered together")); } if p.get("indices").is_some() { diff --git a/crates/engine/src/cmd/create.rs b/crates/engine/src/cmd/create.rs index ab3dc966c..6860df149 100644 --- a/crates/engine/src/cmd/create.rs +++ b/crates/engine/src/cmd/create.rs @@ -721,7 +721,7 @@ fn text_create(s: &mut Session, p: &Value) -> Result { if rotation != 0.0 { t.xf *= vectorcraft_geom::rotate_deg(rotation); } - t.vertical = p.get("vertical").and_then(Value::as_bool).unwrap_or(false); + t.vertical = switch_or(p, "vertical", false, "text.create")?; t.para = new_type_para(); new_type_alignment(s, p, "text.create", &mut t)?; if let Some(a) = p.get("area") { diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 5b68b28d8..bd0e7704f 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -11,7 +11,7 @@ use super::clipboard::SwatchChoices; use super::{ CommandSpec, Result, Session, always, bad, bool_or, can_clear_history, can_redo, can_undo, checked_ids_param, cmd, f64_or, f64_req, finite_numbers, has_clipboard, has_doc, has_selection, has_selection_guides_or_artboard_tool, has_selection_or_guides, id_param, index, ok, - point_param, str_param, + point_param, str_param, switch_or, }; use crate::{Clipboard, EngineError, inspect}; @@ -817,6 +817,7 @@ fn cut_direct_selection(s: &mut Session, roots: &[NodeId]) -> Result { } fn clear(s: &mut Session, p: &Value) -> Result { + let guides = switch_or(p, "guides", false, "edit.clear")?; let explicit = if p.get("ids").is_some() { Some(checked_ids_param(s, p, "ids", "edit.clear")?) } else { None }; // Direct-selected anchors or segments: remove those, and any objects selected whole with them. if explicit.is_none() && (!s.doc()?.selection.anchors.is_empty() || !s.doc()?.selection.segments.is_empty()) { @@ -829,7 +830,7 @@ fn clear(s: &mut Session, p: &Value) -> Result { let st = s.doc()?; // Guides selected with the art go with it (not when ids are given, unless `guides` says so: // Cut, which names the objects it copied). - let with_guides = explicit.is_none() || bool_or(p, "guides", false); + let with_guides = explicit.is_none() || guides; let is_explicit = explicit.is_some(); let targets = explicit.unwrap_or_else(|| st.selection.in_paint_order(&st.doc)); // Keep the selected ids themselves, without promoting compound members. Delete a selected diff --git a/crates/engine/src/cmd/effectcmd.rs b/crates/engine/src/cmd/effectcmd.rs index 1fcd4602e..f4c0073a3 100644 --- a/crates/engine/src/cmd/effectcmd.rs +++ b/crates/engine/src/cmd/effectcmd.rs @@ -16,7 +16,7 @@ use vectorcraft_render::effects; use super::appearance::{ItemTarget, aligned, appearance_targets, index_param, item_indices, item_target, item_target_at}; use super::expand::has_seeded; use super::rasterfx; -use super::{CommandSpec, EngineError, NodeId, Result, Session, always, bad, cmd, fileio, has_doc, str_param}; +use super::{CommandSpec, EngineError, NodeId, Result, Session, always, bad, cmd, fileio, has_doc, str_param, switch_or}; /// The effect commands of the Effect menu and the Appearance panel. pub fn specs() -> Vec { @@ -116,12 +116,13 @@ pub(crate) fn effect_targets(s: &Session, p: &Value, item: ItemTarget, index: us // Whether `item` names an item in the stack it is a row of (else the edit is object-level there). let anchor = roots.get(item.anchor(s, &roots)).and_then(|a| d.node(*a)); let anchor_level = anchor.map(|a| item.effects_item(&a.appearance, cmd)).transpose()?.flatten(); + let aligned = aligned(p, cmd)?; let mut out = Vec::with_capacity(roots.len()); for (id, item) in roots.iter().zip(per) { let (Some(item), Some(n)) = (item, d.node(*id)) else { continue }; let at = item.effects_item(&n.appearance, cmd)?; // An object-level effect this target shares with the first is where it is in this target. - let index = match (first.filter(|_| aligned(p)), at, anchor_level) { + let index = match (first.filter(|_| aligned), at, anchor_level) { (Some(f), None, None) => SharedStack::of(&[&*f.appearance, &*n.appearance]).effect_in(index, 1).unwrap_or(index), _ => index, }; @@ -236,6 +237,7 @@ pub(crate) fn apply(s: &mut Session, p: &Value) -> Result { } let effect = effects::new_effect(id, ¶ms).ok_or_else(|| bad(C, format!("unknown effect `{id}`")))?; let label = effects::effect_info(id).map(|e| e.label.trim_end_matches('…').to_string()).unwrap_or_default(); + aligned(p, C)?; if let Some(loose) = loose_for_pathfinder(s, p, id)? { let gid = s.edit(&label, |d, sel| { let gid = super::object::group_nodes(d, &loose)?; @@ -375,10 +377,10 @@ fn move_effect(s: &mut Session, p: &Value) -> Result { let to = index_param(p, "to", C)?; let src = item_target_at(s, p, "fromItem", C)?; let dst = if p.get("toItem").is_some() { item_target_at(s, p, "toItem", C)? } else { src }; - let copy = p.get("copy").and_then(Value::as_bool).unwrap_or(false); + let copy = switch_or(p, "copy", false, C)?; let roots = appearance_targets(s, p)?; // Where an aligned effect would go in each of several different stacks isn't one place. - if aligned(p) && super::appearance::appearances_differ(&s.doc()?.doc, &roots) { + if aligned(p, C)? && super::appearance::appearances_differ(&s.doc()?.doc, &roots) { return Err(bad(C, "effects of objects whose appearances differ can't be moved together")); } // Where the effect landed in each object it moved in, in the order of `ids`. diff --git a/crates/engine/src/cmd/maskedit.rs b/crates/engine/src/cmd/maskedit.rs index 83c7abf7c..65bf15086 100644 --- a/crates/engine/src/cmd/maskedit.rs +++ b/crates/engine/src/cmd/maskedit.rs @@ -14,7 +14,7 @@ use std::sync::Arc; use serde_json::{Value, json}; use vectorcraft_doc::{Document, MaskEdit, Node, NodeId, NodeKind, Selection}; -use super::{CommandSpec, EngineError, Result, Session, bad, cmd, has_doc, id_param}; +use super::{CommandSpec, EngineError, Result, Session, bad, cmd, has_doc, id_param, switch_or}; /// Name of the temporary mask-editing layer. pub const MASK_EDIT_LAYER: &str = "Opacity Mask Editing"; @@ -138,7 +138,7 @@ fn view_mask(s: &mut Session, p: &Value) -> Result { const C: &str = "transparency.viewOpacityMask"; let st = s.doc()?; let editing = st.doc.mask_edit.map(|m| m.object); - if !p.get("on").and_then(Value::as_bool).unwrap_or(st.shown_mask().is_none()) { + if !switch_or(p, "on", st.shown_mask().is_none(), C)? { let st = s.doc_mut()?; let id = st.mask_view.take(); st.revision += 1; diff --git a/crates/engine/src/cmd/path.rs b/crates/engine/src/cmd/path.rs index 27cba98b5..e0de6bd4a 100644 --- a/crates/engine/src/cmd/path.rs +++ b/crates/engine/src/cmd/path.rs @@ -1191,7 +1191,7 @@ fn convert_anchors(s: &mut Session, p: &Value) -> Result { pub(crate) fn delete_anchors(s: &mut Session, p: &Value) -> Result { // `guides: true` (Cut, #3226): the selected ruler guides go in the same undo step. - let with_guides = p.get("guides").and_then(Value::as_bool).unwrap_or(false); + let with_guides = switch_or(p, "guides", false, "path.deleteAnchors")?; let st = s.doc()?; let sel = st.selection.anchors.clone(); let segments = st.selection.segments.clone(); diff --git a/crates/engine/src/cmd/plugin.rs b/crates/engine/src/cmd/plugin.rs index cd03a4151..8d1bb0408 100644 --- a/crates/engine/src/cmd/plugin.rs +++ b/crates/engine/src/cmd/plugin.rs @@ -20,7 +20,7 @@ use serde_json::{Map, Value, json}; use vectorcraft_doc::{Effect, Node}; use vectorcraft_plugins::{Budget, Kind, Plugin, effect, export, objects, registry}; -use super::{CommandSpec, EngineError, NodeId, Result, Session, always, bad, cmd, has_doc, ids_param, str_param}; +use super::{CommandSpec, EngineError, NodeId, Result, Session, always, bad, cmd, has_doc, ids_param, str_param, switch_or}; /// What a plug-in error looks like to the user. A run the caller cancelled changed nothing. fn err(e: vectorcraft_plugins::Error) -> EngineError { @@ -288,12 +288,13 @@ fn load_given(c: &str, p: &Value) -> Result<(Plugin, Vec)> { /// `plugin.list` entry. fn install(s: &mut Session, p: &Value) -> Result { const C: &str = "plugin.install"; + let (replace, allow_heavy) = (switch_or(p, "replace", true, C)?, switch_or(p, "allowHeavy", false, C)?); let (plugin, bytes) = load_given(C, p)?; - if !p.get("replace").and_then(Value::as_bool).unwrap_or(true) && registry::get(plugin.id()).is_some() { + if !replace && registry::get(plugin.id()).is_some() { return Err(bad(C, format!("a plug-in with id {:?} is already installed", plugin.id()))); } // A heavy plug-in is installed once the user agrees: whoever installs it asks them first. - if plugin.manifest().budget == Budget::Heavy && !p.get("allowHeavy").and_then(Value::as_bool).unwrap_or(false) { + if plugin.manifest().budget == Budget::Heavy && !allow_heavy { let (minutes, times) = vectorcraft_plugins::heavy_terms(); return Err(bad( C, diff --git a/crates/engine/src/cmd/printtiling.rs b/crates/engine/src/cmd/printtiling.rs index 6c683ac79..3dabe747e 100644 --- a/crates/engine/src/cmd/printtiling.rs +++ b/crates/engine/src/cmd/printtiling.rs @@ -10,7 +10,7 @@ use vectorcraft_pdf::{TileOrigin, TilingPage}; use super::fileio::pdf::pdf_error; use super::print::{settings, to_json}; -use super::{CommandSpec, Result, Session, bad, bool_or, cmd, has_doc, index}; +use super::{CommandSpec, Result, Session, bad, bool_or, cmd, has_doc, index, switch_or}; const SET: &str = "print.tiling.set"; @@ -48,7 +48,7 @@ pub fn specs() -> Vec { fn show(s: &mut Session, p: &Value) -> Result { let st = s.doc_mut()?; - st.print_tiling = p.get("on").and_then(Value::as_bool).unwrap_or(!st.print_tiling); + st.print_tiling = switch_or(p, "on", !st.print_tiling, "view.printTiling")?; Ok(json!({ "on": st.print_tiling })) } diff --git a/crates/engine/src/cmd/textedit.rs b/crates/engine/src/cmd/textedit.rs index 33fa1a514..fba84acaf 100644 --- a/crates/engine/src/cmd/textedit.rs +++ b/crates/engine/src/cmd/textedit.rs @@ -1585,7 +1585,7 @@ fn create_in_path(s: &mut Session, p: &Value) -> Result { }; let kind = if on_path { TextKind::OnPath { path: path.clone(), start, end: None } } else { TextKind::Area { frame: path.clone() } }; let mut t = TextObject { - vertical: p.get("vertical").and_then(Value::as_bool).unwrap_or(false), + vertical: switch_or(p, "vertical", false, C)?, kind, xf: Affine::IDENTITY, runs: vec![TextRun { text, style, inline: None }], diff --git a/crates/engine/src/tests/appearance.rs b/crates/engine/src/tests/appearance.rs index ada5fdd2f..8d7023727 100644 --- a/crates/engine/src/tests/appearance.rs +++ b/crates/engine/src/tests/appearance.rs @@ -981,3 +981,36 @@ fn removing_shared_and_unshared_rows_removes_only_the_shared_ones_elsewhere() { assert!(node(&s, a).appearance.items.is_empty(), "a loses both"); assert_eq!(node(&s, b).appearance.items, fills_b, "b loses only its stroke, keeping both fills"); } + +#[test] +fn invalid_aligned_flags_leave_the_appearance_alone() { + let mut s = session(); + let id = rect(&mut s, 100.0); + run(&mut s, "effect.apply", json!({"effect": "distort.twist"})); + let before = node(&s, id); + let before_undo = s.doc().unwrap().history.undo.len(); + for (command, params) in [ + ("appearance.setItem", json!({"index": 0, "opacity": 50, "aligned": "true"})), + ("appearance.removeItem", json!({"index": 0, "aligned": 1})), + ("appearance.duplicateItem", json!({"index": 0, "aligned": "false"})), + ("appearance.moveItem", json!({"from": 0, "to": 1, "aligned": 0})), + ("stroke.set", json!({"item": 1, "weight": 5, "aligned": "true"})), + ("stroke.set", json!({"weight": 5, "aligned": "true"})), + ("transparency.set", json!({"item": 0, "opacity": 50, "aligned": "true"})), + ("effect.apply", json!({"effect": "distort.roughen", "item": 0, "aligned": "true"})), + ("effect.remove", json!({"index": 0, "aligned": "true"})), + ] { + assert!(s.execute(command, ¶ms).is_err(), "{command} accepted {params}"); + assert_eq!(node(&s, id), before, "{command} changed the object"); + assert_eq!(s.doc().unwrap().history.undo.len(), before_undo, "{command} recorded an undo step"); + } + // Null, as for every command, is the same as leaving `aligned` out. + run(&mut s, "appearance.removeItem", json!({"index": 0, "aligned": null})); + assert_eq!(node(&s, id).appearance.items.len(), before.appearance.items.len() - 1); + // A Pathfinder effect on loose objects groups them first: the flag is checked before that. + let other = rect(&mut s, 250.0); + run(&mut s, "select.set", json!({"ids": [id.0, other.0]})); + let before_undo = s.doc().unwrap().history.undo.len(); + assert!(s.execute("effect.apply", &json!({"effect": "pathfinder.add", "aligned": "true"})).is_err()); + assert_eq!(s.doc().unwrap().history.undo.len(), before_undo, "grouped the objects"); +} diff --git a/crates/engine/src/tests/effectedit.rs b/crates/engine/src/tests/effectedit.rs index f660928ea..2d10c3f5d 100644 --- a/crates/engine/src/tests/effectedit.rs +++ b/crates/engine/src/tests/effectedit.rs @@ -70,6 +70,29 @@ fn effects_move_and_copy_between_the_object_and_its_items() { assert!(s.execute("effect.move", &json!({"from": 0, "fromItem": 1, "to": 0, "toItem": 7})).is_err()); } +#[test] +fn invalid_effect_move_flags_leave_the_effects_alone() { + let (mut s, id) = session_with_rect(); + run(&mut s, "effect.apply", json!({"effect": "distort.twist"})); + let before_undo = s.doc().unwrap().history.undo.len(); + let to_fill = json!({"from": 0, "fromItem": null, "to": 0, "toItem": 0}); + for (key, value) in [("copy", json!("true")), ("copy", json!(1)), ("aligned", json!("false"))] { + let mut params = to_fill.clone(); + params[key] = value; + assert!(s.execute("effect.move", ¶ms).is_err(), "accepted {params}"); + assert_eq!(fx(&s, id, None), ["distort.twist"], "moved the effect for {params}"); + assert!(fx(&s, id, Some(0)).is_empty(), "copied the effect for {params}"); + assert_eq!(s.doc().unwrap().history.undo.len(), before_undo, "recorded undo for {params}"); + } + // Null, as for every command, is the same as leaving the flag out: a move. + let mut params = to_fill; + params["copy"] = Value::Null; + params["aligned"] = Value::Null; + run(&mut s, "effect.move", params); + assert!(fx(&s, id, None).is_empty()); + assert_eq!(fx(&s, id, Some(0)), ["distort.twist"]); +} + /// `effect.duplicate` copies a fill's effect right after it. #[test] fn duplicate_selected_works_for_effects() { diff --git a/crates/engine/src/tests/maskview.rs b/crates/engine/src/tests/maskview.rs index e222a136d..9809d09e7 100644 --- a/crates/engine/src/tests/maskview.rs +++ b/crates/engine/src/tests/maskview.rs @@ -69,3 +69,23 @@ fn view_opacity_mask_shows_its_luminance_and_hides_the_artwork() { assert!(s.execute("transparency.viewOpacityMask", &json!({"on": true, "id": other.0})).is_err()); assert!(s.doc().unwrap().mask_view.is_none()); } + +#[test] +fn invalid_view_opacity_mask_parameters_leave_the_view_unchanged() { + let mut s = Session::new(); + s.execute("file.new", &json!({"width": 300, "height": 300})).unwrap(); + let obj = rect(&mut s, 0.0, 200.0, "#ff0000"); + let m = rect(&mut s, 0.0, 100.0, "#00ff00"); + s.execute("select.set", &json!({"ids": [obj.0, m.0]})).unwrap(); + s.execute("transparency.makeOpacityMask", &json!({})).unwrap(); + let undo = s.doc().unwrap().history.undo.len(); + for params in [json!({"on": "true"}), json!({"on": 1})] { + assert!(s.execute("transparency.viewOpacityMask", ¶ms).is_err(), "accepted {params}"); + let st = s.doc().unwrap(); + assert!(st.doc.mask_edit.is_none() && st.shown_mask().is_none(), "showed the mask for {params}"); + assert_eq!(st.history.undo.len(), undo, "recorded undo for {params}"); + } + // Left out, or null as for every command, it toggles. + assert_eq!(s.execute("transparency.viewOpacityMask", &json!(null)).unwrap(), json!({"on": true, "id": obj.0})); + assert_eq!(s.execute("transparency.viewOpacityMask", &json!({"on": null})).unwrap(), json!({"on": false, "id": obj.0})); +} diff --git a/crates/engine/src/tests/menucmds.rs b/crates/engine/src/tests/menucmds.rs index 82dc07e55..ad862376c 100644 --- a/crates/engine/src/tests/menucmds.rs +++ b/crates/engine/src/tests/menucmds.rs @@ -3827,6 +3827,30 @@ fn guides_and_art_are_shift_selected_together() { assert_eq!(guides(&s).len(), 2); } +#[test] +fn invalid_clear_guides_parameters_delete_nothing() { + let mut s = session(); + let a = rect(&mut s, 10.0, 10.0, 40.0, 20.0); + s.execute("guide.add", &json!({"vertical": true, "pos": 200})).unwrap(); + sel(&mut s, &[a]); + s.execute("guide.select", &json!({"indexes": [0], "toggle": true})).unwrap(); + let n = undo_len(&s); + for params in [json!({"ids": [a.0], "guides": "true"}), json!({"guides": 1})] { + assert!(s.execute("edit.clear", ¶ms).is_err(), "accepted {params}"); + assert!(s.doc().unwrap().doc.node(a).is_some(), "deleted the art for {params}"); + assert_eq!((guides(&s).len(), undo_len(&s)), (1, n), "changed the document for {params}"); + } + // Direct-selected anchors take the same check. + s.execute("select.anchors", &json!({"id": a.0, "anchors": [[0, 1]], "mode": "set"})).unwrap(); + assert!(s.execute("edit.clear", &json!({"guides": "false"})).is_err()); + assert!(!s.doc().unwrap().selection.anchors.is_empty()); + assert_eq!(undo_len(&s), n); + // Null, as for every command, is the same as leaving `guides` out: with ids, the guides stay. + s.execute("edit.clear", &json!({"ids": [a.0], "guides": null})).unwrap(); + assert!(s.doc().unwrap().doc.node(a).is_none()); + assert_eq!(guides(&s).len(), 1); +} + /// When ruler guides and art are selected together, moving the art moves the guides as well; /// copy leaves both in place and moves the copies; single undo reverts both. #[test] diff --git a/crates/engine/src/tests/plugins.rs b/crates/engine/src/tests/plugins.rs index 1d0926f53..194c56178 100644 --- a/crates/engine/src/tests/plugins.rs +++ b/crates/engine/src/tests/plugins.rs @@ -493,6 +493,36 @@ fn a_heavy_filter_installs_once_the_user_agrees() { assert!(install(&mut s, DESATURATE).get("budget").is_none()); } +#[test] +fn invalid_install_flags_are_refused() { + let mut s = session(); + let id = "test.engine.heavy.flags"; + let data = vectorcraft_format::base64_encode(&heavy(id, 1)); + for allow in [json!("true"), json!(1)] { + let e = s.execute("plugin.install", &json!({"dataBase64": data, "allowHeavy": allow})).unwrap_err().to_string(); + assert!(e.contains("`allowHeavy` must be true or false"), "allowHeavy {allow}: {e}"); + assert!(vectorcraft_plugins::registry::get(id).is_none(), "installed with allowHeavy {allow}"); + } + // Null, as for every command, is the same as leaving the flag out. + let e = s.execute("plugin.install", &json!({"dataBase64": data, "allowHeavy": null})).unwrap_err().to_string(); + assert!(e.contains("heavy budget"), "{e}"); + run(&mut s, "plugin.install", json!({"dataBase64": data, "allowHeavy": true})); + for replace in [json!("false"), json!(0)] { + let e = s.execute("plugin.install", &json!({"dataBase64": data, "allowHeavy": true, "replace": replace})).unwrap_err().to_string(); + assert!(e.contains("`replace` must be true or false"), "replace {replace}: {e}"); + } + run(&mut s, "plugin.install", json!({"dataBase64": data, "allowHeavy": true, "replace": null})); + assert!(vectorcraft_plugins::registry::get(id).is_some()); + // A plug-in with the usual budget is refused too. + let light = "test.engine.light.flags"; + let manifest = format!(r#"{{"id":"{light}","name":"Test {light}","version":"1","kind":"filter"}}"#); + let output = json!({"objects": [{"path": triangle(), "name": "Plug-in"}]}); + let data = vectorcraft_format::base64_encode(&wat::parse_str(tpl::spinning(&manifest, 1, &output.to_string())).unwrap()); + let e = s.execute("plugin.install", &json!({"dataBase64": data, "allowHeavy": "false"})).unwrap_err().to_string(); + assert!(e.contains("`allowHeavy` must be true or false"), "{e}"); + assert!(vectorcraft_plugins::registry::get(light).is_none()); +} + /// The user agreed once: a heavy filter kept in the plug-in store is installed again at the next /// start without asking, with its budget. #[test] diff --git a/crates/engine/src/tests/printtiling.rs b/crates/engine/src/tests/printtiling.rs index 9d8282053..7bc37d8b1 100644 --- a/crates/engine/src/tests/printtiling.rs +++ b/crates/engine/src/tests/printtiling.rs @@ -182,3 +182,18 @@ fn settings_saved_before_the_tile_origin_still_load() { assert_eq!((set["copies"].clone(), set["tileOrigin"]["placed"].clone()), (json!(2), json!(false))); assert_eq!(tiling(&mut s, json!({}))["pages"].as_array().unwrap().len(), 1); } + +#[test] +fn invalid_print_tiling_parameters_leave_the_view_unchanged() { + let mut s = session(); + let undo = s.doc().unwrap().history.undo.len(); + for params in [json!({"on": "true"}), json!({"on": 1})] { + assert!(s.execute("view.printTiling", ¶ms).is_err(), "accepted {params}"); + let st = s.doc().unwrap(); + assert!(!st.print_tiling, "showed the tiling for {params}"); + assert_eq!(st.history.undo.len(), undo, "recorded undo for {params}"); + } + // Left out, or null as for every command, it toggles. + assert_eq!(s.execute("view.printTiling", &json!(null)).unwrap(), json!({"on": true})); + assert_eq!(s.execute("view.printTiling", &json!({"on": null})).unwrap(), json!({"on": false})); +} diff --git a/docs/mcp.md b/docs/mcp.md index 26f38f758..31e64ed56 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -1349,6 +1349,7 @@ then composite against the art below it). SVG has no non-isolated groups, so the (default: the mask being edited, else the first selected masked object) as greyscale coverage (white = opaque) and the mask is edited; called again it shows the artwork, still editing. It is view state of the open document: the canvas (and `ui.screenshot`) shows it, exports never do, and it ends when mask editing ends. +The command rejects an `on` value that is not a boolean. Omitting `on` toggles the current value. Isolation mode and mask editing exclude each other. In isolation mode `transparency.editOpacityMask` and `transparency.viewOpacityMask` are off, and `transparency.makeOpacityMask` refuses one target, an object or a layer, @@ -1433,6 +1434,8 @@ or unknown id leaves the document unchanged. An empty `ids` array does nothing. are kept, and selecting an ancestor together with its descendants deletes that subtree once. Without `ids`, direct-selected anchors or selected ruler guides retain their usual Clear behavior. Cut still removes the objects it copied, including a whole compound when a member is selected. +With `ids`, `guides: true` also deletes the selected ruler guides. The command rejects a `guides` value that is not a +boolean. `object.setProps` with `visible: false` or `locked: true` deselects every selected object that is hidden or locked after the call: the selected objects it hides or locks, selected objects inside them, and selected objects @@ -2058,6 +2061,8 @@ it (Alt-drag). `appearance.duplicateItem {index, to?}` / `{indices}` and `appear act on several fills/strokes at once, and `appearance.showAllHidden` makes every hidden fill, stroke and effect visible again. In remote mode, `effect.dialog {effect, index, item}` opens an applied effect's dialog prefilled (`ui.dialog.confirm` runs `effect.setParams`); see the control protocol for the "already applied" question. +`effect.move` rejects a `copy` value that is not a boolean, and every command that takes `aligned` rejects an `aligned` +value that is not a boolean. Effect dialogs use the targeted object, group, or layer shown in Appearance. The Transform dialog previews valid numbers as they are typed, including Copies, distances with units, and rotation. With Preview off, changes apply @@ -4940,6 +4945,7 @@ outside `tileRange` dimmer). The Print Tiling tool (`printTiling`) shows them to what is drawn, from the layout `print.preview` and both writers (PDF and PostScript) use, so the canvas and the printed job agree: `{tileOrigin, pages: [{artboard, number, printed, page, imageable}]}` (`[x0, y0, x1, y1]` in document space; every tile when tiling, else each page once, not once per ink). +`view.printTiling` rejects an `on` value that is not a boolean. Omitting `on` toggles the current value. Dragging with the tool puts the top-left corner of the first page's imageable area where the pointer is (snapping to the artboard's edges): `print.tiling.set {origin: [x, y], artboard?}` saves it with the print settings as @@ -5834,6 +5840,7 @@ that no built-in format has. A failing plug-in returns an error and leaves the d no file). A filter whose manifest asks for a heavy budget (minutes rather than a minute, `budget: "heavy"` in its `plugin.inspect {path | dataBase64}` entry) installs only with `allowHeavy: true`: ask the user first. `plugin.run` runs it where it is called, so the call may take minutes; `notifications/cancelled` stops it. +`plugin.install` rejects a `replace` or `allowHeavy` value that is not a boolean. ```json {"name":"run_command","arguments":{"command":"plugin.install","params":{"path":"/plugins/desaturate.wasm"}}}