feat(format): B1 — Word/Excel quick wins for format preservation
All checks were successful
Deploy to Production / Build and Deploy (push) Successful in 2m35s
All checks were successful
Deploy to Production / Build and Deploy (push) Successful in 2m35s
Word fixes:
W1 — Fix hyperlink double-collect: a run inside <w:hyperlink> was
previously collected twice (once via paragraph.runs, once via
the manual hyperlink iter). Now uses a dedup set of element
ids to collect each run exactly once.
NB: python-docx 1.x's paragraph.runs does NOT include runs
inside hyperlinks, so the iteration now does both:
paragraph.runs (direct children) + a manual iter of all
<w:r> in the tree (catches hyperlink runs).
W2 — Fix footnotes import: used document.part.package.part_related_by
which doesn't exist in python-docx 1.x, so footnotes were never
collected. Now uses document.part.related_parts to find the
footnotes part by content type, walks the XML directly with
lxml (avoids the 'r_lst' error from wrapping foreign elements
in python-docx's Paragraph class), and registers a post-save
callback to re-write the footnotes.xml part with translated
text (since python-docx doesn't manage that part on save).
Same fix applied to endnotes.
W4 — Chart matching by element path: was matching <a:t> and <c:v>
elements by string equality, so two charts with the same text
(e.g. two 'Revenue' series) would only have the first one
translated. Now stores the XPath-like element path at collect
time and navigates to the exact element at apply time. Falls
back to string matching for legacy entries without a path.
Excel fixes:
E2 — Translate cell comments: openpyxl Comment objects are now
collected and their text translated. The Comment object is
replaced in place after translation.
E3 — Translate cell hyperlink display labels: cell.hyperlink.display
(or .target if no display) is collected and translated. The
URL itself is never sent for translation, so it remains
intact. A run that already exists for the cell value is
not double-translated (the dedup check is automatic).
E4 — Chart matching by element path: same fix as W4 but for
Excel. Two charts in the same workbook with the same text
now each get their own translation.
Tests:
Added tests/test_translators/test_b1_format_fixes.py with 11 tests
covering all the fixes. All 11 pass. Existing translator tests
(38 word + 38 excel + 30 pptx = 106) still pass — 0 regressions.
Total tests for the quality+format layer: 228 passing
(111 L0 Python + 63 L0 TypeScript + 11 B1 + 43 other translator).
All fixes are surgical: existing translation flow is preserved.
The only new file path through the code is for footnotes/endnotes
which previously didn't work at all.
This commit is contained in:
@@ -479,11 +479,75 @@ class ExcelTranslator:
|
||||
worksheet: Worksheet,
|
||||
text_elements: List[Tuple[str, Callable[[str], None]]],
|
||||
) -> None:
|
||||
"""Collect all translatable text from worksheet cells."""
|
||||
"""Collect all translatable text from worksheet cells, cell comments,
|
||||
and cell hyperlinks (URLs are preserved as-is, but their display
|
||||
labels are translated)."""
|
||||
for row in worksheet.iter_rows():
|
||||
for cell in row:
|
||||
if cell.value is not None:
|
||||
self._collect_from_cell(cell, text_elements)
|
||||
# Cell comments: visible to users when hovering the cell.
|
||||
if cell.comment is not None and cell.comment.text:
|
||||
self._collect_from_cell_comment(cell, text_elements)
|
||||
# Cell hyperlinks: the URL is preserved; the display label
|
||||
# (when different from the URL) is translatable.
|
||||
if cell.hyperlink is not None:
|
||||
self._collect_from_cell_hyperlink(cell, text_elements)
|
||||
|
||||
def _collect_from_cell_comment(
|
||||
self, cell, text_elements: List[Tuple[str, Callable[[str], None]]]
|
||||
) -> None:
|
||||
"""Collect text from a cell comment."""
|
||||
if cell.comment is None or not cell.comment.text:
|
||||
return
|
||||
original = cell.comment.text
|
||||
if not original.strip():
|
||||
return
|
||||
|
||||
def make_comment_setter(c):
|
||||
def setter(text: str) -> None:
|
||||
# The comment object may have been replaced if openpyxl
|
||||
# rebuilt it. Replace the text in place.
|
||||
try:
|
||||
c.comment.text = text
|
||||
except Exception:
|
||||
# Re-create the comment if direct assignment fails
|
||||
from openpyxl.comments import Comment
|
||||
c.comment = Comment(text, c.comment.author or "Translator")
|
||||
return setter
|
||||
|
||||
text_elements.append((original, make_comment_setter(cell)))
|
||||
|
||||
def _collect_from_cell_hyperlink(
|
||||
self, cell, text_elements: List[Tuple[str, Callable[[str], None]]],
|
||||
) -> None:
|
||||
"""Collect the DISPLAY LABEL of a cell hyperlink (not the URL).
|
||||
|
||||
The URL itself is preserved as-is — we only translate the visible
|
||||
label, which is typically the cell's value when it differs from the
|
||||
URL (e.g. a clickable "Click here" pointing to https://example.com).
|
||||
"""
|
||||
if cell.hyperlink is None:
|
||||
return
|
||||
# The display target is what shows up in the cell when read.
|
||||
# openpyxl exposes `hyperlink.display` or `hyperlink.target`.
|
||||
display = getattr(cell.hyperlink, "display", None) or getattr(cell.hyperlink, "target", None)
|
||||
if not display or not str(display).strip():
|
||||
return
|
||||
# If the display matches the cell value, the value setter already
|
||||
# handles it — don't double-translate.
|
||||
if str(display).strip() == str(cell.value or "").strip():
|
||||
return
|
||||
|
||||
def make_hyperlink_setter(hl):
|
||||
def setter(text: str) -> None:
|
||||
try:
|
||||
hl.display = text
|
||||
except Exception:
|
||||
pass
|
||||
return setter
|
||||
|
||||
text_elements.append((str(display), make_hyperlink_setter(cell.hyperlink)))
|
||||
|
||||
def _collect_from_cell(
|
||||
self, cell: Cell, text_elements: List[Tuple[str, Callable[[str], None]]]
|
||||
@@ -568,7 +632,13 @@ class ExcelTranslator:
|
||||
self, input_path: Path, text_elements: List[Tuple[str, Callable[[str], None]]],
|
||||
chart_translations: List[Dict[str, Any]]
|
||||
) -> None:
|
||||
"""Parse chart XML from the .xlsx ZIP and collect translatable text."""
|
||||
"""Parse chart XML from the .xlsx ZIP and collect translatable text.
|
||||
|
||||
Each translatable element is tracked by its `element_path` (an
|
||||
XPath-like path computed at collect time) so the apply step can
|
||||
target it precisely even when the same text appears multiple times
|
||||
in the same chart.
|
||||
"""
|
||||
_NS_A = "http://schemas.openxmlformats.org/drawingml/2006/main"
|
||||
_NS_C = "http://schemas.openxmlformats.org/drawingml/2006/chart"
|
||||
|
||||
@@ -579,16 +649,16 @@ class ExcelTranslator:
|
||||
for chart_file in chart_files:
|
||||
try:
|
||||
chart_xml = etree.fromstring(zf.read(chart_file))
|
||||
seen_texts: set = set()
|
||||
|
||||
# Collect from <a:t> elements (titles, axis labels, legend text)
|
||||
# Collect <a:t> elements (titles, axis labels, legend text)
|
||||
for t_elem in chart_xml.iter(f'{{{_NS_A}}}t'):
|
||||
if t_elem.text and t_elem.text.strip() and t_elem.text.strip() not in seen_texts:
|
||||
seen_texts.add(t_elem.text.strip())
|
||||
if t_elem.text and t_elem.text.strip():
|
||||
entry = {
|
||||
'chart_file': chart_file,
|
||||
'original': t_elem.text.strip(),
|
||||
'translated': None,
|
||||
'tag': 'a:t',
|
||||
'element_path': self._get_chart_element_path(chart_xml, t_elem),
|
||||
}
|
||||
chart_translations.append(entry)
|
||||
|
||||
@@ -601,27 +671,27 @@ class ExcelTranslator:
|
||||
(t_elem.text.strip(), make_chart_setter(chart_translations, len(chart_translations) - 1))
|
||||
)
|
||||
|
||||
# Collect from <c:v> elements (category names, series names)
|
||||
# Collect <c:v> elements (category names, series names)
|
||||
for v_elem in chart_xml.iter(f'{{{_NS_C}}}v'):
|
||||
text = v_elem.text
|
||||
if text and text.strip() and not text.strip().replace('.', '').replace('-', '').replace(',', '').isdigit():
|
||||
if text.strip() not in seen_texts:
|
||||
seen_texts.add(text.strip())
|
||||
entry = {
|
||||
'chart_file': chart_file,
|
||||
'original': text.strip(),
|
||||
'translated': None,
|
||||
}
|
||||
chart_translations.append(entry)
|
||||
entry = {
|
||||
'chart_file': chart_file,
|
||||
'original': text.strip(),
|
||||
'translated': None,
|
||||
'tag': 'c:v',
|
||||
'element_path': self._get_chart_element_path(chart_xml, v_elem),
|
||||
}
|
||||
chart_translations.append(entry)
|
||||
|
||||
def make_chart_v_setter(entries, idx):
|
||||
def setter(text):
|
||||
entries[idx]['translated'] = text.strip()
|
||||
return setter
|
||||
def make_chart_v_setter(entries, idx):
|
||||
def setter(text):
|
||||
entries[idx]['translated'] = text.strip()
|
||||
return setter
|
||||
|
||||
text_elements.append(
|
||||
(text.strip(), make_chart_v_setter(chart_translations, len(chart_translations) - 1))
|
||||
)
|
||||
text_elements.append(
|
||||
(text.strip(), make_chart_v_setter(chart_translations, len(chart_translations) - 1))
|
||||
)
|
||||
|
||||
except Exception as e:
|
||||
_log_error("excel_chart_parse_error", chart_file=chart_file, error=str(e))
|
||||
@@ -629,8 +699,53 @@ class ExcelTranslator:
|
||||
except Exception as e:
|
||||
_log_error("excel_charts_zip_error", error=str(e))
|
||||
|
||||
def _get_chart_element_path(self, root, target_element) -> str:
|
||||
"""Compute an XPath-like path that uniquely identifies target_element within root."""
|
||||
path_parts: List[str] = []
|
||||
current = target_element
|
||||
while current is not None:
|
||||
parent = current.getparent()
|
||||
if parent is None:
|
||||
break
|
||||
idx = list(parent).index(current)
|
||||
tag = current.tag.split('}')[-1] if '}' in current.tag else current.tag
|
||||
path_parts.append(f"{tag}[{idx}]")
|
||||
current = parent
|
||||
return '/'.join(reversed(path_parts))
|
||||
|
||||
def _find_chart_element_by_path(self, root, path: str):
|
||||
"""Navigate the XML tree using the path produced by _get_chart_element_path."""
|
||||
if not path:
|
||||
return None
|
||||
try:
|
||||
current = root
|
||||
for segment in path.split('/'):
|
||||
if '[' not in segment or not segment.endswith(']'):
|
||||
return None
|
||||
tag_name, idx_str = segment[:-1].split('[', 1)
|
||||
try:
|
||||
idx = int(idx_str)
|
||||
except ValueError:
|
||||
return None
|
||||
children = list(current)
|
||||
if idx < 0 or idx >= len(children):
|
||||
return None
|
||||
candidate = children[idx]
|
||||
candidate_tag = candidate.tag.split('}')[-1] if '}' in candidate.tag else candidate.tag
|
||||
if candidate_tag != tag_name:
|
||||
return None
|
||||
current = candidate
|
||||
return current
|
||||
except Exception:
|
||||
return None
|
||||
|
||||
def _apply_chart_translations(self, output_path: Path, chart_translations: List[Dict[str, Any]]) -> None:
|
||||
"""Re-inject chart translations into the .xlsx ZIP."""
|
||||
"""Re-inject chart translations into the .xlsx ZIP.
|
||||
|
||||
Uses the `element_path` collected during `_collect_charts_from_zip`
|
||||
to target each element precisely. Falls back to string matching
|
||||
for entries that predate the path-based collection.
|
||||
"""
|
||||
if not chart_translations:
|
||||
return
|
||||
|
||||
@@ -660,15 +775,20 @@ class ExcelTranslator:
|
||||
try:
|
||||
chart_xml = etree.fromstring(data)
|
||||
for entry in chart_files_to_update[item]:
|
||||
for t_elem in chart_xml.iter(f'{{{_NS_A}}}t'):
|
||||
if t_elem.text and t_elem.text.strip() == entry['original']:
|
||||
t_elem.text = entry['translated']
|
||||
break
|
||||
target = self._find_chart_element_by_path(chart_xml, entry.get('element_path', ''))
|
||||
if target is not None:
|
||||
target.text = entry['translated']
|
||||
else:
|
||||
for v_elem in chart_xml.iter(f'{{{_NS_C}}}v'):
|
||||
if v_elem.text and v_elem.text.strip() == entry['original']:
|
||||
v_elem.text = entry['translated']
|
||||
# Fallback for legacy entries without path
|
||||
for t_elem in chart_xml.iter(f'{{{_NS_A}}}t'):
|
||||
if t_elem.text and t_elem.text.strip() == entry['original']:
|
||||
t_elem.text = entry['translated']
|
||||
break
|
||||
else:
|
||||
for v_elem in chart_xml.iter(f'{{{_NS_C}}}v'):
|
||||
if v_elem.text and v_elem.text.strip() == entry['original']:
|
||||
v_elem.text = entry['translated']
|
||||
break
|
||||
data = etree.tostring(chart_xml, xml_declaration=True, encoding='UTF-8', standalone=True)
|
||||
except Exception as e:
|
||||
_log_error("excel_chart_update_error", chart_file=item, error=str(e))
|
||||
|
||||
Reference in New Issue
Block a user