From 078b8bdcd2b1c87ada6e299a47e1d3dc37d145d7 Mon Sep 17 00:00:00 2001 From: Hamza Date: Thu, 5 Nov 2026 12:15:32 +0500 Subject: [PATCH 1/5] 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"); +} From 3eb83555820ebaf8df105e3284b273a619234b4e Mon Sep 17 00:00:00 2001 From: aiongg <67543387+aiongg@users.noreply.github.com> Date: Thu, 5 Nov 2026 07:42:43 +0000 Subject: [PATCH 2/5] Apply review fixes - crates/engine/src/cmd/datamerge/fill.rs --- crates/engine/src/cmd/datamerge/fill.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/engine/src/cmd/datamerge/fill.rs b/crates/engine/src/cmd/datamerge/fill.rs index 288dbf44f..fdde21825 100644 --- a/crates/engine/src/cmd/datamerge/fill.rs +++ b/crates/engine/src/cmd/datamerge/fill.rs @@ -349,6 +349,11 @@ fn fill_nested_markers(doc: &mut designcraft_doc::Document, fields: &[DataField] mapped.sort(); mapped.dedup(); for sid in mapped { + if !doc.story(sid).is_some_and(|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)) + }) { + continue; + } 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)) { From d22331c60f3c7518fdb7d9c861805a0bdc240da0 Mon Sep 17 00:00:00 2001 From: aiongg <67543387+aiongg@users.noreply.github.com> Date: Thu, 5 Nov 2026 07:50:55 +0000 Subject: [PATCH 3/5] Apply review fixes - crates/engine/src/cmd/datamerge.rs --- crates/engine/src/cmd/datamerge.rs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/cmd/datamerge.rs b/crates/engine/src/cmd/datamerge.rs index e834127fd..28c3be75f 100644 --- a/crates/engine/src/cmd/datamerge.rs +++ b/crates/engine/src/cmd/datamerge.rs @@ -434,7 +434,7 @@ fn overset_count(doc: &Document) -> usize { .count() } -fn add_placeholder(d: &mut Document, sel: &Selection, p: &Value, name: &str) -> Result { +fn add_placeholder(d: &mut Document, sel: &mut Selection, p: &Value, name: &str) -> Result { let (source_id, field) = { let source = d.data_merge.sources.first().ok_or_else(|| bad("data.placeholder.add", "select a data source"))?; let field = @@ -490,10 +490,21 @@ fn add_placeholder(d: &mut Document, sel: &Selection, p: &Value, name: &str) -> if role != PlaceholderRole::Text { return Err(bad("data.placeholder.add", "a hyperlink field can't go in a table cell; insert it as text")); } + if name.contains(['<', '>', '\n']) { + return Err(bad("data.placeholder.add", "this field's name can't be used in a table cell")); + } 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 cell = t.cell.map(|c| { + d.story(t.story).and_then(|st| st.tables.get(&c.table)).map_or(c, |tb| { + let (row, col) = tb.owner(c.row, c.col); + designcraft_doc::CellAddr { row, col, ..c } + }) + }); + let st = d.text_story_mut(t.story, 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); + let caret = at + marker.len(); + *sel = Selection::text(designcraft_doc::TextSel { anchor: caret, focus: caret, cell, ..t }); 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"))?; From 12106c76000f1a11858f89798853f29a734be39e Mon Sep 17 00:00:00 2001 From: Hamza Date: Thu, 5 Nov 2026 12:54:12 +0500 Subject: [PATCH 4/5] Data Merge: a field in a cell leaves the caret after it, goes into a merged cell, refuses a name a marker can't hold; endnotes are filled; the walk is bounded From review: after inserting a field in a cell the cell selection stayed, so the next keystroke replaced the cell; the caret now follows the field. A covered position resolves to its merged cell. A field name with < > or a line break can't be read back from a marker, so it isn't inserted in a cell. Endnote texts are filled like footnotes, and nested texts are followed to MAX_NESTED_DEPTH (32). --- crates/engine/src/cmd/datamerge.rs | 47 ++++++---- crates/engine/src/cmd/datamerge/fill.rs | 80 ++++++++++------ crates/engine/src/cmd/datamerge/tests.rs | 113 +++++++++++++++++++++++ 3 files changed, 195 insertions(+), 45 deletions(-) diff --git a/crates/engine/src/cmd/datamerge.rs b/crates/engine/src/cmd/datamerge.rs index 28c3be75f..28855d8ea 100644 --- a/crates/engine/src/cmd/datamerge.rs +++ b/crates/engine/src/cmd/datamerge.rs @@ -482,29 +482,40 @@ fn add_placeholder(d: &mut Document, sel: &mut 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()) { + // The insertion point is in a table cell or a footnote: the field goes into that text as + // its marker, which the merge fills as it fills a typed one. Such a text is a story of its + // own that a placeholder can't name, and the selection's offsets are its own, so they used + // to put the marker in the table's story instead (#2315). + if !explicit_story + && let Some(t) = sel.text + && let Some(pressed) = t.cell + { if role != PlaceholderRole::Text { - return Err(bad("data.placeholder.add", "a hyperlink field can't go in a table cell; insert it as text")); + return Err(bad("data.placeholder.add", "a hyperlink field can't go in a table cell or a note; insert it as text")); } - if name.contains(['<', '>', '\n']) { - return Err(bad("data.placeholder.add", "this field's name can't be used in a table cell")); + // The merge finds a marker by its name, so a name that can't be read back out of one + // would stay a marker: say so rather than insert it. + if name.is_empty() || name.contains(['<', '>', '\n']) { + return Err(bad( + "data.placeholder.add", + format!("the field \"{name}\" can't be inserted in a table cell or a note: its name holds < > or a line break"), + )); } - let marker = format!("<<{name}>>"); - let cell = t.cell.map(|c| { - d.story(t.story).and_then(|st| st.tables.get(&c.table)).map_or(c, |tb| { - let (row, col) = tb.owner(c.row, c.col); - designcraft_doc::CellAddr { row, col, ..c } - }) + // A position a merged cell covers is the merged cell's: the field goes where it shows. + let cell = d.story(t.story).and_then(|st| st.tables.get(&pressed.table)).map_or(pressed, |tab| { + let (row, col) = tab.owner(pressed.row, pressed.col); + designcraft_doc::CellAddr { row, col, ..pressed } }); - let st = d.text_story_mut(t.story, 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())); + let marker = format!("<<{name}>>"); + let st = d.text_story_mut(t.story, Some(cell)).ok_or_else(|| bad("data.placeholder.add", "no table cell there"))?; + let at = if cell == pressed { t.range().start.min(st.text.len()) } else { st.text.len() }; + let at = designcraft_doc::floor_char_boundary(&st.text, at); st.replace(at..at, &marker); - let caret = at + marker.len(); - *sel = Selection::text(designcraft_doc::TextSel { anchor: caret, focus: caret, cell, ..t }); + // The caret goes after the field, in that text, as after typing; a cell selection + // would have the next keystroke replace the whole cell, field and all. + let end = at + marker.len(); + sel.cells = None; + sel.text = Some(designcraft_doc::TextSel { anchor: end, focus: end, cell: Some(cell), ..t }); 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"))?; diff --git a/crates/engine/src/cmd/datamerge/fill.rs b/crates/engine/src/cmd/datamerge/fill.rs index fdde21825..38da7eada 100644 --- a/crates/engine/src/cmd/datamerge/fill.rs +++ b/crates/engine/src/cmd/datamerge/fill.rs @@ -332,47 +332,73 @@ 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. +/// How deep the merge follows texts inside texts (a table in a cell of a table, a note in a cell). +/// Documents don't come anywhere near it; a file that goes deeper is filled down to here instead of +/// exhausting the stack. +pub(super) const MAX_NESTED_DEPTH: usize = 32; + +/// The `<>` markers in the table cells, footnotes and endnotes of the stories being filled. +/// Those texts are stories inside their story, which no placeholder can name and `typed_reps` +/// doesn't read, so a field in a table was left as its marker (#2315). A story, table or note is +/// copied only when something inside it holds a marker of a known field. 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 { - if !doc.story(sid).is_some_and(|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)) - }) { + if !doc.story(sid).is_some_and(|st| inner_has(st, fields, 0)) { continue; } 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)); - } + fill_inner(st, fields, row, 0); + } +} + +/// Whether `st`'s own text holds a marker of a known field. +fn own_has(st: &Story, fields: &[DataField]) -> bool { + find_markers(&st.text).iter().any(|(_, _, name)| fields.iter().any(|f| f.name == *name)) +} + +/// Whether a text inside `st` (a table cell, footnote or endnote, or one inside those), not `st`'s +/// own text, holds a marker of a known field. +fn inner_has(st: &Story, fields: &[DataField], depth: usize) -> bool { + if depth >= MAX_NESTED_DEPTH { + return false; + } + let holds = |t: &Story| own_has(t, fields) || inner_has(t, fields, depth + 1); + st.tables.values().any(|t| t.cells.iter().any(|c| holds(&c.text))) || st.notes.iter().chain(&st.endnotes).any(|n| holds(&n.text)) +} + +/// Fill the known fields' markers in the texts inside `st`, not in its own text. +fn fill_inner(st: &mut Story, fields: &[DataField], row: &[String], depth: usize) { + if depth >= MAX_NESTED_DEPTH { + return; + } + let holds = |t: &Story| own_has(t, fields) || inner_has(t, fields, depth + 1); + for t in st.tables.values_mut() { + if t.cells.iter().any(|c| holds(&c.text)) { + for c in &mut Arc::make_mut(t).cells { + fill_own(&mut c.text, fields, row); + fill_inner(&mut c.text, fields, row, depth + 1); } } - 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)); - } + } + for n in st.notes.iter_mut().chain(st.endnotes.iter_mut()) { + if holds(&n.text) { + let n = Arc::make_mut(n); + fill_own(&mut n.text, fields, row); + fill_inner(&mut n.text, fields, row, depth + 1); } } } -/// 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)) +/// Replace the markers of known fields in `st`'s own text with the record's values. +fn fill_own(st: &mut Story, fields: &[DataField], row: &[String]) { + 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)); + } } fn find_markers(text: &str) -> Vec<(usize, usize, String)> { diff --git a/crates/engine/src/cmd/datamerge/tests.rs b/crates/engine/src/cmd/datamerge/tests.rs index 66eaea7de..119e159d2 100644 --- a/crates/engine/src/cmd/datamerge/tests.rs +++ b/crates/engine/src/cmd/datamerge/tests.rs @@ -1337,3 +1337,116 @@ fn a_field_converted_into_a_table_is_filled_in_its_cell() { exec(&mut s, "data.preview", json!({"record": 1})); assert_eq!(cell_texts(&s), ["Name", "Ada"], "the field is filled in its cell"); } + +/// A table `Who⇥Where` with the caret in its second cell, and a source with `Name` and `City`. +fn table_with_caret_in_cell(s: &mut Session, csv: &str) -> Value { + new_doc(s, 1); + select_csv(s, csv); + let made = text_frame(s, [36.0, 36.0, 400.0, 200.0], "Who\tWhere", None); + exec(s, "text.select", json!({"story": made["story"], "anchor": 0, "focus": 9})); + exec(s, "table.convertFromText", json!({"columnSeparator": "tab"})); + exec(s, "table.select", json!({"story": made["story"], "rows": [0, 0], "cols": [1, 1]})); + made +} + +/// After a field goes into a cell the caret is after it, in that cell, and the cell selection is +/// gone: typing goes on after the field instead of replacing the cell, field and all. +#[test] +fn typing_after_a_field_inserted_in_a_cell_goes_on_after_it() { + let mut s = Session::new(); + table_with_caret_in_cell(&mut s, "Name,City\nAda,London\n"); + exec(&mut s, "data.placeholder.add", json!({"field": "City"})); + let sel = &s.documents()[s.active_index().unwrap()].selection; + assert!(sel.cells.is_none(), "no cell selection left"); + exec(&mut s, "text.insert", json!({"text": ", "})); + assert_eq!(cell_texts(&s), ["Who", "<>, Where"]); +} + +/// A position that a merged cell covers is the merged cell's: the field goes into the merged cell, +/// where it shows, not into the hidden one under it. +#[test] +fn a_field_inserted_in_a_merged_cell_goes_where_it_shows() { + let mut s = Session::new(); + let made = table_with_caret_in_cell(&mut s, "Name,City\nAda,London\n"); + exec(&mut s, "table.merge", json!({"story": made["story"], "rows": [0, 0], "cols": [0, 1]})); + exec(&mut s, "table.select", json!({"story": made["story"], "rows": [0, 0], "cols": [1, 1]})); + exec(&mut s, "data.placeholder.add", json!({"field": "City"})); + let cells = cell_texts(&s); + assert!(cells[0].contains("<>"), "in the merged cell: {cells:?}"); + assert!(!cells[1].contains("<>"), "not in the covered one: {cells:?}"); + exec(&mut s, "data.preview", json!({"record": 1})); + assert!(cell_texts(&s)[0].contains("London"), "and filled: {:?}", cell_texts(&s)); +} + +/// A field whose name a marker can't hold (it has `<` or `>`) can't be filled from a cell, so it +/// isn't inserted there; in body text it still becomes an anchored placeholder. +#[test] +fn a_field_named_with_angle_brackets_is_refused_in_a_cell() { + let mut s = Session::new(); + let made = table_with_caret_in_cell(&mut s, "Customer ,City\n7,London\n"); + let e = s.execute("data.placeholder.add", &json!({"field": "Customer "})).expect_err("refused").to_string(); + assert!(e.contains("can't be inserted in a table cell"), "{e}"); + assert_eq!(cell_texts(&s), ["Who", "Where"], "nothing inserted"); + let other = text_frame(&mut s, [36.0, 300.0, 400.0, 400.0], "No. ", None); + exec(&mut s, "text.select", json!({"story": other["story"], "anchor": 4, "focus": 4})); + assert!(exec(&mut s, "data.placeholder.add", json!({"field": "Customer "}))["id"].is_u64(), "body text still takes it"); + let _ = made; +} + +/// 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() { + 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], "See the note", None); + exec(&mut s, "text.select", json!({"story": made["story"], "anchor": 12, "focus": 12})); + exec(&mut s, "endnote.insert", json!({"text": "From <>"})); + let endnotes = |s: &Session| -> Vec { + s.documents()[s.active_index().unwrap()].doc.stories.values().flat_map(|st| st.endnotes.iter().map(|e| e.text.text.clone())).collect() + }; + assert_eq!(endnotes(&s), ["From <>"]); + exec(&mut s, "data.preview", json!({"record": 1})); + assert_eq!(endnotes(&s), ["From London"]); +} + +/// Tables nested in cells far deeper than any document goes are filled down to the merge's depth +/// limit, and the preview returns instead of exhausting the stack. +#[test] +fn deeply_nested_tables_are_filled_down_to_the_limit_without_overflowing() { + use designcraft_doc::{Cell, ParaFormat, Table}; + 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], "x", None); + let sid = StoryId(made["story"].as_u64().unwrap()); + // `depth` tables, each in the one cell of the next, each cell's own text `<>`. + let nest = |depth: usize| { + let mut inner: Option = None; + for i in 0..depth { + let mut t = Table::new(1000 + i as u64, 1, 1, 0, 0, 100.0); + t.cells[0] = Cell::with_text("<>", ParaFormat::default()); + if let Some(child) = inner.take() { + t.cells[0].text.insert_table(0, child); + } + inner = Some(t); + } + inner.unwrap() + }; + s.edit(|d, _| { + d.story_mut(sid).unwrap().insert_table(0, nest(500)); + Ok(()) + }) + .unwrap(); + exec(&mut s, "data.preview", json!({"record": 1})); + let doc = &s.documents()[s.active_index().unwrap()].doc; + let mut t = doc.story(sid).unwrap().tables.values().next().unwrap().clone(); + let mut filled = Vec::new(); + for _ in 0..40 { + filled.push(t.cells[0].text.text.contains("Ada")); + let Some(next) = t.cells[0].text.tables.values().next().cloned() else { break }; + t = next; + } + assert!(filled[..super::fill::MAX_NESTED_DEPTH].iter().all(|f| *f), "filled down to the limit: {filled:?}"); + assert!(!filled[super::fill::MAX_NESTED_DEPTH], "and not past it"); +} From 72877fae246ca6c206835488ebd22b1571f4fc81 Mon Sep 17 00:00:00 2001 From: aiongg <67543387+aiongg@users.noreply.github.com> Date: Thu, 5 Nov 2026 08:09:17 +0000 Subject: [PATCH 5/5] Apply review fixes - crates/engine/src/cmd/datamerge.rs --- crates/engine/src/cmd/datamerge.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/engine/src/cmd/datamerge.rs b/crates/engine/src/cmd/datamerge.rs index 28855d8ea..f8d6d12a3 100644 --- a/crates/engine/src/cmd/datamerge.rs +++ b/crates/engine/src/cmd/datamerge.rs @@ -60,7 +60,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}; with the insertion point in a table cell or a note → {cell: true, marker} (a typed marker, not an anchored placeholder)", has_doc, placeholder_add ),