diff --git a/crates/rdocx-layout/src/engine.rs b/crates/rdocx-layout/src/engine.rs index ffd75ce9..48e7e586 100644 --- a/crates/rdocx-layout/src/engine.rs +++ b/crates/rdocx-layout/src/engine.rs @@ -490,9 +490,27 @@ pub fn layout_paragraph( let resolved_ppr = style_resolver::resolve_paragraph_properties(para_style_id, styles); - // Merge direct paragraph properties let mut effective_ppr = resolved_ppr; - if let Some(ref direct_ppr) = para.properties { + + // A numbering level carries paragraph properties of its own, mainly the + // indentation for that level. They sit between the style and direct + // formatting, so merge them before the direct properties rather than + // after. Without this every level of a list draws at the same indent. + let direct_ppr = para.properties.as_ref(); + let list_num_id = direct_ppr.and_then(|p| p.num_id).or(effective_ppr.num_id); + let list_ilvl = direct_ppr + .and_then(|p| p.num_ilvl) + .or(effective_ppr.num_ilvl) + .unwrap_or(0); + if let (Some(num_id), Some(numbering)) = (list_num_id, input.numbering.as_ref()) + && let Some(lvl_ppr) = + style_resolver::level_paragraph_properties(num_id, list_ilvl, numbering) + { + merge_direct_ppr(&mut effective_ppr, lvl_ppr); + } + + // Merge direct paragraph properties + if let Some(direct_ppr) = direct_ppr { merge_direct_ppr(&mut effective_ppr, direct_ppr); } diff --git a/crates/rdocx-layout/src/style_resolver.rs b/crates/rdocx-layout/src/style_resolver.rs index 9dae495b..9ef2c2d0 100644 --- a/crates/rdocx-layout/src/style_resolver.rs +++ b/crates/rdocx-layout/src/style_resolver.rs @@ -154,6 +154,20 @@ pub fn resolve_run_properties( effective } +/// The paragraph properties a numbering level carries, mainly its indentation. +/// +/// In the property chain these sit between the paragraph style and direct +/// formatting, so the indent for a list level applies unless the paragraph +/// sets its own. +pub fn level_paragraph_properties( + num_id: u32, + ilvl: u32, + numbering: &CT_Numbering, +) -> Option<&CT_PPr> { + let abs = numbering.get_abstract_num_for(num_id)?; + abs.levels.iter().find(|l| l.ilvl == ilvl)?.ppr.as_ref() +} + /// Generate the marker text for a numbered/bulleted list item. pub fn generate_marker( num_id: u32, @@ -164,6 +178,13 @@ pub fn generate_marker( let abs = numbering.get_abstract_num_for(num_id)?; let lvl = abs.levels.iter().find(|l| l.ilvl == ilvl)?; + // Counters belong to the abstract definition, not the numbering instance. + // Writers such as Pandoc and LibreOffice emit a separate w:num per list + // block while pointing them all at one w:abstractNum, and readers are + // expected to carry the count across them. Keying on num_id restarted the + // sequence at every block, so a two item list rendered as "1." twice. + let counter_id = abs.abstract_num_id; + let num_fmt = lvl.num_fmt.unwrap_or(ST_NumberFormat::Decimal); let start = lvl.start.unwrap_or(1); let lvl_text = lvl.lvl_text.as_deref().unwrap_or("%1."); @@ -171,8 +192,8 @@ pub fn generate_marker( let marker_text = if num_fmt == ST_NumberFormat::Bullet { lvl_text.to_string() } else { - let count = state.advance(num_id, ilvl, start); - format_lvl_text(lvl_text, num_id, ilvl, count, numbering, state) + let count = state.advance(counter_id, ilvl, start); + format_lvl_text(lvl_text, num_id, counter_id, ilvl, count, numbering, state) }; let marker_rpr = lvl.rpr.clone().unwrap_or_default(); @@ -187,6 +208,7 @@ pub fn generate_marker( fn format_lvl_text( template: &str, num_id: u32, + counter_id: u32, current_ilvl: u32, current_count: u32, numbering: &CT_Numbering, @@ -204,7 +226,7 @@ fn format_lvl_text( let count = if lvl_idx == current_ilvl { current_count } else { - state.current(num_id, lvl_idx) + state.current(counter_id, lvl_idx) }; let fmt = abs .levels @@ -404,4 +426,68 @@ mod tests { assert_eq!(to_letter(27, false), "a"); // wraps assert_eq!(to_letter(1, true), "A"); } + + /// Two numbering instances that share one abstract definition are one + /// list and must keep counting. + /// + /// Pandoc and LibreOffice both emit a separate `w:num` per list block + /// pointing at the same `w:abstractNum`. Keying counters on num_id made + /// every block restart, so a two item list rendered as "1." twice. + #[test] + fn shared_abstract_definition_continues_the_count() { + let mut numbering = CT_Numbering::new(); + let first = numbering.add_numbered_list(); + let abstract_id = numbering + .nums + .iter() + .find(|n| n.num_id == first) + .unwrap() + .abstract_num_id; + + // A second instance pointing at the same abstract definition. + let second = numbering.nums.iter().map(|n| n.num_id).max().unwrap() + 1; + numbering.nums.push(rdocx_oxml::numbering::CT_Num { + num_id: second, + abstract_num_id: abstract_id, + }); + + let mut state = NumberingState::new(); + let a = generate_marker(first, 0, &numbering, &mut state).unwrap(); + let b = generate_marker(second, 0, &numbering, &mut state).unwrap(); + assert_eq!(a.marker_text, "1."); + assert_eq!(b.marker_text, "2.", "the count must carry across instances"); + } + + /// Separate abstract definitions are separate lists and each restarts. + #[test] + fn separate_abstract_definitions_count_independently() { + let mut numbering = CT_Numbering::new(); + let first = numbering.add_numbered_list(); + let second = numbering.add_numbered_list(); + + let mut state = NumberingState::new(); + let a = generate_marker(first, 0, &numbering, &mut state).unwrap(); + let b = generate_marker(second, 0, &numbering, &mut state).unwrap(); + assert_eq!(a.marker_text, "1."); + assert_eq!(b.marker_text, "1."); + } + + /// Each level carries its own indent, and deeper levels step further in. + #[test] + fn level_paragraph_properties_expose_per_level_indent() { + let mut numbering = CT_Numbering::new(); + let num_id = numbering.add_numbered_list(); + + let lvl0 = level_paragraph_properties(num_id, 0, &numbering) + .expect("level 0 should carry paragraph properties"); + let lvl1 = level_paragraph_properties(num_id, 1, &numbering) + .expect("level 1 should carry paragraph properties"); + + let left0 = lvl0.ind_left.expect("level 0 indent").0; + let left1 = lvl1.ind_left.expect("level 1 indent").0; + assert!( + left1 > left0, + "level 1 must indent further than level 0, got {left0} then {left1}" + ); + } } diff --git a/scripts/hash_baseline.json b/scripts/hash_baseline.json index 1d727db3..5ce72483 100644 --- a/scripts/hash_baseline.json +++ b/scripts/hash_baseline.json @@ -1,6 +1,6 @@ { "entries": { - "contract:page1.png": "23cc71026c8cc3ba4860e517ca2ba9df9062caed69baeacaed21a911d5bc6ba5", + "contract:page1.png": "504977f2017680736254709429370708554166410856873fd5c38a1d5283b98a", "contract:word/document.xml": "1ecd9138e09122a5980ca80451ddd8f93d4e6d3af8bb80e582dc8cb697277dfe", "contract:word/numbering.xml": "01d6cb0aa0ecc30b3d2a77c4df062b1c727d70ff9cbc0824910f049daf878a26", "contract:word/styles.xml": "0dc0b047b6019b798b83bfe4d66eb91d14b03caea3f6bdb0934d48fbb863fba4", @@ -12,7 +12,7 @@ "invoice:word/document.xml": "bb2d71711f6a613044dfabc166ae340354345666a98687e02df815efa220c655", "invoice:word/numbering.xml": null, "invoice:word/styles.xml": "0dc0b047b6019b798b83bfe4d66eb91d14b03caea3f6bdb0934d48fbb863fba4", - "letter:page1.png": "402d059d7918aaab46c92bd0c12eab69ddf1e0129c86560d466d7f750db0da8b", + "letter:page1.png": "d3a00cf6c37a1eb6c1897e36b062c10e8b0fdc8c2a6f26c1034026f83578d1c0", "letter:word/document.xml": "54f74a133887b1b93da1070517f9d7f687dbdc27b1307f1696c4b41f66d5fc05", "letter:word/numbering.xml": "c6511604704117eb00ad2faffb9173e4f48d22f62557ca72a51d60b7907c8058", "letter:word/styles.xml": "0dc0b047b6019b798b83bfe4d66eb91d14b03caea3f6bdb0934d48fbb863fba4", @@ -29,5 +29,5 @@ "report:word/numbering.xml": "2aa0599486d98573be0b7febb04ec63c08d3dfc5341b0f722b93cae113803bbc", "report:word/styles.xml": "0dc0b047b6019b798b83bfe4d66eb91d14b03caea3f6bdb0934d48fbb863fba4" }, - "reason": "F-003 initial deterministic baseline" + "reason": "List levels now apply their own w:pPr indent (#13), so page-one PNGs for letter and contract shift. Verified by rendering both before and after: the only visual change is list markers moving to their level indent with correct hanging indent. No other content moved." }