Skip to content

Commit c26cd97

Browse files
committed
refactor(teams): drop duplicate _escape_table_cell, use shared (#70)
teams/format_converter.py had a byte-identical private copy of shared/card_utils.escape_table_cell. Removed the copy, imported the shared function, renamed the 2 call sites. Also added direct unit tests for shared.card_utils.escape_table_cell + render_gfm_table — both were previously covered only transitively through adapter tests. First sub-PR of #70 (shared-helper consolidation). Scope check: most of #70's five planned helpers already live in shared/card_utils.py — only escape_table_cell (this PR) and button-style mapping (Teams + Discord, remaining work) are still duplicated per-adapter. https://claude.ai/code/session_01WhrgpELQJJSakBnwSNuwGJ
1 parent 1c5521a commit c26cd97

2 files changed

Lines changed: 52 additions & 7 deletions

File tree

src/chat_sdk/adapters/teams/format_converter.py

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,7 @@
2222
get_node_value,
2323
parse_markdown,
2424
)
25-
26-
27-
def _escape_table_cell(value: str) -> str:
28-
"""Escape pipe characters in table cells for GFM rendering."""
29-
return value.replace("\\", "\\\\").replace("|", "\\|").replace("\n", " ")
25+
from chat_sdk.shared.card_utils import escape_table_cell
3026

3127

3228
class TeamsFormatConverter(BaseFormatConverter):
@@ -194,11 +190,11 @@ def _table_to_gfm(self, node: Content) -> str:
194190

195191
lines: list[str] = []
196192
# Header row
197-
lines.append(f"| {' | '.join(_escape_table_cell(c) for c in rows[0])} |")
193+
lines.append(f"| {' | '.join(escape_table_cell(c) for c in rows[0])} |")
198194
# Separator
199195
separators = ["---"] * len(rows[0])
200196
lines.append(f"| {' | '.join(separators)} |")
201197
# Data rows
202198
for row in rows[1:]:
203-
lines.append(f"| {' | '.join(_escape_table_cell(c) for c in row)} |")
199+
lines.append(f"| {' | '.join(escape_table_cell(c) for c in row)} |")
204200
return "\n".join(lines)

tests/test_cards.py

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
is_card_element,
99
table_element_to_ascii,
1010
)
11+
from chat_sdk.shared.card_utils import escape_table_cell, render_gfm_table
1112

1213

1314
class TestIsCardElement:
@@ -159,3 +160,51 @@ def test_unknown_element(self):
159160
def test_button_element_returns_none(self):
160161
child = {"type": "button", "label": "Click me"}
161162
assert card_child_to_fallback_text(child) is None
163+
164+
165+
class TestEscapeTableCell:
166+
"""Tests for shared.card_utils.escape_table_cell."""
167+
168+
def test_plain_text_passthrough(self):
169+
assert escape_table_cell("hello world") == "hello world"
170+
171+
def test_pipe_escaped(self):
172+
assert escape_table_cell("a|b") == r"a\|b"
173+
174+
def test_backslash_doubled_before_pipe_escape(self):
175+
# Backslash must be doubled FIRST so that a literal `\|` in input
176+
# doesn't collide with the subsequent pipe-escape.
177+
assert escape_table_cell(r"a\b") == r"a\\b"
178+
assert escape_table_cell(r"a\|b") == r"a\\\|b"
179+
180+
def test_newline_collapsed_to_space(self):
181+
assert escape_table_cell("line1\nline2") == "line1 line2"
182+
183+
def test_multiple_substitutions(self):
184+
assert escape_table_cell("a|b\nc\\d") == r"a\|b c\\d"
185+
186+
def test_empty_string(self):
187+
assert escape_table_cell("") == ""
188+
189+
190+
class TestRenderGfmTable:
191+
"""Tests for shared.card_utils.render_gfm_table."""
192+
193+
def test_basic_table(self):
194+
lines = render_gfm_table(["h1", "h2"], [["a", "b"], ["c", "d"]])
195+
assert lines == [
196+
"| h1 | h2 |",
197+
"| --- | --- |",
198+
"| a | b |",
199+
"| c | d |",
200+
]
201+
202+
def test_cells_are_escaped(self):
203+
lines = render_gfm_table(["col"], [["pipe|inside"], ["has\nnewline"]])
204+
assert r"pipe\|inside" in lines[2]
205+
assert "has newline" in lines[3]
206+
207+
def test_empty_rows(self):
208+
# No data rows — only header + separator.
209+
lines = render_gfm_table(["only"], [])
210+
assert lines == ["| only |", "| --- |"]

0 commit comments

Comments
 (0)