diff --git a/README.md b/README.md index b7c4a72..0e04b30 100644 --- a/README.md +++ b/README.md @@ -128,7 +128,7 @@ with Document.open("reviewed.docx", author="Editor") as doc: # Find text across element boundaries match = doc.find_text("Aim: To") - if match and match.spans_boundary: + if match and match.spans_revision: print("Text spans a revision boundary") # Replace works even across revision boundaries @@ -147,8 +147,8 @@ from docx_editor import Document, EditOperation with Document.open("contract.docx", author="Editor") as doc: refs = doc.list_paragraphs() doc.batch_edit([ - EditOperation(action="replace", find="old", replace_with="new", paragraph="P2#f3c1"), - EditOperation(action="delete", text="remove this", paragraph="P5#d4e5"), + EditOperation.replace("old", "new", paragraph="P2#f3c1"), + EditOperation.delete("remove this", paragraph="P5#d4e5"), ]) doc.save() ``` diff --git a/docs/api.md b/docs/api.md index 6b27e76..1cc1c47 100644 --- a/docs/api.md +++ b/docs/api.md @@ -191,16 +191,22 @@ Find text in the document, including text spanning XML element boundaries. **Parameters:** - `text` (str): Text to search for -- `occurrence` (int): Which occurrence to return. Defaults to 0. +- `occurrence` (int): Which occurrence to return, counted document-wide. Defaults to 0. -**Returns:** Match information, or None if not found +**Returns:** [`SearchResult`](#searchresult), or None if not found **Example:** ```python match = doc.find_text("Aim: To") -if match and match.spans_boundary: - print("Text spans multiple XML contexts") +if match: + if match.spans_revision: + print("Text spans a tracked-revision boundary") + # The ref + in-paragraph occurrence pin the exact match for a follow-up edit + doc.replace( + "Aim: To", "Goal: To", + paragraph=match.paragraph_ref, occurrence=match.paragraph_occurrence, + ) ``` #### `count_matches(text)` @@ -329,11 +335,14 @@ Apply multiple edits after validating paragraph hashes up front. from docx_editor import EditOperation new_refs = doc.batch_edit([ - EditOperation(action="replace", find="old", replace_with="new", paragraph="P2#f3c1"), - EditOperation(action="delete", text="remove this", paragraph="P5#d4e5"), + EditOperation.replace("old", "new", paragraph="P2#f3c1"), + EditOperation.delete("remove this", paragraph="P5#d4e5"), ]) ``` +Prefer the typed constructors ([`EditOperation`](#editoperation)) — they validate +arguments when the operation is built, so mistakes fail fast instead of at apply time. + #### `batch_rewrite(rewrites)` Rewrite multiple paragraphs after validating paragraph hashes up front. @@ -632,6 +641,114 @@ for rev in revisions: --- +## EditOperation + +A single edit operation for `batch_edit()`. Build operations with the typed +constructors below — they validate arguments at construction time with the same +rules `batch_edit()` applies, so mistakes surface immediately. The raw +`EditOperation(action=..., ...)` form remains supported. + +```python +from docx_editor import EditOperation +``` + +### Constructors + +#### `EditOperation.replace(find, replace_with, *, paragraph, occurrence=0)` + +- `find` (str): Text to find and replace. Must be non-empty. +- `replace_with` (str): Replacement text. Empty string is allowed (replacing + with nothing is a valid tracked deletion). + +#### `EditOperation.delete(text, *, paragraph, occurrence=0)` + +- `text` (str): Text to mark as deleted. Must be non-empty. + +#### `EditOperation.insert_after(anchor, text, *, paragraph, occurrence=0)` + +#### `EditOperation.insert_before(anchor, text, *, paragraph, occurrence=0)` + +- `anchor` (str): Text to find as insertion point. Must be non-empty. +- `text` (str): Text to insert. + +All constructors also take: + +- `paragraph` (str): Paragraph reference from `list_paragraphs()`, such as `P2#f3c1` +- `occurrence` (int): Which occurrence within the paragraph. Defaults to 0. Must be >= 0. + +**Raises:** `ValueError` at construction time if the paragraph ref is malformed, +`occurrence` is negative, a search-target argument (`find`, delete `text`, +`anchor`) is empty, or a payload argument (`replace_with`, insert `text`) is +`None` — payloads may be empty strings, search targets may not. Each +signature mirrors the corresponding `Document` method 1:1, so +`doc.replace(...)` translates mechanically to `EditOperation.replace(...)`. + +### Example + +```python +new_refs = doc.batch_edit([ + EditOperation.replace("30 days", "60 days", paragraph="P2#f3c1"), + EditOperation.delete("obsolete clause", paragraph="P5#d4e5"), + EditOperation.insert_after("Section 5", " (as amended)", paragraph="P7#b1c2"), +]) +``` + +--- + +## SearchResult + +The result of `Document.find_text()`. Carries no XML/DOM internals. + +```python +from docx_editor import SearchResult +``` + +### Attributes + +| Attribute | Type | Description | +|-----------|------|-------------| +| `start` | int | Start offset of the match in the containing paragraph's visible text | +| `end` | int | Exclusive end offset, same coordinate space | +| `text` | str | The matched text | +| `paragraph_ref` | str | Hash-anchored ref like `P3#a7b2`, usable as the `paragraph=` argument of follow-up edits | +| `paragraph_occurrence` | int | Occurrence index of this match within its paragraph, usable as the `occurrence=` argument of follow-up edits | +| `spans_revision` | bool | True if the match crosses a tracked-revision boundary | + +`start`/`end` are offsets within the matched paragraph's visible text, **not** +document-wide offsets. Coordinate systems differ between search and edit: +`find_text`'s `occurrence` counts matches document-wide, while edit methods +count within one paragraph — `paragraph_occurrence` bridges the two, so always +pass it alongside `paragraph_ref` when chaining into an edit. `paragraph_ref` +is computed at search time and — like refs from `list_paragraphs()` — goes +stale once that paragraph is edited. + +### Example + +```python +match = doc.find_text("30 days") +if match: + doc.replace( + "30 days", "60 days", + paragraph=match.paragraph_ref, occurrence=match.paragraph_occurrence, + ) +``` + +--- + +## Deprecated internals + +The text-map machinery (`TextMap`, `TextMapMatch`, `TextPosition`, +`build_text_map`, `find_in_text_map`) is no longer part of the public API: +these names have been removed from `docx_editor.__all__`, and accessing them +via the top-level package emits a `DeprecationWarning`. They will be removed +from the package namespace in the next release. + +Use `Document.find_text()` / [`SearchResult`](#searchresult) instead. If you +genuinely need the internals (raw DOM positions), import them from +`docx_editor.xml_editor`. + +--- + ## Exceptions ### `TextNotFoundError` diff --git a/docs/quickstart.md b/docs/quickstart.md index 614206e..54d1e93 100644 --- a/docs/quickstart.md +++ b/docs/quickstart.md @@ -124,9 +124,9 @@ from docx_editor import Document, EditOperation with Document.open("contract.docx") as doc: new_refs = doc.batch_edit([ - EditOperation(action="replace", find="30 days", replace_with="60 days", paragraph="P2#f3c1"), - EditOperation(action="delete", text="obsolete clause", paragraph="P5#d4e5"), - EditOperation(action="insert_after", anchor="Section 5", text=" (as amended)", paragraph="P3#b2c4"), + EditOperation.replace("30 days", "60 days", paragraph="P2#f3c1"), + EditOperation.delete("obsolete clause", paragraph="P5#d4e5"), + EditOperation.insert_after("Section 5", " (as amended)", paragraph="P3#b2c4"), ]) print(new_refs) doc.save() diff --git a/docx_editor/__init__.py b/docx_editor/__init__.py index 2bb67e1..e22511f 100644 --- a/docx_editor/__init__.py +++ b/docx_editor/__init__.py @@ -54,18 +54,13 @@ WorkspaceSyncError, XMLError, ) -from .track_changes import EditOperation, EditValidationResult, Revision +from .track_changes import EditOperation, EditValidationResult, Revision, SearchResult from .xml_editor import ( ParagraphInfo, ParagraphLocation, ParagraphRef, TableCell, - TextMap, - TextMapMatch, - TextPosition, - build_text_map, compute_paragraph_hash, - find_in_text_map, ) __all__ = [ @@ -74,6 +69,7 @@ "EditOperation", "EditValidationResult", "Revision", + "SearchResult", "Comment", # Exceptions "DocxEditError", @@ -92,16 +88,34 @@ "HashMismatchError", "ParagraphIndexError", "BatchOperationError", - # Text map & paragraph refs - "TextPosition", - "TextMap", - "TextMapMatch", + # Paragraph refs "ParagraphInfo", "ParagraphRef", - "build_text_map", "compute_paragraph_hash", - "find_in_text_map", # Paragraph location "ParagraphLocation", "TableCell", ] + +# Internal text-map machinery removed from the public API (see SearchResult / +# Document.find_text instead). Accessing these via the top-level package warns +# for one release before removal; the real objects remain importable from +# docx_editor.xml_editor. +_DEPRECATED_INTERNALS = frozenset({"TextMap", "TextMapMatch", "TextPosition", "build_text_map", "find_in_text_map"}) + + +def __getattr__(name: str): + if name in _DEPRECATED_INTERNALS: + import warnings + + from . import xml_editor + + warnings.warn( + f"docx_editor.{name} is internal and will be removed from the public API " + f"in the next release; use Document.find_text()/SearchResult instead, or " + f"import from docx_editor.xml_editor if you need the internals.", + DeprecationWarning, + stacklevel=2, + ) + return getattr(xml_editor, name) + raise AttributeError(f"module {__name__!r} has no attribute {name!r}") diff --git a/docx_editor/document.py b/docx_editor/document.py index c71cb88..94770c1 100644 --- a/docx_editor/document.py +++ b/docx_editor/document.py @@ -13,7 +13,7 @@ from .comments import Comment, CommentManager from .exceptions import HashMismatchError, ParagraphIndexError -from .track_changes import EditOperation, EditValidationResult, Revision, RevisionManager +from .track_changes import EditOperation, EditValidationResult, Revision, RevisionManager, SearchResult from .workspace import Workspace from .xml_editor import ( DocxXMLEditor, @@ -162,13 +162,42 @@ def workspace_path(self) -> Path: # ==================== Track Changes API ==================== - def find_text(self, text: str, occurrence: int = 0): + def find_text(self, text: str, occurrence: int = 0) -> SearchResult | None: """Find text in the document, including across element boundaries. - Returns match info or None if not found. + Args: + text: Text to search for + occurrence: Which occurrence document-wide (0 = first, 1 = second, etc.) + + Returns: + A SearchResult, or None if the text (or that occurrence) is not + found. Fields: + + - ``start`` / ``end``: character offsets of the match in the + *containing paragraph's* visible text (not document-wide). + - ``text``: the matched text. + - ``paragraph_ref``: hash-anchored ref like "P3#a7b2", directly + usable as the ``paragraph=`` argument of follow-up edits. Valid + until that paragraph is edited. + - ``paragraph_occurrence``: occurrence index of this match within + its paragraph — pass as the ``occurrence=`` of a follow-up edit + (edit methods count occurrences within the paragraph, not + document-wide). + - ``spans_revision``: True if the match crosses a tracked-revision + boundary (e.g. part of it is inside a tracked insertion). + + Example: + match = doc.find_text("30 days") + if match: + doc.replace( + "30 days", + "60 days", + paragraph=match.paragraph_ref, + occurrence=match.paragraph_occurrence, + ) """ self._ensure_open() - return self._revision_manager._find_across_boundaries(text, occurrence) + return self._revision_manager.find_text(text, occurrence) def count_matches(self, text: str) -> int: """Count how many times a text string appears in the document. @@ -587,8 +616,8 @@ def batch_edit( Example: new_refs = doc.batch_edit([ - EditOperation(action="replace", find="old", replace_with="new", paragraph="P20#a7b2"), - EditOperation(action="delete", text="remove", paragraph="P15#f3c1"), + EditOperation.replace("old", "new", paragraph="P20#a7b2"), + EditOperation.delete("remove", paragraph="P15#f3c1"), ]) # Pre-flight the batch (note: same-paragraph sequential effects are diff --git a/docx_editor/track_changes.py b/docx_editor/track_changes.py index 7cc7da1..249286e 100644 --- a/docx_editor/track_changes.py +++ b/docx_editor/track_changes.py @@ -33,7 +33,14 @@ @dataclass class EditOperation: - """A single edit operation for batch processing.""" + """A single edit operation for batch processing. + + Prefer the typed constructors (:meth:`replace`, :meth:`delete`, + :meth:`insert_after`, :meth:`insert_before`) — they validate arguments at + construction time with the same rules ``batch_edit`` applies, so mistakes + surface immediately instead of at apply time. The raw + ``EditOperation(action=..., ...)`` form remains supported. + """ action: Literal["replace", "delete", "insert_after", "insert_before"] paragraph: str # Required: hash-anchored ref like "P3#a7b2" @@ -43,6 +50,107 @@ class EditOperation: anchor: str | None = None # For insert_after/insert_before occurrence: int = 0 + @staticmethod + def _validate_common(constructor: str, paragraph: str, occurrence: int) -> None: + """Construction-time checks shared by all typed constructors.""" + ParagraphRef.parse(paragraph) + if occurrence < 0: + raise ValueError(f"EditOperation.{constructor}(): occurrence must be >= 0, got {occurrence}") + + @classmethod + def replace(cls, find: str, replace_with: str, *, paragraph: str, occurrence: int = 0) -> "EditOperation": + """Build a validated replace operation (mirrors ``Document.replace``). + + Args: + find: Text to find and replace (must be non-empty) + replace_with: Replacement text (empty string allowed — replacing + with nothing is a valid tracked deletion) + paragraph: Paragraph reference from list_paragraphs() (e.g., "P2#f3c1") + occurrence: Which occurrence within the paragraph (0 = first) + + Raises: + ValueError: If the paragraph ref is malformed, ``occurrence`` is + negative, ``find`` is empty, or ``replace_with`` is None. + """ + cls._validate_common("replace", paragraph, occurrence) + if not find: + raise ValueError("EditOperation.replace(): 'find' must be a non-empty string — the text to search for") + if replace_with is None: + raise ValueError("EditOperation.replace(): 'replace_with' must be a string (empty string is allowed)") + return cls( + action="replace", + paragraph=paragraph, + find=find, + replace_with=replace_with, + occurrence=occurrence, + ) + + @classmethod + def delete(cls, text: str, *, paragraph: str, occurrence: int = 0) -> "EditOperation": + """Build a validated delete operation (mirrors ``Document.delete``). + + Args: + text: Text to mark as deleted (must be non-empty) + paragraph: Paragraph reference from list_paragraphs() (e.g., "P2#f3c1") + occurrence: Which occurrence within the paragraph (0 = first) + + Raises: + ValueError: If the paragraph ref is malformed, ``occurrence`` is + negative, or ``text`` is empty. + """ + cls._validate_common("delete", paragraph, occurrence) + if not text: + raise ValueError("EditOperation.delete(): 'text' must be a non-empty string — the text to mark as deleted") + return cls(action="delete", paragraph=paragraph, text=text, occurrence=occurrence) + + @classmethod + def _insert( + cls, + action: Literal["insert_after", "insert_before"], + anchor: str, + text: str, + paragraph: str, + occurrence: int, + ) -> "EditOperation": + cls._validate_common(action, paragraph, occurrence) + if not anchor: + raise ValueError(f"EditOperation.{action}(): 'anchor' must be a non-empty string — the text to insert near") + if text is None: + raise ValueError(f"EditOperation.{action}(): 'text' must be a string (empty string is allowed)") + return cls(action=action, paragraph=paragraph, anchor=anchor, text=text, occurrence=occurrence) + + @classmethod + def insert_after(cls, anchor: str, text: str, *, paragraph: str, occurrence: int = 0) -> "EditOperation": + """Build a validated insert_after operation (mirrors ``Document.insert_after``). + + Args: + anchor: Text to find as insertion point (must be non-empty) + text: Text to insert after the anchor + paragraph: Paragraph reference from list_paragraphs() (e.g., "P2#f3c1") + occurrence: Which occurrence of anchor within the paragraph (0 = first) + + Raises: + ValueError: If the paragraph ref is malformed, ``occurrence`` is + negative, ``anchor`` is empty, or ``text`` is None. + """ + return cls._insert("insert_after", anchor, text, paragraph, occurrence) + + @classmethod + def insert_before(cls, anchor: str, text: str, *, paragraph: str, occurrence: int = 0) -> "EditOperation": + """Build a validated insert_before operation (mirrors ``Document.insert_before``). + + Args: + anchor: Text to find as insertion point (must be non-empty) + text: Text to insert before the anchor + paragraph: Paragraph reference from list_paragraphs() (e.g., "P2#f3c1") + occurrence: Which occurrence of anchor within the paragraph (0 = first) + + Raises: + ValueError: If the paragraph ref is malformed, ``occurrence`` is + negative, ``anchor`` is empty, or ``text`` is None. + """ + return cls._insert("insert_before", anchor, text, paragraph, occurrence) + @dataclass class EditValidationResult: @@ -54,6 +162,41 @@ class EditValidationResult: error: str | None = None # human-readable reason when not valid +@dataclass(frozen=True) +class SearchResult: + """Public result of ``Document.find_text`` — no DOM internals. + + ``start``/``end`` are character offsets in the *containing paragraph's* + visible text (text maps are per-paragraph), not document-wide offsets. + + ``paragraph_ref`` is computed at search time and is directly usable as the + ``paragraph=`` argument of follow-up edits; like refs from + ``list_paragraphs()``, it is valid until that paragraph is edited. + + ``find_text``'s ``occurrence`` counts matches document-wide, while edit + methods count within one paragraph — ``paragraph_occurrence`` bridges the + two: pass it as the ``occurrence=`` of a follow-up edit to target exactly + the match ``find_text`` located. + """ + + start: int # Start offset in the paragraph's visible text + end: int # Exclusive end offset, same coordinate space + text: str # The matched text + paragraph_ref: str # Hash-anchored ref like "P3#a7b2" + paragraph_occurrence: int # Occurrence index of this match within its paragraph + spans_revision: bool # True if the match crosses a tracked-revision boundary + + +@dataclass(frozen=True) +class _LocatedMatch: + """Internal: a text-map match plus the paragraph identity needed for refs.""" + + match: TextMapMatch + paragraph_index: int # 1-based document-order index of the containing w:p + paragraph: Element # The containing w:p element + paragraph_occurrence: int # Occurrence index of the match within that paragraph + + @dataclass class Revision: """Represents a tracked change (insertion or deletion).""" @@ -575,14 +718,17 @@ def _find_match_containing_node(self, text: str, elem) -> TextMapMatch | None: local_occ += 1 return None - def _find_across_boundaries(self, text: str, occurrence: int = 0) -> TextMapMatch | None: + def _find_across_boundaries_located(self, text: str, occurrence: int = 0) -> _LocatedMatch | None: """Find the nth occurrence of text across element boundaries. - Searches across all paragraphs using text maps. - Returns TextMapMatch or None. + Searches across all paragraphs using text maps, keeping paragraph + identity so callers can build hash-anchored refs. + + Returns: + A _LocatedMatch, or None if not found. """ current_occurrence = 0 - for paragraph in self.editor.dom.getElementsByTagName("w:p"): + for idx, paragraph in enumerate(self.editor.dom.getElementsByTagName("w:p"), start=1): text_map = build_text_map(paragraph) local_occ = 0 while True: @@ -590,11 +736,43 @@ def _find_across_boundaries(self, text: str, occurrence: int = 0) -> TextMapMatc if match is None: break if current_occurrence == occurrence: - return match + return _LocatedMatch( + match=match, + paragraph_index=idx, + paragraph=paragraph, + paragraph_occurrence=local_occ, + ) current_occurrence += 1 local_occ += 1 return None + def _find_across_boundaries(self, text: str, occurrence: int = 0) -> TextMapMatch | None: + """Find the nth occurrence of text across element boundaries. + + Searches across all paragraphs using text maps. + Returns TextMapMatch or None. + """ + located = self._find_across_boundaries_located(text, occurrence) + return located.match if located is not None else None + + def find_text(self, text: str, occurrence: int = 0) -> SearchResult | None: + """Find the nth occurrence of text, as a public SearchResult. + + Searches across element boundaries; ``occurrence`` counts matches + document-wide (0 = first). Returns None if not found. + """ + located = self._find_across_boundaries_located(text, occurrence) + if located is None: + return None + return SearchResult( + start=located.match.start, + end=located.match.end, + text=located.match.text, + paragraph_ref=f"P{located.paragraph_index}#{compute_paragraph_hash(located.paragraph)}", + paragraph_occurrence=located.paragraph_occurrence, + spans_revision=located.match.spans_boundary, + ) + def replace_text(self, find: str, replace_with: str, occurrence: int = 0, paragraph: str | None = None) -> int: """Replace text with tracked changes (deletion + insertion). diff --git a/skills/docx/SKILL.md b/skills/docx/SKILL.md index 72cb88e..b782705 100644 --- a/skills/docx/SKILL.md +++ b/skills/docx/SKILL.md @@ -218,7 +218,9 @@ for p in doc.list_paragraphs(): # P2#f3c1| The payment term is 30 days... # P3#b2c4| Section 3. Terms and conditions... -# Find text (returns TextMapMatch or None, works across element boundaries) +# Find text (returns SearchResult or None, works across element boundaries). +# Chain into an edit with paragraph=match.paragraph_ref plus +# occurrence=match.paragraph_occurrence (edits count occurrences per paragraph). match = doc.find_text("30 days") # Get all visible text (inserted text included, deleted text excluded) @@ -431,14 +433,16 @@ from docx_editor import Document, EditOperation with Document.open("file.docx", author=author) as doc: refs = doc.list_paragraphs() new_refs = doc.batch_edit([ - EditOperation(action="replace", find="old term", replace_with="new term", paragraph="P2#f3c1"), - EditOperation(action="delete", text="remove this", paragraph="P5#d4e5"), - EditOperation(action="insert_after", anchor="Section 5", text=" (amended)", paragraph="P3#b2c4"), + EditOperation.replace("old term", "new term", paragraph="P2#f3c1"), + EditOperation.delete("remove this", paragraph="P5#d4e5"), + EditOperation.insert_after("Section 5", " (amended)", paragraph="P3#b2c4"), ]) # new_refs[0] = "P2#c3d4" — fresh ref for paragraph 2 doc.save() ``` +Build operations with the typed constructors (`EditOperation.replace/.delete/.insert_after/.insert_before` — same signatures as the `Document` methods). They validate arguments immediately and raise `ValueError` with a field-specific message, instead of failing later at apply time. + If any hash is stale, the entire batch is rejected before any edits are applied. ### Error Handling & Recovery @@ -522,9 +526,9 @@ doc.replace("30", "60", paragraph="P2#f3c1") # Multiple independent swaps — use batch_edit(): # "CFO" → "Finance Director", "audit committee" → "board", "December 31st" → "January 15th" doc.batch_edit([ - EditOperation(action="replace", find="CFO", replace_with="Finance Director", paragraph="P5#a7b2"), - EditOperation(action="replace", find="audit committee", replace_with="board", paragraph="P5#a7b2"), - EditOperation(action="replace", find="December 31st", replace_with="January 15th", paragraph="P5#a7b2"), + EditOperation.replace("CFO", "Finance Director", paragraph="P5#a7b2"), + EditOperation.replace("audit committee", "board", paragraph="P5#a7b2"), + EditOperation.replace("December 31st", "January 15th", paragraph="P5#a7b2"), ]) ``` diff --git a/tests/test_batch_edit.py b/tests/test_batch_edit.py index 1b33a26..010b608 100644 --- a/tests/test_batch_edit.py +++ b/tests/test_batch_edit.py @@ -824,3 +824,106 @@ def boom(xml_bytes): # raises before any mutation, so visible text is still unchanged # even though the rollback re-parse itself was swallowed. assert doc.get_visible_text() == before_text + + +class TestEditOperationConstructors: + """Typed constructors validate at construction time, mirroring apply-time rules.""" + + def test_replace_builds_op(self): + op = EditOperation.replace("old", "new", paragraph="P2#f3c1", occurrence=1) + assert op == EditOperation(action="replace", paragraph="P2#f3c1", find="old", replace_with="new", occurrence=1) + + def test_delete_builds_op(self): + op = EditOperation.delete("gone", paragraph="P2#f3c1") + assert op == EditOperation(action="delete", paragraph="P2#f3c1", text="gone") + + def test_insert_after_builds_op(self): + op = EditOperation.insert_after("anchor", " tail", paragraph="P2#f3c1") + assert op == EditOperation(action="insert_after", paragraph="P2#f3c1", anchor="anchor", text=" tail") + + def test_insert_before_builds_op(self): + op = EditOperation.insert_before("anchor", "head ", paragraph="P2#f3c1") + assert op == EditOperation(action="insert_before", paragraph="P2#f3c1", anchor="anchor", text="head ") + + def test_typed_ops_apply_end_to_end(self, multi_para_doc): + """A batch built exclusively from typed constructors applies cleanly.""" + doc, _ = multi_para_doc + refs = doc.list_paragraphs() + ops = [ + EditOperation.replace("item 3", "ITEM_THREE", paragraph=refs[2].split("|")[0]), + EditOperation.delete("The report includes findings", paragraph=refs[4].split("|")[0]), + EditOperation.insert_after("item 7", " (amended)", paragraph=refs[6].split("|")[0]), + EditOperation.insert_before("The committee", "NOTE: ", paragraph=refs[8].split("|")[0]), + ] + doc.batch_edit(ops) + # Inserts land relative to the run containing the anchor (each fixture + # paragraph is a single run), so assert per-paragraph outcomes. + paras = doc.list_paragraphs(max_chars=200) + assert "ITEM_THREE" in paras[2] + assert "The report includes findings" not in paras[4] + assert "(amended)" in paras[6] + assert "NOTE: " in paras[8] + + def test_constructor_rules_match_apply_rules(self, multi_para_doc): + """Drift guard: whatever the constructors accept must pass apply-time validation. + + If ``_resolve_action_target`` ever becomes stricter than the typed + constructors (or vice versa), this dry-run disagrees and fails. + """ + doc, _ = multi_para_doc + refs = doc.list_paragraphs() + ops = [ + # Boundary inputs the constructors deliberately allow: + EditOperation.replace("item 1", "", paragraph=refs[0].split("|")[0]), + EditOperation.delete("item 2", paragraph=refs[1].split("|")[0]), + EditOperation.insert_after("item 3", "", paragraph=refs[2].split("|")[0]), + EditOperation.insert_before("item 4", "x", paragraph=refs[3].split("|")[0]), + ] + results = doc.batch_edit(ops, dry_run=True) + assert all(r.valid for r in results), [r.error for r in results] + + def test_malformed_paragraph_ref_rejected(self): + with pytest.raises(ValueError, match="Invalid paragraph reference 'P3'"): + EditOperation.replace("a", "b", paragraph="P3") + + def test_empty_paragraph_ref_rejected(self): + with pytest.raises(ValueError, match="Invalid paragraph reference"): + EditOperation.delete("a", paragraph="") + + def test_negative_occurrence_rejected(self): + with pytest.raises(ValueError, match=r"EditOperation\.replace\(\): occurrence must be >= 0, got -1"): + EditOperation.replace("a", "b", paragraph="P2#f3c1", occurrence=-1) + + def test_replace_empty_find_rejected(self): + with pytest.raises(ValueError, match=r"EditOperation\.replace\(\): 'find' must be a non-empty string"): + EditOperation.replace("", "b", paragraph="P2#f3c1") + + def test_delete_empty_text_rejected(self): + with pytest.raises(ValueError, match=r"EditOperation\.delete\(\): 'text' must be a non-empty string"): + EditOperation.delete("", paragraph="P2#f3c1") + + def test_insert_after_empty_anchor_rejected(self): + with pytest.raises(ValueError, match=r"EditOperation\.insert_after\(\): 'anchor' must be a non-empty"): + EditOperation.insert_after("", "x", paragraph="P2#f3c1") + + def test_insert_before_empty_anchor_rejected(self): + with pytest.raises(ValueError, match=r"EditOperation\.insert_before\(\): 'anchor' must be a non-empty"): + EditOperation.insert_before("", "x", paragraph="P2#f3c1") + + def test_replace_with_empty_string_accepted(self): + """Replacing with nothing is a valid tracked deletion — parity with apply-time rules.""" + op = EditOperation.replace("old", "", paragraph="P2#f3c1") + assert op.replace_with == "" + + def test_replace_with_none_rejected(self): + with pytest.raises(ValueError, match=r"EditOperation\.replace\(\): 'replace_with' must be a string"): + EditOperation.replace("old", None, paragraph="P2#f3c1") # type: ignore[arg-type] + + def test_insert_empty_text_accepted(self): + """Empty insert text is allowed at apply time, so the constructor allows it too.""" + op = EditOperation.insert_after("anchor", "", paragraph="P2#f3c1") + assert op.text == "" + + def test_insert_none_text_rejected(self): + with pytest.raises(ValueError, match=r"EditOperation\.insert_before\(\): 'text' must be a string"): + EditOperation.insert_before("anchor", None, paragraph="P2#f3c1") # type: ignore[arg-type] diff --git a/tests/test_coverage_gaps.py b/tests/test_coverage_gaps.py index ea0d94e..4df7f2a 100644 --- a/tests/test_coverage_gaps.py +++ b/tests/test_coverage_gaps.py @@ -189,7 +189,7 @@ def test_delete_spanning_regular_and_insertion(self, clean_workspace): doc.insert_after("fox", " ADDED", paragraph=ref) match = doc.find_text("fox ADDED") - if match is None or not match.spans_boundary: + if match is None or not match.spans_revision: doc.close() pytest.skip("Expected boundary not created") @@ -209,7 +209,7 @@ def test_delete_spanning_insertion_and_regular(self, clean_workspace): doc.insert_after("fox", " ADDED", paragraph=ref) match = doc.find_text("ADDED jumps") - if match is None or not match.spans_boundary: + if match is None or not match.spans_revision: doc.close() pytest.skip("Expected boundary not created") @@ -226,7 +226,9 @@ def test_delete_entirely_within_insertion(self, clean_workspace): ref = find_ref(doc, "fox") doc.insert_after("fox", " beautiful amazing", paragraph=ref) - match = doc.find_text("beautiful") + # Precondition needs per-character ins-state, which the public + # SearchResult intentionally hides — use the internal match here. + match = doc._revision_manager._find_across_boundaries("beautiful") if match is None or not all(p.is_inside_ins for p in match.positions): doc.close() pytest.skip("Text not entirely within insertion") @@ -245,7 +247,7 @@ def test_delete_mixed_state_roundtrip(self, clean_workspace, temp_dir): doc.insert_after("fox", " ADDED", paragraph=ref) match = doc.find_text("fox ADDED") - if match is None or not match.spans_boundary: + if match is None or not match.spans_revision: doc.close() pytest.skip("Expected boundary not created") @@ -402,7 +404,7 @@ def test_find_text_second_occurrence(self, clean_workspace): if match0 is None or match1 is None: doc.close() pytest.skip("Not enough occurrences of 'the'") - assert match0.positions[0].node is not match1.positions[0].node + assert (match0.paragraph_ref, match0.start) != (match1.paragraph_ref, match1.start) doc.close() def test_replace_second_occurrence_cross_boundary(self, clean_workspace): @@ -464,7 +466,7 @@ def test_insert_after_cross_boundary_anchor(self, clean_workspace): doc.insert_after("fox", " RED", paragraph=ref) match = doc.find_text("fox RED") - if match is None or not match.spans_boundary: + if match is None or not match.spans_revision: doc.close() pytest.skip("Boundary not created") @@ -482,7 +484,7 @@ def test_insert_before_cross_boundary_anchor(self, clean_workspace): doc.insert_after("fox", " RED", paragraph=ref) match = doc.find_text("fox RED") - if match is None or not match.spans_boundary: + if match is None or not match.spans_revision: doc.close() pytest.skip("Boundary not created") @@ -500,7 +502,7 @@ def test_insert_cross_boundary_roundtrip(self, clean_workspace, temp_dir): doc.insert_after("fox", " RED", paragraph=ref) match = doc.find_text("fox RED") - if match is None or not match.spans_boundary: + if match is None or not match.spans_revision: doc.close() pytest.skip("Boundary not created") diff --git a/tests/test_document.py b/tests/test_document.py index b5f75f3..50abc90 100644 --- a/tests/test_document.py +++ b/tests/test_document.py @@ -1,12 +1,14 @@ """Tests for the main Document class.""" import json +import re import zipfile import pytest from conftest import find_ref -from docx_editor import Document +import docx_editor +from docx_editor import Document, SearchResult from docx_editor.exceptions import DocumentOpenError, WorkspaceSyncError from docx_editor.workspace import Workspace @@ -677,8 +679,12 @@ def test_find_text_simple(self, clean_workspace): doc = Document.open(clean_workspace) match = doc.find_text("fox") assert match is not None + assert isinstance(match, SearchResult) assert match.text == "fox" - assert not match.spans_boundary + assert not match.spans_revision + assert re.fullmatch(r"P\d+#[0-9a-f]{4}", match.paragraph_ref) + assert 0 <= match.start < match.end + assert match.paragraph_occurrence == 0 doc.close() def test_find_text_not_found(self, clean_workspace): @@ -697,7 +703,35 @@ def test_find_text_after_insertion(self, clean_workspace): # "dog. INSERTED" spans original run + insertion match = doc.find_text("dog. INSERTED") assert match is not None - assert match.spans_boundary + assert match.spans_revision + doc.close() + + def test_find_text_ref_chains_into_edit(self, clean_workspace): + """paragraph_ref from find_text is directly usable for a follow-up edit.""" + doc = Document.open(clean_workspace) + match = doc.find_text("fox") + assert match is not None + doc.replace("fox", "cat", paragraph=match.paragraph_ref) + assert "cat" in doc.get_visible_text() + doc.close() + + def test_find_text_paragraph_occurrence_chains_into_edit(self, clean_workspace): + """paragraph_occurrence pins which in-paragraph match a follow-up edit targets. + + find_text's occurrence counts document-wide; edit methods count within + the paragraph. Without passing paragraph_occurrence through, the edit + would silently target the paragraph's first match instead of the one + find_text located. + """ + doc = Document.open(clean_workspace) + # "he" appears twice in the fox paragraph: in "The" and in "the lazy" + match = doc.find_text("he", occurrence=1) + assert match is not None + assert match.paragraph_occurrence == 1 + doc.replace("he", "XX", paragraph=match.paragraph_ref, occurrence=match.paragraph_occurrence) + text = doc.get_visible_text() + assert "tXX lazy" in text + assert "The quick" in text # the paragraph's first match is untouched doc.close() def test_find_text_after_close_raises(self, clean_workspace): @@ -707,6 +741,34 @@ def test_find_text_after_close_raises(self, clean_workspace): doc.find_text("test") +class TestPublicApiSurface: + """The text-map internals are deprecated at the top level; SearchResult is public.""" + + DEPRECATED = ["TextMap", "TextMapMatch", "TextPosition", "build_text_map", "find_in_text_map"] + + @pytest.mark.parametrize("name", DEPRECATED) + def test_deprecated_internal_warns_and_forwards(self, name): + """Accessing an internal via the top-level package warns and returns the real object.""" + from docx_editor import xml_editor + + with pytest.warns(DeprecationWarning, match=f"docx_editor.{name} is internal"): + obj = getattr(docx_editor, name) + assert obj is getattr(xml_editor, name) + + def test_deprecated_internals_not_in_all(self): + for name in self.DEPRECATED: + assert name not in docx_editor.__all__ + + def test_search_result_is_public(self): + assert "SearchResult" in docx_editor.__all__ + assert docx_editor.SearchResult is SearchResult + + def test_unknown_attribute_raises(self): + name = "DoesNotExist" + with pytest.raises(AttributeError, match="no attribute 'DoesNotExist'"): + getattr(docx_editor, name) + + class TestDocumentSaveStaleness: def test_document_save_raises_on_external_change(self, temp_docx): import os diff --git a/tests/test_mixed_state.py b/tests/test_mixed_state.py index b53af0f..c7e68ad 100644 --- a/tests/test_mixed_state.py +++ b/tests/test_mixed_state.py @@ -59,7 +59,7 @@ def test_replace_spanning_regular_and_insertion(self, clean_workspace): doc.insert_after("dog.", " ADDED", paragraph=ref) match = doc.find_text("dog. ADDED") - if match is None or not match.spans_boundary: + if match is None or not match.spans_revision: doc.close() pytest.skip("Expected boundary not created") @@ -86,7 +86,9 @@ def test_replace_fully_within_insertion(self, clean_workspace): ref = find_ref(doc, "dog.") doc.insert_after("dog.", " beautiful amazing", paragraph=ref) - match = doc.find_text("beautiful") + # Precondition needs per-character ins-state, which the public + # SearchResult intentionally hides — use the internal match here. + match = doc._revision_manager._find_across_boundaries("beautiful") if match is None: doc.close() pytest.skip("Text not found in insertion") @@ -129,7 +131,7 @@ def test_rpr_preservation_across_split(self, clean_workspace): doc.insert_after("dog.", " ADDED", paragraph=ref) match = doc.find_text("dog. ADDED") - if match is None or not match.spans_boundary: + if match is None or not match.spans_revision: doc.close() pytest.skip("Expected boundary not created") @@ -146,7 +148,7 @@ def test_roundtrip_mixed_state_replace(self, clean_workspace, temp_dir): doc.insert_after("dog.", " ADDED", paragraph=ref) match = doc.find_text("dog. ADDED") - if match is None or not match.spans_boundary: + if match is None or not match.spans_revision: doc.close() pytest.skip("Expected boundary not created") @@ -168,7 +170,9 @@ def test_roundtrip_within_insertion_replace(self, clean_workspace, temp_dir): ref = find_ref(doc, "dog.") doc.insert_after("dog.", " beautiful amazing", paragraph=ref) - match = doc.find_text("beautiful") + # Precondition needs per-character ins-state, which the public + # SearchResult intentionally hides — use the internal match here. + match = doc._revision_manager._find_across_boundaries("beautiful") if match is None or not all(pos.is_inside_ins for pos in match.positions): doc.close() pytest.skip("Text not entirely within insertion") diff --git a/tests/test_multi_wt.py b/tests/test_multi_wt.py index f569e9d..1c3eda6 100644 --- a/tests/test_multi_wt.py +++ b/tests/test_multi_wt.py @@ -185,8 +185,10 @@ def test_delete_in_multi_wt_insertion(self, tmp_path): text = doc.get_visible_text() assert "hello world" in text - # Delete "o wor" — spans both w:t nodes inside the insertion - match = doc.find_text("o wor") + # Delete "o wor" — spans both w:t nodes inside the insertion. + # Per-character ins-state is not on the public SearchResult — use the + # internal match for this structural assertion. + match = doc._revision_manager._find_across_boundaries("o wor") assert match is not None assert all(pos.is_inside_ins for pos in match.positions)