Merge pull request #6163 from dotCooCoo/fix/targets-id-param

Commands: an id that is not an object id is refused by commands that otherwise use the selection

crates/engine/src/cmd.rs

@@ -575,21 +575,26 @@ pub(crate) fn named_id(p: &Value) -> Result<Option<NodeId>> {
     }
 }
 
-/// Objects a command targets: explicit `ids` param or the selection.
-pub(crate) fn targets(s: &Session, p: &Value) -> Result<Vec<NodeId>> {
-    // 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<Option<NodeId>> {
+    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<Vec<NodeId>> {
+    match named_ids(p)? {
+        Some(ids) => Ok(ids),
+        None => Ok(s.doc()?.selection.objects.clone()),
     }
 }
 

crates/engine/src/cmd/draw2/anchors.rs

@@ -14,7 +14,7 @@ pub(super) fn curvature(s: &mut Session, p: &Value) -> Result<Value> {
         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]);

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<Value> {
     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"));

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<CommandSpec> {
@@ -336,7 +336,7 @@ fn release(s: &mut Session, p: &Value) -> Result<Value> {
     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<Value> {
 }
 
 fn locate(s: &mut Session, p: &Value) -> Result<Value> {
+    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;

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<CommandSpec> {
     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<NodeId> {
-    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<PlacementOptions> {
 /// `id`, else the first selected image or placed document.
 fn linked_param(st: &crate::DocState, p: &Value, cmd: &str) -> Result<NodeId> {
     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)),

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<T>(
     label: &str,
     f: impl FnOnce(&mut vectorcraft_geom::SubPath, &mut Vec<usize>, &mut [Arc<Node>]) -> Result<T>,
 ) -> Result<T> {
-    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`"))?,
     };

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<Value> {
     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<NodeId> {
 
 fn view_mask(s: &mut Session, p: &Value) -> Result<Value> {
     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<Value> {
         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(_) => {}

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};
 

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<Value> {
 }
 
 pub(super) fn isolate(s: &mut Session, p: &Value) -> Result<Value> {
-    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;

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<Value> {
 pub(crate) fn image_info(s: &mut Session, p: &Value) -> Result<Value> {
     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")),

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<NodeId> {
-    let id = id_param(p, "id")
+fn target(s: &Session, p: &Value, cmd: &str) -> Result<NodeId> {
+    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<Value> {
-    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<Value> {
     Ok(json!({"ids": ids.iter().map(|i| i.0).collect::<Vec<_>>()}))
 }
 fn expand(s: &mut Session, p: &Value) -> Result<Value> {
-    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;

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<CommandSpec> {
@@ -290,7 +290,7 @@ fn style_index(d: &Document, name: &str) -> Result<usize> {
 
 /// 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`)"))?,
     };

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<Value> {
     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::<Option<Vec<_>>>();

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<Vec<(NodeId, usize, usize, bool)>> {
     // (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"));

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<String, Valu
 
 /// The selected text's attributes: the first selected character for a nonempty range,
 /// the insertion style at a lone `start`, or the first selected text object's first run.
-/// Paragraph capture always samples the paragraph at `start`.
-fn selection_attrs(s: &Session, p: &Value, kind: Kind) -> Result<Option<Map<String, Value>>> {
+/// 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<Option<Map<String, Value>>> {
     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<Option<Map<Stri
 /// The selected text's character (`para`: paragraph) attributes, every one of them: what a
 /// library keeps of a text style. `None` without text selected.
 pub(crate) fn selected_style_attrs(s: &Session, para: bool) -> Result<Option<Map<String, Value>>> {
-    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<Map<String, Value>> {
-    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<Value> {
 /// (`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<String, Value>>) -> Result<Value> {
     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<Value> {
     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<String, V
 
 fn redefine(s: &mut Session, p: &Value, kind: Kind) -> Result<Value> {
     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")
 }
 

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();

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"}`).

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.