From e362820bc45f497fb4188bd98e44cf5b37fde8fc Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 11 Nov 2026 17:42:24 -0800 Subject: [PATCH 1/5] Data merge: an image field in text places the image, not its path An image field inserted at a text insertion point, or typed there as <>, merged as text, so each record showed the image's path. It now merges as an inline image at its natural size, scaled down to fit the smallest text column of the story's frames. A missing file is listed in missingImages and its marker is removed. An explicit image role at a text insertion point places an inline image too. An image path that starts with one separator (\Photos\a.jpg) is tried from the drive's root first, then beside the data file and the document. --- crates/engine/src/cmd/datamerge.rs | 18 ++- crates/engine/src/cmd/datamerge/fill.rs | 190 ++++++++++++++++++----- crates/engine/src/cmd/datamerge/parse.rs | 7 +- crates/engine/src/cmd/datamerge/tests.rs | 151 ++++++++++++++++++ docs/agents.md | 5 +- 5 files changed, 324 insertions(+), 47 deletions(-) diff --git a/crates/engine/src/cmd/datamerge.rs b/crates/engine/src/cmd/datamerge.rs index 7cb332fd3..6fac73836 100644 --- a/crates/engine/src/cmd/datamerge.rs +++ b/crates/engine/src/cmd/datamerge.rs @@ -58,7 +58,7 @@ pub fn specs() -> Vec { "Insert Field", [], None, - "{field, role?: text|image|qr|hyperlink, story?, at?, end?, item?} → {id}", + "{field, role?: text|image|qr|hyperlink, story?, at?, end?, item?} → {id}; an image field at a text insertion point merges as an inline image", has_doc, placeholder_add ), @@ -484,8 +484,11 @@ fn add_placeholder(d: &mut Document, sel: &Selection, p: &Value, name: &str) -> true } else if explicit_story || role_param == Some("text") { false - } else if matches!(role_param, Some("image" | "qr")) { + } else if role_param == Some("qr") { true + } else if role_param == Some("image") { + // An image role with the insertion point in text places an inline image there. + sel.text.is_none() } else { sel.text.is_none() && !sel.items.is_empty() }; @@ -497,6 +500,9 @@ fn add_placeholder(d: &mut Document, sel: &Selection, p: &Value, name: &str) -> DataFieldKind::Qr => PlaceholderRole::Qr, DataFieldKind::Text => return Err(bad("data.placeholder.add", "say which role to use on this frame")), } + } else if field.kind == DataFieldKind::Image { + // An image field at the insertion point places the image inline in the text. + PlaceholderRole::Image } else { PlaceholderRole::Text }; @@ -515,14 +521,14 @@ fn add_placeholder(d: &mut Document, sel: &Selection, p: &Value, name: &str) -> } PlaceholderAnchor::Item { id: item } } else { - if matches!(role, PlaceholderRole::Image | PlaceholderRole::Qr) { - return Err(bad("data.placeholder.add", "image and qr placeholders bind to a frame")); + if role == PlaceholderRole::Qr { + return Err(bad("data.placeholder.add", "qr placeholders bind to a frame")); } let (story, at, end) = text_target(p, sel).ok_or_else(|| bad("data.placeholder.add", "select text or a frame"))?; if d.story(story).is_none() { return Err(bad("data.placeholder.add", "no story")); } - if role == PlaceholderRole::Text { + if matches!(role, PlaceholderRole::Text | PlaceholderRole::Image) { let marker = format!("<<{name}>>"); let at = { let text = d.story(story).map(|st| st.text.clone()).unwrap_or_default(); @@ -574,7 +580,7 @@ pub(crate) fn sync_placeholders(doc: &mut Document) { } PlaceholderAnchor::Text { story, start, end } => { let text = doc.story(story).map(|st| st.text.clone()).unwrap_or_default(); - if ph.role != PlaceholderRole::Text { + if !matches!(ph.role, PlaceholderRole::Text | PlaceholderRole::Image) { if start <= end && end <= text.len() { keep.push(ph); } diff --git a/crates/engine/src/cmd/datamerge/fill.rs b/crates/engine/src/cmd/datamerge/fill.rs index a7b4a0ba6..caea4d67e 100644 --- a/crates/engine/src/cmd/datamerge/fill.rs +++ b/crates/engine/src/cmd/datamerge/fill.rs @@ -8,10 +8,10 @@ use std::path::{Path, PathBuf}; use std::sync::Arc; use designcraft_doc::{ - Asset, AssetId, Content, DataField, Fill, Fitting, Graphic, Hyperlink, HyperlinkDest, HyperlinkSource, Item, ItemId, MergeOptions, - PlaceholderAnchor, PlaceholderRole, Shape, SpreadRef, Story, StoryId, + AnchorPosition, AnchoredObject, Asset, AssetId, Content, DataField, DataFieldKind, Fill, Fitting, Graphic, Hyperlink, HyperlinkDest, + HyperlinkSource, Item, ItemId, MergeOptions, OBJECT_MARK, PlaceholderAnchor, PlaceholderRole, Shape, SpreadRef, Story, StoryId, }; -use designcraft_geom::{Affine, Rect, Vec2}; +use designcraft_geom::{Affine, Rect, Vec2, shapes}; use super::parse::image_candidates; use crate::EngineError; @@ -147,6 +147,16 @@ struct Rep { start: usize, end: usize, text: String, + /// An inline image that takes the range's place; `text` is then its object mark. + object: Option, +} + +/// An image field at a text range: the placed image goes into the text in the marker's place. +struct InlineImage { + story: StoryId, + start: usize, + end: usize, + field: String, } /// Where `fill_row` looks for linked images: the merge options and the two search folders. @@ -199,6 +209,7 @@ impl ImageCache { #[derive(Default)] struct Pending { reps: Vec, + images: Vec, links: Vec<(PlaceholderAnchor, HyperlinkDest, String)>, parent: bool, report: FillReport, @@ -225,8 +236,14 @@ pub fn fill_row( PlaceholderAnchor::Item { .. } => plan_item(doc, maps, ph, &cell, record, sources, &mut pending), } } - let Pending { mut reps, links, parent, mut report } = pending; - typed_reps(doc, fields, row, maps, &placeholders, &mut reps); + let Pending { mut reps, mut images, links, parent, mut report } = pending; + typed_reps(doc, fields, row, maps, &placeholders, &mut reps, &mut images); + for InlineImage { story, start, end, field } in images { + let cell = cell_of(fields, row, &field); + let object = inline_image(doc, story, &field, cell, record, sources, &mut report); + let text = if object.is_some() { OBJECT_MARK.to_string() } else { String::new() }; + reps.push(Rep { story, start, end, text, object }); + } apply_reps(doc, &reps); for (anchor, dest, field) in links { let anchor = match anchor { @@ -261,13 +278,14 @@ fn plan_text( return; }; match ph.role { - PlaceholderRole::Text => pending.reps.push(Rep { story: mapped, start, end, text: cell }), + PlaceholderRole::Text => pending.reps.push(Rep { story: mapped, start, end, text: cell, object: None }), + PlaceholderRole::Image => pending.images.push(InlineImage { story: mapped, start, end, field: ph.field.clone() }), PlaceholderRole::Hyperlink => { if let Some(dest) = classify_link(&cell) { pending.links.push((PlaceholderAnchor::Text { story: mapped, start, end }, dest, ph.field.clone())); } } - PlaceholderRole::Image | PlaceholderRole::Qr => {} + PlaceholderRole::Qr => {} } } @@ -306,6 +324,7 @@ fn typed_reps( maps: &Maps, placeholders: &[designcraft_doc::Placeholder], reps: &mut Vec, + images: &mut Vec, ) { let mut stories: Vec<(StoryId, StoryId)> = maps.stories.iter().map(|(a, b)| (*a, *b)).collect(); stories.sort_by_key(|(a, _)| a.0); @@ -315,7 +334,9 @@ fn typed_reps( let covers: Vec<(usize, usize)> = placeholders .iter() .filter_map(|ph| match (&ph.role, &ph.anchor) { - (PlaceholderRole::Text, PlaceholderAnchor::Text { story, start, end }) if *story == original => Some((*start, *end)), + (PlaceholderRole::Text | PlaceholderRole::Image, PlaceholderAnchor::Text { story, start, end }) if *story == original => { + Some((*start, *end)) + } _ => None, }) .collect(); @@ -323,10 +344,12 @@ fn typed_reps( if covers.iter().any(|(a, b)| start < *b && end > *a) { continue; } - if !fields.iter().any(|f| f.name == name) { + let Some(field) = fields.iter().find(|f| f.name == name) else { continue }; + if field.kind == DataFieldKind::Image { + images.push(InlineImage { story: mapped, start, end, field: name }); continue; } - reps.push(Rep { story: mapped, start, end, text: cell_of(fields, row, &name).to_string() }); + reps.push(Rep { story: mapped, start, end, text: cell_of(fields, row, &name).to_string(), object: None }); } } } @@ -367,6 +390,15 @@ fn apply_reps(doc: &mut designcraft_doc::Document, reps: &[Rep]) { for r in mine { if r.start <= r.end && r.end <= st.text.len() { st.replace(r.start..r.end, &r.text); + // The replacement made a blank object for its mark; the image takes that slot. + if let Some(obj) = &r.object + && st.text.get(r.start..).is_some_and(|t| t.starts_with(OBJECT_MARK)) + { + let k = st.text[..r.start].matches(OBJECT_MARK).count(); + if let Some(slot) = st.objects.get_mut(k) { + *slot = Arc::new(obj.clone()); + } + } } } } @@ -420,10 +452,19 @@ fn create_link(doc: &mut designcraft_doc::Document, anchor: PlaceholderAnchor, d true } -fn place_image(doc: &mut designcraft_doc::Document, item: ItemId, field: &str, cell: &str, record: u32, sources: &Sources, report: &mut FillReport) { - let Sources { options, data_dir, doc_dir, images } = *sources; +/// An image file named by a cell: where it was found, its bytes, its pixel size and its natural size. +struct LoadedImage { + path: PathBuf, + bytes: Arc>, + pixels: (u32, u32), + natural: (f64, f64), +} + +/// The first candidate for `cell` that reads and decodes; `None` when none does. +fn load_image(cell: &str, sources: &Sources) -> Option { + let Sources { data_dir, doc_dir, images, .. } = *sources; let cands = image_candidates(cell, data_dir, doc_dir); - let found = cands.iter().find_map(|p| { + let (path, bytes) = cands.iter().find_map(|p| { // Read once per merge: many records naming one file is the normal case, and reading it // per record is the amplification (#1595). let bytes = images.get_or_read(p, || { @@ -435,23 +476,98 @@ fn place_image(doc: &mut designcraft_doc::Document, item: ItemId, field: &str, c crate::io::read_file(p).ok().filter(|b| !b.is_empty()) })?; Some((p.clone(), bytes)) + })?; + let (pixels, natural) = designcraft_render::image_size(&bytes).zip(designcraft_render::natural_size(&bytes, 0))?; + Some(LoadedImage { path, bytes, pixels, natural }) +} + +fn report_missing(report: &mut FillReport, record: u32, field: &str, cell: &str) { + report.missing.push(MissingImage { record, field: field.to_string(), path: cell.to_string() }); + report.warnings.push(format!("Record {record}, field {field}: missing image {cell}.")); +} + +/// Add `image` to the document as a new asset, linked to its file when the options say so. +fn add_image_asset(doc: &mut designcraft_doc::Document, image: LoadedImage, field: &str, options: &MergeOptions) -> AssetId { + let fresh = AssetId(doc.alloc()); + let name = image.path.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_else(|| field.to_string()); + let link = options.link_images.then(|| image.path.to_string_lossy().into_owned()); + doc.add_asset(Asset { + id: fresh, + name, + mime: designcraft_render::image_mime(&image.bytes).to_string(), + link, + // The cached buffer is shared with every other record naming this file, rather than + // copied per record (#1595). + data: image.bytes, + pixels: Some(image.pixels), + ..Default::default() + }) +} + +/// The image `cell` names, as an inline object for the text of `story`. It comes in at its +/// natural size, scaled down to fit the smallest text column of the story's frames (never scaled up). +fn inline_image( + doc: &mut designcraft_doc::Document, + story: StoryId, + field: &str, + cell: &str, + record: u32, + sources: &Sources, + report: &mut FillReport, +) -> Option { + let Some(image) = load_image(cell, sources) else { + report_missing(report, record, field, cell); + return None; + }; + let (nw, nh) = image.natural; + let frame = doc.story(story).and_then(|st| st.frames.first().copied()).and_then(|id| doc.item(id)); + let layer = frame.map_or_else(|| doc.default_layer(), |it| it.layer); + // The marker may flow into any of the story's frames: fit the smallest column among them. + let fit = designcraft_compose::frame_specs(doc, story) + .iter() + .flat_map(|f| f.columns()) + .map(|b| (b.width() / nw).min(b.height() / nh)) + .reduce(f64::min) + .unwrap_or(1.0); + let k = if fit.is_finite() && fit > 0.0 { fit.min(1.0) } else { 1.0 }; + let rect = Rect::new(0.0, 0.0, nw * k, nh * k); + let xf = Affine::scale(k); + if super::super::file::validate_placement((nw, nh), rect, xf, "data.merge").is_err() { + report.missing.push(MissingImage { record, field: field.to_string(), path: cell.to_string() }); + report.warnings.push(format!("Record {record}, field {field}: image {cell} exceeds finite geometry limits.")); + return None; + } + let asset = add_image_asset(doc, image, field, sources.options); + let mut it = Item::new(ItemId(doc.alloc()), layer, Shape::Rectangle, shapes::rectangle(rect)); + it.object_style = doc.styles.default_graphic_frame.clone(); + if let Some(e) = doc.styles.object_effects(&it.object_style) { + it.effects = e.clone(); + } + it.content = Content::Graphic(Graphic { + layer_options: None, + asset, + size: (nw, nh), + xf, + auto_fit: Fitting::None, + fit_align: 0, + crop: [0.0; 4], + wrap: Default::default(), + clip: None, + fill: Default::default(), }); - let Some((path, bytes)) = found else { + Some(AnchoredObject::new(it, AnchorPosition::default())) +} + +fn place_image(doc: &mut designcraft_doc::Document, item: ItemId, field: &str, cell: &str, record: u32, sources: &Sources, report: &mut FillReport) { + let options = sources.options; + let Some(image) = load_image(cell, sources) else { if let Some(it) = doc.item_mut(item) { it.content = Content::Unassigned; } - report.missing.push(MissingImage { record, field: field.to_string(), path: cell.to_string() }); - report.warnings.push(format!("Record {record}, field {field}: missing image {cell}.")); - return; - }; - let Some(((pw, ph), (nw, nh))) = designcraft_render::image_size(&bytes).zip(designcraft_render::natural_size(&bytes, 0)) else { - if let Some(it) = doc.item_mut(item) { - it.content = Content::Unassigned; - } - report.missing.push(MissingImage { record, field: field.to_string(), path: cell.to_string() }); - report.warnings.push(format!("Record {record}, field {field}: missing image {cell}.")); + report_missing(report, record, field, cell); return; }; + let (nw, nh) = image.natural; let Some(inner) = doc.item(item).map(|it| it.inner_bounds()) else { report.warnings.push(format!("Record {record}, field {field}: missing image {cell}.")); report.missing.push(MissingImage { record, field: field.to_string(), path: cell.to_string() }); @@ -488,20 +604,7 @@ fn place_image(doc: &mut designcraft_doc::Document, item: ItemId, field: &str, c report.warnings.push(format!("Record {record}, field {field}: image {cell} exceeds finite geometry limits.")); return; } - let fresh = AssetId(doc.alloc()); - let name = path.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_else(|| field.to_string()); - let link = options.link_images.then(|| path.to_string_lossy().into_owned()); - g.asset = doc.add_asset(Asset { - id: fresh, - name, - mime: designcraft_render::image_mime(&bytes).to_string(), - link, - // The cached buffer is shared with every other record naming this file, rather than - // copied per record (#1595). - data: bytes, - pixels: Some((pw, ph)), - ..Default::default() - }); + g.asset = add_image_asset(doc, image, field, options); if let Some(it) = doc.item_mut(item) { it.content = Content::Graphic(g); } @@ -662,4 +765,15 @@ mod image_path_tests { let found = image_candidates("img/x.png", Some(data), Some(doc)); assert_eq!(found, [data.join("img").join("x.png"), doc.join("img").join("x.png")]); } + + /// A path that starts with one separator is tried as it is, then beside the data file and the + /// document without the separator. Two separators still name another computer. + #[cfg(unix)] + #[test] + fn a_rooted_image_cell_is_also_tried_beside_the_data_and_the_document() { + let (data, doc) = (Path::new("/work/data"), Path::new("/work/doc")); + let found = image_candidates("/img/x.png", Some(data), Some(doc)); + assert_eq!(found, [Path::new("/img/x.png").to_path_buf(), data.join("img").join("x.png"), doc.join("img").join("x.png")]); + assert!(image_candidates("//192.0.2.1/img/x.png", Some(data), Some(doc)).is_empty()); + } } diff --git a/crates/engine/src/cmd/datamerge/parse.rs b/crates/engine/src/cmd/datamerge/parse.rs index dfc49e842..5b8717baa 100644 --- a/crates/engine/src/cmd/datamerge/parse.rs +++ b/crates/engine/src/cmd/datamerge/parse.rs @@ -288,15 +288,18 @@ fn next_slot(col: i32, row: i32, rows_first: bool, fits_col: &impl Fn(i32) -> bo /// Files to try for an image cell: an absolute path as it is, a relative one beside the data /// file, then beside the document. Relative paths are not resolved against the process working /// directory, and paths that may not be followed ([`crate::io::document_path`]: URLs, other -/// computers, devices) are not candidates. +/// computers, devices) are not candidates. A path that starts with one separator (`\Photos\x.jpg`) +/// is tried as it is first, then beside the data file and the document with that separator dropped. pub fn image_candidates(cell: &str, data_dir: Option<&Path>, doc_dir: Option<&Path>) -> Vec { let cell = cell.trim(); if cell.is_empty() { return Vec::new(); } + let below = cell.strip_prefix(['\\', '/']).filter(|rest| !rest.is_empty() && !rest.starts_with(['\\', '/'])); let found = std::iter::once(crate::io::document_path(cell, None)) .chain(data_dir.map(|dir| crate::io::folder_path(cell, dir))) - .chain(doc_dir.map(|dir| crate::io::folder_path(cell, dir))); + .chain(doc_dir.map(|dir| crate::io::folder_path(cell, dir))) + .chain(below.into_iter().flat_map(|rest| [data_dir, doc_dir].into_iter().flatten().map(move |dir| crate::io::folder_path(rest, dir)))); let mut out: Vec = Vec::new(); for p in found.flatten() { if !out.contains(&p) { diff --git a/crates/engine/src/cmd/datamerge/tests.rs b/crates/engine/src/cmd/datamerge/tests.rs index 4793aac17..8ca5291a6 100644 --- a/crates/engine/src/cmd/datamerge/tests.rs +++ b/crates/engine/src/cmd/datamerge/tests.rs @@ -749,6 +749,157 @@ fn role_overrides_column_kind_qr_and_hyperlink() { assert_eq!(kinds, vec!["email:a@b.c".to_string(), "url:https://example.com".to_string()]); } +fn write_png_sized(path: &Path, width: u32, height: u32) { + let pixels = vec![255; width as usize * height as usize * 4]; + let bytes = designcraft_render::Rendered { width, height, pixels }.to_png(); + assert_eq!(designcraft_render::image_size(&bytes), Some((width, height)), "the fixture png must decode"); + if let Some(dir) = path.parent() { + std::fs::create_dir_all(dir).unwrap(); + } + std::fs::write(path, bytes).unwrap(); +} + +/// The images anchored in the stories of document `index`, as (file name, width, height), by name. +fn inline_images(s: &Session, index: usize) -> Vec<(String, f64, f64)> { + let doc = &s.documents()[index].doc; + let mut out = Vec::new(); + for st in doc.stories.values() { + for o in &st.objects { + if let Content::Graphic(g) = &o.item.content { + let name = doc.assets.get(&g.asset).map(|a| a.name.clone()).unwrap_or_default(); + let (w, h) = o.size(); + out.push((name, w, h)); + } + } + } + out.sort_by(|a, b| a.0.cmp(&b.0)); + out +} + +/// An image field inserted at a text insertion point placed the path as text. It places the +/// image in the text: at its natural size, scaled down to fit the frame, with a missing file +/// reported and its marker removed. +#[test] +fn an_image_field_at_the_insertion_point_merges_as_an_inline_image() { + let dir = scratch("inline-image"); + let data = dir.join("data"); + write_png_sized(&data.join("pics").join("small.png"), 40, 20); + write_png_sized(&data.join("pics").join("wide.png"), 800, 100); + write(&data.join("cards.csv"), "Name,@Photo\nAda,pics/small.png\nGrace,pics/wide.png\nKai,pics/none.png\n"); + + let mut s = Session::new(); + new_doc(&mut s, 1); + exec(&mut s, "data.source.select", json!({"path": data.join("cards.csv")})); + let made = text_frame(&mut s, [36.0, 36.0, 236.0, 236.0], "Photo: ", None); + exec(&mut s, "text.select", json!({"story": made["story"], "anchor": 7, "focus": 7})); + exec(&mut s, "data.placeholder.add", json!({"field": "Photo"})); + let template = &s.documents()[0].doc; + assert_eq!(template.stories.values().next().unwrap().text, "Photo: <>"); + let width = template.item(designcraft_doc::ItemId(made["id"].as_u64().unwrap())).unwrap().inner_bounds().width(); + + exec(&mut s, "data.preview", json!({"record": 1})); + assert_eq!(inline_images(&s, 0).len(), 1, "the preview shows the image"); + exec(&mut s, "data.preview.stop", json!({})); + + let r = exec(&mut s, "data.merge", json!({})); + assert_eq!(r["records"], 3); + let missing = r["missingImages"].as_array().unwrap(); + assert_eq!(missing.len(), 1, "{missing:?}"); + assert_eq!((missing[0]["record"].as_u64(), missing[0]["path"].as_str()), (Some(3), Some("pics/none.png"))); + let mark = designcraft_doc::OBJECT_MARK; + assert_eq!(texts(&s), vec!["Photo: ".to_string(), format!("Photo: {mark}"), format!("Photo: {mark}")]); + let placed = inline_images(&s, s.active_index().unwrap()); + assert_eq!(placed.len(), 2, "{placed:?}"); + let (small, wide) = (&placed[0], &placed[1]); + assert_eq!(small.0, "small.png"); + assert!((small.1 - 40.0).abs() < 0.01 && (small.2 - 20.0).abs() < 0.01, "a small image keeps its natural size: {small:?}"); + assert_eq!(wide.0, "wide.png"); + assert!((wide.1 - width).abs() < 0.01 && (wide.2 - width / 8.0).abs() < 0.01, "a wide image is scaled down to the frame: {wide:?}"); +} + +/// A typed `<>` marker for an image column places the image too, from an absolute path; +/// an explicit text role still writes the path as text. +#[test] +fn a_typed_image_marker_places_the_image_and_a_text_role_keeps_the_path() { + let dir = scratch("typed-image"); + let pic = dir.join("pic.png"); + write_png_sized(&pic, 10, 30); + let mut s = Session::new(); + new_doc(&mut s, 1); + select_csv(&mut s, &format!("@Photo\n{}\n", pic.display())); + text_frame(&mut s, [36.0, 36.0, 300.0, 300.0], "A <> B", None); + let r = exec(&mut s, "data.merge", json!({})); + assert_eq!(r["missingImages"].as_array().map(Vec::len), Some(0)); + assert_eq!(texts(&s), vec![format!("A {} B", designcraft_doc::OBJECT_MARK)]); + let placed = inline_images(&s, s.active_index().unwrap()); + assert_eq!(placed.len(), 1); + assert!((placed[0].1 - 10.0).abs() < 0.01 && (placed[0].2 - 30.0).abs() < 0.01, "{placed:?}"); + + let mut s = Session::new(); + new_doc(&mut s, 1); + select_csv(&mut s, &format!("@Photo\n{}\n", pic.display())); + let made = text_frame(&mut s, [36.0, 36.0, 300.0, 300.0], "", None); + exec(&mut s, "text.select", json!({"story": made["story"], "anchor": 0, "focus": 0})); + exec(&mut s, "data.placeholder.add", json!({"field": "Photo", "role": "text"})); + exec(&mut s, "data.merge", json!({})); + assert_eq!(texts(&s), vec![pic.display().to_string()]); + assert!(inline_images(&s, s.active_index().unwrap()).is_empty()); +} + +/// An explicit image role at a text insertion point places an inline image too, and in a frame +/// with two columns the image is scaled down to one column's width. +#[test] +fn an_inline_image_role_at_text_fits_one_column() { + let dir = scratch("inline-columns"); + let pic = dir.join("wide.png"); + write_png_sized(&pic, 800, 100); + let mut s = Session::new(); + new_doc(&mut s, 1); + select_csv(&mut s, &format!("@Photo\n{}\n", pic.display())); + let made = text_frame(&mut s, [36.0, 36.0, 456.0, 336.0], "", None); + exec(&mut s, "object.textFrameOptions", json!({"ids": [made["id"]], "columns": 2, "gutter": 20})); + exec(&mut s, "text.select", json!({"story": made["story"], "anchor": 0, "focus": 0})); + exec(&mut s, "data.placeholder.add", json!({"field": "Photo", "role": "image"})); + let r = exec(&mut s, "data.merge", json!({})); + assert_eq!(r["missingImages"].as_array().map(Vec::len), Some(0), "{r}"); + let placed = inline_images(&s, s.active_index().unwrap()); + assert_eq!(placed.len(), 1, "{placed:?}"); + // 420 pt wide, less the 20 pt gutter, gives two 200 pt columns. + assert!((placed[0].1 - 200.0).abs() < 0.01 && (placed[0].2 - 25.0).abs() < 0.01, "{placed:?}"); + + // The story threads on into a 100 pt frame: the image fits that one, wherever the marker lands. + let mut s = Session::new(); + new_doc(&mut s, 1); + select_csv(&mut s, &format!("@Photo\n{}\n", pic.display())); + let made = text_frame(&mut s, [36.0, 36.0, 456.0, 336.0], "", None); + let small = exec(&mut s, "frame.create", json!({"rect": [36.0, 400.0, 136.0, 500.0], "content": "unassigned"})); + exec(&mut s, "text.thread", json!({"from": made["id"], "to": small["id"]})); + exec(&mut s, "text.select", json!({"story": made["story"], "anchor": 0, "focus": 0})); + exec(&mut s, "data.placeholder.add", json!({"field": "Photo"})); + exec(&mut s, "data.merge", json!({})); + let placed = inline_images(&s, s.active_index().unwrap()); + assert_eq!(placed.len(), 1, "{placed:?}"); + assert!(placed[0].1 <= 100.0 + 0.01, "{placed:?}"); +} + +/// A cell that starts with one separator (`\pics\a.png`) and names no file from the drive's root +/// is found beside the data file. +#[test] +fn an_image_path_with_a_leading_separator_is_found_beside_the_data_file() { + let dir = scratch("rooted-image"); + let data = dir.join("data"); + write_png_sized(&data.join("dc-merge-rooted-pics").join("a.png"), 12, 12); + write(&data.join("cards.csv"), "@Photo\n\\dc-merge-rooted-pics\\a.png\n/dc-merge-rooted-pics/a.png\n"); + let mut s = Session::new(); + new_doc(&mut s, 1); + exec(&mut s, "data.source.select", json!({"path": data.join("cards.csv")})); + let frame = exec(&mut s, "frame.create", json!({"rect": [36.0, 36.0, 80.0, 80.0], "content": "graphic"})); + exec(&mut s, "data.placeholder.add", json!({"field": "Photo", "item": frame["id"]})); + let r = exec(&mut s, "data.merge", json!({})); + assert_eq!(r["records"], 2); + assert_eq!(r["missingImages"].as_array().map(Vec::len), Some(0), "{r}"); +} + #[test] fn single_record_merge_copies_pages_and_leaves_the_template() { let mut s = Session::new(); diff --git a/docs/agents.md b/docs/agents.md index 621173a85..d2f416c83 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -88,7 +88,10 @@ external PDF/X certification. `data.source.select {path, sheet?}` links a CSV/TSV/`.xlsx` file to the document (`data.source.update` re-reads it, `data.source.remove` drops it); `data.fields` lists its fields. `data.placeholder.add {field, role?, story?, at?, end?, item?}` -marks text or a frame as a field (`role`: text, image, qr, hyperlink); `data.options` sets records, tiling and image +marks text or a frame as a field (`role`: text, image, qr, hyperlink). An image field (a `@` column) inserted at a text +insertion point, or typed there as `<>`, merges as an inline image at its natural size, scaled down to fit the +frame. An image path is absolute or relative to the data file's folder, then the document's; a path that starts with +one separator (`\Photos\a.jpg`) is tried from the drive's root first. `data.options` sets records, tiling and image fitting; `data.preview {record}` / `data.preview.stop` show one record without saving it. `data.merge` now **creates a new document** and leaves the template unchanged (the old `spread` parameter, which From d490e0fb185b472c82c5e320d77b66e7b262b8a0 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 11 Nov 2026 18:14:15 -0800 Subject: [PATCH 2/5] Data merge: a QR field at a text insertion point is refused instead of written as text Insert Field with a QR column and the text cursor in a frame added a text placeholder, so a merge wrote the QR payload as plain text. It now returns "qr placeholders bind to a frame" and leaves the story unchanged. --- crates/engine/src/cmd/datamerge.rs | 3 +++ crates/engine/src/cmd/datamerge/tests.rs | 14 ++++++++++++++ docs/agents.md | 7 ++++--- 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/cmd/datamerge.rs b/crates/engine/src/cmd/datamerge.rs index 6fac73836..411fd545c 100644 --- a/crates/engine/src/cmd/datamerge.rs +++ b/crates/engine/src/cmd/datamerge.rs @@ -503,6 +503,9 @@ fn add_placeholder(d: &mut Document, sel: &Selection, p: &Value, name: &str) -> } else if field.kind == DataFieldKind::Image { // An image field at the insertion point places the image inline in the text. PlaceholderRole::Image + } else if field.kind == DataFieldKind::Qr { + // A QR field at the insertion point is refused below: QR codes bind to a frame. + PlaceholderRole::Qr } else { PlaceholderRole::Text }; diff --git a/crates/engine/src/cmd/datamerge/tests.rs b/crates/engine/src/cmd/datamerge/tests.rs index 8ca5291a6..39b1988e2 100644 --- a/crates/engine/src/cmd/datamerge/tests.rs +++ b/crates/engine/src/cmd/datamerge/tests.rs @@ -817,6 +817,20 @@ fn an_image_field_at_the_insertion_point_merges_as_an_inline_image() { assert!((wide.1 - width).abs() < 0.01 && (wide.2 - width / 8.0).abs() < 0.01, "a wide image is scaled down to the frame: {wide:?}"); } +/// A QR field at a text insertion point is refused instead of merging its payload as text. +#[test] +fn a_qr_field_at_the_insertion_point_is_refused() { + let mut s = Session::new(); + new_doc(&mut s, 1); + select_csv(&mut s, "@Photo,#Code\n/x.png,hello\n"); + let made = text_frame(&mut s, [36.0, 36.0, 300.0, 300.0], "Hi ", None); + exec(&mut s, "text.select", json!({"story": made["story"], "anchor": 3, "focus": 3})); + let msg = err(&mut s, "data.placeholder.add", json!({"field": "Code"})); + assert!(msg.contains("bind to a frame"), "{msg}"); + assert_eq!(texts(&s), vec!["Hi ".to_string()]); + assert!(s.documents()[0].doc.data_merge.placeholders.is_empty()); +} + /// A typed `<>` marker for an image column places the image too, from an absolute path; /// an explicit text role still writes the path as text. #[test] diff --git a/docs/agents.md b/docs/agents.md index d2f416c83..13977d2af 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -90,9 +90,10 @@ external PDF/X certification. `data.source.remove` drops it); `data.fields` lists its fields. `data.placeholder.add {field, role?, story?, at?, end?, item?}` marks text or a frame as a field (`role`: text, image, qr, hyperlink). An image field (a `@` column) inserted at a text insertion point, or typed there as `<>`, merges as an inline image at its natural size, scaled down to fit the -frame. An image path is absolute or relative to the data file's folder, then the document's; a path that starts with -one separator (`\Photos\a.jpg`) is tried from the drive's root first. `data.options` sets records, tiling and image -fitting; `data.preview {record}` / `data.preview.stop` show one record without saving it. +frame. A QR field (a `#` column) inserted at a text insertion point is refused; bind it to a frame instead. An image +path is absolute or relative to the data file's folder, then the document's; a path that starts with one separator +(`\Photos\a.jpg`) is tried from the drive's root first. `data.options` sets records, tiling and image fitting; +`data.preview {record}` / `data.preview.stop` show one record without saving it. `data.merge` now **creates a new document** and leaves the template unchanged (the old `spread` parameter, which appended pages to the template, is an error). It uses the linked source, or inline `csv` / `rows` / `path` / `bytes`: From b73c21db37acf667461d99007be84ee7c2a8099b Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 11 Nov 2026 20:36:39 -0800 Subject: [PATCH 3/5] Data merge: a file that is not an image does not hide a later image candidate The image search stopped at the first file that could be read and checked whether it decodes afterwards. A non-image file at an earlier candidate (the drive root, or beside the data file) then hid a valid image beside the data file or the document, and the field was reported missing. The decode check now runs inside the search, so only an image ends it. --- crates/engine/src/cmd/datamerge/fill.rs | 11 ++++++----- crates/engine/src/cmd/datamerge/tests.rs | 22 ++++++++++++++++++++++ 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/crates/engine/src/cmd/datamerge/fill.rs b/crates/engine/src/cmd/datamerge/fill.rs index caea4d67e..ebe950fb6 100644 --- a/crates/engine/src/cmd/datamerge/fill.rs +++ b/crates/engine/src/cmd/datamerge/fill.rs @@ -464,7 +464,7 @@ struct LoadedImage { fn load_image(cell: &str, sources: &Sources) -> Option { let Sources { data_dir, doc_dir, images, .. } = *sources; let cands = image_candidates(cell, data_dir, doc_dir); - let (path, bytes) = cands.iter().find_map(|p| { + cands.iter().find_map(|p| { // Read once per merge: many records naming one file is the normal case, and reading it // per record is the amplification (#1595). let bytes = images.get_or_read(p, || { @@ -475,10 +475,11 @@ fn load_image(cell: &str, sources: &Sources) -> Option { } crate::io::read_file(p).ok().filter(|b| !b.is_empty()) })?; - Some((p.clone(), bytes)) - })?; - let (pixels, natural) = designcraft_render::image_size(&bytes).zip(designcraft_render::natural_size(&bytes, 0))?; - Some(LoadedImage { path, bytes, pixels, natural }) + // A file that does not decode leaves the search going, so a valid image at a later + // candidate is still found. + let (pixels, natural) = designcraft_render::image_size(&bytes).zip(designcraft_render::natural_size(&bytes, 0))?; + Some(LoadedImage { path: p.clone(), bytes, pixels, natural }) + }) } fn report_missing(report: &mut FillReport, record: u32, field: &str, cell: &str) { diff --git a/crates/engine/src/cmd/datamerge/tests.rs b/crates/engine/src/cmd/datamerge/tests.rs index 39b1988e2..5480a5686 100644 --- a/crates/engine/src/cmd/datamerge/tests.rs +++ b/crates/engine/src/cmd/datamerge/tests.rs @@ -914,6 +914,28 @@ fn an_image_path_with_a_leading_separator_is_found_beside_the_data_file() { assert_eq!(r["missingImages"].as_array().map(Vec::len), Some(0), "{r}"); } +/// A file at an earlier candidate that is not an image does not hide a valid image at a later +/// one: the search goes on past it. +#[test] +fn a_non_image_file_beside_the_data_does_not_hide_the_image_beside_the_document() { + let dir = scratch("undecodable-image"); + let (data, docs) = (dir.join("data"), dir.join("docs")); + write(&data.join("pics").join("a.png"), "not an image"); + write_png_sized(&docs.join("pics").join("a.png"), 12, 12); + write(&data.join("cards.csv"), "@Photo\npics/a.png\n"); + let mut s = Session::new(); + new_doc(&mut s, 1); + exec(&mut s, "data.source.select", json!({"path": data.join("cards.csv")})); + exec(&mut s, "file.save", json!({"path": docs.join("layout.designcraft")})); + let frame = exec(&mut s, "frame.create", json!({"rect": [36.0, 36.0, 80.0, 80.0], "content": "graphic"})); + exec(&mut s, "data.placeholder.add", json!({"field": "Photo", "item": frame["id"]})); + exec(&mut s, "data.options", json!({"linkImages": true})); + let r = exec(&mut s, "data.merge", json!({})); + assert_eq!(r["missingImages"].as_array().map(Vec::len), Some(0), "{r}"); + let link = graphic_link(&s).unwrap_or_default(); + assert!(link.contains("docs"), "{link}"); +} + #[test] fn single_record_merge_copies_pages_and_leaves_the_template() { let mut s = Session::new(); From fad00dd646298479046accd3102ee21e776f3914 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Thu, 12 Nov 2026 05:55:38 -0800 Subject: [PATCH 4/5] Data merge: an image field in a table cell or a note is refused by name With the insertion point in a table cell or a note, data.placeholder.add with an image field returned "a hyperlink field can't go in a table cell or a note". It now returns a message about image fields that names the two ways on: bind the field to a frame, or pass role text to merge its path. Nothing is inserted on the error, and role text still puts the marker in the cell. Docs: docs/agents.md and the command's params text. --- crates/engine/src/cmd/datamerge.rs | 10 +++++++++- crates/engine/src/cmd/datamerge/tests.rs | 19 +++++++++++++++++++ docs/agents.md | 4 +++- 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/cmd/datamerge.rs b/crates/engine/src/cmd/datamerge.rs index 213b3d1cb..e03da2d05 100644 --- a/crates/engine/src/cmd/datamerge.rs +++ b/crates/engine/src/cmd/datamerge.rs @@ -58,7 +58,7 @@ pub fn specs() -> Vec { "Insert Field", [], None, - "{field, role?: text|image|qr|hyperlink, story?, at?, end?, item?} → {id}; an image field at a text insertion point merges as an inline image; with the insertion point in a table cell or a note → {cell: true, marker} (a typed marker, not an anchored placeholder)", + "{field, role?: text|image|qr|hyperlink, story?, at?, end?, item?} → {id}; an image field at a text insertion point merges as an inline image; with the insertion point in a table cell or a note → {cell: true, marker} (a typed marker, not an anchored placeholder; an image field there is an error unless role is text)", has_doc, placeholder_add ), @@ -535,6 +535,14 @@ fn add_placeholder(d: &mut Document, sel: &mut Selection, p: &Value, name: &str) && let Some(t) = sel.text && let Some(pressed) = t.cell { + // The merge fills such a text with the record's value as text, so an image there + // would merge as its path. + if role == PlaceholderRole::Image { + return Err(bad( + "data.placeholder.add", + "an image field can't go in a table cell or a note; bind it to a frame, or insert it with role text to merge its path", + )); + } if role != PlaceholderRole::Text { return Err(bad("data.placeholder.add", "a hyperlink field can't go in a table cell or a note; insert it as text")); } diff --git a/crates/engine/src/cmd/datamerge/tests.rs b/crates/engine/src/cmd/datamerge/tests.rs index e5cf28497..5f5c4a118 100644 --- a/crates/engine/src/cmd/datamerge/tests.rs +++ b/crates/engine/src/cmd/datamerge/tests.rs @@ -1605,6 +1605,25 @@ fn a_field_named_with_angle_brackets_is_refused_in_a_cell() { let _ = made; } +/// An image field with the insertion point in a table cell is refused with a message about image +/// fields, and nothing is inserted; with role text its marker goes into the cell. +#[test] +fn an_image_field_in_a_cell_is_refused_unless_its_role_is_text() { + let mut s = Session::new(); + table_with_caret_in_cell(&mut s, "Name,@Photo\nAda,pics/a.png\n"); + for params in [json!({"field": "Photo"}), json!({"field": "Photo", "role": "image"})] { + let e = s.execute("data.placeholder.add", ¶ms).expect_err("refused").to_string(); + assert!(e.contains("an image field can't go in a table cell or a note"), "{e}"); + assert!(!e.contains("hyperlink"), "{e}"); + assert_eq!(cell_texts(&s), ["Who", "Where"], "nothing inserted"); + } + let r = exec(&mut s, "data.placeholder.add", json!({"field": "Photo", "role": "text"})); + assert_eq!(r["marker"], "<>", "{r}"); + assert_eq!(cell_texts(&s), ["Who", "<>Where"]); + exec(&mut s, "data.preview", json!({"record": 1})); + assert_eq!(cell_texts(&s), ["Who", "pics/a.pngWhere"], "the text role merges the path"); +} + /// Endnote text is filled too: it is a text inside its story, like a footnote's. #[test] fn a_field_in_an_endnote_is_filled() { diff --git a/docs/agents.md b/docs/agents.md index 13977d2af..b2ec4c156 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -90,7 +90,9 @@ external PDF/X certification. `data.source.remove` drops it); `data.fields` lists its fields. `data.placeholder.add {field, role?, story?, at?, end?, item?}` marks text or a frame as a field (`role`: text, image, qr, hyperlink). An image field (a `@` column) inserted at a text insertion point, or typed there as `<>`, merges as an inline image at its natural size, scaled down to fit the -frame. A QR field (a `#` column) inserted at a text insertion point is refused; bind it to a frame instead. An image +frame. The merge writes only text into a table cell or a note: an image field inserted there is refused unless `role` +is `text`, and its `<>` marker, inserted or typed, merges as the path. A QR field (a `#` column) inserted at a +text insertion point is refused; bind it to a frame instead. An image path is absolute or relative to the data file's folder, then the document's; a path that starts with one separator (`\Photos\a.jpg`) is tried from the drive's root first. `data.options` sets records, tiling and image fitting; `data.preview {record}` / `data.preview.stop` show one record without saving it. From 329806477e0bd05f7bf05f16e44d06fdb9cfb5f8 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Thu, 12 Nov 2026 07:45:10 -0800 Subject: [PATCH 5/5] Data merge: an explicit QR role in text is refused, and the caret follows each field data.placeholder.add with role qr and the insertion point in text bound the QR code to the first selected frame, which the caret was not in. It now returns "qr placeholders bind to a frame", as a QR field there does. A field inserted at the insertion point left the caret before its marker, so a second field went in front of the first. The caret now moves past the marker, as after typing. Docs: docs/agents.md. --- crates/engine/src/cmd/datamerge.rs | 9 +++++- crates/engine/src/cmd/datamerge/tests.rs | 38 ++++++++++++++++++++++++ docs/agents.md | 5 ++-- 3 files changed, 49 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/cmd/datamerge.rs b/crates/engine/src/cmd/datamerge.rs index e03da2d05..c57d507fb 100644 --- a/crates/engine/src/cmd/datamerge.rs +++ b/crates/engine/src/cmd/datamerge.rs @@ -485,7 +485,9 @@ fn add_placeholder(d: &mut Document, sel: &mut Selection, p: &Value, name: &str) } else if explicit_story || role_param == Some("text") { false } else if role_param == Some("qr") { - true + // A QR role with the insertion point in text is refused below rather than bound to a + // selected frame the caret is not in. + sel.text.is_none() } else if role_param == Some("image") { // An image role with the insertion point in text places an inline image there. sel.text.is_none() @@ -585,6 +587,11 @@ fn add_placeholder(d: &mut Document, sel: &mut Selection, p: &Value, name: &str) st.replace(at..at, &marker); } d.adjust_text_ranges(story, at..at, marker.len()); + // The caret moves past the field, as after typing, so the next field goes after it. + if let Some(t) = sel.text.as_mut().filter(|t| t.story == story && t.cell.is_none()) { + let shift = |pos: usize| if pos >= at { pos + marker.len() } else { pos }; + (t.anchor, t.focus) = (shift(t.anchor), shift(t.focus)); + } PlaceholderAnchor::Text { story, start: at, end: at.saturating_add(marker.len()) } } else { let text = d.story(story).map(|st| st.text.clone()).unwrap_or_default(); diff --git a/crates/engine/src/cmd/datamerge/tests.rs b/crates/engine/src/cmd/datamerge/tests.rs index 5f5c4a118..9180964c2 100644 --- a/crates/engine/src/cmd/datamerge/tests.rs +++ b/crates/engine/src/cmd/datamerge/tests.rs @@ -831,6 +831,44 @@ fn a_qr_field_at_the_insertion_point_is_refused() { assert!(s.documents()[0].doc.data_merge.placeholders.is_empty()); } +/// An explicit QR role at a text insertion point is refused too, rather than binding the QR code +/// to a selected frame the caret is not in. +#[test] +fn an_explicit_qr_role_at_the_insertion_point_is_refused() { + let mut s = Session::new(); + new_doc(&mut s, 1); + select_csv(&mut s, "Name,#Code\nAda,hello\n"); + let made = text_frame(&mut s, [36.0, 36.0, 300.0, 300.0], "Hi ", None); + let other = text_frame(&mut s, [36.0, 400.0, 300.0, 600.0], "", None); + let other_id = designcraft_doc::ItemId(other["id"].as_u64().unwrap()); + let sid = StoryId(made["story"].as_u64().unwrap()); + let doc = s.doc_mut().unwrap(); + doc.selection = Selection::text(TextSel { story: sid, anchor: 3, focus: 3, frame: None, cell: None }); + doc.selection.items = vec![other_id]; + let msg = err(&mut s, "data.placeholder.add", json!({"field": "Code", "role": "qr"})); + assert!(msg.contains("bind to a frame"), "{msg}"); + let doc = &s.documents()[0].doc; + assert!(doc.data_merge.placeholders.is_empty()); + assert_eq!(doc.item(other_id).unwrap().label, "", "the other frame is not bound"); +} + +/// Fields inserted one after another at the insertion point keep their order: the caret moves +/// past each marker, as after typing. +#[test] +fn fields_inserted_at_the_insertion_point_keep_their_order() { + let mut s = Session::new(); + new_doc(&mut s, 1); + select_csv(&mut s, "Name,City,@Photo\nAda,Paris,/x.png\n"); + let made = text_frame(&mut s, [36.0, 36.0, 300.0, 300.0], "Hi ", None); + exec(&mut s, "text.select", json!({"story": made["story"], "anchor": 3, "focus": 3})); + exec(&mut s, "data.placeholder.add", json!({"field": "Name", "role": "text"})); + exec(&mut s, "data.placeholder.add", json!({"field": "City", "role": "text"})); + exec(&mut s, "data.placeholder.add", json!({"field": "Photo"})); + assert_eq!(texts(&s), vec!["Hi <><><>".to_string()]); + let t = s.documents()[0].selection.text.unwrap(); + assert_eq!((t.anchor, t.focus), (28, 28)); +} + /// A typed `<>` marker for an image column places the image too, from an absolute path; /// an explicit text role still writes the path as text. #[test] diff --git a/docs/agents.md b/docs/agents.md index b2ec4c156..8058510e6 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -91,8 +91,9 @@ external PDF/X certification. marks text or a frame as a field (`role`: text, image, qr, hyperlink). An image field (a `@` column) inserted at a text insertion point, or typed there as `<>`, merges as an inline image at its natural size, scaled down to fit the frame. The merge writes only text into a table cell or a note: an image field inserted there is refused unless `role` -is `text`, and its `<>` marker, inserted or typed, merges as the path. A QR field (a `#` column) inserted at a -text insertion point is refused; bind it to a frame instead. An image +is `text`, and its `<>` marker, inserted or typed, merges as the path. A QR field (a `#` column) or `role: "qr"` +at a text insertion point is refused; bind it to a frame with `item` instead. A field inserted at the insertion point +puts the caret after its marker. An image path is absolute or relative to the data file's folder, then the document's; a path that starts with one separator (`\Photos\a.jpg`) is tried from the drive's root first. `data.options` sets records, tiling and image fitting; `data.preview {record}` / `data.preview.stop` show one record without saving it.