diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 664d1ec3..8e89d825 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -1085,9 +1085,34 @@ fn set(s: &mut Session, p: &Value) -> Result { .collect(), None => Vec::new(), }; + let mut warnings = font_style_warnings(&s.ds()?.doc, &ids, &props); + // Auto layout keys go to the layers that can have auto layout; with none among the targets, + // the first refuses them (`props::layout_mut`). + let layout_keys: Vec<&str> = props.keys().map(String::as_str).filter(|k| super::props::LAYOUT_KEYS.contains(k)).collect(); + let mut without_layout = props.clone(); + let mut skipped = BTreeSet::new(); + if !layout_keys.is_empty() { + let doc = &s.ds()?.doc; + let frame_like = |i: &NodeId| doc.node(*i).is_some_and(|n| n.kind.is_frame_like()); + if ids.iter().any(frame_like) { + skipped = ids.iter().copied().filter(|i| !frame_like(i)).collect(); + without_layout.retain(|k, _| !layout_keys.contains(&k.as_str())); + } + if !skipped.is_empty() { + let names: Vec = + skipped.iter().take(10).filter_map(|i| doc.node(*i)).map(|n| format!("{} “{}” ({})", n.id, n.name, n.kind.type_name())).collect(); + let more = if skipped.len() > names.len() { format!(" and {} more", skipped.len() - names.len()) } else { String::new() }; + warnings.push(format!( + "{} skipped on {}{more}: only frames, components and instances have auto layout", + layout_keys.iter().map(|k| format!("`{k}`")).collect::>().join(", "), + names.join(", ") + )); + } + } let doc = s.doc_mut()?; for i in ids { - super::props::apply("node.set", doc, i, &props)?; + let props = if skipped.contains(&i) { &without_layout } else { &props }; + super::props::apply("node.set", doc, i, props)?; } if let Some(o) = pivot { for (i, q) in pinned { @@ -1097,7 +1122,43 @@ fn set(s: &mut Session, p: &Value) -> Result { } } } - ok() + if warnings.is_empty() { ok() } else { Ok(json!({"ok": true, "warnings": warnings})) } +} + +/// A `fontStyle` draws in the family's face nearest to it, as a text layer resolves it. When that +/// face has another weight or slant than the name asks for ("Black" on a family whose heaviest is +/// Bold), say which face it is. Families that aren't installed, and names that match a style or a +/// named instance (spelled any way: "SemiBold", "Semi Bold"), say nothing. +fn font_style_warnings(doc: &figcraft_doc::Document, ids: &[NodeId], props: &serde_json::Map) -> Vec { + let Some(style) = props.get("fontStyle").and_then(Value::as_str) else { return Vec::new() }; + let mut families: Vec = match props.get("fontFamily").and_then(Value::as_str) { + Some(f) => vec![f.to_string()], + None => ids + .iter() + .filter_map(|i| doc.node(*i)) + .filter_map(|n| n.kind.text()) + .flat_map(|t| t.spans().into_iter().map(|(_, _, st)| st.font_family.clone()).collect::>()) + .collect(), + }; + families.sort(); + families.dedup(); + let norm = |s: &str| s.to_lowercase().replace([' ', '-', '_'], ""); + let (want_w, want_i) = (i32::from(figcraft_doc::style_weight(style)), style.to_lowercase().contains("italic")); + families + .into_iter() + .filter_map(|family| { + let styles = figcraft_text::family_styles(&family); + if styles.is_empty() || styles.iter().any(|x| norm(x) == norm(style)) { + return None; + } + let face = figcraft_text::find_face(&family, style).filter(|f| f.family.eq_ignore_ascii_case(&family))?; + let w = face.axis("wght").map_or(i32::from(face.weight), |a| a.clamp(f64::from(want_w)).round() as i32); + if face.instance(style).is_some() || (w == want_w && face.italic == want_i) { + return None; + } + Some(format!("{family} has no `{style}` style: it draws in {}, the nearest (it has: {})", face.style, styles.join(", "))) + }) + .collect() } /// `stroke.widthProfile`: a preset or points (through `node.set`'s `widthProfile`, so instances diff --git a/crates/engine/src/cmd/props.rs b/crates/engine/src/cmd/props.rs index 1b8c45c8..e9578b62 100644 --- a/crates/engine/src/cmd/props.rs +++ b/crates/engine/src/cmd/props.rs @@ -260,8 +260,41 @@ fn ensure_layout(n: &mut Node) -> &mut AutoLayout { n.layout.get_or_insert_with(|| AutoLayout { padding: [0.0; 4], gap: 0.0, ..Default::default() }) } +/// The keys that set a frame's auto layout (through [`layout_mut`]). +pub const LAYOUT_KEYS: &[&str] = &[ + "gap", + "itemSpacing", + "columnGap", + "crossGap", + "counterAxisSpacing", + "rowGap", + "padding", + "paddingTop", + "paddingRight", + "paddingBottom", + "paddingLeft", + "paddingHorizontal", + "paddingVertical", + "mainAlign", + "primaryAxisAlignItems", + "crossAlign", + "counterAxisAlignItems", + "contentAlign", + "counterAxisAlignContent", + "wrap", + "gridColumns", + "gridRows", + "strokesIncluded", + "reverseZ", + "columnTracks", + "gridColumnTracks", + "rowTracks", + "gridRowTracks", +]; + /// 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. +/// 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`). fn layout_mut<'a>(cmd: &str, k: &str, n: &'a mut Node) -> Result<&'a mut AutoLayout> { if !n.kind.is_frame_like() { return Err(bad( @@ -869,15 +902,6 @@ pub fn apply_text_style(cmd: &str, t: &mut TextData, start: usize, end: usize, k } "fontStyle" => { let st = string_val(v).ok_or_else(|| bad(cmd, "`fontStyle` must be a string"))?; - // A style the family doesn't have would draw in another face without a word; refuse it - // and list the family's styles. A family that isn't installed keeps any name (files - // from elsewhere name their fonts' styles). - for (_, _, span) in t.spans().into_iter().filter(|(a, b, _)| *b > start && *a < end.max(start + 1)) { - let styles = figcraft_text::family_styles(&span.font_family); - if !styles.is_empty() && !styles.iter().any(|x| x.eq_ignore_ascii_case(&st)) { - return Err(bad(cmd, format!("{} has no `{st}` style (it has: {})", span.font_family, styles.join(", ")))); - } - } t.restyle(start, end, |s| s.set_font_style(st.clone())); } "fontWeight" => { diff --git a/crates/engine/src/node_set_refusals_tests.rs b/crates/engine/src/node_set_refusals_tests.rs index f465c519..7d0b68c8 100644 --- a/crates/engine/src/node_set_refusals_tests.rs +++ b/crates/engine/src/node_set_refusals_tests.rs @@ -1,5 +1,6 @@ -//! `node.set` refuses what can't apply instead of storing it where nothing reads it: auto layout -//! keys on layers that can't have auto layout, and a `fontStyle` the font family doesn't have. +//! `node.set` doesn't store what can't apply: auto layout keys go only to layers that can have auto +//! layout (the others are named in `warnings`, and with none of them the call is refused), and a +//! `fontStyle` the family lacks says which face draws instead. use serde_json::{Value, json}; @@ -32,24 +33,53 @@ fn auto_layout_keys_on_layers_without_auto_layout_are_refused() { } // Frames still take them, with or without a layout mode first. let f = id(&ex(&mut s, "node.create", json!({"type": "frame"}))["id"]); - ex(&mut s, "node.set", json!({"ids": [f], "props": {"gap": 8, "padding": 4}})); + let out = ex(&mut s, "node.set", json!({"ids": [f], "props": {"gap": 8, "padding": 4}})); assert_eq!(s.doc().unwrap().node(f).unwrap().layout.as_ref().map(|l| l.gap), Some(8.0)); + assert!(out.get("warnings").is_none(), "{out}"); } #[test] -fn a_font_style_the_family_lacks_is_refused_with_the_styles_it_has() { +fn with_several_layers_the_frames_take_auto_layout_keys_and_the_others_are_named() { + let mut s = Session::new(); + let f = id(&ex(&mut s, "node.create", json!({"type": "frame", "name": "Card"}))["id"]); + let t = id(&ex(&mut s, "node.create", json!({"type": "text", "text": "Hi", "name": "Title"}))["id"]); + let out = ex(&mut s, "node.set", json!({"ids": [f, t], "props": {"gap": 8, "opacity": 0.5}})); + let doc = s.doc().unwrap(); + assert_eq!(doc.node(f).unwrap().layout.as_ref().map(|l| l.gap), Some(8.0)); + assert!(doc.node(t).unwrap().layout.is_none(), "no hidden auto layout on the text"); + assert_eq!(doc.node(t).unwrap().opacity, 0.5, "the text still gets the other keys"); + let w = out["warnings"][0].as_str().unwrap(); + assert!(w.contains("`gap`") && w.contains(&format!("{t} “Title”")) && w.contains("TEXT"), "{w}"); + // One undo step for all of it. + ex(&mut s, "edit.undo", json!({})); + assert!(s.doc().unwrap().node(f).unwrap().layout.as_ref().is_none_or(|l| l.gap != 8.0)); +} + +#[test] +fn a_font_style_draws_in_the_nearest_face_and_says_so_when_it_differs() { figcraft_text::scan_system_fonts(); - // Any installed family with a known list of styles will do (CI machines differ). - let Some((family, styles)) = - figcraft_text::family_names().into_iter().map(|f| (f.clone(), figcraft_text::family_styles(&f))).find(|(_, st)| !st.is_empty()) - else { - return; - }; + // Any installed family with a regular and a bold face and nothing heavier (CI machines differ). + let pick = figcraft_text::family_names().into_iter().find_map(|f| { + let st = figcraft_text::family_styles(&f); + let has = |w: u16| st.iter().any(|x| figcraft_doc::style_weight(x) == w && !x.to_lowercase().contains("italic")); + (has(400) && has(700) && !st.iter().any(|x| figcraft_doc::style_weight(x) > 700)).then_some((f, st)) + }); + let Some((family, styles)) = pick else { return }; let mut s = Session::new(); let t = id(&ex(&mut s, "node.create", json!({"type": "text", "text": "Hi", "props": {"fontFamily": family}}))["id"]); - let msg = bad_msg(s.execute("node.set", &json!({"ids": [t], "props": {"fontStyle": "Definitely Not A Style"}}))); - assert!(msg.contains(&family) && msg.contains(&styles[0]), "{msg}"); - ex(&mut s, "node.set", json!({"ids": [t], "props": {"fontStyle": styles[0]}})); - // A family that isn't installed keeps any style name (files from elsewhere name theirs). - ex(&mut s, "node.set", json!({"ids": [t], "props": {"fontFamily": "No Such Family 7f3a", "fontStyle": "Hairline"}})); + let set = |s: &mut Session, style: &str| ex(s, "node.set", json!({"ids": [t], "props": {"fontStyle": style}})); + // Any spelling of a style it has, and names that resolve to the weight they ask for, say nothing. + let bold = styles.iter().find(|x| figcraft_doc::style_weight(x) == 700).unwrap().clone(); + for style in [bold.clone(), bold.to_uppercase(), bold.replace(' ', ""), "Roman".into(), "Normal".into()] { + let out = set(&mut s, &style); + assert!(out.get("warnings").is_none(), "{style}: {out}"); + } + // A heavier weight than the family has applies (as a text layer resolves it) and says which face draws. + let out = set(&mut s, "Black"); + let w = out["warnings"][0].as_str().unwrap(); + assert!(w.contains(&family) && w.contains("`Black`") && w.contains("nearest"), "{w}"); + assert_eq!(s.doc().unwrap().node(t).unwrap().kind.text().unwrap().spans()[0].2.font_style, "Black"); + // A family that isn't installed keeps any style name without a word (files from elsewhere name theirs). + let out = ex(&mut s, "node.set", json!({"ids": [t], "props": {"fontFamily": "No Such Family 7f3a", "fontStyle": "Hairline"}})); + assert!(out.get("warnings").is_none(), "{out}"); } diff --git a/crates/mcp/src/tools.rs b/crates/mcp/src/tools.rs index a5a2c062..16f1b556 100644 --- a/crates/mcp/src/tools.rs +++ b/crates/mcp/src/tools.rs @@ -163,7 +163,7 @@ pub fn tool_definitions() -> Vec { tool( "set_properties", "Set properties", - "Set properties on one or more layers as one undo step (engine `node.set`). Keys: name, visible, locked, opacity (0-1), blend, x, y (relative to the parent frame; groups don't count), width, height, rotation (degrees), fill (colour) / fills (array of paints: \"#hex\" or {\"type\":\"solid\",\"color\":\"#hex\"} or {\"type\":\"linearGradient\",\"stops\":[\"#hex\",\"#hex\"]} or stops [{\"position\":0..1,\"color\":\"#hex\"}], optional from/to handles in the unit box; radialGradient too), stroke / strokes, strokeWeight, strokeAlign (inside|center|outside), cornerRadius (n or [tl,tr,br,bl]), effects (array, e.g. {\"type\":\"dropShadow\",\"color\":\"#00000040\",\"offset\":[0,4],\"radius\":8}), clip, layout (null|horizontal|vertical|grid), gap, padding (n or [t,r,b,l]), mainAlign (min|center|max|spaceBetween|spaceAround|spaceEvenly), crossAlign, wrap, contentAlign (auto|spaceBetween: wrapped lines), sizingH / sizingV (fixed|hug|fill), constraints {horizontal, vertical}, characters, fontFamily, fontStyle, fontSize, lineHeight, letterSpacing, textAlign, textAutoResize. Auto layout keys (gap, padding, mainAlign, crossAlign, wrap, …) apply to frames, components and instances only; on other layers they are an error, as is a fontStyle the font family doesn't have (the error lists its styles). Run list_commands with filter \"node.set\" for the full list. Unknown keys are an error. Returns the updated layers.", + "Set properties on one or more layers as one undo step (engine `node.set`). Keys: name, visible, locked, opacity (0-1), blend, x, y (relative to the parent frame; groups don't count), width, height, rotation (degrees), fill (colour) / fills (array of paints: \"#hex\" or {\"type\":\"solid\",\"color\":\"#hex\"} or {\"type\":\"linearGradient\",\"stops\":[\"#hex\",\"#hex\"]} or stops [{\"position\":0..1,\"color\":\"#hex\"}], optional from/to handles in the unit box; radialGradient too), stroke / strokes, strokeWeight, strokeAlign (inside|center|outside), cornerRadius (n or [tl,tr,br,bl]), effects (array, e.g. {\"type\":\"dropShadow\",\"color\":\"#00000040\",\"offset\":[0,4],\"radius\":8}), clip, layout (null|horizontal|vertical|grid), gap, padding (n or [t,r,b,l]), mainAlign (min|center|max|spaceBetween|spaceAround|spaceEvenly), crossAlign, wrap, contentAlign (auto|spaceBetween: wrapped lines), sizingH / sizingV (fixed|hug|fill), constraints {horizontal, vertical}, characters, fontFamily, fontStyle, fontSize, lineHeight, letterSpacing, textAlign, textAutoResize. Auto layout keys (gap, padding, mainAlign, crossAlign, wrap, …) apply to frames, components and instances only: other layers among the ids are skipped and named in `warnings`, and with none that can take them the call is an error. A fontStyle draws in the family's nearest face; `warnings` says when that face has another weight or slant than the name asks for. Run list_commands with filter \"node.set\" for the full list. Unknown keys are an error. Returns the updated layers.", obj( json!({"ids": ids_schema("Layer ids (default: the selection)"), "props": {"type": "object", "description": "Property values to set"}}), &["props"], @@ -544,7 +544,7 @@ fn set_properties(b: &mut dyn Backend, a: &Map) -> Result i, None => b @@ -555,7 +555,11 @@ fn set_properties(b: &mut dyn Backend, a: &Map) -> Result = ids.iter().take(20).filter_map(|i| b.call("node.get", json!({"id": i, "depth": 0})).ok()).collect(); - Ok(ToolResult::json(&json!({"ok": true, "nodes": nodes}))) + let mut out = json!({"ok": true, "nodes": nodes}); + if let Some(w) = warnings { + out["warnings"] = w; + } + Ok(ToolResult::json(&out)) } fn inspect_document(b: &mut dyn Backend, a: &Map) -> Result { diff --git a/crates/mcp/tests/protocol.rs b/crates/mcp/tests/protocol.rs index 89f2a649..6e943a1e 100644 --- a/crates/mcp/tests/protocol.rs +++ b/crates/mcp/tests/protocol.rs @@ -208,3 +208,14 @@ fn serve_accepts_a_limit_sized_frame_and_a_final_unterminated_frame() { assert_eq!(lines[0]["result"], json!({})); assert_eq!(lines[1]["id"], 2); } + +#[test] +fn set_properties_passes_on_what_it_skipped() { + let mut c = Client::new(); + let f = c.ok("create_node", json!({"type": "frame"}))["id"].clone(); + let t = c.ok("create_node", json!({"type": "text", "text": "Hi"}))["id"].clone(); + let r = c.ok("set_properties", json!({"ids": [f, t], "props": {"gap": 8}})); + assert!(r["warnings"][0].as_str().is_some_and(|w| w.contains("`gap`") && w.contains("TEXT")), "{r}"); + assert!(c.ok("set_properties", json!({"ids": [f], "props": {"gap": 4}})).get("warnings").is_none()); + assert!(c.err("set_properties", json!({"ids": [t], "props": {"gap": 4}})).contains("auto layout")); +}