The icon column is measured, not assumed to be two cells
🗑 is drawn in one cell where 💾, 🌍, ⏳ and 📱 are drawn in two, and a command row that wrote `icon + " "` started its label wherever its own icon ended. So Delete contact… sat a column left of Archive contact above it (JP). Ten of the app's lexicon icons are like this — 🗑 ✎ ⚙ ▶ ★ ↻ ↕ ⇄ ⌨ # — so any list mixing them with the other twenty-seven was out of line; the Contacts tail's 🗑 Purge against 📂 View archived was the same fault one screen over. Not an emoji-width entry. The classifier is clear that both authorities and the terminal agree 🗑 is one cell: it sits outside Emoji_Presentation and carries no VS16, so the font draws a one-cell text glyph and everyone is right. Forcing it into the wide set would reserve a cell nothing draws and pull the border in — the mistake 🛣 already cost the Trophy case. The measurement was never wrong; the layout assumed it away. The trace and TX sweep screens had solved this properly and privately, each with its own copy: declare the marks a list can draw, measure the widest, pad every mark out to it. That is now menus.icon_lane/icon_mark, THE icon column, and those two delegate rather than duplicate. marked_label takes the lane so a mixed list can pass it, defaulting to measuring the single icon — which stays right for a list whose rows all lead with the same mark, and is precisely what misaligns one that doesn't. The node page measures its column once over every action it can offer, so the group ends aligned however many of archive/restore/delete a given contact earns. PicoCalc is unaffected and stays that way by construction: it draws no icon lane at all, so the column measures zero, the separator goes with it, and the tint lands on the label as before. The tests assert in display cells rather than characters. That distinction is the bug — 🗑 plus two spaces and 📂 plus one are three cells and different string lengths, and an index-based assertion would have called the fix broken and the break fixed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
424c33ac93
commit
026e78078e
6 files changed
+175
-41
No files matched your search
@@ -40,7 +40,7 @@ from typing import TYPE_CHECKING
|
||||
|
||||
from .archived_screen import open_archived
|
||||
from .contactlist import ContactListScreen, ContactRow
|
||||
from .menus import marked_label
|
||||
from .menus import icon_lane, marked_label
|
||||
from .purge_screen import purge_contacts
|
||||
from .tui import Choice, Separator
|
||||
from .tui.screen import CANCEL
|
||||
@@ -59,6 +59,11 @@ YOU = ("you",)
|
||||
_PURGE = ("purge",)
|
||||
_ARCHIVED = ("archived",)
|
||||
|
||||
#: Every mark the tail's maintenance rows lead with — the set the icon column is measured
|
||||
#: over (see :func:`~meshterm.ui.menus.icon_lane`). Declared rather than inferred so the
|
||||
#: column is one width for the whole tail whichever of the two rows a given visit draws.
|
||||
_TAIL_ICONS = ("🗑", "📂")
|
||||
|
||||
|
||||
class ContactsScreen(ContactListScreen):
|
||||
"""A full-screen, Ctrl+arrow-sortable list of this node and its known contacts.
|
||||
@@ -114,17 +119,23 @@ class ContactsScreen(ContactListScreen):
|
||||
# the way in to what the sweep has already taken. The two sit together because they
|
||||
# are the two halves of one idea, and the archived row is *under* the purge for the
|
||||
# same reason: it is where the purge's output went.
|
||||
# One icon column for both rows: 🗑 draws a single cell where 📂 draws two, so an
|
||||
# unpadded mark would start the purge row's label a column left of the other's.
|
||||
lane = icon_lane(_TAIL_ICONS)
|
||||
tail: list = []
|
||||
if contacts:
|
||||
tail = [
|
||||
Separator(" "),
|
||||
Choice(title=marked_label("🗑", "Purge contacts…", "err"), value=_PURGE),
|
||||
Choice(
|
||||
title=marked_label("🗑", "Purge contacts…", "err", lane=lane),
|
||||
value=_PURGE,
|
||||
),
|
||||
]
|
||||
if archived:
|
||||
tail = tail or [Separator(" ")]
|
||||
# The tally rides the row muted, as a status atom rather than as part of the
|
||||
# verb: it is what the row leads to, not what the row does.
|
||||
label = marked_label("📂", "View archived contacts", "")
|
||||
label = marked_label("📂", "View archived contacts", "", lane=lane)
|
||||
label.append(f" · {archived}", style="muted")
|
||||
tail.append(Choice(title=label, value=_ARCHIVED))
|
||||
super().__init__(
|
||||
|
||||
+57
-3
@@ -85,7 +85,55 @@ def command_icon(icon: str) -> str:
|
||||
return glyph(icon) if get_platform().menu_icons else ""
|
||||
|
||||
|
||||
def marked_label(icon: str, label: str, style: str) -> Text:
|
||||
def icon_lane(icons: Iterable[str]) -> int:
|
||||
"""THE icon column's width in cells: the widest mark among ``icons`` on this platform.
|
||||
|
||||
A list's icons are **not all the same width**, and the terminal is the authority on
|
||||
which are which: of the app's lexicon, ten (``🗑 ✎ ⚙ ▶ ★ ↻ ↕ ⇄ ⌨ #``) draw one cell
|
||||
and the rest two. A row that simply wrote ``icon + " "`` therefore started its label a
|
||||
column left of its two-cell siblings — which is what put ``🗑 Delete contact…`` out of
|
||||
line under ``💾 Archive contact`` on the node page (JP, 2026-09-01). So a list declares
|
||||
the icons it uses, measures the column once, and pads every mark out to it.
|
||||
|
||||
Measured rather than hard-coded because the marks themselves change with the platform:
|
||||
the PicoCalc draws no icon lane on a command row at all (see :func:`command_icon`), so
|
||||
the column measures zero and the labels take back the cells — the same alignment rule
|
||||
wherever the icons land.
|
||||
|
||||
Args:
|
||||
icons: Every mark the list's rows can lead with, as written on the regular platform.
|
||||
|
||||
Returns:
|
||||
The column width in cells, or ``0`` where this platform draws no icons (and for an
|
||||
empty set, which is the same thing to a caller).
|
||||
"""
|
||||
return max((cell_len(command_icon(icon)) for icon in icons if icon), default=0)
|
||||
|
||||
|
||||
def icon_mark(icon: str, style: str, lane: int) -> Text:
|
||||
"""One row's mark, tinted, padded to ``lane`` cells, its trailing space included.
|
||||
|
||||
The single place a command row's icon column is written, so a mark one cell narrower
|
||||
than its neighbours cannot shift the label after it. An emptied lane (``lane`` of
|
||||
``0``) appends nothing at all, separator included — the cells belong to the label.
|
||||
|
||||
Args:
|
||||
icon: The row's icon, as written on the regular platform.
|
||||
style: The theme style the mark is drawn in.
|
||||
lane: The column width from :func:`icon_lane`.
|
||||
|
||||
Returns:
|
||||
The padded mark, or empty :class:`~rich.text.Text` where there is no lane.
|
||||
"""
|
||||
mark = command_icon(icon) if icon else ""
|
||||
if not lane or not mark:
|
||||
return Text()
|
||||
text = Text(mark, style=style)
|
||||
text.append(" " * (lane - cell_len(mark) + 1))
|
||||
return text
|
||||
|
||||
|
||||
def marked_label(icon: str, label: str, style: str, *, lane: Optional[int] = None) -> Text:
|
||||
"""A command row whose icon carries a tint — the tint moving to the label if it goes.
|
||||
|
||||
The app marks a destructive command by tinting its icon, not its words (``🗑`` in
|
||||
@@ -98,12 +146,18 @@ def marked_label(icon: str, label: str, style: str) -> Text:
|
||||
icon: The row's icon, as written on the regular platform.
|
||||
label: The row's words, with no icon and no leading space.
|
||||
style: The theme style the mark (or, iconless, the label) is drawn in.
|
||||
lane: The icon column's width in cells, from :func:`icon_lane` over every icon the
|
||||
surrounding list uses. **Pass it whenever the list mixes icon widths**, or the
|
||||
narrow ones start their labels a column early. ``None`` (the default) measures
|
||||
this icon alone, which is right for a list whose rows all lead with the same
|
||||
mark, and for the lone action row.
|
||||
|
||||
Returns:
|
||||
The composed row label.
|
||||
"""
|
||||
mark = command_icon(icon)
|
||||
return Text.assemble((f"{mark} ", style), label) if mark else Text(label, style=style)
|
||||
width = icon_lane((icon,)) if lane is None else lane
|
||||
mark = icon_mark(icon, style, width)
|
||||
return Text.assemble(mark, label) if mark.plain else Text(label, style=style)
|
||||
|
||||
|
||||
def command_label(label: LabelT) -> LabelT:
|
||||
|
||||
@@ -88,7 +88,7 @@ from ..core.geo import EARTH_RADIUS_KM, usable_fix
|
||||
from ..core.models import NODE_TYPE_LABELS, Contact, utcnow
|
||||
from ..platforms import Platform, on_platform
|
||||
from .mapcanvas import RGB
|
||||
from .menus import command_icon
|
||||
from . import menus
|
||||
from .minimap import MiniMap
|
||||
from .pathgraph import (
|
||||
DST_NODE,
|
||||
@@ -447,6 +447,13 @@ class NodeDetailScreen(Screen):
|
||||
self._routes = routes
|
||||
self._info_actions = info_actions or []
|
||||
self._trace_action = trace_action
|
||||
#: The action rows' icon column, in cells — measured once over every mark this page
|
||||
#: can draw, so a one-cell ``🗑`` pads out to its two-cell siblings and every label
|
||||
#: starts in the same column. Zero on a platform that draws no icon lane at all.
|
||||
self._icon_lane = menus.icon_lane(
|
||||
action.glyph
|
||||
for action in (*self._info_actions, *( [trace_action] if trace_action else [] ))
|
||||
)
|
||||
self._tab_index = 0
|
||||
self._row_index = 0
|
||||
#: The highlighted route on the Routes tab (drives the graph); tracks the cursor as
|
||||
@@ -1043,10 +1050,13 @@ class NodeDetailScreen(Screen):
|
||||
# destructive row announces itself — so where the lane goes, the tint lands on the
|
||||
# label instead. Same move as :func:`~meshterm.ui.menus.marked_label` makes for a
|
||||
# menu row, for the same reason: a delete must not read like any other action.
|
||||
mark = command_icon(action.glyph) if action.glyph else ""
|
||||
if mark:
|
||||
text.append(f"{mark} ", style=action.glyph_style)
|
||||
text.append(action.label, style="" if mark else action.glyph_style)
|
||||
#
|
||||
# Padded to the page's own lane rather than written as `icon + " "`: this page mixes
|
||||
# a one-cell 🗑 with two-cell siblings, and the unpadded form started its label a
|
||||
# column early (JP, 2026-09-01).
|
||||
mark = menus.icon_mark(action.glyph, action.glyph_style, self._icon_lane)
|
||||
text.append_text(mark)
|
||||
text.append(action.label, style="" if mark.plain else action.glyph_style)
|
||||
if selected:
|
||||
text.style = "cursor"
|
||||
text.no_wrap = True
|
||||
|
||||
@@ -96,7 +96,7 @@ from ..services import trace_runner
|
||||
from ..services.records import first_repeated_edge
|
||||
from ..services.topology import render_forced_spec
|
||||
from .braillechart import meter
|
||||
from .menus import command_icon, marked_label, section_heading
|
||||
from .menus import icon_lane, icon_mark, marked_label, section_heading
|
||||
from .theme import snr_style
|
||||
from .tui.render import render_lines, render_to_ansi
|
||||
from .tui.screen import ListWindow, Screen
|
||||
@@ -172,28 +172,13 @@ _ACTION_ICONS = ("✎", "⇄", "⚡", "⚙", "#", "▶")
|
||||
|
||||
|
||||
def _icon_lane() -> int:
|
||||
"""The action rows' icon column, in cells: the widest mark on this platform.
|
||||
|
||||
Measured rather than hard-coded because the marks themselves change with the
|
||||
platform: PicoCalc draws no icon lane on a command row at all
|
||||
(:func:`~meshterm.ui.menus.command_icon`), so the column measures zero and the labels
|
||||
take the cells — the same alignment rule wherever the icons land.
|
||||
"""
|
||||
return max(cell_len(command_icon(icon)) for icon in _ACTION_ICONS)
|
||||
"""The action rows' icon column, in cells — this screen's own marks, measured once."""
|
||||
return icon_lane(_ACTION_ICONS)
|
||||
|
||||
|
||||
def _icon(text: Text, icon: str, style: str) -> None:
|
||||
"""Append an action row's mark in the icon column, its trailing space included.
|
||||
|
||||
A one-cell mark pads out to :func:`_icon_lane` so every label starts in the same
|
||||
column; an emptied lane (see there) appends nothing, separator included.
|
||||
"""
|
||||
lane = _icon_lane()
|
||||
if not lane:
|
||||
return
|
||||
mark = command_icon(icon)
|
||||
text.append(mark, style=style)
|
||||
text.append(" " * (lane - cell_len(mark) + 1))
|
||||
"""Append an action row's mark in the icon column, its trailing space included."""
|
||||
text.append_text(icon_mark(icon, style, _icon_lane()))
|
||||
|
||||
|
||||
def _lane_lines(label: str, value: list[Text], width: int) -> list[str]:
|
||||
|
||||
@@ -33,7 +33,6 @@ import asyncio
|
||||
import re
|
||||
from typing import TYPE_CHECKING, Any, Callable, Optional
|
||||
|
||||
from rich.cells import cell_len
|
||||
from rich.console import Group, RenderableType
|
||||
from rich.table import Table
|
||||
from rich.text import Text
|
||||
@@ -42,7 +41,7 @@ from ..core.connection import REMOTE_TX_MAX, REMOTE_TX_MIN
|
||||
from ..core.models import Contact, LoginResult, TraceResult, TxLevelResult, TxOptResult
|
||||
from ..services import trace_runner, tx_optimizer
|
||||
from ..services.topology import build_topology, render_custom_spec
|
||||
from .menus import command_icon
|
||||
from .menus import icon_lane, icon_mark
|
||||
from .pathline import SELF_GLYPH, PathHop, PathLine, cut_to, path_line
|
||||
from .theme import name_style, snr_style
|
||||
from .trace_screen import TracingDialog, _collapse_trace_width, snr_bar
|
||||
@@ -76,18 +75,13 @@ _ACTION_ICONS = ("✎", "⚙", "#", "▶", "★")
|
||||
|
||||
|
||||
def _icon_lane() -> int:
|
||||
"""The action rows' icon column, in cells: the widest mark on this platform."""
|
||||
return max(cell_len(command_icon(icon)) for icon in _ACTION_ICONS)
|
||||
"""The action rows' icon column, in cells — this screen's own marks, measured once."""
|
||||
return icon_lane(_ACTION_ICONS)
|
||||
|
||||
|
||||
def _icon(text: Text, icon: str, style: str) -> None:
|
||||
"""Append an action row's mark in the icon column, its trailing space included."""
|
||||
lane = _icon_lane()
|
||||
if not lane:
|
||||
return
|
||||
mark = command_icon(icon)
|
||||
text.append(mark, style=style)
|
||||
text.append(" " * (lane - cell_len(mark) + 1))
|
||||
text.append_text(icon_mark(icon, style, _icon_lane()))
|
||||
|
||||
|
||||
class TxSweepScreen(Screen):
|
||||
|
||||
@@ -7,6 +7,7 @@ regression here fails once rather than on every screen.
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
from rich.text import Text
|
||||
|
||||
from meshterm.ui.menus import (
|
||||
@@ -15,7 +16,10 @@ from meshterm.ui.menus import (
|
||||
column_header,
|
||||
exit_rows,
|
||||
fit_cells,
|
||||
icon_lane,
|
||||
icon_mark,
|
||||
lane_header,
|
||||
marked_label,
|
||||
menu_rows,
|
||||
section_heading,
|
||||
)
|
||||
@@ -168,3 +172,79 @@ def test_main_menu_sections_answer_the_menus_own_question() -> None:
|
||||
# No section is so big it stops being a grouping (the old Mesh bucket held seven of
|
||||
# nineteen rows), and none is a bucket of one.
|
||||
assert all(2 <= len(names) <= 5 for names in sections.values())
|
||||
|
||||
|
||||
#: Two lexicon icons the terminal genuinely draws in different widths — the whole reason
|
||||
#: the icon column has to be measured. Of the app's icons, ten (``🗑 ✎ ⚙ ▶ ★ ↻ ↕ ⇄ ⌨ #``)
|
||||
#: draw one cell and the rest two.
|
||||
_NARROW, _WIDE = "🗑", "📂"
|
||||
|
||||
|
||||
def test_the_icon_column_is_the_widest_mark_a_list_can_draw() -> None:
|
||||
"""A list declares its icons and the column measures them; an empty set has no column."""
|
||||
from rich.cells import cell_len
|
||||
|
||||
assert cell_len(_NARROW) == 1 and cell_len(_WIDE) == 2, "the premise of this whole lane"
|
||||
assert icon_lane((_NARROW,)) == 1 # a list of only narrow marks keeps a narrow column
|
||||
assert icon_lane((_WIDE,)) == 2
|
||||
assert icon_lane((_NARROW, _WIDE)) == 2 # a mixed list pads up to its widest
|
||||
assert icon_lane(()) == 0
|
||||
assert icon_lane(("",)) == 0 # a row with no icon contributes no column
|
||||
|
||||
|
||||
def test_a_narrow_mark_pads_out_to_its_wider_siblings() -> None:
|
||||
"""THE fix for a row whose label started a column early (JP, 2026-09-01).
|
||||
|
||||
``🗑 Delete contact…`` sat one cell left of ``💾 Archive contact`` because the row wrote
|
||||
``icon + " "`` and the terminal draws the two icons in different widths. Both marks now
|
||||
occupy the same number of *cells*, which is the only measure the terminal cares about —
|
||||
the character counts still differ, and asserting on those is what hid this.
|
||||
"""
|
||||
from rich.cells import cell_len
|
||||
|
||||
lane = icon_lane((_NARROW, _WIDE))
|
||||
narrow = icon_mark(_NARROW, "err", lane)
|
||||
wide = icon_mark(_WIDE, "", lane)
|
||||
assert cell_len(narrow.plain) == cell_len(wide.plain) == lane + 1
|
||||
assert len(narrow.plain) != len(wide.plain), "cells, not characters, are the measure"
|
||||
|
||||
|
||||
def test_marked_label_lines_up_a_mixed_list_when_told_its_lane() -> None:
|
||||
"""Labels start in the same cell once the caller passes the list's measured column."""
|
||||
from rich.cells import cell_len
|
||||
|
||||
lane = icon_lane((_NARROW, _WIDE))
|
||||
rows = [
|
||||
marked_label(_NARROW, "Purge contacts…", "err", lane=lane),
|
||||
marked_label(_WIDE, "View archived contacts", "", lane=lane),
|
||||
]
|
||||
starts = {cell_len(row.plain[: row.plain.index(word)])
|
||||
for row, word in zip(rows, ("Purge", "View"))}
|
||||
assert starts == {lane + 1}
|
||||
|
||||
# Without the lane each mark measures itself, which is right for a list whose rows all
|
||||
# lead with the same icon — and is exactly what misaligns a mixed one.
|
||||
solo = [marked_label(_NARROW, "A", "err"), marked_label(_WIDE, "B", "")]
|
||||
assert len({cell_len(row.plain[: row.plain.index(letter)])
|
||||
for row, letter in zip(solo, ("A", "B"))}) == 2
|
||||
|
||||
|
||||
def test_an_iconless_platform_collapses_the_column_and_keeps_the_tint(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""No icon lane means no cells spent and no separator — and the tint moves to the words.
|
||||
|
||||
A destructive row announces itself by its red mark; drop the mark and the claim has to
|
||||
land somewhere, or a delete reads like any other action.
|
||||
"""
|
||||
from meshterm.platforms import PICOCALC, REGULAR, set_platform
|
||||
|
||||
set_platform(PICOCALC)
|
||||
try:
|
||||
assert icon_lane((_NARROW, _WIDE)) == 0
|
||||
assert icon_mark(_NARROW, "err", 0).plain == ""
|
||||
row = marked_label(_NARROW, "Delete contact…", "err", lane=0)
|
||||
assert row.plain == "Delete contact…"
|
||||
assert any(span.style == "err" for span in row.spans) or row.style == "err"
|
||||
finally:
|
||||
set_platform(REGULAR)
|
||||
Reference in new issue
Block a user