From 80c29046811f0ee0e5eb7447803106c29c194db2 Mon Sep 17 00:00:00 2001 From: KnottyDyes <47539659+KnottyDyes@users.noreply.github.com> Date: Sun, 11 Oct 2026 22:10:01 -0500 Subject: [PATCH 1/5] fix(mcp): prevent request replay after lost replies Do not resend a request after a transport failure. Report the uncertain outcome and reconnect on the next explicit request. Validation: all six cargo xtask ci gates passed on the merge candidate; 1,177 native tests passed and all 15 WASM packages checked. Lost-reply, invalid UTF-8 and timeout probes reproduced duplicate edits before the fix and one edit afterward. Task: #514 --- crates/mcp/src/backend.rs | 60 +++++++++++++++++++++++++++++++++++---- docs/mcp.md | 3 +- 2 files changed, 56 insertions(+), 7 deletions(-) diff --git a/crates/mcp/src/backend.rs b/crates/mcp/src/backend.rs index 0b940aa3a..c4930cc99 100644 --- a/crates/mcp/src/backend.rs +++ b/crates/mcp/src/backend.rs @@ -78,15 +78,11 @@ impl Backend for Remote { let id = self.next_id; self.next_id += 1; let line = json!({"id": id, "method": method, "params": params}).to_string(); - // One retry with a fresh connection (the app may have restarted). let reply = match self.roundtrip(&line) { Ok(r) => r, - Err(_) => { + Err(e) => { self.conn = None; - self.roundtrip(&line).map_err(|e| { - self.conn = None; - format!("DesignCraft app at {} is not reachable: {e}", self.addr) - })? + return Err(format!("DesignCraft app at {}: {e}; operation may have completed; inspect state before retrying", self.addr)); } }; let v: Value = serde_json::from_str(reply.trim()).map_err(|e| format!("bad reply from app: {e}"))?; @@ -105,3 +101,55 @@ impl Backend for Remote { format!("connected to the DesignCraft app at {}", self.addr) } } + +#[cfg(test)] +mod tests { + use super::*; + use std::net::TcpListener; + + #[test] + fn remote_does_not_replay_when_reply_is_lost() { + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let addr = listener.local_addr().unwrap().to_string(); + let server = std::thread::spawn(move || { + let first = { + let (stream, _) = listener.accept().unwrap(); + stream.set_read_timeout(Some(Duration::from_secs(5))).unwrap(); + let mut reader = BufReader::new(stream); + let mut line = String::new(); + assert!(reader.read_line(&mut line).unwrap() > 0); + let request: Value = serde_json::from_str(&line).unwrap(); + assert_eq!(request["method"], "engine.execute"); + assert_eq!(request["params"]["command"], "frame.create"); + request + }; + let mut applied = 1; + let (mut stream, _) = listener.accept().unwrap(); + stream.set_read_timeout(Some(Duration::from_secs(5))).unwrap(); + stream.set_write_timeout(Some(Duration::from_secs(5))).unwrap(); + let mut reader = BufReader::new(stream.try_clone().unwrap()); + loop { + let mut line = String::new(); + assert!(reader.read_line(&mut line).unwrap() > 0); + let request: Value = serde_json::from_str(&line).unwrap(); + if request == first { + applied += 1; + writeln!(stream, "{}", json!({"id": request["id"], "ok": true, "result": {"applied": applied}})).unwrap(); + } else { + assert_eq!(request, json!({"id": 2, "method": "document.inspect", "params": {}})); + writeln!(stream, "{}", json!({"id": request["id"], "ok": true, "result": {"applied": applied}})).unwrap(); + return applied; + } + } + }); + let mut remote = Remote::connect(&addr).unwrap(); + + let result = remote.call("engine.execute", json!({"command": "frame.create", "params": {"rect": [0, 0, 100, 100]}})); + let next = remote.call("document.inspect", json!({})); + let applied = server.join().unwrap(); + + assert_eq!(applied, 1, "lost reply must not replay the applied edit; client returned {result:?}"); + assert!(result.is_err(), "lost reply must report an uncertain outcome: {result:?}"); + assert_eq!(next, Ok(json!({"applied": 1})), "the next explicit request must reconnect successfully"); + } +} diff --git a/docs/mcp.md b/docs/mcp.md index 772f8a91f..17c03a3df 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -51,7 +51,8 @@ claude mcp add designcraft -- designcraft-cli mcp | UI-only commands via `execute` (`view.*`, `window.*`, `help.*`, `edit.dynamicSpelling`, and `app.*` except `app.links`) | Error | Work | In connected mode every tool maps to a control-channel method (`engine.execute`, `ui.render`, `ui.pointer`, …), so -what the agent does shows up live in the window. The connection is re-established once if the app restarts. +what the agent does shows up live in the window. A failed request is not resent: the operation may have completed, +so inspect the document before retrying. The next request re-establishes the connection. ## Coordinates From a21e8db14fa24df3fc6136ccb2ec93d8aebbdb1d Mon Sep 17 00:00:00 2001 From: seanth Date: Sun, 11 Oct 2026 23:27:52 -0400 Subject: [PATCH 2/5] fix(compose): key parent-page compositions by absolute page index Parent-page text now uses the absolute page index as its cache key, so repeated page numbers and renumbering do not reuse another page's composition. Validation: targeted canvas and PDF regressions, plus all six cargo xtask ci gates on main after #514 (1,179 native tests passed, 3 ignored; 15 crates passed WASM compilation checks). Task: #523 --- crates/compose/src/cache.rs | 32 ++++++++++------ crates/engine/src/cmd/variables.rs | 59 +++++++++++++++++++++++++++++- crates/pdf/src/export.rs | 31 ++++++++-------- crates/render/src/fx.rs | 8 ++-- crates/render/src/lib.rs | 27 +++++++------- 5 files changed, 111 insertions(+), 46 deletions(-) diff --git a/crates/compose/src/cache.rs b/crates/compose/src/cache.rs index 6d6b1b980..8aabbf0f1 100644 --- a/crates/compose/src/cache.rs +++ b/crates/compose/src/cache.rs @@ -13,7 +13,8 @@ use designcraft_doc::{Document, Item, Story, StoryId, Styles, WrapMode}; use crate::vars::RunningIndex; use crate::{ComposeOptions, ComposedStory}; -type Key = (StoryId, Option); +/// A story, composed for one absolute page (parent-page items) or for its own frames (`None`). +type Key = (StoryId, Option); struct Entry { sig: Vec, @@ -55,11 +56,23 @@ impl Cache { Self::default() } - /// The composed story, from cache or freshly composed. - pub fn get(&self, doc: &Document, sid: StoryId, page_name: Option<&str>) -> Arc { + /// The composed story, from cache or freshly composed. `page` is the absolute page a + /// parent-page item is shown on (its page number markers and variables are that page's), or + /// `None` for a story's own frames. Pages are identified by index, not name: names repeat when + /// sections restart their numbering. + pub fn get(&self, doc: &Document, sid: StoryId, page: Option) -> Arc { let Some(story) = doc.stories.get(&sid) else { return Arc::new(ComposedStory { story: sid, ..Default::default() }) }; let (mut sig, keep_items) = signature(doc, story); - let key = (sid, page_name.map(str::to_string)); + let key = (sid, page); + // The key holds the page's index, which a renumbering doesn't change: its name (section + // start number, style, prefix) is part of the signature. + let page_name = page.map(|p| doc.page_name(p)); + if let Some(name) = &page_name { + use std::hash::{Hash, Hasher}; + let mut h = std::collections::hash_map::DefaultHasher::new(); + name.hash(&mut h); + sig.push(h.finish() as usize); + } let with_vars = crate::vars::has_vars(story); let running = with_vars && crate::vars::has_running(doc, story); if with_vars { @@ -125,23 +138,20 @@ impl Cache { return e.out.clone(); } } - // Parent-page items are composed per page; variables need that page's index. - let page = match page_name { - Some(n) if with_vars => (0..doc.page_count()).find(|&i| doc.page_name(i) == n), - _ => None, - }; + // Parent-page items are composed per page: markers show its name, variables need its index. + let page = page.filter(|_| with_vars); let running = running.then(|| self.running_index(doc)); let out = Arc::new(crate::compose_story( doc, sid, - &ComposeOptions { page_name: page_name.map(str::to_string), page, running, label: None, xrefs: with_xrefs.then(|| self.xref_index(doc)) }, + &ComposeOptions { page_name, page, running, label: None, xrefs: with_xrefs.then(|| self.xref_index(doc)) }, )); let mut g = self.map.lock().unwrap_or_else(|e| e.into_inner()); let stamp = g.1; if g.0.len() > 4096 { g.0.retain(|_, e| stamp - e.stamp < 64); } - let replaced = g.0.insert(key.clone(), Entry { sig, _keep: (story.clone(), doc.styles.clone(), keep_items), out: out.clone(), stamp }); + let replaced = g.0.insert(key, Entry { sig, _keep: (story.clone(), doc.styles.clone(), keep_items), out: out.clone(), stamp }); drop(g); if key.1.is_none() && let Some(old) = replaced diff --git a/crates/engine/src/cmd/variables.rs b/crates/engine/src/cmd/variables.rs index bbcfaf0a1..e7772d4ea 100644 --- a/crates/engine/src/cmd/variables.rs +++ b/crates/engine/src/cmd/variables.rs @@ -129,6 +129,63 @@ fn define(s: &mut Session, p: &Value) -> Result { mod tests { use super::*; + /// A parent-page page-number marker follows Numbering & Section Options: changing a section's + /// start number renames its pages without changing their index or the number of sections, and + /// the marker composed for a page shows the new name. + #[test] + fn parent_page_number_follows_section_renumbering() { + let mut s = Session::new(); + s.execute("file.new", &json!({"pages": 3, "facingPages": false})).unwrap(); + let marker = designcraft_doc::story::PAGE_NUMBER.to_string(); + let r = s + .execute("frame.create", &json!({"spread": {"kind": "parent", "index": 0}, "rect": [72, 30, 500, 60], "content": "text", "text": marker})) + .unwrap(); + let sid = s.doc().unwrap().doc.item(designcraft_doc::ItemId(r["id"].as_u64().unwrap())).unwrap().text_frame().unwrap().story; + assert_eq!(s.doc().unwrap().doc.story(sid).unwrap().text, marker); + let shown = |s: &Session, page: usize| -> usize { + let d = s.doc().unwrap().doc.clone(); + let cs = s.cache.get(&d, sid, Some(page)); + cs.frames.iter().flat_map(|f| f.lines.iter()).map(|l| l.glyphs.iter().filter(|g| g.visible && g.adv > 0.0).count()).sum() + }; + assert_eq!(shown(&s, 2), 1, "page 3"); + s.execute("layout.section", &json!({"page": 1, "startNumber": 100})).unwrap(); + assert_eq!(s.doc().unwrap().doc.page_name(2), "102"); + assert_eq!(shown(&s, 2), 3, "page 102 after renumbering"); + } + + /// Two sections that both number from 1 repeat page names ("1, 2, 1, 2"). A parent-page running + /// header is composed for the page it is shown on, not the first page with that name. + #[test] + fn running_header_on_parent_page_with_repeated_page_numbers() { + let mut s = Session::new(); + s.execute("file.new", &json!({"pages": 4, "facingPages": false})).unwrap(); + s.execute("layout.section", &json!({"page": 3, "startNumber": 1})).unwrap(); + let names: Vec = (0..4).map(|i| s.doc().unwrap().doc.page_name(i)).collect(); + assert_eq!(names, ["1", "2", "1", "2"]); + s.execute("style.paragraph.create", &json!({"name": "Heading"})).ok(); + // The only heading is on the third page (the second section's page 1). + let sp = s.doc().unwrap().doc.page_loc(2).unwrap().0; + let r = s.execute("frame.create", &json!({"spread": sp, "rect": [72, 200, 400, 260], "content": "text", "text": "Epsilon"})).unwrap(); + let sid = s.doc().unwrap().doc.item(designcraft_doc::ItemId(r["id"].as_u64().unwrap())).unwrap().text_frame().unwrap().story; + s.execute("text.select", &json!({"story": sid.0, "anchor": 0, "focus": 1})).unwrap(); + s.execute("style.paragraph.apply", &json!({"name": "Heading"})).unwrap(); + s.execute("variables.define", &json!({"name": "Chapter Head", "type": "runningHeader", "style": "Heading"})).unwrap(); + let r = s + .execute("frame.create", &json!({"spread": {"kind": "parent", "index": 0}, "rect": [72, 30, 500, 60], "content": "text", "text": ""})) + .unwrap(); + let sid = s.doc().unwrap().doc.item(designcraft_doc::ItemId(r["id"].as_u64().unwrap())).unwrap().text_frame().unwrap().story; + s.execute("text.select", &json!({"story": sid.0, "anchor": 0, "focus": 0})).unwrap(); + s.execute("variables.insert", &json!({"name": "Chapter Head"})).unwrap(); + let d = s.doc().unwrap().doc.clone(); + let shown = |page: usize| -> usize { + let cs = s.cache.get(&d, sid, Some(page)); + cs.frames.iter().flat_map(|f| f.lines.iter()).map(|l| l.glyphs.iter().filter(|g| g.visible && g.adv > 0.0).count()).sum() + }; + assert_eq!(shown(0), 0, "no heading on or before the first page"); + assert_eq!(shown(2), "Epsilon".len(), "the third page (named 1) shows its own heading"); + assert_eq!(shown(3), "Epsilon".len(), "the fourth page (named 2) carries it"); + } + #[test] fn running_header_on_parent_page_follows_headings() { let mut s = Session::new(); @@ -158,7 +215,7 @@ mod tests { assert!(matches!(d.find(fid).unwrap().spread, designcraft_doc::SpreadRef::Parent(0))); // Visible glyphs of the header composed for each page: "Alpha · 3" vs "Epsilon · 3". let shown = |page: usize| -> usize { - let cs = s.cache.get(&d, sid, Some(&d.page_name(page))); + let cs = s.cache.get(&d, sid, Some(page)); cs.frames.iter().flat_map(|f| f.lines.iter()).map(|l| l.glyphs.iter().filter(|g| g.visible && g.adv > 0.0).count()).sum() }; let (p1, p2, p3) = (shown(0), shown(1), shown(2)); diff --git a/crates/pdf/src/export.rs b/crates/pdf/src/export.rs index d92fc242b..dcf8abb83 100644 --- a/crates/pdf/src/export.rs +++ b/crates/pdf/src/export.rs @@ -630,7 +630,6 @@ impl Exporter<'_> { let Some((ppi, ppage)) = doc.parent_page_for(first + pi) else { continue }; let parent = &doc.parents[ppi]; let dx = page.x - parent.pages[ppage].x; - let page_name = doc.page_name(first + pi); for it in &parent.items { if it.layer != layer.id || page.overridden.contains(&it.id) { continue; @@ -639,7 +638,7 @@ impl Exporter<'_> { if parent.page_at_x(it.bounds().center().x) != Some(ppage) && parent.pages.len() > 1 { continue; } - self.top_item(s, it, Affine::translate((dx, 0.0)), Some(&page_name), true); + self.top_item(s, it, Affine::translate((dx, 0.0)), Some(first + pi), true); } } for it in &sp.items { @@ -679,9 +678,9 @@ impl Exporter<'_> { } /// A spread or parent item, inside its structure tag when tagging. - fn top_item(&mut self, s: &mut Surface, it: &Item, parent: Affine, page_name: Option<&str>, on_parent: bool) { + fn top_item(&mut self, s: &mut Surface, it: &Item, parent: Affine, page: Option, on_parent: bool) { if !self.opts.tagged || it.hidden || it.nonprinting { - return self.item(s, it, parent, page_name); + return self.item(s, it, parent, page); } let story = it.text_frame().map(|t| t.story); let figure = !it.alt_text.is_empty() || matches!(it.content, Content::Graphic(_)) || it.has_nested_items(); @@ -689,7 +688,7 @@ impl Exporter<'_> { // Page furniture and decoration. let kind = if on_parent { ArtifactType::Page } else { ArtifactType::Other }; s.start_tagged(ContentTag::Artifact(Artifact::new(kind, None))); - self.item(s, it, parent, page_name); + self.item(s, it, parent, page); s.end_tagged(); return; } @@ -700,12 +699,12 @@ impl Exporter<'_> { self.tags.push(TagEntry::Story(sid)); } self.tag_story = Some(sid); - self.item(s, it, parent, page_name); + self.item(s, it, parent, page); self.tag_story = None; return; } let id = s.start_tagged(ContentTag::Other); - self.item(s, it, parent, page_name); + self.item(s, it, parent, page); s.end_tagged(); let alt = (!it.alt_text.is_empty()).then(|| it.alt_text.clone()); let mut g = TagGroup::new(Tag::Figure(alt)); @@ -751,10 +750,10 @@ impl Exporter<'_> { tree } - fn item(&mut self, s: &mut Surface, it: &Item, parent: Affine, page_name: Option<&str>) { + fn item(&mut self, s: &mut Surface, it: &Item, parent: Affine, page: Option) { let gf = &it.effects.gradient_feather; if !gf.on || it.hidden || it.nonprinting { - return self.item_inner(s, it, parent, page_name); + return self.item_inner(s, it, parent, page); } // Gradient feather: a luminosity mask (white = opaque) over the object. let xf = parent * it.xf; @@ -804,11 +803,11 @@ impl Exporter<'_> { ms.finish(); let stream = sb.finish(); s.push_mask(krilla::mask::Mask::new(stream, krilla::mask::MaskType::Luminosity)); - self.item_inner(s, it, parent, page_name); + self.item_inner(s, it, parent, page); s.pop(); } - fn item_inner(&mut self, s: &mut Surface, it: &Item, parent: Affine, page_name: Option<&str>) { + fn item_inner(&mut self, s: &mut Surface, it: &Item, parent: Affine, page: Option) { if it.hidden || it.nonprinting { return; } @@ -843,7 +842,7 @@ impl Exporter<'_> { s.push_clip_path(c, &FillRule::NonZero); } for c in it.children() { - self.item(s, c, xf, page_name); + self.item(s, c, xf, page); } if clip.is_some() { s.pop(); @@ -863,7 +862,7 @@ impl Exporter<'_> { if !it.children().is_empty() { let pushes = Self::push_group(s, it.opacity, it.blend) + Self::push_isolation(s, it); for c in it.shown_children() { - self.item(s, c, xf, page_name); + self.item(s, c, xf, page); } for _ in 0..pushes { s.pop(); @@ -921,7 +920,7 @@ impl Exporter<'_> { } } Content::Text(tfr) => { - let cs = self.cache.get(doc, tfr.story, page_name); + let cs = self.cache.get(doc, tfr.story, page); if let (Some(ft), Some(pt)) = (cs.frame(it.id), &tfr.options.path) { // Type on a path: the glyphs as outlines along the path. for (si, gp) in designcraft_compose::path_glyphs(&cs, ft, &bp, pt) { @@ -966,11 +965,11 @@ impl Exporter<'_> { if let Content::Text(tfr) = &it.content && let Some(st) = doc.story(tfr.story).filter(|st| !st.objects.is_empty()) { - let cs = self.cache.get(doc, tfr.story, page_name); + let cs = self.cache.get(doc, tfr.story, page); if let Some(ft) = cs.frame(it.id) { for o in &ft.objects { if let Some(obj) = st.objects.get(o.index) { - self.item(s, &obj.item, xf * Affine::translate(o.origin.to_vec2()), page_name); + self.item(s, &obj.item, xf * Affine::translate(o.origin.to_vec2()), page); } } } diff --git a/crates/render/src/fx.rs b/crates/render/src/fx.rs index 77bb18780..6b29516f4 100644 --- a/crates/render/src/fx.rs +++ b/crates/render/src/fx.rs @@ -66,7 +66,7 @@ fn shadow(sigma: f64, color: peniko::Color) -> Filter { impl Renderer { /// Draw an item with soft effects: below-effects, the (feathered) body, inner shadow. - pub(crate) fn draw_item_fx(&mut self, ctx: &mut RenderContext, f: &Frame, it: &Item, bp: &BezPath, xf: Affine, page_name: Option<&str>) { + pub(crate) fn draw_item_fx(&mut self, ctx: &mut RenderContext, f: &Frame, it: &Item, bp: &BezPath, xf: Affine, page: Option) { let doc = f.doc; let e = &it.effects; let sw = if it.stroke.is_none() { 0.0 } else { it.stroke.extent() }; @@ -95,7 +95,7 @@ impl Renderer { let moved = Frame { view: fr.view * Affine::translate(off), ..*fr }; c.set_transform(moved.view); c.push_layer(None, None, None, None, Some(filter)); - me.draw_body(c, &moved, it, bp, xf, page_name); + me.draw_body(c, &moved, it, bp, xf, page); if grow > 0.0 && solid { c.set_transform(moved.view * xf); c.set_paint(peniko::Color::BLACK); @@ -119,7 +119,7 @@ impl Renderer { self.with_filters(ctx, f, reach, w / 4.0, |me, c, fr| { c.set_transform(fr.view * xf); c.push_clip_layer(bp); - me.draw_body(c, fr, it, bp, xf, page_name); + me.draw_body(c, fr, it, bp, xf, page); // Keep the body where the blurred inset silhouette is. c.set_transform(Affine::IDENTITY); c.push_layer(None, Some(BlendMode::new(Mix::Normal, Compose::DestIn)), None, None, None); @@ -139,7 +139,7 @@ impl Renderer { c.pop_layer(); }); } else { - self.draw_body(ctx, f, it, bp, xf, page_name); + self.draw_body(ctx, f, it, bp, xf, page); } // Directional feather: one fading ramp per side, multiplied in. let df = &e.directional_feather; diff --git a/crates/render/src/lib.rs b/crates/render/src/lib.rs index b0f8df133..2cc5c548f 100644 --- a/crates/render/src/lib.rs +++ b/crates/render/src/lib.rs @@ -216,7 +216,7 @@ pub struct Renderer { resources: Resources, glyphs: text::GlyphCache, /// Composed stories looked up during the current render. - stories: HashMap<(StoryId, Option), Arc>, + stories: HashMap<(StoryId, Option), Arc>, pub threads: u16, pub stats: FrameStats, } @@ -408,7 +408,6 @@ impl Renderer { let Some((ppi, ppage)) = doc.parent_page_for(first + pi) else { continue }; let parent = &doc.parents[ppi]; let dx = page.x - parent.pages[ppage].x; - let page_name = doc.page_name(first + pi); let pr = parent.pages[ppage].bounds(); for it in &parent.items { if it.layer != layer.id || page.overridden.contains(&it.id) { @@ -419,7 +418,7 @@ impl Renderer { continue; } let _ = pr; - self.draw_item(ctx, f, it, Affine::translate((dx, 0.0)), Some(&page_name)); + self.draw_item(ctx, f, it, Affine::translate((dx, 0.0)), Some(first + pi)); } } } @@ -431,7 +430,7 @@ impl Renderer { } } - fn draw_item(&mut self, ctx: &mut RenderContext, f: &Frame, it: &Item, parent: Affine, page_name: Option<&str>) { + fn draw_item(&mut self, ctx: &mut RenderContext, f: &Frame, it: &Item, parent: Affine, page: Option) { if it.hidden || f.opts.hidden.contains(&it.id) || f.opts.printing_only && it.nonprinting { return; } @@ -458,7 +457,7 @@ impl Renderer { ctx.set_transform(f.view * xf); ctx.push_clip_layer(&bp); for c in it.children() { - self.draw_item(ctx, f, c, xf, page_name); + self.draw_item(ctx, f, c, xf, page); } ctx.pop_layer(); if !it.stroke.is_none() { @@ -489,11 +488,11 @@ impl Renderer { ctx.fill_path(&c.path.to_bezpath()); ctx.pop_layer(); } - self.draw_item(ctx, f, c, xf, page_name); + self.draw_item(ctx, f, c, xf, page); } } else { for c in it.shown_children() { - self.draw_item(ctx, f, c, xf, page_name); + self.draw_item(ctx, f, c, xf, page); } } if layered { @@ -520,9 +519,9 @@ impl Renderer { ctx.push_layer(None, Some(blend_mode(it.blend)), Some(it.opacity), None, None); } if effects { - self.draw_item_fx(ctx, f, it, &bp, xf, page_name); + self.draw_item_fx(ctx, f, it, &bp, xf, page); } else { - self.draw_body(ctx, f, it, &bp, xf, page_name); + self.draw_body(ctx, f, it, &bp, xf, page); } if layered { ctx.pop_layer(); @@ -531,18 +530,18 @@ impl Renderer { /// The composed story for a frame (memoised for the current render: the cache lookup /// validates the whole thread each time). - fn story(&mut self, f: &Frame, sid: StoryId, page_name: Option<&str>) -> Arc { - let key = (sid, page_name.map(str::to_string)); + fn story(&mut self, f: &Frame, sid: StoryId, page: Option) -> Arc { + let key = (sid, page); if let Some(cs) = self.stories.get(&key) { return cs.clone(); } - let cs = f.cache.get(f.doc, sid, page_name); + let cs = f.cache.get(f.doc, sid, page); self.stories.insert(key, cs.clone()); cs } /// Fill, content and stroke of a leaf item (no opacity, blend or effects). - pub(crate) fn draw_body(&mut self, ctx: &mut RenderContext, f: &Frame, it: &Item, bp: &BezPath, xf: Affine, page_name: Option<&str>) { + pub(crate) fn draw_body(&mut self, ctx: &mut RenderContext, f: &Frame, it: &Item, bp: &BezPath, xf: Affine, page: Option) { let doc = f.doc; // Fill. if !it.fill.is_none() { @@ -575,7 +574,7 @@ impl Renderer { ctx.pop_layer(); } Content::Text(tf) => { - let cs = self.story(f, tf.story, page_name); + let cs = self.story(f, tf.story, page); if let Some(ft) = cs.frame(it.id) { match &tf.options.path { Some(pt) => self.draw_path_text(ctx, f, &cs, ft, xf, bp, pt), From fae9bc84ce244a3dc50e169163507f4915756eab Mon Sep 17 00:00:00 2001 From: KnottyDyes <47539659+KnottyDyes@users.noreply.github.com> Date: Sun, 11 Oct 2026 22:30:21 -0500 Subject: [PATCH 3/5] fix(text): reject unknown character fill swatches Reject unknown character fill swatches before modifying text or undo history. Registered swatch names remain valid, including names that resemble hex colours. Validation: invalid-name atomicity and save/reopen regressions, plus all six cargo xtask ci gates on main after #514 and #523 (1,186 native tests passed, 3 ignored; 15 crates passed WASM compilation checks). Task: #403 --- crates/engine/src/cmd/find.rs | 1 + crates/engine/src/cmd/style.rs | 152 +++++++++++++++++++++++++++++++++ crates/engine/src/cmd/text.rs | 13 +++ 3 files changed, 166 insertions(+) diff --git a/crates/engine/src/cmd/find.rs b/crates/engine/src/cmd/find.rs index 878160400..5dc0bc8ad 100644 --- a/crates/engine/src/cmd/find.rs +++ b/crates/engine/src/cmd/find.rs @@ -181,6 +181,7 @@ fn change(s: &mut Session, p: &Value) -> Result { over.set_json(k, v).map_err(|e| bad("find.change", e))?; } } + super::text::validate_char_fill(&s.doc()?.doc, &over, "find.change")?; let targets = scope(s, p)?; s.edit(|d, sel| { let mut count = 0usize; diff --git a/crates/engine/src/cmd/style.rs b/crates/engine/src/cmd/style.rs index 296a8d07b..8a520b685 100644 --- a/crates/engine/src/cmd/style.rs +++ b/crates/engine/src/cmd/style.rs @@ -444,6 +444,7 @@ fn create_para(s: &mut Session, p: &Value) -> Result { chars = full; } } + super::text::validate_char_fill(&s.doc()?.doc, &chars, "style.paragraph.create")?; let based_on = str_param(p, "basedOn").map(str::to_string); let next = str_param(p, "nextStyle").map(str::to_string); s.edit(|d, _| { @@ -470,6 +471,7 @@ fn CharProps_to_attrs(p: &designcraft_doc::CharProps) -> CharAttrs { fn edit_para(s: &mut Session, p: &Value) -> Result { let name = str_param(p, "name").ok_or_else(|| bad("style.paragraph.edit", "missing name"))?.to_string(); let chars: CharAttrs = attrs(p.get("chars"), |a: &mut CharAttrs, k, v| a.set_json(k, v))?; + super::text::validate_char_fill(&s.doc()?.doc, &chars, "style.paragraph.edit")?; let rename = str_param(p, "rename").map(str::to_string); let based = p.get("basedOn").cloned(); s.edit(|d, _| { @@ -621,6 +623,7 @@ fn delete_para(s: &mut Session, p: &Value) -> Result { fn create_char(s: &mut Session, p: &Value) -> Result { let base = str_param(p, "name").unwrap_or("Character Style 1").to_string(); let chars: CharAttrs = attrs(p.get("chars"), |a: &mut CharAttrs, k, v| a.set_json(k, v))?; + super::text::validate_char_fill(&s.doc()?.doc, &chars, "style.character.create")?; // Based on [None] is based on nothing. let based_on = str_param(p, "basedOn").filter(|b| *b != designcraft_doc::NO_CHAR_STYLE).map(str::to_string); s.edit(|d, _| { @@ -641,6 +644,7 @@ fn edit_char(s: &mut Session, p: &Value) -> Result { return Err(bad("style.character.edit", "[None] can't be edited")); } let chars: CharAttrs = attrs(p.get("chars"), |a: &mut CharAttrs, k, v| a.set_json(k, v))?; + super::text::validate_char_fill(&s.doc()?.doc, &chars, "style.character.edit")?; // `null` unsets an attribute (the style no longer sets it). let unset: Vec = p .get("chars") @@ -1045,6 +1049,154 @@ fn rename_in(it: &mut designcraft_doc::Item, from: &str, to: &str) { mod color_tests { use super::*; + fn fill_session() -> Session { + let mut session = Session::new(); + session.execute("file.new", &json!({})).unwrap(); + let frame = session.execute("frame.create", &json!({"rect": [0, 0, 200, 100], "content": "text", "text": "Hi"})).unwrap(); + session.execute("text.select", &json!({"story": frame["story"], "anchor": 0, "focus": 2})).unwrap(); + session.execute("style.paragraph.create", &json!({"name": "Existing"})).unwrap(); + session.execute("style.character.create", &json!({"name": "Existing"})).unwrap(); + session.execute("type.char", &json!({"size": 18})).unwrap(); + session.execute("edit.undo", &json!({})).unwrap(); + session + } + + fn assert_fill_rejected(session: &mut Session, command: &str, params: Value) { + let before = session.doc().unwrap().clone(); + let journal = session.journal.clone(); + let fill = params + .pointer("/chars/fill") + .or_else(|| params.pointer("/attrs/fill")) + .or_else(|| params.get("fill")) + .and_then(Value::as_str) + .unwrap_or("Missing Ink"); + + let result = session.execute(command, ¶ms); + + match result { + Err(crate::EngineError::BadParams { cmd, msg }) => { + assert_eq!(cmd, command); + assert!(msg.contains(fill), "{msg}"); + assert!(msg.contains("swatch.create") && msg.contains("returned name"), "{msg}"); + } + other => panic!("expected BadParams for {command}, got {other:?}"), + } + let after = session.doc().unwrap(); + assert!(std::sync::Arc::ptr_eq(&before.doc, &after.doc)); + assert_eq!(before.selection, after.selection); + assert_eq!(before.revision, after.revision); + assert_eq!(format!("{:?}", before.history), format!("{:?}", after.history)); + assert_eq!(journal, session.journal); + } + + #[test] + fn char_fill_rejects_unknown_flat_and_nested_names_when_formatting() { + for fill in ["Missing Ink", "#ff0000", "[black]", " [Black]"] { + for params in [json!({"fill": fill, "size": 24}), json!({"attrs": {"fill": fill, "size": 24}})] { + let mut session = fill_session(); + assert_fill_rejected(&mut session, "type.char", params); + } + } + } + + #[test] + fn char_fill_rejects_unknown_names_when_creating_or_editing_styles() { + for command in ["style.paragraph.create", "style.paragraph.edit", "style.character.create", "style.character.edit"] { + for fill in ["Missing Ink", "#ff0000"] { + let mut session = fill_session(); + session.execute("style.paragraph.apply", &json!({"name": "Existing"})).unwrap(); + session.execute("style.character.apply", &json!({"name": "Existing"})).unwrap(); + assert_fill_rejected(&mut session, command, json!({"name": "Existing", "rename": "Renamed", "chars": {"fill": fill, "size": 24}})); + } + } + } + + #[test] + fn char_fill_rejects_inherited_unknown_name_when_creating_from_selection() { + let mut session = fill_session(); + session + .edit(|document, selection| { + let selected = selection.text.unwrap(); + document.story_mut(selected.story).unwrap().format_chars(selected.range(), |format| format.over.fill = Some("Missing Ink".into())); + Ok(()) + }) + .unwrap(); + + assert_fill_rejected(&mut session, "style.paragraph.create", json!({"name": "Inherited", "fromSelection": true})); + } + + #[test] + fn char_fill_accepts_valid_override_when_selection_has_unknown_name() { + let mut session = fill_session(); + session + .edit(|document, selection| { + let selected = selection.text.unwrap(); + document.story_mut(selected.story).unwrap().format_chars(selected.range(), |format| format.over.fill = Some("Missing Ink".into())); + Ok(()) + }) + .unwrap(); + + session.execute("style.paragraph.create", &json!({"name": "Override", "fromSelection": true, "chars": {"fill": "[Paper]"}})).unwrap(); + + assert_eq!(session.doc().unwrap().doc.styles.para("Override").unwrap().chars.fill.as_deref(), Some("[Paper]")); + } + + #[test] + fn char_fill_rejects_unknown_name_when_changing_text_even_without_output() { + for (find, change) in [("Hi", "Bye"), ("Hi", ""), ("Absent", "Bye")] { + for fill in ["Missing Ink", "#ff0000"] { + let mut session = fill_session(); + assert_fill_rejected(&mut session, "find.change", json!({"find": find, "change": change, "attrs": {"fill": fill}})); + } + } + } + + #[test] + fn char_fill_preserves_exact_registered_names_when_used_at_all_boundaries() { + for fill in ["Brand Ink", "#ff0000", "[None]", "[Black]"] { + let mut session = fill_session(); + if !fill.starts_with('[') { + let created = session.execute("swatch.create", &json!({"name": fill, "color": "#108080"})).unwrap(); + assert_eq!(created["name"], fill); + } + for params in [json!({"fill": fill}), json!({"attrs": {"fill": fill}})] { + session.execute("type.char", ¶ms).unwrap(); + assert_eq!(session.execute("type.selectionAttrs", &json!({})).unwrap()["chars"]["fill"], fill); + } + for kind in ["paragraph", "character"] { + session.execute(&format!("style.{kind}.create"), &json!({"name": "Colored", "chars": {"fill": fill}})).unwrap(); + session.execute(&format!("style.{kind}.edit"), &json!({"name": "Existing", "chars": {"fill": fill}})).unwrap(); + let document = &session.doc().unwrap().doc; + for name in ["Colored", "Existing"] { + let attrs = if kind == "paragraph" { + &document.styles.para(name).unwrap().chars + } else { + &document.styles.char_style(name).unwrap().chars + }; + assert_eq!(attrs.fill.as_deref(), Some(fill)); + } + } + assert_eq!(session.execute("find.change", &json!({"find": "Hi", "change": "Bye", "attrs": {"fill": fill}})).unwrap()["count"], 1); + let selected = session.doc().unwrap().selection.text.unwrap(); + let story = session.doc().unwrap().doc.story(selected.story).unwrap(); + assert_eq!(story.text, "Bye"); + assert_eq!(story.format_after(0).over.fill.as_deref(), Some(fill)); + } + } + + #[test] + fn char_fill_accepts_omitted_and_null_attributes_when_used_at_all_boundaries() { + for chars in [json!({"size": 18}), json!({"fill": null})] { + let mut session = fill_session(); + session.execute("type.char", &json!({"attrs": chars})).unwrap(); + for kind in ["paragraph", "character"] { + session.execute(&format!("style.{kind}.create"), &json!({"name": "Nullable", "chars": chars})).unwrap(); + session.execute(&format!("style.{kind}.edit"), &json!({"name": "Existing", "chars": chars})).unwrap(); + } + assert_eq!(session.execute("find.change", &json!({"find": "Hi", "change": "Bye", "attrs": chars})).unwrap()["count"], 1); + } + } + #[test] fn percent_components_at_most_one_stay_percent() { use designcraft_color::Color; diff --git a/crates/engine/src/cmd/text.rs b/crates/engine/src/cmd/text.rs index 4950d82b9..71d390d02 100644 --- a/crates/engine/src/cmd/text.rs +++ b/crates/engine/src/cmd/text.rs @@ -819,6 +819,18 @@ fn warichu_cmd(s: &mut Session, p: &Value) -> Result { format_chars(s, &Value::Object(body)) } +pub(crate) fn validate_char_fill(document: &designcraft_doc::Document, attrs: &CharAttrs, command_id: &str) -> Result<()> { + if let Some(fill) = &attrs.fill + && document.swatch(fill).is_none() + { + return Err(bad( + command_id, + format!("unknown character fill `{fill}`; create a swatch with `swatch.create` and use its returned name as `fill`"), + )); + } + Ok(()) +} + pub(crate) fn format_chars(s: &mut Session, attrs: &Value) -> Result { let mut a = CharAttrs::default(); if let Some(o) = attrs.as_object() { @@ -826,6 +838,7 @@ pub(crate) fn format_chars(s: &mut Session, attrs: &Value) -> Result { a.set_json(k, v).map_err(|e| bad("type.char", e))?; } } + validate_char_fill(&s.doc()?.doc, &a, "type.char")?; let cleared: Vec = attrs.as_object().map(|o| o.iter().filter(|(_, v)| v.is_null()).map(|(k, _)| k.clone()).collect()).unwrap_or_default(); let targets = format_targets(s); s.edit(|d, _| { From 5789caaa6042b2e1b0743553fba0a84e974faf20 Mon Sep 17 00:00:00 2001 From: moppeeer Date: Mon, 12 Oct 2026 05:54:43 +0200 Subject: [PATCH 4/5] fix(objects): reject unknown fill and stroke swatches Reject unknown object fill, stroke and gap swatch names before editing the document, and suggest an existing name where possible. Validation: three independent atomicity and compatibility probes, plus all six cargo xtask ci gates against current main (1,187 native tests passed, 3 ignored; all 15 applicable crates passed WASM compilation checks). ROADMAP progress entries are retained. Task: #520 --- 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 ec8eb0920..f100d45be 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -130,6 +130,7 @@ list) · [roadmap.md](docs/roadmap.md) (milestones, plan) · [parity-checklist.m Newest first. - **2026-10-12** · Words in capitals hyphenate like capitalized words (FERRUM COM-MODI), as in InDesign; Hyphenate Capitalized Words off keeps them whole. No readiness percentage change. +- **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-11 (printing):** `file.printers` and `file.print` use craft-libs/craft-print for CUPS and Windows. DesignCraft retains PDF export, page selection, spreads, marks and bleed; dry runs create no temporary PDF or print job. Windows selects matching or default driver paper and shrinks oversized sheets without enlarging smaller ones; queue duplex defaults are preserved. Headless tests exercise exported bytes, job settings, dry runs and refusals; shared native Windows CI passes, including paused-queue submission and cleanup. PostScript, production print controls and physical output validation remain open. 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. diff --git a/crates/engine/src/cmd/object.rs b/crates/engine/src/cmd/object.rs index ca405a181..5570ee93a 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, @@ -2163,7 +2180,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(); @@ -2726,3 +2743,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(); + } + } +} From 835edae6594f10892142eda9c64141471a009d4c Mon Sep 17 00:00:00 2001 From: moppeeer Date: Mon, 12 Oct 2026 05:58:49 +0200 Subject: [PATCH 5/5] feat(commands): return IDs from move, duplicate and repeat Return ordered item IDs from Move, Duplicate and Step and Repeat while preserving the existing moved and created counts, selection and undo behavior. Scripts can use the IDs to operate on copies. Validation: eight independent probes covering both swatch validation and returned IDs, plus all six cargo xtask ci gates on main after #520 (1,188 native tests passed, 3 ignored; all 15 applicable crates passed WASM compilation checks). Both ROADMAP entries and command regression modules are retained. Task: #521 --- ROADMAP.md | 1 + crates/engine/src/cmd/object.rs | 46 ++++++++++++++++++++++++++++++--- 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index f100d45be..d33353caa 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -130,6 +130,7 @@ list) · [roadmap.md](docs/roadmap.md) (milestones, plan) · [parity-checklist.m Newest first. - **2026-10-12** · Words in capitals hyphenate like capitalized words (FERRUM COM-MODI), as in InDesign; Hyphenate Capitalized Words off keeps them whole. No readiness percentage change. +- **2026-10-11** · `transform.move {copy}`, Edit › Duplicate and Step and Repeat return the copies' ids, so a script can go on with them. No readiness percentage change. - **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-11 (printing):** `file.printers` and `file.print` use craft-libs/craft-print for CUPS and Windows. DesignCraft retains PDF export, page selection, spreads, marks and bleed; dry runs create no temporary PDF or print job. Windows selects matching or default driver paper and shrinks oversized sheets without enlarging smaller ones; queue duplex defaults are preserved. Headless tests exercise exported bytes, job settings, dry runs and refusals; shared native Windows CI passes, including paused-queue submission and cleanup. PostScript, production print controls and physical output validation remain open. No readiness percentage change. diff --git a/crates/engine/src/cmd/object.rs b/crates/engine/src/cmd/object.rs index 5570ee93a..cd13be117 100644 --- a/crates/engine/src/cmd/object.rs +++ b/crates/engine/src/cmd/object.rs @@ -37,7 +37,15 @@ pub fn specs() -> Vec { frame_create ), cmd!("line.create", "Create Line", [], None, "{spread?, a: [x,y], b: [x,y]}", has_doc, line_create), - cmd!("transform.move", "Move", ["Object", "Transform"], None, "{dx, dy, copy?: bool, ids?, toSpread?}", has_selection, transform_move), + cmd!( + "transform.move", + "Move", + ["Object", "Transform"], + None, + "{dx, dy, copy?: bool, ids?, toSpread?} → {moved, ids: the objects moved (with copy, the copies)}", + has_selection, + transform_move + ), cmd!( "transform.resize", "Resize", @@ -582,7 +590,7 @@ pub fn specs() -> Vec { "Step and Repeat…", ["Edit"], Some("Cmd+Alt+U"), - "{count?: 1..1000, dx?, dy?, rows?, columns?, ids?} — copies of the selection (or ids) offset by (dx, dy); with rows/columns, a grid of at most 1000 cells, and at most 1000 item copies including group contents", + "{count?: 1..1000, dx?, dy?, rows?, columns?, ids?} — copies of the selection (or ids) offset by (dx, dy); with rows/columns, a grid of at most 1000 cells, and at most 1000 item copies including group contents → {created, ids: the copies}", has_selection, step_and_repeat ), @@ -998,7 +1006,7 @@ fn transform_move(s: &mut Session, p: &Value) -> Result { } it.xf = Affine::translate((dx, dy)) * it.xf; } - Ok(json!({"moved": ids.len()})) + Ok(json!({"moved": ids.len(), "ids": ids.iter().map(|i| i.0).collect::>()})) }) } @@ -2108,7 +2116,8 @@ fn step_and_repeat(s: &mut Session, p: &Value) -> Result { } } *sel = Selection::items(all.clone()); - Ok(json!({"created": all.len() - ids.len()})) + let copies: Vec = all.iter().skip(ids.len()).map(|i| i.0).collect(); + Ok(json!({"created": copies.len(), "ids": copies})) }) } @@ -2744,6 +2753,35 @@ mod blend_mode_tests { } } +#[cfg(test)] +mod copy_ids_tests { + use serde_json::json; + + #[test] + fn copies_report_their_ids() { + let mut s = crate::Session::new(); + s.execute("file.new", &json!({})).unwrap(); + let a = s.execute("frame.create", &json!({"rect": [0, 0, 50, 50]})).unwrap()["id"].as_u64().unwrap(); + let ids = |r: &serde_json::Value| r["ids"].as_array().unwrap().iter().map(|v| v.as_u64().unwrap()).collect::>(); + let selected = |s: &crate::Session| s.doc().unwrap().selection.items.iter().map(|i| i.0).collect::>(); + // Moving reports what moved; copying reports the copies, which are selected. + assert_eq!(ids(&s.execute("transform.move", &json!({"dx": 5, "dy": 0, "ids": [a]})).unwrap()), [a]); + let r = s.execute("transform.move", &json!({"dx": 100, "dy": 0, "copy": true, "ids": [a]})).unwrap(); + let copy = ids(&r); + assert!(copy.len() == 1 && copy[0] != a && s.doc().unwrap().doc.item(designcraft_doc::ItemId(copy[0])).is_some(), "{r}"); + assert_eq!(selected(&s), copy); + // Edit › Duplicate. + let r = s.execute("edit.duplicate", &json!({})).unwrap(); + let dup = ids(&r); + assert!(dup.len() == 1 && dup[0] != copy[0], "{r}"); + assert_eq!(selected(&s), dup); + // Step and Repeat: every copy. + let r = s.execute("edit.stepAndRepeat", &json!({"count": 3, "dx": 0, "dy": 60, "ids": [a]})).unwrap(); + assert_eq!((r["created"].as_u64(), ids(&r).len()), (Some(3), 3), "{r}"); + assert!(ids(&r).iter().all(|i| s.doc().unwrap().doc.item(designcraft_doc::ItemId(*i)).is_some())); + } +} + #[cfg(test)] mod swatch_name_tests { use serde_json::json;