diff --git a/crates/engine/src/cmd/variablecmds.rs b/crates/engine/src/cmd/variablecmds.rs index 46361c851..889ca0b07 100644 --- a/crates/engine/src/cmd/variablecmds.rs +++ b/crates/engine/src/cmd/variablecmds.rs @@ -55,7 +55,7 @@ fn check_bindable(doc: &vectorcraft_doc::Document, ids: &[NodeId], kind: Variabl for id in ids { match doc.node(*id) { None => return Err(EngineError::NoNode(*id)), - Some(_) if !doc.variable_target(*id) => return Err(bad(cmd, format!("node {} is locked", id.0))), + Some(_) if !doc.variable_target(*id) => return Err(bad(cmd, not_a_target(doc, *id))), Some(n) if kind == VariableKind::Text && !matches!(n.kind, NodeKind::Text(_)) => { return Err(bad(cmd, format!("node {} is not type", id.0))); } @@ -65,6 +65,20 @@ fn check_bindable(doc: &vectorcraft_doc::Document, ids: &[NodeId], kind: Variabl Ok(()) } +/// Why `variable_target` refuses `id`: the object is locked, a group or layer that holds it is +/// locked, or one that holds it is hidden, checked in that order. +fn not_a_target(doc: &vectorcraft_doc::Document, id: NodeId) -> String { + let holders: Vec<&vectorcraft_doc::Node> = + doc.ancestry(id).unwrap_or_default().into_iter().filter(|i| *i != id).filter_map(|i| doc.node(i)).collect(); + if doc.node(id).is_some_and(|n| n.locked) { + format!("node {} is locked", id.0) + } else if holders.iter().any(|n| n.locked) { + format!("node {} is in a locked group or layer", id.0) + } else { + format!("node {} is in a hidden group or layer", id.0) + } +} + fn dataset_value(cmd: &str, var: &str, kind: VariableKind, v: &Value) -> Result { match kind { VariableKind::Text => v.as_str().map(|t| DataValue::Text(t.to_string())), @@ -136,13 +150,14 @@ fn rename(s: &mut Session, p: &Value) -> Result { /// Make Text Dynamic / Make Visibility Dynamic: define a variable of that kind and bind the /// selection to it in one step, naming it after what it drives — the panel's buttons. fn make_dynamic(s: &mut Session, kind: VariableKind, stem: &str) -> Result { - const C: &str = "variable.makeDynamic"; + // Errors name the command that ran: `variable.make{stem}Dynamic`. + let c = &format!("variable.make{stem}Dynamic"); let ids = selected_roots(s)?; if ids.is_empty() { - return Err(bad(C, "select the objects to bind")); + return Err(bad(c, "select the objects to bind")); } let doc = &s.doc()?.doc; - check_bindable(doc, &ids, kind, C)?; + check_bindable(doc, &ids, kind, c)?; // Named after the first object it drives, as the reference panel does; a second variable // over the same art takes a number rather than colliding. let shown = ids.first().and_then(|id| doc.node(*id)).map(|n| n.display_name()).unwrap_or_default(); diff --git a/crates/engine/src/tests_variables.rs b/crates/engine/src/tests_variables.rs index dbc81c0ea..aa05e7963 100644 --- a/crates/engine/src/tests_variables.rs +++ b/crates/engine/src/tests_variables.rs @@ -253,6 +253,40 @@ fn locked_art_is_neither_bound_nor_rewritten() { assert!(!s.doc().unwrap().doc.node(r).unwrap().visible, "the unlocked rectangle still applied"); } +/// Binding fails for locked art and for art in a hidden or locked group or layer, and the +/// error names which of the three applies. +#[test] +fn binding_errors_say_whether_a_layer_or_group_is_hidden_or_locked() { + let mut s = session(); + s.execute("variable.define", &json!({"name": "Name", "kind": "text"})).unwrap(); + let layer = NodeId(s.execute("layer.new", &json!({})).unwrap()["id"].as_u64().unwrap()); + let t = text(&mut s, "x"); + let r = rect(&mut s); + let bind = |s: &mut Session| s.execute("variable.bind", &json!({"variable": "Name", "ids": [t.0]})).unwrap_err().to_string(); + + s.execute("layer.setProps", &json!({"id": layer.0, "visible": false})).unwrap(); + let e = bind(&mut s); + assert!(e.contains(&format!("node {} is in a hidden group or layer", t.0)), "{e}"); + s.execute("layer.setProps", &json!({"id": layer.0, "visible": true})).unwrap(); + + s.execute("select.set", &json!({"ids": [t.0, r.0]})).unwrap(); + s.execute("object.group", &json!({})).unwrap(); + s.execute("object.hide", &json!({})).unwrap(); + let e = bind(&mut s); + assert!(e.contains(&format!("node {} is in a hidden group or layer", t.0)), "{e}"); + s.execute("object.showAll", &json!({})).unwrap(); + + s.execute("layer.setProps", &json!({"id": layer.0, "locked": true})).unwrap(); + let e = bind(&mut s); + assert!(e.contains(&format!("node {} is in a locked group or layer", t.0)), "{e}"); + s.execute("layer.setProps", &json!({"id": layer.0, "locked": false})).unwrap(); + + s.execute("select.set", &json!({"ids": [t.0]})).unwrap(); + s.execute("object.lock", &json!({})).unwrap(); + let e = bind(&mut s); + assert!(e.contains(&format!("node {} is locked", t.0)), "{e}"); +} + /// Make Text Dynamic / Make Visibility Dynamic: what the panel's buttons run. One step from a /// selection to a bound variable, named after what it drives. #[test] @@ -284,6 +318,18 @@ fn making_a_selection_dynamic_names_and_binds_it() { assert!(s.execute("variable.makeTextDynamic", &json!({})).is_err(), "a rectangle is not type"); } +/// Make Text Dynamic reports its errors under its own command id. +#[test] +fn make_dynamic_errors_name_the_command() { + let mut s = session(); + let t = text(&mut s, "x"); + let r = rect(&mut s); + s.execute("select.set", &json!({"ids": [t.0, r.0]})).unwrap(); + s.execute("object.group", &json!({})).unwrap(); + let e = s.execute("variable.makeTextDynamic", &json!({})).unwrap_err().to_string(); + assert!(e.contains("`variable.makeTextDynamic`") && e.contains("is not type"), "{e}"); +} + /// Capture Data Set records what the art holds now, which is how rows are made — the art is /// edited and captured, rather than the values typed. #[test]