Skip to content

Commit ca2c22e

Browse files
authored
Merge pull request #230 from Maxteabag/worktree-fix-issue-229-copy-markup
fix: strip Rich markup from copied cells/rows
2 parents 73204ea + 9425eb9 commit ca2c22e

2 files changed

Lines changed: 116 additions & 5 deletions

File tree

‎sqlit/domains/results/ui/mixins/results.py‎

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,32 @@
44

55
from typing import Any
66

7+
from rich.errors import MarkupError
8+
from rich.text import Text
9+
710
from sqlit.shared.ui.protocols import ResultsMixinHost
811
from sqlit.shared.ui.widgets import SqlitDataTable
912

1013
MIN_TIMER_DELAY_S = 0.001
1114

1215

16+
def _strip_table_markup(table: Any, value: Any) -> Any:
17+
"""Strip Rich markup from a cell value when the table renders markup.
18+
19+
The results filter stores cells with Rich markup (e.g. `[bold #FFFF00]Ja[/]ne`)
20+
to highlight matches. Reading those cells back for clipboard or value-view
21+
purposes would otherwise leak the markup as literal text (issue #229).
22+
"""
23+
if not isinstance(value, str):
24+
return value
25+
if not getattr(table, "render_markup", False):
26+
return value
27+
try:
28+
return Text.from_markup(value).plain
29+
except MarkupError:
30+
return value
31+
32+
1333
class ResultsMixin:
1434
"""Mixin providing results handling functionality."""
1535

@@ -280,7 +300,7 @@ def action_view_cell_full(self: ResultsMixinHost) -> None:
280300
return
281301
try:
282302
_cursor_row, cursor_col = table.cursor_coordinate
283-
value = table.get_cell_at(table.cursor_coordinate)
303+
value = _strip_table_markup(table, table.get_cell_at(table.cursor_coordinate))
284304
except Exception:
285305
return
286306

@@ -429,7 +449,7 @@ def action_copy_cell(self: ResultsMixinHost) -> None:
429449
self.notify("No results", severity="warning")
430450
return
431451
try:
432-
value = table.get_cell_at(table.cursor_coordinate)
452+
value = _strip_table_markup(table, table.get_cell_at(table.cursor_coordinate))
433453
except Exception:
434454
return
435455
self._copy_text(str(value) if value is not None else "NULL")
@@ -503,7 +523,9 @@ def action_copy_row(self: ResultsMixinHost) -> None:
503523
self.notify("No results", severity="warning")
504524
return
505525
try:
506-
row_values = table.get_row_at(table.cursor_row)
526+
row_values = [
527+
_strip_table_markup(table, v) for v in table.get_row_at(table.cursor_row)
528+
]
507529
except Exception:
508530
return
509531

@@ -546,7 +568,7 @@ def action_ry_cell(self: ResultsMixinHost) -> None:
546568
self.notify("No results", severity="warning")
547569
return
548570
try:
549-
value = table.get_cell_at(table.cursor_coordinate)
571+
value = _strip_table_markup(table, table.get_cell_at(table.cursor_coordinate))
550572
except Exception:
551573
return
552574
self._copy_text(str(value) if value is not None else "NULL")
@@ -560,7 +582,9 @@ def action_ry_row(self: ResultsMixinHost) -> None:
560582
self.notify("No results", severity="warning")
561583
return
562584
try:
563-
row_values = table.get_row_at(table.cursor_row)
585+
row_values = [
586+
_strip_table_markup(table, v) for v in table.get_row_at(table.cursor_row)
587+
]
564588
except Exception:
565589
return
566590
text = self._format_tsv([], [tuple(row_values)])
Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
"""Regression tests for issue #229 - Rich markup leaking into copied cells.
2+
3+
When the results filter is active, cells are stored in the table with Rich
4+
markup (e.g. `[bold #FFFF00]Ja[/]ne`). Copy actions used to feed that markup
5+
straight to the clipboard. These tests drive `action_copy_cell` / _row through
6+
their public entry points against a fake table that mimics the filter state,
7+
so they fail without the fix and pass with it.
8+
"""
9+
10+
from __future__ import annotations
11+
12+
from typing import Any
13+
14+
import pytest
15+
16+
from sqlit.domains.results.ui.mixins.results import ResultsMixin
17+
18+
19+
class _FakeTable:
20+
def __init__(self, cells: list[tuple[str, ...]], render_markup: bool) -> None:
21+
self._cells = cells
22+
self.row_count = len(cells)
23+
self.render_markup = render_markup
24+
self.cursor_row = 0
25+
self.cursor_coordinate = (0, 0)
26+
27+
def get_cell_at(self, coord: Any) -> Any:
28+
if isinstance(coord, tuple):
29+
row, col = coord
30+
else:
31+
row, col = coord.row, coord.column
32+
return self._cells[row][col]
33+
34+
def get_row_at(self, row: int) -> list[Any]:
35+
return list(self._cells[row])
36+
37+
38+
class _FakeApp(ResultsMixin):
39+
"""Just enough harness to exercise the copy actions without Textual."""
40+
41+
def __init__(self, cells: list[tuple[str, ...]], *, render_markup: bool = True) -> None:
42+
self._table = _FakeTable(cells, render_markup=render_markup)
43+
self.clipboard_text: str | None = None
44+
45+
def _get_active_results_context(self) -> tuple[Any, list, list, bool]:
46+
return self._table, [], [], False
47+
48+
def _copy_text(self, text: str) -> bool: # type: ignore[override]
49+
self.clipboard_text = text
50+
return True
51+
52+
def _flash_table_yank(self, *_args: Any, **_kwargs: Any) -> None:
53+
pass
54+
55+
def notify(self, *_args: Any, **_kwargs: Any) -> None:
56+
pass
57+
58+
def _clear_leader_pending(self) -> None:
59+
pass
60+
61+
62+
@pytest.mark.parametrize(
63+
"action_name",
64+
["action_copy_cell", "action_ry_cell"],
65+
)
66+
def test_copy_cell_strips_filter_markup(action_name: str) -> None:
67+
app = _FakeApp([("[bold #FFFF00]Ja[/]ne",)])
68+
getattr(app, action_name)()
69+
assert app.clipboard_text == "Jane"
70+
71+
72+
@pytest.mark.parametrize(
73+
"action_name",
74+
["action_copy_row", "action_ry_row"],
75+
)
76+
def test_copy_row_strips_filter_markup(action_name: str) -> None:
77+
app = _FakeApp([("[bold #FFFF00]Ja[/]ne", "Doe")])
78+
getattr(app, action_name)()
79+
assert app.clipboard_text == "Jane\tDoe"
80+
81+
82+
def test_copy_cell_preserves_literal_brackets_when_not_rendering_markup() -> None:
83+
# When the table is in plain mode, cells are stored verbatim. We must NOT
84+
# treat brackets as markup, or legitimate data like "[bold]hi" gets eaten.
85+
app = _FakeApp([("[bold]hello",)], render_markup=False)
86+
app.action_copy_cell()
87+
assert app.clipboard_text == "[bold]hello"

0 commit comments

Comments
 (0)