From bbb97175d98e7beaac40abd595f668a576e90ee7 Mon Sep 17 00:00:00 2001 From: Hadrien Mary Date: Sun, 27 Sep 2026 17:09:15 +0200 Subject: [PATCH 1/4] Make split_run take the direct body index of find_content_index Document::split_run resolved its first argument with Document::paragraph, which counts paragraphs, skips tables and enters block content controls. RunPosition, find_content_index, add_comment and insert_paragraph use the direct body child index instead, which is also what HLD 10 and F-X109 state for split_run. With a table or a block content control before the target, splitting a run then anchoring a comment at the same index split one paragraph and commented another, or failed on a run length. split_run now reads the paragraph at that direct slot and writes it back to the same slot, so one resolution serves both steps. An index that names a table, a block content control or preserved XML returns an error naming that kind instead of splitting another paragraph. The Python method also accepts a Paragraph handle, which reaches paragraphs inside block content controls, and it measures the revision bump on the paragraph it split. A handle to a direct body paragraph takes the same native path as its index. A table cell paragraph handle is refused, because its index counts the paragraphs inside cell content controls and Cell::paragraph_mut does not. The rustdoc of RunPosition and Document::paragraph and the HLD 03 and 10 statements now name the index each one takes. GitHub issue #163. --- crates/rdocx-py/python/rdocx/_rdocx.pyi | 2 +- crates/rdocx-py/src/document.rs | 87 +++++++++++++--- crates/rdocx-py/tests/test_core.py | 92 +++++++++++++++++ crates/rdocx-py/tests/typing_smoke.py | 1 + crates/rdocx/src/comments.rs | 3 +- crates/rdocx/src/document.rs | 50 +++++++-- crates/rdocx/tests/regression_test.rs | 131 ++++++++++++++++++++++++ docs/hld/03-architecture.md | 11 +- docs/hld/10-bindings-spec.md | 11 +- 9 files changed, 354 insertions(+), 34 deletions(-) diff --git a/crates/rdocx-py/python/rdocx/_rdocx.pyi b/crates/rdocx-py/python/rdocx/_rdocx.pyi index f5e4ef18..4b3cfcfb 100644 --- a/crates/rdocx-py/python/rdocx/_rdocx.pyi +++ b/crates/rdocx-py/python/rdocx/_rdocx.pyi @@ -428,7 +428,7 @@ class Document: self, story: Story, relationship_id: str, data: bytes ) -> None: ... def split_run( - self, body_index: int, run_index: int, character_offset: int + self, body_index: int | Paragraph, run_index: int, character_offset: int ) -> int: ... def to_pdf(self) -> bytes: ... def render_page_to_png(self, page_index: int, dpi: float = 150.0) -> bytes | None: ... diff --git a/crates/rdocx-py/src/document.rs b/crates/rdocx-py/src/document.rs index b81167c6..5d343dca 100644 --- a/crates/rdocx-py/src/document.rs +++ b/crates/rdocx-py/src/document.rs @@ -7,7 +7,7 @@ use pyo3::prelude::*; use pyo3::types::{PyAny, PyBytes, PyList, PyTuple}; use smallvec::smallvec; -use crate::paragraph::{PyParagraph, PyParagraphCollection}; +use crate::paragraph::{ParagraphLocation, PyParagraph, PyParagraphCollection}; use crate::rdocx_to_pyerr; use crate::table::{PyTable, PyTableCollection}; @@ -979,6 +979,32 @@ impl PyDocument { ))) } + /// Split a run of the direct body paragraph at `body_index`. The revision + /// advances only when a continuation run is created. + fn split_body_run( + &mut self, + py: Python<'_>, + body_index: usize, + run_index: usize, + character_offset: usize, + ) -> PyResult { + let paragraph_index = self.inner.paragraph_index_of_content(body_index); + let run_count = |document: &rdocx::Document| { + paragraph_index + .and_then(|index| document.paragraph(index)) + .map(|paragraph| paragraph.run_count()) + }; + let before = run_count(&self.inner); + let boundary = self + .inner + .split_run(body_index, run_index, character_offset) + .map_err(|error| rdocx_to_pyerr(py, error))?; + if run_count(&self.inner) != before { + self.revisions.bump(); + } + Ok(boundary) + } + /// Run a native mutation that reports how many things it changed. /// /// The GIL is released while it runs, and live handles are staled only @@ -1193,26 +1219,57 @@ impl PyDocument { } fn split_run( - &mut self, + slf: Py, py: Python<'_>, - body_index: usize, + body_index: &Bound<'_, PyAny>, run_index: usize, character_offset: usize, ) -> PyResult { - let before = self - .inner - .paragraph(body_index) - .map(|paragraph| paragraph.run_count()); - let boundary = self + let Ok(paragraph) = body_index.cast::() else { + // An integer is the direct body index, as in `RunPosition`. + let body_index = body_index.extract::().map_err(|error| { + if error.is_instance_of::(py) { + PyTypeError::new_err("body_index must be an int or a Paragraph handle") + } else { + error + } + })?; + return slf + .borrow_mut(py) + .split_body_run(py, body_index, run_index, character_offset); + }; + let paragraph = paragraph.borrow(); + if !paragraph.belongs_to(py, &slf) { + return Err(PyValueError::new_err( + "paragraph handle belongs to a different document", + )); + } + // A cell handle counts paragraphs inside cell content controls, which + // `Cell::paragraph_mut` does not, so it could name another paragraph. + let ParagraphLocation::Body(paragraph_index) = paragraph.validate(py)? else { + return Err(PyValueError::new_err( + "split_run does not accept a table cell paragraph handle", + )); + }; + let mut document = slf.borrow_mut(py); + if let Some(body_index) = document.inner.content_index_of_paragraph(paragraph_index) { + return document.split_body_run(py, body_index, run_index, character_offset); + } + // A paragraph inside a block content control has no direct body index. + let run_count = |document: &rdocx::Document| { + document + .paragraph(paragraph_index) + .map(|paragraph| paragraph.run_count()) + }; + let before = run_count(&document.inner); + let boundary = document .inner - .split_run(body_index, run_index, character_offset) + .paragraph_mut(paragraph_index) + .ok_or_else(|| PyIndexError::new_err("paragraph index out of range"))? + .split_run(run_index, character_offset) .map_err(|error| rdocx_to_pyerr(py, error))?; - let after = self - .inner - .paragraph(body_index) - .map(|paragraph| paragraph.run_count()); - if before != after { - self.revisions.bump(); + if run_count(&document.inner) != before { + document.revisions.bump(); } Ok(boundary) } diff --git a/crates/rdocx-py/tests/test_core.py b/crates/rdocx-py/tests/test_core.py index 0f8ce927..19594290 100644 --- a/crates/rdocx-py/tests/test_core.py +++ b/crates/rdocx-py/tests/test_core.py @@ -1291,6 +1291,98 @@ def test_split_run_rejects_bad_coordinates_without_changing_the_document(): assert document.to_bytes() == before +def test_split_run_takes_the_direct_body_index_after_a_table(): + import rdocx + + # GitHub issue #163: the index find_content_index returns must address + # the same paragraph in split_run. + document = rdocx.Document() + document.add_paragraph("Alpha paragraph before the table.") + document.add_table(1, 1).cell(0, 0).text = "cell" + document.add_paragraph("Beta paragraph after the table.") + document.add_paragraph("Gamma paragraph at the end.") + target = next(p for p in document.paragraphs if p.text.startswith("Beta")) + body_index = document.find_content_index(target) + assert body_index == 2 + + assert document.split_run(body_index, 0, 4) == 1 + assert [[run.text for run in p.runs] for p in document.paragraphs] == [ + ["Alpha paragraph before the table."], + ["Beta", " paragraph after the table."], + ["Gamma paragraph at the end."], + ] + + before = document.to_bytes() + with pytest.raises(rdocx.RdocxError, match="is a table, not a paragraph"): + document.split_run(1, 0, 1) + with pytest.raises(TypeError, match="must be an int or a Paragraph handle"): + document.split_run("Beta", 0, 1) + with pytest.raises(OverflowError): + document.split_run(-1, 0, 1) + assert document.to_bytes() == before + + +def test_split_run_accepts_a_paragraph_handle_in_a_control_or_the_body(): + import rdocx + + document = _replace_document_body( + rdocx.Document(), + "Alpha." + "" + "Control one." + "Control two." + "" + '' + "" + "" + "In control." + "" + "Cell text." + "" + "" + "Beta.", + ) + control_two = document.paragraphs[2] + assert control_two.text == "Control two." + run = control_two.runs[0] + before_no_op = document.to_bytes() + assert document.split_run(control_two, 0, 0) == 0 + assert document.to_bytes() == before_no_op + assert run.text == "Control two." + + assert document.split_run(control_two, 0, 7) == 1 + with pytest.raises(rdocx.StaleElementError): + run.text + with pytest.raises(rdocx.StaleElementError): + document.split_run(control_two, 0, 1) + assert [r.text for r in document.paragraphs[2].runs] == ["Control", " two."] + + # A cell handle is refused, because its index counts the paragraph + # inside the cell's content control and the cell writer does not. + cell = document.tables[0].rows[0].cells[0] + assert [p.text for p in cell.paragraphs] == ["In control.", "Cell text."] + before_cell = document.to_bytes() + for cell_paragraph in cell.paragraphs: + with pytest.raises(ValueError, match="table cell paragraph handle"): + document.split_run(cell_paragraph, 0, 2) + assert document.to_bytes() == before_cell + + beta = document.paragraphs[3] + assert document.find_content_index(beta) == 3 + beta_run = beta.runs[0] + assert document.split_run(beta, 0, 5) == 1 + assert beta_run.text == "Beta." + assert document.split_run(beta, 0, 4) == 1 + assert [r.text for r in document.paragraphs[3].runs] == ["Beta", "."] + + other = rdocx.Document() + other.add_paragraph("elsewhere") + before = document.to_bytes() + with pytest.raises(ValueError, match="different document"): + document.split_run(other.paragraphs[0], 0, 1) + assert document.to_bytes() == before + + def test_python_round_three_authoring_and_inspection_is_typed_and_lossless(): import rdocx diff --git a/crates/rdocx-py/tests/typing_smoke.py b/crates/rdocx-py/tests/typing_smoke.py index 51ebfa05..ae8f1821 100644 --- a/crates/rdocx-py/tests/typing_smoke.py +++ b/crates/rdocx-py/tests/typing_smoke.py @@ -63,6 +63,7 @@ def exercise_rdocx_types(path: Path) -> None: paragraph_format: ParagraphFormat = paragraph.paragraph_format paragraph_format.keep_together = None split_boundary: int = document.split_run(0, 0, 1) + handle_boundary: int = document.split_run(paragraph, 0, 1) paragraphs: ParagraphCollection = document.paragraphs first: Paragraph = paragraphs[0] sliced: list[Paragraph] = paragraphs[:] diff --git a/crates/rdocx/src/comments.rs b/crates/rdocx/src/comments.rs index 1e85cd82..609cb492 100644 --- a/crates/rdocx/src/comments.rs +++ b/crates/rdocx/src/comments.rs @@ -27,7 +27,8 @@ const DEFAULT_COMMENTS_EXTENDED_PART: &str = "/word/commentsExtended.xml"; /// A stable insertion point between runs in a body paragraph. #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct RunPosition { - /// Index in the document body's paragraph and table sequence. + /// Direct body child index, where a table or a block content control + /// counts as one child. `Document::find_content_index` returns it. pub body_index: usize, /// Run insertion index in the selected paragraph. pub run_index: usize, diff --git a/crates/rdocx/src/document.rs b/crates/rdocx/src/document.rs index 5c4b2b46..a09eacc7 100644 --- a/crates/rdocx/src/document.rs +++ b/crates/rdocx/src/document.rs @@ -14529,7 +14529,9 @@ impl Document { .collect() } - /// Get an immutable reference to a paragraph by index (among paragraphs only). + /// Get an immutable reference to a paragraph by its index in + /// [`Self::paragraphs`]. That index skips body tables and enters block + /// content controls, so it is not the direct body index. pub fn paragraph(&self, index: usize) -> Option> { self.document .body @@ -14661,7 +14663,8 @@ impl Document { result } - /// Get a mutable reference to a paragraph by index (among paragraphs only). + /// Get a mutable reference to a paragraph by the same index as + /// [`Self::paragraph`]. pub fn paragraph_mut(&mut self, index: usize) -> Option> { self.invalidate_layout(); let mut remaining = index; @@ -14672,6 +14675,14 @@ impl Document { /// Split a direct body paragraph run at a Unicode scalar offset of its /// literal text and return the resulting run boundary. /// + /// `body_index` is the direct body child index that `RunPosition`, + /// [`Self::find_content_index`], [`Self::add_comment`] and + /// [`Self::insert_paragraph`] use, where a table or a block content + /// control counts as one child. An index that names a table, a block + /// content control or preserved XML is an error that names its kind. A + /// paragraph inside a block content control is split through + /// `paragraph_mut(i).split_run`. + /// /// Zero returns the boundary before the selected run and the literal text /// length returns the boundary after it without changing the document. /// Interior offsets create a continuation with cloned run properties while @@ -14683,23 +14694,40 @@ impl Document { run_index: usize, character_offset: usize, ) -> Result { - let original = self.paragraph(body_index).ok_or_else(|| { - Error::Other(format!("body paragraph index {body_index} is out of range")) - })?; - let mut candidate = original.inner.clone(); + let not_a_paragraph = |kind: &str| { + Error::Other(format!( + "body index {body_index} is {kind}, not a paragraph" + )) + }; + let original = match self.document.body.content.get(body_index) { + Some(BodyContent::Paragraph(paragraph)) => paragraph, + Some(BodyContent::Table(_)) => return Err(not_a_paragraph("a table")), + Some(BodyContent::ContentControl(_)) => { + return Err(not_a_paragraph("a block content control")); + } + Some(BodyContent::RawXml(_)) => return Err(not_a_paragraph("preserved XML")), + None => { + return Err(Error::Other(format!( + "body index {body_index} is out of range" + ))); + } + }; + let mut candidate = original.clone(); let boundary = Paragraph { inner: &mut candidate, } .split_run(run_index, character_offset)?; - if candidate == *original.inner { + if candidate == *original { return Ok(boundary); } - self.invalidate_layout(); - let mut remaining = body_index; - let paragraph = nth_paragraph_in_body(&mut self.document.body.content, &mut remaining) - .ok_or_else(|| Error::Other("body paragraph disappeared during split".to_owned()))?; + let Some(BodyContent::Paragraph(paragraph)) = + self.document.body.content.get_mut(body_index) + else { + unreachable!("body index {body_index} was checked to be a paragraph"); + }; *paragraph = candidate; + self.invalidate_layout(); Ok(boundary) } diff --git a/crates/rdocx/tests/regression_test.rs b/crates/rdocx/tests/regression_test.rs index d143b3c7..cc1cd2e1 100644 --- a/crates/rdocx/tests/regression_test.rs +++ b/crates/rdocx/tests/regression_test.rs @@ -1634,6 +1634,137 @@ fn checked_table_cell_comment_range_is_atomic_and_reopens() { assert_eq!(reopened.comments()[0].text(), "review"); } +/// GitHub issue #163: the index `find_content_index` returns is a direct body +/// child index, and every body API that takes an index must address the same +/// paragraph with it, whatever tables or block content controls precede it. +mod direct_body_index_coordinates { + use rdocx::{Document, RunPosition, RunRange}; + + const TABLE: &str = r#"c00c01c10c11"#; + const CONTROL: &str = r#"Control one.Control two."#; + + fn paragraph(text: &str) -> String { + format!("{text}") + } + + fn document_from_body(body: &str) -> Document { + super::document_with_content_controls(&super::wrap_word_body(body)) + } + + fn run_texts(document: &Document) -> Vec> { + document + .paragraphs() + .iter() + .map(|paragraph| paragraph.runs().map(|run| run.text()).collect()) + .collect() + } + + fn one_run_range(body_index: usize) -> RunRange { + RunRange { + start: RunPosition { + body_index, + run_index: 0, + }, + end: RunPosition { + body_index, + run_index: 1, + }, + } + } + + /// Visible text between the range markers of comment `id` in the saved + /// main part. The tests keep each range inside one paragraph. + fn commented_text(document: &mut Document, id: i32) -> String { + let xml = super::document_xml(document); + let start_marker = format!(r#""#); + let end_marker = format!(r#""#); + let start = xml.find(&start_marker).expect("comment start marker") + start_marker.len(); + let end = start + xml[start..].find(&end_marker).expect("comment end marker"); + super::f_x093_visible_text(&xml[start..end]) + } + + #[test] + fn split_run_takes_the_direct_body_index_after_a_table() { + let mut document = Document::new(); + document.add_paragraph("Alpha paragraph before the table."); + document + .add_table(1, 1) + .row(0) + .unwrap() + .cell(0) + .unwrap() + .set_text("cell"); + document.add_paragraph("Beta paragraph after the table."); + document.add_paragraph("Gamma paragraph at the end."); + let beta = document.find_content_index("Beta").unwrap(); + assert_eq!(beta, 2); + + assert_eq!(document.split_run(beta, 0, 4).unwrap(), 1); + assert_eq!( + run_texts(&document), + [ + vec!["Alpha paragraph before the table."], + vec!["Beta", " paragraph after the table."], + vec!["Gamma paragraph at the end."], + ] + ); + + let id = document + .add_comment(one_run_range(beta), "Ada", None, "Which word?") + .unwrap(); + assert_eq!(commented_text(&mut document, id), "Beta"); + + document.insert_paragraph(beta, "Inserted."); + let texts: Vec = document.paragraphs().iter().map(|p| p.text()).collect(); + assert_eq!( + texts[1..3], + ["Inserted.", "Beta paragraph after the table."] + ); + } + + #[test] + fn split_run_takes_the_direct_body_index_after_a_block_content_control() { + let mut document = document_from_body(&format!( + "{}{CONTROL}{}{}", + paragraph("Alpha."), + paragraph("Beta paragraph."), + paragraph("Gamma.") + )); + let beta = document.find_content_index("Beta").unwrap(); + assert_eq!(beta, 2); + let mut expected = run_texts(&document); + expected[3] = vec!["Beta".to_owned(), " paragraph.".to_owned()]; + + assert_eq!(document.split_run(beta, 0, 4).unwrap(), 1); + assert_eq!(run_texts(&document), expected); + let id = document + .add_comment(one_run_range(beta), "Ada", None, "Which word?") + .unwrap(); + assert_eq!(commented_text(&mut document, id), "Beta"); + } + + #[test] + fn split_run_names_the_body_child_that_is_not_a_paragraph() { + let mut document = document_from_body(&format!( + r#"{}{TABLE}{CONTROL}{}{}"#, + paragraph("Alpha."), + paragraph("Custom."), + paragraph("Beta.") + )); + let before = document.to_bytes().unwrap(); + for (body_index, kind) in [(1, "table"), (2, "content control"), (3, "preserved XML")] { + let error = document.split_run(body_index, 0, 1).unwrap_err(); + assert!( + error.to_string().contains(kind), + "body index {body_index}: {error}" + ); + } + let error = document.split_run(5, 0, 1).unwrap_err(); + assert!(error.to_string().contains("out of range"), "{error}"); + assert_eq!(document.to_bytes().unwrap(), before); + } +} + fn f_x090_cross_part_drawing_package() -> Vec { let mut document = f255_story_document(); document.add_picture(b"body image", "body.png", Length::pt(1.0), Length::pt(1.0)); diff --git a/docs/hld/03-architecture.md b/docs/hld/03-architecture.md index 97ce19c6..341f23cb 100644 --- a/docs/hld/03-architecture.md +++ b/docs/hld/03-architecture.md @@ -1484,13 +1484,18 @@ location only after the image part, relationship, drawing identity, and story content all validate together. `Document::split_run` creates an exact accepted-view run boundary without -changing `RunPosition`. It clones the selected paragraph, resolves the selected +changing `RunPosition`. Its paragraph argument is the same direct body child +index as `RunPosition` and `find_content_index`, and an index that names a +table, a block content control, or preserved XML fails with an error naming +that kind. It clones the selected paragraph, resolves the selected recursive source path, counts Unicode scalar values only in literal text, partitions ordered zero-width children at their source boundary, repairs hyperlink and marker coordinates, and publishes the clone only on success. Zero and end offsets select existing boundaries and leave typed state, layout, -and binding revisions unchanged. A structural edit makes an earlier path-backed -Python run handle stale. +and binding revisions unchanged. A Python `Paragraph` handle to a paragraph +inside a block content control splits through `Document::paragraph_mut`, which +clears the cached layout even for those offsets. A structural edit makes an +earlier path-backed Python run handle stale. Word bookmark mutation input reuses the same top-level `RunPosition` and half-open `RunRange` boundary as comments. `Document::bookmarks` returns diff --git a/docs/hld/10-bindings-spec.md b/docs/hld/10-bindings-spec.md index a6a75eb7..68022d07 100644 --- a/docs/hld/10-bindings-spec.md +++ b/docs/hld/10-bindings-spec.md @@ -1006,12 +1006,17 @@ although third-party editors may renumber them. `RunPosition` and `RunRange` define top-level paragraph run boundaries with an inclusive start and exclusive end. `Document::split_run` splits one direct-body run at a Unicode scalar offset of its literal text, so -such a boundary can fall inside what was one run. The second part keeps the run +such a boundary can fall inside what was one run. Its first argument is the +same direct body child index as `RunPosition` and `find_content_index`. An +index that names a table, a block content control, or preserved XML is an +error naming that kind. The second part keeps the run properties and the enclosing hyperlink. Tabs, breaks, fields, drawings, references, and preserved raw children have zero width. Zero and the literal text length return the existing boundary without mutation. Interior success -returns the new continuation index. Python exposes the same method and advances -the binding revision only when a continuation is created. `CommentRef` exposes +returns the new continuation index. Python exposes the same method and also +accepts a `Paragraph` handle in place of the index, which reaches paragraphs +inside block content controls. A table cell paragraph handle is refused. The +binding revision advances only when a continuation is created. `CommentRef` exposes comment metadata, text, parent identity, and resolved state without permitting part-local mutation. `rdocx-cli comment` lists, adds, replies to, resolves, and removes comments. Add ranges use explicit zero-based, half-open body paragraph From 0547a314f9393e6295bb9f1a8fd77ad58051e74e Mon Sep 17 00:00:00 2001 From: Hadrien Mary Date: Sun, 27 Sep 2026 17:10:49 +0200 Subject: [PATCH 2/4] Report bookmark ranges in the direct body index as well Document::add_bookmark takes a RunRange whose body index is the direct body child index, but Document::bookmarks reports the paragraph ordinal counted recursively through table cells and block content controls. A bookmark added after a table with more than one cell, or after a block content control, read back with a different body index than the one it was added with, so that index could not be passed back to add_comment, split_run or add_bookmark. The recursive ordinal stays in range() because REF numbering in field.rs reads it as its paragraph key. BookmarkRef gains an additive direct_range() that carries the direct body child index, recorded while the paragraphs are collected, and is None when either marker sits in a table cell or a block content control. Run indexes are unchanged and stay the accepted-view boundaries of range(). They equal the direct run indexes add_bookmark takes only in a paragraph without inline content controls or tracked insertions, which GitHub issue #172 covers. GitHub issue #163. --- crates/rdocx/src/comments.rs | 51 +++++++++++++++++++++++++-- crates/rdocx/tests/regression_test.rs | 46 ++++++++++++++++++++++++ docs/hld/03-architecture.md | 4 ++- docs/hld/10-bindings-spec.md | 8 ++++- 4 files changed, 104 insertions(+), 5 deletions(-) diff --git a/crates/rdocx/src/comments.rs b/crates/rdocx/src/comments.rs index 609cb492..c067fb8f 100644 --- a/crates/rdocx/src/comments.rs +++ b/crates/rdocx/src/comments.rs @@ -61,6 +61,7 @@ pub struct BookmarkRef { id: Option, name: Option, range: Option, + direct_range: Option, text: String, issue: Option, } @@ -75,10 +76,28 @@ impl BookmarkRef { } /// Return the accepted-view half-open range reported by `Document::bookmarks`. + /// + /// Its body index is the paragraph ordinal counted recursively through + /// tables and block content controls, which field numbering reads. + /// [`Self::direct_range`] reports the direct body child index instead. pub fn range(&self) -> Option { self.range } + /// Return the same range with the direct body child index that + /// `RunPosition`, `Document::add_bookmark` and + /// `Document::find_content_index` use. + /// + /// It is `None` when either marker sits in a table cell or a block content + /// control, which have no direct body index. Run indexes are the same + /// accepted-view boundaries as [`Self::range`]. They equal the + /// `RunPosition` run index that `Document::add_bookmark` and + /// `Document::add_comment` take only in a paragraph without inline + /// content controls or tracked insertions. + pub fn direct_range(&self) -> Option { + self.direct_range + } + pub fn text(&self) -> &str { &self.text } @@ -257,10 +276,13 @@ impl Document { /// /// Reported body indexes count typed paragraphs recursively through tables and /// block content controls. Reported run indexes use accepted-view run boundaries. + /// `BookmarkRef::direct_range` also reports the direct body child index when + /// both markers sit in direct body paragraphs. pub fn bookmarks(&self) -> Vec { #[derive(Clone)] struct Marker { position: RunPosition, + direct_position: Option, start: bool, id: Option, name: Option, @@ -268,14 +290,28 @@ impl Document { let mut markers = Vec::new(); let mut paragraphs = Vec::new(); - collect_main_story_paragraphs(&self.document.body.content, &mut paragraphs); - for (body_index, paragraph) in paragraphs.into_iter().enumerate() { + for (direct_index, item) in self.document.body.content.iter().enumerate() { + let mut item_paragraphs = Vec::new(); + collect_main_story_paragraphs(std::slice::from_ref(item), &mut item_paragraphs); + let direct_index = matches!(item, BodyContent::Paragraph(_)).then_some(direct_index); + paragraphs.extend( + item_paragraphs + .into_iter() + .map(|paragraph| (paragraph, direct_index)), + ); + } + for (body_index, (paragraph, direct_index)) in paragraphs.into_iter().enumerate() { for marker in ¶graph.bookmark_markers { + let run_index = marker.projected_run_index(); markers.push(Marker { position: RunPosition { body_index, - run_index: marker.projected_run_index(), + run_index, }, + direct_position: direct_index.map(|body_index| RunPosition { + body_index, + run_index, + }), start: marker.is_start(), id: marker.id(), name: marker.name().map(str::to_owned), @@ -295,6 +331,7 @@ impl Document { id: None, name: marker.name.clone(), range: None, + direct_range: None, text: String::new(), issue: Some("bookmark marker has a malformed or missing id".to_owned()), }, @@ -344,6 +381,12 @@ impl Document { (Some(candidate), None) } }; + let direct_range = range.and_then(|_| { + Some(RunRange { + start: markers[starts[0]].direct_position?, + end: markers[ends[0]].direct_position?, + }) + }); let text = range .map(|_| { bookmark_range_text( @@ -361,6 +404,7 @@ impl Document { id: Some(id), name, range, + direct_range, text, issue, }, @@ -386,6 +430,7 @@ impl Document { bookmark.name.as_deref().unwrap_or("") )); bookmark.range = None; + bookmark.direct_range = None; bookmark.text.clear(); } } diff --git a/crates/rdocx/tests/regression_test.rs b/crates/rdocx/tests/regression_test.rs index cc1cd2e1..027a9724 100644 --- a/crates/rdocx/tests/regression_test.rs +++ b/crates/rdocx/tests/regression_test.rs @@ -1763,6 +1763,52 @@ mod direct_body_index_coordinates { assert!(error.to_string().contains("out of range"), "{error}"); assert_eq!(document.to_bytes().unwrap(), before); } + + #[test] + fn bookmark_direct_range_reports_the_index_add_bookmark_took() { + let mut document = document_from_body(&format!( + "{}{TABLE}{CONTROL}{}", + paragraph("Alpha."), + paragraph("Beta paragraph.") + )); + let beta = document.find_content_index("Beta").unwrap(); + assert_eq!(beta, 3); + document.add_bookmark("beta", one_run_range(beta)).unwrap(); + + let bookmark = document.bookmarks().remove(0); + assert_eq!(bookmark.text(), "Beta paragraph."); + assert_eq!(bookmark.direct_range(), Some(one_run_range(beta))); + // range() keeps the recursive paragraph ordinal that REF numbering + // reads: Alpha, the four cells and the two control paragraphs. + assert_eq!(bookmark.range().unwrap().start.body_index, 7); + } + + #[test] + fn bookmark_direct_range_is_none_when_a_marker_is_nested() { + let marked = |id: u32, name: &str, text: &str| { + format!( + r#"{text}"# + ) + }; + let document = document_from_body(&format!( + r#"Alpha.{}{}Control end.{}"#, + marked(1, "cell", "In a cell."), + marked(2, "control", "In a control."), + marked(4, "direct", "Direct.") + )); + let bookmarks = document.bookmarks(); + let named = |name: &str| { + bookmarks + .iter() + .find(|bookmark| bookmark.name() == Some(name)) + .unwrap() + }; + for name in ["cell", "control", "mixed"] { + assert!(named(name).range().is_some(), "{name}: {:?}", named(name)); + assert_eq!(named(name).direct_range(), None, "{name}"); + } + assert_eq!(named("direct").direct_range(), Some(one_run_range(3))); + } } fn f_x090_cross_part_drawing_package() -> Vec { diff --git a/docs/hld/03-architecture.md b/docs/hld/03-architecture.md index 341f23cb..fb5836d5 100644 --- a/docs/hld/03-architecture.md +++ b/docs/hld/03-architecture.md @@ -1502,7 +1502,9 @@ half-open `RunRange` boundary as comments. `Document::bookmarks` returns immutable correlated summaries in typed main-story paragraph order through tables and block content controls. A reported body index is that recursive paragraph ordinal, and its run index is the accepted-view boundary used to -extract bookmark text. Marker encounter order resolves direction when start +extract bookmark text. `BookmarkRef::direct_range` reports the same range with +the direct body child index of `RunPosition` when both markers sit in direct +body paragraphs, and `None` otherwise. Marker encounter order resolves direction when start and end share one accepted boundary, so end before start remains reversed and start before end is a valid empty range. Isolated projection refresh after a run, comment, or bookmark edit carries the original Word namespace aliases. diff --git a/docs/hld/10-bindings-spec.md b/docs/hld/10-bindings-spec.md index 68022d07..c03b3400 100644 --- a/docs/hld/10-bindings-spec.md +++ b/docs/hld/10-bindings-spec.md @@ -1034,7 +1034,13 @@ preserve a document already redacted through the native facade. Native Word callers use `Document::bookmarks` for immutable `BookmarkRef` summaries and `Document::add_bookmark` for atomic insertion over the existing top-level half-open `RunRange`. A summary exposes an optional id, name, range, -current text, and marker issue. Insertion validates the Word name and both +direct range, current text, and marker issue. The range counts paragraphs +recursively through tables and block content controls. The direct range uses +the `RunPosition` body index that `add_bookmark` takes and is present only when +both markers sit in direct body paragraphs. Its run indexes stay the +accepted-view boundaries of the range, which equal the run indexes +`add_bookmark` takes only in a paragraph without inline content controls or +tracked insertions. Insertion validates the Word name and both boundaries, rejects duplicate or producer-reserved names, and returns the allocated nonnegative id. The shared recursive `Field` model retains the complete `REF` and `PAGEREF` instruction, target argument, cached display, From 58c0c687b28c3df4c86e466c96846044493ba71d Mon Sep 17 00:00:00 2001 From: Hadrien Mary Date: Sun, 27 Sep 2026 17:12:40 +0200 Subject: [PATCH 3/4] Anchor story comments on their paragraph after a block control Document::add_story_comment resolved a StoryRunRange paragraph by counting the paragraph items before it, which are the direct paragraphs of the owner only, and then took that many paragraphs through a resolver that also enters block content controls. When a block content control with paragraphs came before the target, in the body or in a table cell, the comment was anchored on a paragraph inside the control, or refused when that paragraph had too few runs. A body paragraph item now resolves to its direct body slot, the count of direct story items before it, which is the slot StoryItemRef::direct_body_index reports. A cell paragraph item resolves to the direct cell paragraph at the same position. Neither side enters a block content control, so the count and the lookup agree. GitHub issue #163. --- crates/rdocx-py/tests/test_core.py | 34 +++++++++++++++ crates/rdocx/src/document.rs | 38 +++++++++++----- crates/rdocx/tests/regression_test.rs | 62 ++++++++++++++++++++++++++- 3 files changed, 122 insertions(+), 12 deletions(-) diff --git a/crates/rdocx-py/tests/test_core.py b/crates/rdocx-py/tests/test_core.py index 19594290..01c38564 100644 --- a/crates/rdocx-py/tests/test_core.py +++ b/crates/rdocx-py/tests/test_core.py @@ -1383,6 +1383,40 @@ def test_split_run_accepts_a_paragraph_handle_in_a_control_or_the_body(): assert document.to_bytes() == before +def test_story_comment_after_a_block_content_control_anchors_on_its_paragraph(): + import rdocx + + document = _replace_document_body( + rdocx.Document(), + "Alpha." + "" + "Control one." + "" + "Beta paragraph.", + ) + item = next( + item + for item in document.story_items + if item.story.kind == "body" + and item.kind == "paragraph" + and item.text == "Beta paragraph." + ) + comment_id = document.add_comment( + rdocx.StoryRunRange( + start=rdocx.StoryRunPosition(item=item, run_index=0), + end=rdocx.StoryRunPosition(item=item, run_index=1), + ), + author="Ada", + text="Here", + ) + xml = _document_xml(document).decode() + start = xml.index(f'commentRangeStart w:id="{comment_id}"') + end = xml.index(f'commentRangeEnd w:id="{comment_id}"') + assert re.findall(r"]*>([^<]*)", xml[start:end]) == [ + "Beta paragraph." + ] + + def test_python_round_three_authoring_and_inspection_is_typed_and_lossless(): import rdocx diff --git a/crates/rdocx/src/document.rs b/crates/rdocx/src/document.rs index a09eacc7..6564999e 100644 --- a/crates/rdocx/src/document.rs +++ b/crates/rdocx/src/document.rs @@ -13088,7 +13088,7 @@ impl Document { } pub(crate) fn story_paragraph_mut(&mut self, location: &ContentLocation) -> Result<&mut CT_P> { - let (part_name, mut paragraph_index, cell_route) = { + let (part_name, paragraph_slot, cell_route) = { let (source, owner) = self.story_source_and_owner(&location.story)?; let part_name = source.part_name.clone(); let source_xml = source.xml.into_owned(); @@ -13116,15 +13116,25 @@ impl Document { } .into()); } - let paragraph_index = items[..item_index] - .iter() - .filter(|item| item.kind == StoryItemKind::Paragraph) - .count(); + // A paragraph item is a direct child of its owner. In the body it + // resolves to its direct body slot, the count of direct items + // before it, and in a cell to its position among the cell's + // direct paragraphs. Neither counts the paragraphs inside a block + // content control. The body section properties are serialized + // last, so they never precede a paragraph. + let preceding = items[..item_index].iter(); + let paragraph_slot = if location.story.kind == StoryKind::Body { + preceding.filter(|item| item.direct_owner_child).count() + } else { + preceding + .filter(|item| item.kind == StoryItemKind::Paragraph) + .count() + }; let cell_route = (location.story.kind == StoryKind::TableCell && part_name == self.doc_part_name) .then(|| modeled_main_cell_route(&source_xml, &owner)) .transpose()?; - (part_name, paragraph_index, cell_route) + (part_name, paragraph_slot, cell_route) }; if part_name != self.doc_part_name { return Err(Error::Other( @@ -13132,10 +13142,10 @@ impl Document { )); } match location.story.kind { - StoryKind::Body => { - nth_paragraph_in_body(&mut self.document.body.content, &mut paragraph_index) - .ok_or_else(|| Error::Other("comment body paragraph is missing".to_owned())) - } + StoryKind::Body => match self.document.body.content.get_mut(paragraph_slot) { + Some(BodyContent::Paragraph(paragraph)) => Ok(paragraph), + _ => Err(Error::Other("comment body paragraph is missing".to_owned())), + }, StoryKind::TableCell => { let (content_index, mut cell_index) = cell_route.ok_or_else(|| { Error::Other("comment position has no modeled table-cell route".to_owned()) @@ -13154,7 +13164,13 @@ impl Document { BodyContent::Paragraph(_) | BodyContent::RawXml(_) => None, } .ok_or_else(|| Error::Other("comment table cell is missing".to_owned()))?; - nth_paragraph_in_cell(cell, &mut paragraph_index) + cell.content + .iter_mut() + .filter_map(|child| match child { + CellContent::Paragraph(paragraph) => Some(paragraph), + CellContent::Table(_) | CellContent::ContentControl(_) => None, + }) + .nth(paragraph_slot) .ok_or_else(|| Error::Other("comment cell paragraph is missing".to_owned())) } _ => Err(Error::Other( diff --git a/crates/rdocx/tests/regression_test.rs b/crates/rdocx/tests/regression_test.rs index 027a9724..4b0a9faf 100644 --- a/crates/rdocx/tests/regression_test.rs +++ b/crates/rdocx/tests/regression_test.rs @@ -1638,7 +1638,9 @@ fn checked_table_cell_comment_range_is_atomic_and_reopens() { /// child index, and every body API that takes an index must address the same /// paragraph with it, whatever tables or block content controls precede it. mod direct_body_index_coordinates { - use rdocx::{Document, RunPosition, RunRange}; + use rdocx::{ + Document, RunPosition, RunRange, StoryItemKind, StoryKind, StoryRunPosition, StoryRunRange, + }; const TABLE: &str = r#"c00c01c10c11"#; const CONTROL: &str = r#"Control one.Control two."#; @@ -1809,6 +1811,64 @@ mod direct_body_index_coordinates { } assert_eq!(named("direct").direct_range(), Some(one_run_range(3))); } + + fn comment_on_story_paragraph(document: &mut Document, kind: StoryKind, text: &str) -> i32 { + let story = super::f254_story(document, kind); + let location = document + .story_items(&story) + .unwrap() + .into_iter() + .find(|item| { + item.kind() == StoryItemKind::Paragraph + && item.text().unwrap().as_deref() == Some(text) + }) + .unwrap() + .location() + .clone(); + document + .add_story_comment( + StoryRunRange { + start: StoryRunPosition { + location: location.clone(), + run_index: 0, + }, + end: StoryRunPosition { + location, + run_index: 1, + }, + }, + "Ada", + None, + "Here", + ) + .unwrap() + } + + #[test] + fn story_comment_after_a_block_content_control_anchors_on_its_paragraph() { + let mut document = document_from_body(&format!( + "{}{CONTROL}{}", + paragraph("Alpha."), + paragraph("Beta paragraph.") + )); + let id = comment_on_story_paragraph(&mut document, StoryKind::Body, "Beta paragraph."); + assert_eq!(commented_text(&mut document, id), "Beta paragraph."); + } + + #[test] + fn story_comment_in_a_cell_after_a_block_content_control_anchors_on_its_paragraph() { + let mut document = document_from_body(&format!( + r#"{CONTROL}{}{}"#, + paragraph("Target cell paragraph."), + paragraph("After.") + )); + let id = comment_on_story_paragraph( + &mut document, + StoryKind::TableCell, + "Target cell paragraph.", + ); + assert_eq!(commented_text(&mut document, id), "Target cell paragraph."); + } } fn f_x090_cross_part_drawing_package() -> Vec { From 128e61bdee3ac705a4b31d5becc0e362b9afb925 Mon Sep 17 00:00:00 2001 From: Hadrien Mary Date: Sun, 27 Sep 2026 17:16:53 +0200 Subject: [PATCH 4/4] Re-record the rdocx archive measurement The direct body index changes grow the rdocx package, mostly through the new regression tests, so the crates.io archive row of the root README and its ARCHIVE_MEASUREMENTS entry in scripts/readme_doctests.py are re-measured with cargo package, as the Docs job requires. GitHub issue #163. --- README.md | 2 +- scripts/readme_doctests.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 479b462e..f0ac3418 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,095,450 compressed bytes, 6,512,553 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/scripts/readme_doctests.py b/scripts/readme_doctests.py index e24c643b..bd8c280f 100644 --- a/scripts/readme_doctests.py +++ b/scripts/readme_doctests.py @@ -383,7 +383,7 @@ 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_095_450, 6_512_553, 36), "rdocx-cli": (33_805, 145_256, 8), "rdocx-html": (15_486, 63_894, 11), "rdocx-layout": (255_752, 1_385_701, 15),