From 078b8bdcd2b1c87ada6e299a47e1d3dc37d145d7 Mon Sep 17 00:00:00 2001 From: Hamza Date: Thu, 5 Nov 2026 12:15:32 +0500 Subject: [PATCH] Data Merge: a field in a table cell goes into the cell and is filled (#2315) With the insertion point in a table cell, data.placeholder.add took the selection's offsets, which are the cell's, as offsets into the table's own story, so the marker landed outside the table. The fill only read each story's own text, so a field in a cell, inserted or converted from text, was left as its marker. A field inserted in a cell now goes into the cell's text as its marker, and the fill replaces the markers in the table cells and footnotes of the stories it fills. --- crates/engine/src/cmd/datamerge.rs | 14 +++++++ crates/engine/src/cmd/datamerge/fill.rs | 39 ++++++++++++++++++ crates/engine/src/cmd/datamerge/tests.rs | 52 ++++++++++++++++++++++++ 3 files changed, 105 insertions(+) diff --git a/crates/engine/src/cmd/datamerge.rs b/crates/engine/src/cmd/datamerge.rs index 4d3cd5dd6..e834127fd 100644 --- a/crates/engine/src/cmd/datamerge.rs +++ b/crates/engine/src/cmd/datamerge.rs @@ -482,6 +482,20 @@ fn add_placeholder(d: &mut Document, sel: &Selection, p: &Value, name: &str) -> if matches!(role, PlaceholderRole::Image | PlaceholderRole::Qr) { return Err(bad("data.placeholder.add", "image and qr placeholders bind to a frame")); } + // The insertion point is in a table cell: the field goes into the cell's text as its + // marker, which the merge fills as it fills a typed one. A cell's text is a story of its + // own that a placeholder can't name, and the selection's offsets are the cell's, so they + // used to put the marker in the table's story instead (#2315). + if !explicit_story && let Some(t) = sel.text.filter(|t| t.cell.is_some()) { + if role != PlaceholderRole::Text { + return Err(bad("data.placeholder.add", "a hyperlink field can't go in a table cell; insert it as text")); + } + let marker = format!("<<{name}>>"); + let st = d.text_story_mut(t.story, t.cell).ok_or_else(|| bad("data.placeholder.add", "no table cell there"))?; + let at = designcraft_doc::floor_char_boundary(&st.text, t.range().start.min(st.text.len())); + st.replace(at..at, &marker); + return Ok(json!({"cell": true, "marker": marker})); + } 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")); diff --git a/crates/engine/src/cmd/datamerge/fill.rs b/crates/engine/src/cmd/datamerge/fill.rs index 825179ac9..288dbf44f 100644 --- a/crates/engine/src/cmd/datamerge/fill.rs +++ b/crates/engine/src/cmd/datamerge/fill.rs @@ -228,6 +228,7 @@ pub fn fill_row( let Pending { mut reps, links, parent, mut report } = pending; typed_reps(doc, fields, row, maps, &placeholders, &mut reps); apply_reps(doc, &reps); + fill_nested_markers(doc, fields, row, maps); for (anchor, dest, field) in links { let anchor = match anchor { PlaceholderAnchor::Text { story, start, end } => { @@ -331,6 +332,44 @@ fn typed_reps( } } +/// The `<>` markers in the table cells and footnotes of the stories being filled. A cell's +/// or a footnote's text is a story of its own inside its story, which no placeholder can name and +/// `typed_reps` doesn't read, so a field in a table was left as its marker (#2315). Only a table +/// or note that holds a marker is copied. +fn fill_nested_markers(doc: &mut designcraft_doc::Document, fields: &[DataField], row: &[String], maps: &Maps) { + let fill = |st: &mut Story| { + let found: Vec<(usize, usize, String)> = + find_markers(&st.text).into_iter().filter(|(_, _, name)| fields.iter().any(|f| f.name == *name)).collect(); + for (start, end, name) in found.into_iter().rev() { + st.replace(start..end, cell_of(fields, row, &name)); + } + }; + let has = |st: &Story| find_markers(&st.text).iter().any(|(_, _, name)| fields.iter().any(|f| f.name == *name)); + let mut mapped: Vec = maps.stories.values().copied().collect(); + mapped.sort(); + mapped.dedup(); + for sid in mapped { + let Some(st) = doc.story_mut(sid) else { continue }; + for t in st.tables.values_mut() { + if t.cells.iter().any(|c| nested_has(&c.text, &has)) { + for c in &mut Arc::make_mut(t).cells { + c.text.for_each_text_mut(&mut |x| fill(x)); + } + } + } + for n in &mut st.notes { + if nested_has(&n.text, &has) { + Arc::make_mut(n).text.for_each_text_mut(&mut |x| fill(x)); + } + } + } +} + +/// Whether `st` or a text inside it (its table cells and notes) satisfies `has`. +fn nested_has(st: &Story, has: &impl Fn(&Story) -> bool) -> bool { + has(st) || st.tables.values().any(|t| t.cells.iter().any(|c| nested_has(&c.text, has))) || st.notes.iter().any(|n| nested_has(&n.text, has)) +} + fn find_markers(text: &str) -> Vec<(usize, usize, String)> { let mut out = Vec::new(); let mut i = 0; diff --git a/crates/engine/src/cmd/datamerge/tests.rs b/crates/engine/src/cmd/datamerge/tests.rs index e2c76a90f..66eaea7de 100644 --- a/crates/engine/src/cmd/datamerge/tests.rs +++ b/crates/engine/src/cmd/datamerge/tests.rs @@ -1285,3 +1285,55 @@ fn merged_records_give_anchored_objects_their_own_ids() { doc.check().unwrap(); designcraft_format::load(&designcraft_format::save(doc).unwrap()).unwrap(); } + +/// The cell texts of the first table in the active document's first story that has one. +fn cell_texts(s: &Session) -> Vec { + let doc = &s.documents()[s.active_index().unwrap()].doc; + let t = doc.stories.values().find_map(|st| st.tables.values().next().cloned()).expect("a table"); + t.cells.iter().map(|c| c.text.text.clone()).collect() +} + +/// A field inserted with the insertion point in a table cell goes into that cell, and the +/// preview and the merge fill it. The cell's offsets used to put the marker in the table's own +/// story, outside the table, and the merge never read a cell's text (#2315). +#[test] +fn a_field_inserted_in_a_table_cell_goes_there_and_is_filled() { + let mut s = Session::new(); + new_doc(&mut s, 1); + select_csv(&mut s, "Name,City\nAda,London\nGrace,Arlington\n"); + let made = text_frame(&mut s, [36.0, 36.0, 400.0, 200.0], "Who\tWhere", None); + exec(&mut s, "text.select", json!({"story": made["story"], "anchor": 0, "focus": 9})); + exec(&mut s, "table.convertFromText", json!({"columnSeparator": "tab"})); + let story_text = |s: &Session| s.documents()[s.active_index().unwrap()].doc.stories.values().next().unwrap().text.clone(); + let before = story_text(&s); + exec(&mut s, "table.select", json!({"story": made["story"], "rows": [0, 0], "cols": [1, 1]})); + let r = exec(&mut s, "data.placeholder.add", json!({"field": "City"})); + assert_eq!(r["marker"], "<>", "{r}"); + assert_eq!(cell_texts(&s), ["Who", "<>Where"], "the marker is in the cell with the caret"); + assert_eq!(story_text(&s), before, "and not in the table's story"); + + exec(&mut s, "data.preview", json!({"record": 2})); + assert_eq!(cell_texts(&s), ["Who", "ArlingtonWhere"], "the preview fills the cell"); + exec(&mut s, "data.preview.stop", json!({})); + assert_eq!(cell_texts(&s), ["Who", "<>Where"], "stopping it puts the marker back"); + exec(&mut s, "data.merge", json!({})); + let merged = &s.documents()[s.active_index().unwrap()].doc; + let mut cities: Vec = + merged.stories.values().flat_map(|st| st.tables.values().map(|t| t.cells[1].text.text.clone()).collect::>()).collect(); + cities.sort(); + assert_eq!(cities, ["ArlingtonWhere", "LondonWhere"], "one table per record, filled"); +} + +/// A field typed (or inserted) in text that is then converted to a table is filled in its cell. +#[test] +fn a_field_converted_into_a_table_is_filled_in_its_cell() { + let mut s = Session::new(); + new_doc(&mut s, 1); + select_csv(&mut s, "Name,City\nAda,London\n"); + let made = text_frame(&mut s, [36.0, 36.0, 400.0, 200.0], "Name\t<>", None); + exec(&mut s, "text.select", json!({"story": made["story"], "anchor": 0, "focus": 13})); + exec(&mut s, "table.convertFromText", json!({"columnSeparator": "tab"})); + assert_eq!(cell_texts(&s), ["Name", "<>"]); + exec(&mut s, "data.preview", json!({"record": 1})); + assert_eq!(cell_texts(&s), ["Name", "Ada"], "the field is filled in its cell"); +}