Skip to content

Commit 8c9db94

Browse files
committed
style: add return type annotations and fix tautological condition
Add explicit return types to private helpers (_get_run_info, _build_cross_boundary_parts, _classify_segments, _find_ancestor). Fix tautological condition in intermediate node removal loop. Add return type to test helper _make_manager.
1 parent 3afcea4 commit 8c9db94

2 files changed

Lines changed: 11 additions & 13 deletions

File tree

‎docx_editor/track_changes.py‎

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,10 @@
77
from dataclasses import dataclass
88
from datetime import datetime
99
from typing import Literal
10+
from xml.dom.minidom import Element
1011

1112
from .exceptions import RevisionError, TextNotFoundError
12-
from .xml_editor import DocxXMLEditor, TextMapMatch, build_text_map, find_in_text_map
13+
from .xml_editor import DocxXMLEditor, TextMapMatch, TextPosition, build_text_map, find_in_text_map
1314

1415

1516
@dataclass
@@ -298,7 +299,7 @@ def suggest_deletion(self, text: str, occurrence: int = 0) -> int:
298299

299300
return -1
300301

301-
def _get_run_info(self, node):
302+
def _get_run_info(self, node) -> tuple[Element | None, str]:
302303
"""Get the parent w:r element and its rPr XML for a w:t node."""
303304
run = node.parentNode
304305
while run and run.nodeName != "w:r":
@@ -319,7 +320,7 @@ def _get_node_text(self, node) -> str:
319320
text += child.data
320321
return text
321322

322-
def _build_cross_boundary_parts(self, match: TextMapMatch):
323+
def _build_cross_boundary_parts(self, match: TextMapMatch) -> list[tuple[Element, str, str, str, str]]:
323324
"""Build per-node data for a cross-boundary match.
324325
325326
Returns list of (run, rPr_xml, before_text, matched_part, after_text) tuples,
@@ -354,7 +355,7 @@ def _build_cross_boundary_parts(self, match: TextMapMatch):
354355
result.append((info["run"], info["rPr_xml"], before, matched, after))
355356
return result
356357

357-
def _classify_segments(self, match: TextMapMatch):
358+
def _classify_segments(self, match: TextMapMatch) -> list[tuple[bool | None, list[TextPosition]]]:
358359
"""Group match positions into contiguous segments by revision context.
359360
360361
Returns list of (is_inside_ins, positions_list) tuples.
@@ -421,7 +422,7 @@ def _replace_same_context(self, match: TextMapMatch, replace_with: str) -> int:
421422
ins_wrapper_xml = f"<w:ins>{new_run_xml}</w:ins>"
422423
nodes = self.editor._parse_fragment(ins_wrapper_xml)
423424
for node in nodes:
424-
ins_parent.insertBefore(node, ins_next)
425+
ins_parent.insertBefore(node, ins_next) # type: ignore[arg-type]
425426
return -1
426427

427428
# Use first run's rPr for the insertion
@@ -525,7 +526,7 @@ def _replace_mixed_state(self, match: TextMapMatch, replace_with: str) -> int:
525526
# Place a marker before ref_node so we can find the insertion point
526527
# after deletion processing (which may remove ref_node).
527528
marker = self.editor.dom.createElement("w:_marker")
528-
ref_node.parentNode.insertBefore(marker, ref_node)
529+
ref_node.parentNode.insertBefore(marker, ref_node) # type: ignore[union-attr]
529530

530531
# Process each segment to delete/remove the matched text
531532
for is_inside_ins, positions in segments:
@@ -564,7 +565,7 @@ def _replace_mixed_state(self, match: TextMapMatch, replace_with: str) -> int:
564565
return int(node.getAttribute("w:id"))
565566
return -1
566567

567-
def _find_ancestor(self, node, tag_name: str):
568+
def _find_ancestor(self, node, tag_name: str) -> Element | None:
568569
"""Find the nearest ancestor with the given tag name."""
569570
parent = node.parentNode
570571
while parent:
@@ -658,12 +659,9 @@ def _remove_from_insertion(self, positions: list) -> None:
658659
else:
659660
self._remove_wt_and_maybe_run(last_node)
660661

661-
# Remove intermediate nodes
662+
# Remove intermediate nodes unconditionally (entire text is matched)
662663
for group in groups[1:-1]:
663-
node = group["node"]
664-
node_text = self._get_node_text(node)
665-
if not node_text or node_text == self._get_node_text(node):
666-
self._remove_wt_and_maybe_run(node)
664+
self._remove_wt_and_maybe_run(group["node"])
667665

668666
def _remove_wt_and_maybe_run(self, wt_node) -> None:
669667
"""Remove a w:t node, and its parent w:r if no w:t children remain."""

‎tests/test_review_fixes.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ def _create_xml(body_xml: str) -> Path:
2121
return _create_xml
2222

2323

24-
def _make_manager(xml_path):
24+
def _make_manager(xml_path) -> RevisionManager:
2525
editor = DocxXMLEditor(xml_path, rsid="00000000", author="Test Author")
2626
return RevisionManager(editor)
2727

0 commit comments

Comments
 (0)