Fix #5559 follow-ups: generated list prefix, hidden-text objects, turned clipped lines, kept-markup checks once per export

- update_generated keeps what precedes a wrapped list's field (r4468529243)
- from_doc keeps objects in hidden text when hidden text is written (r4469018820)
- clipped cell lines in a turned frame are drawn once, clipped, turned (r4469018900)
- kept::while_writing works out reply map and page fingerprint once per export (r4468529227)
- odt_charts fixture declares its namespaces (r4468529259)
- guard test for character styles based on a missing parent (r4469018843, not a bug)

crates/doc/src/kept.rs

@@ -263,7 +263,9 @@ impl KeptXml {
     }
     /// Whether `doc`'s page setup is still as it was read.
     pub fn pages_as_read(&self, doc: &crate::Document) -> bool {
-        self.pages_fp.is_some_and(|fp| fp == pages_fingerprint(doc))
+        let Some(fp) = self.pages_fp else { return false };
+        let now = in_scope(doc, |sc| *sc.pages.get_or_insert_with(|| pages_fingerprint(doc))).unwrap_or_else(|| pages_fingerprint(doc));
+        fp == now
     }
     /// Whether `doc`'s style definitions are still as they were read.
     pub fn styles_as_read(&self, doc: &crate::Document) -> bool {
@@ -384,11 +386,62 @@ const MAX_REPLIES: usize = 10_000;
 /// What comment `root` is with its replies (and theirs): each one's author, date, state and
 /// text, and how many they are.
 pub fn comments_fingerprint(doc: &crate::Document, root: u32) -> (u64, usize) {
-    family_fingerprint(doc, root, &replies(doc))
+    family_fingerprint(doc, root, &scoped_replies(doc))
+}
+
+/// What a writer asks of a document's kept markup for many paragraphs, worked out once while
+/// [`while_writing`] it: each comment's replies, and its page setup's fingerprint. Without it a
+/// document of C comments took C² steps (and its page setup serialized once per note) to save.
+struct Scope {
+    /// The document's address: it can't move or change while borrowed for the writing.
+    doc: usize,
+    replies: Option<std::rc::Rc<HashMap<u32, Vec<u32>>>>,
+    pages: Option<u64>,
+}
+
+thread_local! {
+    static SCOPE: std::cell::RefCell<Option<Scope>> = const { std::cell::RefCell::new(None) };
+}
+
+#[cfg(test)]
+thread_local! {
+    /// How many times [`replies`] was built (tests).
+    static REPLY_BUILDS: std::cell::Cell<usize> = const { std::cell::Cell::new(0) };
+}
+
+/// Puts the scope before it back when dropped (when `f` returns or unwinds).
+struct Restore(Option<Scope>);
+
+impl Drop for Restore {
+    fn drop(&mut self) {
+        let prev = self.0.take();
+        SCOPE.with(|s| *s.borrow_mut() = prev);
+    }
+}
+
+/// Run `f`, which writes `doc`, with what the kept markup asks of `doc` worked out at most once.
+pub fn while_writing<R>(doc: &crate::Document, f: impl FnOnce() -> R) -> R {
+    let key = std::ptr::from_ref(doc) as usize;
+    let prev = SCOPE.with(|s| s.replace(Some(Scope { doc: key, replies: None, pages: None })));
+    let _restore = Restore(prev);
+    f()
+}
+
+/// The scope for `doc`, if it is being written: `f` of it.
+fn in_scope<R>(doc: &crate::Document, f: impl FnOnce(&mut Scope) -> R) -> Option<R> {
+    let key = std::ptr::from_ref(doc) as usize;
+    SCOPE.with(|s| s.borrow_mut().as_mut().filter(|sc| sc.doc == key).map(f))
+}
+
+/// [`replies`], built once while `doc` is being written.
+fn scoped_replies(doc: &crate::Document) -> std::rc::Rc<HashMap<u32, Vec<u32>>> {
+    in_scope(doc, |sc| sc.replies.get_or_insert_with(|| std::rc::Rc::new(replies(doc))).clone()).unwrap_or_else(|| std::rc::Rc::new(replies(doc)))
 }
 
 /// Each comment's replies, by the comment's id.
 fn replies(doc: &crate::Document) -> HashMap<u32, Vec<u32>> {
+    #[cfg(test)]
+    REPLY_BUILDS.with(|n| n.set(n.get() + 1));
     let mut out: HashMap<u32, Vec<u32>> = HashMap::new();
     for (id, c) in &doc.comments {
         if let Some(p) = c.parent {
@@ -694,4 +747,34 @@ mod tests {
         assert_eq!(k.group(p), Some((1, 1)));
         assert_eq!(k.story_fps.get(&id), Some(&story_fingerprint(&doc, id)));
     }
+
+    /// Writing a document checks each kept paragraph's comments against what was read: the
+    /// comments' replies are looked up once for the whole document, not once per comment.
+    #[test]
+    fn writing_many_comments_looks_their_replies_up_once() {
+        use crate::para::InlineObject;
+        let n = 50u32;
+        let text = (0..n).map(|i| format!("Para {i}")).collect::<Vec<_>>().join("\n");
+        let mut doc = crate::Document::from_text(&text);
+        for i in 0..n {
+            let part = doc.add_part(crate::PartKind::Comment, vec![crate::para_block(Paragraph::with_text("Note", Default::default()))]);
+            doc.comments.insert(i, crate::Comment { parent: None, part, ..Default::default() });
+            doc.para_mut(crate::StoryRef::Body, &crate::Path::top(i as usize))
+                .unwrap()
+                .insert_object(0, InlineObject::CommentStart { id: i }, &Default::default())
+                .unwrap();
+        }
+        let mut k = KeptXml { format: "odt".into(), ..Default::default() };
+        for i in 0..n {
+            k.comment_fps.insert(i, comments_fingerprint(&doc, i).0);
+        }
+        let xml = "<text:p><office:annotation>x</office:annotation></text:p>";
+        let before = REPLY_BUILDS.with(std::cell::Cell::get);
+        let all = while_writing(&doc, || doc.body.iter().filter_map(|b| b.as_para()).all(|p| k.comments_as_read(&doc, p, xml)));
+        assert!(all, "every paragraph's comment is as read");
+        assert_eq!(REPLY_BUILDS.with(std::cell::Cell::get) - before, 1);
+        // Outside a writing, nothing is kept between calls.
+        assert!(doc.body.first().and_then(|b| b.as_para()).is_some_and(|p| k.comments_as_read(&doc, p, xml)));
+        assert_eq!(REPLY_BUILDS.with(std::cell::Cell::get) - before, 2);
+    }
 }

crates/engine/src/cmd/citations/generated.rs

@@ -72,7 +72,7 @@ const MAX_WRAPPED: usize = 10_000;
 /// A generated list (index, bibliography, table of figures or authorities) held as a complex
 /// field that starts at body block `i` and runs over its entries: its field code and the block
 /// it ends in. `None` for anything else (a TOC has its own update).
-fn wrapped_list(s: &Session, i: usize) -> Option<(String, usize, usize)> {
+fn wrapped_list(s: &Session, i: usize) -> Option<(String, usize, usize, usize)> {
     let p = s.doc.body.get(i)?.as_para()?;
     let (k, off, instr) = p.objects_with_offsets().enumerate().find_map(|(k, (off, o))| match o {
         InlineObject::FieldStart { instr, .. } => {
@@ -97,7 +97,7 @@ fn wrapped_list(s: &Session, i: usize) -> Option<(String, usize, usize)> {
                 InlineObject::FieldEnd => {
                     depth = depth.saturating_sub(1);
                     if depth == 0 {
-                        return Some((instr, j, at + wordcraft_doc::para::OBJ.len_utf8()));
+                        return Some((instr, off, j, at + wordcraft_doc::para::OBJ.len_utf8()));
                     }
                 }
                 _ => {}
@@ -198,12 +198,13 @@ pub fn update_generated(s: &mut Session) -> Result<(), CmdError> {
         // field's own paragraph, as WordCraft inserts one, its old entries rebuilt below.
         // What follows its closing field end, and its first paragraph's properties and section
         // break, are the document's and stay.
-        if let Some((instr, last, end)) = wrapped_list(s, i) {
+        if let Some((instr, start, last, end)) = wrapped_list(s, i) {
             // The entries between its first and last paragraphs go.
             for _ in i.saturating_add(1)..last {
                 s.doc.remove_block(StoryRef::Body, &Path::top(i + 1))?;
             }
-            // Its first paragraph keeps only the field; its last (or, ending in its first, the
+            // Its first paragraph keeps what comes before the field (bookmarks, other fields)
+            // and the field; its last (or, ending in its first, the
             // same) loses what is up to and through the field end, and goes when nothing is left
             // of it and it ends no section.
             let first_end = if last > i {
@@ -217,15 +218,15 @@ pub fn update_generated(s: &mut Session) -> Result<(), CmdError> {
                 end
             };
             let p = s.doc.para_mut(StoryRef::Body, &Path::top(i))?;
-            p.delete(0, first_end)?;
+            p.delete(start, first_end)?;
             p.insert_object(
-                0,
+                start,
                 InlineObject::Field { instr, result: String::new(), locked: false, dirty: false, code: false, form: String::new() },
                 &Default::default(),
             )?;
             // What followed the field end in its own paragraph comes after the list, in a
             // paragraph of its own that takes the section break.
-            let field = wordcraft_doc::para::OBJ.len_utf8();
+            let field = start + wordcraft_doc::para::OBJ.len_utf8();
             if p.len() > field {
                 let rest = p.split_off(field)?;
                 s.doc.insert_block(StoryRef::Body, &Path::top(i + 1), Block::Para(rest))?;
@@ -432,7 +433,7 @@ mod tests {
             p.insert_object(n, InlineObject::FieldEnd, &props).unwrap();
         }
         let s = Session::new(d);
-        assert_eq!(wrapped_list(&s, 0).map(|(instr, last, _)| (instr, last)), Some(("BIBLIOGRAPHY".to_string(), 1)));
+        assert_eq!(wrapped_list(&s, 0).map(|(instr, _, last, _)| (instr, last)), Some(("BIBLIOGRAPHY".to_string(), 1)));
     }
 
     /// A wrapped list as a session holds it: the field starting paragraph `first`'s text, ending
@@ -487,4 +488,35 @@ mod tests {
         assert_eq!(t.get(1).map(String::as_str), Some(" Then this"), "{t:?}");
         assert_eq!(t.last().map(String::as_str), Some("Below"));
     }
+
+    /// What comes before a wrapped list's field in its first paragraph (a bookmark, an empty
+    /// `PAGE` field) stays there, before the rebuilt list's field.
+    #[test]
+    fn what_comes_before_a_wrapped_lists_field_stays() {
+        let mut s = wrapped("Old one\nOld two\nBelow", 0, 1, "Old two".len());
+        let props = CharProps::default();
+        if let Ok(p) = s.doc.para_mut(StoryRef::Body, &Path::top(0)) {
+            p.insert_object(0, InlineObject::FieldEnd, &props).unwrap();
+            p.insert_object(0, InlineObject::FieldStart { instr: "PAGE".into(), locked: false, dirty: false }, &props).unwrap();
+            p.insert_object(0, InlineObject::BookmarkStart { name: "Sources".into() }, &props).unwrap();
+        }
+        update_generated(&mut s).unwrap();
+        let first = s.doc.body.first().and_then(|b| b.as_para());
+        let kinds: Vec<&str> = first
+            .map(|p| {
+                p.objects
+                    .iter()
+                    .map(|o| match o {
+                        InlineObject::BookmarkStart { .. } => "bookmark",
+                        InlineObject::FieldStart { instr, .. } if instr == "PAGE" => "page",
+                        InlineObject::FieldEnd => "end",
+                        InlineObject::Field { instr, .. } if instr.starts_with("BIBLIOGRAPHY") => "list",
+                        _ => "other",
+                    })
+                    .collect()
+            })
+            .unwrap_or_default();
+        assert_eq!(kinds, ["bookmark", "page", "end", "list"]);
+        assert_eq!(texts(&s), ["", "Below"]);
+    }
 }

crates/engine/tests/it/odt_charts.rs

@@ -15,7 +15,8 @@ fn odt() -> Vec<u8> {
     let content = format!(
         r#"<office:document-content {NS}><office:body><office:text><text:p>Sales <draw:frame svg:width="4in" svg:height="2.5in"><draw:object xlink:href="./Object 1"/></draw:frame></text:p></office:text></office:body></office:document-content>"#
     );
-    let chart = r#"<office:document-content><office:body><office:chart><chart:chart chart:class="chart:line"><chart:title><text:p>Visits</text:p></chart:title><chart:plot-area><chart:axis chart:dimension="x"><chart:categories table:cell-range-address="local-table.$A$2:.$A$4"/></chart:axis><chart:series chart:values-cell-range-address="local-table.$B$2:.$B$4" chart:label-cell-address="local-table.$B$1"/></chart:plot-area><table:table table:name="local-table"><table:table-header-rows><table:table-row><table:table-cell/><table:table-cell office:value-type="string"><text:p>Site</text:p></table:table-cell></table:table-row></table:table-header-rows><table:table-rows><table:table-row><table:table-cell office:value-type="string"><text:p>Mon</text:p></table:table-cell><table:table-cell office:value-type="float" office:value="3"/></table:table-row><table:table-row><table:table-cell office:value-type="string"><text:p>Tue</text:p></table:table-cell><table:table-cell office:value-type="float" office:value="5"/></table:table-row><table:table-row><table:table-cell office:value-type="string"><text:p>Wed</text:p></table:table-cell><table:table-cell office:value-type="float" office:value="4"/></table:table-row></table:table-rows></table:table></chart:chart></office:chart></office:body></office:document-content>"#;
+    // A well-formed chart part, its prefixes declared as LibreOffice writes them.
+    let chart = r#"<office:document-content xmlns:office="urn:oasis:names:tc:opendocument:xmlns:office:1.0" xmlns:chart="urn:oasis:names:tc:opendocument:xmlns:chart:1.0" xmlns:text="urn:oasis:names:tc:opendocument:xmlns:text:1.0" xmlns:table="urn:oasis:names:tc:opendocument:xmlns:table:1.0"><office:body><office:chart><chart:chart chart:class="chart:line"><chart:title><text:p>Visits</text:p></chart:title><chart:plot-area><chart:axis chart:dimension="x"><chart:categories table:cell-range-address="local-table.$A$2:.$A$4"/></chart:axis><chart:series chart:values-cell-range-address="local-table.$B$2:.$B$4" chart:label-cell-address="local-table.$B$1"/></chart:plot-area><table:table table:name="local-table"><table:table-header-rows><table:table-row><table:table-cell/><table:table-cell office:value-type="string"><text:p>Site</text:p></table:table-cell></table:table-row></table:table-header-rows><table:table-rows><table:table-row><table:table-cell office:value-type="string"><text:p>Mon</text:p></table:table-cell><table:table-cell office:value-type="float" office:value="3"/></table:table-row><table:table-row><table:table-cell office:value-type="string"><text:p>Tue</text:p></table:table-cell><table:table-cell office:value-type="float" office:value="5"/></table:table-row><table:table-row><table:table-cell office:value-type="string"><text:p>Wed</text:p></table:table-cell><table:table-cell office:value-type="float" office:value="4"/></table:table-row></table:table-rows></table:table></chart:chart></office:chart></office:body></office:document-content>"#;
     let mut z = zip::ZipWriter::new(std::io::Cursor::new(Vec::new()));
     let o = zip::write::SimpleFileOptions::default().compression_method(zip::CompressionMethod::Stored);
     for (name, data) in [("mimetype", "application/vnd.oasis.opendocument.text"), ("content.xml", content.as_str()), ("Object 1/content.xml", chart)]

crates/formats/src/model/from_doc.rs

@@ -494,6 +494,7 @@ fn flow_paras_in(doc: &Document, p: &Paragraph, opts: FlowOpts) -> Vec<Para> {
         let hidden = props.hidden.unwrap_or(false) || props.del.is_some();
         // Formats that keep hidden text get it, marked hidden.
         let hidden_text = (props.hidden.unwrap_or(false) && !opts.chars) || (props.del.is_some() && !tracked);
+        let hidden_obj = (props.hidden.unwrap_or(false) && !opts.chars) || props.del.is_some();
         let mut f = run_fmt(doc, props, in_code, opts);
         if tracked {
             (f.ins, f.del) = rev_marks(doc, props);
@@ -541,8 +542,9 @@ fn flow_paras_in(doc: &Document, p: &Paragraph, opts: FlowOpts) -> Vec<Para> {
             let obj = p.objects.get(k);
             k += 1;
             control.interrupt(obj, &mut out, &f);
-            // Hidden text, or a hidden drawing: not in the copy.
-            if hidden || obj.and_then(InlineObject::frame).is_some_and(|(_, _, f)| f.hidden) {
+            // Hidden text a format leaves out, a deleted object, or a hidden drawing: not in the
+            // copy (a format keeping hidden text keeps its objects too, as it keeps the text).
+            if hidden_obj || obj.and_then(InlineObject::frame).is_some_and(|(_, _, f)| f.hidden) {
                 continue;
             }
             let from = out.inlines.len();
@@ -1146,7 +1148,9 @@ pub fn from_doc_tracked(doc: &Document) -> Flow {
 
 /// [`from_doc`] keeping what `opts` asks for.
 pub fn from_doc_with(doc: &Document, opts: FlowOpts) -> Flow {
-    flow_of(doc, opts)
+    // Paragraphs kept as read are checked against the document's comments and page setup:
+    // worked out once for the whole document, not once per paragraph.
+    wordcraft_doc::kept::while_writing(doc, || flow_of(doc, opts))
 }
 
 /// `blocks` of `doc` (a header, footer or note) as flow blocks, keeping what `opts` asks for.

crates/formats/src/odt/tests_more/char_styles.rs

@@ -71,3 +71,21 @@ fn what_a_character_style_turns_off_stays_off() {
     let plain = s.split(r#"style:name="Plain""#).nth(1).and_then(|r| r.split("</style:style>").next()).unwrap_or("missing");
     assert!(plain.contains(r#"fo:font-weight="normal""#) && plain.contains(r#"fo:font-style="normal""#), "{plain}");
 }
+
+#[test]
+fn a_character_style_based_on_a_missing_one_keeps_its_own_look() {
+    // "Orphan" names a parent the file doesn't define; "Tilted" comes after it.
+    let styles = format!(
+        r#"<office:document-styles {NS_T}><office:styles><style:style style:name="Orphan" style:family="text" style:parent-style-name="Nowhere"><style:text-properties fo:font-weight="bold"/></style:style><style:style style:name="Tilted" style:family="text"><style:text-properties fo:font-style="italic"/></style:style></office:styles></office:document-styles>"#
+    );
+    let content = format!(
+        r#"<office:document-content {NS_T}><office:body><office:text><text:p><text:span text:style-name="Orphan">aa</text:span> <text:span text:style-name="Tilted">bb</text:span></text:p></office:text></office:body></office:document-content>"#
+    );
+    let d = import(&package(&content, Some(&styles))).unwrap();
+    let r = runs(&d);
+    assert_eq!(
+        r.iter().map(|x| (x.0.as_str(), x.1.as_deref(), x.2, x.3)).collect::<Vec<_>>(),
+        [("aa", Some("Orphan"), true, false), ("bb", Some("Tilted"), false, true)]
+    );
+    assert!(d.styles.get("Nowhere").is_none(), "no style for the missing parent");
+}

crates/formats/src/odt/tests_more/chars.rs

@@ -268,3 +268,19 @@ fn emphasis_marks_are_read_and_written() {
     let back: Vec<Option<E>> = back.body[0].as_para().unwrap().runs.iter().map(|r| r.props.emphasis).collect();
     assert_eq!(back, [Some(E::Dot), Some(E::Comma), Some(E::Circle), Some(E::UnderDot), Some(E::Dot), None]);
 }
+
+#[test]
+fn objects_in_hidden_text_are_kept() {
+    // Hidden text is written, marked hidden: so are the bookmarks and fields it holds.
+    let hidden = CharProps { hidden: Some(true), ..Default::default() };
+    let mut p = Paragraph::with_text("Shown. Hidden words.", CharProps::default());
+    let n = p.len();
+    p.format(7, n, &|c| c.hidden = Some(true)).unwrap();
+    p.insert_object(14, wordcraft_doc::para::InlineObject::BookmarkStart { name: "Secret".into() }, &hidden).unwrap();
+    let mut d = Document::new();
+    d.body = vec![para_block(p)];
+    let back = import(&export(&d).unwrap()).unwrap();
+    let p = back.body[0].as_para().unwrap();
+    assert!(p.text.contains("Hidden"), "{:?}", p.text);
+    assert!(p.objects.iter().any(|o| matches!(o, wordcraft_doc::para::InlineObject::BookmarkStart { name } if name == "Secret")), "{:?}", p.objects);
+}

crates/layout/src/display/page.rs

@@ -133,8 +133,11 @@ fn stories_display(
             if hidden.as_ref().is_some_and(|h| h.hides(it)) {
                 continue;
             }
+            // A line in a clipped cell is drawn in its clip; in a turned text box, with the box's
+            // other lines (clipped there), so it is drawn once.
             if let Placed::Lines { story, path, .. } = it
                 && let Some(rect) = cell_clip(clips, *story, &path.0)
+                && turned_frame(&frames, it).is_none()
             {
                 let mut inner = Vec::new();
                 item(doc, it, opts, authors, a, &mut inner);
@@ -166,7 +169,15 @@ fn stories_display(
                         if let Some((_, rect, spin)) = frames.get(k) {
                             let mut inner = Vec::new();
                             for line in items.iter().filter(|i| turned_frame(&frames, i) == Some(k)) {
-                                item(doc, line, opts, authors, a, &mut inner);
+                                if let Placed::Lines { story, path, .. } = line
+                                    && let Some(rect) = cell_clip(clips, *story, &path.0)
+                                {
+                                    let mut clipped = Vec::new();
+                                    item(doc, line, opts, authors, a, &mut clipped);
+                                    inner.push(Draw::Clip { rect, items: clipped });
+                                } else {
+                                    item(doc, line, opts, authors, a, &mut inner);
+                                }
                             }
                             super::objects::spun(*spin, *rect, inner, out);
                         }

crates/layout/src/tests/textbox.rs

@@ -358,3 +358,45 @@ fn a_turned_linked_box_turns_only_its_own_lines() {
     let items: Vec<display::Draw> = l.pages[0].items.iter().flat_map(|it| shown.draws(it, &Default::default())).collect();
     assert_eq!(turned_texts(&items), (true, true));
 }
+
+#[test]
+fn a_clipped_line_in_a_turned_box_is_drawn_once_turned_and_clipped() {
+    // Lines in a cell cut off at its edge (an exact-height row) inside a turned text box: drawn
+    // once, clipped, inside the box's turn (it was drawn upright and clipped, and again turned).
+    let mut d = Document::from_text("one");
+    let text = "Words in the box.";
+    let float = wordcraft_doc::para::Float { wrap: wordcraft_doc::para::Wrap::Square, x: 20.0, y: 100.0, rot: 30.0, ..Default::default() };
+    let b = text_box_with(&mut d, 0, text, 144.0, 200.0, TextBody::default(), float);
+    let l = lay(&d);
+    let mut page = l.pages[0].clone();
+    let in_box: Vec<usize> = page
+        .items
+        .iter()
+        .enumerate()
+        .filter(|(_, it)| matches!(it, Placed::Lines { story: StoryRef::Part(s), .. } if *s == b))
+        .map(|(i, _)| i)
+        .collect();
+    // Its lines taken as ones in a clipped cell (table 5, row 0, cell 0).
+    let Some(Placed::Lines { path, .. }) = in_box.first().and_then(|&i| page.items.get_mut(i)) else { panic!("the box has no lines") };
+    *path = wordcraft_doc::Path(vec![5, 0, 0, 0]);
+    let rect = Rect::new(0.0, 0.0, 1000.0, 1000.0);
+    page.items.push(Placed::Cell {
+        rect,
+        covered: false,
+        table: wordcraft_doc::Path(vec![5]),
+        row: 0,
+        cell: 0,
+        story: StoryRef::Part(b),
+        clip: true,
+    });
+    let draws = display::page_display(&d, &page, &display::DisplayOptions::default());
+    let upright_clips = draws.iter().filter(|x| matches!(x, display::Draw::Clip { .. })).count();
+    let turned_clips: usize = draws
+        .iter()
+        .map(|x| match x {
+            display::Draw::Rotated { items, .. } => items.iter().filter(|g| matches!(g, display::Draw::Clip { .. })).count(),
+            _ => 0,
+        })
+        .sum();
+    assert_eq!((upright_clips, turned_clips), (0, 1));
+}