From 7e69c82b89f8d8bf92e43494703714b2072710a4 Mon Sep 17 00:00:00 2001 From: japbcoelho Date: Mon, 5 Oct 2026 22:44:37 -0300 Subject: [PATCH] Engine: commands given ids run without a selection A command that documents ids (object.textFrameOptions, transform.rotate, ...) acts on the objects a call names, yet its enabled check looked only at the selection. With a text caret or nothing selected, {"ids": [5]} failed with: command `object.textFrameOptions` is not available right now: nothing selected Execute now lets such a call through when every named object exists, and says "no object with id N" when one doesn't. transform.set, object.arrange and edit.stepAndRepeat, which already acted on ids, now document them. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/mod.rs | 17 ++++++++++++++++- crates/engine/src/cmd/object.rs | 31 ++++++++++++++++++++++++++++--- crates/engine/src/lib.rs | 11 ++++++++++- 3 files changed, 54 insertions(+), 5 deletions(-) diff --git a/crates/engine/src/cmd/mod.rs b/crates/engine/src/cmd/mod.rs index d1bb82d..ccc605c 100644 --- a/crates/engine/src/cmd/mod.rs +++ b/crates/engine/src/cmd/mod.rs @@ -113,9 +113,12 @@ pub fn always(_: &Session) -> std::result::Result<(), String> { pub fn has_doc(s: &Session) -> std::result::Result<(), String> { s.active().map(|_| ()).ok_or_else(|| "no document open".into()) } +/// Why [`has_selection`] disables a command. A command that documents `ids` still runs when the +/// call names its objects (see [`named_targets`]). +pub const NOTHING_SELECTED: &str = "nothing selected"; pub fn has_selection(s: &Session) -> std::result::Result<(), String> { has_doc(s)?; - if s.active().is_some_and(|d| !d.selection.items.is_empty()) { Ok(()) } else { Err("nothing selected".into()) } + if s.active().is_some_and(|d| !d.selection.items.is_empty()) { Ok(()) } else { Err(NOTHING_SELECTED.into()) } } pub fn has_text(s: &Session) -> std::result::Result<(), String> { has_doc(s)?; @@ -260,6 +263,18 @@ pub(crate) fn spread_param(p: &Value, key: &str) -> SpreadRef { } } +/// The objects a call names with a non-empty `ids` or an `id`, for a command whose params +/// document `ids` (it acts on [`targets`]). `None` when the call names none. +pub(crate) fn named_targets(spec: &CommandSpec, p: &Value) -> Option> { + if !spec.params.split(|c: char| !c.is_ascii_alphanumeric()).any(|w| w == "ids") { + return None; + } + if let Some(ids) = ids_param(p, "ids").filter(|v| !v.is_empty()) { + return Some(ids); + } + id_param(p, "id").map(|i| vec![i]) +} + /// Targets: `ids` / `id` params or the selection. pub(crate) fn targets(s: &Session, p: &Value) -> Result> { if let Some(ids) = ids_param(p, "ids") { diff --git a/crates/engine/src/cmd/object.rs b/crates/engine/src/cmd/object.rs index 489438a..06e05cc 100644 --- a/crates/engine/src/cmd/object.rs +++ b/crates/engine/src/cmd/object.rs @@ -114,12 +114,12 @@ pub fn specs() -> Vec { "Transform Panel", [], None, - "{x?, y?, width?, height?, scaleX? (%), scaleY? (%), rotation? (°), shear? (°), ref?: 0..8} — reference-point based geometry; rotation and shear are absolute (Transformations are Totals decides whether nested objects measure them on the pasteboard)", + "{x?, y?, width?, height?, scaleX? (%), scaleY? (%), rotation? (°), shear? (°), ref?: 0..8, ids?} — reference-point based geometry; rotation and shear are absolute (Transformations are Totals decides whether nested objects measure them on the pasteboard)", has_selection, transform_set ), cmd!(query "transform.info", "Transform Values", [], None, "{ids?} → {scaleX, scaleY (%), rotation, shear (°), content?: the same for a placed graphic} of the first target", has_selection, transform_info), - cmd!("object.arrange", "Arrange", ["Object", "Arrange"], None, "{to: front|forward|backward|back}", has_selection, arrange), + cmd!("object.arrange", "Arrange", ["Object", "Arrange"], None, "{to: front|forward|backward|back, ids?}", has_selection, arrange), cmd!("object.bringToFront", "Bring to Front", ["Object", "Arrange"], Some("Cmd+Shift+]"), "{}", has_selection, |s, _| s .execute("object.arrange", &json!({"to": "front"}))), cmd!("object.bringForward", "Bring Forward", ["Object", "Arrange"], Some("Cmd+]"), "{}", has_selection, |s, _| s @@ -555,7 +555,7 @@ pub fn specs() -> Vec { "Step and Repeat…", ["Edit"], Some("Cmd+Alt+U"), - "{count?: 1, dx?, dy?, rows?, columns?} — copies of the selection offset by (dx, dy); with rows/columns, a grid", + "{count?: 1, dx?, dy?, rows?, columns?, ids?} — copies of the selection (or ids) offset by (dx, dy); with rows/columns, a grid", has_selection, step_and_repeat ), @@ -2390,3 +2390,28 @@ mod line_end_tests { assert_eq!(story(&s).paras.len(), 4); } } + +#[cfg(test)] +mod named_target_tests { + use super::*; + + /// With a text caret (or nothing) selected, a command given `ids` failed + /// with "command `object.textFrameOptions` is not available right now: nothing selected". + #[test] + fn ids_act_without_a_selection() { + let mut s = Session::new(); + s.execute("file.new", &json!({})).unwrap(); + let r = s.execute("frame.create", &json!({"rect": [36, 36, 300, 300], "content": "text", "text": "Hello"})).unwrap(); + assert!(s.doc().unwrap().selection.items.is_empty(), "frame.create leaves a text caret"); + let e = s.execute("object.textFrameOptions", &json!({"columns": 2})).unwrap_err().to_string(); + assert!(e.contains("nothing selected"), "{e}"); + s.execute("object.textFrameOptions", &json!({"ids": [r["id"]], "columns": 2})).unwrap(); + let id = ItemId(r["id"].as_u64().unwrap()); + let columns = |s: &Session| s.doc().unwrap().doc.item(id).and_then(|i| i.text_frame().map(|t| t.options.columns)); + assert_eq!(columns(&s), Some(2)); + s.execute("transform.set", &json!({"id": r["id"], "x": 100, "ref": 0})).unwrap(); + let e = s.execute("object.textFrameOptions", &json!({"ids": [99999], "columns": 1})).unwrap_err().to_string(); + assert!(e.contains("no object with id 99999"), "{e}"); + assert_eq!(columns(&s), Some(2)); + } +} diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 793a55d..029b7b8 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -354,7 +354,16 @@ impl Session { fn execute_unguarded(&mut self, id: &str, params: &Value) -> Result { let spec = find_command(id).ok_or_else(|| EngineError::UnknownCommand(id.into()))?; - (spec.enabled)(self).map_err(|e| EngineError::Disabled(id.into(), e))?; + if let Err(e) = (spec.enabled)(self) { + // A command that takes `ids` acts on the objects the call names, so it needs no + // selection: with a text caret or nothing selected, `{"ids": [5]}` still runs. + let named = if e == cmd::NOTHING_SELECTED { cmd::named_targets(spec, params) } else { None }; + let Some(ids) = named else { return Err(EngineError::Disabled(id.into(), e)) }; + let st = self.doc()?; + if let Some(gone) = ids.iter().find(|i| st.doc.item(**i).is_none()) { + return Err(EngineError::Disabled(id.into(), format!("no object with id {}", gone.0))); + } + } let before = self.active().map(|d| (d.uid, d.doc.clone())); let r = (spec.run)(self, params)?; // Undo, redo or deleting layers can take the active layer away: new objects would land on