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

@@ -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"));

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 `<<Field>>` 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<StoryId> = 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;

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<String> {
+    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"], "<<City>>", "{r}");
+    assert_eq!(cell_texts(&s), ["Who", "<<City>>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", "<<City>>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<String> =
+        merged.stories.values().flat_map(|st| st.tables.values().map(|t| t.cells[1].text.text.clone()).collect::<Vec<_>>()).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<<Name>>", 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", "<<Name>>"]);
+    exec(&mut s, "data.preview", json!({"record": 1}));
+    assert_eq!(cell_texts(&s), ["Name", "Ada"], "the field is filled in its cell");
+}