diff --git a/crates/engine/src/cmd.rs b/crates/engine/src/cmd.rs index 98dfe3aaa..a26c75da2 100644 --- a/crates/engine/src/cmd.rs +++ b/crates/engine/src/cmd.rs @@ -575,21 +575,26 @@ pub(crate) fn named_id(p: &Value) -> Result> { } } -/// Objects a command targets: explicit `ids` param or the selection. -pub(crate) fn targets(s: &Session, p: &Value) -> Result> { - // Objects given are used as given: a value that isn't an object id fails rather than the - // command acting on the selection instead (#785). - match p.get("ids").filter(|v| !v.is_null()) { - Some(Value::Array(a)) => return a.iter().map(object_id).collect(), - Some(v) => return Err(EngineError::Other(format!("`ids` must be an array of object ids, not {v}"))), - None => {} +/// The object `p` gives as `key`, for a command that acts on the selection (or makes something new) +/// without it: `None` when `p` leaves it out or gives null. Any other value that isn't an object id +/// (`"abc"`, `"5"`, `true`, `{}`, `-1`, `2.5`) is an error under `cmd` that names `key`, never the +/// selection. The id is not looked up in the document. +pub(crate) fn object_param(p: &Value, key: &str, cmd: &str) -> Result> { + match p.get(key) { + None | Some(Value::Null) => Ok(None), + Some(Value::String(r)) if key == "id" && r.starts_with('$') => { + Err(bad(cmd, format!("`id` is the step reference {r:?}, which only `run` and `command_batch` resolve"))) + } + Some(v) => v.as_u64().map(|id| Some(NodeId(id))).ok_or_else(|| bad(cmd, format!("`{key}` must be a whole number"))), } - match p.get("id").filter(|v| !v.is_null()) { - Some(v @ Value::Number(_)) => Ok(vec![object_id(v)?]), - // A step reference a batch couldn't resolve (other strings are other ids, such as an - // effect's). - Some(Value::String(r)) if r.starts_with('$') => Err(unresolved_reference(r)), - _ => Ok(s.doc()?.selection.objects.clone()), +} + +/// Objects a command targets: its `ids`, else its `id` as `ids` of one ([`named_ids`]), else the +/// selection. An `ids` or `id` that isn't an object id is an error, never the selection. +pub(crate) fn targets(s: &Session, p: &Value) -> Result> { + match named_ids(p)? { + Some(ids) => Ok(ids), + None => Ok(s.doc()?.selection.objects.clone()), } } diff --git a/crates/engine/src/cmd/draw2/anchors.rs b/crates/engine/src/cmd/draw2/anchors.rs index df55e5207..dbca59b00 100644 --- a/crates/engine/src/cmd/draw2/anchors.rs +++ b/crates/engine/src/cmd/draw2/anchors.rs @@ -14,7 +14,7 @@ pub(super) fn curvature(s: &mut Session, p: &Value) -> Result { return Err(bad(C, "need at least one point")); } let sp = vectorcraft_tools::draw2::curvature_spline(&pts, switch_or(p, "closed", false, C)?); - if let Some(id) = id_param(p, "id") { + if let Some(id) = crate::cmd::object_param(p, "id", C)? { s.edit("Curvature", |d, sel| { *path_mut(d, id)? = PathData::single(sp); sel.set([id]); diff --git a/crates/engine/src/cmd/effectcmd.rs b/crates/engine/src/cmd/effectcmd.rs index e6568c5ea..21f43e0ee 100644 --- a/crates/engine/src/cmd/effectcmd.rs +++ b/crates/engine/src/cmd/effectcmd.rs @@ -222,6 +222,16 @@ pub(crate) const ONE_3D: &str = "An object takes one 3D effect. Edit the existin pub(crate) fn apply(s: &mut Session, p: &Value) -> Result { const C: &str = "effect.apply"; let id = str_param(p, "effect").or_else(|| str_param(p, "id")).ok_or_else(|| bad(C, "missing `effect` id"))?; + // An effect given as `id` names no objects: the targets are read with that `id` as null. + let effect_as_id; + let p = if str_param(p, "effect").is_none() { + let mut q = p.clone(); + q["id"] = Value::Null; + effect_as_id = q; + &effect_as_id + } else { + p + }; let mut params = p.get("params").cloned().unwrap_or(Value::Null); if !params.is_null() && !params.is_object() { return Err(bad(C, "`params` must be an object")); diff --git a/crates/engine/src/cmd/layerpanel.rs b/crates/engine/src/cmd/layerpanel.rs index cf5887074..0a81a947e 100644 --- a/crates/engine/src/cmd/layerpanel.rs +++ b/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, checked_id, cmd, has_doc, id_param, ok, switch_or}; +use super::{CommandSpec, Result, Session, bad, checked_id, cmd, has_doc, object_param, ok, switch_or}; use crate::EngineError; pub fn specs() -> Vec { @@ -336,7 +336,7 @@ fn release(s: &mut Session, p: &Value) -> Result { const C: &str = "layer.releaseToLayers"; let build = switch_or(p, "build", false, C)?; let st = s.doc()?; - let id = match id_param(p, "id") { + let id = match object_param(p, "id", C)? { Some(id) => id, None => match st.highlighted_rows().as_slice() { [id] => *id, @@ -434,8 +434,9 @@ fn reverse(s: &mut Session, p: &Value) -> Result { } fn locate(s: &mut Session, p: &Value) -> Result { + let id = object_param(p, "id", "layer.locate")?; let st = s.doc_mut()?; - let id = id_param(p, "id").or_else(|| st.selection.objects.last().copied()).ok_or_else(|| bad("layer.locate", "nothing selected"))?; + let id = id.or_else(|| st.selection.objects.last().copied()).ok_or_else(|| bad("layer.locate", "nothing selected"))?; let ancestry = st.doc.ancestry(id).ok_or(EngineError::NoNode(id))?; st.layer_rows = vec![id]; st.revision += 1; diff --git a/crates/engine/src/cmd/links.rs b/crates/engine/src/cmd/links.rs index 4573ea2c2..1f1cff613 100644 --- a/crates/engine/src/cmd/links.rs +++ b/crates/engine/src/cmd/links.rs @@ -32,7 +32,7 @@ use vectorcraft_doc::{Appearance, Document, ImageBlob, ImageObject, LinkInfo, No use vectorcraft_geom::{Affine, PathData, Rect, shapes}; use super::fileio::{self, RasterImage, absolute_path, file_created, file_stamp, read_file, write_file}; -use super::{CommandSpec, EngineError, Result, Session, bad, bool_or, cmd, has_doc, id_param, ids_param, str_param}; +use super::{CommandSpec, EngineError, Result, Session, bad, bool_or, cmd, has_doc, ids_param, object_param, str_param}; pub fn specs() -> Vec { vec![ @@ -1298,7 +1298,9 @@ fn selected_images(st: &crate::DocState, keep: impl Fn(&ImageObject) -> bool) -> /// `id`, else the first selected image. fn image_param(st: &crate::DocState, p: &Value, cmd: &str) -> Result { - let id = id_param(p, "id").or_else(|| selected_images(st, |_| true).first().copied()).ok_or_else(|| bad(cmd, "select an image, or pass id"))?; + let id = object_param(p, "id", cmd)? + .or_else(|| selected_images(st, |_| true).first().copied()) + .ok_or_else(|| bad(cmd, "select an image, or pass id"))?; match image_of(&st.doc, id) { Some(_) => Ok(id), None if st.doc.node(id).is_none() => Err(crate::EngineError::NoNode(id)), @@ -1327,7 +1329,7 @@ fn placement_of(d: &Document, id: NodeId) -> Option { /// `id`, else the first selected image or placed document. fn linked_param(st: &crate::DocState, p: &Value, cmd: &str) -> Result { let first = || st.selection.objects.iter().copied().find(|id| placement_of(&st.doc, *id).is_some()); - let id = id_param(p, "id").or_else(first).ok_or_else(|| bad(cmd, "select an image, or pass id"))?; + let id = object_param(p, "id", cmd)?.or_else(first).ok_or_else(|| bad(cmd, "select an image, or pass id"))?; match placement_of(&st.doc, id) { Some(_) => Ok(id), None if st.doc.node(id).is_none() => Err(crate::EngineError::NoNode(id)), diff --git a/crates/engine/src/cmd/live/blend.rs b/crates/engine/src/cmd/live/blend.rs index 9933b2071..7d7c21227 100644 --- a/crates/engine/src/cmd/live/blend.rs +++ b/crates/engine/src/cmd/live/blend.rs @@ -7,7 +7,7 @@ use super::common::{ use super::mesh::index_param; use super::{ Affine, Anchor, Appearance, Arc, Beside, BlendDefaults, BlendOrientation, BlendSpacing, BlendSpec, Document, EngineError, Node, NodeId, NodeKind, - Paint, PathData, Point, Result, Session, Spine, Value, Vec2, bad, f64_req, id_param, json, live, number_or, ok, str_param, switch_or, + Paint, PathData, Point, Result, Session, Spine, Value, Vec2, bad, f64_req, json, live, number_or, ok, str_param, switch_or, }; /// The first blend among `roots`. @@ -160,7 +160,7 @@ fn edit_spine( label: &str, f: impl FnOnce(&mut vectorcraft_geom::SubPath, &mut Vec, &mut [Arc]) -> Result, ) -> Result { - let id = match id_param(p, "id") { + let id = match crate::cmd::object_param(p, "id", cmd)? { Some(id) => id, None => *selected_of(s, is_blend).first().ok_or_else(|| bad(cmd, "select a blend or give `id`"))?, }; diff --git a/crates/engine/src/cmd/maskedit.rs b/crates/engine/src/cmd/maskedit.rs index 65bf15086..01721471c 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, switch_or}; +use super::{CommandSpec, EngineError, Result, Session, bad, cmd, has_doc, object_param, switch_or}; /// Name of the temporary mask-editing layer. pub const MASK_EDIT_LAYER: &str = "Opacity Mask Editing"; @@ -117,7 +117,7 @@ fn enter(s: &mut Session, p: &Value) -> Result { if st.doc.mask_edit.is_some() { return Err(bad(C, "already editing an opacity mask")); } - let id = id_param(p, "id").or_else(|| first_masked(st)).ok_or_else(|| bad(C, "select an object with an opacity mask"))?; + let id = object_param(p, "id", C)?.or_else(|| first_masked(st)).ok_or_else(|| bad(C, "select an object with an opacity mask"))?; let layer = start(s, id)?; Ok(json!({ "layer": layer.0 })) } @@ -136,6 +136,7 @@ fn start(s: &mut Session, id: NodeId) -> Result { fn view_mask(s: &mut Session, p: &Value) -> Result { const C: &str = "transparency.viewOpacityMask"; + let given = object_param(p, "id", C)?; let st = s.doc()?; let editing = st.doc.mask_edit.map(|m| m.object); if !switch_or(p, "on", st.shown_mask().is_none(), C)? { @@ -144,7 +145,7 @@ fn view_mask(s: &mut Session, p: &Value) -> Result { st.revision += 1; return Ok(json!({ "on": false, "id": id.map(|i| i.0) })); } - let id = id_param(p, "id").or(editing).or_else(|| first_masked(st)).ok_or_else(|| bad(C, "select an object with an opacity mask"))?; + let id = given.or(editing).or_else(|| first_masked(st)).ok_or_else(|| bad(C, "select an object with an opacity mask"))?; match editing { Some(e) if e != id => return Err(bad(C, "already editing another object's opacity mask")), Some(_) => {} diff --git a/crates/engine/src/cmd/object.rs b/crates/engine/src/cmd/object.rs index bac3540fa..ade05bd95 100644 --- a/crates/engine/src/cmd/object.rs +++ b/crates/engine/src/cmd/object.rs @@ -16,7 +16,7 @@ use super::edit::{duplicate_in, outermost, selected_roots, selected_roots_to_tra use super::opacitymask::percent; use super::{ CommandSpec, Result, Session, bad, bool_or, check_numbers, check_switches, cmd, f64_or, f64_req, has_doc, has_multi, has_selection, - has_selection_or_guides, id_param, ids_param, matrix_param, ok, point_param, str_param, targets, + has_selection_or_guides, ids_param, matrix_param, object_param, ok, point_param, str_param, targets, }; use crate::{EngineError, LastTransform}; diff --git a/crates/engine/src/cmd/object/arrange.rs b/crates/engine/src/cmd/object/arrange.rs index 2e7d2a8e4..7aef60fca 100644 --- a/crates/engine/src/cmd/object/arrange.rs +++ b/crates/engine/src/cmd/object/arrange.rs @@ -1,5 +1,5 @@ use super::align::choice; -use super::{Document, EngineError, Node, NodeId, NodeKind, Result, Session, Value, bad, id_param, json, ok, selected_roots}; +use super::{Document, EngineError, Node, NodeId, NodeKind, Result, Session, Value, bad, json, object_param, ok, selected_roots}; pub(super) enum Arrange { Front, @@ -293,7 +293,7 @@ pub(super) fn show_all(s: &mut Session, p: &Value) -> Result { } pub(super) fn isolate(s: &mut Session, p: &Value) -> Result { - let id = id_param(p, "id") + let id = object_param(p, "id", "object.isolate")? .or_else(|| s.active().and_then(|d| d.selection.objects.first().copied())) .ok_or_else(|| bad("object.isolate", "missing id"))?; let guides_locked = s.menu.guides_locked; diff --git a/crates/engine/src/cmd/place.rs b/crates/engine/src/cmd/place.rs index d78ca2782..0de33a22c 100644 --- a/crates/engine/src/cmd/place.rs +++ b/crates/engine/src/cmd/place.rs @@ -47,7 +47,7 @@ use vectorcraft_geom::{Affine, Point, Rect, shapes}; use vectorcraft_pdf::CropTo; use super::fileio::{self, Format, RasterImage}; -use super::{CommandSpec, Result, Session, always, bad, bool_or, cmd, finite_numbers, has_doc, id_param, str_param, unique_name}; +use super::{CommandSpec, Result, Session, always, bad, bool_or, cmd, finite_numbers, has_doc, object_param, str_param, unique_name}; use crate::{EngineError, MAX_COORD}; const PLACE: &str = "file.place"; @@ -866,7 +866,7 @@ pub fn read_info(p: &Value) -> Result { pub(crate) fn image_info(s: &mut Session, p: &Value) -> Result { const C: &str = "image.info"; let st = s.doc()?; - let id = match (id_param(p, "id"), &st.selection.objects[..]) { + let id = match (object_param(p, "id", C)?, &st.selection.objects[..]) { (Some(id), _) => id, (None, [id]) => *id, _ => return Err(bad(C, "select one image or pass id")), diff --git a/crates/engine/src/cmd/shaper.rs b/crates/engine/src/cmd/shaper.rs index 511e8ce4f..44ec938bc 100644 --- a/crates/engine/src/cmd/shaper.rs +++ b/crates/engine/src/cmd/shaper.rs @@ -2,7 +2,7 @@ //! art retained underneath. Recipes select topology regions; source edits re-evaluate them in //! the same undo transaction. Rendering and exporters only see ordinary groups and paths. use super::pathops::Beside; -use super::{CommandSpec, EngineError, Result, Session, bad, bool_or, checked_ids_param, cmd, f64_or, has_doc, id_param, point_param}; +use super::{CommandSpec, EngineError, Result, Session, bad, bool_or, checked_ids_param, cmd, f64_or, has_doc, id_param, object_param, point_param}; use serde_json::{Value, json}; use std::{ collections::{BTreeMap, HashMap}, @@ -785,15 +785,15 @@ fn end_isolation(s: &mut Session) -> Result<()> { st.isolation = editing_layer(&st.doc); Ok(()) } -fn target(s: &Session, p: &Value) -> Result { - let id = id_param(p, "id") +fn target(s: &Session, p: &Value, cmd: &str) -> Result { + let id = object_param(p, "id", cmd)? .or_else(|| s.active().and_then(|d| d.selection.objects.first().copied())) - .ok_or_else(|| bad("shaper.release", "select a Shaper group"))?; - s.doc()?.doc.node(id).filter(|n| n.extras().shaper.is_some()).ok_or_else(|| bad("shaper.release", "select a Shaper group"))?; + .ok_or_else(|| bad(cmd, "select a Shaper group"))?; + s.doc()?.doc.node(id).filter(|n| n.extras().shaper.is_some()).ok_or_else(|| bad(cmd, "select a Shaper group"))?; Ok(id) } fn release(s: &mut Session, p: &Value) -> Result { - let id = target(s, p)?; + let id = target(s, p, "shaper.release")?; let ids = s.edit("Release Shaper Group", |d, sel| { let original = d.node(id).map(sources).ok_or(EngineError::NoNode(id))?; let (parent, index, _) = d.position(id).ok_or(EngineError::NoNode(id))?; @@ -809,7 +809,7 @@ fn release(s: &mut Session, p: &Value) -> Result { Ok(json!({"ids": ids.iter().map(|i| i.0).collect::>()})) } fn expand(s: &mut Session, p: &Value) -> Result { - let id = target(s, p)?; + let id = target(s, p, "shaper.expand")?; s.edit("Expand Shaper Group", |d, sel| { let n = d.node_mut(id).ok_or(EngineError::NoNode(id))?; n.extras_mut().shaper = None; diff --git a/crates/engine/src/cmd/style.rs b/crates/engine/src/cmd/style.rs index a15ea9ff4..36780707f 100644 --- a/crates/engine/src/cmd/style.rs +++ b/crates/engine/src/cmd/style.rs @@ -20,7 +20,7 @@ use vectorcraft_color::Paint; use vectorcraft_doc::{Appearance, DEFAULT_GRAPHIC_STYLE, Document, GraphicStyle, Node, NodeId, NodeKind}; use vectorcraft_geom::Rect; -use super::{CommandSpec, Result, Session, always, bad, bool_or, cmd, has_doc, id_param, index, ok, str_param, unique_name}; +use super::{CommandSpec, Result, Session, always, bad, bool_or, cmd, has_doc, index, object_param, ok, str_param, unique_name}; use crate::EngineError; pub fn specs() -> Vec { @@ -290,7 +290,7 @@ fn style_index(d: &Document, name: &str) -> Result { /// The object a style is made from: `id`, else the first selected object. fn source<'a>(s: &'a Session, p: &Value, cmd: &str) -> Result<(NodeId, &'a Node)> { - let id = match id_param(p, "id") { + let id = match object_param(p, "id", cmd)? { Some(id) => id, None => super::edit::selected_roots(s)?.first().copied().ok_or_else(|| bad(cmd, "select an object (or pass `id`)"))?, }; diff --git a/crates/engine/src/cmd/symboledit.rs b/crates/engine/src/cmd/symboledit.rs index 0c365919b..618ef2ee3 100644 --- a/crates/engine/src/cmd/symboledit.rs +++ b/crates/engine/src/cmd/symboledit.rs @@ -20,7 +20,7 @@ use vectorcraft_doc::{Document, Node, NodeId, NodeKind, Selection, SymbolEdit, S use vectorcraft_geom::{Affine, Point}; use super::brushsym::{natural_size, place_xf}; -use super::{EngineError, Result, Session, bad, id_param, number_or, ok, str_param}; +use super::{EngineError, Result, Session, bad, number_or, object_param, ok, str_param}; /// The name of the undo step that enters symbol editing mode. const ENTER: &str = "Edit Symbol"; @@ -65,7 +65,7 @@ pub(crate) fn enter(s: &mut Session, p: &Value) -> Result { if st.doc.pattern_edit.is_some() { return Err(bad(C, "finish editing the pattern first")); } - let given = id_param(p, "id").map(|i| vec![i]).or_else(|| super::ids_param(p, "ids").filter(|v| !v.is_empty())); + let given = object_param(p, "id", C)?.map(|i| vec![i]).or_else(|| super::ids_param(p, "ids").filter(|v| !v.is_empty())); let (name, xf, instance) = match given { Some(ids) => { let symbols = ids.iter().map(|id| instance_of(&st.doc, *id)).collect::>>(); diff --git a/crates/engine/src/cmd/textedit.rs b/crates/engine/src/cmd/textedit.rs index fba84acaf..29d1de28f 100644 --- a/crates/engine/src/cmd/textedit.rs +++ b/crates/engine/src/cmd/textedit.rs @@ -1676,7 +1676,7 @@ pub(crate) fn type_ranges( at_caret: impl Fn(&str, usize) -> Option<(usize, usize)>, ) -> Result> { // (text, its range; None: all of it). - let targets: Vec<(NodeId, Option<(usize, usize)>)> = match id_param(p, "id") { + let targets: Vec<(NodeId, Option<(usize, usize)>)> = match super::object_param(p, "id", c)? { // An object that isn't type is refused as `ids` of it is. Some(id) if s.doc()?.doc.node(id).is_some_and(|n| !matches!(n.kind, NodeKind::Text(_))) => { return Err(bad(c, "no text objects selected")); diff --git a/crates/engine/src/cmd/textstyles.rs b/crates/engine/src/cmd/textstyles.rs index f8ce2d691..17b2f0f0c 100644 --- a/crates/engine/src/cmd/textstyles.rs +++ b/crates/engine/src/cmd/textstyles.rs @@ -12,7 +12,7 @@ use vectorcraft_doc::{CharStyle, Document, Node, NodeId, NodeKind, ParaStyle, Te use super::clipboard::resources::import_text_styles; use super::typecmd::{refresh_bounds, text_targets}; -use super::{CommandSpec, Result, Run, Session, bad, bool_or, has_doc, id_param, index, ok, str_param}; +use super::{CommandSpec, Result, Run, Session, bad, bool_or, has_doc, index, object_param, ok, str_param}; pub const NORMAL_CHAR: &str = "[Normal Character Style]"; pub const NORMAL_PARA: &str = "[Normal Paragraph Style]"; @@ -439,10 +439,11 @@ fn update_users(d: &mut Document, kind: Kind, name: &str, old: &Map Result>> { +/// Paragraph capture always samples the paragraph at `start`. An `id` that isn't an object id is +/// an error under the `verb` command of `kind`. +fn selection_attrs(s: &Session, p: &Value, kind: Kind, verb: &str) -> Result>> { let d = &s.doc()?.doc; - let id = match id_param(p, "id") { + let id = match object_param(p, "id", &kind.cmd(verb))? { Some(id) => Some(id), None => text_targets(s, &json!({}), "").ok().and_then(|v| v.first().copied()), }; @@ -464,14 +465,14 @@ fn selection_attrs(s: &Session, p: &Value, kind: Kind) -> Result Result>> { - selection_attrs(s, &json!({}), if para { Kind::Para } else { Kind::Char }) + selection_attrs(s, &json!({}), if para { Kind::Para } else { Kind::Char }, "new") } /// The attributes `charStyle.new` (`para`: `paraStyle.new`) would give a style made now from /// `target` (its `id`, `start` and `end`; empty for the selected objects). `None` without text /// there. pub fn target_style_attrs(s: &Session, target: &Value, para: bool) -> Option> { - selection_attrs(s, target, if para { Kind::Para } else { Kind::Char }).ok().flatten() + selection_attrs(s, target, if para { Kind::Para } else { Kind::Char }, "new").ok().flatten() } /// The attributes a new character or paragraph style receives when no text is selected. @@ -644,7 +645,7 @@ fn new(s: &mut Session, p: &Value, kind: Kind) -> Result { /// (`charStyle.duplicate`), which `attrs` keeps as [`attrs_param`] keeps the target text's. fn new_from(s: &mut Session, p: &Value, kind: Kind, copied: Option<&Map>) -> Result { let c = kind.cmd("new"); - let target = selection_attrs(s, p, kind)?; + let target = selection_attrs(s, p, kind, "new")?; let attrs = match attrs_param(&s.doc()?.doc, p, kind, "new", copied.or(target.as_ref()))? { Some(a) => a, None => match target { @@ -685,8 +686,9 @@ fn apply(s: &mut Session, p: &Value, kind: Kind) -> Result { let style_name = (name != kind.normal()).then(|| name.clone()); // A Type tool range or whole text objects. let text_range = super::typecmd::TextRange::parse(p, &c)?; - let range = id_param(p, "id").filter(|_| text_range.is_some()); - let ids = match range.or_else(|| id_param(p, "id")) { + let given = object_param(p, "id", &c)?; + let range = given.filter(|_| text_range.is_some()); + let ids = match given { Some(id) => vec![id], None => text_targets(s, p, &c)?, }; @@ -767,7 +769,7 @@ fn redefine_to(s: &mut Session, kind: Kind, name: &str, new_attrs: Map Result { let name = name_param(p, kind, "redefine")?.to_string(); - let attrs = selection_attrs(s, p, kind)?.ok_or_else(|| bad(&kind.cmd("redefine"), "select text to redefine the style from"))?; + let attrs = selection_attrs(s, p, kind, "redefine")?.ok_or_else(|| bad(&kind.cmd("redefine"), "select text to redefine the style from"))?; redefine_to(s, kind, &name, attrs, "redefine") } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index e2fe63c17..b9e247ed9 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -3617,16 +3617,18 @@ fn given_targets_are_used_as_given_and_range_style_takes_the_selected_type() { } /// #785: commands whose `ids` or `id` the shared check doesn't read (it reads `id` where it stands -/// for `ids` of one) refuse one that isn't an object id. They don't act on the selection or the -/// current layer instead, and the document and its undo history stay as they are. +/// for `ids` of one) refuse one that isn't an object id. They don't act on the selection, the +/// current layer or a new object instead, and the document, the selection and the undo history +/// stay as they are. #[test] fn malformed_ids_are_refused_by_commands_that_default_to_the_selection() { let mut s = session(); let a = rect(&mut s, 100.0, 100.0, 80.0, 60.0); let b = rect(&mut s, 140.0, 120.0, 80.0, 60.0); let graph = s.execute("graph.create", &json!({"type": "column", "x": 400, "y": 300, "width": 120, "height": 80, "rows": [[1, 2]]})).unwrap(); + let text = s.execute("text.create", &json!({"x": 10, "y": 60, "text": "type", "size": 30})).unwrap()["id"].clone(); let ids = [json!([a.0, "x"]), json!(3)]; - let id = [json!("2"), json!("$1.id"), json!(2.5)]; + let id = [json!("2"), json!("$1.id"), json!(2.5), json!("abc"), json!(true), json!({})]; let commands = [ ("object.blend.make", json!({}), "ids", &ids[..], json!([a.0, b.0])), ("object.envelope.makeWithWarp", json!({}), "ids", &ids[..], json!([a.0, b.0])), @@ -3636,6 +3638,26 @@ fn malformed_ids_are_refused_by_commands_that_default_to_the_selection() { ("perspective.editText", json!({}), "id", &id[..], json!([a.0])), ("layer.clippingMask.toggle", json!({}), "id", &id[..], json!([a.0, b.0])), ("layer.newSublayer", json!({}), "parent", &id[..], json!([a.0])), + ("object.isolate", json!({}), "id", &id[..], json!([a.0])), + ("graphicStyle.new", json!({"name": "From a"}), "id", &id[..], json!([a.0])), + ("charStyle.new", json!({"name": "From type"}), "id", &id[..], json!([text])), + ("paraStyle.new", json!({"name": "From type"}), "id", &id[..], json!([text])), + ("charStyle.apply", json!({"name": "[Normal Character Style]"}), "id", &id[..], json!([text])), + ("type.step", json!({"attribute": "size"}), "id", &id[..], json!([text])), + ("layer.releaseToLayers", json!({"build": true}), "id", &id[..], json!([a.0])), + ("layer.locate", json!({}), "id", &id[..], json!([a.0])), + ("transparency.editOpacityMask", json!({}), "id", &id[..], json!([a.0])), + ("transparency.viewOpacityMask", json!({}), "id", &id[..], json!([a.0])), + ("links.info", json!({}), "id", &id[..], json!([a.0])), + ("links.unembed", json!({}), "id", &id[..], json!([a.0])), + ("links.goTo", json!({}), "id", &id[..], json!([a.0])), + ("shaper.release", json!({}), "id", &id[..], json!([a.0])), + ("shaper.expand", json!({}), "id", &id[..], json!([a.0])), + ("image.info", json!({}), "id", &id[..], json!([a.0])), + ("symbol.edit", json!({}), "id", &id[..], json!([a.0])), + ("object.blend.spine.addAnchor", json!({"x": 120, "y": 110}), "id", &id[..], json!([a.0])), + ("path.curvature", json!({"points": [[0, 0], [50, 40], [100, 0]]}), "id", &id[..], json!([a.0])), + ("css.export", json!({"unnamed": true}), "id", &id[..], json!([a.0])), ]; let mut wrong = vec![]; for (cmd, params, key, values, selected) in commands { @@ -3645,21 +3667,44 @@ fn malformed_ids_are_refused_by_commands_that_default_to_the_selection() { if let Some(p) = p.as_object_mut() { p.insert(key.into(), bad.clone()); } - let (doc, undo) = (s.doc().unwrap().doc.clone(), s.doc().unwrap().history.undo.len()); + let (doc, selection, undo) = (s.doc().unwrap().doc.clone(), s.doc().unwrap().selection.clone(), s.doc().unwrap().history.undo.len()); let r = s.execute(cmd, &p); - let refused = r.as_ref().err().map(|e| e.to_string()).is_some_and(|e| e.contains("object id") || e.contains("step reference")); - let same = s.doc().unwrap().doc == doc && s.doc().unwrap().history.undo.len() == undo; + let named = |e: &str| e.contains("object id") || e.contains("step reference") || e.contains(&format!("`{key}` must be")); + let refused = r.as_ref().err().map(|e| e.to_string()).is_some_and(|e| named(&e)); + let same = s.doc().unwrap().doc == doc && s.doc().unwrap().selection == selection && s.doc().unwrap().history.undo.len() == undo; if !(refused && same) { wrong.push(format!("{cmd} {p}: {r:?}, document unchanged: {same}")); } if !same { - s.execute("edit.undo", &json!({})).ok(); + let st = s.doc_mut().unwrap(); + (st.doc, st.selection) = (doc, selection); + st.history.undo.truncate(undo); } } } assert!(wrong.is_empty(), "{}", wrong.join("\n")); } +/// An omitted or null `id` still means the selection, and an object id names that object. +/// `effect.apply` with the effect given as `id` (its older key) still applies it to the selection, +/// as one undo step. +#[test] +fn an_omitted_id_still_means_the_selection_and_effect_apply_takes_an_effect_as_id() { + let mut s = session(); + let a = rect(&mut s, 100.0, 100.0, 80.0, 60.0); + s.execute("select.set", &json!({"ids": [a.0]})).unwrap(); + for p in [json!({}), json!({"id": null}), json!({"id": a.0})] { + assert_eq!(s.execute("layer.locate", &p).unwrap()["id"], a.0, "{p}"); + } + for p in [json!({"unnamed": true}), json!({"id": null, "unnamed": true}), json!({"id": a.0, "unnamed": true})] { + assert_eq!(s.execute("css.export", &p).unwrap()["rules"], 1, "{p}"); + } + let undo = s.doc().unwrap().history.undo.len(); + let r = s.execute("effect.apply", &json!({"id": "stylize.dropShadow"})).unwrap(); + assert_eq!(r["ids"], json!([a.0])); + assert_eq!(s.doc().unwrap().history.undo.len(), undo + 1); +} + #[test] fn malformed_anchor_selection_never_replaces_or_partially_changes_selection() { let mut s = session(); diff --git a/docs/mcp.md b/docs/mcp.md index 96c0f72dc..98bdf6739 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -380,8 +380,14 @@ explicit `ids`, which stand in for the selection, so they work with nothing sele `ids?|id?` (or `id?, ids?: … (instead of id)`), `id` is `ids` of one object: `{"id": 7}` does what `{"ids": [7]}` does, with or without a selection, and an `id` or `ids` that is malformed (not a non-negative integer) or names no object is refused under the command's name, never replaced by the selection (`effect.apply {"id": "warp.arc"}` -without `effect` names the effect). A command that leaves out an object selected inside a selected group also -leaves it out when `ids` list it together with a group that holds it. +without `effect` names the effect). Other 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`, `graphicStyle.new`, +`graphicStyle.redefine`, `charStyle.new`, `charStyle.apply`, `charStyle.redefine` and their `paraStyle.*` +counterparts, `image.info`, `links.info`, `links.goTo`, `links.unembed`, `shaper.release`, `shaper.expand`, +`transparency.editOpacityMask`, `transparency.viewOpacityMask`, `symbol.edit`, the `object.blend.spine.*` commands +and `path.curvature`) refuse an `id` that is not a non-negative integer (`"abc"`, `"5"`, `true`) with an error and +change nothing; an omitted or null `id` keeps the default. A command that leaves out an object selected inside a +selected group also leaves it out when `ids` list it together with a group that holds it. Paint values (`fill`, `stroke`) accept `"#rrggbb"`, `"none"`, `[r,g,b]` (0..1), `{"c","m","y","k"}`, `{"gray"}`, or a full `paint.setFill` params object (`{"gradient": …}`, `{"swatch": "name"}`). diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index dcd2f3a06..a0d1be2b2 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -805,6 +805,7 @@ Moved from the ROADMAP's "Shipped so far" on 2026-10-10. Update it when a featur Edit › Keyboard Shortcuts lists each of the Align panel's Align Objects, Distribute Objects and Distribute Spacing buttons under Align, with no default shortcut, as the reference app's [release notes](https://web.archive.org/web/20261006170350/https://helpx.adobe.com/illustrator/desktop/new-features/release-notes.html) describe for its Align and Distribute commands. A shortcut set there runs what a click on the button runs, with the Align To choice, the key object and the Distribute Spacing value (`align.horizontalLeft` and the other `align.*` UI commands). 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. 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.