From 20040621c992bd0cc2c077a15dd6f4602d207831 Mon Sep 17 00:00:00 2001 From: Peter Adams <18162810+Maxteabag@users.noreply.github.com> Date: Mon, 25 May 2026 11:54:05 +0200 Subject: [PATCH 1/2] fix: strip Rich markup from copied cells/rows (#229) --- sqlit/domains/results/ui/mixins/results.py | 34 ++++++++++++++++++---- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/sqlit/domains/results/ui/mixins/results.py b/sqlit/domains/results/ui/mixins/results.py index 1f5bf106..0081f587 100644 --- a/sqlit/domains/results/ui/mixins/results.py +++ b/sqlit/domains/results/ui/mixins/results.py @@ -4,12 +4,32 @@ from typing import Any +from rich.errors import MarkupError +from rich.text import Text + from sqlit.shared.ui.protocols import ResultsMixinHost from sqlit.shared.ui.widgets import SqlitDataTable MIN_TIMER_DELAY_S = 0.001 +def _strip_table_markup(table: SqlitDataTable | None, value: Any) -> Any: + """Strip Rich markup from a cell value when the table renders markup. + + The results filter stores cells with Rich markup (e.g. `[bold #FFFF00]Ja[/]ne`) + to highlight matches. Reading those cells back for clipboard or value-view + purposes would otherwise leak the markup as literal text (issue #229). + """ + if not isinstance(value, str): + return value + if not getattr(table, "render_markup", False): + return value + try: + return Text.from_markup(value).plain + except MarkupError: + return value + + class ResultsMixin: """Mixin providing results handling functionality.""" @@ -280,7 +300,7 @@ def action_view_cell_full(self: ResultsMixinHost) -> None: return try: _cursor_row, cursor_col = table.cursor_coordinate - value = table.get_cell_at(table.cursor_coordinate) + value = _strip_table_markup(table, table.get_cell_at(table.cursor_coordinate)) except Exception: return @@ -429,7 +449,7 @@ def action_copy_cell(self: ResultsMixinHost) -> None: self.notify("No results", severity="warning") return try: - value = table.get_cell_at(table.cursor_coordinate) + value = _strip_table_markup(table, table.get_cell_at(table.cursor_coordinate)) except Exception: return self._copy_text(str(value) if value is not None else "NULL") @@ -503,7 +523,9 @@ def action_copy_row(self: ResultsMixinHost) -> None: self.notify("No results", severity="warning") return try: - row_values = table.get_row_at(table.cursor_row) + row_values = [ + _strip_table_markup(table, v) for v in table.get_row_at(table.cursor_row) + ] except Exception: return @@ -546,7 +568,7 @@ def action_ry_cell(self: ResultsMixinHost) -> None: self.notify("No results", severity="warning") return try: - value = table.get_cell_at(table.cursor_coordinate) + value = _strip_table_markup(table, table.get_cell_at(table.cursor_coordinate)) except Exception: return self._copy_text(str(value) if value is not None else "NULL") @@ -560,7 +582,9 @@ def action_ry_row(self: ResultsMixinHost) -> None: self.notify("No results", severity="warning") return try: - row_values = table.get_row_at(table.cursor_row) + row_values = [ + _strip_table_markup(table, v) for v in table.get_row_at(table.cursor_row) + ] except Exception: return text = self._format_tsv([], [tuple(row_values)]) From 9425eb943fa18fdbc314746ba05dcf2e0b23f440 Mon Sep 17 00:00:00 2001 From: Peter Adams <18162810+Maxteabag@users.noreply.github.com> Date: Mon, 25 May 2026 11:58:48 +0200 Subject: [PATCH 2/2] test: regression for issue #229 --- sqlit/domains/results/ui/mixins/results.py | 2 +- tests/unit/test_results_copy_markup.py | 87 ++++++++++++++++++++++ 2 files changed, 88 insertions(+), 1 deletion(-) create mode 100644 tests/unit/test_results_copy_markup.py diff --git a/sqlit/domains/results/ui/mixins/results.py b/sqlit/domains/results/ui/mixins/results.py index 0081f587..a9be0b7a 100644 --- a/sqlit/domains/results/ui/mixins/results.py +++ b/sqlit/domains/results/ui/mixins/results.py @@ -13,7 +13,7 @@ MIN_TIMER_DELAY_S = 0.001 -def _strip_table_markup(table: SqlitDataTable | None, value: Any) -> Any: +def _strip_table_markup(table: Any, value: Any) -> Any: """Strip Rich markup from a cell value when the table renders markup. The results filter stores cells with Rich markup (e.g. `[bold #FFFF00]Ja[/]ne`) diff --git a/tests/unit/test_results_copy_markup.py b/tests/unit/test_results_copy_markup.py new file mode 100644 index 00000000..8832a51b --- /dev/null +++ b/tests/unit/test_results_copy_markup.py @@ -0,0 +1,87 @@ +"""Regression tests for issue #229 - Rich markup leaking into copied cells. + +When the results filter is active, cells are stored in the table with Rich +markup (e.g. `[bold #FFFF00]Ja[/]ne`). Copy actions used to feed that markup +straight to the clipboard. These tests drive `action_copy_cell` / _row through +their public entry points against a fake table that mimics the filter state, +so they fail without the fix and pass with it. +""" + +from __future__ import annotations + +from typing import Any + +import pytest + +from sqlit.domains.results.ui.mixins.results import ResultsMixin + + +class _FakeTable: + def __init__(self, cells: list[tuple[str, ...]], render_markup: bool) -> None: + self._cells = cells + self.row_count = len(cells) + self.render_markup = render_markup + self.cursor_row = 0 + self.cursor_coordinate = (0, 0) + + def get_cell_at(self, coord: Any) -> Any: + if isinstance(coord, tuple): + row, col = coord + else: + row, col = coord.row, coord.column + return self._cells[row][col] + + def get_row_at(self, row: int) -> list[Any]: + return list(self._cells[row]) + + +class _FakeApp(ResultsMixin): + """Just enough harness to exercise the copy actions without Textual.""" + + def __init__(self, cells: list[tuple[str, ...]], *, render_markup: bool = True) -> None: + self._table = _FakeTable(cells, render_markup=render_markup) + self.clipboard_text: str | None = None + + def _get_active_results_context(self) -> tuple[Any, list, list, bool]: + return self._table, [], [], False + + def _copy_text(self, text: str) -> bool: # type: ignore[override] + self.clipboard_text = text + return True + + def _flash_table_yank(self, *_args: Any, **_kwargs: Any) -> None: + pass + + def notify(self, *_args: Any, **_kwargs: Any) -> None: + pass + + def _clear_leader_pending(self) -> None: + pass + + +@pytest.mark.parametrize( + "action_name", + ["action_copy_cell", "action_ry_cell"], +) +def test_copy_cell_strips_filter_markup(action_name: str) -> None: + app = _FakeApp([("[bold #FFFF00]Ja[/]ne",)]) + getattr(app, action_name)() + assert app.clipboard_text == "Jane" + + +@pytest.mark.parametrize( + "action_name", + ["action_copy_row", "action_ry_row"], +) +def test_copy_row_strips_filter_markup(action_name: str) -> None: + app = _FakeApp([("[bold #FFFF00]Ja[/]ne", "Doe")]) + getattr(app, action_name)() + assert app.clipboard_text == "Jane\tDoe" + + +def test_copy_cell_preserves_literal_brackets_when_not_rendering_markup() -> None: + # When the table is in plain mode, cells are stored verbatim. We must NOT + # treat brackets as markup, or legitimate data like "[bold]hi" gets eaten. + app = _FakeApp([("[bold]hello",)], render_markup=False) + app.action_copy_cell() + assert app.clipboard_text == "[bold]hello"