From 6f053fc7b30ea27928785effabc4f94396199448 Mon Sep 17 00:00:00 2001 From: moppeeer Date: Sun, 11 Oct 2026 20:06:43 +0200 Subject: [PATCH] fix(engine): Object fills and strokes must name a swatch object.fill and object.stroke stored any string as the swatch, so a name like `Paper` or `Black` (the swatches are `[Paper]`, `[Black]`) painted nothing without an error. They now refuse an unknown name, change nothing, and suggest the swatch it was probably meant to be. Co-Authored-By: Claude Opus 5.5 --- ROADMAP.md | 1 + crates/engine/src/cmd/object.rs | 53 +++++++++++++++++++++++++++++++-- 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 9a0f71e70..f939403f7 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -129,6 +129,7 @@ list) · [roadmap.md](docs/roadmap.md) (milestones, plan) · [parity-checklist.m Newest first. +- **2026-10-11** · `object.fill` and `object.stroke` refuse a swatch name the document doesn't have (it painted nothing) and suggest the name meant (`Paper` → `[Paper]`). No readiness percentage change. - **2026-10-10 (UI.4–UI.5):** Panel tabs can move into other groups, split the dock, float, resize and return to their saved locations through shared docking transactions. Layouts persist in custom workspaces; UI and control commands share the same validated action path. Shared pane geometry and widget-state theme application retain all five palettes and local fonts. Split panes send keyboard input only to the focused canvas and scope contextual toolbar IDs per pane. No readiness percentage change. - **2026-10-10** · Version 0.6.0; releases add a Linux riscv64 tar.gz (cross-compiled, CLI smoke-tested under QEMU). diff --git a/crates/engine/src/cmd/object.rs b/crates/engine/src/cmd/object.rs index 17c44c27a..593f2b1d8 100644 --- a/crates/engine/src/cmd/object.rs +++ b/crates/engine/src/cmd/object.rs @@ -14,6 +14,17 @@ use serde_json::{Value, json}; use super::{CommandSpec, bad, bool_or, cmd, f64_or, has_doc, has_selection, ok, point_param, rect_param, spread_param, str_param, targets}; use crate::{Result, Session}; +/// `name` must be a swatch of the document: an unknown name would paint nothing. The error names +/// the swatch it was probably meant to be (`Paper` → `[Paper]`, a different case). +fn known_swatch(doc: &designcraft_doc::Document, name: &str, cmd: &str, key: &str) -> Result<()> { + if doc.swatch(name).is_some() { + return Ok(()); + } + let bare = |n: &str| n.trim().trim_start_matches('[').trim_end_matches(']').to_lowercase(); + let hint = doc.swatches.iter().find(|s| bare(&s.name) == bare(name)).map(|s| format!(" (did you mean `{}`?)", s.name)).unwrap_or_default(); + Err(bad(cmd, format!("{key}: no swatch named `{name}`{hint}; create one with `swatch.create` and use its returned name"))) +} + pub fn specs() -> Vec { vec![ cmd!( @@ -160,8 +171,9 @@ pub fn specs() -> Vec { ) } ), - cmd!("object.fill", "Fill", [], None, "{swatch, tint?: 0..1, ids?}", has_selection, |s, p| { + cmd!("object.fill", "Fill", [], None, "{swatch (a swatch's name, e.g. [Paper]), tint?: 0..1, ids?}", has_selection, |s, p| { let sw = str_param(p, "swatch").ok_or_else(|| bad("object.fill", "missing swatch"))?.to_string(); + known_swatch(&s.doc()?.doc, &sw, "object.fill", "swatch")?; let tint = f64_or(p, "tint", 1.0) as f32; set_flag(s, p, move |i| i.fill = Fill { swatch: sw.clone(), tint, ..i.fill.clone() }, false) }), @@ -173,6 +185,11 @@ pub fn specs() -> Vec { "{swatch?, tint?, weight?, align?: center|inside|outside, type?: {kind:…}, cap?: butt|round|projecting, join?: miter|round|bevel, miterLimit?, start?, end?: none|simple|simpleWide|triangle|triangleWide|barbed|curved|circle|circleSolid|square|squareSolid|bar, gapSwatch?, gapTint?, gapOverprint?: bool, ids?}", has_selection, |s, p| { + for key in ["swatch", "gapSwatch"] { + if let Some(name) = p.get(key).and_then(Value::as_str) { + known_swatch(&s.doc()?.doc, name, "object.stroke", key)?; + } + } let p2 = p.clone(); set_flag( s, @@ -2151,7 +2168,7 @@ mod scale_tests { s.execute("file.new", &json!({})).unwrap(); let r = s.execute("frame.create", &json!({"rect": [100, 100, 200, 150]})).unwrap(); let id = ItemId(r["id"].as_u64().unwrap()); - s.execute("object.stroke", &json!({"weight": 4, "swatch": "Black"})).unwrap(); + s.execute("object.stroke", &json!({"weight": 4, "swatch": "[Black]"})).unwrap(); let weight = |s: &Session| s.doc().unwrap().doc.item(id).unwrap().stroke.weight; let bounds = |s: &Session| s.doc().unwrap().doc.item(id).unwrap().bounds(); s.execute("transform.scale", &json!({"sx": 2.0})).unwrap(); @@ -2714,3 +2731,35 @@ mod blend_mode_tests { assert_eq!(it.opacity, 1.0); } } + +#[cfg(test)] +mod swatch_name_tests { + use serde_json::json; + + #[test] + fn fills_and_strokes_must_name_a_swatch_and_change_nothing_otherwise() { + let mut s = crate::Session::new(); + s.execute("file.new", &json!({})).unwrap(); + let id = s.execute("frame.create", &json!({"rect": [0, 0, 100, 100]})).unwrap()["id"].clone(); + for (cmd, params, hint) in [ + ("object.fill", json!({"swatch": "Paper", "ids": [id]}), Some("[Paper]")), + ("object.fill", json!({"swatch": "[black]", "ids": [id]}), Some("[Black]")), + ("object.fill", json!({"swatch": "Missing Ink", "ids": [id]}), None), + ("object.stroke", json!({"swatch": "#ff0000", "weight": 4, "ids": [id]}), None), + ("object.stroke", json!({"gapSwatch": "paper", "ids": [id]}), Some("[Paper]")), + ] { + let before = s.doc().unwrap().doc.clone(); + let e = s.execute(cmd, ¶ms).unwrap_err().to_string(); + assert!(e.contains("no swatch named") && e.contains("swatch.create"), "{cmd}: {e}"); + if let Some(h) = hint { + assert!(e.contains(&format!("did you mean `{h}`")), "{cmd}: {e}"); + } + assert!(std::sync::Arc::ptr_eq(&before, &s.doc().unwrap().doc), "{cmd}: nothing changed"); + } + // Real names, [None] included, still work. + for sw in ["[Paper]", "[None]", "C=100 M=0 Y=0 K=0"] { + s.execute("object.fill", &json!({"swatch": sw, "ids": [id]})).unwrap(); + s.execute("object.stroke", &json!({"swatch": sw, "ids": [id]})).unwrap(); + } + } +}