Merge pull request #1275 from storytold/codex/docx-wordcraft-parity

fix(docx,wordmodel): style revisions, lnNumType guard, deterministic Paragraph Debug

crates/docx/src/read/props.rs

@@ -817,20 +817,21 @@ pub fn sectpr(pc: &PropCtx, e: &El) -> (SectionProps, Vec<HfRef>) {
                 s.footnote_columns = k.attr("w:val").and_then(u32_of).filter(|v| (1..=45).contains(v));
             }
             "w:lnNumType" => {
-                let mut l = LineNumbering::default();
-                if let Some(v) = k.attr("w:countBy").and_then(u32_of) {
-                    l.count_by = v.clamp(1, 100);
+                // Word leaves line numbering off unless `w:countBy` gives an interval above 0,
+                // whatever `w:start`, `w:distance` and `w:restart` say (#6209, observed in Word).
+                if let Some(count_by) = k.attr("w:countBy").and_then(u32_of).filter(|&v| v > 0) {
+                    let mut l = LineNumbering { count_by: count_by.clamp(1, 100), ..LineNumbering::default() };
+                    if let Some(v) = k.attr("w:start").and_then(int) {
+                        l.start = (v.clamp(0, 32_767) + 1) as u32;
+                    }
+                    l.distance = tw(k, "w:distance").unwrap_or(0.0).max(0.0);
+                    l.restart = match k.attr("w:restart") {
+                        Some("newSection") => LineNumberRestart::Section,
+                        Some("continuous") => LineNumberRestart::Continuous,
+                        _ => LineNumberRestart::Page,
+                    };
+                    s.line_numbers = Some(l);
                 }
-                if let Some(v) = k.attr("w:start").and_then(int) {
-                    l.start = (v.clamp(0, 32_767) + 1) as u32;
-                }
-                l.distance = tw(k, "w:distance").unwrap_or(0.0).max(0.0);
-                l.restart = match k.attr("w:restart") {
-                    Some("newSection") => LineNumberRestart::Section,
-                    Some("continuous") => LineNumberRestart::Continuous,
-                    _ => LineNumberRestart::Page,
-                };
-                s.line_numbers = Some(l);
             }
             "w:vAlign" => {
                 s.valign = match k.attr("w:val") {

crates/docx/src/read/styles.rs

@@ -1,8 +1,9 @@
 use super::{
-    AbstractNum, DocxError, El, Level, LevelSuffix, Num, NumFormat, PartFormat, Reader, Style, StyleKind, StyleSheet, TablePart, TableStyleParts,
-    VAlign, flag, int, props, tw, u32_of,
+    AbstractNum, DocxError, El, Level, LevelSuffix, Num, NumFormat, PartFormat, Reader, RevisionKind, Style, StyleKind, StyleSheet, TablePart,
+    TableStyleParts, VAlign, flag, int, props, tw, u32_of,
 };
 use crate::units::tw_i16;
+use craft_wordmodel::props::PropChange;
 use std::sync::Arc;
 
 /// Built-in style names Word stores in lower case, and how the UI shows them.
@@ -137,26 +138,50 @@ impl Reader<'_> {
                     ),
                 ..Default::default()
             };
+            let mut old_para = None;
+            let mut old_chr = None;
             if let Some(p) = s.child("w:pPr") {
                 st.para = self.pc.ppr(p).0;
+                if let Some(ch) = p.child("w:pPrChange") {
+                    let old = ch.child("w:pPr").map(|o| self.pc.ppr(o).0.formatting()).unwrap_or_default();
+                    old_para = Some((self.revision(RevisionKind::Format, ch), old));
+                }
             }
             // Word gives its built-in heading styles their outline level whether or not the file
             // writes `w:outlineLvl` (some producers leave it out): "heading 2" is level 2 in the
             // Navigation pane and the table of contents, whatever its id (#4696).
             if kind == StyleKind::Paragraph
                 && st.builtin
-                && st.para.outline_level.is_none()
                 && let Some(level) = s.child_val("w:name").and_then(builtin_heading_level)
             {
-                st.para.outline_level = Some(level);
+                if st.para.outline_level.is_none() {
+                    st.para.outline_level = Some(level);
+                }
+                if let Some((_, old)) = &mut old_para
+                    && old.outline_level.is_none()
+                {
+                    old.outline_level = Some(level);
+                }
             }
             if let Some(rp) = s.child("w:rPr") {
                 st.chr = self.pc.rpr(rp);
+                if let Some(ch) = rp.child("w:rPrChange") {
+                    let old = ch.child("w:rPr").map(|o| self.pc.rpr(o).formatting()).unwrap_or_default();
+                    old_chr = Some((self.revision(RevisionKind::Format, ch), old));
+                }
             }
             if kind == StyleKind::Table {
                 let parts = self.table_style_parts(s, &mut st);
                 st.table = (parts != TableStyleParts::default()).then_some(parts);
             }
+            // CT_Style has no `stylePrChange`. Keep its two valid revision carriers separate: a
+            // foreign document may attribute paragraph and run changes to different authors.
+            if let Some((rev, old)) = old_para {
+                st.para_change = PropChange::boxed(rev, old);
+            }
+            if let Some((rev, old)) = old_chr {
+                st.chr_change = PropChange::boxed(rev, old);
+            }
             // A style defined twice (some generators write their own Heading 1 after a
             // placeholder one) is the last definition, whole: Word drops the earlier one rather
             // than merging the two (observed with synthetic documents).
@@ -181,6 +206,24 @@ impl Reader<'_> {
         for (s, n) in sheet.styles.iter_mut().zip(fixed) {
             s.para.numbering = n;
         }
+        let old_fixed: Vec<Option<Option<craft_wordmodel::props::NumRef>>> = sheet
+            .styles
+            .iter()
+            .map(|s| {
+                s.para_change.as_ref().map(|ch| match ch.old.numbering {
+                    Some(n) if n.num == props::LEVEL_ONLY => {
+                        let base = s.based_on.as_deref().and_then(|b| props::chain_list(&sheet, b));
+                        base.map(|num| craft_wordmodel::props::NumRef { num, level: n.level })
+                    }
+                    other => other,
+                })
+            })
+            .collect();
+        for (s, n) in sheet.styles.iter_mut().zip(old_fixed) {
+            if let (Some(ch), Some(n)) = (&mut s.para_change, n) {
+                ch.old.numbering = n;
+            }
+        }
         if sheet.default_para.numbering.is_some_and(|n| n.num == props::LEVEL_ONLY) {
             sheet.default_para.numbering = None;
         }

crates/docx/src/write/glossary.rs

@@ -87,7 +87,7 @@ impl Writer<'_> {
         }
         rels.add(rt::GLOSSARY, &file(part), false);
         push_part(entries, overrides, part, w.into_bytes(), GLOSSARY_CONTENT_TYPE, prels);
-        push_part(entries, overrides, styles, styles_xml(doc), &format!("{CT_WML}styles+xml"), PartRels::default());
+        push_part(entries, overrides, styles, styles_xml(self), &format!("{CT_WML}styles+xml"), PartRels::default());
         if lists {
             push_part(entries, overrides, numbering, numbering_xml(doc), &format!("{CT_WML}numbering+xml"), PartRels::default());
         }

crates/docx/src/write/parts.rs

@@ -126,7 +126,7 @@ pub(crate) fn write_parts(doc: &Document, flavor: Flavor) -> Result<Vec<(String,
     write_comments(doc, &comment_paras, &ns_refs, &mut wr, &mut entries, &mut overrides, &mut rels);
 
     rels.add(rt::STYLES, "styles.xml", false);
-    push_part(&mut entries, &mut overrides, "word/styles.xml", styles_xml(doc), &format!("{CT_WML}styles+xml"), PartRels::default());
+    push_part(&mut entries, &mut overrides, "word/styles.xml", styles_xml(&mut wr), &format!("{CT_WML}styles+xml"), PartRels::default());
     if !doc.numbering.nums.is_empty() || !doc.numbering.abstracts.is_empty() {
         rels.add(rt::NUMBERING, "numbering.xml", false);
         push_part(&mut entries, &mut overrides, "word/numbering.xml", numbering_xml(doc), &format!("{CT_WML}numbering+xml"), PartRels::default());

crates/docx/src/write/story/para.rs

@@ -294,7 +294,7 @@ impl Writer<'_> {
     }
 
     /// `w:id`, `w:author` and `w:date` of a tracked change by revision `idx`.
-    pub(super) fn change_attrs(&mut self, idx: u32) -> ChangeAttrs {
+    pub(crate) fn change_attrs(&mut self, idx: u32) -> ChangeAttrs {
         let (id, author, date) = self.rev_attrs(idx);
         let mut a = vec![("w:id", id), ("w:author", author)];
         if !date.is_empty() {

crates/docx/src/write/styles.rs

@@ -1,4 +1,6 @@
-use super::{Document, Level, LevelSuffix, ParaProps, Style, StyleKind, TablePart, VAlign, W, props, xml};
+use super::{Level, LevelSuffix, ParaProps, Style, StyleKind, TablePart, VAlign, W, Writer, props, xml};
+use crate::write::props::open_change;
+use craft_wordmodel::Document;
 
 /// Built-in names Word keeps in lower case in styles.xml.
 fn file_style_name(name: &str) -> String {
@@ -16,7 +18,8 @@ fn file_style_name(name: &str) -> String {
     name.to_string()
 }
 
-pub(super) fn styles_xml(doc: &Document) -> Vec<u8> {
+pub(super) fn styles_xml(wr: &mut Writer<'_>) -> Vec<u8> {
+    let doc = wr.doc;
     let s = &doc.styles;
     let mut w = W::new();
     let ns = xml::body_ns();
@@ -50,13 +53,15 @@ pub(super) fn styles_xml(doc: &Document) -> Vec<u8> {
     // Kinds whose default style the file named (`w:default`); the English ids stand in otherwise.
     let flagged: Vec<StyleKind> = s.styles.iter().filter(|st| st.default).map(|st| st.kind).collect();
     for st in &s.styles {
-        style_xml(&mut w, st, &flagged);
+        style_xml(wr, &mut w, st, &flagged);
     }
     w.close("w:styles");
     w.into_bytes()
 }
 
-fn style_xml(w: &mut W, st: &Style, flagged: &[StyleKind]) {
+/// One `w:style` element. A tracked change of its paragraph/run properties is nested in the
+/// corresponding property element as `w:pPrChange` / `w:rPrChange`.
+fn style_xml(wr: &mut Writer<'_>, w: &mut W, st: &Style, flagged: &[StyleKind]) {
     let ty = match st.kind {
         StyleKind::Paragraph => "paragraph",
         StyleKind::Character => "character",
@@ -102,16 +107,7 @@ fn style_xml(w: &mut W, st: &Style, flagged: &[StyleKind]) {
     if st.locked {
         w.empty("w:locked", &[]);
     }
-    if !st.para.is_empty() && st.kind != StyleKind::Character {
-        w.open("w:pPr", &[]);
-        props::ppr_inner(w, &st.para, false, None);
-        w.close("w:pPr");
-    }
-    if props::has_rpr(&st.chr) && st.kind != StyleKind::Numbering {
-        w.open("w:rPr", &[]);
-        props::rpr_inner(w, &st.chr);
-        w.close("w:rPr");
-    }
+    style_property_xml(wr, w, st);
     if let Some(t) = &st.table {
         if t.borders.is_some() || t.cell_margins.is_some() || t.band_size.is_some() || t.col_band_size.is_some() {
             w.open("w:tblPr", &[]);
@@ -179,6 +175,37 @@ fn style_xml(w: &mut W, st: &Style, flagged: &[StyleKind]) {
     w.close("w:style");
 }
 
+fn style_property_xml(wr: &mut Writer<'_>, w: &mut W, st: &Style) {
+    let para_change = st.para_change.as_deref().filter(|ch| ch.old.formatting() != st.para.formatting());
+    if (!st.para.is_empty() || para_change.is_some()) && st.kind != StyleKind::Character {
+        w.open("w:pPr", &[]);
+        props::ppr_inner(w, &st.para, false, None);
+        if let Some(ch) = para_change {
+            let a = wr.change_attrs(ch.rev);
+            open_change(w, "w:pPrChange", &a);
+            w.open("w:pPr", &[]);
+            props::ppr_inner(w, &ch.old, false, None);
+            w.close("w:pPr");
+            w.close("w:pPrChange");
+        }
+        w.close("w:pPr");
+    }
+    let run_change = st.chr_change.as_deref().filter(|ch| ch.old.formatting() != st.chr.formatting());
+    if (props::has_rpr(&st.chr) || run_change.is_some()) && st.kind != StyleKind::Numbering {
+        w.open("w:rPr", &[]);
+        props::rpr_inner(w, &st.chr);
+        if let Some(ch) = run_change {
+            let a = wr.change_attrs(ch.rev);
+            open_change(w, "w:rPrChange", &a);
+            w.open("w:rPr", &[]);
+            props::rpr_inner(w, &ch.old);
+            w.close("w:rPr");
+            w.close("w:rPrChange");
+        }
+        w.close("w:rPr");
+    }
+}
+
 pub(super) fn numbering_xml(doc: &Document) -> Vec<u8> {
     let mut w = W::new();
     let ns = xml::body_ns();

crates/docx/tests/it/roundtrip.rs

@@ -269,6 +269,7 @@ fn styles_and_lists_round_trip() {
         locked: true,
         default: false,
         table: None,
+        ..Default::default()
     });
     Arc::make_mut(&mut d.styles).upsert(Style {
         id: "MyStyleChar".into(),

crates/wordmodel/src/para.rs

@@ -24,7 +24,7 @@ pub use inline_object::{CanvasStyle, GroupChild, InlineObject, MAX_GROUP_CHILDRE
 pub use run::{COLUMN_BREAK, LINE_BREAK, NB_HYPHEN, NBSP, OBJ, PAGE_BREAK, Run, SOFT_HYPHEN, next_rev};
 
 /// A paragraph.
-#[derive(Clone, Debug, Serialize, Deserialize)]
+#[derive(Clone, Serialize, Deserialize)]
 pub struct Paragraph {
     pub text: String,
     pub runs: Vec<Run>,
@@ -54,6 +54,19 @@ impl PartialEq for Paragraph {
     }
 }
 
+impl std::fmt::Debug for Paragraph {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.debug_struct("Paragraph")
+            .field("text", &self.text)
+            .field("runs", &self.runs)
+            .field("props", &self.props)
+            .field("mark", &self.mark)
+            .field("objects", &self.objects)
+            .field("section", &self.section)
+            .finish_non_exhaustive()
+    }
+}
+
 impl Default for Paragraph {
     fn default() -> Self {
         Paragraph::new()

crates/wordmodel/src/styles.rs

@@ -6,8 +6,8 @@ use serde::{Deserialize, Serialize};
 use std::collections::BTreeMap;
 
 use crate::props::{
-    Align, Border, BorderStyle, Borders, CharProps, LineSpacing, ParaProps, Rgb, TabAlign, TabLeader, TabStop, TableLook, TextColor, Underline,
-    VAlign,
+    Align, Border, BorderStyle, Borders, CharProps, LineSpacing, ParaProps, PropChange, Rgb, TabAlign, TabLeader, TabStop, TableLook, TextColor,
+    Underline, VAlign,
 };
 
 #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Serialize, Deserialize, Default)]
@@ -52,6 +52,12 @@ pub struct Style {
     pub default: bool,
     /// Table style: conditional formatting for header row, banding etc.
     pub table: Option<TableStyleParts>,
+    /// Tracked change to the style's paragraph properties (`w:pPrChange`).
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub para_change: Option<Box<PropChange<ParaProps>>>,
+    /// Tracked change to the style's run properties (`w:rPrChange`).
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub chr_change: Option<Box<PropChange<CharProps>>>,
 }
 
 /// Table style conditional formats (shading + character formatting per region).