diff --git a/crates/engine/src/cmd/params.rs b/crates/engine/src/cmd/params.rs index a5433a68..13142f25 100644 --- a/crates/engine/src/cmd/params.rs +++ b/crates/engine/src/cmd/params.rs @@ -150,6 +150,12 @@ fn accepted(doc: &'static str, depth: usize) -> Option> { Some(keys) } +/// Does the description `doc` name `key` as a parameter (its own keys, and those of the commands +/// its `…` entries name)? +pub(crate) fn takes_key(doc: &'static str, key: &str) -> bool { + doc_keys(doc).is_ok_and(|parsed| parsed.keys.contains(&key)) || accepted(doc, 0).is_some_and(|keys| keys.contains(&key)) +} + /// The edits (insert, delete, replace, swap of neighbours) that turn `a` into `b`, ignoring case. fn distance(a: &str, b: &str) -> usize { let (a, b): (Vec, Vec) = (a.chars().map(|c| c.to_ascii_lowercase()).collect(), b.chars().map(|c| c.to_ascii_lowercase()).collect()); diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 1658e831..f63c7be4 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -1300,6 +1300,17 @@ impl Session { true } + /// `spec`'s `enabled` with `ids` selected instead of the selection, which is then put back. + fn enabled_with_selection(&mut self, spec: &CommandSpec, ids: Vec) -> std::result::Result<(), String> { + let Some(d) = self.active_mut() else { return (spec.enabled)(self) }; + let old = std::mem::replace(&mut d.selection, Selection { objects: ids, ..Selection::default() }); + let r = (spec.enabled)(self); + if let Some(d) = self.active_mut() { + d.selection = old; + } + r + } + /// Execute a command by id. This is THE entry point for every frontend. pub fn execute(&mut self, id: &str, params: &Value) -> Result { let spec = find_command(id).ok_or_else(|| EngineError::UnknownCommand(id.to_string()))?; @@ -1309,7 +1320,15 @@ impl Session { cmd::params::check_params(id, params)?; // Clear's menu needs a selection; an agent supplying explicit targets only needs a // document. The command validates those targets before making any edit. - let enabled = if id == "edit.clear" && params.get("ids").is_some() { cmd::has_doc(self) } else { (spec.enabled)(self) }; + let enabled = if id == "edit.clear" && params.get("ids").is_some() { + cmd::has_doc(self) + } else if let Some(ids) = given_ids(spec.params, params) { + // Objects given as `ids` stand in for the selection: the command is enabled as it is + // with them selected (#1575). + self.enabled_with_selection(spec, ids) + } else { + (spec.enabled)(self) + }; if let Err(why) = enabled { return Err(EngineError::Disabled(id.to_string(), why)); } @@ -1770,6 +1789,8 @@ mod tests_halftone; #[cfg(test)] mod tests_history_mem; #[cfg(test)] +mod tests_ids_without_selection; +#[cfg(test)] mod tests_inline; #[cfg(test)] mod tests_journal; @@ -1955,3 +1976,10 @@ mod tests_widthprofiles; mod tests_widthtool; #[cfg(test)] mod tests_xform; + +/// The non-empty `ids` given to a command whose params description names `ids` as a key (the +/// key callers outside the app may pass it, [`cmd::params::takes_key`]). +fn given_ids(description: &'static str, params: &Value) -> Option> { + let ids = cmd::ids_param(params, "ids").filter(|ids| !ids.is_empty())?; + cmd::params::takes_key(description, "ids").then_some(ids) +} diff --git a/crates/engine/src/tests_ids_without_selection.rs b/crates/engine/src/tests_ids_without_selection.rs new file mode 100644 index 00000000..710fe7d2 --- /dev/null +++ b/crates/engine/src/tests_ids_without_selection.rs @@ -0,0 +1,83 @@ +//! #1575: commands that take `ids` work on the objects given, with nothing selected; the +//! selection is left as it was. + +use serde_json::json; + +use super::*; + +fn doc_with_rect() -> (Session, u64) { + let mut s = Session::new(); + s.execute("file.new", &json!({"width": 200, "height": 200})).unwrap(); + let id = s.execute("shape.rectangle", &json!({"x": 10, "y": 10, "width": 50, "height": 40})).unwrap()["id"].as_u64().unwrap(); + s.execute("select.none", &json!({})).unwrap(); + (s, id) +} + +#[test] +fn ids_stand_in_for_the_selection() { + for (cmd, params) in [ + ("object.repeat.radial", json!({})), + ("edit.colors.invert", json!({})), + ("object.pattern.make", json!({"name": "P"})), + ("symbol.new", json!({"name": "S"})), + ("object.mesh.create", json!({})), + ("object.makePixelPerfect", json!({})), + ] { + let (mut s, id) = doc_with_rect(); + let mut p = params; + p["ids"] = json!([id]); + let r = s.execute(cmd, &p); + assert!(r.is_ok(), "{cmd}: {r:?}"); + } +} + +#[test] +fn the_selection_is_put_back_and_the_check_still_applies() { + let (mut s, id) = doc_with_rect(); + // Disabled even with the object given: Make with Top Object needs two. + let r = s.execute("object.envelope.makeWithTopObject", &json!({"ids": [id]})); + assert!(matches!(r, Err(EngineError::Disabled(..))), "{r:?}"); + assert!(s.doc().unwrap().selection.objects.is_empty(), "the stand-in selection is put back"); + // Without ids, nothing selected is still nothing selected. + let r = s.execute("object.repeat.radial", &json!({})); + assert!(matches!(r, Err(EngineError::Disabled(..))), "{r:?}"); +} + +#[test] +fn only_an_ids_key_counts() { + let p = json!({"ids": [3]}); + assert_eq!(given_ids("{ids?: [id]}", &p), Some(vec![NodeId(3)])); + assert_eq!(given_ids("{id?|ids?, name}", &p), Some(vec![NodeId(3)])); + assert_eq!(given_ids("{grids: n, guides: bool}", &p), None); + // A word in what the description says about another key isn't the key. + assert_eq!(given_ids("{name: of the swatch the ids of the selection take}", &p), None); + assert_eq!(given_ids("{ids?: [id]}", &json!({"ids": []})), None); +} + +/// Every command that names `ids` and is refused with nothing selected: given the object as `ids` +/// it does what it does with the object selected (the same result or the same error, and the same +/// document), so no command is let through to run on an empty selection. +#[test] +fn ids_do_what_the_selection_does_in_every_command_that_takes_them() { + let mut checked = vec![]; + for spec in command_specs() { + let (mut given, id) = doc_with_rect(); + if !cmd::params::takes_key(spec.params, "ids") || (spec.enabled)(&given).is_ok() { + continue; + } + let (mut selected, _) = doc_with_rect(); + selected.execute("select.set", &json!({"ids": [id]})).unwrap(); + let (a, b) = (given.execute(spec.id, &json!({"ids": [id]})), selected.execute(spec.id, &json!({}))); + match (&a, &b) { + (Ok(_), Ok(_)) => {} + (Err(x), Err(y)) => assert_eq!(x.to_string(), y.to_string(), "{}", spec.id), + _ => panic!("{}: with ids {a:?}, with the selection {b:?}", spec.id), + } + match (given.doc(), selected.doc()) { + (Ok(g), Ok(s)) => assert!(g.doc == s.doc, "{}: the documents differ", spec.id), + (g, s) => assert_eq!(g.is_ok(), s.is_ok(), "{}", spec.id), + } + checked.push((spec.id, a.is_ok())); + } + assert!(checked.iter().filter(|c| c.1).count() >= 20, "{checked:?}"); +}