diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 57ddb369e..92e64de6a 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -137,7 +137,7 @@ pub fn specs() -> Vec { "Duplicate", ["Edit"], None, - "{dx?, dy?} duplicate the selection in place (offset optional)", + "{ids?: [id…] (selected first; default: the selection), dx?, dy?} duplicate the selection in place (offset optional)", has_selection, duplicate ), diff --git a/crates/engine/src/cmd/object.rs b/crates/engine/src/cmd/object.rs index 8a6db151c..3e33c9a33 100644 --- a/crates/engine/src/cmd/object.rs +++ b/crates/engine/src/cmd/object.rs @@ -32,7 +32,7 @@ pub fn specs() -> Vec { "Move…", ["Object", "Transform"], Some("Cmd+Shift+M"), - "{dx, dy, copy?, objects?: bool (Transform Objects, default true; false: the art stays and, with patterns, only its pattern tiles transform), patterns?: bool (Transform Patterns: pattern fills and strokes transform with the art; default: prefs transformPatternTiles), typeAreas?: bool (area type: the frame is reshaped and the text reflows at its size)}. With anchors selected with Direct Selection (and no copy), only those anchors and their handles transform, about the centre of their own box.", + "{ids?: [id…] (selected first; default: the selection), dx, dy, copy?, objects?: bool (Transform Objects, default true; false: the art stays and, with patterns, only its pattern tiles transform), patterns?: bool (Transform Patterns: pattern fills and strokes transform with the art; default: prefs transformPatternTiles), typeAreas?: bool (area type: the frame is reshaped and the text reflows at its size)}. With anchors selected with Direct Selection (and no copy), only those anchors and their handles transform, about the centre of their own box.", has_selection_or_guides, move_cmd ), @@ -41,7 +41,7 @@ pub fn specs() -> Vec { "Rotate…", ["Object", "Transform"], None, - "{angle: deg (counter-clockwise), absolute?: bool (angle is the bounding box's new angle, not an amount), origin?: [x,y] (default: the bounding box centre), copy?, objects?: bool (Transform Objects, default true; false: the art stays and, with patterns, only its pattern tiles transform), patterns?: bool (Transform Patterns: pattern fills and strokes transform with the art; default: prefs transformPatternTiles), typeAreas?: bool (area type: the frame is reshaped and the text reflows at its size)} → {ids}. With anchors selected with Direct Selection (and no copy), only those anchors and their handles transform, about the centre of their own box.", + "{ids?: [id…] (selected first; default: the selection), angle: deg (counter-clockwise), absolute?: bool (angle is the bounding box's new angle, not an amount), origin?: [x,y] (default: the bounding box centre), copy?, objects?: bool (Transform Objects, default true; false: the art stays and, with patterns, only its pattern tiles transform), patterns?: bool (Transform Patterns: pattern fills and strokes transform with the art; default: prefs transformPatternTiles), typeAreas?: bool (area type: the frame is reshaped and the text reflows at its size)} → {ids}. With anchors selected with Direct Selection (and no copy), only those anchors and their handles transform, about the centre of their own box.", has_selection, rotate ), @@ -50,7 +50,7 @@ pub fn specs() -> Vec { "Scale…", ["Object", "Transform"], None, - "{sx: %, sy?: %, origin?: [x,y], copy?, strokes?: bool (Scale Strokes & Effects: stroke weights, dashes and effect distances scale; off keeps them, type strokes included), corners?: bool (Scale Corners: live corner radii scale), objects?: bool (Transform Objects, default true; false: the art stays and, with patterns, only its pattern tiles transform), patterns?: bool (Transform Patterns: pattern fills and strokes transform with the art; default: prefs transformPatternTiles), typeAreas?: bool (area type: the frame is reshaped and the text reflows at its size)} (strokes/corners default to the preferences). With anchors selected with Direct Selection (and no copy), only those anchors and their handles transform, about the centre of their own box.", + "{ids?: [id…] (selected first; default: the selection), sx: %, sy?: %, origin?: [x,y], copy?, strokes?: bool (Scale Strokes & Effects: stroke weights, dashes and effect distances scale; off keeps them, type strokes included), corners?: bool (Scale Corners: live corner radii scale), objects?: bool (Transform Objects, default true; false: the art stays and, with patterns, only its pattern tiles transform), patterns?: bool (Transform Patterns: pattern fills and strokes transform with the art; default: prefs transformPatternTiles), typeAreas?: bool (area type: the frame is reshaped and the text reflows at its size)} (strokes/corners default to the preferences). With anchors selected with Direct Selection (and no copy), only those anchors and their handles transform, about the centre of their own box.", has_selection, scale ), @@ -59,7 +59,7 @@ pub fn specs() -> Vec { "Reflect…", ["Object", "Transform"], None, - "{axis: \"vertical\"|\"horizontal\"|deg, origin?, copy?, objects?: bool (Transform Objects, default true; false: the art stays and, with patterns, only its pattern tiles transform), patterns?: bool (Transform Patterns: pattern fills and strokes transform with the art; default: prefs transformPatternTiles), typeAreas?: bool (area type: the frame is reshaped and the text reflows at its size)}. With anchors selected with Direct Selection (and no copy), only those anchors and their handles transform, about the centre of their own box.", + "{ids?: [id…] (selected first; default: the selection), axis: \"vertical\"|\"horizontal\"|deg, origin?, copy?, objects?: bool (Transform Objects, default true; false: the art stays and, with patterns, only its pattern tiles transform), patterns?: bool (Transform Patterns: pattern fills and strokes transform with the art; default: prefs transformPatternTiles), typeAreas?: bool (area type: the frame is reshaped and the text reflows at its size)}. With anchors selected with Direct Selection (and no copy), only those anchors and their handles transform, about the centre of their own box.", has_selection, reflect ), @@ -68,7 +68,7 @@ pub fn specs() -> Vec { "Shear…", ["Object", "Transform"], None, - "{angle: deg, axis?: \"horizontal\"|\"vertical\", origin?, copy?, objects?: bool (Transform Objects, default true; false: the art stays and, with patterns, only its pattern tiles transform), patterns?: bool (Transform Patterns: pattern fills and strokes transform with the art; default: prefs transformPatternTiles), typeAreas?: bool (area type: the frame is reshaped and the text reflows at its size)}. With anchors selected with Direct Selection (and no copy), only those anchors and their handles transform, about the centre of their own box.", + "{ids?: [id…] (selected first; default: the selection), angle: deg, axis?: \"horizontal\"|\"vertical\", origin?, copy?, objects?: bool (Transform Objects, default true; false: the art stays and, with patterns, only its pattern tiles transform), patterns?: bool (Transform Patterns: pattern fills and strokes transform with the art; default: prefs transformPatternTiles), typeAreas?: bool (area type: the frame is reshaped and the text reflows at its size)}. With anchors selected with Direct Selection (and no copy), only those anchors and their handles transform, about the centre of their own box.", has_selection, shear ), @@ -90,36 +90,104 @@ pub fn specs() -> Vec { has_selection_or_guides, nudge ), - cmd!("object.arrange.bringToFront", "Bring to Front", ["Object", "Arrange"], Some("Cmd+Shift+]"), "{}", has_selection, |s, _| arrange( - s, - Arrange::Front - )), - cmd!("object.arrange.bringForward", "Bring Forward", ["Object", "Arrange"], Some("Cmd+]"), "{}", has_selection, |s, _| arrange( - s, - Arrange::Forward - )), - cmd!("object.arrange.sendBackward", "Send Backward", ["Object", "Arrange"], Some("Cmd+["), "{}", has_selection, |s, _| arrange( - s, - Arrange::Backward - )), - cmd!("object.arrange.sendToBack", "Send to Back", ["Object", "Arrange"], Some("Cmd+Shift+["), "{}", has_selection, |s, _| arrange( - s, - Arrange::Back - )), - cmd!("object.arrange.sendToCurrentLayer", "Send to Current Layer", ["Object", "Arrange"], None, "{}", has_selection, send_to_current_layer), - cmd!("object.group", "Group", ["Object"], Some("Cmd+G"), "{} → {id}", has_selection, group), - cmd!("object.ungroup", "Ungroup", ["Object"], Some("Cmd+Shift+G"), "{}", has_selection, ungroup), - cmd!("object.lock", "Selection", ["Object", "Lock"], Some("Cmd+2"), "{}", has_selection, lock), + cmd!( + "object.arrange.bringToFront", + "Bring to Front", + ["Object", "Arrange"], + Some("Cmd+Shift+]"), + "{ids?: [id…] (selected first; default: the selection)}", + has_selection, + |s, _| arrange(s, Arrange::Front) + ), + cmd!( + "object.arrange.bringForward", + "Bring Forward", + ["Object", "Arrange"], + Some("Cmd+]"), + "{ids?: [id…] (selected first; default: the selection)}", + has_selection, + |s, _| arrange(s, Arrange::Forward) + ), + cmd!( + "object.arrange.sendBackward", + "Send Backward", + ["Object", "Arrange"], + Some("Cmd+["), + "{ids?: [id…] (selected first; default: the selection)}", + has_selection, + |s, _| arrange(s, Arrange::Backward) + ), + cmd!( + "object.arrange.sendToBack", + "Send to Back", + ["Object", "Arrange"], + Some("Cmd+Shift+["), + "{ids?: [id…] (selected first; default: the selection)}", + has_selection, + |s, _| arrange(s, Arrange::Back) + ), + cmd!( + "object.arrange.sendToCurrentLayer", + "Send to Current Layer", + ["Object", "Arrange"], + None, + "{ids?: [id…] (selected first; default: the selection)}", + has_selection, + send_to_current_layer + ), + cmd!( + "object.group", + "Group", + ["Object"], + Some("Cmd+G"), + "{ids?: [id…] (selected first; default: the selection)} → {id}", + has_selection, + group + ), + cmd!( + "object.ungroup", + "Ungroup", + ["Object"], + Some("Cmd+Shift+G"), + "{ids?: [id…] (selected first; default: the selection)}", + has_selection, + ungroup + ), + cmd!( + "object.lock", + "Selection", + ["Object", "Lock"], + Some("Cmd+2"), + "{ids?: [id…] (selected first; default: the selection)}", + has_selection, + lock + ), cmd!("object.unlockAll", "Unlock All", ["Object"], Some("Cmd+Alt+2"), "{}", has_doc, unlock_all), - cmd!("object.hide", "Selection", ["Object", "Hide"], Some("Cmd+3"), "{}", has_selection, hide), + cmd!( + "object.hide", + "Selection", + ["Object", "Hide"], + Some("Cmd+3"), + "{ids?: [id…] (selected first; default: the selection)}", + has_selection, + hide + ), cmd!("object.showAll", "Show All", ["Object"], Some("Cmd+Alt+3"), "{}", has_doc, show_all), - cmd!("object.compoundPath.make", "Make", ["Object", "Compound Path"], Some("Cmd+8"), "{}", has_selection, compound_make), + cmd!( + "object.compoundPath.make", + "Make", + ["Object", "Compound Path"], + Some("Cmd+8"), + "{ids?: [id…] (selected first; default: the selection)}", + has_selection, + compound_make + ), cmd!( "object.compoundPath.release", "Release", ["Object", "Compound Path"], Some("Cmd+Alt+Shift+8"), - "{} release the selected compound paths into their paths, each with the compound path's paint; a path of several subpaths (an SVG `d` with several `M`s) splits into one path per subpath", + "{ids?: [id…] (selected first; default: the selection)} release the selected compound paths into their paths, each with the compound path's paint; a path of several subpaths (an SVG `d` with several `M`s) splits into one path per subpath", has_selection, compound_release ), @@ -128,7 +196,7 @@ pub fn specs() -> Vec { "Make", ["Object", "Clipping Mask"], Some("Cmd+7"), - "{} the topmost selected object (a path, compound path or text, which loses its paint) clips the others: compound holes, even-odd fills and glyph outlines clip as drawn. Paint given to the clipping path later (paint commands with its id) shows: its fill behind the clipped art, its stroke over it → {id} of the clip group", + "{ids?: [id…] (selected first; default: the selection)} the topmost selected object (a path, compound path or text, which loses its paint) clips the others: compound holes, even-odd fills and glyph outlines clip as drawn. Paint given to the clipping path later (paint commands with its id) shows: its fill behind the clipped art, its stroke over it → {id} of the clip group", has_multi, clip_make ), @@ -137,7 +205,7 @@ pub fn specs() -> Vec { "Release", ["Object", "Clipping Mask"], Some("Cmd+Alt+7"), - "{} the selected clip groups become plain groups; their clipping path (path, compound path or text) stays, with the paint it has (none unless painted after Make)", + "{ids?: [id…] (selected first; default: the selection)} the selected clip groups become plain groups; their clipping path (path, compound path or text) stays, with the paint it has (none unless painted after Make)", has_selection, clip_release ), diff --git a/crates/engine/src/cmd/pathops.rs b/crates/engine/src/cmd/pathops.rs index 1538c3614..a4e68f07e 100644 --- a/crates/engine/src/cmd/pathops.rs +++ b/crates/engine/src/cmd/pathops.rs @@ -22,7 +22,7 @@ pub fn specs() -> Vec { "Unite", ["Window", "Pathfinder"], None, - "{} operate on the selected objects (back → front) → {ids}", + "{ids?: [id…] (selected first; default: the selection)} operate on the selected objects (back → front) → {ids}", has_selection, |s, _| run_pf(s, PathfinderOp::Unite) ), @@ -31,7 +31,7 @@ pub fn specs() -> Vec { "Minus Front", ["Window", "Pathfinder"], None, - "{} operate on the selected objects (back → front) → {ids}", + "{ids?: [id…] (selected first; default: the selection)} operate on the selected objects (back → front) → {ids}", has_selection, |s, _| run_pf(s, PathfinderOp::MinusFront) ), @@ -40,7 +40,7 @@ pub fn specs() -> Vec { "Intersect", ["Window", "Pathfinder"], None, - "{} operate on the selected objects (back → front) → {ids}", + "{ids?: [id…] (selected first; default: the selection)} operate on the selected objects (back → front) → {ids}", has_selection, |s, _| run_pf(s, PathfinderOp::Intersect) ), @@ -49,7 +49,7 @@ pub fn specs() -> Vec { "Exclude", ["Window", "Pathfinder"], None, - "{} operate on the selected objects (back → front) → {ids}", + "{ids?: [id…] (selected first; default: the selection)} operate on the selected objects (back → front) → {ids}", has_selection, |s, _| run_pf(s, PathfinderOp::Exclude) ), @@ -58,7 +58,7 @@ pub fn specs() -> Vec { "Divide", ["Window", "Pathfinder"], None, - "{} operate on the selected objects (back → front) → {ids}", + "{ids?: [id…] (selected first; default: the selection)} operate on the selected objects (back → front) → {ids}", has_selection, |s, _| run_pf(s, PathfinderOp::Divide) ), @@ -67,7 +67,7 @@ pub fn specs() -> Vec { "Trim", ["Window", "Pathfinder"], None, - "{} operate on the selected objects (back → front) → {ids}", + "{ids?: [id…] (selected first; default: the selection)} operate on the selected objects (back → front) → {ids}", has_selection, |s, _| run_pf(s, PathfinderOp::Trim) ), @@ -76,7 +76,7 @@ pub fn specs() -> Vec { "Merge", ["Window", "Pathfinder"], None, - "{} operate on the selected objects (back → front) → {ids}", + "{ids?: [id…] (selected first; default: the selection)} operate on the selected objects (back → front) → {ids}", has_selection, |s, _| run_pf(s, PathfinderOp::Merge) ), @@ -85,7 +85,7 @@ pub fn specs() -> Vec { "Crop", ["Window", "Pathfinder"], None, - "{} operate on the selected objects (back → front) → {ids}", + "{ids?: [id…] (selected first; default: the selection)} operate on the selected objects (back → front) → {ids}", has_multi, |s, _| run_pf(s, PathfinderOp::Crop) ), @@ -94,7 +94,7 @@ pub fn specs() -> Vec { "Outline", ["Window", "Pathfinder"], None, - "{} operate on the selected objects (back → front) → {ids}", + "{ids?: [id…] (selected first; default: the selection)} operate on the selected objects (back → front) → {ids}", has_selection, |s, _| run_pf(s, PathfinderOp::Outline) ), @@ -103,7 +103,7 @@ pub fn specs() -> Vec { "Minus Back", ["Window", "Pathfinder"], None, - "{} operate on the selected objects (back → front) → {ids}", + "{ids?: [id…] (selected first; default: the selection)} operate on the selected objects (back → front) → {ids}", has_selection, |s, _| run_pf(s, PathfinderOp::MinusBack) ), diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 8cac922ea..3aa8821cb 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -1385,7 +1385,15 @@ impl Session { self.note_depth = 1; self.batch_stash = None; } - let r = if self.depth == 0 { self.run_guarded(id, |s| (spec.run)(s, params)) } else { self.run_nested(|s| (spec.run)(s, params)) }; + // A selection-only command given `ids` selects them first, in the same undo step (#2130). + let select_first = SELECTS_GIVEN_IDS.contains(&id).then(|| params.get("ids")).flatten(); + let run = |s: &mut Session| { + if let Some(ids) = select_first { + s.execute("select.set", &serde_json::json!({ "ids": ids }))?; + } + (spec.run)(s, params) + }; + let r = if self.depth == 0 { self.run_guarded(id, run) } else { self.run_nested(run) }; let r = r?; if self.depth == 0 { self.inherit_new_art(); @@ -2022,6 +2030,8 @@ mod tests_scalestrokes; #[cfg(test)] mod tests_scripts; #[cfg(test)] +mod tests_select_ids_first; +#[cfg(test)] mod tests_shaper; #[cfg(test)] mod tests_shortcutset; @@ -2086,6 +2096,41 @@ mod tests_widthtool; #[cfg(test)] mod tests_xform; +/// Commands that act on the selection and take `ids` by selecting them first (`select.set`, with +/// its checks), then running as on the selection: the result and the selection afterwards are the +/// menu command's (#2130). +pub(crate) const SELECTS_GIVEN_IDS: &[&str] = &[ + "edit.duplicate", + "object.arrange.bringForward", + "object.arrange.bringToFront", + "object.arrange.sendBackward", + "object.arrange.sendToBack", + "object.arrange.sendToCurrentLayer", + "object.clippingMask.make", + "object.clippingMask.release", + "object.compoundPath.make", + "object.compoundPath.release", + "object.group", + "object.hide", + "object.lock", + "object.move", + "object.pathfinder.crop", + "object.pathfinder.divide", + "object.pathfinder.exclude", + "object.pathfinder.intersect", + "object.pathfinder.merge", + "object.pathfinder.minusBack", + "object.pathfinder.minusFront", + "object.pathfinder.outline", + "object.pathfinder.trim", + "object.pathfinder.unite", + "object.reflect", + "object.rotate", + "object.scale", + "object.shear", + "object.ungroup", +]; + /// 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> { diff --git a/crates/engine/src/tests_select_ids_first.rs b/crates/engine/src/tests_select_ids_first.rs new file mode 100644 index 000000000..fbeb2c867 --- /dev/null +++ b/crates/engine/src/tests_select_ids_first.rs @@ -0,0 +1,155 @@ +//! #2130: selection-only commands take `ids`: the objects given are selected first and the +//! command runs on them, as `select.set` followed by the command does, in one undo step. + +use serde_json::{Value, json}; + +use super::*; + +/// Three rectangles and an ellipse, nothing selected → the session and their ids. +fn doc() -> (Session, Vec) { + let mut s = Session::new(); + // Separately created fixtures must compare equal across clock ticks. + s.execute("file.new", &json!({"width": 400, "height": 300, "created": null})).unwrap(); + let mut ids = vec![]; + for (x, y) in [(10, 10), (50, 30), (80, 50)] { + ids.push(s.execute("shape.rectangle", &json!({"x": x, "y": y, "width": 80, "height": 60})).unwrap()["id"].as_u64().unwrap()); + } + ids.push(s.execute("shape.ellipse", &json!({"x": 40, "y": 20, "width": 100, "height": 100})).unwrap()["id"].as_u64().unwrap()); + s.execute("select.none", &json!({})).unwrap(); + (s, ids) +} + +/// Runs `setup` (commands on the selection, given the fixture's ids), then `cmd` two ways: after +/// `select.set {ids}`, and with `ids` in its params and something else selected. Both must leave +/// the same document, selection and result. +/// Makes what a case runs on (given the fixture's ids) → the ids to give. +type Setup<'a> = &'a dyn Fn(&mut Session, &[u64]) -> Vec; + +fn same_as_selecting_first(cmd: &str, params: Value, setup: Setup<'_>) { + let run = |given: bool| { + let (mut s, ids) = doc(); + let targets = setup(&mut s, &ids); + let r = if given { + // Another object selected: the given ones take its place. + s.execute("select.set", &json!({"ids": [ids[3]]})).unwrap(); + let mut p = params.clone(); + p["ids"] = json!(targets); + s.execute(cmd, &p) + } else { + s.execute("select.set", &json!({"ids": targets})).unwrap(); + s.execute(cmd, ¶ms) + }; + let st = s.doc().unwrap(); + (r.map_err(|e| e.to_string()), st.doc.clone(), st.selection.objects.clone()) + }; + let (by_selection, by_ids) = (run(false), run(true)); + assert!(by_selection.0.is_ok(), "{cmd}: {:?}", by_selection.0); + assert_eq!(by_ids.0, by_selection.0, "{cmd}: result"); + assert!(by_ids.1 == by_selection.1, "{cmd}: the document differs"); + assert_eq!(by_ids.2, by_selection.2, "{cmd}: selection"); +} + +fn none(_: &mut Session, ids: &[u64]) -> Vec { + ids[..3].to_vec() +} + +#[test] +fn ids_are_selected_first() { + let grouped = |s: &mut Session, ids: &[u64]| { + s.execute("select.set", &json!({"ids": &ids[..2]})).unwrap(); + let g = s.execute("object.group", &json!({})).unwrap()["id"].as_u64().unwrap(); + s.execute("select.none", &json!({})).unwrap(); + vec![g] + }; + let clipped = |s: &mut Session, ids: &[u64]| { + s.execute("select.set", &json!({"ids": &ids[..3]})).unwrap(); + let g = s.execute("object.clippingMask.make", &json!({})).unwrap()["id"].as_u64().unwrap(); + s.execute("select.none", &json!({})).unwrap(); + vec![g] + }; + let compound = |s: &mut Session, ids: &[u64]| { + s.execute("select.set", &json!({"ids": &ids[..2]})).unwrap(); + let c = s.execute("object.compoundPath.make", &json!({})).unwrap()["id"].as_u64().unwrap(); + s.execute("select.none", &json!({})).unwrap(); + vec![c] + }; + let one = |_: &mut Session, ids: &[u64]| vec![ids[1]]; + let cases: Vec<(&str, Value, Setup<'_>)> = vec![ + ("object.group", json!({}), &none), + ("object.ungroup", json!({}), &grouped), + ("object.clippingMask.make", json!({}), &none), + ("object.clippingMask.release", json!({}), &clipped), + ("object.compoundPath.make", json!({}), &none), + ("object.compoundPath.release", json!({}), &compound), + ("object.arrange.bringToFront", json!({}), &one), + ("object.arrange.bringForward", json!({}), &one), + ("object.arrange.sendBackward", json!({}), &one), + ("object.arrange.sendToBack", json!({}), &one), + ("object.arrange.sendToCurrentLayer", json!({}), &one), + ("object.lock", json!({}), &one), + ("object.hide", json!({}), &one), + ("edit.duplicate", json!({"dx": 5, "dy": 5}), &none), + ("object.move", json!({"dx": 12, "dy": -4}), &none), + ("object.rotate", json!({"angle": 30}), &none), + ("object.scale", json!({"sx": 150}), &none), + ("object.reflect", json!({"axis": "vertical"}), &none), + ("object.shear", json!({"angle": 20}), &none), + ("object.pathfinder.unite", json!({}), &none), + ("object.pathfinder.minusFront", json!({}), &none), + ("object.pathfinder.intersect", json!({}), &none), + ("object.pathfinder.exclude", json!({}), &none), + ("object.pathfinder.divide", json!({}), &none), + ("object.pathfinder.trim", json!({}), &none), + ("object.pathfinder.merge", json!({}), &none), + ("object.pathfinder.crop", json!({}), &none), + ("object.pathfinder.outline", json!({}), &none), + ("object.pathfinder.minusBack", json!({}), &none), + ]; + for (cmd, params, setup) in cases { + same_as_selecting_first(cmd, params, setup); + } +} + +/// Every command that selects its `ids` first says so in its params description, so `command_list` +/// and the params audit (#1442) know the key. +#[test] +fn the_descriptions_name_ids() { + for id in SELECTS_GIVEN_IDS { + let spec = cmd::find_command(id).unwrap_or_else(|| panic!("{id} is not a command")); + assert!(cmd::params::takes_key(spec.params, "ids"), "{id}: {}", spec.params); + } +} + +#[test] +fn one_undo_step_takes_it_back() { + let (mut s, ids) = doc(); + let before = s.doc().unwrap().doc.clone(); + let g = s.execute("object.group", &json!({"ids": &ids[..3]})).unwrap()["id"].as_u64().unwrap(); + assert!(matches!(s.doc().unwrap().doc.node(NodeId(g)).unwrap().kind, vectorcraft_doc::NodeKind::Group { .. })); + s.execute("edit.undo", &json!({})).unwrap(); + assert!(s.doc().unwrap().doc == before); +} + +#[test] +fn bad_ids_are_refused_and_change_nothing() { + let (mut s, ids) = doc(); + s.execute("select.set", &json!({"ids": [ids[3]]})).unwrap(); + let before = s.doc().unwrap().doc.clone(); + for bad in [json!([999]), json!([ids[0], -1]), json!(["a"])] { + let r = s.execute("object.group", &json!({"ids": bad})); + assert!(r.is_err(), "{bad}: {r:?}"); + assert!(s.doc().unwrap().doc == before, "{bad}"); + assert_eq!(s.doc().unwrap().selection.objects, vec![NodeId(ids[3])], "{bad}: the selection stays"); + } +} + +#[test] +fn without_ids_the_selection_is_used_as_before() { + let (mut s, ids) = doc(); + s.execute("select.set", &json!({"ids": &ids[..2]})).unwrap(); + let g = s.execute("object.group", &json!({})).unwrap()["id"].as_u64().unwrap(); + assert_eq!(s.doc().unwrap().doc.node(NodeId(g)).unwrap().children().unwrap().len(), 2); + // Nothing selected and no ids: still disabled. + s.execute("select.none", &json!({})).unwrap(); + assert!(matches!(s.execute("object.group", &json!({})), Err(EngineError::Disabled(..)))); +}