Merge main into #870

crates/engine/src/cmd/arrange.rs

@@ -528,6 +528,7 @@ pub(crate) fn resize_cmd(s: &mut Session, p: &Value) -> Result<Value> {
     let (col, row) = string(p, "anchor").and_then(|a| crate::tools::anchor_of(&a)).unwrap_or((0, 0));
     let (ax, ay) = (f64::from(col) / 2.0, f64::from(row) / 2.0);
     let doc = s.doc_mut()?;
+    let before = super::styles::bound_before(doc, &ids);
     for i in ids {
         let n = doc.get(i)?;
         let (ow, oh) = (n.width, n.height);
@@ -551,6 +552,7 @@ pub(crate) fn resize_cmd(s: &mut Session, p: &Value) -> Result<Value> {
         }
         super::props::restore_worlds(doc, &kept)?;
     }
+    super::styles::detach_edited(doc, &before);
     ok()
 }
 

crates/engine/src/cmd/component.rs

@@ -608,7 +608,7 @@ fn make_slot(s: &mut Session, p: &Value) -> Result<Value> {
     let owner = Owner::resolve(doc, id).ok_or_else(|| bad(cmd, "the frame isn't inside a component"))?;
     let name = string(p, "name").unwrap_or_else(|| fresh_prop_name(doc, &owner, &n.name));
     let preferred = get(p, "preferred").cloned().unwrap_or(Value::Null);
-    add_prop(s, &json!({"id": id, "layer": id, "name": name, "type": "slot", "preferred": preferred}))
+    add_prop_for(cmd, s, &json!({"id": id, "layer": id, "name": name, "type": "slot", "preferred": preferred}))
 }
 
 fn remove_slot(s: &mut Session, p: &Value) -> Result<Value> {
@@ -1039,7 +1039,12 @@ fn valid_prop_name(cmd: &str, name: &str) -> Result<String> {
 }
 
 fn add_prop(s: &mut Session, p: &Value) -> Result<Value> {
-    let cmd = "component.addProperty";
+    add_prop_for("component.addProperty", s, p)
+}
+
+/// Add a component property for command `cmd`, which errors name (`component.makeSlot` adds its
+/// slot property here).
+fn add_prop_for(cmd: &str, s: &mut Session, p: &Value) -> Result<Value> {
     let target = targets(s, p).first().copied().ok_or_else(|| bad(cmd, "select a component"))?;
     let name = valid_prop_name(cmd, &req_string(cmd, p, "name")?)?;
     let t = req_string(cmd, p, "type")?;

crates/engine/src/cmd/edit.rs

@@ -261,7 +261,7 @@ pub fn specs() -> Vec<CommandSpec> {
         }),
         spec!("effect.set", "Set effect", "{ids?, index, effect {..}}", true, |s, p| {
             let i = req_index("effect.set", p, "index")?;
-            let e: Effect = parse("effect.set", p, "effect")?.ok_or_else(|| bad("effect.set", "`effect` required"))?;
+            let e = super::props::effect_from("effect.set", p, "effect")?.ok_or_else(|| bad("effect.set", "`effect` required"))?;
             for id in req_targets("effect.set", s, p)? {
                 let n = s.doc_mut()?.get_mut(id)?;
                 if let Some(slot) = n.effects.get_mut(i) {
@@ -952,9 +952,13 @@ pub fn opacity_from_digits(digits: &str) -> Option<f32> {
 fn opacity_keys(s: &mut Session, p: &Value) -> Result<Value> {
     let digits = req_string("object.opacityKeys", p, "digits")?;
     let o = opacity_from_digits(&digits).ok_or_else(|| bad("object.opacityKeys", "`digits` must be one or two digits"))?;
-    for i in req_targets("object.opacityKeys", s, p)? {
-        s.doc_mut()?.get_mut(i)?.opacity = o;
+    let ids = req_targets("object.opacityKeys", s, p)?;
+    let doc = s.doc_mut()?;
+    let before = super::styles::bound_before(doc, &ids);
+    for i in ids {
+        doc.get_mut(i)?.opacity = o;
     }
+    super::styles::detach_edited(doc, &before);
     Ok(json!({"opacity": o}))
 }
 
@@ -1202,10 +1206,12 @@ fn set(s: &mut Session, p: &Value) -> Result<Value> {
         }
     }
     let doc = s.doc_mut()?;
+    let before = super::styles::bound_before(doc, &ids);
     for i in ids {
         let props = if skipped.contains(&i) { &without_layout } else { &props };
         super::props::apply("node.set", doc, i, props)?;
     }
+    super::styles::detach_edited(doc, &before);
     if let Some(o) = pivot {
         for (i, q) in pinned {
             let d = o - doc.world_transform(i) * q;
@@ -1363,7 +1369,7 @@ fn paint_set(s: &mut Session, p: &Value, fills: bool) -> Result<Value> {
         if let Some(b) = blend(p, "blend") {
             slot.blend = b;
         }
-        super::lists::remap_stop_bindings(n, fills, at, &old);
+        super::lists::rebind_edited_paint(n, fills, at, &old);
         if fills {
             n.styles.fill = None;
         } else {
@@ -1397,8 +1403,8 @@ fn pattern_fill(s: &mut Session, p: &Value) -> Result<Value> {
 }
 
 fn effect_add(s: &mut Session, p: &Value) -> Result<Value> {
-    let e: Effect = match get(p, "effect") {
-        Some(v) => serde_json::from_value(v.clone()).map_err(|e| bad("effect.add", e.to_string()))?,
+    let e: Effect = match super::props::effect_from("effect.add", p, "effect")? {
+        Some(e) => e,
         None => {
             let ty = string(p, "type").unwrap_or_else(|| "dropShadow".into());
             Effect::of_type(&ty).ok_or_else(|| bad("effect.add", format!("unknown effect type `{ty}`")))?

crates/engine/src/cmd/file.rs

@@ -92,6 +92,20 @@ pub fn specs() -> Vec<CommandSpec> {
         ),
         spec!("file.placeImage", "Place image", "{path | data, x?, y?}", true, super::create::place_image),
         spec!("file.close", "Close tab", "{index?, force? (discard unsaved changes)}", false, close),
+        spec!("file.moveTab", "Move tab", "{from, to} (indices among open documents)", false, |s, p| {
+            let from = req_index("file.moveTab", p, "from")?;
+            let to = req_index("file.moveTab", p, "to")?;
+            if from >= s.docs.len() || to >= s.docs.len() {
+                return Err(bad("file.moveTab", "no such document"));
+            }
+            if from != to {
+                let active_uid = s.docs.get(s.active).map(|d| d.uid);
+                let doc = s.docs.remove(from);
+                s.docs.insert(to, doc);
+                s.active = active_uid.and_then(|uid| s.docs.iter().position(|d| d.uid == uid)).unwrap_or(0);
+            }
+            Ok(json!({"index": to}))
+        }),
         spec!(
             "file.recover",
             "Recover file",
@@ -1374,14 +1388,36 @@ fn close(s: &mut Session, p: &Value) -> Result<Value> {
         return Err(EngineError::Disabled("file.close".into(), "the document has unsaved changes (pass force: true to discard)".into()));
     }
     let uid = ds.uid;
+    let active_uid = s.docs.get(s.active).map(|d| d.uid);
     s.docs.remove(i);
-    if s.active >= s.docs.len() {
-        s.active = s.docs.len().saturating_sub(1);
-    }
+    // Removing a tab before the active one shifts its index; keep the same document selected.
+    s.active = active_uid.and_then(|a| s.docs.iter().position(|d| d.uid == a)).unwrap_or(i.min(s.docs.len().saturating_sub(1)));
     crate::branches::close_tabs(s, uid, None);
     ok()
 }
 
+#[cfg(test)]
+mod tab_order_tests {
+    use super::*;
+
+    #[test]
+    fn moving_and_closing_inactive_tabs_preserves_the_active_document() {
+        let mut session = Session::new();
+        for name in ["Two", "Three", "Four"] {
+            session.execute("file.new", &json!({"name": name})).unwrap();
+        }
+        session.execute("doc.switch", &json!({"index": 2})).unwrap();
+        let active_uid = session.docs[session.active].uid;
+        let moved_uid = session.docs[0].uid;
+        session.execute("file.moveTab", &json!({"from": 0, "to": 2})).unwrap();
+        assert_eq!(session.docs[2].uid, moved_uid);
+        assert_eq!(session.docs[session.active].uid, active_uid);
+        session.execute("file.close", &json!({"index": 0})).unwrap();
+        assert_eq!(session.docs[session.active].uid, active_uid);
+        assert!(session.execute("file.moveTab", &json!({"from": 0, "to": 99})).is_err());
+    }
+}
+
 /// Ctrl+Tab / Ctrl+Shift+Tab: the next or previous open document, wrapping around.
 fn cycle_doc(s: &mut Session, dir: i64) -> Result<Value> {
     let n = s.docs.len() as i64;

crates/engine/src/cmd/lists.rs

@@ -112,7 +112,8 @@ pub fn stop_binding(field: &str) -> Option<usize> {
 /// Keep the gradient stop bindings of paint `index` of `n`'s fills (or strokes) on their stops after
 /// the paint changed from `old`: one stop removed shifts the later ones down (the removed one's
 /// binding goes); a paint that is no longer a gradient drops them. Other edits keep stop indices.
-pub(crate) fn remap_stop_bindings(n: &mut Node, fills: bool, index: usize, old: &figcraft_color::Paint) {
+/// Returns the removed stop's index.
+pub(crate) fn remap_stop_bindings(n: &mut Node, fills: bool, index: usize, old: &figcraft_color::Paint) -> Option<usize> {
     use figcraft_color::PaintKind;
     let list = if fills { BoundList::Fills } else { BoundList::Strokes };
     let new = if fills { n.fills.get(index) } else { n.strokes.get(index) };
@@ -122,11 +123,11 @@ pub(crate) fn remap_stop_bindings(n: &mut Node, fills: bool, index: usize, old:
             // The first stop that differs is the removed one when the rest moved down unchanged.
             let k = a.stops.iter().zip(&b.stops).position(|(x, y)| x != y).unwrap_or(b.stops.len());
             if a.stops.get(k + 1..) != b.stops.get(k..) {
-                return;
+                return None;
             }
             Some(k)
         }
-        (_, Some(PaintKind::Gradient(_))) => return,
+        (_, Some(PaintKind::Gradient(_))) => return None,
         _ => None,
     };
     let keys: Vec<(String, Option<String>)> = n
@@ -150,6 +151,42 @@ pub(crate) fn remap_stop_bindings(n: &mut Node, fills: bool, index: usize, old:
         }
     }
     n.bound.extend(kept);
+    removed
+}
+
+/// After an explicit edit changed paint `index` of `n`'s fills (or strokes) from `old`: its stop
+/// bindings follow a removed stop, then each binding whose value the edit changed (the colour, the
+/// opacity, a stop's colour) lets go, so the edit stays instead of being put back from the
+/// variable. Bindings whose value the edit kept stay; a paint that is gone takes its bindings.
+pub(crate) fn rebind_edited_paint(n: &mut Node, fills: bool, index: usize, old: &figcraft_color::Paint) {
+    let removed = remap_stop_bindings(n, fills, index, old);
+    let mut old = old.clone();
+    if let (Some(k), figcraft_color::PaintKind::Gradient(g)) = (removed, &mut old.kind)
+        && k < g.stops.len()
+    {
+        g.stops.remove(k);
+    }
+    let list = if fills { BoundList::Fills } else { BoundList::Strokes };
+    let new = if fills { n.fills.get(index) } else { n.strokes.get(index) }.cloned();
+    n.bound.retain(|key, _| match list_binding(key) {
+        Some((l, i, field)) if l == list && i == index => new.as_ref().is_some_and(|new| paint_field_kept(&old, new, field)),
+        _ => true,
+    });
+}
+
+/// Whether a paint's bound `field` (`color`, `opacity`, `stops/<j>/color`, …) has the same value in
+/// `new` as in `old`.
+fn paint_field_kept(old: &figcraft_color::Paint, new: &figcraft_color::Paint, field: &str) -> bool {
+    use figcraft_color::PaintKind;
+    match field {
+        "color" => matches!((&old.kind, &new.kind), (PaintKind::Solid { color: a }, PaintKind::Solid { color: b }) if a == b),
+        "opacity" => old.opacity == new.opacity,
+        _ => match (stop_binding(field), &old.kind, &new.kind) {
+            (Some(j), PaintKind::Gradient(a), PaintKind::Gradient(b)) => a.stops.get(j).map(|s| s.color) == b.stops.get(j).map(|s| s.color),
+            (Some(_), _, _) => false,
+            _ => old == new,
+        },
+    }
 }
 
 /// Where item `k` lands when the item at `from` moves to `to`.

crates/engine/src/cmd/props.rs

@@ -61,6 +61,48 @@ pub fn snake(v: &Value) -> Value {
     }
 }
 
+/// `v` as an effect list, refusing keys an effect doesn't read: serde dropped them without a word,
+/// so a shadow given `offsetY` kept the default offset.
+pub fn effects_from(cmd: &str, v: &Value) -> Result<Vec<Effect>> {
+    let effects: Vec<Effect> = serde_json::from_value(snake(v)).map_err(|e| bad(cmd, format!("effects: {e}")))?;
+    for (i, given) in v.as_array().into_iter().flatten().enumerate() {
+        refuse_ignored_effect_keys(cmd, &format!("effects[{i}]"), given, |v| serde_json::from_value(snake(v)).ok())?;
+    }
+    Ok(effects)
+}
+
+/// `p[key]` as one effect, read as serde reads it (its aliases too), refusing keys it doesn't read.
+pub fn effect_from(cmd: &str, p: &Value, key: &str) -> Result<Option<Effect>> {
+    let Some(v) = get(p, key) else { return Ok(None) };
+    let effect: Effect = serde_json::from_value(v.clone()).map_err(|e| bad(cmd, format!("`{key}`: {e}")))?;
+    refuse_ignored_effect_keys(cmd, &format!("`{key}`"), v, |v| serde_json::from_value(v.clone()).ok())?;
+    Ok(Some(effect))
+}
+
+/// Refuse the keys of effect `given` that `parse` ignores, naming the keys the effect takes. Set to
+/// null, a key it reads fails to parse or changes the effect; a key it ignores changes nothing.
+fn refuse_ignored_effect_keys(cmd: &str, at: &str, given: &Value, parse: impl Fn(&Value) -> Option<Effect>) -> Result<()> {
+    let Some(fields) = given.as_object() else { return Ok(()) };
+    let parsed = parse(given);
+    let ignored: Vec<&str> = fields
+        .iter()
+        .filter(|(_, v)| !v.is_null())
+        .filter(|(k, _)| {
+            let mut probe = fields.clone();
+            probe.insert((*k).clone(), Value::Null);
+            parse(&Value::Object(probe)).is_some_and(|e| Some(e) == parsed)
+        })
+        .map(|(k, _)| k.as_str())
+        .collect();
+    if ignored.is_empty() {
+        return Ok(());
+    }
+    let takes = parsed.and_then(|e| serde_json::to_value(e).ok()).map(crate::inspect::camel).unwrap_or_default();
+    let kind = takes.get("type").and_then(Value::as_str).unwrap_or("effect").to_string();
+    let names: Vec<&str> = takes.as_object().into_iter().flatten().map(|(k, _)| k.as_str()).collect();
+    Err(bad(cmd, format!("{at}: unknown properties: {} (a {kind} takes {})", ignored.join(", "), names.join(", "))))
+}
+
 pub(crate) fn enum_of<T: serde::de::DeserializeOwned>(v: &Value) -> Option<T> {
     let s = v.as_str()?;
     // Accept camelCase, snake/upper-case and spaced variants.
@@ -297,6 +339,12 @@ pub const LAYOUT_KEYS: &[&str] = &[
     "gridRowTracks",
 ];
 
+/// Drop the bindings of auto layout fields (gap, padding…): they go with the auto layout, or
+/// re-applying them would bring it back.
+pub fn drop_layout_bindings(n: &mut Node) {
+    n.bound.retain(|k, _| !LAYOUT_KEYS.contains(&k.as_str()));
+}
+
 /// The auto layout of a frame-like node, created when it has none yet. Other nodes can't hold an
 /// auto layout: an auto layout key on them is refused, not stored where nothing reads it
 /// (`node.set` on several layers skips them instead, see `edit::set`).
@@ -315,18 +363,32 @@ fn layout_mut<'a>(cmd: &str, k: &str, n: &'a mut Node) -> Result<&'a mut AutoLay
     Ok(ensure_layout(n))
 }
 
-/// Apply a property map to one node. `x`/`y` are relative to the parent.
-pub fn apply(cmd: &str, doc: &mut Document, id: NodeId, props: &Map<String, Value>) -> Result<()> {
-    let mut unknown = Vec::new();
-    // Geometry keys first so rotation applies to the final size.
-    let order = |k: &str| match k {
+/// Replace a node's fills (or strokes). Bindings on the paints whose bound values the new list
+/// changed let go, as an explicit edit does (see `lists::rebind_edited_paint`).
+fn set_paints(n: &mut Node, fills: bool, paints: Vec<Paint>) {
+    let old = std::mem::replace(if fills { &mut n.fills } else { &mut n.strokes }, paints);
+    for (i, p) in old.iter().enumerate() {
+        super::lists::rebind_edited_paint(n, fills, i, p);
+    }
+}
+
+/// The order [`apply`] takes keys in: geometry first so rotation applies to the final size, variable
+/// bindings last so paints set in the same call don't detach them.
+fn apply_order(k: &str) -> u8 {
+    match k {
         "width" | "height" | "w" | "h" => 0,
         "rotation" => 2,
         "x" | "y" => 3,
+        "boundVariables" => 4,
         _ => 1,
-    };
+    }
+}
+
+/// Apply a property map to one node. `x`/`y` are relative to the parent.
+pub fn apply(cmd: &str, doc: &mut Document, id: NodeId, props: &Map<String, Value>) -> Result<()> {
+    let mut unknown = Vec::new();
     let mut keys: Vec<(&String, &Value)> = props.iter().collect();
-    keys.sort_by_key(|(k, _)| order(k));
+    keys.sort_by_key(|(k, _)| apply_order(k));
     for (k, v) in keys {
         let k = k.as_str();
         match k {
@@ -398,15 +460,15 @@ pub fn apply(cmd: &str, doc: &mut Document, id: NodeId, props: &Map<String, Valu
                 let b = v.as_str().and_then(figcraft_color::BlendMode::from_name).ok_or_else(|| bad(cmd, "unknown blend mode"))?;
                 doc.get_mut(id)?.blend = b;
             }
-            "fills" => doc.get_mut(id)?.fills = paints_of(v).ok_or_else(|| bad(cmd, "`fills` must be paints or colours"))?,
+            "fills" => set_paints(doc.get_mut(id)?, true, paints_of(v).ok_or_else(|| bad(cmd, "`fills` must be paints or colours"))?),
             "fill" => {
                 let c = color_of(v).ok_or_else(|| bad(cmd, "`fill` must be a colour"))?;
-                doc.get_mut(id)?.fills = vec![Paint::solid(c)];
+                set_paints(doc.get_mut(id)?, true, vec![Paint::solid(c)]);
             }
-            "strokes" => doc.get_mut(id)?.strokes = paints_of(v).ok_or_else(|| bad(cmd, "`strokes` must be paints or colours"))?,
+            "strokes" => set_paints(doc.get_mut(id)?, false, paints_of(v).ok_or_else(|| bad(cmd, "`strokes` must be paints or colours"))?),
             "stroke" => {
                 let c = color_of(v).ok_or_else(|| bad(cmd, "`stroke` must be a colour"))?;
-                doc.get_mut(id)?.strokes = vec![Paint::solid(c)];
+                set_paints(doc.get_mut(id)?, false, vec![Paint::solid(c)]);
             }
             "strokeWeight" => doc.get_mut(id)?.stroke.weight = f64_of(v).ok_or_else(|| bad(cmd, "`strokeWeight` must be a number"))?.max(0.0),
             "strokeAlign" => doc.get_mut(id)?.stroke.align = enum_of(v).ok_or_else(|| bad(cmd, "strokeAlign: inside|center|outside"))?,
@@ -462,7 +524,7 @@ pub fn apply(cmd: &str, doc: &mut Document, id: NodeId, props: &Map<String, Valu
                 }
             }
             "cornerSmoothing" => doc.get_mut(id)?.corners.smoothing = f64_of(v).unwrap_or(0.0).clamp(0.0, 1.0),
-            "effects" => doc.get_mut(id)?.effects = serde_json::from_value(snake(v)).map_err(|e| bad(cmd, format!("effects: {e}")))?,
+            "effects" => doc.get_mut(id)?.effects = effects_from(cmd, v)?,
             "clip" | "clipsContent" => doc.get_mut(id)?.clip = v.as_bool().ok_or_else(|| bad(cmd, "`clip` must be a boolean"))?,
             "isMask" => doc.get_mut(id)?.is_mask = v.as_bool().unwrap_or(false),
             "devStatus" => {
@@ -486,8 +548,14 @@ pub fn apply(cmd: &str, doc: &mut Document, id: NodeId, props: &Map<String, Valu
                     return Err(bad(cmd, "auto layout needs a frame"));
                 }
                 match v {
-                    Value::Null => n.layout = None,
-                    Value::String(s) if s.eq_ignore_ascii_case("none") => n.layout = None,
+                    Value::Null => {
+                        n.layout = None;
+                        drop_layout_bindings(n);
+                    }
+                    Value::String(s) if s.eq_ignore_ascii_case("none") => {
+                        n.layout = None;
+                        drop_layout_bindings(n);
+                    }
                     Value::String(s) => {
                         let mode = match s.to_ascii_lowercase().as_str() {
                             "horizontal" | "row" => LayoutMode::Horizontal,

crates/engine/src/cmd/structure.rs

@@ -10,11 +10,13 @@ use super::{CommandSpec, ok};
 use crate::params::*;
 use crate::{EngineError, Result, Session, bad, spec};
 
+/// The layer structure commands: group, frame, mask, boolean operations, flatten, auto layout
+/// add/remove, move to page and vector path edits.
 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 +44,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",
@@ -71,7 +73,9 @@ pub fn specs() -> Vec<CommandSpec> {
                 // Hugging needs auto layout and the children that filled have nothing to fill: both
                 // become fixed (the frame keeps filling its own auto layout parent).
                 let doc = s.doc_mut()?;
-                doc.get_mut(i)?.layout = None;
+                let n = doc.get_mut(i)?;
+                n.layout = None;
+                super::props::drop_layout_bindings(n);
                 figcraft_layout::coherent_sizing(doc, i);
             }
             ok()
@@ -128,26 +132,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();
@@ -285,7 +290,7 @@ fn convert_to_section(s: &mut Session, p: &Value) -> Result<Value> {
         n.grids.clear();
         n.guides.clear();
         n.styles.grid = None;
-        n.bound.retain(|k, _| !k.starts_with("padding") && !matches!(k.as_str(), "gap" | "counterAxisSpacing"));
+        super::props::drop_layout_bindings(n);
         let kids = n.children.clone();
         for k in kids {
             // Out of the flow there is nothing to fill or to be absolute against.
@@ -338,7 +343,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 +477,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 +524,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();

crates/engine/src/cmd/styles.rs

@@ -110,16 +110,26 @@ pub fn specs() -> Vec<CommandSpec> {
             ))
         }),
         spec!("variable.createCollection", "Create variable collection", "{name, modes? [names]}", true, |s, p| {
-            let name = req_string("variable.createCollection", p, "name")?;
-            let doc = s.doc_mut()?;
-            if doc.variables.collections.iter().any(|c| c.name == name) {
-                return Err(bad("variable.createCollection", format!("a collection named `{name}` already exists")));
+            let cmd = "variable.createCollection";
+            let name = req_string(cmd, p, "name")?.trim().to_string();
+            if name.is_empty() {
+                return Err(bad(cmd, "name can't be empty"));
             }
-            let id = doc.new_collection_id();
             let modes: Vec<String> = match get(p, "modes") {
-                Some(Value::Array(a)) => a.iter().filter_map(|m| m.as_str().map(str::to_string)).collect(),
+                Some(Value::Array(a)) => a.iter().filter_map(|m| m.as_str().map(|m| m.trim().to_string())).collect(),
                 _ => vec!["Mode 1".into()],
             };
+            if modes.is_empty() {
+                return Err(bad(cmd, "a collection needs at least one mode"));
+            }
+            if modes.iter().any(String::is_empty) {
+                return Err(bad(cmd, "mode names can't be empty"));
+            }
+            let doc = s.doc_mut()?;
+            if doc.variables.collections.iter().any(|c| c.name == name) {
+                return Err(bad(cmd, format!("a collection named `{name}` already exists")));
+            }
+            let id = doc.new_collection_id();
             let modes = modes.into_iter().enumerate().map(|(i, m)| Mode { id: format!("{id}/mode{}", i + 1), name: m }).collect();
             doc.variables.collections.push(Collection { id: id.clone(), name, modes, extends: None });
             Ok(json!({"id": id}))
@@ -950,6 +960,104 @@ pub fn apply_bindings_to(doc: &mut Document, only: Option<&std::collections::BTr
     apply_bindings_matching(doc, only, |_| true);
 }
 
+/// The layers `ids` before an explicit edit, for [`detach_edited`] (only those with bindings).
+pub(crate) fn bound_before(doc: &Document, ids: &[NodeId]) -> Vec<std::sync::Arc<Node>> {
+    ids.iter().filter_map(|i| doc.node_arc(*i)).filter(|n| !n.bound.is_empty()).cloned().collect()
+}
+
+/// An explicit edit sticks: each layer's own bound field (not a list item's) whose value the edit
+/// changed drops its binding, which would otherwise write the variable's value back (#809).
+/// Compared by value, so keys written unchanged keep their bindings and fields moved along with
+/// another (proportions, min/max) lose theirs.
+pub(crate) fn detach_edited(doc: &mut Document, before: &[std::sync::Arc<Node>]) {
+    for old in before {
+        let Some(n) = doc.node(old.id) else { continue };
+        let edited: Vec<String> = n
+            .bound
+            .keys()
+            // A binding the edit itself made (`boundVariables`) stays.
+            .filter(|f| old.bound.get(*f) == n.bound.get(*f))
+            .filter(|f| super::lists::list_binding(f).is_none() && !f.starts_with(crate::components::PROP_BINDING))
+            .filter(|f| bound_field(old, f) != bound_field(n, f))
+            .cloned()
+            .collect();
+        if edited.is_empty() {
+            continue;
+        }
+        if let Ok(n) = doc.get_mut(old.id) {
+            for f in edited {
+                n.bound.remove(&f);
+            }
+        }
+    }
+}
+
+/// The stored value a layer-level binding writes, comparable (`None`: the layer has no such field).
+fn bound_field(n: &Node, field: &str) -> Option<String> {
+    let text = |f: &dyn Fn(&TextStyle) -> String| match &n.kind {
+        NodeKind::Text(t) => Some(std::iter::once(&t.style).chain(t.runs.iter().map(|r| &r.style)).map(f).collect::<Vec<_>>().join(",")),
+        _ => None,
+    };
+    let layout = n.layout.as_ref();
+    let cl = &n.child_layout;
+    let pad = |i: usize| layout.and_then(|l| l.padding.get(i).copied());
+    Some(match field {
+        "width" => format!("{}", n.width),
+        "height" => format!("{}", n.height),
+        "minWidth" => format!("{:?}", cl.min_w),
+        "maxWidth" => format!("{:?}", cl.max_w),
+        "minHeight" => format!("{:?}", cl.min_h),
+        "maxHeight" => format!("{:?}", cl.max_h),
+        "opacity" => format!("{}", n.opacity),
+        "visible" => format!("{}", n.visible),
+        "gap" | "columnGap" => format!("{:?}", layout.map(|l| l.gap)),
+        "crossGap" | "rowGap" => format!("{:?}", layout.map(|l| l.cross_gap)),
+        "padding" => format!("{:?}", layout.map(|l| l.padding)),
+        "paddingTop" => format!("{:?}", pad(0)),
+        "paddingRight" => format!("{:?}", pad(1)),
+        "paddingBottom" => format!("{:?}", pad(2)),
+        "paddingLeft" => format!("{:?}", pad(3)),
+        "paddingHorizontal" => format!("{:?}", (pad(1), pad(3))),
+        "paddingVertical" => format!("{:?}", (pad(0), pad(2))),
+        "cornerRadius" => format!("{:?}", n.corners.radii),
+        "topLeftRadius" => format!("{:?}", n.corners.radii.first()),
+        "topRightRadius" => format!("{:?}", n.corners.radii.get(1)),
+        "bottomRightRadius" => format!("{:?}", n.corners.radii.get(2)),
+        "bottomLeftRadius" => format!("{:?}", n.corners.radii.get(3)),
+        "strokeWeight" => format!("{:?}", (n.stroke.weight, n.stroke.sides)),
+        "strokeTopWeight" | "strokeRightWeight" | "strokeBottomWeight" | "strokeLeftWeight" => format!("{:?}", n.stroke.sides),
+        "arcStart" | "arcSweep" | "arcInner" => match &n.kind {
+            NodeKind::Ellipse { start, sweep, inner } => format!(
+                "{}",
+                if field == "arcStart" {
+                    start
+                } else if field == "arcSweep" {
+                    sweep
+                } else {
+                    inner
+                }
+            ),
+            _ => return None,
+        },
+        "characters" => match &n.kind {
+            NodeKind::Text(t) => t.characters.clone(),
+            _ => return None,
+        },
+        "paragraphSpacing" | "paragraphIndent" => match &n.kind {
+            NodeKind::Text(t) => format!("{}", if field == "paragraphSpacing" { t.paragraph_spacing } else { t.paragraph_indent }),
+            _ => return None,
+        },
+        "fontFamily" => text(&|s| s.font_family.clone())?,
+        "fontStyle" => text(&|s| s.font_style.clone())?,
+        "fontWeight" => text(&|s| s.weight().to_string())?,
+        "fontSize" => text(&|s| s.font_size.to_string())?,
+        "lineHeight" => text(&|s| format!("{:?}", s.line_height))?,
+        "letterSpacing" => text(&|s| format!("{:?}", s.letter_spacing))?,
+        // shortcut: other layer-level fields keep their binding on edits; add a reader when one is bound in practice.
+        _ => return None,
+    })
+}
+
 /// Imported files (`.fig`, REST) keep a solid paint's alpha in its opacity, a bound paint's too,
 /// while the bound colour brings its own alpha: take the variable's alpha back out of the opacity
 /// (styles' paints and layers') before the bindings are first resolved, so it applies once.
@@ -1007,6 +1115,8 @@ pub(crate) fn apply_paint_bindings_on_load(doc: &mut Document) {
     doc.mark_dirty(pending);
 }
 
+/// Re-apply the variable bindings whose field passes `include`, on the `only` nodes (every bound
+/// node when `None`). Auto layout fields wait while their node has no auto layout.
 fn apply_bindings_matching(doc: &mut Document, only: Option<&std::collections::BTreeSet<NodeId>>, include: impl Fn(&str) -> bool) {
     use super::lists::{BoundList, list_binding};
     if doc.variables.variables.is_empty() {
@@ -1021,6 +1131,7 @@ fn apply_bindings_matching(doc: &mut Document, only: Option<&std::collections::B
         let modes = doc.modes_for(id);
         let Some(n) = doc.node(id) else { continue };
         let binds = n.bound.clone();
+        let has_layout = n.layout.is_some();
         let mut sets = serde_json::Map::new();
         let mut effect_sets: Vec<(usize, String, VariableValue)> = Vec::new();
         let mut paint_sets: Vec<(bool, usize, Option<usize>, Color)> = Vec::new();
@@ -1030,6 +1141,10 @@ fn apply_bindings_matching(doc: &mut Document, only: Option<&std::collections::B
             if !include(&field) {
                 continue;
             }
+            // A binding never creates an auto layout: without one it waits for the next.
+            if !has_layout && super::props::LAYOUT_KEYS.contains(&field.as_str()) {
+                continue;
+            }
             let Some(val) = doc.variables.resolve(&var, &modes) else { continue };
             // One list item's value: a paint's colour, an effect's or a layout grid's field.
             if let Some((list, i, prop)) = list_binding(&field) {

crates/engine/src/cmd/text.rs

@@ -18,6 +18,7 @@ fn style(s: &mut Session, p: &Value, f: impl Fn(&mut TextStyle)) -> Result<Value
         let ds = s.ds_mut()?;
         let before = ds.doc.clone();
         let doc = ds.doc_mut();
+        let bound = super::styles::bound_before(doc, &[id]);
         if let NodeKind::Text(t) = &mut doc.get_mut(id)?.kind {
             if a == b {
                 t.restyle(0, t.char_count(), &f);
@@ -25,19 +26,21 @@ fn style(s: &mut Session, p: &Value, f: impl Fn(&mut TextStyle)) -> Result<Value
                 t.restyle(a, b, &f);
             }
         }
+        super::styles::detach_edited(doc, &bound);
         crate::normalize(doc, &before);
         return ok();
     }
     let ids = req_targets("text", s, p)?;
     let doc = s.doc_mut()?;
-    for i in ids {
-        for d in doc.subtree(i) {
-            if let Some(NodeKind::Text(t)) = doc.node_mut(d).map(|n| &mut n.kind) {
-                let n = t.char_count();
-                t.restyle(0, n, &f);
-            }
+    let ids: Vec<NodeId> = ids.into_iter().flat_map(|i| doc.subtree(i)).collect();
+    let before = super::styles::bound_before(doc, &ids);
+    for d in ids {
+        if let Some(NodeKind::Text(t)) = doc.node_mut(d).map(|n| &mut n.kind) {
+            let n = t.char_count();
+            t.restyle(0, n, &f);
         }
     }
+    super::styles::detach_edited(doc, &before);
     ok()
 }
 
@@ -348,11 +351,13 @@ pub fn specs() -> Vec<CommandSpec> {
             let c = string(p, "characters").or_else(|| string(p, "text")).ok_or_else(|| bad("text.set", "`characters` required"))?;
             let ids = req_targets("text.set", s, p)?;
             let doc = s.doc_mut()?;
+            let before = super::styles::bound_before(doc, &ids);
             for i in ids {
                 let mut m = Map::new();
                 m.insert("characters".into(), json!(c));
                 super::props::apply("text.set", doc, i, &m)?;
             }
+            super::styles::detach_edited(doc, &before);
             ok()
         }),
         spec!(
@@ -364,6 +369,7 @@ pub fn specs() -> Vec<CommandSpec> {
                 let ids = req_targets("text.style", s, p)?;
                 let m = p.as_object().cloned().unwrap_or_default();
                 let doc = s.doc_mut()?;
+                let before = super::styles::bound_before(doc, &ids);
                 for i in ids {
                     let n = doc.get_mut(i)?;
                     let NodeKind::Text(t) = &mut n.kind else { return Err(bad("text.style", format!("{i} is not text"))) };
@@ -377,6 +383,7 @@ pub fn specs() -> Vec<CommandSpec> {
                         super::props::apply_text_style("text.style", t, a, b, k, v)?;
                     }
                 }
+                super::styles::detach_edited(doc, &before);
                 ok()
             }
         ),