From 97d19a7978ff084b3c20c596b96028f5a7634766 Mon Sep 17 00:00:00 2001 From: Jean-Pierre Martineau Date: Sat, 19 Sep 2026 17:17:31 -0400 Subject: [PATCH] The popup shows a mark, not the tags around one message_dialog draws the message it is handed and reads its spans to pick a border tone, so a markup string reaches it unparsed: the reader saw a literal [ok] and [/ok] around the checkmark, and a failed write would have been framed in the success colour, since a plain string has no spans to read a tone from. It is a styled Text now, the way every other caller writes one. Only the mark is styled. That is enough for the border to read the tone, and it leaves the path in body ink where it stays legible -- which is what the mark is for. The reason for the failure follows it muted, on its own line. The test asserts on the spans rather than on the words, because the bug was invisible to anything reading the text: the tags were *in* the string either way. Co-Authored-By: Claude Opus 5 (1M context) --- meshterm/ui/diagnostics.py | 28 +++++++++++++++++++-------- tests/test_diagnostics.py | 39 +++++++++++++++++++++++++++++++++++++- 2 files changed, 58 insertions(+), 9 deletions(-) diff --git a/meshterm/ui/diagnostics.py b/meshterm/ui/diagnostics.py index 8b81a23..2612f83 100644 --- a/meshterm/ui/diagnostics.py +++ b/meshterm/ui/diagnostics.py @@ -26,6 +26,8 @@ from __future__ import annotations from pathlib import Path from typing import TYPE_CHECKING +from rich.text import Text + from .about import AboutPage from .markdown import render_markdown @@ -117,23 +119,33 @@ class DiagnosticsPage(AboutPage): self._session.run_detached(run()) - def _write(self) -> str: - """Write the document, and say what happened in one marked-up line. + def _write(self) -> Text: + """Write the document, and say what happened as the popup's message. - The message carries its own mark — ``✓`` or ``✗`` — which is also what tells the - popup which border to take, so a failed write arrives red without anything here - naming a colour. + A :class:`~rich.text.Text` with real spans rather than a markup string: the dialog + draws the message as given and reads its *spans* to pick a border tone (see + ``session._message_border``), so markup left as text would both print its own tags + and leave a failure framed in the success colour. + + Only the **mark** is styled — ``✓`` or ``✗``, the app's own ok/err pair — which is + enough for the border to read the tone and leaves the path itself in body ink, + where it stays legible. That is what the mark is for. """ from ..tools.diagnostics import write_markdown + message = Text() try: written = write_markdown(self._source, self._save_path) except OSError as exc: # A read-only home, a full disk, a path that is not there. The page itself is # untouched and still readable, which is the fallback that matters. - reason = exc.strerror or str(exc) - return f"[err]✗[/err] could not save to {self._save_path}\n{reason}" - return f"[ok]✓[/ok] saved to\n{written}" + message.append("✗ ", style="err") + message.append(f"could not save to\n{self._save_path}\n\n") + message.append(exc.strerror or str(exc), style="muted") + return message + message.append("✓ ", style="ok") + message.append(f"saved to\n{written}") + return message async def open_diagnostics_page(ctx: AppContext, source: str, *, save_path: Path) -> None: diff --git a/tests/test_diagnostics.py b/tests/test_diagnostics.py index 9749f97..af77a1f 100644 --- a/tests/test_diagnostics.py +++ b/tests/test_diagnostics.py @@ -17,6 +17,7 @@ from pathlib import Path import pytest from rich.console import Console +from rich.text import Text from meshterm.context import AppContext from meshterm.core import hostinfo @@ -298,6 +299,26 @@ def test_a_save_that_fails_is_reported_not_raised(ctx: AppContext, tmp_path: Pat assert "Build" in _body(page) # the page itself is untouched +def test_the_popup_carries_its_tone_as_spans_not_as_markup(ctx: AppContext, tmp_path: Path) -> None: + """The message is a styled Text, so no tag is ever printed and the border reads right. + + A markup *string* reaches this dialog unparsed: the reader sees the literal tags and a + failure is framed in the success colour, because the border is picked from the + message's spans. The mark carries the tone; nothing else needs to. + """ + from meshterm.ui.diagnostics import SAVE_KEY + from meshterm.ui.tui.session import _message_border + + for target, expected in ((tmp_path / "ok.md", "accent"), (_blocked(tmp_path), "err")): + session = _Session(keep=True) + page = _page(ctx, session=session, save_path=target) + page.handle("text", SAVE_KEY) + message = session.shown[0] + assert isinstance(message, Text) + assert "[/" not in message.plain and "[ok]" not in message.plain + assert _message_border(message) == expected + + def test_a_held_save_key_does_not_stack_popups(ctx: AppContext, tmp_path: Path) -> None: """One save at a time: a second press while one is in flight is ignored.""" from meshterm.ui.diagnostics import SAVE_KEY @@ -494,6 +515,18 @@ def _console(width: int) -> Console: return Console(width=width, color_system=None, highlight=False, markup=False) +def _blocked(tmp_path: Path) -> Path: + """A destination no OS will accept: a "directory" that is really a file. + + Refused by both the mkdir and the write, everywhere, without needing permissions the + suite cannot count on. + """ + blocker = tmp_path / "blocker" + if not blocker.exists(): + blocker.write_text("not a directory", encoding="utf-8") + return blocker / "inside" / "out.md" + + def _strip(line: str) -> str: """One rendered line with its SGR runs removed.""" return re.sub(r"\x1b\[[0-9;]*m", "", line) @@ -508,11 +541,13 @@ class _Session: needs a save still in flight. """ - def __init__(self, *, defer: bool = False) -> None: + def __init__(self, *, defer: bool = False, keep: bool = False) -> None: """Start with nothing shown and nothing started.""" self.messages: list[str] = [] + self.shown: list[object] = [] self.started: list[object] = [] self._defer = defer + self._keep = keep def invalidate(self) -> None: """A repaint request, which a test has no screen to repaint.""" @@ -528,6 +563,8 @@ class _Session: async def message_dialog(self, message, *, title: str = "") -> None: """Record what the acknowledgement popup would have said.""" self.messages.append(str(message)) + if self._keep: + self.shown.append(message) def _page(ctx: AppContext, *, session: _Session | None = None, save_path: Path | None = None):