From 3fcc1e26945b335f3619809280f61a71a2da74bf Mon Sep 17 00:00:00 2001 From: naxvog Date: Sat, 10 Oct 2026 12:17:17 +0200 Subject: [PATCH] Lists: honour a list's own level definitions (w:lvlOverride/w:lvl) (#134) A w:num can redefine any of its abstract list's levels with a whole w:lvl inside w:lvlOverride. We kept only the start value from it and numbered with the abstract list's levels, so documents that rely on it showed the wrong labels: a construction specification (SpecLink) read "A. PART 1 GENERAL" and "I. SECTION INCLUDES" where Word shows "PART 1 GENERAL" and "1.01 SECTION INCLUDES". Saving such a document also dropped the definitions, so Word showed the wrong labels afterwards. Num::level_overrides keeps them; Numbering::level and the list counters prefer them over the abstract list's levels; the .docx reader reads them with the abstract levels' parser and the writer writes them back (one w:lvlOverride per level with its startOverride and/or lvl). In docxide-pdf's Word reference corpus 6 of 382 documents carry such definitions; the specification's SSIM goes 13% -> 41%, the others restate their abstract lists and are unchanged. Tests: counters use a list's own levels ("" and "%2.%3" in decimalZero give "1.01", "1.02", "2.01") and fall back to the abstract list for the rest; level overrides round-trip through .docx beside start overrides. Co-authored-by: Oliver Zieschang <4963183+naxvog@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 --- crates/doc/src/numbering.rs | 40 ++++++++++++++--- crates/docx/src/read/mod.rs | 10 +++-- crates/docx/src/write/mod.rs | 81 ++++++++++++++++++++-------------- crates/docx/tests/roundtrip.rs | 23 ++++++++++ crates/formats/src/model.rs | 2 +- 5 files changed, 114 insertions(+), 42 deletions(-) diff --git a/crates/doc/src/numbering.rs b/crates/doc/src/numbering.rs index 7cbae72..7fe0648 100644 --- a/crates/doc/src/numbering.rs +++ b/crates/doc/src/numbering.rs @@ -73,6 +73,10 @@ pub struct Num { pub abstract_id: u32, /// Per-level start overrides (level, start). pub start_overrides: Vec<(u8, u32)>, + /// Levels this list defines itself instead of taking them from its abstract list (level, + /// definition): Word's `w:lvlOverride/w:lvl`. + #[serde(skip_serializing_if = "Vec::is_empty")] + pub level_overrides: Vec<(u8, Level)>, } #[derive(Clone, Debug, PartialEq, Serialize, Deserialize, Default)] @@ -194,6 +198,9 @@ impl Numbering { self.abstracts.iter().find(|a| a.id == n.abstract_id) } pub fn level(&self, num: u32, level: u8) -> Option<&Level> { + if let Some((_, l)) = self.num(num)?.level_overrides.iter().find(|(i, _)| *i == level) { + return Some(l); + } self.abstract_of(num)?.levels.get(level as usize) } /// Create a new list of `kind`; returns the `num` id. @@ -201,7 +208,7 @@ impl Numbering { let aid = self.abstracts.iter().map(|a| a.id + 1).max().unwrap_or(0); self.abstracts.push(AbstractNum { id: aid, name: None, levels: levels_for(kind) }); let nid = self.nums.iter().map(|n| n.id + 1).max().unwrap_or(1).max(1); - self.nums.push(Num { id: nid, abstract_id: aid, start_overrides: Vec::new() }); + self.nums.push(Num { id: nid, abstract_id: aid, ..Default::default() }); nid } /// A new `num` that restarts numbering of an existing list. @@ -209,7 +216,7 @@ impl Numbering { let a = self.num(num)?.abstract_id; let nid = self.nums.iter().map(|n| n.id + 1).max().unwrap_or(1).max(1); let starts = self.abstracts.iter().find(|x| x.id == a).map(|x| x.levels.iter().enumerate().map(|(i, l)| (i as u8, l.start)).collect()); - self.nums.push(Num { id: nid, abstract_id: a, start_overrides: starts.unwrap_or_default() }); + self.nums.push(Num { id: nid, abstract_id: a, start_overrides: starts.unwrap_or_default(), ..Default::default() }); Some(nid) } /// Find an existing list whose first level matches `kind`, so consecutive bullet clicks reuse it. @@ -238,7 +245,9 @@ impl Counters { let n = numbering.num(num)?; let abs = numbering.abstract_of(num)?; let lv = (level as usize).min(8); - let def = abs.levels.get(lv)?.clone(); + // This list's own definition of a level wins over its abstract list's. + let level_def = |k: usize| numbering.level(num, k as u8); + let def = level_def(lv)?.clone(); let st = self.state.entry(abs.id).or_insert([0; 9]); if let std::collections::hash_map::Entry::Vacant(e) = self.seen.entry(num) { e.insert(true); @@ -270,8 +279,8 @@ impl Counters { chars.next(); let k = (d as usize).saturating_sub(1).min(8); let val = vals.get(k).copied().unwrap_or(0); - let val = if val == 0 { abs.levels.get(k).map(|l| l.start).unwrap_or(1) } else { val }; - let fmt = if def.legal && k != lv { NumFormat::Decimal } else { abs.levels.get(k).map(|l| l.format).unwrap_or(NumFormat::Decimal) }; + let val = if val == 0 { level_def(k).map(|l| l.start).unwrap_or(1) } else { val }; + let fmt = if def.legal && k != lv { NumFormat::Decimal } else { level_def(k).map(|l| l.format).unwrap_or(NumFormat::Decimal) }; label.push_str(&fmt.format(val)); } else { label.push(c); @@ -285,6 +294,27 @@ impl Counters { mod tests { use super::*; + #[test] + fn a_list_s_own_level_definitions_win() { + // A specification's article numbers: level 1 counts parts with no label of its own, level 2 + // shows "part.article" with a zero-padded article (Word's lvlOverride/lvl). + let mut n = Numbering::default(); + let id = n.add_list(ListKind::Numbered); + let lvl = |format, text: &str| Level { format, text: text.into(), ..Level::default() }; + if let Some(num) = n.nums.iter_mut().find(|x| x.id == id) { + num.level_overrides = vec![(1, lvl(NumFormat::Decimal, "")), (2, lvl(NumFormat::DecimalZero, "%2.%3"))]; + } + let mut c = Counters::default(); + assert_eq!(c.next_label(&n, id, 1).unwrap().0, ""); + assert_eq!(c.next_label(&n, id, 2).unwrap().0, "1.01"); + assert_eq!(c.next_label(&n, id, 2).unwrap().0, "1.02"); + assert_eq!(c.next_label(&n, id, 1).unwrap().0, ""); + assert_eq!(c.next_label(&n, id, 2).unwrap().0, "2.01"); + // Levels it doesn't define still come from the abstract list. + assert_eq!(c.next_label(&n, id, 0).unwrap().0, "1."); + assert_eq!(n.level(id, 2).map(|l| l.format), Some(NumFormat::DecimalZero)); + } + #[test] fn numbered_labels() { let mut n = Numbering::default(); diff --git a/crates/docx/src/read/mod.rs b/crates/docx/src/read/mod.rs index 8f5270d..f3f843e 100644 --- a/crates/docx/src/read/mod.rs +++ b/crates/docx/src/read/mod.rs @@ -420,16 +420,20 @@ impl Reader<'_> { let mut nums = Vec::new(); for n in root.children("w:num").take(10_000) { let (Some(id), Some(abs)) = (n.attr("w:numId").and_then(u32_of), n.child_val("w:abstractNumId").and_then(u32_of)) else { continue }; - let mut start_overrides = Vec::new(); - for o in n.children("w:lvlOverride") { + let (mut start_overrides, mut level_overrides) = (Vec::new(), Vec::new()); + for o in n.children("w:lvlOverride").take(9) { let lvl = o.attr("w:ilvl").and_then(u32_of).unwrap_or(0).min(8) as u8; if let Some(s) = o.child_val("w:startOverride").and_then(u32_of) { start_overrides.push((lvl, s)); } else if let Some(s) = o.child("w:lvl").and_then(|l| l.child_val("w:start")).and_then(u32_of) { start_overrides.push((lvl, s)); } + // A whole level definition replaces the abstract list's for this list. + if let Some(l) = o.child("w:lvl") { + level_overrides.push((lvl, self.level(l, lvl as usize))); + } } - nums.push(Num { id, abstract_id: abs, start_overrides }); + nums.push(Num { id, abstract_id: abs, start_overrides, level_overrides }); } // Abstracts that only link to a numbering style take that style's list levels. for (aid, style) in links { diff --git a/crates/docx/src/write/mod.rs b/crates/docx/src/write/mod.rs index d1747d3..042fc67 100644 --- a/crates/docx/src/write/mod.rs +++ b/crates/docx/src/write/mod.rs @@ -5,7 +5,7 @@ mod story; use std::collections::{BTreeMap, HashMap}; -use wordcraft_doc::numbering::LevelSuffix; +use wordcraft_doc::numbering::{Level, LevelSuffix}; use wordcraft_doc::styles::{Style, StyleKind}; use wordcraft_doc::{Blocks, Document, PartKind}; @@ -607,44 +607,26 @@ fn numbering_xml(doc: &Document) -> Vec { w.val("w:name", n); } for (i, l) in a.levels.iter().take(9).enumerate() { - w.open("w:lvl", &[("w:ilvl", &i.to_string())]); - w.val("w:start", &l.start.to_string()); - w.val("w:numFmt", l.format.ooxml()); - if !l.restart { - w.val("w:lvlRestart", "0"); - } - if let Some(s) = &l.style { - w.val("w:pStyle", s); - } - if l.legal { - w.empty("w:isLgl", &[]); - } - match l.suffix { - LevelSuffix::Tab => {} - LevelSuffix::Space => w.val("w:suff", "space"), - LevelSuffix::Nothing => w.val("w:suff", "nothing"), - } - w.val("w:lvlText", &l.text); - w.val("w:lvlJc", props::align_val(l.align)); - w.open("w:pPr", &[]); - let ind = crate::units::twips(l.indent); - if l.hanging >= 0.0 { - w.empty("w:ind", &[("w:left", &ind), ("w:hanging", &crate::units::twips(l.hanging))]); - } else { - w.empty("w:ind", &[("w:left", &ind), ("w:firstLine", &crate::units::twips(-l.hanging))]); - } - w.close("w:pPr"); - props::rpr(&mut w, &l.chr); - w.close("w:lvl"); + write_level(&mut w, i, l); } w.close("w:abstractNum"); } for n in &doc.numbering.nums { w.open("w:num", &[("w:numId", &n.id.to_string())]); w.val("w:abstractNumId", &n.abstract_id.to_string()); - for (lvl, start) in &n.start_overrides { - w.open("w:lvlOverride", &[("w:ilvl", &lvl.min(&8).to_string())]); - w.val("w:startOverride", &start.to_string()); + for lvl in 0..9u8 { + let start = n.start_overrides.iter().find(|(l, _)| (*l).min(8) == lvl).map(|(_, s)| *s); + let level = n.level_overrides.iter().find(|(l, _)| (*l).min(8) == lvl).map(|(_, l)| l); + if start.is_none() && level.is_none() { + continue; + } + w.open("w:lvlOverride", &[("w:ilvl", &lvl.to_string())]); + if let Some(start) = start { + w.val("w:startOverride", &start.to_string()); + } + if let Some(level) = level { + write_level(&mut w, lvl as usize, level); + } w.close("w:lvlOverride"); } w.close("w:num"); @@ -653,6 +635,39 @@ fn numbering_xml(doc: &Document) -> Vec { w.into_bytes() } +/// One `w:lvl` (list level `i`). +fn write_level(w: &mut W, i: usize, l: &Level) { + w.open("w:lvl", &[("w:ilvl", &i.to_string())]); + w.val("w:start", &l.start.to_string()); + w.val("w:numFmt", l.format.ooxml()); + if !l.restart { + w.val("w:lvlRestart", "0"); + } + if let Some(s) = &l.style { + w.val("w:pStyle", s); + } + if l.legal { + w.empty("w:isLgl", &[]); + } + match l.suffix { + LevelSuffix::Tab => {} + LevelSuffix::Space => w.val("w:suff", "space"), + LevelSuffix::Nothing => w.val("w:suff", "nothing"), + } + w.val("w:lvlText", &l.text); + w.val("w:lvlJc", props::align_val(l.align)); + w.open("w:pPr", &[]); + let ind = crate::units::twips(l.indent); + if l.hanging >= 0.0 { + w.empty("w:ind", &[("w:left", &ind), ("w:hanging", &crate::units::twips(l.hanging))]); + } else { + w.empty("w:ind", &[("w:left", &ind), ("w:firstLine", &crate::units::twips(-l.hanging))]); + } + w.close("w:pPr"); + props::rpr(w, &l.chr); + w.close("w:lvl"); +} + fn settings_xml(doc: &Document, footnotes: bool, endnotes: bool) -> Vec { let s = &doc.settings; let mut w = W::new(); diff --git a/crates/docx/tests/roundtrip.rs b/crates/docx/tests/roundtrip.rs index 5a9aae0..67257f3 100644 --- a/crates/docx/tests/roundtrip.rs +++ b/crates/docx/tests/roundtrip.rs @@ -814,3 +814,26 @@ fn ensure_empty_and_table_end_document_is_valid() { let r = rt(&e); assert_eq!(r.body.len(), 1); } + +#[test] +fn list_level_overrides_round_trip() { + use wordcraft_doc::numbering::{Counters, Level}; + use wordcraft_doc::section::NumFormat; + let mut d = Document::new(); + let num = d.numbering.add_list(ListKind::Numbered); + let restart = d.numbering.restart(num).unwrap(); + let own = Level { format: NumFormat::DecimalZero, text: "%1.%2".into(), indent: 26.5, hanging: 26.5, ..Level::default() }; + if let Some(n) = d.numbering.nums.iter_mut().find(|n| n.id == restart) { + n.level_overrides = vec![(1, own.clone())]; + } + let back = wordcraft_docx::read(&wordcraft_docx::write(&d).unwrap()).unwrap(); + let n = back.numbering.num(restart).unwrap(); + assert_eq!(n.level_overrides.len(), 1); + let (lvl, got) = &n.level_overrides[0]; + assert_eq!((*lvl, got.format, got.text.as_str(), got.indent, got.hanging), (1, NumFormat::DecimalZero, "%1.%2", 26.5, 26.5)); + // The start overrides that `restart` wrote survive beside it. + assert_eq!(n.start_overrides, d.numbering.num(restart).unwrap().start_overrides); + let mut c = Counters::default(); + assert_eq!(c.next_label(&back.numbering, restart, 0).unwrap().0, "1."); + assert_eq!(c.next_label(&back.numbering, restart, 1).unwrap().0, "1.01"); +} diff --git a/crates/formats/src/model.rs b/crates/formats/src/model.rs index f4f3ba9..5edbeb0 100644 --- a/crates/formats/src/model.rs +++ b/crates/formats/src/model.rs @@ -373,7 +373,7 @@ impl Builder<'_> { let aid = numbering.abstracts.iter().map(|a| a.id + 1).max().unwrap_or(0); numbering.abstracts.push(AbstractNum { id: aid, name: None, levels }); let nid = numbering.nums.iter().map(|n| n.id + 1).max().unwrap_or(1).max(1); - numbering.nums.push(Num { id: nid, abstract_id: aid, start_overrides: Vec::new() }); + numbering.nums.push(Num { id: nid, abstract_id: aid, ..Default::default() }); nid }