From 5ee2bda8fc6fdaaec7c515935e3858dbfca085b0 Mon Sep 17 00:00:00 2001 From: AiBizzApps <133780730+goldsbd@users.noreply.github.com> Date: Sat, 10 Oct 2026 17:16:48 +0700 Subject: [PATCH] M6: TOC follow-ups from #52 review, and a whole TOC heading in LibreOffice (#74) * M6: close the TOC field in its last entry, not in a text box inside it `para_inner` took `toc_end_here` only after writing the paragraph's content, so a nested story in the last TOC entry (a text box) reached `para_inner` first, took the flag, and wrote the field's end inside `w:txbxContent`. Take it before the content. Review follow-up on #52. Co-Authored-By: Claude Opus 5.5 * M6: Update Table reads a heading above the TOC at its own index Pass 1 shifted every heading by `entries.len()` to account for the entries now in the body, but entries go in below the TOC heading, so a heading above it never moves. Its lookup landed on a TOC entry and took that page (a Preface on page 1 above a TOC on page 2 showed 2). Shift only headings below the TOC. Review follow-up on #52. Co-Authored-By: Claude Opus 5.5 * M6: start the TOC field in the first entry, as Word does The writer opened the TOC field at the end of the "Contents" heading. Word reads that fine, but LibreOffice splits the heading: "Content" as body text, then "s" as the TOC title (on the next page when the TOC follows a page break). Word itself writes the heading before the field and begins the field in the first entry; written that way, LibreOffice shows the heading whole. Hold the heading's field back and write its begin/instruction/separate at the start of the first entry, taken before the paragraph content so a text box in the heading or an entry can't claim it (as with the end). The reader moves a TOC field that starts a paragraph right after a TOC Heading back to the end of the heading, so WordCraft's own files read back as before and a TOC saved by Word now updates in WordCraft too (before, Update Table left Word's first entry behind as a duplicate). Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- crates/docx/src/read/mod.rs | 33 ++++++++++++++- crates/docx/src/write/mod.rs | 8 +++- crates/docx/src/write/story.rs | 63 ++++++++++++++++++++--------- crates/docx/tests/roundtrip.rs | 53 ++++++++++++++++++++++++ crates/engine/src/cmd/references.rs | 6 +-- crates/engine/src/tests.rs | 24 +++++++++++ 6 files changed, 162 insertions(+), 25 deletions(-) diff --git a/crates/docx/src/read/mod.rs b/crates/docx/src/read/mod.rs index 7899f49..8f5270d 100644 --- a/crates/docx/src/read/mod.rs +++ b/crates/docx/src/read/mod.rs @@ -7,10 +7,11 @@ use std::collections::{BTreeMap, HashMap, HashSet}; use std::sync::Arc; use wordcraft_doc::numbering::{AbstractNum, Level, LevelSuffix, Num}; +use wordcraft_doc::para::OBJ; use wordcraft_doc::props::Rgb; use wordcraft_doc::section::NumFormat; use wordcraft_doc::styles::{Style, StyleKind, StyleSheet, TableStyleParts}; -use wordcraft_doc::{Blocks, Comment, Document, PartKind, Revision, RevisionKind}; +use wordcraft_doc::{Block, Blocks, Comment, Document, InlineObject, PartKind, Revision, RevisionKind}; use crate::DocxError; use crate::package::{Package, Rels, rt}; @@ -104,6 +105,7 @@ pub fn read(bytes: &[u8]) -> Result { let mut blocks = Blocks::new(); r.read_blocks(&mut sc, body, &rels, &mut blocks, 0); r.flush_pending(&mut sc, &mut blocks); + toc_field_to_heading(&mut blocks); r.doc.body = blocks; if let Some(s) = body.child("w:sectPr") { r.doc.last_section = r.read_section(s, &rels); @@ -122,6 +124,35 @@ pub fn read(bytes: &[u8]) -> Result { Ok(doc) } +/// Word starts a TOC field in the first entry; the engine keeps it at the end of the TOC heading +/// (the paragraph holding the field marks where the TOC is). Move it there when one precedes it. +fn toc_field_to_heading(body: &mut Blocks) { + for i in 1..body.len() { + let heading = body[i - 1].as_para().filter(|p| p.props.style.as_deref() == Some("TOCHeading")); + let has_toc = |o: &InlineObject| matches!(o, InlineObject::Field { instr, .. } if is_toc(instr)); + if heading.is_none_or(|h| h.objects.iter().any(has_toc)) { + continue; + } + let Some((field, props)) = body[i].as_para().and_then(|p| match p.object_at(0) { + Some(f @ InlineObject::Field { result, .. }) if result.is_empty() && has_toc(f) => Some((f.clone(), p.props_of_char(0).clone())), + _ => None, + }) else { + continue; + }; + if let Some(Block::Para(p)) = body.get_mut(i).map(Arc::make_mut) { + let _ = p.delete(0, OBJ.len_utf8()); + } + if let Some(Block::Para(h)) = body.get_mut(i - 1).map(Arc::make_mut) { + let end = h.len(); + let _ = h.insert_object(end, field, &props); + } + } +} + +fn is_toc(instr: &str) -> bool { + instr.trim_start().get(..3).is_some_and(|k| k.eq_ignore_ascii_case("TOC")) +} + /// Built-in style names Word stores in lower case, and how the UI shows them. pub(crate) const LOWER_NAMES: &[(&str, &str)] = &[ ("caption", "Caption"), diff --git a/crates/docx/src/write/mod.rs b/crates/docx/src/write/mod.rs index 8fac322..d1747d3 100644 --- a/crates/docx/src/write/mod.rs +++ b/crates/docx/src/write/mod.rs @@ -69,8 +69,12 @@ pub(crate) struct Writer<'d> { pending_mark: Option<&'static str>, /// The note being written (is footnote, part id): its reference to itself is the mark above. current_note: Option<(bool, u32)>, - /// Writing a TOC heading: its TOC field stays open so the entries become the field's result. + /// Writing a TOC heading: its TOC field is held back so the entries become the field's result. toc_hold_end: bool, + /// The held TOC field (instruction, locked, props), opened in the first entry as Word does. + toc_field: Option<(String, bool, wordcraft_doc::props::CharProps)>, + /// Open the held TOC field at the start of the paragraph being written. + toc_begin_here: bool, /// Close the open TOC field at the end of the paragraph being written. toc_end_here: bool, /// Media keys actually referenced by a written drawing. @@ -96,6 +100,8 @@ pub fn write(doc: &Document) -> Result, DocxError> { pending_mark: None, current_note: None, toc_hold_end: false, + toc_field: None, + toc_begin_here: false, toc_end_here: false, used_media: Default::default(), }; diff --git a/crates/docx/src/write/story.rs b/crates/docx/src/write/story.rs index 572719e..0ebfdcb 100644 --- a/crates/docx/src/write/story.rs +++ b/crates/docx/src/write/story.rs @@ -39,6 +39,7 @@ impl Writer<'_> { w.raw(TOC_SDT_OPEN); self.toc_hold_end = true; } + self.toc_begin_here = toc_field_open && i == start + 1; self.toc_end_here = toc_field_open && i == end; } match &**b { @@ -47,8 +48,9 @@ impl Writer<'_> { } if let Some((start, end)) = toc { if i == start { - // `field` clears the flag when it leaves the TOC field open. - toc_field_open = !std::mem::take(&mut self.toc_hold_end); + // `field` holds the heading's TOC field back for the first entry. + self.toc_hold_end = false; + toc_field_open = self.toc_field.is_some(); } if i == end { w.raw(""); @@ -111,8 +113,17 @@ impl Writer<'_> { w.empty(mark, &[]); w.raw(""); } + // Taken before the content so a nested story (a text box in an entry) can't claim them. + if std::mem::take(&mut self.toc_begin_here) + && let Some((instr, locked, props)) = self.toc_field.take() + { + self.rev_open(w, &props); + self.field_start(w, &instr, locked, &props); + self.rev_close(w, &props); + } + let toc_end = std::mem::take(&mut self.toc_end_here); self.para_content(w, p, rels, depth); - if std::mem::take(&mut self.toc_end_here) { + if toc_end { w.raw(r#""#); } w.close("w:p"); @@ -459,6 +470,12 @@ impl Writer<'_> { } fn field(&mut self, w: &mut W, instr: &str, result: &str, locked: bool, props: &CharProps) { + if self.toc_hold_end && is_toc(instr) { + // A TOC heading's field opens in the first entry and closes after the last (see `toc_span`). + self.toc_hold_end = false; + self.toc_field = Some((instr.to_string(), locked, props.clone())); + return; + } let del = self.is_del(props); let run = |w: &mut W, f: &dyn Fn(&mut W)| { w.open("w:r", &[]); @@ -467,16 +484,7 @@ impl Writer<'_> { w.close("w:r"); }; self.rev_open(w, props); - run(w, &|w| { - if locked { - w.empty("w:fldChar", &[("w:fldCharType", "begin"), ("w:fldLock", "1")]) - } else { - w.empty("w:fldChar", &[("w:fldCharType", "begin")]) - } - }); - let instr_text = format!(" {} ", instr.trim()); - run(w, &|w| w.leaf(if del { "w:delInstrText" } else { "w:instrText" }, &[("xml:space", "preserve")], &instr_text)); - run(w, &|w| w.empty("w:fldChar", &[("w:fldCharType", "separate")])); + self.field_start(w, instr, locked, props); if !result.is_empty() { run(w, &|w| { let mut buf = String::new(); @@ -503,16 +511,31 @@ impl Writer<'_> { flush(w, &mut buf); }); } - // A TOC heading's field is closed after its last entry (see `toc_span`). - let toc_open = self.toc_hold_end && is_toc(instr); - if toc_open { - self.toc_hold_end = false; - } else { - run(w, &|w| w.empty("w:fldChar", &[("w:fldCharType", "end")])); - } + run(w, &|w| w.empty("w:fldChar", &[("w:fldCharType", "end")])); self.rev_close(w, props); } + /// A field's begin, instruction and separate runs. + fn field_start(&self, w: &mut W, instr: &str, locked: bool, props: &CharProps) { + let del = self.is_del(props); + let run = |w: &mut W, f: &dyn Fn(&mut W)| { + w.open("w:r", &[]); + rpr(w, props); + f(w); + w.close("w:r"); + }; + run(w, &|w| { + if locked { + w.empty("w:fldChar", &[("w:fldCharType", "begin"), ("w:fldLock", "1")]) + } else { + w.empty("w:fldChar", &[("w:fldCharType", "begin")]) + } + }); + let instr_text = format!(" {} ", instr.trim()); + run(w, &|w| w.leaf(if del { "w:delInstrText" } else { "w:instrText" }, &[("xml:space", "preserve")], &instr_text)); + run(w, &|w| w.empty("w:fldChar", &[("w:fldCharType", "separate")])); + } + #[allow(clippy::too_many_arguments)] fn drawing_open(&mut self, w: &mut W, float: &Float, iw: f32, ih: f32, docpr: &str, name: &str, alt: &str) { let (cx, cy) = (emu(iw.max(0.0)), emu(ih.max(0.0))); diff --git a/crates/docx/tests/roundtrip.rs b/crates/docx/tests/roundtrip.rs index 2e9bdb4..5a9aae0 100644 --- a/crates/docx/tests/roundtrip.rs +++ b/crates/docx/tests/roundtrip.rs @@ -504,15 +504,68 @@ fn toc_field_wraps_its_entries() { let last_entry = xml.find("Detail").unwrap(); assert!(sdt < xml.find("Contents").unwrap() && xml.find("").unwrap() < xml.find(">Body<").unwrap(), "{xml}"); assert!(begin < xml.find("Intro").unwrap() && fend > last_entry && fend < xml.find("").unwrap(), "field must span the entries: {xml}"); + // Word starts the field in the first entry; started in the heading, LibreOffice splits the heading. + let heading_end = xml[..xml.find("Intro").unwrap()].rfind("").unwrap(); + assert!(begin > heading_end, "field must start in the first entry, not the heading: {xml}"); // WordCraft reads its own TOC back unchanged. let r = wordcraft_docx::read(&bytes).expect("read"); let ps = paras(&r); assert_eq!(ps.iter().map(|p| p.plain_text()).collect::>(), ["Contents", "Intro\t1", "Detail\t2", "Body"]); assert!(matches!(ps[0].objects.first(), Some(InlineObject::Field { instr, .. }) if instr.starts_with("TOC")), "{:?}", ps[0].objects); + assert!(ps[1].objects.is_empty(), "the field goes back on the heading: {:?}", ps[1].objects); assert_eq!(ps[1].props.style.as_deref(), Some("TOC1")); } +/// The TOC field's begin and end belong to the entries' own paragraphs, even when the heading or +/// an entry holds a text box: the box's paragraphs are written inside them and must not take either. +#[test] +fn toc_field_skips_nested_stories() { + let mut d = Document::new(); + let text_box = |d: &mut Document, text: &str| { + let story = d.add_part(PartKind::TextBox, vec![para_block(Paragraph::with_text(text, CharProps::default()))]); + InlineObject::Shape { + kind: ShapeKind::TextBox, + w: 72.0, + h: 36.0, + fill: None, + stroke: None, + stroke_width: 0.0, + float: Float::default(), + story: Some(story), + } + }; + let mut head = Paragraph::with_text("Contents", CharProps::default()).styled("TOCHeading"); + let end = head.len(); + head.insert_object( + end, + InlineObject::Field { instr: "TOC \\o \"1-1\" \\h \\z \\u".into(), result: String::new(), locked: false }, + &CharProps::default(), + ) + .unwrap(); + let end = head.len(); + head.insert_object(end, text_box(&mut d, "Heading box"), &CharProps::default()).unwrap(); + let mut last = Paragraph::with_text("Detail\t2", CharProps::default()).styled("TOC1"); + let end = last.len(); + last.insert_object(end, text_box(&mut d, "Boxed"), &CharProps::default()).unwrap(); + d.body = vec![para_block(head), para_block(last), para_block(Paragraph::with_text("Body", CharProps::default()))]; + + let bytes = wordcraft_docx::write(&d).expect("write"); + let mut zip = zip::ZipArchive::new(std::io::Cursor::new(&bytes)).unwrap(); + let mut xml = String::new(); + std::io::Read::read_to_string(&mut zip.by_name("word/document.xml").unwrap(), &mut xml).unwrap(); + let boxes: Vec<(usize, usize)> = xml.match_indices("").map(|(a, _)| (a, a + xml[a..].find("").unwrap())).collect(); + assert_eq!(boxes.len(), 2, "{xml}"); + let in_box = |i: usize| boxes.iter().any(|(a, b)| (*a..*b).contains(&i)); + for kind in ["begin", "separate", "end"] { + let at: Vec = xml.match_indices(&format!(r#"fldCharType="{kind}""#)).map(|(i, _)| i).collect(); + assert_eq!(at.len(), 1, "{kind}: {xml}"); + assert!(!in_box(at[0]), "field {kind} written inside a text box: {xml}"); + } + let (begin, end) = (xml.find(r#"fldCharType="begin""#).unwrap(), xml.find(r#"fldCharType="end""#).unwrap()); + assert!(begin > boxes[0].1 && end > boxes[1].1 && end < xml.find("").unwrap(), "{xml}"); +} + #[test] fn tracked_changes_round_trip() { let mut d = Document::new(); diff --git a/crates/engine/src/cmd/references.rs b/crates/engine/src/cmd/references.rs index 8f652b9..d898ce6 100644 --- a/crates/engine/src/cmd/references.rs +++ b/crates/engine/src/cmd/references.rs @@ -165,8 +165,8 @@ pub fn update_toc(s: &mut Session) -> Result<(), CmdError> { let leads: Vec = entries.iter().map(|(path, ..)| s.doc.para(StoryRef::Body, path).map_or(0, |p| p.text.bytes().take_while(|b| *b == 0x0C).count())).collect(); for pass in 0..2 { - // Pass 1 lays out with the entries in place, so headings sit `entries.len()` blocks - // further down and their pages account for the TOC's own length. + // Pass 1 lays out with the entries in place, so headings below the TOC sit `entries.len()` + // blocks further down and their pages account for the TOC's own length. s.touch(); let l = s.layout(); if pass == 1 { @@ -177,7 +177,7 @@ pub fn update_toc(s: &mut Session) -> Result<(), CmdError> { } let shift = if pass == 0 { 0 } else { entries.len() }; for (k, (path, level, text)) in entries.iter().enumerate() { - let target = Path::top(path.last() + shift); + let target = Path::top(path.last() + if path.last() > start { shift } else { 0 }); let off = leads.get(k).copied().unwrap_or(0); let page = l.caret(&Pos { story: StoryRef::Body, path: target, off }).and_then(|c| l.pages.get(c.page)).map(|pg| pg.number).unwrap_or(1); let mut para = Paragraph::with_text(&format!("{text}\t{page}"), CharProps::default()).styled(&format!("TOC{}", level + 1)); diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 166ab92..2cbbaeb 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -290,6 +290,30 @@ fn toc_page_numbers_follow_headings() { } } +/// A heading above the TOC keeps its index when the entries go in below it, so its page is read +/// where it is, not `entries` blocks further down. +#[test] +fn toc_page_number_for_heading_above_toc() { + let mut s = s(); + run(&mut s, "text.insert", json!({"text": "Preface"})); + run(&mut s, "para.heading1", json!({})); + run(&mut s, "text.newParagraph", json!({})); + run(&mut s, "text.pageBreak", json!({})); + run(&mut s, "references.toc", json!({})); + run(&mut s, "caret.docEnd", json!({})); + for name in ["Alpha", "Bravo"] { + run(&mut s, "text.pageBreak", json!({})); + run(&mut s, "text.insert", json!({"text": name})); + run(&mut s, "para.heading1", json!({})); + run(&mut s, "text.newParagraph", json!({})); + } + run(&mut s, "references.updateToc", json!({})); + let t = text(&s); + for (name, page) in [("Preface", 1), ("Alpha", 3), ("Bravo", 4)] { + assert!(t.contains(&format!("{name}\t{page}")), "{name} should be on page {page}: {t:?}"); + } +} + #[test] fn failed_command_leaves_document_unchanged() { let mut s = s();