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) <noreply@anthropic.com>
This commit is contained in:
1 parent
72f72b84cd
commit
97d19a7978
2 files changed
+58
-9
No files matched your search
@@ -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:
|
||||
|
||||
@@ -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):
|
||||
|
||||
Reference in new issue
Block a user