Conversation
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 tensorbee#159.
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 tensorbee#160.
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 tensorbee#160.
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 tensorbee#160.
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 tensorbee#159 and tensorbee#160.
Replacement in text boxes walks the raw XML of the document part and of each header and footer, parses the w:p children of every w:txbxContent and writes those paragraphs back. Every other child was skipped on read and never written. The document layer stores the rewritten part as soon as one text box in it has a hit, so a single hit deleted the tables, block content controls, bookmarks and empty paragraphs of every text box in that part. try_replace_text, replace_all, replace_regex and render_template all go through this walker, for the DrawingML and the VML copy of a text box alike. The walker now edits each paragraph in place and copies every other child through verbatim, so each text box keeps its content in its order. Only a paragraph that the edit changed is re-serialised. The others are copied as read, so they also keep the start-tag attributes that CT_P does not model, such as w:rsidR and w14:paraId. Two defects of the same loop are fixed with it. The closing tag is the one read from the part instead of a fixed w:txbxContent, which did not match a start tag under another prefix. A part that ends inside a text box is now an error instead of an endless read. GitHub issue tensorbee#160.
A replacement that spans runs puts its text in the first run and removes the runs it leaves empty. It removed them with a plain retain over the direct runs and then only clamped the hyperlink spans. The bookmarks, comment ranges, inline content controls, raw children, revisions, equations and ruby spans of a paragraph are kept by run index, so each one after a removed run slid one run later, past the text it preceded, and a hyperlink could lose a run or gain the next one. The retain also removed every other run without typed content in the paragraph: an empty run, and a run holding only raw XML such as a drawing Word writes in mc:AlternateContent, which a single match anywhere in the paragraph deleted. The removal now goes through the remapping that comment removal already used, moved into a shared CT_P::remove_runs, so every anchor stays on the boundary that remains and a hyperlink left without runs is dropped. That remapping left the ruby spans out, so comment removal slid them too. It now moves them, and drops an annotation left without base runs, which the writer skips anyway. Only a run the replacement emptied is removed. A run keeps its start-tag attributes as a raw record, which does not count as content. GitHub issue tensorbee#160.
try_replace_text, replace_all, the regex replacement, render_template and rdocx replace skipped the text of content controls. The body walkers matched a body-level control to nothing, so a paragraph that a Google Docs export wraps at body level was never read. The paragraph replacement read only the direct runs, so a run wrapped in an inline control was missed at every level, in the body, in table cells, in text boxes and in header and footer paragraphs. Paragraph.text reads those runs since tensorbee#118, so the read and replace views disagreed on the same run. The text box walker copied its tables and block controls without reading them. The body walkers now go through the visitor that already reaches tables, cells and controls at every level. The paragraph replacement reads its direct runs and the runs of its inline controls, nested ones included, in the order CT_P::runs reads them. A match must lie within one stretch of runs: the direct runs between two controls, or the runs of one control between its nested ones. A match that straddles a control boundary is left as it is, never half replaced, and the search goes on after it, so a shorter match that a pattern such as \d+ finds inside it cannot replace part of it either. The direct runs on both sides of a control no longer read as one text, which matched words a reader does not see. A run the replacement empties inside a control is removed with its content index bookkeeping. The literal and the regex replacement share one table, row, cell and control recursion instead of two copies of it. The text box walker edits its tables and block controls through the typed parsers and writes back only those it changed. The typed writers leave out namespace declarations, so a rewritten element declares those of its start tag again. One that declares a namespace deeper down, other than the part binds it, keeps its bytes and counts nothing, since its rewrite would leave the prefix unbound or bound to another namespace. render_template reads its tags in the same stretches, so a tag inside an inline control, or in a table or control of a text box, is rendered, where it was left in the output before. A tag that straddles a control boundary is reported as an invalid template. GitHub issue tensorbee#160.
The header and footer model types only its paragraphs. Every other child of the part, such as a table or a content control like the page-number control Word writes in a footer, is kept as raw XML without its position, and a rewrite wrote all of them after the last paragraph. Replacement rewrote the part as soon as one paragraph matched, so a footer whose page-number control preceded its text came back with the control moved below it. Replacement never read those raw children either, so text in a header table or in a header or footer control was not replaced. The part was also read with trimmed text events, which a raw child is captured with, so the page-number control holding "Page " came back as "Page" once the part was rewritten. CT_HdrFtr now records how many paragraphs precede each raw child, and the namespace bindings of its root, and writes each child back where it was. A child a caller adds is still written after the last paragraph. It reads the part without trimming, as CT_Document does, and still skips the text between the children of the root. Replacement parses a raw table or block control with the typed parsers under those bindings, as the text box walker does, and writes back only one it changed, so the others keep their bytes. render_template reads the tags of those blocks too. The model stays a list of paragraphs rather than a full typed body, so the published type only gains two fields that are not public. GitHub issue tensorbee#160.
The replacement changes, the text box fix this branch is stacked on and their tests grow the rdocx-oxml, rdocx and rdocx-cli packages, so the crates.io archive rows in README.md, crates/rdocx-oxml/README.md and crates/rdocx-cli/README.md and their ARCHIVE_MEASUREMENTS entries are re-measured on top of the stacked tree. GitHub issue tensorbee#160.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
match across runs removes the runs it empties. It used a plain
retainover the direct runs and then clamped the hyperlink spans, so every
bookmark, comment range, inline content control, raw child, revision,
equation and ruby span after a removed run slid one run later. The removal
now goes through the remapping that comment removal already used, moved
into a shared
CT_P::remove_runs(rdocx-oxmltext.rs). That remappingleft the ruby spans out, so it now moves them too, which also fixes comment
removal, and drops an annotation left without base runs. Only a run the
replacement emptied is removed. Before, a match removed every run of the
paragraph without typed content, including a run whose only child is a
drawing in
mc:AlternateContent(the adjacent defect noted in Keep every child of a text box that replacement rewrites #180).of
try_replace_text,replace_all,replace_regexandreplace_all_regexgo through the visitor that already reaches tables,cells and controls.
replace_in_paragraphandreplace_regex_in_paragraphread the runs of inline controls, nested ones included, in the order
CT_P::runsreads them. A match that straddles a content-control boundaryis not replaced, and the search goes on after it. The text box walker edits
its tables and block controls through the typed parsers and keeps the
namespace declarations of the elements it rewrites.
render_templatereadsits tags the same way.
CT_HdrFtrrecords where each raw child (table, content control) satbetween the paragraphs, and the namespace bindings of its root. A rewrite
puts each child back in place instead of after the last paragraph, and
replacement parses a raw table or block control with the typed parsers and
writes back only one it changed.
CT_HdrFtr::from_xmlno longer trims thetext events a raw child is captured with, so
Pagebefore a page numberkeeps its space.
rdocx,rdocx-oxmlandrdocx-clion top of the stacked tree.Part of #160. This closes the replace side of section 1:
try_replace_text,rdocx replaceand the regex replacement now reach content controls in thebody, inline, in table cells, nested, in text boxes, and in headers and
footers. The read side of section 1 (
rdocx text, images, word count,headings, links) is #177. The Markdown, HTML and EPUB exporters and the other
sections of #160 are outside this PR.
Why
Google Docs exports wrap runs, and whole paragraphs, in
goog_rdk_Ncontentcontrols, and Word keeps them. Word writes its page numbers in a footer as a
content control too. On main:
BodyContent::ContentControl(_)to nothing, so aparagraph wrapped at body level was never read.
replace_in_paragraphbuilt its text from the direct runs only. A run inan inline control was missed at every level, and since Python:
story_itemsandhyperlinkstake quadratic time;Paragraph.textandParagraph.runsskip tracked insertions and inline content controls #118 the read view(
Paragraph.text,Paragraph.runs) shows that run, so the two viewsdisagreed on the same run.
replace_in_sdtreturned 0 forSdtContent::Run.so
al+ control +phamatchedalpha, a word the reader does not see.position. Replacement never read them, and when it rewrote the part for a
paragraph hit, it wrote them all after the last paragraph, so a footer's
page-number control moved below its text. The raw copy of each of them had
also lost the edge spaces of its text, since the part was read with trimmed
text events.
them (and deleted them before Keep every child of a text box that replacement rewrites #180).
Without
--expect, the CLI and the Python call succeeded with nothingreplaced. With it,
rdocx replacefailed withfound 0.Notes
Decisions taken, as agreed for this issue:
counted as no match and left as it is, never half replaced. A match must
lie within one stretch of runs: the direct runs between two inline
controls, or the runs of one control between its nested controls. The
search runs over the whole paragraph text, so
^,$and\bkeep theirparagraph meaning, and matches are found left to right without overlap, as
str::replaceandRegex::replace_allfind them. A straddling match isskipped whole and the search goes on after it, so no later match starts
inside it. Without that,
\d+over12and a control holding34wouldreplace the
34alone. For a literal, the only difference from searchingeach stretch on its own is a literal that overlaps itself, such as
aaover
aand a control holdingaa, which is left as it is.full typed body model.
CT_HdrFtrkeepsparagraphsandextra_xmlasthey are. A raw block is parsed only when replacement visits it, and
re-serialised only when it holds a match, so every other block keeps its
bytes.
writers leave out namespace declarations. For a raw table or block control
in a header, footer or text box, the declarations of its start tag are
written again on the rewritten element. A block that declares a namespace
deeper down, on a cell for example, with a namespace the part does not bind
that prefix to, keeps its bytes and counts 0, since its rewrite would leave
the prefix unbound or bound to another namespace. A redundant declaration,
same namespace as the part root, does not block the rewrite.
Behaviour changes:
the Python binding (same native counts), the CLI and the WASM binding.
Before,
--expectfailed on such files and the unguarded calls silentlyreplaced nothing.
body-level control (
docPartGallery"Table of Contents"), and each entryholds the text of its heading. A heading word therefore counts once more
for its entry:
try_replace_text("Introduction", "Overview")returns 2 on adocument with that heading and its entry, where it returned 1, so
rdocx replace --expect 1on such a file now fails withfound 2, andrender_templaterenders a tag in a cached entry. The same holds for otherdocPart controls such as a cover page or a page-number control. The
regression test
a_table_of_contents_entry_is_replaced_with_its_headingpins it.
it did not empty is kept, an empty run or a run holding only raw XML
included. A run that keeps only its start-tag attributes (
w:rsidR) countsas emptied. A hyperlink left without runs is dropped, as comment removal
already does, and so is a ruby annotation left without base runs, which the
writer skipped anyway.
keep the source bytes they were read with (
CT_Sdt::remove_contentkeepsthe content-index bookkeeping aligned).
render_template: a tag inside an inline control, or in a table or controlof a text box or a header, is rendered. Before it was left in the output.
A tag that a control boundary splits is reported as an invalid template.
Before, it was an error too when one half sat inside the control, and it
was rendered across the control when both halves sat in direct runs around
it. A
{% %}marker that fills an inline control alone in its paragraph isnow recognised.
between the paragraphs where they were, with the edge spaces of their text.
This applies to every typed rewrite of a loaded header, but replacement is
the only one on main. Reading a header without trimming changes nothing
else: the typed paragraphs already kept their text, and the text between
the children of the root is still skipped. The hash harness is unchanged.
API changes, none breaking:
placeholder: the public replacement functions keep theirsignatures and change behaviour as above.
replace_in_header_footer,replace_regex_in_header_footerand the three*_xml_partfunctions alsoreach raw tables and block controls. Three
#[doc(hidden)]functions givethe template what a replacement reads:
replaceable_texts,header_footer_replaceable_textsandxml_part_replaceable_texts.CT_HdrFtrgains two fields that are not public. It already had a privatefield (
watermarks), so no caller can build it with a struct literal, andthis does not break the published type.
control recursion taking a paragraph callback, instead of two copies.
Replacement addresses a run with the existing
AcceptedRunPath.Decisions left to the maintainer:
cached entries keeps them consistent with their headings until the next
TOC update, it follows the acceptance of Producer traits: a matrix over every operation, and what still fails in it and around it #160 section 1 (every level), and
skipping docPart controls would bring back a mismatch between the read and
replace views, since the read side reaches those controls too (Make the body read walkers reach content controls #177).
bytes, where the body path fails the whole call (
cannot identify retained ... nested namespace owner). Recommendation: keep it, since the header andtext box walkers return counts, the saved part stays well formed, and
--expectreports the missing replacement.w:insorw:moveTowrapper, as on main, and such a wrapper does not split a stretch.
CT_P::textdoes not read them either, whileParagraphRef::textdoes.Recommendation: keep it out of replacement, since an edit there changes a
revision, and decide it with the revision work.
is neither read nor replaced, and they do not split a stretch.
Recommendation: leave it until the parser admits them.
template uses them. Recommendation: keep them hidden.
Interactions with other PRs:
root_attributestoCT_HdrFtr, next to the two fields addedhere, in the struct,
new()and thefrom_xmlconstructor. The textualconflict is resolved by keeping both. This PR also turns off text trimming
in that constructor.
wprefix ofCT_P::from_xml. Until it lands, atext-box paragraph whose runs carry
w:rsidRstill fails the text boxwalker for its whole part (as Keep every child of a text box that replacement rewrites #180 notes), and a text-box control with such
runs is skipped and kept as it is. Headers are not affected, because their
raw blocks are parsed with the bindings of the part root. After Fix the TOC rebuild and text boxes on runs that carry identity attributes #183, both
are reached with no change here.
document.rs, in other hunks.With both PRs, every row of the issue's section 1 script should print
True,1andReplaced 1 occurrence(s). I did not run the combination.CT_P::remove_comment_anchorsnow delegates its run removal toCT_P::remove_runs, with the same code plus the ruby spans. A PR thatedits that function would conflict textually.
Other notes:
header table or a cross-run placeholder that empties a run.
rdocxrow isre-measured by other PRs of this batch too, so it needs one fresh
measurement at integration.
Tests
Added:
placeholder.rsunit tests:removing_emptied_runs_keeps_later_anchors_in_place:{{na+me}},then a bookmark start, a comment range start, an inline control, a
w:proofErrand a ruby overBASEbeforex. After the literal and theregex replacement, every anchor still precedes
xand the ruby stillwraps
BASE.replace_keeps_the_runs_it_did_not_empty: an empty run and a run holdingonly
mc:AlternateContentsurvive a match elsewhere in the paragraph.a_control_run_after_emptied_ones_keeps_its_source_bytes: a match acrosstwo runs of an inline control removes the emptied one, and the third run
is written with the exact bytes it was read with.
a_match_stays_within_one_stretch_of_runs:replaceable_textssplits atevery control boundary, nested ones included, five straddling literals
replace nothing, and a regex with
^and$anchors at the paragraphedges.
no_match_starts_inside_a_straddling_one:\d+and\d{2,}over12,a control holding
34 endand56replace only56.replace_in_textbox_reaches_its_tables_and_controls: the text boxfixture of Keep every child of a text box that replacement rewrites #180 with placeholders in its table and control. Four hits, in
order, and the untouched table of the second text box keeps its bytes.
replace_in_textbox_keeps_the_namespaces_its_tables_declareandreplace_in_header_footer_keeps_the_namespaces_its_tables_declare: atable declaring
w14on its start tag is replaced and every prefix of thepart resolves. One declaring it on a cell keeps its bytes, unless the
header root binds
w14the same way, where it is replaced too.replace_in_header_footer_reaches_its_tables_and_controls: aq:prefixed header with a table, a paragraph, a control and an untouched
control. Three hits in order, and the untouched control keeps its bytes.
header_footer.rs:raw_children_keep_their_place_between_paragraphsanda_raw_child_keeps_the_edge_spaces_of_its_text(a pretty-printed footerwhose page-number control holds
Page).regression_test.rs:mod replacement_reaches_content_controls,placed next to the text box replacement module of Keep every child of a text box that replacement rewrites #180:
every_walker_replaces_the_tag_of_every_location_once: the matrix. Tenlocations (body block control, inline control, cell control, four nested
controls, a block control and an inline control in a text box, a header
table, a header control, the footer page-number control, an inline control
in a footer paragraph) against
try_replace_text,replace_regexandreplace_all. Each cell checks the count of 1, the saved part, everyother tag, every
w:tag, the order and bytes of the header and footerblocks, and the trailing space of the run before each tag in those
blocks.
a_template_renders_the_tag_of_every_location: onerender_templateover the ten locations.
a_match_that_straddles_a_control_boundary_is_not_replaced:al+control
pha one,al+ controlX+pha two, andal+ controlalpha three, with both walkers, checked throughParagraph::text.a_quantified_match_that_straddles_a_control_boundary_is_not_replaced:\d+and\d{2,}overOrder 12, a control holding34 endand56give
Order 1234 end N.a_table_of_contents_entry_is_replaced_with_its_heading: a Word-shapedTOC control and its heading give a count of 2.
a_rewritten_table_keeps_the_namespaces_it_declares: the namespace casethrough
Document::try_replace_textand a save, in a header and in a VMLtext box. Every prefix of the saved part resolves.
a_template_tag_that_straddles_a_control_boundary_is_an_error.the_reported_google_docs_controls_are_replaced: the issue's three shapes(no control, run wrapped, paragraph wrapped), with the read view checked
before and after.
integration.rs:replace_with_expect_counts_the_text_of_content_controls_everywhere, nextto the other
replacetests.rdocx replace --expect 1over eightlocations.
On the stacked base, the matrix, template, straddle and reproduction
regression tests and the CLI test fail. The straddle test shows the old false
match (
ALPHAX two), and the CLI test stops atexpected 1 replacement(s) of "{{inline}}", found 0. The straddling templatetest passes there too, for another reason (half of the tag was invisible), and
pins the error. The unit tests use new functions, so I checked the key ones by
mutation: restoring the plain
retainfails the anchor test, leaving the rubyspans out of
remove_runsfails it too, dropping the content-index shift failsthe source-bytes test, resuming one character after a straddling match fails
both quantified tests (count 2,
12N end N), skipping the namespace step failsthe three namespace tests (count 2), and trimming the header
reader again fails the edge-space tests.
The issue's section 1 script with this branch (debug build):
The
Falseof the block row is the read side, fixed by #177.matrix_producer_traits.pywith this branch: thereplacecolumn isokonall 11 rows. The
inline content control on first runsrow,n=0in theissue, now reads
replace=okandself=2. The onlyFAILcells left arecmpfldandselfof theempty comments partrow, which #185 addresses.matrix_identity_attributes.pyhasreplace=okon all 18 rows.Run on macOS arm64, debug build, at the head of the branch:
cargo fmt --all --check: clean.cargo clippy -p rdocx-oxml -p rdocx -p rdocx-cli -p rdocx-layout -p rdocx-html -p rdocx-wasm -p rdocx-py --all-targets --all-features -- -D warnings: clean.cargo test -p rdocx-oxml: 558 passed, plus 1 doctest.cargo test -p rdocx --no-fail-fast: regression 577 passed (7 ignored),doctests passed. Lib and integration have only environment failures
(below).
cargo test -p rdocx-cli: 2 and 18 passed.rdocx-layout: 292 passed.rdocx-html: 20 passed.rdocx-wasm: passed.cargo check --target wasm32-unknown-unknown -p rdocx-wasm: clean.python3 scripts/hash_harness.py --check: 49 entries match.python3 scripts/prose_check.py: 0 violations.python3 scripts/readme_doctests.py: passed. After the lastre-measurement,
readme_doctests.validate_inventory()validated 27 READMEsand 22 package inventories.
maturin developbuild: 51 passed (withouttest_python_docx_parity.pyand
test_rendering_threads.py, which fail here for environment reasons).mypy and stubtest were not run.
Environment failures seen here, all known on this machine:
large_word_and_presentation_pdfs_preserve_logical_reading_orderand
word_and_powerpoint_chart_pixels_are_identical.odt_reader_matches_pinned_libreoffice_structure,public_authored_theme_and_fonts_match_pinned_word_resolution,sanitized_public_authoring_fixture_passes_every_conformance_stage,section_page_semantics_match_pinned_libreoffice_renderandevery_conditional_table_region_matches_word.