From 267a1524148a0e615231dcaa9ebbc6d4a783f9d2 Mon Sep 17 00:00:00 2001 From: Hadrien Mary Date: Sun, 27 Sep 2026 17:20:30 +0200 Subject: [PATCH 1/5] Ignore the content-control id when comparing documents compare() refused any pair whose content controls differed only by w:sdtPr/w:id, with "comparison cannot revise content-control properties". Word and Google Docs renumber that id when they save, so two versions of a document with a table-of-contents control could not be compared at all, although nothing in the control had changed. The id was part of control_property_signature, the one tuple that drives control alignment, the refusal and the accept and reject postconditions. Leaving it out of that tuple keeps the three consistent. Controls that differ only by id now align as unchanged and the redline keeps the original w:sdtPr bytes, and a change inside such a control is revised like any other. A w:sdtPr that holds none of the compared properties reads like no w:sdtPr, so a control that gains or loses an id-only w:sdtPr compares too. A differing w:tag, alias, control type or data binding still refuses the pair. HLD 03 now says the id is not part of the control shell. GitHub issue #159. --- crates/rdocx/src/comparison.rs | 28 ++++-- crates/rdocx/tests/regression_test.rs | 128 ++++++++++++++++++++++++++ docs/hld/03-architecture.md | 9 +- 3 files changed, 152 insertions(+), 13 deletions(-) diff --git a/crates/rdocx/src/comparison.rs b/crates/rdocx/src/comparison.rs index 19c557ed..5553d681 100644 --- a/crates/rdocx/src/comparison.rs +++ b/crates/rdocx/src/comparison.rs @@ -29,7 +29,6 @@ thread_local! { type ControlPropertySignature<'a> = Option<( Option<&'a str>, Option<&'a str>, - Option, Option, Option<&'a rdocx_oxml::content_control::CT_DataBinding>, )>; @@ -5478,16 +5477,25 @@ fn control_signature(control: &CT_Sdt) -> String { ) } +/// The content-control properties that alignment, refusal and the accept and +/// reject postconditions compare. +/// +/// `w:id` is left out. Producers renumber it on save and it carries no +/// content, so a pair that differs only by it keeps the original's `w:sdtPr`. +/// A `w:sdtPr` with none of these properties reads like no `w:sdtPr`. fn control_property_signature(control: &CT_Sdt) -> ControlPropertySignature<'_> { - control.properties.as_ref().map(|properties| { - ( - properties.alias.as_deref(), - properties.tag.as_deref(), - properties.id, - properties.control_type, - properties.data_binding.as_ref(), - ) - }) + control + .properties + .as_ref() + .map(|properties| { + ( + properties.alias.as_deref(), + properties.tag.as_deref(), + properties.control_type, + properties.data_binding.as_ref(), + ) + }) + .filter(|signature| !matches!(signature, (None, None, None, None))) } fn modeled_control_content(control: &CT_Sdt) -> Vec<&SdtContent> { diff --git a/crates/rdocx/tests/regression_test.rs b/crates/rdocx/tests/regression_test.rs index d143b3c7..dbd61d6f 100644 --- a/crates/rdocx/tests/regression_test.rs +++ b/crates/rdocx/tests/regression_test.rs @@ -30379,6 +30379,134 @@ fn comparison_treats_empty_paragraph_properties_as_absent() { assert!(document_xml(&mut empty).contains(" Vec { + document + .revisions() + .iter() + .map(|revision| revision.kind()) + .collect() + } + + /// Check that accepting gives the edited side and rejecting the original, + /// each compared again with no diagnostic and no revision. + fn assert_resolutions(tracked: &[u8], original: &Document, edited: &Document) { + for (resolve, expected) in [ + ( + Document::accept_all as fn(&mut Document) -> rdocx::Result, + edited, + ), + (Document::reject_all, original), + ] { + let mut resolved = Document::from_bytes(tracked).unwrap(); + resolve(&mut resolved).unwrap(); + let diagnostics = resolved + .compare(expected, "postcondition", TIMESTAMP) + .unwrap(); + assert!(diagnostics.is_empty(), "{diagnostics:?}"); + assert_eq!(revision_kinds(&resolved), []); + } + } + + /// Compare two bodies and return the revision kinds and the redline. + fn compared_kinds(original_xml: &str, edited_xml: &str) -> (Vec, String) { + let original = document_with_content_controls(original_xml); + let edited = document_with_content_controls(edited_xml); + let mut compared = document_with_content_controls(original_xml); + let diagnostics = compared + .compare(&edited, "R", TIMESTAMP) + .expect("producer noise must not refuse the pair"); + assert!(diagnostics.is_empty(), "{diagnostics:?}"); + assert_resolutions(&compared.to_bytes().unwrap(), &original, &edited); + (revision_kinds(&compared), document_xml(&mut compared)) + } + + fn table_of_contents_control(id: Option<&str>, first_entry: &str) -> String { + let id = id.map_or_else(String::new, |value| format!(r#""#)); + wrap_word_body(&format!( + r#"Before the content control.{id}{first_entry} entryBeta entryGamma entryAfter the content control."# + )) + } + + fn google_docs_inline_control(id: &str, word: &str) -> String { + wrap_word_body(&format!( + r#"Before {word} after."# + )) + } + + #[test] + fn a_content_control_identity_is_not_content() { + for (original, edited, kept_id) in [ + (None, Some("-2000000001"), None), + (Some("-2000000001"), None, Some("-2000000001")), + (Some("11"), Some("12"), Some("11")), + ] { + let (kinds, tracked) = compared_kinds( + &table_of_contents_control(original, "Alpha"), + &table_of_contents_control(edited, "Alpha"), + ); + assert_eq!(kinds, [], "{original:?} -> {edited:?}"); + let ids = [original, edited] + .into_iter() + .flatten() + .filter(|id| tracked.contains(&format!(r#""#))) + .collect::>(); + assert_eq!(ids, kept_id.into_iter().collect::>(), "{tracked}"); + + let (kinds, tracked) = compared_kinds( + &table_of_contents_control(original, "Alpha"), + &table_of_contents_control(edited, "Delta"), + ); + assert_eq!( + kinds, + [RevisionKind::Deletion, RevisionKind::Insertion], + "{original:?} -> {edited:?}" + ); + assert_eq!(tracked.matches("").count(), 1, "{tracked}"); + } + + let (kinds, _) = compared_kinds( + &google_docs_inline_control("-1854911024", "Alpha"), + &google_docs_inline_control("1374263513", "Alpha"), + ); + assert_eq!(kinds, []); + let (kinds, tracked) = compared_kinds( + &google_docs_inline_control("-1854911024", "Alpha"), + &google_docs_inline_control("1374263513", "Delta"), + ); + assert_eq!(kinds, [RevisionKind::Deletion, RevisionKind::Insertion]); + assert!( + tracked.contains(r#""#), + "{tracked}" + ); + + let bare_control = |properties: &str| { + wrap_word_body(&format!( + r#"{properties}Alpha entryAfter the content control."# + )) + }; + let id_only = r#""#; + for (original, edited) in [("", id_only), (id_only, "")] { + let (kinds, tracked) = compared_kinds(&bare_control(original), &bare_control(edited)); + assert_eq!(kinds, [], "{original:?} -> {edited:?}"); + assert_eq!( + tracked.contains(""), + !original.is_empty(), + "{tracked}" + ); + } + } +} + #[test] fn unmodelled_property_changes_report_a_diagnostic() { for (original, edited, expected_location) in [ diff --git a/docs/hld/03-architecture.md b/docs/hld/03-architecture.md index 97ce19c6..ca07b49e 100644 --- a/docs/hld/03-architecture.md +++ b/docs/hld/03-architecture.md @@ -910,9 +910,12 @@ section properties emit property revisions that retain the original property sidecars. Unsupported formatting differences retain the original bytes and produce stable `ComparisonDiagnostic` values at the actual story path. Inputs with existing modeled revisions or differing story and control shells are -rejected unless their story category is ignored. Attributed text alignment -retains owner, formatting, content position, and raw-child boundaries, then -coalesces adjacent equal-owner edits into minimal revision wrappers. +rejected unless their story category is ignored. A content control's `w:id` +is producer identity and not part of its shell, so controls that differ only +by it align, compare, and keep the original `w:sdtPr`. Attributed text +alignment retains owner, formatting, content position, and raw-child +boundaries, then coalesces adjacent equal-owner edits into minimal revision +wrappers. When a main story gains a trailing run of paragraphs, comparison marks the original final paragraph boundary once, marks each intermediate inserted paragraph boundary once, and leaves the final inserted paragraph mark as the From 2e6b3b0cfd211978b8a453d2bbc4ef516a5f406f Mon Sep 17 00:00:00 2001 From: Hadrien Mary Date: Sun, 27 Sep 2026 17:21:08 +0200 Subject: [PATCH 2/5] Keep xml:space on replaced runs and compare it only at text edges A run rewritten by try_replace_text lost xml:space="preserve" on its w:t, because replace_in_single_run and replace_across_runs recomputed the flag from the new text alone. Google Docs writes the flag on every w:t, so a no-op replacement, common in a scripted pass that normalizes wording, turned an unchanged run into a deletion and an insertion once the edited copy was compared against its source. The three rewrite sites, which regex replacement shares, now keep a flag the producer wrote and still add one when the new text starts or ends with a space. compare() also read the flag as content wherever it appeared, because the run signature formatted CT_Text whole. The flag only changes how text reads when whitespace sits at an edge, so the text signature now counts it only there. The same signature serves alignment and the accept and reject postconditions, so they stay consistent, and a run that matches keeps the original bytes. Word and character granularity and the ignore options compare text units, which the redline writes back as their own w:t. Each unit copied the flag of its text, so a space split out of a flagged text differed from the same space in an unflagged one. A unit with whitespace at an edge now always carries the flag, so on that path whitespace reads as whitespace whatever the source flag was, and the flag is never content. Files from two producers that place the flag differently then compare the same way at every granularity, and a space split out of a text keeps the flag in the redline, where Word used to drop it. On that path, the shortcut that keeps runs whole when every run of a paragraph matches reads the flag the same way. Otherwise a run that differed only by the flag at an edge was rewritten one unit per run, which moved a bookmark end indexed by run and refused the pair. GitHub issue #160. --- crates/rdocx-oxml/src/placeholder.rs | 37 +++++- crates/rdocx/src/comparison.rs | 51 +++++++- crates/rdocx/tests/regression_test.rs | 171 +++++++++++++++++++++++++- 3 files changed, 248 insertions(+), 11 deletions(-) diff --git a/crates/rdocx-oxml/src/placeholder.rs b/crates/rdocx-oxml/src/placeholder.rs index 0bf078dd..59780350 100644 --- a/crates/rdocx-oxml/src/placeholder.rs +++ b/crates/rdocx-oxml/src/placeholder.rs @@ -153,7 +153,9 @@ fn replace_in_single_run( new_text.push_str(replacement); new_text.push_str(&t.text[byte_end..]); t.text = new_text; - t.preserve_space = t.text.starts_with(' ') || t.text.ends_with(' '); + // Keep the flag the producer wrote. Dropping it rewrites an unchanged + // run, which a later comparison against the source then reports. + t.preserve_space = t.preserve_space || t.text.starts_with(' ') || t.text.ends_with(' '); } } @@ -176,7 +178,7 @@ fn replace_across_runs( new_text.push_str(&t.text[..first_byte_offset]); new_text.push_str(replacement); t.text = new_text; - t.preserve_space = t.text.starts_with(' ') || t.text.ends_with(' '); + t.preserve_space = t.preserve_space || t.text.starts_with(' ') || t.text.ends_with(' '); } // Handle the last run: replace from start to match end within that content item. @@ -189,7 +191,7 @@ fn replace_across_runs( let ch_len = remaining.chars().next().map(|c| c.len_utf8()).unwrap_or(0); let byte_end = last_byte_offset + ch_len; t.text = t.text[byte_end..].to_string(); - t.preserve_space = t.text.starts_with(' ') || t.text.ends_with(' '); + t.preserve_space = t.preserve_space || t.text.starts_with(' ') || t.text.ends_with(' '); } // Clear text content from runs strictly between first and last. @@ -796,6 +798,35 @@ mod tests { assert_eq!(p.runs[1].properties.as_ref().unwrap().italic, Some(true)); } + #[test] + fn replace_keeps_the_producer_space_flag() { + let mut preserved = CT_R::new("WORD"); + if let RunContent::Text(text) = &mut preserved.content[0] { + text.preserve_space = true; + } + let mut p = CT_P::new(); + p.runs.push(preserved.clone()); + p.runs.push(preserved); + p.add_run("tail"); + + assert_eq!(replace_in_paragraph(&mut p, "WORD", "WORD"), 2); + assert_eq!(replace_in_paragraph(&mut p, "DWO", "D-WO"), 1); + assert_eq!(replace_in_paragraph(&mut p, "tail", " end"), 1); + let flags = p + .runs + .iter() + .map(|run| match &run.content[0] { + RunContent::Text(text) => (text.text.as_str(), text.preserve_space), + _ => unreachable!(), + }) + .collect::>(); + assert_eq!( + flags, + [("WORD-WO", true), ("RD", true), (" end", true)], + "a rewritten run keeps a producer flag and gains one at a text edge" + ); + } + #[test] fn replace_multiple_occurrences() { let mut p = make_para(&["{{x}} and {{x}}"]); diff --git a/crates/rdocx/src/comparison.rs b/crates/rdocx/src/comparison.rs index 5553d681..02c33e98 100644 --- a/crates/rdocx/src/comparison.rs +++ b/crates/rdocx/src/comparison.rs @@ -2984,8 +2984,16 @@ fn compare_granular_paragraph( ))); } - let original_run_signatures = original.runs.iter().map(run_signature).collect::>(); - let edited_run_signatures = edited.runs.iter().map(run_signature).collect::>(); + let original_run_signatures = original + .runs + .iter() + .map(attributed_run_signature) + .collect::>(); + let edited_run_signatures = edited + .runs + .iter() + .map(attributed_run_signature) + .collect::>(); if original_run_signatures == edited_run_signatures && original.content_controls == edited.content_controls { @@ -3557,12 +3565,30 @@ fn granular_text(text: &CT_Text, options: &ComparisonOptions) -> Vec { fragments .into_iter() .map(|value| CT_Text { + // A unit is written back as its own `w:t`, where edge whitespace + // needs the flag to survive. Whitespace in a unit then reads as + // whitespace whatever the source flag was. + preserve_space: text.preserve_space || has_edge_whitespace(&value), text: value, - preserve_space: text.preserve_space, }) .collect() } +/// The whole-run signature that agrees with the attributed units. +/// +/// It reads the space flag the way `granular_text` writes it on every unit, +/// so a run that differs only by the flag stays whole instead of being +/// rewritten one unit per run, which would move run-indexed bookmark ends. +fn attributed_run_signature(run: &CT_R) -> String { + let mut run = run.clone(); + for content in &mut run.content { + if let RunContent::Text(text) | RunContent::DeletedText(text) = content { + text.preserve_space |= has_edge_whitespace(&text.text); + } + } + run_signature(&run) +} + fn whitespace_fragments(text: &str) -> Vec { let mut output = Vec::new(); let mut current = String::new(); @@ -5410,10 +5436,29 @@ fn run_content_signature(content: &RunContent) -> String { match content { RunContent::Field(_) => "field-owner".to_owned(), RunContent::Drawing(drawing) => format!("Drawing({:?})", drawing_signature(drawing)), + RunContent::Text(text) => format!("Text({:?})", text_signature(text)), + RunContent::DeletedText(text) => format!("DeletedText({:?})", text_signature(text)), content => format!("{content:?}"), } } +/// The text and whether its `xml:space="preserve"` changes how it reads. +/// +/// The flag only protects whitespace at an edge of the text. Producers write +/// it on every `w:t` or only where needed, so elsewhere it is serialization. +fn text_signature(text: &CT_Text) -> (&str, bool) { + ( + &text.text, + text.preserve_space && has_edge_whitespace(&text.text), + ) +} + +/// Whether XML whitespace starts or ends the text. +fn has_edge_whitespace(text: &str) -> bool { + let whitespace = |character: char| matches!(character, ' ' | '\t' | '\n' | '\r'); + text.starts_with(whitespace) || text.ends_with(whitespace) +} + fn table_signature(table: &CT_Tbl) -> String { format!( "{:?}:{:?}:{:?}:{:?}", diff --git a/crates/rdocx/tests/regression_test.rs b/crates/rdocx/tests/regression_test.rs index dbd61d6f..5afa2345 100644 --- a/crates/rdocx/tests/regression_test.rs +++ b/crates/rdocx/tests/regression_test.rs @@ -30385,7 +30385,7 @@ fn comparison_treats_empty_paragraph_properties_as_absent() { /// revision. mod compare_producer_noise { use super::*; - use rdocx::RevisionKind; + use rdocx::{ComparisonGranularity, ComparisonOptions, RevisionKind}; const TIMESTAMP: &str = "2026-09-27T12:00:00Z"; @@ -30399,7 +30399,12 @@ mod compare_producer_noise { /// Check that accepting gives the edited side and rejecting the original, /// each compared again with no diagnostic and no revision. - fn assert_resolutions(tracked: &[u8], original: &Document, edited: &Document) { + fn assert_resolutions( + tracked: &[u8], + original: &Document, + edited: &Document, + options: &ComparisonOptions, + ) { for (resolve, expected) in [ ( Document::accept_all as fn(&mut Document) -> rdocx::Result, @@ -30410,7 +30415,7 @@ mod compare_producer_noise { let mut resolved = Document::from_bytes(tracked).unwrap(); resolve(&mut resolved).unwrap(); let diagnostics = resolved - .compare(expected, "postcondition", TIMESTAMP) + .compare_with_options(expected, "postcondition", TIMESTAMP, options) .unwrap(); assert!(diagnostics.is_empty(), "{diagnostics:?}"); assert_eq!(revision_kinds(&resolved), []); @@ -30419,14 +30424,22 @@ mod compare_producer_noise { /// Compare two bodies and return the revision kinds and the redline. fn compared_kinds(original_xml: &str, edited_xml: &str) -> (Vec, String) { + compared_kinds_with(original_xml, edited_xml, &ComparisonOptions::default()) + } + + fn compared_kinds_with( + original_xml: &str, + edited_xml: &str, + options: &ComparisonOptions, + ) -> (Vec, String) { let original = document_with_content_controls(original_xml); let edited = document_with_content_controls(edited_xml); let mut compared = document_with_content_controls(original_xml); let diagnostics = compared - .compare(&edited, "R", TIMESTAMP) + .compare_with_options(&edited, "R", TIMESTAMP, options) .expect("producer noise must not refuse the pair"); assert!(diagnostics.is_empty(), "{diagnostics:?}"); - assert_resolutions(&compared.to_bytes().unwrap(), &original, &edited); + assert_resolutions(&compared.to_bytes().unwrap(), &original, &edited, options); (revision_kinds(&compared), document_xml(&mut compared)) } @@ -30505,6 +30518,154 @@ mod compare_producer_noise { ); } } + + fn replaced_copy(source_xml: &str, old: &str, new: &str) -> String { + let mut document = document_with_content_controls(source_xml); + assert_eq!(document.try_replace_text(old, new).unwrap(), 1); + let mut edited = Document::from_bytes(&document.to_bytes().unwrap()).unwrap(); + document_xml(&mut edited) + } + + #[test] + fn a_rewritten_run_keeps_its_producer_space_flag() { + let preserved = wrap_word_body( + r#"Paragraph 1.WORD"#, + ); + let edited = replaced_copy(&preserved, "WORD", "WORD"); + assert!( + edited.contains(r#"WORD"#), + "{edited}" + ); + let (kinds, _) = compared_kinds(&preserved, &edited); + assert_eq!(kinds, []); + + let edge_space_removed = replaced_copy( + &wrap_word_body(r#"WORD "#), + "WORD ", + "WORD", + ); + assert!( + edge_space_removed.contains(r#"WORD"#), + "{edge_space_removed}" + ); + } + + #[test] + fn the_space_flag_is_content_only_at_a_text_edge() { + let paragraphs = |first: &str, second: &str| { + wrap_word_body(&format!( + r#"{first}{second}"# + )) + }; + let paragraph = |text: &str| paragraphs("Paragraph 1.", text); + // Bookmark ends are indexed by run, so an unchanged run must stay whole. + let bookmarked = |text: &str| { + wrap_word_body(&format!( + r#"{text}tail"# + )) + }; + let changed = &[RevisionKind::Deletion, RevisionKind::Insertion][..]; + let unchanged = &[][..]; + // Each case gives the kinds of the whole-run path and of the + // attributed path. The attributed path writes every unit with edge + // whitespace with the flag, so the flag is never content there. + let cases = [ + ( + paragraph(r#"WORD"#), + paragraph("WORD"), + unchanged, + unchanged, + ), + ( + paragraph(r#"two words"#), + paragraph("two words"), + unchanged, + unchanged, + ), + ( + paragraph("two words"), + paragraph(r#"two words"#), + unchanged, + unchanged, + ), + ( + paragraph(r#"WORD "#), + paragraph("WORD "), + changed, + unchanged, + ), + ( + bookmarked(r#"two words "#), + bookmarked("two words "), + changed, + unchanged, + ), + ( + paragraph("WORD"), + paragraph("text"), + changed, + changed, + ), + // Google Docs flags every `w:t` and Word only where needed. + ( + paragraphs( + r#"two words"#, + r#"WORD"#, + ), + paragraphs("two words", "text"), + changed, + changed, + ), + ]; + for options in [ + ComparisonOptions::default(), + ComparisonOptions { + ignore_formatting: true, + ..Default::default() + }, + ComparisonOptions { + granularity: ComparisonGranularity::Word, + ..Default::default() + }, + ComparisonOptions { + granularity: ComparisonGranularity::Character, + ..Default::default() + }, + ] { + for (original, edited, whole_run, attributed) in &cases { + let expected = if options == ComparisonOptions::default() { + whole_run + } else { + attributed + }; + for (left, right) in [(original, edited), (edited, original)] { + let (kinds, _) = compared_kinds_with(left, right, &options); + assert_eq!(kinds, *expected, "{options:?}: {left} -> {right}"); + } + } + } + + // A space split out of a text keeps reading as a space in the redline. + for granularity in [ + ComparisonGranularity::Word, + ComparisonGranularity::Character, + ] { + let (kinds, tracked) = compared_kinds_with( + ¶graph("two words"), + ¶graph("two wordy"), + &ComparisonOptions { + granularity, + ..Default::default() + }, + ); + assert_eq!(kinds, changed, "{granularity:?}"); + assert!(!tracked.contains(" "), "{tracked}"); + assert!( + tracked.contains(r#" "#), + "{tracked}" + ); + } + } } #[test] From c8880e276a7ac6c0414af67a5f45355db6a99973 Mon Sep 17 00:00:00 2001 From: Hadrien Mary Date: Sun, 27 Sep 2026 17:21:40 +0200 Subject: [PATCH 3/5] Treat the default portrait orientation as absent in compare Comparing a file whose w:pgSz carries w:orient="portrait" against its rdocx-edited copy reported a section_property_change next to the real edit. The pgSz writer emits w:orient only for landscape, so a modelled edit drops the default value, and section_properties_xml compared the two sections as whole CT_SectPr values, Some(Portrait) against None. Portrait is the schema default, so both modelled sections now read it as absent before the equality test. A paragraph that holds a bookmark, hyperlink, comment range or control compares its whole paragraph properties, section break included, so the same rule applies there. Without it such a section-break paragraph kept a formatting diagnostic, or refused the pair when its text changed. A real change to landscape is still a section property revision. The serializer is left alone, because the typed defaults of every generated document carry Some(Portrait) and writing it would move the document.xml hash baselines. GitHub issue #160. --- crates/rdocx/src/comparison.rs | 24 +++++++++++-- crates/rdocx/tests/regression_test.rs | 49 +++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 2 deletions(-) diff --git a/crates/rdocx/src/comparison.rs b/crates/rdocx/src/comparison.rs index 02c33e98..37d28126 100644 --- a/crates/rdocx/src/comparison.rs +++ b/crates/rdocx/src/comparison.rs @@ -11,6 +11,7 @@ use rdocx_oxml::content_control::{CT_Sdt, SdtContent}; use rdocx_oxml::document::{BodyContent, CT_Document}; use rdocx_oxml::namespace::W_NS; use rdocx_oxml::properties::CT_PPr; +use rdocx_oxml::shared::ST_PageOrientation; use rdocx_oxml::table::{CT_Row, CT_Tbl, CT_TblPr, CT_Tc, CT_TrPr, CellContent}; use rdocx_oxml::text::{CT_P, CT_R, CT_Text, RunContent}; use sha2::{Digest, Sha256}; @@ -4002,8 +4003,22 @@ fn nonempty_paragraph_properties(mut properties: CT_PPr) -> Option { } fn paragraph_properties_differ(original: Option<&CT_PPr>, edited: Option<&CT_PPr>) -> bool { - original.cloned().and_then(nonempty_paragraph_properties) - != edited.cloned().and_then(nonempty_paragraph_properties) + let modeled = |properties: Option<&CT_PPr>| { + properties.cloned().and_then(|mut properties| { + if let Some(section) = properties.sect_pr.as_mut() { + clear_default_orientation(section); + } + nonempty_paragraph_properties(properties) + }) + }; + modeled(original) != modeled(edited) +} + +/// Portrait is the schema default, which the section writer omits. +fn clear_default_orientation(section: &mut rdocx_oxml::document::CT_SectPr) { + if section.orientation == Some(ST_PageOrientation::Portrait) { + section.orientation = None; + } } fn section_properties_xml( @@ -4033,6 +4048,8 @@ fn section_properties_xml( original_modeled.change = None; let mut edited_modeled = edited.clone(); edited_modeled.change = None; + clear_default_orientation(&mut original_modeled); + clear_default_orientation(&mut edited_modeled); if original_modeled == edited_modeled { return section_property_xml(original); } @@ -5810,6 +5827,9 @@ fn paragraph_formatting(paragraph: &CT_P) -> Option { properties.numbering_revision_position = None; properties.change = None; properties.revision_xml.clear(); + if let Some(section) = properties.sect_pr.as_mut() { + clear_default_orientation(section); + } properties }) } diff --git a/crates/rdocx/tests/regression_test.rs b/crates/rdocx/tests/regression_test.rs index 5afa2345..08f17b7a 100644 --- a/crates/rdocx/tests/regression_test.rs +++ b/crates/rdocx/tests/regression_test.rs @@ -30666,6 +30666,55 @@ mod compare_producer_noise { ); } } + + fn page_body(orientation: &str) -> String { + wrap_word_body(&format!( + r#"Paragraph 1, lorem ipsum dolor sit amet.WORDParagraph 3, lorem ipsum dolor sit amet."# + )) + } + + #[test] + fn the_default_page_orientation_is_not_a_section_change() { + let portrait = page_body(r#" w:orient="portrait""#); + let edited = replaced_copy(&portrait, "3, lorem", "3, LOREM"); + assert!(!edited.contains("w:orient"), "{edited}"); + let (kinds, _) = compared_kinds(&portrait, &edited); + assert_eq!(kinds, [RevisionKind::Deletion, RevisionKind::Insertion]); + + let (kinds, _) = compared_kinds(&portrait, &page_body("")); + assert_eq!(kinds, []); + let (kinds, _) = compared_kinds(&page_body(""), &portrait); + assert_eq!(kinds, []); + + let (kinds, tracked) = compared_kinds(&portrait, &page_body(r#" w:orient="landscape""#)); + assert_eq!(kinds, [RevisionKind::SectionPropertyChange]); + assert!(tracked.contains(r#"w:orient="landscape""#), "{tracked}"); + + // A bookmark makes the section-break paragraph take the complex path, + // as Google Docs writes around headings. + let bookmarked_break = |orientation: &str, heading: &str| { + wrap_word_body(&format!( + r#"{heading}Second section."# + )) + }; + let portrait = r#" w:orient="portrait""#; + for (original, edited) in [(portrait, ""), ("", portrait)] { + let (kinds, _) = compared_kinds( + &bookmarked_break(original, "Heading"), + &bookmarked_break(edited, "Heading"), + ); + assert_eq!(kinds, [], "{original:?} -> {edited:?}"); + let (kinds, _) = compared_kinds( + &bookmarked_break(original, "Heading"), + &bookmarked_break(edited, "Title"), + ); + assert_eq!( + kinds, + [RevisionKind::Deletion, RevisionKind::Insertion], + "{original:?} -> {edited:?}" + ); + } + } } #[test] From a607736f5a7d8b2dc3789ab5893b9ccf2e4da104 Mon Sep 17 00:00:00 2001 From: Hadrien Mary Date: Sun, 27 Sep 2026 17:22:23 +0200 Subject: [PATCH 4/5] Compare a field packed in one run like the same field split into runs compare() failed with "complex field source has no end boundary" on a file against its own copy once update_page_fields had refreshed a field packed in one run, as Google Docs writes footer fields. The refreshed result lands inside that run and changes w:dirty, so compare_field_xml extracts the result on both sides, and complex_field_result looked for the end character only after the end of the run holding separate. In a packed run the end sits inside that same run. complex_field_result now splits the run so that separate ends a run and end starts one before it reads the result. The new runs repeat the original start tag and run properties, and a run with nothing on the far side of the character is left alone, so a field whose separate and end each have a run of their own yields the same bytes as before. The postconditions read a field as its owner only, so the split does not reach them. The same split fixes a quieter loss. update_page_fields writes the result of an uncached one-run-per-part field into the run of end, and the redline then carried an empty insertion without the new page number. It now inserts the result. GitHub issue #160. --- crates/rdocx/src/comparison.rs | 96 ++++++++++++++++++++++++--- crates/rdocx/tests/regression_test.rs | 96 +++++++++++++++++++++++++++ 2 files changed, 182 insertions(+), 10 deletions(-) diff --git a/crates/rdocx/src/comparison.rs b/crates/rdocx/src/comparison.rs index 37d28126..7f7ff9d1 100644 --- a/crates/rdocx/src/comparison.rs +++ b/crates/rdocx/src/comparison.rs @@ -5141,17 +5141,20 @@ fn deleted_text_xml(xml: &str) -> String { } fn complex_field_result(xml: &str) -> Result<(String, String, String)> { - let separate = xml - .find("fldCharType=\"separate\"") - .or_else(|| xml.find("fldCharType='separate'")) + let separate = field_character(xml, 0, "separate") .ok_or_else(|| Error::Other("complex field source has no separate boundary".to_owned()))?; - let (_, result_start) = containing_run(xml, separate)?; - let end_marker = xml[result_start..] - .find("fldCharType=\"end\"") - .or_else(|| xml[result_start..].find("fldCharType='end'")) - .map(|offset| result_start + offset) + // A producer may pack a whole field in one run, as Google Docs writes page + // fields. Ending a run after `separate` and starting one at `end` reads it + // as the same field written one run per part. + let xml = split_field_run(xml, separate, true)?; + let (_, result_start) = containing_run(&xml, separate)?; + let end_marker = field_character(&xml, result_start, "end") .ok_or_else(|| Error::Other("complex field source has no end boundary".to_owned()))?; - let (result_end, _) = containing_run(xml, end_marker)?; + let xml = split_field_run(&xml, end_marker, false)?; + // The split may have moved the `end` character further along. + let end_marker = field_character(&xml, result_start, "end") + .ok_or_else(|| Error::Other("complex field source has no end boundary".to_owned()))?; + let (result_end, _) = containing_run(&xml, end_marker)?; Ok(( xml[..result_start].to_owned(), xml[result_start..result_end].to_owned(), @@ -5159,6 +5162,48 @@ fn complex_field_result(xml: &str) -> Result<(String, String, String)> { )) } +fn field_character(xml: &str, from: usize, kind: &str) -> Option { + let tail = &xml[from..]; + tail.find(&format!("fldCharType=\"{kind}\"")) + .or_else(|| tail.find(&format!("fldCharType='{kind}'"))) + .map(|offset| from + offset) +} + +/// Split the run holding the field character at `marker` so that the +/// character ends its run (`after`) or starts it. +/// +/// Both runs repeat the original start tag and run properties. A run with no +/// content on that side of the character is returned unchanged. +fn split_field_run(xml: &str, marker: usize, after: bool) -> Result { + let (run_start, run_end) = containing_run(xml, marker)?; + let run = &xml[run_start..run_end]; + let children = direct_element_spans(run)?; + let character = children + .iter() + .position(|child| child.contains(&(marker - run_start))) + .ok_or_else(|| Error::Other("complex field character is not a run child".to_owned()))?; + let is_properties = |child: &Range| { + let mut reader = Reader::from_reader(run[child.clone()].as_bytes()); + matches!( + reader.read_event(), + Ok(Event::Start(element) | Event::Empty(element)) + if element.local_name().as_ref() == b"rPr" + ) + }; + let first_content = usize::from(children.first().is_some_and(is_properties)); + let split = if after { character + 1 } else { character }; + if split <= first_content || split >= children.len() { + return Ok(xml.to_owned()); + } + let head = &run[..children[first_content].start]; + let close = run + .rfind(" Result<(usize, usize)> { let mut reader = Reader::from_reader(xml.as_bytes()); reader.config_mut().trim_text(false); @@ -6360,7 +6405,8 @@ fn utf8_error(error: impl std::fmt::Display) -> Error { mod tests { use super::{ ComparisonGranularity, ComparisonOptions, FAIL_AFTER_COMPARISON_STAGING, - attributed_run_units, comparison_postcondition_error, story_document, word_fragments, + attributed_run_units, comparison_postcondition_error, complex_field_result, story_document, + word_fragments, }; use crate::Document; use rdocx_oxml::document::BodyContent; @@ -6452,6 +6498,36 @@ mod tests { ); } + #[test] + fn a_packed_field_result_is_read_as_one_run_per_part() { + for w in ["w", "q"] { + let shell = format!(r#"<{w}:r {w}:rsidR="00AB12CD"><{w}:rPr><{w}:b/>"#); + let close = format!(""); + let character = |kind: &str| format!(r#"<{w}:fldChar {w}:fldCharType="{kind}"/>"#); + let (begin, separate, end) = + (character("begin"), character("separate"), character("end")); + let code = format!("<{w}:instrText>PAGE"); + let result = format!("<{w}:t>1"); + let expected = ( + format!("{shell}{begin}{code}{separate}{close}"), + format!("{shell}{result}{close}"), + format!("{shell}{end}{close}"), + ); + for field in [ + format!("{shell}{begin}{code}{separate}{result}{end}{close}"), + format!("{shell}{begin}{code}{separate}{close}{shell}{result}{end}{close}"), + format!("{}{}{}", expected.0, expected.1, expected.2), + ] { + assert_eq!(complex_field_result(&field).unwrap(), expected, "{field}"); + } + let uncached = format!("{shell}{begin}{code}{separate}{end}{close}"); + assert_eq!( + complex_field_result(&uncached).unwrap(), + (expected.0, String::new(), expected.2) + ); + } + } + #[test] fn staged_comparison_postcondition_failure_preserves_bytes_and_layout_cache() { let mut original = Document::new(); diff --git a/crates/rdocx/tests/regression_test.rs b/crates/rdocx/tests/regression_test.rs index 08f17b7a..db90661d 100644 --- a/crates/rdocx/tests/regression_test.rs +++ b/crates/rdocx/tests/regression_test.rs @@ -30715,6 +30715,102 @@ mod compare_producer_noise { ); } } + + fn document_with_footer(footer_paragraph: &str) -> Document { + let mut seed = Document::new(); + let mut package = + oxml_opc::OpcPackage::from_reader(std::io::Cursor::new(seed.to_bytes().unwrap())) + .unwrap(); + package.set_part( + "/word/footer1.xml", + format!(r#"{footer_paragraph}"#).into_bytes(), + ); + package.content_types.add_override( + "/word/footer1.xml", + "application/vnd.openxmlformats-officedocument.wordprocessingml.footer+xml", + ); + let footer_id = package + .get_or_create_part_rels("/word/document.xml") + .add(oxml_opc::relationship::rel_types::FOOTER, "footer1.xml"); + package.set_part( + "/word/document.xml", + format!( + r#"Lorem ipsum dolor sit amet."# + ) + .into_bytes(), + ); + let mut bytes = std::io::Cursor::new(Vec::new()); + package.write_to(&mut bytes).unwrap(); + Document::from_bytes(bytes.get_ref()).unwrap() + } + + fn page_field(packed: bool, cached: bool) -> String { + let parts = [ + r#""#, + r#" PAGE "#, + r#""#, + if cached { "1" } else { "" }, + r#""#, + ]; + let field = if packed { + format!("{}", parts.concat()) + } else { + parts + .iter() + .filter(|part| !part.is_empty()) + .map(|part| format!("{part}")) + .collect() + }; + format!(r#"Page {field}"#) + } + + /// Compare a footer field against its copy refreshed by `update_page_fields`. + fn refreshed_field_comparison(packed: bool, cached: bool) -> String { + let source = document_with_footer(&page_field(packed, cached)) + .to_bytes() + .unwrap(); + let mut refreshed = Document::from_bytes(&source).unwrap(); + refreshed.update_page_fields().unwrap(); + let refreshed = Document::from_bytes(&refreshed.to_bytes().unwrap()).unwrap(); + let mut compared = Document::from_bytes(&source).unwrap(); + let diagnostics = compared + .compare(&refreshed, "R", TIMESTAMP) + .unwrap_or_else(|error| panic!("packed={packed} cached={cached}: {error}")); + assert!(diagnostics.is_empty(), "{diagnostics:?}"); + assert_resolutions( + &compared.to_bytes().unwrap(), + &Document::from_bytes(&source).unwrap(), + &refreshed, + &ComparisonOptions::default(), + ); + comparison_part_xml(&mut compared, "/word/footer1.xml") + } + + #[test] + fn a_packed_field_compares_like_the_same_field_split_into_runs() { + let separate = r#""#; + let result_and_end = |footer: &str| footer[footer.find(separate).unwrap()..].to_owned(); + for (cached, deleted) in [(true, "1"), (false, "")] { + let split = refreshed_field_comparison(false, cached); + let packed = refreshed_field_comparison(true, cached); + assert_eq!( + result_and_end(&split), + format!( + r#"{separate}{deleted}1"# + ), + "cached={cached}" + ); + assert_eq!( + result_and_end(&packed), + result_and_end(&split), + "cached={cached}" + ); + assert!( + packed.contains(r#" PAGE "#), + "{packed}" + ); + } + } } #[test] From 83b92bbf69942de8d7c3aacdeee19cc0da59ff26 Mon Sep 17 00:00:00 2001 From: Hadrien Mary Date: Sun, 27 Sep 2026 19:09:51 +0200 Subject: [PATCH 5/5] Re-record the archive measurements of rdocx and rdocx-oxml The comparison fixes, the kept xml:space flag and their tests grow the rdocx and rdocx-oxml packages, so the crates.io archive rows in README.md and crates/rdocx-oxml/README.md and their ARCHIVE_MEASUREMENTS entries are re-measured. GitHub issues #159 and #160. --- README.md | 2 +- crates/rdocx-oxml/README.md | 2 +- scripts/readme_doctests.py | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 479b462e..412db0df 100644 --- a/README.md +++ b/README.md @@ -39,7 +39,7 @@ rows are the enforced release-mode bounds plus one dated observation. | Measurement | Value | Version | Platform | Build mode | Input | Command | Statistic | Measured on | |---|---|---|---|---|---|---|---|---| -| Crates.io archive: rdocx | 1,092,256 compressed bytes, 6,498,484 member bytes, 36 members | 0.14.0 | macOS 26.6.2, Apple M5 Max, arm64 | `cargo package --locked --no-verify` | Tracked `rdocx` package inventory | `python3 scripts/readme_doctests.py --record-measurements` | gzip archive bytes, tar member bytes, tar member count | 2026-09-26 | +| Crates.io archive: rdocx | 1,097,973 compressed bytes, 6,523,992 member bytes, 36 members | 0.14.0 | macOS 26.6.2, Apple M5 Max, arm64 | `cargo package --locked --no-verify` | Tracked `rdocx` package inventory | `python3 scripts/readme_doctests.py --record-measurements` | gzip archive bytes, tar member bytes, tar member count | 2026-09-26 | | Large-document layout throughput | minimum 250 pages/s, observed 31,019.1 pages/s | rdocx 0.14.0 | macOS 26.6.2, Apple M5 Max, arm64 | release, one test thread | 1,000 one-page paragraphs with deterministic fonts | `cargo test -p rdocx --test regression_test --release a_thousand_page_document_paginates_and_renders_within_the_declared_limits -- --ignored --exact --nocapture --test-threads=1` | pages per wall-clock second | 2026-09-19 | | Large-document layout peak allocation | maximum 64 MiB, observed 29.03 MiB | rdocx 0.14.0 | macOS 26.6.2, Apple M5 Max, arm64 | release, one test thread | 1,000 one-page paragraphs with deterministic fonts | `cargo test -p rdocx --test regression_test --release a_thousand_page_document_paginates_and_renders_within_the_declared_limits -- --ignored --exact --nocapture --test-threads=1` | peak live allocation | 2026-09-19 | | Large-document PDF throughput | minimum 1,000 pages/s, observed 60,058.0 pages/s | rdocx 0.14.0 | macOS 26.6.2, Apple M5 Max, arm64 | release, one test thread | 1,000 deterministic layout pages | `cargo test -p rdocx --test regression_test --release a_thousand_page_document_paginates_and_renders_within_the_declared_limits -- --ignored --exact --nocapture --test-threads=1` | pages per wall-clock second | 2026-09-19 | diff --git a/crates/rdocx-oxml/README.md b/crates/rdocx-oxml/README.md index 11fef241..dabac199 100644 --- a/crates/rdocx-oxml/README.md +++ b/crates/rdocx-oxml/README.md @@ -17,7 +17,7 @@ schema order, and retains unmodelled XML alongside typed edits. | Measurement | Value | Version | Platform | Build mode | Input | Command | Statistic | Measured on | |---|---|---|---|---|---|---|---|---| -| Crates.io archive: rdocx-oxml | 367,500 compressed bytes, 2,380,047 member bytes, 32 members | 0.14.0 | macOS 26.6.2, Apple M5 Max, arm64 | `cargo package --locked --no-verify` | Tracked `rdocx-oxml` package inventory | `python3 scripts/readme_doctests.py --record-measurements` | gzip archive bytes, tar member bytes, tar member count | 2026-09-19 | +| Crates.io archive: rdocx-oxml | 367,851 compressed bytes, 2,381,304 member bytes, 32 members | 0.14.0 | macOS 26.6.2, Apple M5 Max, arm64 | `cargo package --locked --no-verify` | Tracked `rdocx-oxml` package inventory | `python3 scripts/readme_doctests.py --record-measurements` | gzip archive bytes, tar member bytes, tar member count | 2026-09-19 | ## Use it when diff --git a/scripts/readme_doctests.py b/scripts/readme_doctests.py index e24c643b..25c233a4 100644 --- a/scripts/readme_doctests.py +++ b/scripts/readme_doctests.py @@ -383,12 +383,12 @@ class ReadmeCase: "oxml-opc": (92_122, 355_510, 12), "oxml-pdf": (66_015, 304_432, 14), "oxml-sml": (12_511, 49_803, 6), - "rdocx": (1_092_256, 6_498_484, 36), + "rdocx": (1_097_973, 6_523_992, 36), "rdocx-cli": (33_805, 145_256, 8), "rdocx-html": (15_486, 63_894, 11), "rdocx-layout": (255_752, 1_385_701, 15), "rdocx-opc": (3_655, 9_668, 6), - "rdocx-oxml": (367_500, 2_380_047, 32), + "rdocx-oxml": (367_851, 2_381_304, 32), "rdocx-pdf": (8_111, 26_758, 6), "rpptx": (407_658, 2_122_094, 16), "rpptx-chart": (6_648, 21_136, 6),