diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..7f111dd --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,18 @@ + +# OpenSpec Instructions + +These instructions are for AI assistants working in this project. + +Always open `@/openspec/AGENTS.md` when the request: +- Mentions planning or proposals (words like proposal, spec, change, plan) +- Introduces new capabilities, breaking changes, architecture shifts, or big performance/security work +- Sounds ambiguous and you need the authoritative spec before coding + +Use `@/openspec/AGENTS.md` to learn: +- How to create and apply change proposals +- Spec format and conventions +- Project structure and guidelines + +Keep this managed block so 'openspec update' can refresh the instructions. + + diff --git a/CLAUDE.md b/CLAUDE.md index 41a1b2a..92f53ae 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,3 +1,22 @@ + +# OpenSpec Instructions + +These instructions are for AI assistants working in this project. + +Always open `@/openspec/AGENTS.md` when the request: +- Mentions planning or proposals (words like proposal, spec, change, plan) +- Introduces new capabilities, breaking changes, architecture shifts, or big performance/security work +- Sounds ambiguous and you need the authoritative spec before coding + +Use `@/openspec/AGENTS.md` to learn: +- How to create and apply change proposals +- Spec format and conventions +- Project structure and guidelines + +Keep this managed block so 'openspec update' can refresh the instructions. + + + # Claude Instructions for docx-editor ## Quick Reference diff --git a/Makefile b/Makefile index e5524cf..04ab3c9 100644 --- a/Makefile +++ b/Makefile @@ -16,7 +16,7 @@ check: ## Run code quality tools. .PHONY: test test: ## Test the code with pytest @echo "πŸš€ Testing code: Running pytest" - @uv run python -m pytest --cov --cov-config=pyproject.toml --cov-report=xml + @uv run python -m pytest tests -n auto --cov --cov-config=pyproject.toml --cov-report=xml .PHONY: build build: clean-build ## Build wheel file diff --git a/README.md b/README.md index 06204ac..3013233 100644 --- a/README.md +++ b/README.md @@ -16,6 +16,8 @@ Pure Python library for Word document track changes and comments, without requir ## Features - **Track Changes**: Replace, delete, and insert text with revision tracking +- **Cross-Boundary Editing**: Find and replace text spanning multiple XML elements and revision boundaries +- **Mixed-State Editing**: Atomic decomposition for text spanning ``/`` boundaries - **Comments**: Add, reply, resolve, and delete comments - **Revision Management**: List, accept, and reject tracked changes - **Cross-Platform**: Works on Linux, macOS, and Windows @@ -48,6 +50,28 @@ with Document.open("contract.docx") as doc: doc.save() ``` +### Cross-Boundary Text Operations + +Text in Word documents with tracked changes can span revision boundaries. `docx-editor` handles this transparently: + +```python +from docx_editor import Document + +with Document.open("reviewed.docx") as doc: + # Get visible text (inserted text included, deleted excluded) + text = doc.get_visible_text() + + # Find text across element boundaries + match = doc.find_text("Aim: To") + if match and match.spans_boundary: + print("Text spans a revision boundary") + + # Replace works even across revision boundaries + doc.replace("Aim: To", "Goal: To") + + doc.save() +``` + ## Claude Code Skill This repo includes a skill for [Claude Code](https://claude.ai/claude-code) that enables AI-assisted Word document editing. diff --git a/docx_editor/__init__.py b/docx_editor/__init__.py index f6c8633..a744909 100644 --- a/docx_editor/__init__.py +++ b/docx_editor/__init__.py @@ -45,6 +45,7 @@ XMLError, ) from .track_changes import Revision +from .xml_editor import TextMap, TextMapMatch, TextPosition, build_text_map, find_in_text_map __all__ = [ # Main classes @@ -64,4 +65,10 @@ "RevisionError", "CommentError", "TextNotFoundError", + # Text map + "TextPosition", + "TextMap", + "TextMapMatch", + "build_text_map", + "find_in_text_map", ] diff --git a/docx_editor/document.py b/docx_editor/document.py index b36a24d..0f2455f 100644 --- a/docx_editor/document.py +++ b/docx_editor/document.py @@ -12,7 +12,7 @@ from .exceptions import WorkspaceSyncError from .track_changes import Revision, RevisionManager from .workspace import Workspace -from .xml_editor import DocxXMLEditor +from .xml_editor import DocxXMLEditor, build_text_map class Document: @@ -117,6 +117,14 @@ def source_path(self) -> Path: # ==================== Track Changes API ==================== + def find_text(self, text: str, occurrence: int = 0): + """Find text in the document, including across element boundaries. + + Returns match info or None if not found. + """ + self._ensure_open() + return self._revision_manager._find_across_boundaries(text, occurrence) + def count_matches(self, text: str) -> int: """Count how many times a text string appears in the document. @@ -137,6 +145,23 @@ def count_matches(self, text: str) -> int: self._ensure_open() return self._revision_manager.count_matches(text) + def get_visible_text(self) -> str: + """Get the visible text of the document. + + Returns flattened text with paragraphs separated by newlines. + Inserted text is included, deleted text is excluded. + + Returns: + The visible text content + """ + self._ensure_open() + paragraphs = self._document_editor.dom.getElementsByTagName("w:p") + parts = [] + for p in paragraphs: + tm = build_text_map(p) + parts.append(tm.text) + return "\n".join(parts) + def replace(self, find: str, replace_with: str, occurrence: int = 0) -> int: """Replace text with tracked changes. diff --git a/docx_editor/track_changes.py b/docx_editor/track_changes.py index 003270d..65281e7 100644 --- a/docx_editor/track_changes.py +++ b/docx_editor/track_changes.py @@ -3,12 +3,14 @@ Provides RevisionManager for creating and managing tracked changes (insertions/deletions). """ +from collections import OrderedDict from dataclasses import dataclass from datetime import datetime from typing import Literal +from xml.dom.minidom import Element from .exceptions import RevisionError, TextNotFoundError -from .xml_editor import DocxXMLEditor +from .xml_editor import DocxXMLEditor, TextMapMatch, TextPosition, build_text_map, find_in_text_map @dataclass @@ -44,14 +46,22 @@ def __init__(self, editor: DocxXMLEditor): def count_matches(self, text: str) -> int: """Count how many times a text string appears in the document. + Uses text maps for accurate counting across element boundaries. + Args: text: Text to search for Returns: Number of occurrences found """ - matches = self.editor.find_all_nodes(tag="w:t", contains=text) - return len(matches) + count = 0 + for paragraph in self.editor.dom.getElementsByTagName("w:p"): + text_map = build_text_map(paragraph) + local_occ = 0 + while find_in_text_map(text_map, text, local_occ) is not None: + count += 1 + local_occ += 1 + return count def _get_nth_match(self, text: str, occurrence: int): """Get the nth occurrence of text (0-indexed). @@ -76,6 +86,45 @@ def _get_nth_match(self, text: str, occurrence: int): ) return matches[occurrence] + def _find_match_containing_node(self, text: str, elem) -> TextMapMatch | None: + """Find the TextMapMatch for `text` whose positions include `elem`. + + Searches through all paragraphs to find the match that involves the + specific w:t node, avoiding occurrence-index drift between search methods. + """ + elem_id = id(elem) + for paragraph in self.editor.dom.getElementsByTagName("w:p"): + text_map = build_text_map(paragraph) + local_occ = 0 + while True: + match = find_in_text_map(text_map, text, local_occ) + if match is None: + break + if any(id(pos.node) == elem_id for pos in match.positions): + return match + 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. + """ + current_occurrence = 0 + for paragraph in self.editor.dom.getElementsByTagName("w:p"): + text_map = build_text_map(paragraph) + local_occ = 0 + while True: + match = find_in_text_map(text_map, text, local_occ) + if match is None: + break + if current_occurrence == occurrence: + return match + current_occurrence += 1 + local_occ += 1 + return None + def replace_text(self, find: str, replace_with: str, occurrence: int = 0) -> int: """Replace text with tracked changes (deletion + insertion). @@ -97,7 +146,11 @@ def replace_text(self, find: str, replace_with: str, occurrence: int = 0) -> int try: elem = self._get_nth_match(find, occurrence) except TextNotFoundError: - raise + # Fall back to cross-boundary search + match = self._find_across_boundaries(find, occurrence) + if match is None: + raise + return self._replace_across_nodes(match, replace_with) # Get the parent run run = elem.parentNode @@ -114,10 +167,23 @@ def replace_text(self, find: str, replace_with: str, occurrence: int = 0) -> int if start_idx == -1: raise TextNotFoundError(f"Text not found: '{find}'") + # If run has multiple w:t children, delegate to cross-boundary path + # to avoid losing sibling w:t nodes when replacing the whole run. + if len(run.getElementsByTagName("w:t")) > 1: + match = self._find_match_containing_node(find, elem) + if match is not None: + return self._replace_across_nodes(match, replace_with) + # Build replacement XML before_text = full_text[:start_idx] after_text = full_text[start_idx + len(find) :] + # Site A: If inside , edit text in-place (no del/ins wrappers) + if self._find_ancestor(run, "w:ins"): + elem.firstChild.data = before_text + replace_with + after_text + _set_xml_space_preserve(elem) + return -1 + # Preserve run properties if present rPr_xml = "" rPr_elems = run.getElementsByTagName("w:rPr") @@ -169,7 +235,11 @@ def suggest_deletion(self, text: str, occurrence: int = 0) -> int: try: elem = self._get_nth_match(text, occurrence) except TextNotFoundError: - raise + # Fall back to cross-boundary search + match = self._find_across_boundaries(text, occurrence) + if match is None: + raise + return self._delete_across_nodes(match) # Get the parent run run = elem.parentNode @@ -186,6 +256,12 @@ def suggest_deletion(self, text: str, occurrence: int = 0) -> int: if start_idx == -1: raise TextNotFoundError(f"Text not found: '{text}'") + # If run has multiple w:t children, delegate to cross-boundary path + if len(run.getElementsByTagName("w:t")) > 1: + match = self._find_match_containing_node(text, elem) + if match is not None: + return self._delete_across_nodes(match) + # Preserve run properties if present rPr_xml = "" rPr_elems = run.getElementsByTagName("w:rPr") @@ -195,6 +271,24 @@ def suggest_deletion(self, text: str, occurrence: int = 0) -> int: before_text = full_text[:start_idx] after_text = full_text[start_idx + len(text) :] + # Site B: If inside , shrink/remove the insertion (no ) + ins_ancestor = self._find_ancestor(run, "w:ins") + if ins_ancestor: + remaining = before_text + after_text + if remaining: + elem.firstChild.data = remaining + _set_xml_space_preserve(elem) + else: + # Entire w:t text removed β€” check if other w:t nodes exist + if len(self._get_wt_nodes_in_ancestor(ins_ancestor)) == 1: + # Sole w:t β€” remove entire + if ins_ancestor.parentNode: + ins_ancestor.parentNode.removeChild(ins_ancestor) + else: + # Other w:t nodes exist β€” remove just this w:t (and run if empty) + self._remove_wt_and_maybe_run(elem) + return -1 + # Build the replacement runs xml_parts = [] @@ -220,6 +314,599 @@ def suggest_deletion(self, text: str, occurrence: int = 0) -> int: return -1 + def _get_run_info(self, node) -> tuple[Element | None, str]: + """Get the parent w:r element and its rPr XML for a w:t node.""" + run = node.parentNode + while run and run.nodeName != "w:r": + run = run.parentNode + if not run: + return None, "" + rPr_xml = "" + rPr_elems = run.getElementsByTagName("w:rPr") + if rPr_elems: + rPr_xml = rPr_elems[0].toxml() + return run, rPr_xml + + def _get_non_text_children_xml(self, run) -> str: + """Serialize non-text, non-rPr children (w:tab, w:br, w:drawing, etc.) of a run.""" + parts = [] + for child in run.childNodes: + if child.nodeType == child.ELEMENT_NODE: + tag = getattr(child, "tagName", "") + if tag not in ("w:t", "w:rPr"): + parts.append(child.toxml()) + return "".join(parts) + + def _get_node_text(self, node) -> str: + """Get text content of a w:t node.""" + text = "" + for child in node.childNodes: + if child.nodeType == child.TEXT_NODE: + text += child.data + return text + + def _build_cross_boundary_parts(self, match: TextMapMatch) -> list[tuple[Element, str, str, str, str, int]]: + """Build per-node data for a cross-boundary match. + + Returns list of (run, rPr_xml, before_text, matched_part, after_text, node_id) tuples, + one per unique w:t node involved in the match. Nodes are in document order. + """ + # Group positions by their w:t node (not run β€” a run can have multiple w:t nodes) + node_data = OrderedDict() + for pos in match.positions: + run, rPr_xml = self._get_run_info(pos.node) + if run is None: + continue + nid = id(pos.node) + if nid not in node_data: + node_data[nid] = { + "run": run, + "rPr_xml": rPr_xml, + "node": pos.node, + "first_offset": pos.offset_in_node, + "last_offset": pos.offset_in_node, + } + else: + node_data[nid]["last_offset"] = pos.offset_in_node + + result = [] + for nid, info in node_data.items(): + node_text = self._get_node_text(info["node"]) + first = info["first_offset"] + last = info["last_offset"] + before = node_text[:first] + matched = node_text[first : last + 1] + after = node_text[last + 1 :] + result.append((info["run"], info["rPr_xml"], before, matched, after, nid)) + return result + + def _classify_segments(self, match: TextMapMatch) -> list[tuple[bool | None, list[TextPosition]]]: + """Group match positions into contiguous segments by revision context. + + Returns list of (is_inside_ins, positions_list) tuples. + """ + segments = [] + current_ins = None + current_positions = [] + for pos in match.positions: + if pos.is_inside_ins != current_ins: + if current_positions: + segments.append((current_ins, current_positions)) + current_ins = pos.is_inside_ins + current_positions = [pos] + else: + current_positions.append(pos) + if current_positions: + segments.append((current_ins, current_positions)) + return segments + + def _replace_across_nodes(self, match: TextMapMatch, replace_with: str) -> int: + """Replace text spanning multiple w:t elements, handling mixed revision contexts.""" + if match.spans_boundary: + return self._replace_mixed_state(match, replace_with) + return self._replace_same_context(match, replace_with) + + def _replace_same_context(self, match: TextMapMatch, replace_with: str) -> int: + """Replace text spanning multiple runs in the same revision context. + + Groups the match by parent run, then for each run: + - Keeps text before the match as an unchanged run + - Puts matched text into w:del + - Keeps text after the match as an unchanged run + - Inserts w:ins with replacement text after the last deletion + """ + parts = self._build_cross_boundary_parts(match) + if not parts: + return -1 + + # Site D: If all positions inside , edit in-place + if all(p.is_inside_ins for p in match.positions): + first_node = match.positions[0].node + ins_elem = self._find_ancestor(first_node, "w:ins") + # Save parent/next sibling before removal may detach ins_elem + ins_parent = ins_elem.parentNode if ins_elem else None + ins_next = ins_elem.nextSibling if ins_elem else None + + self._remove_from_insertion(match.positions) + + first_rPr = parts[0][1] + new_run_xml = f"{first_rPr}{_escape_xml(replace_with)}" + + if ins_elem and ins_elem.parentNode: + # ins_elem still in DOM β€” insert replacement inside it + first_child = ins_elem.firstChild + if first_child: + self.editor.insert_before(first_child, new_run_xml) + else: + self.editor.append_to(ins_elem, new_run_xml) + elif ins_parent: + # ins_elem was fully removed β€” wrap replacement in a new + ins_wrapper_xml = f"{new_run_xml}" + if ins_next: + self.editor.insert_before(ins_next, ins_wrapper_xml) + else: + self.editor.append_to(ins_parent, ins_wrapper_xml) + return -1 + + # Use first run's rPr for the insertion + first_rPr = parts[0][1] + + # Group parts by run for multi-w:t preservation + run_order: list[int] = [] + run_map: dict[int, dict] = {} + for run, rPr_xml, before, matched, after, nid in parts: + rid = id(run) + if rid not in run_map: + run_order.append(rid) + run_map[rid] = {"run": run, "rPr_xml": rPr_xml, "parts": []} + run_map[rid]["parts"].append((before, matched, after, nid)) + + xml_parts = [] + part_idx = 0 + total_parts = len(parts) + for rid in run_order: + info = run_map[rid] + run = info["run"] + rPr_xml = info["rPr_xml"] + + # Build deterministic node-to-part mapping using node ids from parts + node_to_part = {nid: (before, matched, after) for before, matched, after, nid in info["parts"]} + + # Preserve non-text children (w:tab, w:br, w:drawing, etc.) + non_text_xml = self._get_non_text_children_xml(run) + if non_text_xml: + xml_parts.append(f"{rPr_xml}{non_text_xml}") + + # Iterate all w:t in document order, emitting unmatched as preserved + all_wt = list(run.getElementsByTagName("w:t")) + parts_emitted = 0 + for wt in all_wt: + if id(wt) in node_to_part: + before, matched, after = node_to_part[id(wt)] + parts_emitted += 1 + + if before: + xml_parts.append(f"{rPr_xml}{_escape_xml(before)}") + xml_parts.append( + f"{rPr_xml}{_escape_xml(matched)}" + ) + + # Insert replacement after the last deletion + if part_idx + parts_emitted == total_parts: + xml_parts.append(f"{first_rPr}{_escape_xml(replace_with)}") + + if after: + xml_parts.append(f"{rPr_xml}{_escape_xml(after)}") + else: + # Unmatched sibling β€” preserve + wt_text = self._get_node_text(wt) + if wt_text: + xml_parts.append(f"{rPr_xml}{_escape_xml(wt_text)}") + + part_idx += len(node_to_part) + + # Replace all affected runs: insert new XML before first run, remove all runs + first_run = parts[0][0] + new_xml = "".join(xml_parts) + nodes = self.editor.insert_before(first_run, new_xml) + + seen = set() + for run, _, _, _, _, _ in parts: + if id(run) in seen: + continue + seen.add(id(run)) + parent = run.parentNode + if parent: + parent.removeChild(run) + + # Find insertion node ID + for node in nodes: + if node.nodeType == node.ELEMENT_NODE and node.tagName == "w:ins": + return int(node.getAttribute("w:id")) + + return -1 + + def _replace_mixed_state(self, match: TextMapMatch, replace_with: str) -> int: + """Replace text spanning revision boundaries via atomic decomposition. + + For each segment: + - Regular text: wrap in (standard deletion) + - Inside : remove the matched portion (undo partial insertion) + + Then insert new text as . + """ + segments = self._classify_segments(match) + + # Get rPr from first position's run for the new insertion + first_run, first_rPr = self._get_run_info(match.positions[0].node) + + # Find the first affected element to use as insertion reference point. + # For regular text, it's the run; for ins text, it's the w:ins element. + first_pos = match.positions[0] + if first_pos.is_inside_ins: + ref_node = self._find_ancestor(first_pos.node, "w:ins") + else: + ref_node = first_run + + if ref_node is None: + return -1 + + # Place a marker before ref_node so we can find the insertion point + # after deletion processing (which may remove ref_node). + marker = self.editor.dom.createComment("replace-marker") + ref_node.parentNode.insertBefore(marker, ref_node) # type: ignore[union-attr] + + # Process each segment to delete/remove the matched text + for is_inside_ins, positions in segments: + if is_inside_ins: + self._remove_from_insertion(positions) + else: + self._delete_regular_segment(positions) + + # Insert replacement after the last sibling following the marker, + # so it appears after any preserved prefix text. + ins_xml = f"{first_rPr}{_escape_xml(replace_with)}" + last_del = None + sibling = marker.nextSibling + while sibling: + if sibling.nodeType == sibling.ELEMENT_NODE: + if sibling.tagName == "w:del": + last_del = sibling + elif last_del is not None: + # Stop at first non-del element after we found a del + break + sibling = sibling.nextSibling + + if last_del: + new_nodes = self.editor.insert_after(last_del, ins_xml) + else: + # No deletions found β€” insert after marker + new_nodes = self.editor.insert_after(marker, ins_xml) + + # Remove marker + if marker.parentNode: + marker.parentNode.removeChild(marker) + + # Return the change ID of the new insertion + for node in new_nodes: + if node.nodeType == node.ELEMENT_NODE and node.tagName == "w:ins": + return int(node.getAttribute("w:id")) + return -1 + + def _find_ancestor(self, node, tag_name: str) -> Element | None: + """Find the nearest ancestor with the given tag name.""" + parent = node.parentNode + while parent: + if parent.nodeType == parent.ELEMENT_NODE and parent.tagName == tag_name: + return parent + parent = parent.parentNode + return None + + def _remove_from_insertion(self, positions: list) -> None: + """Remove matched text from inside a element. + + Handles segments spanning multiple w:t nodes within the insertion. + If the entire insertion text is matched, removes the element. + If partial, truncates or splits. + """ + # Group positions by w:t node to handle multi-node segments + node_groups = OrderedDict() + for pos in positions: + nid = id(pos.node) + if nid not in node_groups: + node_groups[nid] = {"node": pos.node, "first": pos.offset_in_node, "last": pos.offset_in_node} + else: + node_groups[nid]["last"] = pos.offset_in_node + + groups = list(node_groups.values()) + first_group = groups[0] + last_group = groups[-1] + + first_node = first_group["node"] + last_node = last_group["node"] + first_offset = first_group["first"] + last_offset = last_group["last"] + + before = self._get_node_text(first_node)[:first_offset] + after = self._get_node_text(last_node)[last_offset + 1 :] + + ins_elem = self._find_ancestor(first_node, "w:ins") + + if not before and not after and len(groups) == len(self._get_wt_nodes_in_ancestor(ins_elem)): + # Entire insertion matched -- remove the element + if ins_elem and ins_elem.parentNode: + ins_elem.parentNode.removeChild(ins_elem) + elif len(groups) == 1 and first_node is last_node: + # Single node β€” use simple truncate/split logic + node_text = self._get_node_text(first_node) + before_text = node_text[:first_offset] + after_text = node_text[last_offset + 1 :] + + if not before_text and not after_text: + # Entire single node matched + if ins_elem and ins_elem.parentNode: + if len(self._get_wt_nodes_in_ancestor(ins_elem)) == 1: + # Sole w:t β€” remove entire + ins_elem.parentNode.removeChild(ins_elem) + else: + # Other w:t nodes exist β€” remove just this w:t (and run if empty) + self._remove_wt_and_maybe_run(first_node) + elif not before_text: + first_node.firstChild.data = after_text + elif not after_text: + first_node.firstChild.data = before_text + else: + # Middle split + first_node.firstChild.data = before_text + run = self._find_ancestor(first_node, "w:r") + if ins_elem and run: + rPr_xml = "" + rPr_elems = run.getElementsByTagName("w:rPr") + if rPr_elems: + rPr_xml = rPr_elems[0].toxml() + after_xml = f"{rPr_xml}{_escape_xml(after_text)}" + self.editor.insert_after(ins_elem, after_xml) + else: + # Multi-node: truncate first node to before, last node to after, + # remove intermediate nodes entirely. + # Only remove the w:t node; remove the run only if no w:t children remain. + if before: + first_node.firstChild.data = before + _set_xml_space_preserve(first_node) + else: + self._remove_wt_and_maybe_run(first_node) + + if after: + last_node.firstChild.data = after + _set_xml_space_preserve(last_node) + else: + self._remove_wt_and_maybe_run(last_node) + + # Remove intermediate nodes unconditionally (entire text is matched) + for group in groups[1:-1]: + self._remove_wt_and_maybe_run(group["node"]) + + def _remove_wt_and_maybe_run(self, wt_node) -> None: + """Remove a w:t node, and its parent w:r if no meaningful children remain. + + Preserves the run if it still contains non-text content children + like w:tab, w:br, w:drawing, etc. + """ + run = self._find_ancestor(wt_node, "w:r") + if wt_node.parentNode: + wt_node.parentNode.removeChild(wt_node) + if run and not run.getElementsByTagName("w:t") and run.parentNode: + has_content_children = any( + n.nodeType == n.ELEMENT_NODE and getattr(n, "tagName", None) not in ("w:t", "w:rPr") + for n in run.childNodes + ) + if not has_content_children: + run.parentNode.removeChild(run) + + def _get_wt_nodes_in_ancestor(self, ancestor) -> list: + """Get all w:t nodes inside an ancestor element.""" + if ancestor is None: + return [] + return ancestor.getElementsByTagName("w:t") + + def _delete_regular_segment(self, positions: list) -> int: + """Delete regular (non-insertion) text by wrapping in . + + Groups positions by run first, then by w:t node within each run, + so that each run is removed exactly once even when it contains + multiple w:t nodes involved in the match. + + Returns the w:id of the first element created, or -1. + """ + # Group positions by run, then by node within each run + run_groups: OrderedDict[int, dict] = OrderedDict() + for pos in positions: + run, rPr_xml = self._get_run_info(pos.node) + if not run: + continue + rid = id(run) + if rid not in run_groups: + run_groups[rid] = {"run": run, "rPr_xml": rPr_xml, "nodes": OrderedDict()} + nid = id(pos.node) + node_map = run_groups[rid]["nodes"] + if nid not in node_map: + node_map[nid] = {"node": pos.node, "first": pos.offset_in_node, "last": pos.offset_in_node} + else: + node_map[nid]["last"] = pos.offset_in_node + + # Flatten to a list of (run_info, node_group) for global indexing + all_node_groups: list[tuple[dict, dict]] = [] + for run_info in run_groups.values(): + for ng in run_info["nodes"].values(): + all_node_groups.append((run_info, ng)) + + total = len(all_node_groups) + first_del_id = -1 + processed_runs: set[int] = set() + + for _global_idx, (run_info, _) in enumerate(all_node_groups): + run = run_info["run"] + rPr_xml = run_info["rPr_xml"] + rid = id(run) + + if rid in processed_runs: + continue + + # Build xml_parts for ALL w:t nodes in this run, preserving unmatched ones + matched_node_ids = {id(ng["node"]) for ng in run_info["nodes"].values()} + node_items = list(run_info["nodes"].values()) + all_wt_nodes = list(run.getElementsByTagName("w:t")) + xml_parts: list[str] = [] + + # Preserve non-text children (w:tab, w:br, w:drawing, etc.) + non_text_xml = self._get_non_text_children_xml(run) + if non_text_xml: + xml_parts.append(f"{rPr_xml}{non_text_xml}") + + for wt_node in all_wt_nodes: + if id(wt_node) not in matched_node_ids: + # Unmatched sibling β€” preserve as-is + wt_text = self._get_node_text(wt_node) + if wt_text: + xml_parts.append(f"{rPr_xml}{_escape_xml(wt_text)}") + continue + + ng = run_info["nodes"][id(wt_node)] + node_text = self._get_node_text(ng["node"]) + first_offset = ng["first"] + last_offset = ng["last"] + + # Determine this node group's position in the global sequence + run_keys = list(run_groups.keys()) + local_idx = node_items.index(ng) + preceding_nodes = sum(len(run_groups[k]["nodes"]) for k in run_keys[: run_keys.index(rid)]) + global_pos = preceding_nodes + local_idx + is_first_overall = global_pos == 0 + is_last_overall = global_pos == total - 1 + + before = node_text[:first_offset] if is_first_overall else "" + after = node_text[last_offset + 1 :] if is_last_overall else "" + + # For intermediate nodes, the entire text is matched + if not is_first_overall and not is_last_overall: + matched = node_text + else: + matched = node_text[first_offset : last_offset + 1] + + if before: + xml_parts.append(f"{rPr_xml}{_escape_xml(before)}") + xml_parts.append(f"{rPr_xml}{_escape_xml(matched)}") + if after: + xml_parts.append(f"{rPr_xml}{_escape_xml(after)}") + + new_xml = "".join(xml_parts) + nodes = self.editor.insert_before(run, new_xml) + if run.parentNode: + run.parentNode.removeChild(run) + processed_runs.add(rid) + + if first_del_id == -1: + for n in nodes: + if n.nodeType == n.ELEMENT_NODE and n.tagName == "w:del": + first_del_id = int(n.getAttribute("w:id")) + break + + return first_del_id + + def _delete_across_nodes(self, match: TextMapMatch) -> int: + """Delete text spanning multiple w:t elements.""" + if match.spans_boundary: + return self._delete_mixed_state(match) + return self._delete_same_context(match) + + def _delete_same_context(self, match: TextMapMatch) -> int: + """Delete text spanning multiple runs in the same revision context.""" + parts = self._build_cross_boundary_parts(match) + if not parts: + return -1 + + # Site F: If all positions inside , remove from insertion directly + if all(p.is_inside_ins for p in match.positions): + self._remove_from_insertion(match.positions) + return -1 + + # Group parts by run, using node ids from _build_cross_boundary_parts + matched_node_ids = {nid for _, _, _, _, _, nid in parts} + run_parts: OrderedDict[int, list] = OrderedDict() + for part in parts: + rid = id(part[0]) + if rid not in run_parts: + run_parts[rid] = [] + run_parts[rid].append(part) + + xml_parts = [] + for _rid, rparts in run_parts.items(): + run = rparts[0][0] + rPr_xml = rparts[0][1] + + # Build deterministic node-to-part mapping using node ids from parts + node_to_part = {nid: (rp_xml, before, matched, after) for _, rp_xml, before, matched, after, nid in rparts} + + # Preserve non-text children (w:tab, w:br, w:drawing, etc.) + non_text_xml = self._get_non_text_children_xml(run) + if non_text_xml: + xml_parts.append(f"{rPr_xml}{non_text_xml}") + + # Emit w:t nodes in document order, matched as , unmatched preserved + all_wt = list(run.getElementsByTagName("w:t")) + for wt in all_wt: + if id(wt) in node_to_part: + rp_xml, before, matched, after = node_to_part[id(wt)] + if before: + xml_parts.append(f"{rp_xml}{_escape_xml(before)}") + xml_parts.append(f"{rp_xml}{_escape_xml(matched)}") + if after: + xml_parts.append(f"{rp_xml}{_escape_xml(after)}") + elif id(wt) not in matched_node_ids: + # Unmatched sibling β€” preserve + wt_text = self._get_node_text(wt) + if wt_text: + xml_parts.append(f"{rPr_xml}{_escape_xml(wt_text)}") + + first_run = parts[0][0] + new_xml = "".join(xml_parts) + nodes = self.editor.insert_before(first_run, new_xml) + + seen = set() + for run, _, _, _, _, _ in parts: + if id(run) in seen: + continue + seen.add(id(run)) + parent = run.parentNode + if parent: + parent.removeChild(run) + + # Find deletion node ID + for node in nodes: + if node.nodeType == node.ELEMENT_NODE and node.tagName == "w:del": + return int(node.getAttribute("w:id")) + + return -1 + + def _delete_mixed_state(self, match: TextMapMatch) -> int: + """Delete text spanning revision boundaries. + + Regular text segments are wrapped in . + Insertion text segments are removed (undoing partial insertion). + """ + segments = self._classify_segments(match) + + first_del_id = -1 + for is_inside_ins, positions in segments: + if is_inside_ins: + self._remove_from_insertion(positions) + else: + del_id = self._delete_regular_segment(positions) + if first_del_id == -1: + first_del_id = del_id + + return first_del_id + def insert_text_after(self, anchor: str, text: str, occurrence: int = 0) -> int: """Insert text after anchor with tracked changes. @@ -258,7 +945,11 @@ def _insert_text(self, anchor: str, text: str, position: Literal["before", "afte try: elem = self._get_nth_match(anchor, occurrence) except TextNotFoundError: - raise TextNotFoundError(f"Anchor text not found: '{anchor}'") from None + # Fall back to cross-boundary search + match = self._find_across_boundaries(anchor, occurrence) + if match is None: + raise TextNotFoundError(f"Anchor text not found: '{anchor}'") from None + return self._insert_near_match(match, text, position) # Get the parent run run = elem.parentNode @@ -274,14 +965,48 @@ def _insert_text(self, anchor: str, text: str, position: Literal["before", "afte if rPr_elems: rPr_xml = rPr_elems[0].toxml() - # Create insertion XML + # Site C: If inside , edit text in-place (safe for multi-w:t) + full_text = elem.firstChild.data if elem.firstChild else "" + anchor_idx = full_text.find(anchor) + + if anchor_idx == -1: + raise TextNotFoundError(f"Anchor text not found: '{anchor}'") + + before_text = full_text[:anchor_idx] + after_text = full_text[anchor_idx + len(anchor) :] + + if self._find_ancestor(run, "w:ins"): + if position == "before": + elem.firstChild.data = before_text + text + anchor + after_text + else: + elem.firstChild.data = before_text + anchor + text + after_text + _set_xml_space_preserve(elem) + return -1 + + # Build split runs + insertion, preserving sibling w:t nodes ins_xml = f"{rPr_xml}{_escape_xml(text)}" + xml_parts = [] + for wt in run.getElementsByTagName("w:t"): + if wt is elem: + # Split this w:t around the anchor + if before_text: + xml_parts.append(f"{rPr_xml}{_escape_xml(before_text)}") + if position == "before": + xml_parts.append(ins_xml) + xml_parts.append(f"{rPr_xml}{_escape_xml(anchor)}") + else: + xml_parts.append(f"{rPr_xml}{_escape_xml(anchor)}") + xml_parts.append(ins_xml) + if after_text: + xml_parts.append(f"{rPr_xml}{_escape_xml(after_text)}") + else: + # Preserve unmatched sibling w:t + wt_text = self._get_node_text(wt) + if wt_text: + xml_parts.append(f"{rPr_xml}{_escape_xml(wt_text)}") - # Insert before or after the run - if position == "after": - nodes = self.editor.insert_after(run, ins_xml) - else: - nodes = self.editor.insert_before(run, ins_xml) + new_xml = "".join(xml_parts) + nodes = self.editor.replace_node(run, new_xml) # Find the insertion node to get its ID for node in nodes: @@ -290,6 +1015,52 @@ def _insert_text(self, anchor: str, text: str, position: Literal["before", "afte return -1 + def _insert_near_match(self, match: TextMapMatch, text: str, position: str) -> int: + """Insert text before/after a cross-boundary match.""" + positions = match.positions + if not positions: + return -1 + + # Get rPr from first run + first_run, rPr_xml = self._get_run_info(positions[0].node) + + # Sites H/I: If ref run is inside , insert bare (no wrapper) + if position == "after": + last_run, _ = self._get_run_info(positions[-1].node) + if not last_run: + return -1 + ref_run = last_run + else: + if not first_run: + return -1 + ref_run = first_run + + all_inside_ins = all(p.is_inside_ins for p in positions) + if all_inside_ins and self._find_ancestor(ref_run, "w:ins"): + bare_xml = f"{rPr_xml}{_escape_xml(text)}" + if position == "after": + self.editor.insert_after(ref_run, bare_xml) + else: + self.editor.insert_before(ref_run, bare_xml) + return -1 + + ins_xml = f"{rPr_xml}{_escape_xml(text)}" + + # If ref_run is inside a , insert relative to the ancestor + # to avoid nesting inside . + ins_ancestor = self._find_ancestor(ref_run, "w:ins") + insert_ref = ins_ancestor if ins_ancestor else ref_run + + if position == "after": + nodes = self.editor.insert_after(insert_ref, ins_xml) + else: + nodes = self.editor.insert_before(insert_ref, ins_xml) + + for node in nodes: + if node.nodeType == node.ELEMENT_NODE and node.tagName == "w:ins": + return int(node.getAttribute("w:id")) + return -1 + def list_revisions(self, author: str | None = None) -> list[Revision]: """List all tracked changes in the document. @@ -474,6 +1245,11 @@ def _restore_deletion(self, del_elem) -> None: self._unwrap_element(del_elem) +def _set_xml_space_preserve(wt_elem) -> None: + """Set xml:space='preserve' on a w:t element to preserve whitespace.""" + wt_elem.setAttribute("xml:space", "preserve") + + def _escape_xml(text: str) -> str: """Escape text for safe XML inclusion.""" return ( diff --git a/docx_editor/xml_editor.py b/docx_editor/xml_editor.py index aab4f11..848a579 100644 --- a/docx_editor/xml_editor.py +++ b/docx_editor/xml_editor.py @@ -6,6 +6,7 @@ import html import random +from dataclasses import dataclass from datetime import datetime, timezone from pathlib import Path @@ -15,6 +16,124 @@ from .exceptions import MultipleNodesFoundError, NodeNotFoundError +@dataclass +class TextPosition: + """A single character's position in the source XML.""" + + node: object # The element + offset_in_node: int # Character offset within the node's text + is_inside_ins: bool # Inside ? + is_inside_del: bool # Inside ? + + +@dataclass +class TextMap: + """Maps visible text to source XML nodes.""" + + text: str # Concatenated visible text + positions: list[TextPosition] # One per character in `text` + + def find(self, search: str, start: int = 0) -> int: + """Find text in the visible text string. Returns index or -1.""" + return self.text.find(search, start) + + def get_nodes_for_range(self, start: int, end: int) -> list[TextPosition]: + """Get TextPosition entries for a character range.""" + return self.positions[start:end] + + +@dataclass +class TextMapMatch: + """A match found in the text map.""" + + start: int # Start index in visible text + end: int # End index in visible text + text: str # The matched text + positions: list[TextPosition] # TextPosition entries for the match + spans_boundary: bool # True if match spans different revision contexts + + +def find_in_text_map(text_map: TextMap, search: str, occurrence: int = 0) -> TextMapMatch | None: + """Find the nth occurrence of text in a text map. + + Returns TextMapMatch or None if not found. + """ + start = 0 + for i in range(occurrence + 1): + idx = text_map.find(search, start) + if idx == -1: + return None + if i < occurrence: + start = idx + 1 + + end = idx + len(search) + positions = text_map.get_nodes_for_range(idx, end) + + # Check if match spans different revision contexts + if positions: + first_ins = positions[0].is_inside_ins + spans = any(p.is_inside_ins != first_ins for p in positions) + else: + spans = False + + return TextMapMatch( + start=idx, + end=end, + text=search, + positions=positions, + spans_boundary=spans, + ) + + +def _is_inside_element(node, tag_name: str) -> bool: + """Check if a node is inside an element with the given tag.""" + parent = node.parentNode + while parent: + if parent.nodeType == parent.ELEMENT_NODE and parent.tagName == tag_name: + return True + parent = parent.parentNode + return False + + +def build_text_map(paragraph) -> TextMap: + """Build a text map for a paragraph element. + + Walks all elements in the paragraph, concatenating visible text + (excluding ) and recording the source node and offset for + each character position. + + Text inside is included (it's visible) with is_inside_ins=True. + Text inside / is excluded from visible text. + """ + text_chars: list[str] = [] + positions: list[TextPosition] = [] + + for node in paragraph.getElementsByTagName("w:t"): + # Skip w:t inside w:del (deleted text uses w:delText, but be safe) + if _is_inside_element(node, "w:del"): + continue + + inside_ins = _is_inside_element(node, "w:ins") + # Get text content from the text node + node_text = "" + for child in node.childNodes: + if child.nodeType == child.TEXT_NODE: + node_text += child.data + + for i, char in enumerate(node_text): + text_chars.append(char) + positions.append( + TextPosition( + node=node, + offset_in_node=i, + is_inside_ins=inside_ins, + is_inside_del=False, + ) + ) + + return TextMap(text="".join(text_chars), positions=positions) + + class XMLEditor: """Editor for manipulating OOXML XML files with line-number-based node finding. diff --git a/openspec/AGENTS.md b/openspec/AGENTS.md new file mode 100644 index 0000000..fb307c7 --- /dev/null +++ b/openspec/AGENTS.md @@ -0,0 +1,456 @@ +# OpenSpec Instructions + +Instructions for AI coding assistants using OpenSpec for spec-driven development. + +## TL;DR Quick Checklist + +- Search existing work: `openspec spec list --long`, `openspec list` (use `rg` only for full-text search) +- Decide scope: new capability vs modify existing capability +- Pick a unique `change-id`: kebab-case, verb-led (`add-`, `update-`, `remove-`, `refactor-`) +- Scaffold: `proposal.md`, `tasks.md`, `design.md` (only if needed), and delta specs per affected capability +- Write deltas: use `## ADDED|MODIFIED|REMOVED|RENAMED Requirements`; include at least one `#### Scenario:` per requirement +- Validate: `openspec validate [change-id] --strict --no-interactive` and fix issues +- Request approval: Do not start implementation until proposal is approved + +## Three-Stage Workflow + +### Stage 1: Creating Changes +Create proposal when you need to: +- Add features or functionality +- Make breaking changes (API, schema) +- Change architecture or patterns +- Optimize performance (changes behavior) +- Update security patterns + +Triggers (examples): +- "Help me create a change proposal" +- "Help me plan a change" +- "Help me create a proposal" +- "I want to create a spec proposal" +- "I want to create a spec" + +Loose matching guidance: +- Contains one of: `proposal`, `change`, `spec` +- With one of: `create`, `plan`, `make`, `start`, `help` + +Skip proposal for: +- Bug fixes (restore intended behavior) +- Typos, formatting, comments +- Dependency updates (non-breaking) +- Configuration changes +- Tests for existing behavior + +**Workflow** +1. Review `openspec/project.md`, `openspec list`, and `openspec list --specs` to understand current context. +2. Choose a unique verb-led `change-id` and scaffold `proposal.md`, `tasks.md`, optional `design.md`, and spec deltas under `openspec/changes//`. +3. Draft spec deltas using `## ADDED|MODIFIED|REMOVED Requirements` with at least one `#### Scenario:` per requirement. +4. Run `openspec validate --strict --no-interactive` and resolve any issues before sharing the proposal. + +### Stage 2: Implementing Changes +Track these steps as TODOs and complete them one by one. +1. **Read proposal.md** - Understand what's being built +2. **Read design.md** (if exists) - Review technical decisions +3. **Read tasks.md** - Get implementation checklist +4. **Implement tasks sequentially** - Complete in order +5. **Confirm completion** - Ensure every item in `tasks.md` is finished before updating statuses +6. **Update checklist** - After all work is done, set every task to `- [x]` so the list reflects reality +7. **Approval gate** - Do not start implementation until the proposal is reviewed and approved + +### Stage 3: Archiving Changes +After deployment, create separate PR to: +- Move `changes/[name]/` β†’ `changes/archive/YYYY-MM-DD-[name]/` +- Update `specs/` if capabilities changed +- Use `openspec archive --skip-specs --yes` for tooling-only changes (always pass the change ID explicitly) +- Run `openspec validate --strict --no-interactive` to confirm the archived change passes checks + +## Before Any Task + +**Context Checklist:** +- [ ] Read relevant specs in `specs/[capability]/spec.md` +- [ ] Check pending changes in `changes/` for conflicts +- [ ] Read `openspec/project.md` for conventions +- [ ] Run `openspec list` to see active changes +- [ ] Run `openspec list --specs` to see existing capabilities + +**Before Creating Specs:** +- Always check if capability already exists +- Prefer modifying existing specs over creating duplicates +- Use `openspec show [spec]` to review current state +- If request is ambiguous, ask 1–2 clarifying questions before scaffolding + +### Search Guidance +- Enumerate specs: `openspec spec list --long` (or `--json` for scripts) +- Enumerate changes: `openspec list` (or `openspec change list --json` - deprecated but available) +- Show details: + - Spec: `openspec show --type spec` (use `--json` for filters) + - Change: `openspec show --json --deltas-only` +- Full-text search (use ripgrep): `rg -n "Requirement:|Scenario:" openspec/specs` + +## Quick Start + +### CLI Commands + +```bash +# Essential commands +openspec list # List active changes +openspec list --specs # List specifications +openspec show [item] # Display change or spec +openspec validate [item] # Validate changes or specs +openspec archive [--yes|-y] # Archive after deployment (add --yes for non-interactive runs) + +# Project management +openspec init [path] # Initialize OpenSpec +openspec update [path] # Update instruction files + +# Interactive mode +openspec show # Prompts for selection +openspec validate # Bulk validation mode + +# Debugging +openspec show [change] --json --deltas-only +openspec validate [change] --strict --no-interactive +``` + +### Command Flags + +- `--json` - Machine-readable output +- `--type change|spec` - Disambiguate items +- `--strict` - Comprehensive validation +- `--no-interactive` - Disable prompts +- `--skip-specs` - Archive without spec updates +- `--yes`/`-y` - Skip confirmation prompts (non-interactive archive) + +## Directory Structure + +``` +openspec/ +β”œβ”€β”€ project.md # Project conventions +β”œβ”€β”€ specs/ # Current truth - what IS built +β”‚ └── [capability]/ # Single focused capability +β”‚ β”œβ”€β”€ spec.md # Requirements and scenarios +β”‚ └── design.md # Technical patterns +β”œβ”€β”€ changes/ # Proposals - what SHOULD change +β”‚ β”œβ”€β”€ [change-name]/ +β”‚ β”‚ β”œβ”€β”€ proposal.md # Why, what, impact +β”‚ β”‚ β”œβ”€β”€ tasks.md # Implementation checklist +β”‚ β”‚ β”œβ”€β”€ design.md # Technical decisions (optional; see criteria) +β”‚ β”‚ └── specs/ # Delta changes +β”‚ β”‚ └── [capability]/ +β”‚ β”‚ └── spec.md # ADDED/MODIFIED/REMOVED +β”‚ └── archive/ # Completed changes +``` + +## Creating Change Proposals + +### Decision Tree + +``` +New request? +β”œβ”€ Bug fix restoring spec behavior? β†’ Fix directly +β”œβ”€ Typo/format/comment? β†’ Fix directly +β”œβ”€ New feature/capability? β†’ Create proposal +β”œβ”€ Breaking change? β†’ Create proposal +β”œβ”€ Architecture change? β†’ Create proposal +└─ Unclear? β†’ Create proposal (safer) +``` + +### Proposal Structure + +1. **Create directory:** `changes/[change-id]/` (kebab-case, verb-led, unique) + +2. **Write proposal.md:** +```markdown +# Change: [Brief description of change] + +## Why +[1-2 sentences on problem/opportunity] + +## What Changes +- [Bullet list of changes] +- [Mark breaking changes with **BREAKING**] + +## Impact +- Affected specs: [list capabilities] +- Affected code: [key files/systems] +``` + +3. **Create spec deltas:** `specs/[capability]/spec.md` +```markdown +## ADDED Requirements +### Requirement: New Feature +The system SHALL provide... + +#### Scenario: Success case +- **WHEN** user performs action +- **THEN** expected result + +## MODIFIED Requirements +### Requirement: Existing Feature +[Complete modified requirement] + +## REMOVED Requirements +### Requirement: Old Feature +**Reason**: [Why removing] +**Migration**: [How to handle] +``` +If multiple capabilities are affected, create multiple delta files under `changes/[change-id]/specs//spec.md`β€”one per capability. + +4. **Create tasks.md:** +```markdown +## 1. Implementation +- [ ] 1.1 Create database schema +- [ ] 1.2 Implement API endpoint +- [ ] 1.3 Add frontend component +- [ ] 1.4 Write tests +``` + +5. **Create design.md when needed:** +Create `design.md` if any of the following apply; otherwise omit it: +- Cross-cutting change (multiple services/modules) or a new architectural pattern +- New external dependency or significant data model changes +- Security, performance, or migration complexity +- Ambiguity that benefits from technical decisions before coding + +Minimal `design.md` skeleton: +```markdown +## Context +[Background, constraints, stakeholders] + +## Goals / Non-Goals +- Goals: [...] +- Non-Goals: [...] + +## Decisions +- Decision: [What and why] +- Alternatives considered: [Options + rationale] + +## Risks / Trade-offs +- [Risk] β†’ Mitigation + +## Migration Plan +[Steps, rollback] + +## Open Questions +- [...] +``` + +## Spec File Format + +### Critical: Scenario Formatting + +**CORRECT** (use #### headers): +```markdown +#### Scenario: User login success +- **WHEN** valid credentials provided +- **THEN** return JWT token +``` + +**WRONG** (don't use bullets or bold): +```markdown +- **Scenario: User login** ❌ +**Scenario**: User login ❌ +### Scenario: User login ❌ +``` + +Every requirement MUST have at least one scenario. + +### Requirement Wording +- Use SHALL/MUST for normative requirements (avoid should/may unless intentionally non-normative) + +### Delta Operations + +- `## ADDED Requirements` - New capabilities +- `## MODIFIED Requirements` - Changed behavior +- `## REMOVED Requirements` - Deprecated features +- `## RENAMED Requirements` - Name changes + +Headers matched with `trim(header)` - whitespace ignored. + +#### When to use ADDED vs MODIFIED +- ADDED: Introduces a new capability or sub-capability that can stand alone as a requirement. Prefer ADDED when the change is orthogonal (e.g., adding "Slash Command Configuration") rather than altering the semantics of an existing requirement. +- MODIFIED: Changes the behavior, scope, or acceptance criteria of an existing requirement. Always paste the full, updated requirement content (header + all scenarios). The archiver will replace the entire requirement with what you provide here; partial deltas will drop previous details. +- RENAMED: Use when only the name changes. If you also change behavior, use RENAMED (name) plus MODIFIED (content) referencing the new name. + +Common pitfall: Using MODIFIED to add a new concern without including the previous text. This causes loss of detail at archive time. If you aren’t explicitly changing the existing requirement, add a new requirement under ADDED instead. + +Authoring a MODIFIED requirement correctly: +1) Locate the existing requirement in `openspec/specs//spec.md`. +2) Copy the entire requirement block (from `### Requirement: ...` through its scenarios). +3) Paste it under `## MODIFIED Requirements` and edit to reflect the new behavior. +4) Ensure the header text matches exactly (whitespace-insensitive) and keep at least one `#### Scenario:`. + +Example for RENAMED: +```markdown +## RENAMED Requirements +- FROM: `### Requirement: Login` +- TO: `### Requirement: User Authentication` +``` + +## Troubleshooting + +### Common Errors + +**"Change must have at least one delta"** +- Check `changes/[name]/specs/` exists with .md files +- Verify files have operation prefixes (## ADDED Requirements) + +**"Requirement must have at least one scenario"** +- Check scenarios use `#### Scenario:` format (4 hashtags) +- Don't use bullet points or bold for scenario headers + +**Silent scenario parsing failures** +- Exact format required: `#### Scenario: Name` +- Debug with: `openspec show [change] --json --deltas-only` + +### Validation Tips + +```bash +# Always use strict mode for comprehensive checks +openspec validate [change] --strict --no-interactive + +# Debug delta parsing +openspec show [change] --json | jq '.deltas' + +# Check specific requirement +openspec show [spec] --json -r 1 +``` + +## Happy Path Script + +```bash +# 1) Explore current state +openspec spec list --long +openspec list +# Optional full-text search: +# rg -n "Requirement:|Scenario:" openspec/specs +# rg -n "^#|Requirement:" openspec/changes + +# 2) Choose change id and scaffold +CHANGE=add-two-factor-auth +mkdir -p openspec/changes/$CHANGE/{specs/auth} +printf "## Why\n...\n\n## What Changes\n- ...\n\n## Impact\n- ...\n" > openspec/changes/$CHANGE/proposal.md +printf "## 1. Implementation\n- [ ] 1.1 ...\n" > openspec/changes/$CHANGE/tasks.md + +# 3) Add deltas (example) +cat > openspec/changes/$CHANGE/specs/auth/spec.md << 'EOF' +## ADDED Requirements +### Requirement: Two-Factor Authentication +Users MUST provide a second factor during login. + +#### Scenario: OTP required +- **WHEN** valid credentials are provided +- **THEN** an OTP challenge is required +EOF + +# 4) Validate +openspec validate $CHANGE --strict --no-interactive +``` + +## Multi-Capability Example + +``` +openspec/changes/add-2fa-notify/ +β”œβ”€β”€ proposal.md +β”œβ”€β”€ tasks.md +└── specs/ + β”œβ”€β”€ auth/ + β”‚ └── spec.md # ADDED: Two-Factor Authentication + └── notifications/ + └── spec.md # ADDED: OTP email notification +``` + +auth/spec.md +```markdown +## ADDED Requirements +### Requirement: Two-Factor Authentication +... +``` + +notifications/spec.md +```markdown +## ADDED Requirements +### Requirement: OTP Email Notification +... +``` + +## Best Practices + +### Simplicity First +- Default to <100 lines of new code +- Single-file implementations until proven insufficient +- Avoid frameworks without clear justification +- Choose boring, proven patterns + +### Complexity Triggers +Only add complexity with: +- Performance data showing current solution too slow +- Concrete scale requirements (>1000 users, >100MB data) +- Multiple proven use cases requiring abstraction + +### Clear References +- Use `file.ts:42` format for code locations +- Reference specs as `specs/auth/spec.md` +- Link related changes and PRs + +### Capability Naming +- Use verb-noun: `user-auth`, `payment-capture` +- Single purpose per capability +- 10-minute understandability rule +- Split if description needs "AND" + +### Change ID Naming +- Use kebab-case, short and descriptive: `add-two-factor-auth` +- Prefer verb-led prefixes: `add-`, `update-`, `remove-`, `refactor-` +- Ensure uniqueness; if taken, append `-2`, `-3`, etc. + +## Tool Selection Guide + +| Task | Tool | Why | +|------|------|-----| +| Find files by pattern | Glob | Fast pattern matching | +| Search code content | Grep | Optimized regex search | +| Read specific files | Read | Direct file access | +| Explore unknown scope | Task | Multi-step investigation | + +## Error Recovery + +### Change Conflicts +1. Run `openspec list` to see active changes +2. Check for overlapping specs +3. Coordinate with change owners +4. Consider combining proposals + +### Validation Failures +1. Run with `--strict` flag +2. Check JSON output for details +3. Verify spec file format +4. Ensure scenarios properly formatted + +### Missing Context +1. Read project.md first +2. Check related specs +3. Review recent archives +4. Ask for clarification + +## Quick Reference + +### Stage Indicators +- `changes/` - Proposed, not yet built +- `specs/` - Built and deployed +- `archive/` - Completed changes + +### File Purposes +- `proposal.md` - Why and what +- `tasks.md` - Implementation steps +- `design.md` - Technical decisions +- `spec.md` - Requirements and behavior + +### CLI Essentials +```bash +openspec list # What's in progress? +openspec show [item] # View details +openspec validate --strict --no-interactive # Is it correct? +openspec archive [--yes|-y] # Mark complete (add --yes for automation) +``` + +Remember: Specs are truth. Changes are proposals. Keep them in sync. diff --git a/openspec/changes/archive/2026-01-29-add-cross-boundary-text-operations/design.md b/openspec/changes/archive/2026-01-29-add-cross-boundary-text-operations/design.md new file mode 100644 index 0000000..e3fbcb8 --- /dev/null +++ b/openspec/changes/archive/2026-01-29-add-cross-boundary-text-operations/design.md @@ -0,0 +1,141 @@ +# Design: Cross-Boundary Text Operations + +## Context + +The library searches for text within individual `` elements. When text spans revision boundaries (``, ``), it cannot be found because no single element contains the complete string. + +This is how Word represents a document with tracked changes: +```xml +Exploratory Aim: + + To examine whether... + +``` + +Users expect to search for "Aim: To" and find it. + +## Goals / Non-Goals + +### Goals +- Enable searching across element boundaries +- Enable replacing text spanning multiple `` elements +- Handle mixed-state editing (text spanning revision boundaries) via atomic decomposition +- Provide read-only flattened text view for analysis + +### Non-Goals +- Cross-paragraph operations (too complex for v1) +- Field codes, bookmarks, or non-text content + +## Decisions + +### Decision 1: Virtual Text Map Architecture + +**What:** Build a per-paragraph flattened text view with position mapping back to source nodes. + +**Why:** This is how Word itself handles search/replace. It's proven, incremental, and doesn't require rewriting the entire XML handling. + +**Data structures:** +```python +@dataclass +class TextPosition: + node: Element # The element + offset_in_node: int # Character offset within the node + is_inside_ins: bool # Inside ? + is_inside_del: bool # Inside ? (excluded from visible text) + +@dataclass +class TextMap: + text: str # Concatenated visible text + positions: list[TextPosition] # One per character +``` + +### Decision 2: Atomic Decomposition for Mixed-State Editing + +**What:** When a replace operation spans revision boundaries, decompose the match into segments by revision context and apply per-segment operations. + +**Alternatives considered:** +| Option | Pros | Cons | +|--------|------|------| +| A. Reject with error | Explicit, safe | Blocks valid use cases | +| B. Implicit accept | "Just works" | Destroys revision history | +| C. Atomic decomposition (chosen) | Handles mixed state, no invalid XML | More complex implementation | + +**Why C:** Users editing documents with existing tracked changes need to replace text that spans boundaries. Rejecting these operations is too limiting for real-world workflows (legal redlines, collaborative editing). Atomic decomposition handles this without producing invalid XML. + +**Algorithm:** + +Given a replace of "Aim: To" where "Aim: " is regular text and "To" is inside `To examine`: + +1. **Decompose** β€” The text map classifies the match into segments: + - Segment 1: "Aim: " β†’ regular text (Node A) + - Segment 2: "To" β†’ inserted text (Node B, inside ``) + +2. **Delete regular text** (Segment 1) β€” Standard logic: wrap "Aim: " in `` + +3. **Delete inserted text** (Segment 2) β€” Cannot wrap in `` (would create invalid `...`). Instead: + - Split the `` element: isolate "To" from " examine" + - Remove the isolated `To` node entirely (undoing that part of the insertion) + - The remaining ` examine` stays intact + +4. **Insert new text** β€” Place `Goal: ` at the split point + +**Segment types and their delete strategies:** + +| Segment type | Delete strategy | +|-------------|----------------| +| Regular text | Wrap in `` (standard) | +| Inside `` | Split insertion, remove target portion (undo partial insertion) | +| Inside `` | Skip (already deleted, not in visible text) | + +### Decision 3: Per-Paragraph Scope + +**What:** Text maps are built per paragraph (``), not document-wide. + +**Why:** +- OOXML structures content by paragraph +- Cross-paragraph edits are rare and complex +- Keeps memory bounded on large documents +- Matches Word's search behavior + +## Risks / Trade-offs + +| Risk | Mitigation | +|------|------------| +| Performance on large documents | Lazy per-paragraph map building; no pre-computation | +| Insertion splitting produces invalid XML | Test extensively with Word; validate output | +| Breaking existing behavior | New methods; existing API unchanged until validated | +| Edge cases in deeply nested revisions | Start with single-level nesting; document unsupported cases | + +## Design Constraints + +### Run Property Preservation + +When splitting runs during cross-boundary operations, each new `` fragment MUST carry the `` from its source run. The current single-element `replace_text` already does this (`track_changes.py:122-142`). The cross-boundary version must apply the same pattern per-segment β€” each segment uses its own source run's `w:rPr`. + +When a match spans runs with different formatting (e.g., bold + italic), each split fragment preserves its original formatting. The new inserted text inherits `w:rPr` from the first run in the match range. + +### Word Compatibility + +Every test that produces a modified `.docx` MUST include a round-trip validation: reopen the output file and verify it parses without errors. This catches malformed XML, missing namespaces, and invalid nesting that would cause Word to show a repair dialog. + +## Testing Strategy + +**Tests-first (TDD) approach:** Write failing tests before implementing each phase. Tests serve as the specification and guide the implementation. + +Each phase begins by writing unit tests that cover the expected behavior described in the spec scenarios. Implementation follows to make the tests pass. + +## Migration Plan + +1. **Phase 1:** Add `get_visible_text()` (read-only, no risk) +2. **Phase 2:** Add `find_text()` with boundary info (read-only, no risk) +3. **Phase 3:** Replace across multiple `` elements in same revision context +4. **Phase 4:** Mixed-state editing via atomic decomposition (spans revision boundaries) + +Rollback: Revert to per-element search if issues found. + +## Resolved Questions + +1. **`get_visible_text()` paragraph boundaries:** Yes, paragraphs are joined with `\n`. Implemented in `document.py`. +2. **Performance target:** Deferred. Lazy per-paragraph map building keeps memory bounded. Benchmarks can be added when real-world usage reveals bottlenecks. +3. **`find_all_text()`:** Deferred. The `occurrence` parameter on `find_text()` covers the common case. A dedicated `find_all_text()` can be added later if needed. +4. **Deleting text inside ``:** Remove the node (undo the insertion). This produces cleaner history and avoids invalid `` nesting inside ``. Implemented in `_remove_from_insertion()`. diff --git a/openspec/changes/archive/2026-01-29-add-cross-boundary-text-operations/proposal.md b/openspec/changes/archive/2026-01-29-add-cross-boundary-text-operations/proposal.md new file mode 100644 index 0000000..f10f822 --- /dev/null +++ b/openspec/changes/archive/2026-01-29-add-cross-boundary-text-operations/proposal.md @@ -0,0 +1,24 @@ +# Change: Add Cross-Boundary Text Operations + +## Why + +Users cannot find or edit text that spans tracked change boundaries. When a document has existing revisions (insertions/deletions), any text search that crosses these boundaries silently fails. This is the #1 user-reported limitation ([Issue #1](https://github.com/pablospe/docx-editor/issues/1)). + +**Example:** Given `"Exploratory Aim: " + "To examine"`, searching for `"Aim: To"` fails because no single `` element contains it. + +## What Changes + +1. **Add Virtual Text Map** - Build flattened text view that maps character positions back to source XML nodes +2. **Add cross-boundary search** - Find text across ``, ``, and `` boundaries +3. **Add boundary-aware replace** - Replace text spanning multiple elements with proper node splitting +4. **Add mixed-state editing** - Handle replacements spanning revision boundaries using atomic decomposition (split operations per segment type) +5. **Add `get_visible_text()` API** - Let users access flattened text for analysis + +## Impact + +- **Affected specs:** text-operations (new capability spec) +- **Affected code:** + - `docx_editor/xml_editor.py` - Add `build_text_map()` method + - `docx_editor/track_changes.py` - Update `_get_nth_match()` and `replace_text()` + - `docx_editor/document.py` - Add `get_visible_text()` public API +- **Breaking changes:** None (new functionality, existing behavior preserved) diff --git a/openspec/changes/archive/2026-01-29-add-cross-boundary-text-operations/specs/text-operations/spec.md b/openspec/changes/archive/2026-01-29-add-cross-boundary-text-operations/specs/text-operations/spec.md new file mode 100644 index 0000000..ccd0d10 --- /dev/null +++ b/openspec/changes/archive/2026-01-29-add-cross-boundary-text-operations/specs/text-operations/spec.md @@ -0,0 +1,98 @@ +## ADDED Requirements + +### Requirement: Virtual Text Map + +The system SHALL provide a flattened text view of document content that maps character positions back to their source XML elements. + +The text map SHALL: +- Concatenate all visible text from `` elements within a paragraph +- Exclude text inside `` elements (deleted content) +- Track whether each character position is inside a `` or `` element +- Map each character position to its source `` node and offset + +#### Scenario: Build text map for paragraph with tracked changes + +- **GIVEN** a paragraph containing: regular text "Hello ", inserted text "beautiful ", regular text "world" +- **WHEN** `build_text_map()` is called on the paragraph +- **THEN** the text map contains "Hello beautiful world" +- **AND** positions 0-5 map to the first `` with `is_inside_ins=False` +- **AND** positions 6-15 map to the second `` with `is_inside_ins=True` +- **AND** positions 16-20 map to the third `` with `is_inside_ins=False` + +#### Scenario: Deleted text excluded from visible text + +- **GIVEN** a paragraph containing: "Hello " and deleted text "old " and "world" +- **WHEN** `build_text_map()` is called +- **THEN** the text map contains "Hello world" (deleted text excluded) + +### Requirement: Cross-Boundary Text Search + +The system SHALL find text that spans multiple XML elements within a paragraph. + +#### Scenario: Search finds text spanning element boundary + +- **GIVEN** a paragraph with "Exploratory Aim: " in one `` and "To examine" in another +- **WHEN** searching for "Aim: To" +- **THEN** the search succeeds and returns match information +- **AND** the match indicates it spans multiple elements + +#### Scenario: Search finds text spanning insertion boundary + +- **GIVEN** a paragraph with "Hello " as regular text and "world" inside `` +- **WHEN** searching for "Hello world" +- **THEN** the search succeeds +- **AND** the match indicates it spans a revision boundary + +### Requirement: Visible Text API + +The system SHALL provide a public API to retrieve the flattened visible text of a document. + +#### Scenario: Get visible text from document + +- **GIVEN** a document with paragraphs containing mixed regular and tracked-change content +- **WHEN** `get_visible_text()` is called +- **THEN** the method returns a string containing all visible text +- **AND** deleted text is excluded +- **AND** inserted text is included + +### Requirement: Boundary-Aware Text Replacement + +The system SHALL replace text that spans multiple `` elements within the same revision context, using proper node splitting. + +#### Scenario: Replace text spanning multiple runs + +- **GIVEN** a paragraph with "Hello " in one run and "world" in another (no revisions) +- **WHEN** `replace_text("Hello world", "Hi there")` is called +- **THEN** the replacement succeeds +- **AND** the original text is wrapped in `` +- **AND** the new text is wrapped in `` + +### Requirement: Mixed-State Editing + +The system SHALL replace text that spans revision boundaries by decomposing the operation into per-segment atomic actions, one for each revision context (regular text, inside ``, inside ``). + +#### Scenario: Replace spanning regular text and insertion + +- **GIVEN** a paragraph with "Exploratory Aim: " as regular text and "To examine" inside `` +- **WHEN** `replace_text("Aim: To", "Goal: To")` is called +- **THEN** the replacement succeeds +- **AND** "Aim: " is wrapped in `` (standard deletion of regular text) +- **AND** "To" is removed from the `` element (undoing partial insertion) +- **AND** "Goal: To" is inserted as a new `` element +- **AND** the remaining " examine" stays inside the original `` + +#### Scenario: Replace fully within insertion + +- **GIVEN** a paragraph with "Hello beautiful world" entirely inside `` +- **WHEN** `replace_text("beautiful", "wonderful")` is called +- **THEN** the replacement succeeds +- **AND** "beautiful" is removed from the insertion (undoing partial insertion) +- **AND** "wonderful" is inserted as a new `` element + +#### Scenario: Insertion node is split when partially matched + +- **GIVEN** a paragraph with `To examine whether` +- **WHEN** a delete operation targets "To" (the first 2 characters) +- **THEN** the `` is split into two parts +- **AND** the `To` portion is removed +- **AND** ` examine whether` remains intact diff --git a/openspec/changes/archive/2026-01-29-add-cross-boundary-text-operations/tasks.md b/openspec/changes/archive/2026-01-29-add-cross-boundary-text-operations/tasks.md new file mode 100644 index 0000000..625c5e5 --- /dev/null +++ b/openspec/changes/archive/2026-01-29-add-cross-boundary-text-operations/tasks.md @@ -0,0 +1,48 @@ +# Tasks: Add Cross-Boundary Text Operations + +**Approach:** Tests-first (TDD). Each phase starts by writing failing tests, then implementing to make them pass. + +## 1. Core Infrastructure + +- [x] 1.1 Add unit tests for text map building (plain text, tracked changes, deleted text) +- [x] 1.2 Add `TextPosition` dataclass to `xml_editor.py` +- [x] 1.3 Add `TextMap` dataclass with `find()` and `get_nodes_for_range()` methods +- [x] 1.4 Implement `build_text_map()` in `DocxXMLEditor` + +## 2. Public API + +- [x] 2.1 Add integration tests for `get_visible_text()` +- [x] 2.2 Add `get_visible_text()` method to `Document` class + +## 3. Cross-Boundary Search + +- [x] 3.1 Add tests for searching across element and revision boundaries +- [x] 3.2 Implement `TextMapMatch` dataclass +- [x] 3.3 Implement `_find_across_boundaries()` using text maps +- [x] 3.4 Add `find_text()` method returning match info with boundary status + +## 4. Boundary-Aware Replace (Same Revision Context) + +- [x] 4.1 Add tests for replace within single element (regression) +- [x] 4.2 Add tests for replace across multiple `` elements in same context +- [x] 4.3 Implement `_replace_across_nodes()` with node splitting logic +- [x] 4.4 Update `replace_text()` to use cross-boundary search + +## 5. Mixed-State Editing (Atomic Decomposition) + +- [x] 5.1 Add tests for replace spanning regular text + `` boundary +- [x] 5.2 Add tests for replace spanning `` + regular text boundary +- [x] 5.3 Add tests for replace fully within `` (regression) +- [x] 5.4 Add tests for `` node splitting (partial match within insertion) +- [x] 5.5 Add tests for `w:rPr` preservation across split runs with different formatting +- [x] 5.6 Add tests for output validity β€” round-trip reopen modified `.docx` to verify well-formed XML +- [x] 5.7 Implement segment decomposition β€” classify match ranges by revision context (regular, inside ``, inside ``) +- [x] 5.8 Implement `` node splitting β€” isolate target text from remaining insertion +- [x] 5.9 Implement per-segment delete strategies (wrap in `` for regular text, remove node for inserted text) +- [x] 5.10 Integrate atomic decomposition into `replace_text()` flow + +## 6. Documentation + +- [x] 6.1 Update README with new capabilities +- [x] 6.2 Add docstrings to new public methods +- [x] 6.3 Add usage examples in docs/ diff --git a/openspec/project.md b/openspec/project.md new file mode 100644 index 0000000..afb410b --- /dev/null +++ b/openspec/project.md @@ -0,0 +1,97 @@ +# Project Context + +## Purpose + +A Python library for editing Microsoft Word (.docx) files with tracked changes support. Enables programmatic document manipulation while preserving revision history - essential for legal, enterprise, and collaborative workflows. + +**Key differentiator:** Native track changes support (insertions, deletions, comments) that Word can display and accept/reject. + +## Tech Stack + +- **Language:** Python 3.10+ +- **Package manager:** uv +- **XML parsing:** defusedxml (secure parsing) +- **Testing:** pytest, pytest-cov +- **Linting/Formatting:** ruff +- **Documentation:** mkdocs with material theme + +## Project Conventions + +### Code Style + +- Follow ruff defaults (based on Black formatting) +- Type hints required for public APIs +- Docstrings for public functions (Google style) +- No magic - explicit configuration only + +### Architecture Patterns + +``` +docx_editor/ +β”œβ”€β”€ document.py # Public API facade (Document class) +β”œβ”€β”€ xml_editor.py # Core XML manipulation (XMLEditor, DocxXMLEditor) +β”œβ”€β”€ track_changes.py # RevisionManager - track changes operations +β”œβ”€β”€ comments.py # CommentManager - comment operations +β”œβ”€β”€ workspace.py # Workspace management (.docx/{stem}/ folders) +β”œβ”€β”€ exceptions.py # Custom exception hierarchy +└── ooxml/ # OOXML utilities (pack/unpack) +``` + +**Key patterns:** +- Facade pattern: `Document` wraps `RevisionManager`/`CommentManager` +- Workspace pattern: Unpacks .docx to temp folder, edits in place, repacks on save +- Attribute injection: Auto-adds tracking metadata (author, date, IDs) + +### Testing Strategy + +- pytest with fixtures for test documents +- 98% code coverage target +- Test documents in `tests/fixtures/` +- Run: `uv run pytest` or `uv run pytest -n auto` for parallel + +### Git Workflow + +- Main branch: `main` +- Feature branches from `main` +- Commits without Co-Authored-By attribution +- PR-based workflow + +## Domain Context + +### OOXML Structure + +Word documents are ZIP archives containing XML files: +- `word/document.xml` - Main content +- `word/comments.xml` - Comments +- `word/people.xml` - Authors + +### Key XML Elements + +| Element | Purpose | +|---------|---------| +| `` | Paragraph | +| `` | Run (text with formatting) | +| `` | Text content | +| `` | Tracked insertion | +| `` | Tracked deletion | +| `` | Deleted text (inside ``) | +| `` | Comment anchors | + +### Revision Tracking + +- Insertions: Wrap content in `` with author/date/id +- Deletions: Convert `` to ``, wrap in `` +- Each revision gets unique `w:id` and `w:rsidR` (session ID) + +## Important Constraints + +1. **Security:** Use defusedxml for all XML parsing (prevents XXE attacks) +2. **Compatibility:** Output must open in Microsoft Word without warnings +3. **Non-destructive:** Operations must preserve existing formatting and metadata +4. **Namespace handling:** OOXML uses many namespaces (w, w14, w16du, etc.) + +## External Dependencies + +- **python-docx:** Not used (we handle OOXML directly for track changes) +- **defusedxml:** Required for secure XML parsing +- No external services or APIs diff --git a/openspec/specs/text-operations/spec.md b/openspec/specs/text-operations/spec.md new file mode 100644 index 0000000..628b8fe --- /dev/null +++ b/openspec/specs/text-operations/spec.md @@ -0,0 +1,104 @@ +# text-operations Specification + +## Purpose + +Enable text search, replacement, and deletion across `` element boundaries within a paragraph, including mixed revision contexts (regular text and tracked insertions). + +## Requirements + +### Requirement: Virtual Text Map + +The system SHALL provide a flattened text view of document content that maps character positions back to their source XML elements. + +The text map SHALL: +- Concatenate all visible text from `` elements within a paragraph +- Exclude text inside `` elements (deleted content) +- Track whether each character position is inside a `` or `` element +- Map each character position to its source `` node and offset + +#### Scenario: Build text map for paragraph with tracked changes + +- **GIVEN** a paragraph containing: regular text "Hello ", inserted text "beautiful ", regular text "world" +- **WHEN** `build_text_map()` is called on the paragraph +- **THEN** the text map contains "Hello beautiful world" +- **AND** positions 0-5 map to the first `` with `is_inside_ins=False` +- **AND** positions 6-15 map to the second `` with `is_inside_ins=True` +- **AND** positions 16-20 map to the third `` with `is_inside_ins=False` + +#### Scenario: Deleted text excluded from visible text + +- **GIVEN** a paragraph containing: "Hello " and deleted text "old " and "world" +- **WHEN** `build_text_map()` is called +- **THEN** the text map contains "Hello world" (deleted text excluded) + +### Requirement: Cross-Boundary Text Search + +The system SHALL find text that spans multiple XML elements within a paragraph. + +#### Scenario: Search finds text spanning element boundary + +- **GIVEN** a paragraph with "Exploratory Aim: " in one `` and "To examine" in another +- **WHEN** searching for "Aim: To" +- **THEN** the search succeeds and returns match information +- **AND** the match indicates it spans multiple elements + +#### Scenario: Search finds text spanning insertion boundary + +- **GIVEN** a paragraph with "Hello " as regular text and "world" inside `` +- **WHEN** searching for "Hello world" +- **THEN** the search succeeds +- **AND** the match indicates it spans a revision boundary + +### Requirement: Visible Text API + +The system SHALL provide a public API to retrieve the flattened visible text of a document. + +#### Scenario: Get visible text from document + +- **GIVEN** a document with paragraphs containing mixed regular and tracked-change content +- **WHEN** `get_visible_text()` is called +- **THEN** the method returns a string containing all visible text +- **AND** deleted text is excluded +- **AND** inserted text is included + +### Requirement: Boundary-Aware Text Replacement + +The system SHALL replace text that spans multiple `` elements within the same revision context, using proper node splitting. + +#### Scenario: Replace text spanning multiple runs + +- **GIVEN** a paragraph with "Hello " in one run and "world" in another (no revisions) +- **WHEN** `replace_text("Hello world", "Hi there")` is called +- **THEN** the replacement succeeds +- **AND** the original text is wrapped in `` +- **AND** the new text is wrapped in `` + +### Requirement: Mixed-State Editing + +The system SHALL replace text that spans revision boundaries by decomposing the operation into per-segment atomic actions, one for each revision context (regular text, inside ``, inside ``). + +#### Scenario: Replace spanning regular text and insertion + +- **GIVEN** a paragraph with "Exploratory Aim: " as regular text and "To examine" inside `` +- **WHEN** `replace_text("Aim: To", "Goal: To")` is called +- **THEN** the replacement succeeds +- **AND** "Aim: " is wrapped in `` (standard deletion of regular text) +- **AND** "To" is removed from the `` element (undoing partial insertion) +- **AND** "Goal: To" is inserted as a new `` element +- **AND** the remaining " examine" stays inside the original `` + +#### Scenario: Replace fully within insertion + +- **GIVEN** a paragraph with "Hello beautiful world" entirely inside `` +- **WHEN** `replace_text("beautiful", "wonderful")` is called +- **THEN** the replacement succeeds +- **AND** "beautiful" is removed from the insertion (undoing partial insertion) +- **AND** "wonderful" is inserted as a new `` element + +#### Scenario: Insertion node is split when partially matched + +- **GIVEN** a paragraph with `To examine whether` +- **WHEN** a delete operation targets "To" (the first 2 characters) +- **THEN** the `` is split into two parts +- **AND** the `To` portion is removed +- **AND** ` examine whether` remains intact diff --git a/tests/conftest.py b/tests/conftest.py index 7db6278..2e0dd67 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -4,8 +4,17 @@ import tempfile from pathlib import Path +import defusedxml.minidom import pytest +NS = 'xmlns:w="http://schemas.openxmlformats.org/wordprocessingml/2006/main"' + + +def parse_paragraph(xml: str): + """Parse XML string and return the first w:p element.""" + doc = defusedxml.minidom.parseString(f"{xml}") + return doc.getElementsByTagName("w:p")[0] + @pytest.fixture def test_data_dir() -> Path: diff --git a/tests/test_coverage_gaps.py b/tests/test_coverage_gaps.py new file mode 100644 index 0000000..6bfe9b8 --- /dev/null +++ b/tests/test_coverage_gaps.py @@ -0,0 +1,482 @@ +"""Tests for coverage gaps in cross-boundary text operations.""" + +import pytest +from conftest import parse_paragraph as _parse_paragraph + +from docx_editor import Document +from docx_editor.xml_editor import ( + TextMapMatch, + TextPosition, + build_text_map, +) + +# -- build_text_map edge cases -- + + +class TestBuildTextMapEdgeCases: + def test_paragraph_with_only_deleted_text(self): + """Paragraph containing only w:delText returns empty text map.""" + p = _parse_paragraph( + "" + '' + "deleted stuff" + "" + "" + ) + tm = build_text_map(p) + assert tm.text == "" + assert tm.positions == [] + + def test_wt_inside_wdel_is_skipped(self): + """w:t inside w:del is skipped (even though normally w:delText is used).""" + p = _parse_paragraph( + "" + '' + "should be skipped" + "" + "visible" + "" + ) + tm = build_text_map(p) + assert tm.text == "visible" + + def test_unicode_and_special_chars(self): + """Text map handles unicode and special characters.""" + p = _parse_paragraph("cafΓ© rΓ©sumΓ© naΓ―ve") + tm = build_text_map(p) + assert tm.text == "cafΓ© rΓ©sumΓ© naΓ―ve" + assert len(tm.positions) == len("cafΓ© rΓ©sumΓ© naΓ―ve") + + def test_xml_entities_in_text(self): + """Text map handles XML entities correctly.""" + p = _parse_paragraph("A & B < C") + tm = build_text_map(p) + assert tm.text == "A & B < C" + + +# -- TextMap edge cases -- + + +class TestTextMapEdgeCases: + def test_find_empty_search(self): + p = _parse_paragraph("Hello") + tm = build_text_map(p) + # Python str.find("") returns 0 + assert tm.find("") == 0 + + def test_find_longer_than_text(self): + p = _parse_paragraph("Hi") + tm = build_text_map(p) + assert tm.find("Hello world") == -1 + + def test_get_nodes_for_range_empty(self): + p = _parse_paragraph("Hello") + tm = build_text_map(p) + assert tm.get_nodes_for_range(2, 2) == [] + + def test_get_nodes_for_range_full(self): + p = _parse_paragraph("Hello") + tm = build_text_map(p) + positions = tm.get_nodes_for_range(0, 5) + assert len(positions) == 5 + + +# -- _classify_segments direct tests -- + + +class TestClassifySegments: + """Direct tests for _classify_segments logic.""" + + def test_single_regular_segment(self): + """All positions regular -> one segment.""" + from unittest.mock import MagicMock + + from docx_editor.track_changes import RevisionManager + + manager = RevisionManager(MagicMock()) + match = TextMapMatch( + start=0, + end=3, + text="abc", + positions=[ + TextPosition(node=None, offset_in_node=0, is_inside_ins=False, is_inside_del=False), + TextPosition(node=None, offset_in_node=1, is_inside_ins=False, is_inside_del=False), + TextPosition(node=None, offset_in_node=2, is_inside_ins=False, is_inside_del=False), + ], + spans_boundary=False, + ) + segments = manager._classify_segments(match) + assert len(segments) == 1 + assert segments[0][0] is False # not inside ins + assert len(segments[0][1]) == 3 + + def test_two_segments_regular_then_ins(self): + """Regular + insertion -> two segments.""" + from unittest.mock import MagicMock + + from docx_editor.track_changes import RevisionManager + + manager = RevisionManager(MagicMock()) + match = TextMapMatch( + start=0, + end=4, + text="abcd", + positions=[ + TextPosition(node=None, offset_in_node=0, is_inside_ins=False, is_inside_del=False), + TextPosition(node=None, offset_in_node=1, is_inside_ins=False, is_inside_del=False), + TextPosition(node=None, offset_in_node=0, is_inside_ins=True, is_inside_del=False), + TextPosition(node=None, offset_in_node=1, is_inside_ins=True, is_inside_del=False), + ], + spans_boundary=True, + ) + segments = manager._classify_segments(match) + assert len(segments) == 2 + assert segments[0][0] is False + assert len(segments[0][1]) == 2 + assert segments[1][0] is True + assert len(segments[1][1]) == 2 + + def test_three_alternating_segments(self): + """Regular + insertion + regular -> three segments.""" + from unittest.mock import MagicMock + + from docx_editor.track_changes import RevisionManager + + manager = RevisionManager(MagicMock()) + match = TextMapMatch( + start=0, + end=3, + text="abc", + positions=[ + TextPosition(node=None, offset_in_node=0, is_inside_ins=False, is_inside_del=False), + TextPosition(node=None, offset_in_node=0, is_inside_ins=True, is_inside_del=False), + TextPosition(node=None, offset_in_node=0, is_inside_ins=False, is_inside_del=False), + ], + spans_boundary=True, + ) + segments = manager._classify_segments(match) + assert len(segments) == 3 + + def test_empty_positions(self): + """Empty positions -> empty segments.""" + from unittest.mock import MagicMock + + from docx_editor.track_changes import RevisionManager + + manager = RevisionManager(MagicMock()) + match = TextMapMatch( + start=0, + end=0, + text="", + positions=[], + spans_boundary=False, + ) + segments = manager._classify_segments(match) + assert segments == [] + + +# -- Mixed-state deletion -- + + +class TestMixedStateDeletion: + """Tests for deleting text spanning revision boundaries.""" + + def test_delete_spanning_regular_and_insertion(self, clean_workspace): + """Delete text that spans regular text + insertion.""" + doc = Document.open(clean_workspace) + doc.insert_after("fox", " ADDED") + + match = doc.find_text("fox ADDED") + if match is None or not match.spans_boundary: + doc.close() + pytest.skip("Expected boundary not created") + + change_id = doc.delete("fox ADDED") + assert change_id >= 0 + + text = doc.get_visible_text() + assert "fox" not in text + assert "ADDED" not in text + doc.close() + + def test_delete_spanning_insertion_and_regular(self, clean_workspace): + """Delete text spanning insertion + regular text.""" + doc = Document.open(clean_workspace) + doc.insert_after("fox", " ADDED") + + match = doc.find_text("ADDED jumps") + if match is None or not match.spans_boundary: + doc.close() + pytest.skip("Expected boundary not created") + + doc.delete("ADDED jumps") + text = doc.get_visible_text() + assert "ADDED" not in text + assert "jumps" not in text + doc.close() + + def test_delete_entirely_within_insertion(self, clean_workspace): + """Delete text entirely within an insertion.""" + doc = Document.open(clean_workspace) + doc.insert_after("fox", " beautiful amazing") + + match = doc.find_text("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") + + doc.delete("beautiful") + text = doc.get_visible_text() + assert "beautiful" not in text + assert "amazing" in text + doc.close() + + def test_delete_mixed_state_roundtrip(self, clean_workspace, temp_dir): + """Round-trip: delete across boundary, save, reopen.""" + doc = Document.open(clean_workspace) + doc.insert_after("fox", " ADDED") + + match = doc.find_text("fox ADDED") + if match is None or not match.spans_boundary: + doc.close() + pytest.skip("Expected boundary not created") + + doc.delete("fox ADDED") + output = temp_dir / "delete_mixed.docx" + doc.save(output) + doc.close() + + doc2 = Document.open(output, force_recreate=True) + text = doc2.get_visible_text() + assert "fox" not in text + assert "ADDED" not in text + doc2.close() + + +# -- _remove_from_insertion all branches -- + + +class TestRemoveFromInsertionBranches: + """Test all 4 branches of _remove_from_insertion.""" + + def test_entire_insertion_removed(self, clean_workspace): + """Removing all text from an insertion removes the w:ins element.""" + doc = Document.open(clean_workspace) + doc.insert_after("fox", " WORD") + + doc.delete("WORD") + text = doc.get_visible_text() + assert "WORD" not in text + doc.close() + + def test_match_at_start_of_insertion(self, clean_workspace): + """Removing text at the start of insertion truncates to remainder.""" + doc = Document.open(clean_workspace) + doc.insert_after("fox", " hello world") + + doc.delete("hello") + text = doc.get_visible_text() + assert "hello" not in text + assert " world" in text or "world" in text + doc.close() + + def test_match_at_end_of_insertion(self, clean_workspace): + """Removing text at end of insertion truncates to start.""" + doc = Document.open(clean_workspace) + doc.insert_after("fox", " hello world") + + doc.delete("world") + text = doc.get_visible_text() + assert "world" not in text + assert "hello" in text + doc.close() + + def test_match_in_middle_of_insertion_splits(self, clean_workspace): + """Removing text in middle of insertion splits into two w:ins elements.""" + doc = Document.open(clean_workspace) + doc.insert_after("fox", " hello beautiful world") + + doc.delete("beautiful") + text = doc.get_visible_text() + assert "beautiful" not in text + assert "hello" in text + assert "world" in text + doc.close() + + def test_middle_split_roundtrip(self, clean_workspace, temp_dir): + """Round-trip: middle split, save, reopen.""" + doc = Document.open(clean_workspace) + doc.insert_after("fox", " hello beautiful world") + + doc.delete("beautiful") + output = temp_dir / "middle_split.docx" + doc.save(output) + doc.close() + + doc2 = Document.open(output, force_recreate=True) + text = doc2.get_visible_text() + assert "beautiful" not in text + assert "hello" in text + assert "world" in text + doc2.close() + + +# -- XML special characters -- + + +class TestXmlSpecialCharacters: + """Tests for XML special characters through cross-boundary paths.""" + + def test_replace_text_with_ampersand(self, clean_workspace): + """Replace with text containing & character.""" + doc = Document.open(clean_workspace) + doc.replace("fox", "fox & cat") + text = doc.get_visible_text() + assert "fox & cat" in text + doc.close() + + def test_replace_text_with_angle_brackets(self, clean_workspace): + """Replace with text containing < and > characters.""" + doc = Document.open(clean_workspace) + doc.replace("fox", "a < b > c") + text = doc.get_visible_text() + assert "a < b > c" in text + doc.close() + + def test_insert_text_with_special_chars(self, clean_workspace): + """Insert text containing XML special characters.""" + doc = Document.open(clean_workspace) + doc.insert_after("fox", " & friends ") + text = doc.get_visible_text() + assert "& friends " in text + doc.close() + + def test_special_chars_roundtrip(self, clean_workspace, temp_dir): + """Round-trip with special characters.""" + doc = Document.open(clean_workspace) + doc.replace("fox", "A & B") + output = temp_dir / "special_chars.docx" + doc.save(output) + doc.close() + + doc2 = Document.open(output, force_recreate=True) + text = doc2.get_visible_text() + assert "A & B" in text + doc2.close() + + +# -- occurrence parameter for cross-boundary -- + + +class TestOccurrenceParameter: + """Tests for occurrence > 0 in cross-boundary operations.""" + + def test_find_text_second_occurrence(self, clean_workspace): + """Find second occurrence across paragraphs.""" + doc = Document.open(clean_workspace) + match0 = doc.find_text("the") + match1 = doc.find_text("the", occurrence=1) + 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 + doc.close() + + def test_replace_second_occurrence_cross_boundary(self, clean_workspace): + """Replace second occurrence when both span boundaries.""" + doc = Document.open(clean_workspace) + doc.insert_after("fox", " test") + doc.insert_after("dog", " test") + + match = doc.find_text("test", occurrence=1) + if match is None: + doc.close() + pytest.skip("Second occurrence not found") + + doc.replace("test", "REPLACED", occurrence=1) + text = doc.get_visible_text() + assert "test" in text + assert "REPLACED" in text + doc.close() + + +# -- find_text edge cases -- + + +class TestFindTextEdgeCases: + """Edge cases for Document.find_text.""" + + def test_find_deleted_text_returns_none(self, clean_workspace): + """Text inside w:del should not be found.""" + doc = Document.open(clean_workspace) + doc.delete("fox") + match = doc.find_text("fox") + assert match is None + doc.close() + + def test_get_visible_text_excludes_deleted(self, clean_workspace): + """Visible text should not include deleted text.""" + doc = Document.open(clean_workspace) + doc.delete("fox") + text = doc.get_visible_text() + assert "fox" not in text + doc.close() + + +# -- insert_after/before with cross-boundary anchor -- + + +class TestInsertWithCrossBoundaryAnchor: + """Tests for insert when anchor text spans element boundaries.""" + + def test_insert_after_cross_boundary_anchor(self, clean_workspace): + """insert_after works when anchor spans a boundary.""" + doc = Document.open(clean_workspace) + doc.insert_after("fox", " RED") + + match = doc.find_text("fox RED") + if match is None or not match.spans_boundary: + doc.close() + pytest.skip("Boundary not created") + + change_id = doc.insert_after("fox RED", " NEW") + assert change_id >= 0 + text = doc.get_visible_text() + assert "fox RED NEW" in text + doc.close() + + def test_insert_before_cross_boundary_anchor(self, clean_workspace): + """insert_before works when anchor spans a boundary.""" + doc = Document.open(clean_workspace) + doc.insert_after("fox", " RED") + + match = doc.find_text("fox RED") + if match is None or not match.spans_boundary: + doc.close() + pytest.skip("Boundary not created") + + change_id = doc.insert_before("fox RED", "NEW ") + assert change_id >= 0 + text = doc.get_visible_text() + assert "NEW fox RED" in text + doc.close() + + def test_insert_cross_boundary_roundtrip(self, clean_workspace, temp_dir): + """Round-trip: insert with cross-boundary anchor, save, reopen.""" + doc = Document.open(clean_workspace) + doc.insert_after("fox", " RED") + + match = doc.find_text("fox RED") + if match is None or not match.spans_boundary: + doc.close() + pytest.skip("Boundary not created") + + doc.insert_after("fox RED", " NEW") + output = temp_dir / "insert_cross_boundary.docx" + doc.save(output) + doc.close() + + doc2 = Document.open(output, force_recreate=True) + text = doc2.get_visible_text() + assert "fox RED NEW" in text + doc2.close() diff --git a/tests/test_cross_boundary_replace.py b/tests/test_cross_boundary_replace.py new file mode 100644 index 0000000..60bcc94 --- /dev/null +++ b/tests/test_cross_boundary_replace.py @@ -0,0 +1,215 @@ +"""Tests for cross-boundary replace operations (Phase 4).""" + +from pathlib import Path + +import defusedxml.minidom + +from docx_editor import Document +from docx_editor.track_changes import RevisionManager, _escape_xml +from docx_editor.xml_editor import DocxXMLEditor, build_text_map + +NS = 'xmlns:w="http://schemas.openxmlformats.org/wordprocessingml/2006/main"' + + +def _make_editor_with_split_runs(tmp_path: Path, runs: list[str], rPr_xml: str = "") -> DocxXMLEditor: + """Create a DocxXMLEditor with a paragraph containing multiple runs. + + Each string in `runs` becomes a separate .... + """ + run_xml = "" + for text in runs: + space_attr = ' xml:space="preserve"' if text and (text[0].isspace() or text[-1].isspace()) else "" + run_xml += f"{rPr_xml}{_escape_xml(text)}" + + doc_xml = f""" + + + {run_xml} + +""" + + xml_path = tmp_path / "document.xml" + xml_path.write_text(doc_xml, encoding="utf-8") + return DocxXMLEditor(xml_path, rsid="00AA1234", author="Test Author") + + +class TestCrossBoundaryReplaceRegression: + """Regression: single-element replace still works.""" + + def test_replace_within_single_element(self, clean_workspace): + """Replace text contained in a single w:t element.""" + doc = Document.open(clean_workspace) + change_id = doc.replace("fox", "cat") + assert change_id >= 0 + assert "cat" in doc.get_visible_text() + assert "fox" not in doc.get_visible_text() + doc.close() + + +class TestReplaceAcrossNodes: + """Unit tests for _replace_across_nodes using crafted XML.""" + + def test_replace_text_spanning_two_runs(self, temp_dir): + """Replace text that spans two consecutive runs.""" + editor = _make_editor_with_split_runs(temp_dir, ["Hello wo", "rld!"]) + mgr = RevisionManager(editor) + + change_id = mgr.replace_text("wo" + "rld", "universe") + assert change_id >= 0 + + # Verify visible text + paras = editor.dom.getElementsByTagName("w:p") + tm = build_text_map(paras[0]) + assert "universe" in tm.text + assert "world" not in tm.text + # "Hello " should be preserved + assert tm.text.startswith("Hello ") + + def test_replace_text_spanning_three_runs(self, temp_dir): + """Replace text that spans three consecutive runs.""" + editor = _make_editor_with_split_runs(temp_dir, ["ab", "cd", "ef"]) + mgr = RevisionManager(editor) + + change_id = mgr.replace_text("bcde", "XY") + assert change_id >= 0 + + paras = editor.dom.getElementsByTagName("w:p") + tm = build_text_map(paras[0]) + assert "aXYf" == tm.text + + def test_replace_entire_run_contents(self, temp_dir): + """Replace text that exactly covers two full runs.""" + editor = _make_editor_with_split_runs(temp_dir, ["Hello", " World"]) + mgr = RevisionManager(editor) + + change_id = mgr.replace_text("Hello World", "Goodbye") + assert change_id >= 0 + + paras = editor.dom.getElementsByTagName("w:p") + tm = build_text_map(paras[0]) + assert "Goodbye" == tm.text + + def test_replace_preserves_rpr(self, temp_dir): + """Replacement insertion preserves run properties from first run.""" + rPr = "" + editor = _make_editor_with_split_runs(temp_dir, ["Hel", "lo"], rPr_xml=rPr) + mgr = RevisionManager(editor) + + mgr.replace_text("Hello", "Hi") + + # The insertion should contain w:b + ins_elems = editor.dom.getElementsByTagName("w:ins") + assert len(ins_elems) == 1 + rPr_elems = ins_elems[0].getElementsByTagName("w:b") + assert len(rPr_elems) >= 1 + + def test_replace_creates_del_and_ins(self, temp_dir): + """Cross-boundary replace creates proper w:del and w:ins elements.""" + editor = _make_editor_with_split_runs(temp_dir, ["foo", "bar"]) + mgr = RevisionManager(editor) + + mgr.replace_text("foobar", "baz") + + del_elems = editor.dom.getElementsByTagName("w:del") + ins_elems = editor.dom.getElementsByTagName("w:ins") + assert len(del_elems) >= 1 + assert len(ins_elems) == 1 + + # Verify del contains the old text + del_texts = editor.dom.getElementsByTagName("w:delText") + del_content = "".join(dt.firstChild.data for dt in del_texts if dt.firstChild) + assert del_content == "foobar" + + def test_replace_saves_valid_xml(self, temp_dir): + """After cross-boundary replace, XML can be saved and re-parsed.""" + editor = _make_editor_with_split_runs(temp_dir, ["Hello wo", "rld!"]) + mgr = RevisionManager(editor) + + mgr.replace_text("world", "universe") + editor.save() + + # Re-parse to verify valid XML + reparsed = defusedxml.minidom.parse(str(temp_dir / "document.xml")) + assert reparsed is not None + + +class TestSuggestDeletionAcrossNodes: + """Unit tests for cross-boundary suggest_deletion.""" + + def test_delete_text_spanning_two_runs(self, temp_dir): + """Delete text that spans two consecutive runs.""" + editor = _make_editor_with_split_runs(temp_dir, ["Hello wo", "rld!"]) + mgr = RevisionManager(editor) + + change_id = mgr.suggest_deletion("world") + assert change_id >= 0 + + paras = editor.dom.getElementsByTagName("w:p") + tm = build_text_map(paras[0]) + assert "world" not in tm.text + assert "Hello " in tm.text + + def test_delete_text_spanning_three_runs(self, temp_dir): + """Delete text that spans three consecutive runs.""" + editor = _make_editor_with_split_runs(temp_dir, ["ab", "cd", "ef"]) + mgr = RevisionManager(editor) + + change_id = mgr.suggest_deletion("bcde") + assert change_id >= 0 + + paras = editor.dom.getElementsByTagName("w:p") + tm = build_text_map(paras[0]) + assert tm.text == "af" + + +class TestCrossBoundaryReplaceRoundtrip: + """Integration round-trip tests using real docx files.""" + + def test_replace_across_boundary_roundtrip(self, clean_workspace, temp_dir): + """Round-trip: create split runs, replace across boundary, save, reopen.""" + doc = Document.open(clean_workspace) + + # Manipulate the XML directly to create split runs + editor = doc._document_editor + paras = editor.dom.getElementsByTagName("w:p") + for p in paras: + tm = build_text_map(p) + if "fox" in tm.text: + # Find the w:t containing "fox" + for t_node in p.getElementsByTagName("w:t"): + if t_node.firstChild and "fox" in t_node.firstChild.data: + text = t_node.firstChild.data + idx = text.find("fox") + # Split: before "fo" | "x jumps" | rest + before = text[: idx + 2] # "...fo" + after = text[idx + 2 :] # "x jumps..." + + run = t_node.parentNode + rPr_xml = "" + rPr_elems = run.getElementsByTagName("w:rPr") + if rPr_elems: + rPr_xml = rPr_elems[0].toxml() + + # Replace with two runs + new_xml = ( + f"{rPr_xml}{_escape_xml(before)}" + f"{rPr_xml}{_escape_xml(after)}" + ) + editor.replace_node(run, new_xml) + break + break + + # Now "fox" spans two runs: "...fo" and "x jumps..." + change_id = doc.replace("fox", "cat") + assert change_id >= 0 + + output = temp_dir / "output.docx" + doc.save(output) + doc.close() + + # Reopen and verify + doc2 = Document.open(output, force_recreate=True) + text = doc2.get_visible_text() + assert "cat" in text + assert "fox" not in text + doc2.close() diff --git a/tests/test_document.py b/tests/test_document.py index b96ebdc..f305265 100644 --- a/tests/test_document.py +++ b/tests/test_document.py @@ -413,3 +413,89 @@ def test_ensure_comment_content_types_already_exists(self, clean_workspace): doc._ensure_comment_content_types() doc.close() + + +class TestDocumentGetVisibleText: + """Tests for get_visible_text().""" + + def test_get_visible_text_basic(self, clean_workspace): + """Test getting visible text from a simple document.""" + doc = Document.open(clean_workspace) + text = doc.get_visible_text() + # The simple.docx test fixture has some text content + assert isinstance(text, str) + assert len(text) > 0 + doc.close() + + def test_get_visible_text_after_insertion(self, clean_workspace): + """Inserted text should appear in visible text.""" + doc = Document.open(clean_workspace) + original = doc.get_visible_text() + doc.insert_after("fox", " INSERTED") + updated = doc.get_visible_text() + assert "INSERTED" in updated + assert len(updated) > len(original) + doc.close() + + def test_get_visible_text_after_deletion(self, clean_workspace): + """Deleted text should NOT appear in visible text.""" + doc = Document.open(clean_workspace) + original = doc.get_visible_text() + assert "fox" in original + doc.delete("fox") + updated = doc.get_visible_text() + assert "fox" not in updated + doc.close() + + def test_get_visible_text_after_replace(self, clean_workspace): + """Replaced text should show new text, not old.""" + doc = Document.open(clean_workspace) + doc.replace("fox", "cat") + text = doc.get_visible_text() + assert "cat" in text + assert "fox" not in text + doc.close() + + def test_get_visible_text_after_close_raises(self, clean_workspace): + """Should raise ValueError after document is closed.""" + doc = Document.open(clean_workspace) + doc.close() + with pytest.raises(ValueError, match="closed"): + doc.get_visible_text() + + +class TestDocumentFindText: + """Tests for find_text().""" + + def test_find_text_simple(self, clean_workspace): + """Find text in a simple document.""" + doc = Document.open(clean_workspace) + match = doc.find_text("fox") + assert match is not None + assert match.text == "fox" + assert not match.spans_boundary + doc.close() + + def test_find_text_not_found(self, clean_workspace): + """Return None when text not found.""" + doc = Document.open(clean_workspace) + match = doc.find_text("nonexistent") + assert match is None + doc.close() + + def test_find_text_after_insertion(self, clean_workspace): + """Find text that spans an insertion boundary.""" + doc = Document.open(clean_workspace) + # insert_after splits the run at the anchor and inserts inline after it + doc.insert_after("fox", " INSERTED") + # "fox INSERTED" spans original run + insertion + match = doc.find_text("fox INSERTED") + assert match is not None + assert match.spans_boundary + doc.close() + + def test_find_text_after_close_raises(self, clean_workspace): + doc = Document.open(clean_workspace) + doc.close() + with pytest.raises(ValueError, match="closed"): + doc.find_text("test") diff --git a/tests/test_mixed_state.py b/tests/test_mixed_state.py new file mode 100644 index 0000000..e503270 --- /dev/null +++ b/tests/test_mixed_state.py @@ -0,0 +1,173 @@ +"""Tests for mixed-state editing (Phase 5: Atomic Decomposition).""" + +import pytest +from conftest import parse_paragraph as _parse_paragraph + +from docx_editor import Document +from docx_editor.xml_editor import build_text_map, find_in_text_map + + +class TestMixedStateDetection: + """Tests that mixed-state matches are correctly detected.""" + + def test_regular_plus_insertion_spans_boundary(self): + """Match spanning regular text + insertion is detected.""" + p = _parse_paragraph( + "" + "Aim: " + '' + "To examine" + "" + "" + ) + tm = build_text_map(p) + match = find_in_text_map(tm, "Aim: To") + assert match is not None + assert match.spans_boundary + + def test_fully_within_insertion_no_boundary(self): + """Match fully within insertion does not span boundary.""" + p = _parse_paragraph( + "" + '' + "Hello beautiful world" + "" + "" + ) + tm = build_text_map(p) + match = find_in_text_map(tm, "beautiful") + assert match is not None + assert not match.spans_boundary + assert all(pos.is_inside_ins for pos in match.positions) + + +class TestMixedStateReplace: + """Integration tests for replace spanning revision boundaries.""" + + def test_replace_spanning_regular_and_insertion(self, clean_workspace): + """Replace text spanning regular text + . + + Regular text ("dog.") should be wrapped in . + Inserted text (" ADDED") should be removed (undo insertion), not del-wrapped. + New text should appear in a . + """ + doc = Document.open(clean_workspace) + # insert_after puts text after the run containing anchor, + # so "dog." is at end of run -> " ADDED" is in right after + doc.insert_after("dog.", " ADDED") + + match = doc.find_text("dog. ADDED") + if match is None or not match.spans_boundary: + doc.close() + pytest.skip("Expected boundary not created") + + change_id = doc.replace("dog. ADDED", "cat. CHANGED") + assert change_id >= 0 + + text = doc.get_visible_text() + assert "cat. CHANGED" in text + assert "dog." not in text + assert "ADDED" not in text + + # Verify XML semantics: " ADDED" should NOT be in any + # (it was an insertion that was undone, not deleted) + dom = doc._document_editor.dom + for del_text in dom.getElementsByTagName("w:delText"): + content = del_text.firstChild.data if del_text.firstChild else "" + assert "ADDED" not in content, "Inserted text should be removed, not wrapped in w:del" + doc.close() + + def test_replace_fully_within_insertion(self, clean_workspace): + """Replace text fully within an -- regression.""" + doc = Document.open(clean_workspace) + doc.insert_after("dog.", " beautiful amazing") + + match = doc.find_text("beautiful") + if match is None: + doc.close() + pytest.skip("Text not found in insertion") + + if not all(pos.is_inside_ins for pos in match.positions): + doc.close() + pytest.skip("Text not entirely within insertion") + + change_id = doc.replace("beautiful", "wonderful") + # change_id is -1 when editing in-place inside an existing insertion + assert isinstance(change_id, int) + text = doc.get_visible_text() + assert "wonderful" in text + assert "beautiful" not in text + doc.close() + + def test_ins_node_splitting(self, clean_workspace): + """When partially matching an insertion, the ins node is split.""" + doc = Document.open(clean_workspace) + doc.insert_after("dog.", " To examine whether") + + match = doc.find_text("To") + if match is None: + doc.close() + pytest.skip("Text not found") + + doc.delete("To") + + text = doc.get_visible_text() + assert "To" not in text + assert "examine whether" in text + doc.close() + + def test_rpr_preservation_across_split(self, clean_workspace): + """w:rPr is preserved when splitting runs.""" + doc = Document.open(clean_workspace) + doc.insert_after("dog.", " ADDED") + + match = doc.find_text("dog. ADDED") + if match is None or not match.spans_boundary: + doc.close() + pytest.skip("Expected boundary not created") + + doc.replace("dog. ADDED", "cat. CHANGED") + text = doc.get_visible_text() + assert "cat. CHANGED" in text + doc.close() + + def test_roundtrip_mixed_state_replace(self, clean_workspace, temp_dir): + """Round-trip: mixed-state replace, save, reopen.""" + doc = Document.open(clean_workspace) + doc.insert_after("dog.", " ADDED") + + match = doc.find_text("dog. ADDED") + if match is None or not match.spans_boundary: + doc.close() + pytest.skip("Expected boundary not created") + + doc.replace("dog. ADDED", "cat. CHANGED") + output = temp_dir / "mixed_state_output.docx" + doc.save(output) + doc.close() + + doc2 = Document.open(output, force_recreate=True) + text = doc2.get_visible_text() + assert "cat. CHANGED" in text + assert "dog." not in text + doc2.close() + + def test_roundtrip_within_insertion_replace(self, clean_workspace, temp_dir): + """Round-trip: replace within insertion, save, reopen.""" + doc = Document.open(clean_workspace) + doc.insert_after("dog.", " beautiful amazing") + + match = doc.find_text("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") + + doc.replace("beautiful", "wonderful") + output = temp_dir / "within_ins_output.docx" + doc.save(output) + doc.close() + + doc2 = Document.open(output, force_recreate=True) + text = doc2.get_visible_text() + assert "wonderful" in text + doc2.close() diff --git a/tests/test_multi_wt.py b/tests/test_multi_wt.py new file mode 100644 index 0000000..e00c551 --- /dev/null +++ b/tests/test_multi_wt.py @@ -0,0 +1,195 @@ +"""Tests for multi-w:t node scenarios. + +These tests verify correct behavior when a single contains +multiple elements, which is valid OOXML (Word produces this +with xml:space differences, tab stops, etc.). + +These tests exercise the multi-node grouping logic in: +- _build_cross_boundary_parts (group by w:t node, not run) +- _delete_regular_segment (handle segment spanning multiple w:t nodes) +- _remove_from_insertion (handle insertion with multiple w:t nodes) +""" + +import defusedxml.minidom +import pytest + +from docx_editor.xml_editor import build_text_map, find_in_text_map + +NS = 'xmlns:w="http://schemas.openxmlformats.org/wordprocessingml/2006/main"' + + +def _parse_body(xml: str): + """Parse XML and return the w:body element.""" + full = f"{xml}" + doc = defusedxml.minidom.parseString(full) + return doc.getElementsByTagName("w:body")[0] + + +class TestMultiWtTextMap: + """Verify text map correctly handles runs with multiple w:t nodes.""" + + def test_two_wt_in_one_run(self): + """A run with two w:t nodes produces correct positions.""" + body = _parse_body("Hello world") + p = body.getElementsByTagName("w:p")[0] + tm = build_text_map(p) + assert tm.text == "Hello world" + + # Positions for 'H' and 'w' should reference different nodes + pos_h = tm.positions[0] + pos_w = tm.positions[6] + assert pos_h.node is not pos_w.node + assert pos_h.offset_in_node == 0 # 'H' at offset 0 in first w:t + assert pos_w.offset_in_node == 0 # 'w' at offset 0 in second w:t + + def test_find_spanning_two_wt_in_same_run(self): + """Finding text that spans two w:t nodes in the same run.""" + body = _parse_body("Hello world") + p = body.getElementsByTagName("w:p")[0] + tm = build_text_map(p) + match = find_in_text_map(tm, "o w") + assert match is not None + assert match.text == "o w" + # The match spans two different w:t nodes + nodes = {id(pos.node) for pos in match.positions} + assert len(nodes) == 2, "Match should span two different w:t nodes" + + +class TestMultiWtCrossBoundary: + """Test cross-boundary operations with multi-w:t runs. + + These use the Document API with crafted XML to trigger the multi-node paths. + """ + + def _create_doc_with_multi_wt(self, tmp_path): + """Create a minimal .docx whose document.xml has a run with two w:t nodes.""" + import shutil + from pathlib import Path + + from docx_editor import Document + + # First create a normal document + fixture = Path(__file__).parent / "test_data" / "simple.docx" + test_docx = tmp_path / "multi_wt.docx" + shutil.copy(fixture, test_docx) + + doc = Document.open(test_docx, force_recreate=True) + return doc + + def _inject_multi_wt_xml(self, doc): + """Replace first paragraph's XML to have a run with two w:t nodes.""" + paragraphs = doc._document_editor.dom.getElementsByTagName("w:p") + if not paragraphs: + return False + + p = paragraphs[0] + # Remove existing children + while p.firstChild: + p.removeChild(p.firstChild) + + # Inject a run with two w:t elements + xml = "Hello beautiful world" + ns = 'xmlns:w="http://schemas.openxmlformats.org/wordprocessingml/2006/main"' + fragment = defusedxml.minidom.parseString(f"{xml}") + for child in fragment.documentElement.childNodes: + imported = p.ownerDocument.importNode(child, True) + p.appendChild(imported) + return True + + def test_delete_spanning_two_wt_in_same_run(self, tmp_path): + """Delete text that spans two w:t nodes within the same run. + + Old code grouped by run id, so both w:t nodes would merge into + one entry with wrong offsets. This caused incorrect text slicing. + """ + doc = self._create_doc_with_multi_wt(tmp_path) + if not self._inject_multi_wt_xml(doc): + doc.close() + pytest.skip("Could not inject XML") + + # Verify the text is correct + text = doc.get_visible_text() + assert "Hello beautiful world" in text + + # Delete "o beautiful w" β€” spans both w:t nodes in the same run + match = doc.find_text("o beautiful w") + assert match is not None + + doc.delete("o beautiful w") + text = doc.get_visible_text() + assert "o beautiful w" not in text + # Should keep "Hell" from first w:t and "orld" from second + assert "Hell" in text + assert "orld" in text + doc.close() + + def test_replace_spanning_two_wt_in_same_run(self, tmp_path): + """Replace text that spans two w:t nodes within the same run. + + Old code stored only the first w:t node per run, so the second + node's text was sliced using offsets from the first node. + """ + doc = self._create_doc_with_multi_wt(tmp_path) + if not self._inject_multi_wt_xml(doc): + doc.close() + pytest.skip("Could not inject XML") + + text = doc.get_visible_text() + assert "Hello beautiful world" in text + + # Replace "o beautiful w" β€” spans both w:t nodes + match = doc.find_text("o beautiful w") + assert match is not None + + doc.replace("o beautiful w", "O REPLACED W") + text = doc.get_visible_text() + assert "o beautiful w" not in text + assert "O REPLACED W" in text + doc.close() + + def test_delete_in_multi_wt_insertion(self, tmp_path): + """Delete text spanning two w:t nodes inside a w:ins element. + + Old _remove_from_insertion only read positions[0].node, ignoring + positions from other w:t nodes β€” causing partial deletion. + """ + doc = self._create_doc_with_multi_wt(tmp_path) + + # Inject a paragraph with an insertion containing two w:t nodes + paragraphs = doc._document_editor.dom.getElementsByTagName("w:p") + if not paragraphs: + doc.close() + pytest.skip("No paragraphs") + + p = paragraphs[0] + while p.firstChild: + p.removeChild(p.firstChild) + + # Regular text + insertion with two w:t nodes + xml = ( + "Before " + '' + "hello world" + "" + " after" + ) + ns = 'xmlns:w="http://schemas.openxmlformats.org/wordprocessingml/2006/main"' + fragment = defusedxml.minidom.parseString(f"{xml}") + for child in fragment.documentElement.childNodes: + imported = p.ownerDocument.importNode(child, True) + p.appendChild(imported) + + 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") + assert match is not None + assert all(pos.is_inside_ins for pos in match.positions) + + doc.delete("o wor") + text = doc.get_visible_text() + assert "o wor" not in text + assert "hell" in text + assert "ld" in text + doc.close() diff --git a/tests/test_multi_wt_safety.py b/tests/test_multi_wt_safety.py new file mode 100644 index 0000000..cb8ed77 --- /dev/null +++ b/tests/test_multi_wt_safety.py @@ -0,0 +1,136 @@ +"""Tests for multi-w:t run safety and related fixes.""" + +from pathlib import Path + +import pytest + +from docx_editor.track_changes import RevisionManager +from docx_editor.xml_editor import DocxXMLEditor + +NS = 'xmlns:w="http://schemas.openxmlformats.org/wordprocessingml/2006/main"' + + +@pytest.fixture +def temp_xml(tmp_path): + def _create_xml(body_xml: str) -> Path: + xml = f'{body_xml}' + xml_path = tmp_path / "test_doc.xml" + xml_path.write_text(xml) + return xml_path + + return _create_xml + + +def _make_manager(xml_path: Path) -> RevisionManager: + editor = DocxXMLEditor(xml_path, rsid="00000000", author="Test Author") + return RevisionManager(editor) + + +def _get_text_content(manager): + dom = manager.editor.dom + result = [] + for wt in dom.getElementsByTagName("w:t"): + parent = wt.parentNode + inside_del = False + while parent: + if ( + parent.localName == "del" + and parent.namespaceURI == "http://schemas.openxmlformats.org/wordprocessingml/2006/main" + ): + inside_del = True + break + parent = parent.parentNode + if not inside_del and wt.firstChild: + result.append(wt.firstChild.data) + return "".join(result) + + +class TestRunDeduplication: + """Test that replacing/deleting across multiple w:t nodes in the same run + doesn't crash from double-removal (comment 2).""" + + def test_replace_across_multi_wt_same_run(self, temp_xml): + xml_path = temp_xml("Hello world") + mgr = _make_manager(xml_path) + mgr.replace_text("lo wor", "LO WOR") + text = _get_text_content(mgr) + assert "LO WOR" in text + + def test_delete_across_multi_wt_same_run(self, temp_xml): + xml_path = temp_xml("Hello world") + mgr = _make_manager(xml_path) + mgr.suggest_deletion("lo wor") + text = _get_text_content(mgr) + assert "lo wor" not in text + + +class TestSingleNodeRemovalGuard: + """Test that removing matched text from a single w:t inside w:ins + doesn't drop sibling w:t nodes (comment 3).""" + + def test_remove_one_run_preserves_sibling(self, temp_xml): + xml_path = temp_xml( + '' + "Hello " + "world" + "" + ) + mgr = _make_manager(xml_path) + mgr.suggest_deletion("world") + text = _get_text_content(mgr) + assert "Hello " in text + # w:ins element should still exist + ins_elements = mgr.editor.dom.getElementsByTagName("w:ins") + assert ins_elements.length > 0 + + def test_remove_one_wt_preserves_sibling_in_same_run(self, temp_xml): + xml_path = temp_xml( + '' + "Hello world" + "" + ) + mgr = _make_manager(xml_path) + mgr.suggest_deletion("world") + text = _get_text_content(mgr) + assert "Hello " in text + ins_elements = mgr.editor.dom.getElementsByTagName("w:ins") + assert ins_elements.length > 0 + + +class TestDeleteRegularSegmentMultiWt: + """Test that _delete_regular_segment works correctly with multi-w:t runs + (comments 4/12).""" + + def test_delete_spanning_multi_wt_no_crash(self, temp_xml): + xml_path = temp_xml("abcdef") + mgr = _make_manager(xml_path) + mgr.suggest_deletion("cd") + text = _get_text_content(mgr) + assert "abef" in text + + def test_delete_middle_wt_preserves_outer(self, temp_xml): + xml_path = temp_xml("Hello big world") + mgr = _make_manager(xml_path) + mgr.suggest_deletion("big ") + text = _get_text_content(mgr) + assert "Hello " in text + assert "world" in text + + +class TestMidNodeInsertionOrder: + """Test that replacement text appears AFTER preserved prefix when match + starts mid-node (comment 11).""" + + def test_replace_mid_node_preserves_prefix_order(self, temp_xml): + xml_path = temp_xml( + "" + "Hello world" + '' + " added" + "" + "" + ) + mgr = _make_manager(xml_path) + mgr.replace_text("world added", "REPLACED") + text = _get_text_content(mgr) + assert text == "Hello REPLACED" diff --git a/tests/test_nested_ins.py b/tests/test_nested_ins.py new file mode 100644 index 0000000..24c9fda --- /dev/null +++ b/tests/test_nested_ins.py @@ -0,0 +1,300 @@ +"""Tests for nested w:ins prevention. + +These tests verify that operations inside existing w:ins elements do not create +nested w:ins or w:del elements, which are invalid in OOXML. +""" + +from pathlib import Path + +import pytest + +from docx_editor.track_changes import RevisionManager +from docx_editor.xml_editor import DocxXMLEditor + +NS = 'xmlns:w="http://schemas.openxmlformats.org/wordprocessingml/2006/main"' + + +@pytest.fixture +def temp_xml(tmp_path): + """Fixture that returns a function to create temp XML files.""" + + def _create_xml(body_xml: str) -> Path: + xml = f'{body_xml}' + xml_path = tmp_path / "test_doc.xml" + xml_path.write_text(xml) + return xml_path + + return _create_xml + + +def _make_manager(xml_path: Path) -> RevisionManager: + """Create a RevisionManager from an XML file path. + + Args: + xml_path: Path to the XML file + + Returns: + RevisionManager instance with the XML loaded + """ + editor = DocxXMLEditor(xml_path, rsid="00000000", author="Test Author") + return RevisionManager(editor) + + +def _assert_no_nested_ins(manager: RevisionManager) -> None: + """Assert no w:ins or w:del is nested inside another w:ins. + + Args: + manager: RevisionManager to check + + Raises: + AssertionError: If nested elements are found + """ + dom = manager.editor.dom + for ins in dom.getElementsByTagName("w:ins"): + # Check no child w:ins + nested_ins = ins.getElementsByTagName("w:ins") + assert len(nested_ins) == 0, f"Found nested w:ins inside w:ins: {ins.toxml()}" + # Check no child w:del + nested_del = ins.getElementsByTagName("w:del") + assert len(nested_del) == 0, f"Found nested w:del inside w:ins: {ins.toxml()}" + + +def _get_text_content(manager: RevisionManager) -> str: + """Extract all visible text from the document. + + Args: + manager: RevisionManager to extract text from + + Returns: + Concatenated text content from all w:t elements not in w:del + """ + dom = manager.editor.dom + result = [] + + # Get all w:t elements + for wt in dom.getElementsByTagName("w:t"): + # Check if inside w:del + parent = wt.parentNode + inside_del = False + while parent: + if ( + parent.localName == "del" + and parent.namespaceURI == "http://schemas.openxmlformats.org/wordprocessingml/2006/main" + ): + inside_del = True + break + parent = parent.parentNode + + if not inside_del and wt.firstChild: + result.append(wt.firstChild.data) + + return "".join(result) + + +class TestReplaceInsideIns: + """Tests for replace operations inside existing w:ins elements.""" + + def test_replace_inside_ins(self, temp_xml): + """Test single-element replace inside w:ins.""" + body_xml = ( + '' + "Hello world" + ) + xml_path = temp_xml(body_xml) + manager = _make_manager(xml_path) + + # Replace "world" with "earth" + manager.replace_text("world", "earth") + + # Assert no nested ins/del + _assert_no_nested_ins(manager) + + # Assert "earth" appears in text + text = _get_text_content(manager) + assert "earth" in text + + +class TestDeleteInsideIns: + """Tests for deletion operations inside existing w:ins elements.""" + + def test_delete_inside_ins(self, temp_xml): + """Test single-element delete inside w:ins.""" + body_xml = ( + '' + "Hello world" + ) + xml_path = temp_xml(body_xml) + manager = _make_manager(xml_path) + + # Delete "world" + manager.suggest_deletion("world") + + # Assert no nested ins/del + _assert_no_nested_ins(manager) + + # "world" should not appear in visible text + text = _get_text_content(manager) + assert "world" not in text + + +class TestInsertInsideIns: + """Tests for insertion operations inside existing w:ins elements.""" + + def test_insert_after_inside_ins(self, temp_xml): + """Test insert_after with anchor inside w:ins.""" + body_xml = ( + 'Hello' + ) + xml_path = temp_xml(body_xml) + manager = _make_manager(xml_path) + + # Insert after "Hello" + manager.insert_text_after("Hello", " world") + + # Assert no nested ins/del + _assert_no_nested_ins(manager) + + # "world" should appear in text + text = _get_text_content(manager) + assert "world" in text + + def test_insert_before_inside_ins(self, temp_xml): + """Test insert_before with anchor inside w:ins.""" + body_xml = ( + 'Hello' + ) + xml_path = temp_xml(body_xml) + manager = _make_manager(xml_path) + + # Insert before "Hello" + manager.insert_text_before("Hello", "Say ") + + # Assert no nested ins/del + _assert_no_nested_ins(manager) + + # "Say" should appear in text + text = _get_text_content(manager) + assert "Say" in text + + +class TestCrossBoundaryInsideIns: + """Tests for cross-boundary operations inside existing w:ins elements.""" + + def test_cross_boundary_replace_all_inside_ins(self, temp_xml): + """Test replace spanning two runs both inside w:ins (site D).""" + body_xml = ( + '' + "Hello world" + ) + xml_path = temp_xml(body_xml) + manager = _make_manager(xml_path) + + # Replace "lo wor" spanning two runs + manager.replace_text("lo wor", "LO WOR") + + # Assert no nested ins/del + _assert_no_nested_ins(manager) + + # "LO WOR" should appear in text + text = _get_text_content(manager) + assert "LO WOR" in text + + def test_cross_boundary_delete_all_inside_ins(self, temp_xml): + """Test delete spanning two runs both inside w:ins (site F).""" + body_xml = ( + '' + "Hello world" + ) + xml_path = temp_xml(body_xml) + manager = _make_manager(xml_path) + + # Delete "lo wor" spanning two runs + manager.suggest_deletion("lo wor") + + # Assert no nested ins/del + _assert_no_nested_ins(manager) + + # "lo wor" should not appear in visible text + text = _get_text_content(manager) + assert "lo wor" not in text + + def test_cross_boundary_insert_near_match_inside_ins(self, temp_xml): + """Test insert near cross-boundary match inside w:ins (sites H/I).""" + body_xml = ( + '' + "Hello world" + ) + xml_path = temp_xml(body_xml) + manager = _make_manager(xml_path) + + # Insert after "lo wor" spanning two runs + manager.insert_text_after("lo wor", "!!") + + # Assert no nested ins/del + _assert_no_nested_ins(manager) + + # "!!" should appear in text + text = _get_text_content(manager) + assert "!!" in text + + +class TestMixedBoundaryScenarios: + """Tests for operations that cross w:ins boundaries.""" + + def test_replace_crossing_ins_boundary_start(self, temp_xml): + """Test replace that starts inside w:ins and ends outside.""" + body_xml = ( + '' + "Hello world" + ) + xml_path = temp_xml(body_xml) + manager = _make_manager(xml_path) + + # Replace "lo wor" crossing boundary + manager.replace_text("lo wor", "LO WOR") + + # Assert no nested ins/del + _assert_no_nested_ins(manager) + + # "LO WOR" should appear + text = _get_text_content(manager) + assert "LO WOR" in text + + def test_replace_crossing_ins_boundary_end(self, temp_xml): + """Test replace that starts outside w:ins and ends inside.""" + body_xml = ( + 'Hello world' + ) + xml_path = temp_xml(body_xml) + manager = _make_manager(xml_path) + + # Replace "lo wor" crossing boundary + manager.replace_text("lo wor", "LO WOR") + + # Assert no nested ins/del + _assert_no_nested_ins(manager) + + # "LO WOR" should appear + text = _get_text_content(manager) + assert "LO WOR" in text + + def test_replace_surrounding_ins(self, temp_xml): + """Test replace that contains an entire w:ins element.""" + body_xml = ( + 'Hello big' + " world" + ) + xml_path = temp_xml(body_xml) + manager = _make_manager(xml_path) + + # Replace "lo big wor" that contains the entire w:ins + manager.replace_text("lo big wor", "LO BIG WOR") + + # Assert no nested ins/del + _assert_no_nested_ins(manager) + + # "LO BIG WOR" should appear + text = _get_text_content(manager) + assert "LO BIG WOR" in text diff --git a/tests/test_review_fixes.py b/tests/test_review_fixes.py new file mode 100644 index 0000000..08194c5 --- /dev/null +++ b/tests/test_review_fixes.py @@ -0,0 +1,373 @@ +"""Tests for specific bugs found during code review of track_changes.py.""" + +from pathlib import Path + +import pytest + +from docx_editor.track_changes import RevisionManager +from docx_editor.xml_editor import DocxXMLEditor + +NS = 'xmlns:w="http://schemas.openxmlformats.org/wordprocessingml/2006/main"' + + +@pytest.fixture +def temp_xml(tmp_path): + def _create_xml(body_xml: str) -> Path: + xml = f'{body_xml}' + xml_path = tmp_path / "test_doc.xml" + xml_path.write_text(xml) + return xml_path + + return _create_xml + + +def _make_manager(xml_path) -> RevisionManager: + editor = DocxXMLEditor(xml_path, rsid="00000000", author="Test Author") + return RevisionManager(editor) + + +def _get_text_content(manager) -> str: + dom = manager.editor.dom + result = [] + for wt in dom.getElementsByTagName("w:t"): + parent = wt.parentNode + inside_del = False + while parent: + if ( + parent.localName == "del" + and parent.namespaceURI == "http://schemas.openxmlformats.org/wordprocessingml/2006/main" + ): + inside_del = True + break + parent = parent.parentNode + if not inside_del and wt.firstChild: + result.append(wt.firstChild.data) + return "".join(result) + + +class TestSiteDPreserveInsWrapper: + """Site D should keep replacement inside w:ins wrapper.""" + + def test_replace_inside_ins_preserves_wrapper(self, temp_xml): + # Two runs inside w:ins, replace all text -> ins_elem gets fully removed + # Replacement should still be inside a w:ins + xml_path = temp_xml( + '' + "ABCD" + ) + mgr = _make_manager(xml_path) + mgr.replace_text("ABCD", "NEW") + text = _get_text_content(mgr) + assert "NEW" in text + # The replacement must be inside a w:ins element + ins_elems = mgr.editor.dom.getElementsByTagName("w:ins") + assert ins_elems.length > 0 + ins_text = [] + for wt in ins_elems[0].getElementsByTagName("w:t"): + if wt.firstChild: + ins_text.append(wt.firstChild.data) + assert "NEW" in "".join(ins_text) + + +class TestInsertTextMultiWtPreservesSiblings: + """_insert_text should preserve sibling w:t nodes in multi-w:t runs.""" + + def test_insert_after_in_multi_wt_run(self, temp_xml): + # Run has two w:t children; insert after text in first w:t + xml_path = temp_xml("Hello world") + mgr = _make_manager(xml_path) + mgr.insert_text_after("Hello", " INSERTED") + text = _get_text_content(mgr) + assert "Hello" in text + assert "INSERTED" in text + assert "world" in text # sibling w:t must be preserved + + +class TestRemoveFromInsertionPreservesSiblingWt: + """_remove_from_insertion multi-node should preserve sibling w:t and set xml:space.""" + + def test_multi_node_removal_preserves_sibling_wt(self, temp_xml): + # w:ins has two runs: first run has two w:t (REMOVE + KEEP), + # second run has one w:t (ALSO). Match "REMOVE" + "ALSO" via cross-boundary. + # But text map concatenates as "REMOVEKEEPALSO", so we match "REMOVEKEEP" + # which spans the first run's two w:t nodes, then the second run's w:t is safe. + # Instead: put KEEP after the matched nodes so the text map is "REMOVEALSOKEEP". + xml_path = temp_xml( + '' + "REMOVE" + "ALSOKEEP" + "" + ) + mgr = _make_manager(xml_path) + mgr.suggest_deletion("REMOVEALSO") + text = _get_text_content(mgr) + assert "KEEP" in text + assert "REMOVE" not in text + assert "ALSO" not in text.replace("KEEP", "") + + def test_truncated_nodes_get_xml_space_preserve(self, temp_xml): + # Multi-node removal where first/last nodes are truncated + xml_path = temp_xml( + '' + "xxAB" + "CDyy" + "" + ) + mgr = _make_manager(xml_path) + mgr.suggest_deletion("ABCD") + text = _get_text_content(mgr) + assert "xx" in text + assert "yy" in text + # Check xml:space="preserve" on truncated nodes + for wt in mgr.editor.dom.getElementsByTagName("w:t"): + if wt.firstChild and wt.firstChild.data in ("xx", "yy"): + assert wt.getAttribute("xml:space") == "preserve" + + +class TestReplaceSameContextPreservesMultiWt: + """_replace_same_context non-ins path should preserve sibling w:t nodes.""" + + def test_replace_preserves_unmatched_wt_siblings(self, temp_xml): + # Two runs, first has two w:t nodes, match spans across runs + xml_path = temp_xml("keepMATCH1MATCH2") + mgr = _make_manager(xml_path) + mgr.replace_text("MATCH1MATCH2", "NEW") + text = _get_text_content(mgr) + assert "keep" in text + assert "NEW" in text + assert "MATCH1" not in text + assert "MATCH2" not in text + + +class TestOccurrenceIndexDrift: + """Multi-w:t fallback must target the same match, not re-count with a different method.""" + + def test_replace_second_occurrence_in_multi_wt_run(self, temp_xml): + # Two paragraphs: first has "cat" in a single w:t, second has "cat" in a + # multi-w:t run (so the fallback triggers). We ask for occurrence=1 (second). + # Bug: _find_across_boundaries with occurrence=1 could return a different + # match than what _get_nth_match found, because the two methods count differently. + xml_path = temp_xml("catcat dog") + mgr = _make_manager(xml_path) + mgr.replace_text("cat", "tiger", occurrence=1) + text = _get_text_content(mgr) + # First "cat" should be untouched, second replaced + assert text.startswith("cat") + assert "tiger" in text + assert "dog" in text # sibling preserved + + def test_delete_second_occurrence_in_multi_wt_run(self, temp_xml): + xml_path = temp_xml("catcat dog") + mgr = _make_manager(xml_path) + mgr.suggest_deletion("cat", occurrence=1) + text = _get_text_content(mgr) + # First "cat" untouched, second deleted, "dog" preserved + assert text.startswith("cat") + assert "dog" in text + # Only one "cat" should remain (the first, untouched) + assert text.count("cat") == 1 + + def test_insert_after_second_occurrence_in_multi_wt_run(self, temp_xml): + xml_path = temp_xml("catcat dog") + mgr = _make_manager(xml_path) + mgr.insert_text_after("cat", "XX", occurrence=1) + text = _get_text_content(mgr) + # "XX" should appear right after second "cat", not at end + assert "catXX" in text + assert "dog" in text + + +class TestSiteDAttributeInjection: + """Site D raw DOM insertion must go through editor helpers for attribute injection.""" + + def test_replace_inside_ins_injects_rsid(self, temp_xml): + # When replacing text across runs inside , the new run should get + # w:rsidR injected by DocxXMLEditor's attribute injection. + xml_path = temp_xml( + '' + "ABCD" + ) + mgr = _make_manager(xml_path) + mgr.replace_text("ABCD", "NEW") + # Find the new w:r containing "NEW" + for wt in mgr.editor.dom.getElementsByTagName("w:t"): + if wt.firstChild and wt.firstChild.data == "NEW": + run = wt.parentNode + assert run.hasAttribute("w:rsidR"), "New run should have w:rsidR from attribute injection" + break + else: + pytest.fail("Could not find w:t with 'NEW'") + + def test_replace_inside_ins_partially_remaining_injects_rsid(self, temp_xml): + # When ins_elem stays in DOM (partial removal), the inserted run should + # also get attribute injection. + xml_path = temp_xml( + '' + "xxABCDyy" + ) + mgr = _make_manager(xml_path) + mgr.replace_text("ABCD", "NEW") + for wt in mgr.editor.dom.getElementsByTagName("w:t"): + if wt.firstChild and wt.firstChild.data == "NEW": + run = wt.parentNode + assert run.hasAttribute("w:rsidR"), "New run should have w:rsidR from attribute injection" + break + else: + pytest.fail("Could not find w:t with 'NEW'") + + +class TestXmlSpacePreserveOnGeneratedWt: + """Generated elements with leading/trailing spaces must have xml:space='preserve'.""" + + def test_replace_preserves_trailing_space_in_before_text(self, temp_xml): + # "Hello world" -> replace "world" -> before_text is "Hello " (trailing space) + xml_path = temp_xml("Hello world") + mgr = _make_manager(xml_path) + mgr.replace_text("world", "earth") + # Find the w:t containing "Hello " + for wt in mgr.editor.dom.getElementsByTagName("w:t"): + if wt.firstChild and wt.firstChild.data == "Hello ": + assert wt.getAttribute("xml:space") == "preserve", ( + "w:t with trailing space must have xml:space='preserve'" + ) + break + else: + pytest.fail("Could not find w:t with 'Hello '") + + def test_delete_preserves_leading_space_in_after_text(self, temp_xml): + # "Hello world" -> delete "Hello" -> after_text is " world" (leading space) + xml_path = temp_xml("Hello world") + mgr = _make_manager(xml_path) + mgr.suggest_deletion("Hello") + for wt in mgr.editor.dom.getElementsByTagName("w:t"): + if wt.firstChild and wt.firstChild.data == " world": + assert wt.getAttribute("xml:space") == "preserve", ( + "w:t with leading space must have xml:space='preserve'" + ) + break + else: + pytest.fail("Could not find w:t with ' world'") + + +class TestOccurrenceFallbackConsistency: + """Initial fallback from _get_nth_match to _find_across_boundaries must find the right match.""" + + def test_replace_cross_boundary_second_occurrence(self, temp_xml): + # First paragraph: "catdog" split across two w:t (not findable by _get_nth_match) + # Second paragraph: "catdog" also split across two w:t + # Asking for occurrence=1 should replace the SECOND "catdog", not the first. + xml_path = temp_xml( + "catdog" + "catdog" + ) + mgr = _make_manager(xml_path) + mgr.replace_text("catdog", "REPLACED", occurrence=1) + text = _get_text_content(mgr) + # First "catdog" should remain, second should be replaced + assert text.startswith("catdog") + assert "REPLACED" in text + + def test_delete_cross_boundary_second_occurrence(self, temp_xml): + xml_path = temp_xml( + "catdog" + "catdog" + ) + mgr = _make_manager(xml_path) + mgr.suggest_deletion("catdog", occurrence=1) + text = _get_text_content(mgr) + # First "catdog" untouched + assert text.startswith("catdog") + # Only one "catdog" should remain + assert text.count("catdog") == 1 + + +class TestMixedStateMarkerPlacement: + """_replace_mixed_state insertion must go after deletion, not before prefix text.""" + + def test_replace_mixed_state_with_before_text(self, temp_xml): + # Regular text "xxAB" followed by ins text "CD" + # Replace "ABCD" -> before_text="xx" from regular, ins text removed + # The replacement should appear AFTER the deletion of "AB", not before "xx" + xml_path = temp_xml( + "" + "xxAB" + '' + "CD" + "" + ) + mgr = _make_manager(xml_path) + mgr.replace_text("ABCD", "NEW") + text = _get_text_content(mgr) + assert "xx" in text + assert "NEW" in text + # "xx" must come BEFORE "NEW" in the visible text + assert text.index("xx") < text.index("NEW"), f"Prefix 'xx' should appear before 'NEW', got: '{text}'" + + +class TestSingleNodeRemovalXmlSpaceAndGuard: + """_remove_from_insertion single-node truncation should set xml:space and guard non-text children.""" + + def test_truncate_before_sets_xml_space(self, temp_xml): + # Delete prefix from " Hello" inside ins -> after_text=" Hello"[len(""):] needs xml:space + xml_path = temp_xml( + 'AB cd' + ) + mgr = _make_manager(xml_path) + mgr.suggest_deletion("AB") + # Remaining text is " cd" which has leading space + for wt in mgr.editor.dom.getElementsByTagName("w:t"): + if wt.firstChild and wt.firstChild.data == " cd": + assert wt.getAttribute("xml:space") == "preserve" + break + else: + pytest.fail("Could not find w:t with ' cd'") + + def test_truncate_after_sets_xml_space(self, temp_xml): + xml_path = temp_xml( + 'cd AB' + ) + mgr = _make_manager(xml_path) + mgr.suggest_deletion("AB") + for wt in mgr.editor.dom.getElementsByTagName("w:t"): + if wt.firstChild and wt.firstChild.data == "cd ": + assert wt.getAttribute("xml:space") == "preserve" + break + else: + pytest.fail("Could not find w:t with 'cd '") + + def test_full_removal_preserves_run_with_tab(self, temp_xml): + # Run has w:t + w:tab; removing w:t should keep run alive for the tab + xml_path = temp_xml( + '' + "GONE" + "STAY" + ) + mgr = _make_manager(xml_path) + mgr.suggest_deletion("GONE") + text = _get_text_content(mgr) + assert "STAY" in text + assert "GONE" not in text + # The run with w:tab should still exist + tabs = mgr.editor.dom.getElementsByTagName("w:tab") + assert tabs.length > 0, "w:tab should be preserved when its sibling w:t is removed" + + +class TestRunRebuildPreservesNonTextChildren: + """_replace_same_context and _delete_same_context should preserve non-text run children.""" + + def test_replace_preserves_tab_in_run(self, temp_xml): + # Run has w:tab + w:t; replacing text should keep the tab + xml_path = temp_xml("MATCHEND") + mgr = _make_manager(xml_path) + mgr.replace_text("MATCHEND", "NEW") + text = _get_text_content(mgr) + assert "NEW" in text + tabs = mgr.editor.dom.getElementsByTagName("w:tab") + assert tabs.length > 0, "w:tab should be preserved during run rebuild" + + def test_delete_preserves_tab_in_run(self, temp_xml): + xml_path = temp_xml("MATCHEND") + mgr = _make_manager(xml_path) + mgr.suggest_deletion("MATCHEND") + tabs = mgr.editor.dom.getElementsByTagName("w:tab") + assert tabs.length > 0, "w:tab should be preserved during deletion rebuild" diff --git a/tests/test_text_map.py b/tests/test_text_map.py new file mode 100644 index 0000000..3923164 --- /dev/null +++ b/tests/test_text_map.py @@ -0,0 +1,237 @@ +"""Tests for text map building (Phase 1: Core Infrastructure).""" + +from conftest import parse_paragraph as _parse_paragraph + +from docx_editor.xml_editor import build_text_map, find_in_text_map + + +class TestBuildTextMapPlainText: + """Tests for text map with plain text (no tracked changes).""" + + def test_single_run(self): + """Single run with plain text.""" + p = _parse_paragraph("Hello world") + tm = build_text_map(p) + assert tm.text == "Hello world" + assert len(tm.positions) == 11 + # All positions should reference the same node + assert all(pos.offset_in_node == i for i, pos in enumerate(tm.positions)) + assert all(not pos.is_inside_ins for pos in tm.positions) + assert all(not pos.is_inside_del for pos in tm.positions) + + def test_multiple_runs(self): + """Multiple runs concatenated.""" + p = _parse_paragraph("Hello world") + tm = build_text_map(p) + assert tm.text == "Hello world" + assert len(tm.positions) == 11 + # First 6 chars from first node, next 5 from second + assert tm.positions[0].node is not tm.positions[6].node + assert tm.positions[5].offset_in_node == 5 # space + assert tm.positions[6].offset_in_node == 0 # 'w' in second node + + def test_empty_paragraph(self): + """Empty paragraph returns empty text map.""" + p = _parse_paragraph("") + tm = build_text_map(p) + assert tm.text == "" + assert tm.positions == [] + + def test_run_with_properties(self): + """Run properties (w:rPr) don't affect text map.""" + p = _parse_paragraph("Bold") + tm = build_text_map(p) + assert tm.text == "Bold" + assert len(tm.positions) == 4 + + +class TestBuildTextMapTrackedChanges: + """Tests for text map with tracked changes.""" + + def test_inserted_text_included(self): + """Text inside is included and marked.""" + p = _parse_paragraph( + "" + "Hello " + '' + "beautiful " + "" + "world" + "" + ) + tm = build_text_map(p) + assert tm.text == "Hello beautiful world" + # Positions 0-5: "Hello " - not inside ins + assert not tm.positions[0].is_inside_ins + assert not tm.positions[5].is_inside_ins + # Positions 6-15: "beautiful " - inside ins + assert tm.positions[6].is_inside_ins + assert tm.positions[15].is_inside_ins + # Positions 16-20: "world" - not inside ins + assert not tm.positions[16].is_inside_ins + + def test_deleted_text_excluded(self): + """Text inside with is excluded.""" + p = _parse_paragraph( + "" + "Hello " + '' + "old " + "" + "world" + "" + ) + tm = build_text_map(p) + assert tm.text == "Hello world" + assert len(tm.positions) == 11 + + def test_mixed_insertions_and_deletions(self): + """Paragraph with both insertions and deletions.""" + p = _parse_paragraph( + "" + "Hello " + '' + "old " + "" + '' + "new " + "" + "world" + "" + ) + tm = build_text_map(p) + assert tm.text == "Hello new world" + # "Hello " - regular + assert not tm.positions[0].is_inside_ins + # "new " - inside ins + assert tm.positions[6].is_inside_ins + assert tm.positions[9].is_inside_ins + # "world" - regular + assert not tm.positions[10].is_inside_ins + + +class TestTextMapFind: + """Tests for TextMap.find() method.""" + + def test_find_simple(self): + """Find text in simple string.""" + p = _parse_paragraph("Hello world") + tm = build_text_map(p) + assert tm.find("world") == 6 + assert tm.find("Hello") == 0 + assert tm.find("missing") == -1 + + def test_find_across_runs(self): + """Find text that spans multiple runs.""" + p = _parse_paragraph("Hello world") + tm = build_text_map(p) + assert tm.find("lo wo") == 3 + + def test_find_across_insertion_boundary(self): + """Find text spanning regular text and insertion.""" + p = _parse_paragraph( + "" + "Hello " + '' + "world" + "" + "" + ) + tm = build_text_map(p) + assert tm.find("Hello world") == 0 + + def test_find_with_start_offset(self): + """Find with start offset.""" + p = _parse_paragraph("hello hello world") + tm = build_text_map(p) + assert tm.find("hello", 0) == 0 + assert tm.find("hello", 1) == 6 + + +class TestTextMapGetNodesForRange: + """Tests for TextMap.get_nodes_for_range().""" + + def test_single_node_range(self): + """Range within a single node.""" + p = _parse_paragraph("Hello world") + tm = build_text_map(p) + positions = tm.get_nodes_for_range(6, 11) # "world" + assert len(positions) == 5 + assert all(pos.node is tm.positions[6].node for pos in positions) + + def test_cross_node_range(self): + """Range spanning two nodes.""" + p = _parse_paragraph("Hello world") + tm = build_text_map(p) + positions = tm.get_nodes_for_range(3, 8) # "lo wo" + assert len(positions) == 5 + # First 3 chars from first node, last 2 from second + assert positions[0].node is not positions[3].node + + +class TestFindInTextMap: + """Tests for find_in_text_map().""" + + def test_find_simple(self): + p = _parse_paragraph("Hello world") + tm = build_text_map(p) + match = find_in_text_map(tm, "world") + assert match is not None + assert match.start == 6 + assert match.end == 11 + assert match.text == "world" + assert not match.spans_boundary + + def test_find_not_found(self): + p = _parse_paragraph("Hello world") + tm = build_text_map(p) + match = find_in_text_map(tm, "missing") + assert match is None + + def test_find_nth_occurrence(self): + p = _parse_paragraph("hello hello hello") + tm = build_text_map(p) + m0 = find_in_text_map(tm, "hello", 0) + m1 = find_in_text_map(tm, "hello", 1) + m2 = find_in_text_map(tm, "hello", 2) + m3 = find_in_text_map(tm, "hello", 3) + assert m0 is not None and m0.start == 0 + assert m1 is not None and m1.start == 6 + assert m2 is not None and m2.start == 12 + assert m3 is None + + def test_find_across_runs(self): + p = _parse_paragraph("Hello world") + tm = build_text_map(p) + match = find_in_text_map(tm, "lo wo") + assert match is not None + assert match.start == 3 + assert not match.spans_boundary # same context (both regular) + + def test_find_spanning_insertion_boundary(self): + p = _parse_paragraph( + "" + "Hello " + '' + "world" + "" + "" + ) + tm = build_text_map(p) + match = find_in_text_map(tm, "Hello world") + assert match is not None + assert match.spans_boundary # crosses ins boundary + + def test_find_entirely_within_insertion(self): + p = _parse_paragraph( + "" + '' + "Hello beautiful world" + "" + "" + ) + tm = build_text_map(p) + match = find_in_text_map(tm, "beautiful") + assert match is not None + assert not match.spans_boundary # entirely within ins + assert all(p.is_inside_ins for p in match.positions) diff --git a/tests/test_track_changes.py b/tests/test_track_changes.py index 643e808..3a4f7f2 100644 --- a/tests/test_track_changes.py +++ b/tests/test_track_changes.py @@ -919,6 +919,7 @@ def test_replace_text_parent_traversal_loop(self): mock_run = MagicMock() mock_run.nodeName = "w:r" + mock_run.parentNode = None mock_run.getElementsByTagName.return_value = [] # No rPr mock_intermediate.nodeName = "other" @@ -954,6 +955,7 @@ def test_suggest_deletion_parent_traversal_loop(self): mock_run = MagicMock() mock_run.nodeName = "w:r" + mock_run.parentNode = None mock_run.getElementsByTagName.return_value = [] # No rPr mock_intermediate.nodeName = "other" @@ -989,6 +991,7 @@ def test_insert_text_parent_traversal_loop(self): mock_run = MagicMock() mock_run.nodeName = "w:r" + mock_run.parentNode = None mock_run.getElementsByTagName.return_value = [] # No rPr mock_intermediate.nodeName = "other" @@ -996,16 +999,17 @@ def test_insert_text_parent_traversal_loop(self): mock_elem.nodeName = "w:t" mock_elem.parentNode = mock_intermediate + mock_elem.firstChild.data = "anchor" mock_editor.find_all_nodes.return_value = [mock_elem] - # Mock insert_after to return a node with w:ins + # Mock replace_node to return a node with w:ins mock_ins_node = MagicMock() mock_ins_node.nodeType = mock_ins_node.ELEMENT_NODE mock_ins_node.tagName = "w:ins" mock_ins_node.getAttribute.return_value = "44" - mock_editor.insert_after.return_value = [mock_ins_node] + mock_editor.replace_node.return_value = [mock_ins_node] manager = RevisionManager(mock_editor) result = manager.insert_text_after("anchor", "new text") @@ -1020,6 +1024,7 @@ def test_replace_text_with_rpr(self): mock_run = MagicMock() mock_run.nodeName = "w:r" + mock_run.parentNode = None # Mock rPr element mock_rPr = MagicMock() @@ -1056,6 +1061,7 @@ def test_suggest_deletion_with_rpr(self): mock_run = MagicMock() mock_run.nodeName = "w:r" + mock_run.parentNode = None # Mock rPr element mock_rPr = MagicMock() @@ -1092,14 +1098,24 @@ def test_insert_text_with_rpr(self): mock_run = MagicMock() mock_run.nodeName = "w:r" + mock_run.parentNode = None # Mock rPr element mock_rPr = MagicMock() mock_rPr.toxml.return_value = "" - mock_run.getElementsByTagName.return_value = [mock_rPr] + + def _get_by_tag(tag): + if tag == "w:rPr": + return [mock_rPr] + if tag == "w:t": + return [mock_elem] + return [] + + mock_run.getElementsByTagName.side_effect = _get_by_tag mock_elem.nodeName = "w:t" mock_elem.parentNode = mock_run + mock_elem.firstChild.data = "anchor" mock_editor.find_all_nodes.return_value = [mock_elem] @@ -1108,14 +1124,14 @@ def test_insert_text_with_rpr(self): mock_ins_node.tagName = "w:ins" mock_ins_node.getAttribute.return_value = "47" - mock_editor.insert_after.return_value = [mock_ins_node] + mock_editor.replace_node.return_value = [mock_ins_node] manager = RevisionManager(mock_editor) result = manager.insert_text_after("anchor", "new text") assert result == 47 # Verify rPr was included in the XML - call_args = mock_editor.insert_after.call_args[0][1] + call_args = mock_editor.replace_node.call_args[0][1] assert "" in call_args def test_replace_text_returns_minus_one_when_no_ins_found(self): @@ -1126,6 +1142,7 @@ def test_replace_text_returns_minus_one_when_no_ins_found(self): mock_run = MagicMock() mock_run.nodeName = "w:r" + mock_run.parentNode = None mock_run.getElementsByTagName.return_value = [] mock_elem.nodeName = "w:t" @@ -1155,6 +1172,7 @@ def test_suggest_deletion_returns_minus_one_when_no_del_found(self): mock_run = MagicMock() mock_run.nodeName = "w:r" + mock_run.parentNode = None mock_run.getElementsByTagName.return_value = [] mock_elem.nodeName = "w:t" @@ -1184,6 +1202,7 @@ def test_insert_text_returns_minus_one_when_no_ins_found(self): mock_run = MagicMock() mock_run.nodeName = "w:r" + mock_run.parentNode = None mock_run.getElementsByTagName.return_value = [] mock_elem.nodeName = "w:t" @@ -1211,6 +1230,7 @@ def test_replace_text_text_mismatch_raises_error(self): mock_run = MagicMock() mock_run.nodeName = "w:r" + mock_run.parentNode = None mock_run.getElementsByTagName.return_value = [] mock_elem.nodeName = "w:t" @@ -1234,6 +1254,7 @@ def test_suggest_deletion_text_mismatch_raises_error(self): mock_run = MagicMock() mock_run.nodeName = "w:r" + mock_run.parentNode = None mock_run.getElementsByTagName.return_value = [] mock_elem.nodeName = "w:t" @@ -1257,6 +1278,7 @@ def test_replace_text_with_before_text_only(self): mock_run = MagicMock() mock_run.nodeName = "w:r" + mock_run.parentNode = None mock_run.getElementsByTagName.return_value = [] mock_elem.nodeName = "w:t" @@ -1289,6 +1311,7 @@ def test_suggest_deletion_with_before_text_only(self): mock_run = MagicMock() mock_run.nodeName = "w:r" + mock_run.parentNode = None mock_run.getElementsByTagName.return_value = [] mock_elem.nodeName = "w:t" @@ -1321,6 +1344,7 @@ def test_suggest_deletion_with_after_text_only(self): mock_run = MagicMock() mock_run.nodeName = "w:r" + mock_run.parentNode = None mock_run.getElementsByTagName.return_value = [] mock_elem.nodeName = "w:t" diff --git a/tests/test_track_changes_coverage.py b/tests/test_track_changes_coverage.py new file mode 100644 index 0000000..d05bf74 --- /dev/null +++ b/tests/test_track_changes_coverage.py @@ -0,0 +1,336 @@ +"""Tests to improve coverage for track_changes.py uncovered lines.""" + +from pathlib import Path + +import pytest + +from docx_editor.track_changes import RevisionManager +from docx_editor.xml_editor import DocxXMLEditor + +NS = 'xmlns:w="http://schemas.openxmlformats.org/wordprocessingml/2006/main"' + + +@pytest.fixture +def temp_xml(tmp_path): + def _create_xml(body_xml: str) -> Path: + xml = f'{body_xml}' + xml_path = tmp_path / "test_doc.xml" + xml_path.write_text(xml) + return xml_path + + return _create_xml + + +def _make_manager(xml_path: Path) -> RevisionManager: + editor = DocxXMLEditor(xml_path, rsid="00000000", author="Test Author") + return RevisionManager(editor) + + +def _get_text_content(manager) -> str: + dom = manager.editor.dom + result = [] + for wt in dom.getElementsByTagName("w:t"): + parent = wt.parentNode + inside_del = False + while parent: + if ( + parent.localName == "del" + and parent.namespaceURI == "http://schemas.openxmlformats.org/wordprocessingml/2006/main" + ): + inside_del = True + break + parent = parent.parentNode + if not inside_del and wt.firstChild: + result.append(wt.firstChild.data) + return "".join(result) + + +class TestReplaceMultiWtDelegatesToCrossBoundary: + """Lines 143-146: replace_text delegates to cross-boundary when run has multiple w:t.""" + + def test_replace_text_multi_wt_run_delegates(self, temp_xml): + # A single run with two w:t children, where the search text is in one of them. + # This triggers lines 143-146 in replace_text. + xml_path = temp_xml("Hello world") + mgr = _make_manager(xml_path) + mgr.replace_text("world", "EARTH") + text = _get_text_content(mgr) + assert "EARTH" in text + assert "world" not in text + + +class TestSuggestDeletionInsideInsRemovesEntireIns: + """Lines 254-255: suggest_deletion removes entire w:ins when sole w:t fully matched.""" + + def test_delete_entire_single_wt_inside_ins(self, temp_xml): + xml_path = temp_xml( + 'Hello' + ) + mgr = _make_manager(xml_path) + mgr.suggest_deletion("Hello") + # The entire w:ins should be removed + assert mgr.editor.dom.getElementsByTagName("w:ins").length == 0 + text = _get_text_content(mgr) + assert text == "" + + +class TestGetRunInfoNoParentRun: + """Lines 294, 296: _get_run_info returns (None, '') when no w:r ancestor.""" + + def test_get_run_info_no_run_parent(self, temp_xml): + xml_path = temp_xml("orphan") + mgr = _make_manager(xml_path) + wt = mgr.editor.dom.getElementsByTagName("w:t")[0] + run, rPr = mgr._get_run_info(wt) + assert run is None + assert rPr == "" + + +class TestBuildCrossBoundaryPartsSkipsOrphan: + """Line 324: _build_cross_boundary_parts skips positions with no parent run.""" + + def test_empty_parts_returns_minus_one(self, temp_xml): + xml_path = temp_xml("Hello world") + mgr = _make_manager(xml_path) + # _replace_same_context with empty parts returns -1 (line 385) + from docx_editor.xml_editor import TextMapMatch + + empty_match = TextMapMatch(start=0, end=0, text="", positions=[], spans_boundary=False) + result = mgr._replace_same_context(empty_match, "X") + assert result == -1 + + +class TestReplaceSameContextAllInsideIns: + """Lines 385, 406-407: _replace_same_context when all positions inside w:ins, + and ins_elem has no firstChild after removal.""" + + def test_replace_across_runs_all_inside_ins_no_remaining(self, temp_xml): + # Two runs inside w:ins, replace spanning both fully -> empties ins, appends + xml_path = temp_xml( + '' + "ABCD" + ) + mgr = _make_manager(xml_path) + mgr.replace_text("ABCD", "NEW") + text = _get_text_content(mgr) + assert "NEW" in text + + +class TestReplaceMixedStateNoDelFound: + """Line 499: _replace_mixed_state inserts after marker when no w:del siblings.""" + + def test_replace_mixed_all_ins_segment(self, temp_xml): + # Match entirely inside w:ins with spans_boundary=True triggers _replace_mixed_state. + # After _remove_from_insertion, no w:del exists, so insert after marker (line 499). + xml_path = temp_xml( + "" + '' + "Hello" + " world" + "" + ) + mgr = _make_manager(xml_path) + # "Hello w" spans ins boundary + mgr.replace_text("Hello w", "NEW W") + text = _get_text_content(mgr) + assert "NEW W" in text + + +class TestRemoveFromInsertionMiddleSplit: + """Lines 582-590: _remove_from_insertion with middle split (before and after text remain).""" + + def test_remove_middle_from_single_node_ins(self, temp_xml): + xml_path = temp_xml( + 'ABCDE' + ) + mgr = _make_manager(xml_path) + # Delete "BCD" from middle of "ABCDE" inside w:ins + mgr.suggest_deletion("BCD") + text = _get_text_content(mgr) + # Should have "A" and "E" remaining + assert "A" in text + assert "E" in text + assert "BCD" not in text + + +class TestRemoveFromInsertionSoleWtRemovesIns: + """Line 567: _remove_from_insertion removes entire w:ins when sole w:t fully matched + (through cross-boundary path).""" + + def test_cross_boundary_delete_entire_ins_sole_wt(self, temp_xml): + xml_path = temp_xml( + '' + "AB" + '' + "CD" + ) + mgr = _make_manager(xml_path) + # Delete "ABCD" spanning two w:ins elements + mgr.suggest_deletion("ABCD") + text = _get_text_content(mgr) + assert text == "" + + +class TestRemoveFromInsertionMultiWtRemoveJustNode: + """Lines 569-575: _remove_from_insertion removes just the w:t node when other w:t exists.""" + + def test_remove_one_wt_from_multi_wt_ins_via_cross_boundary(self, temp_xml): + # Two w:t nodes in same ins, delete one entirely via cross-boundary + xml_path = temp_xml( + '' + "AB" + "CD" + "" + ) + mgr = _make_manager(xml_path) + mgr.suggest_deletion("CD") + text = _get_text_content(mgr) + assert "AB" in text + assert "CD" not in text + # w:ins should still exist + assert mgr.editor.dom.getElementsByTagName("w:ins").length > 0 + + +class TestRemoveFromInsertionMultiNodeNoBeforeNoAfter: + """Lines 598-600, 605-607, 611-613: Multi-node removal with no before/after text.""" + + def test_multi_node_removal_no_before_no_after(self, temp_xml): + # Three runs inside w:ins, delete spanning all three fully + xml_path = temp_xml( + '' + "AB" + "CD" + "EF" + "" + ) + mgr = _make_manager(xml_path) + mgr.suggest_deletion("ABCDEF") + text = _get_text_content(mgr) + assert text == "" + + def test_multi_node_removal_with_before_and_after(self, temp_xml): + # Three runs inside w:ins, delete middle portion spanning all three + xml_path = temp_xml( + '' + "xxAB" + "CD" + "EFyy" + "" + ) + mgr = _make_manager(xml_path) + mgr.suggest_deletion("ABCDEF") + text = _get_text_content(mgr) + assert "xx" in text + assert "yy" in text + assert "ABCDEF" not in text + + +class TestGetWtNodesInAncestorNone: + """Line 618: _get_wt_nodes_in_ancestor returns [] for None ancestor.""" + + def test_none_ancestor(self, temp_xml): + xml_path = temp_xml("x") + mgr = _make_manager(xml_path) + assert mgr._get_wt_nodes_in_ancestor(None) == [] + + +class TestDeleteRegularSegmentUnmatchedWt: + """Lines 675-678: _delete_regular_segment preserves unmatched w:t siblings.""" + + def test_delete_preserves_unmatched_wt(self, temp_xml): + # Already tested in test_multi_wt_safety.py but let's cover the + # specific "unmatched sibling" branch in _delete_regular_segment + xml_path = temp_xml("prefixMATCHsuffix") + mgr = _make_manager(xml_path) + mgr.suggest_deletion("MATCH") + text = _get_text_content(mgr) + assert "prefix" in text + assert "suffix" in text + assert "MATCH" not in text + + +class TestDeleteRegularSegmentIntermediateNode: + """Line 698: intermediate node uses entire text as matched.""" + + def test_three_wt_nodes_spanning_deletion(self, temp_xml): + # Three separate runs, delete spanning all three + xml_path = temp_xml("xxABCDEFyy") + mgr = _make_manager(xml_path) + mgr.suggest_deletion("ABCDEF") + text = _get_text_content(mgr) + assert "xx" in text + assert "yy" in text + + +class TestDeleteSameContextReturnMinusOne: + """Lines 732, 804: _delete_same_context returns -1 for empty parts.""" + + def test_delete_same_context_empty_parts(self, temp_xml): + xml_path = temp_xml("Hello") + mgr = _make_manager(xml_path) + from docx_editor.xml_editor import TextMapMatch + + empty_match = TextMapMatch(start=0, end=0, text="", positions=[], spans_boundary=False) + result = mgr._delete_same_context(empty_match) + assert result == -1 + + +class TestInsertNearMatchInsideInsBeforePosition: + """Line 955: _insert_near_match with position='before' inside w:ins.""" + + def test_insert_before_cross_boundary_inside_ins(self, temp_xml): + xml_path = temp_xml( + '' + "Hello world" + ) + mgr = _make_manager(xml_path) + # Insert before "lo wor" which spans two runs inside w:ins + mgr.insert_text_before("lo wor", "XX") + text = _get_text_content(mgr) + assert "XX" in text + + +class TestSuggestDeletionMultiWtDelegates: + """Lines 230-233: suggest_deletion delegates to cross-boundary for multi-w:t run.""" + + def test_suggest_deletion_multi_wt_delegates(self, temp_xml): + xml_path = temp_xml("Hello world") + mgr = _make_manager(xml_path) + mgr.suggest_deletion("world") + text = _get_text_content(mgr) + assert "Hello " in text + assert "world" not in text + + +class TestDeleteMixedState: + """Line 933 (approximate): _delete_mixed_state processes mixed segments.""" + + def test_delete_crossing_ins_boundary(self, temp_xml): + xml_path = temp_xml( + "" + "Hello " + '' + "world" + "" + ) + mgr = _make_manager(xml_path) + mgr.suggest_deletion("lo world") + text = _get_text_content(mgr) + assert "lo" not in text + assert "world" not in text + assert "Hel" in text + + +class TestReplaceSameContextInsAppendNoFirstChild: + """Lines 405-407: _replace_same_context appends when ins_elem has no firstChild.""" + + def test_replace_all_inside_ins_removes_then_appends(self, temp_xml): + # Two runs both inside ins, replace entire content. + xml_path = temp_xml( + '' + "XY" + ) + mgr = _make_manager(xml_path) + mgr.replace_text("XY", "NEW") + text = _get_text_content(mgr) + assert "NEW" in text