M1: wrap errors name the command that was run

Grouping, framing, auto layout, masks, outline stroke and the boolean
commands share wrap(), which named `object.group` in every refusal
(`layout.add` on a page: "invalid parameters for `object.group`").
wrap() now takes the calling command's id.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
MortenHusted
2026-10-18 13:58:13 +02:00
co-authored by Claude Opus 5.5
parent 018b8166b2
commit 4707cb1579
3 changed files with 34 additions and 16 deletions
+1 -1
View File
@@ -124,7 +124,7 @@ pub(crate) fn apply(s: &mut Session, cmd: &str, p: &Value, op: BoolOp) -> Result
}
return Ok(json!({"ids": ids, "operation": op}));
}
let out = crate::cmd::structure::wrap_boolean(s, p, op)?;
let out = crate::cmd::structure::wrap_boolean(s, p, cmd, op)?;
// Operands of a new group follow its operation.
if let Some(g) = crate::params::id_of(&out["id"]) {
let doc = s.doc_mut()?;
+16 -15
View File
@@ -12,9 +12,9 @@ use crate::{EngineError, Result, Session, bad, spec};
pub fn specs() -> Vec<CommandSpec> {
vec![
spec!("object.group", "Group selection", "{ids?, name?}", true, |s, p| wrap(s, p, Wrap::Group)),
spec!("object.group", "Group selection", "{ids?, name?}", true, |s, p| wrap(s, p, "object.group", Wrap::Group)),
spec!("object.ungroup", "Ungroup selection", "{ids?}", true, ungroup),
spec!("object.frameSelection", "Frame selection", "{ids?, name?}", true, |s, p| wrap(s, p, Wrap::Frame)),
spec!("object.frameSelection", "Frame selection", "{ids?, name?}", true, |s, p| wrap(s, p, "object.frameSelection", Wrap::Frame)),
spec!("object.removeFrame", "Remove frame", "{ids?}", true, ungroup),
spec!(
"object.convertToGroup",
@@ -42,7 +42,7 @@ pub fn specs() -> Vec<CommandSpec> {
"Wrap in new section",
"{ids?, name?} (layers on the page or in a section; the section wraps them with some room)",
true,
|s, p| wrap(s, p, Wrap::Section)
|s, p| wrap(s, p, "object.wrapInSection", Wrap::Section)
),
spec!(
"object.resizeToFit",
@@ -128,26 +128,27 @@ const SECTION_PAD: f64 = 40.0;
const BOOL_PARAMS: &str = "{ids?} — layers inside boolean groups: sets their own operation (how each combines with the operands below it); one boolean group: sets its operation (every operand follows it); otherwise wraps the layers in a new boolean group";
/// Wrap the selection in a new boolean group doing `op`.
pub(crate) fn wrap_boolean(s: &mut Session, p: &Value, op: BoolOp) -> Result<Value> {
wrap(s, p, Wrap::Bool(op))
pub(crate) fn wrap_boolean(s: &mut Session, p: &Value, cmd: &str, op: BoolOp) -> Result<Value> {
wrap(s, p, cmd, Wrap::Bool(op))
}
/// Wrap the selection in a new container at the topmost selected layer's place.
fn wrap(s: &mut Session, p: &Value, w: Wrap) -> Result<Value> {
let ids = req_targets("object.group", s, p)?;
/// Wrap the selection in a new container at the topmost selected layer's place. Errors name `cmd`,
/// the command that asked.
fn wrap(s: &mut Session, p: &Value, cmd: &str, w: Wrap) -> Result<Value> {
let ids = req_targets(cmd, s, p)?;
let doc = s.doc_mut()?;
let ids = z_sorted(doc, &roots(doc, &ids));
let top = *ids.last().ok_or_else(|| bad("object.group", "nothing selected"))?;
let parent = doc.get(top)?.parent.ok_or_else(|| bad("object.group", "can't group a page"))?;
let top = *ids.last().ok_or_else(|| bad(cmd, "nothing selected"))?;
let parent = doc.get(top)?.parent.ok_or_else(|| bad(cmd, "can't group a page"))?;
// Sections sit on the page or in sections, never in frames or groups.
let board = |doc: &Document, i: NodeId| {
doc.node(i).and_then(|n| n.parent).and_then(|p| doc.node(p)).is_some_and(|p| p.is_page() || matches!(p.kind, NodeKind::Section { .. }))
};
if matches!(w, Wrap::Section) && !ids.iter().all(|i| board(doc, *i)) {
return Err(bad("object.wrapInSection", "only layers on the page or in a section can be wrapped in a section"));
return Err(bad(cmd, "only layers on the page or in a section can be wrapped in a section"));
}
if !matches!(w, Wrap::Section) && ids.iter().any(|i| doc.node(*i).is_some_and(|n| matches!(n.kind, NodeKind::Section { .. }))) {
return Err(bad("object.group", "sections can't go in groups or frames"));
return Err(bad(cmd, "sections can't go in groups or frames"));
}
let index = doc.index_in_parent(top).map(|i| i + 1);
let mut bounds = doc.bounds_of(&ids).unwrap_or_default();
@@ -338,7 +339,7 @@ fn mask(s: &mut Session, p: &Value) -> Result<Value> {
return ok();
}
// Group, then the bottom-most child is the mask.
let r = wrap(s, &json!({"ids": ids, "name": "Mask group"}), Wrap::Group)?;
let r = wrap(s, &json!({"ids": ids, "name": "Mask group"}), "object.mask", Wrap::Group)?;
let gid = id_of(&r["id"]).ok_or_else(|| EngineError::Other("group failed".into()))?;
let doc = s.doc_mut()?;
if let Some(first) = doc.get(gid)?.children.first().copied() {
@@ -472,7 +473,7 @@ fn outline_stroke(s: &mut Session, p: &Value) -> Result<Value> {
let f = doc.get_mut(fill_id)?;
f.strokes.clear();
}
let r = wrap(s, &json!({"ids": [fill_id, vid], "name": n.name}), Wrap::Group)?;
let r = wrap(s, &json!({"ids": [fill_id, vid], "name": n.name}), "vector.outlineStroke", Wrap::Group)?;
if let Some(g) = id_of(&r["id"]) {
out.push(g);
}
@@ -519,7 +520,7 @@ fn layout_add(s: &mut Session, p: &Value) -> Result<Value> {
ids
} else {
// Wrap non-frames in a new frame without a fill, then add auto layout to it.
let r = wrap(s, &json!({"ids": ids}), Wrap::Frame)?;
let r = wrap(s, &json!({"ids": ids}), "layout.add", Wrap::Frame)?;
let fid = id_of(&r["id"]).ok_or_else(|| EngineError::Other("wrap failed".into()))?;
let f = s.doc_mut()?.get_mut(fid)?;
f.fills.clear();
@@ -194,3 +194,20 @@ fn only_frames_on_the_page_or_in_a_section_become_sections() {
assert!(err.contains("holds sections"), "{err}");
assert!(matches!(kind(&s, sec), NodeKind::Section { .. }), "nothing changed");
}
#[test]
fn wrapping_errors_name_the_command_that_was_run() {
// Grouping, framing, auto layout, masks and booleans share one wrapping step; a refusal names
// the command that asked, not `object.group`.
let mut s = Session::new();
let page = s.page().unwrap();
for cmd in ["object.group", "object.frameSelection", "layout.add", "bool.union"] {
let err = s.execute(cmd, &json!({"ids": [page]})).unwrap_err().to_string();
assert!(err.contains(&format!("`{cmd}`")), "{cmd}: {err}");
}
let r = rect(&mut s, page, 0.0, 0.0);
let f = id(ex(&mut s, "frame.create", json!({"x": 100, "y": 0, "width": 100, "height": 100})));
let inner = rect(&mut s, f, 10.0, 10.0);
let err = s.execute("object.wrapInSection", &json!({"ids": [inner, r]})).unwrap_err().to_string();
assert!(err.contains("`object.wrapInSection`"), "{err}");
}