From d19e2ec7124eb2ab2f0e20a4ddccaee18e06e9dd Mon Sep 17 00:00:00 2001 From: Jean-Pierre Martineau Date: Wed, 9 Sep 2026 00:48:07 -0400 Subject: [PATCH] Channels goes through the shared row builders it was working around MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It was never in the dual-platform gallery, which is why none of this was caught. Adding it first, then fixing what it showed. The column header was a hand-built f-string with a five-cell indent and no idea of the render width: at 53 columns it ran to 54 and wrapped, costing a content row out of twenty-six and drawing the pinned landmark twice, and its indent assumed a two-cell channel glyph where the console draws one. It goes through column_header now, which measures the glyph and sheds ACTIVITY before it wraps. The command rows hand-padded their icons, so the one-cell marks (↕ ✎ πŸ—‘) started their labels a column left of the two-cell ones - the Organize row had a two-space hack and a four-line comment about exactly this. They share a measured icon lane now, and the clear row tints its mark rather than its words. The empty message count was Β·, which in that same row is also the status-atom separator and what the console folds πŸ”• to: a muted channel with no messages drew it twice, two cells apart, meaning different things. It is β—‹, the app's empty mark. Also: the detail title was a 42-cell parenthesised blob that filled the PicoCalc's whole title bar, leaving nowhere to say Esc leaves - it is "Channel β€” name", with the openness and hash now leading a vital-signs line that sheds atoms rather than wrapping. The manager says "Enter open", which is what its rows do. The slot count chains with Β· rather than an em dash. And the four Add rows are replaced by one empty state when there is no free slot to add into, instead of all four dead-ending in the same refusal. Co-Authored-By: Claude Opus 5 (1M context) --- meshterm/ui/channels.py | 201 +++++++++++++++++++++++++++++----------- tests/test_channels.py | 41 +++++++- tests/test_gallery.py | 75 +++++++++++++++ 3 files changed, 262 insertions(+), 55 deletions(-) diff --git a/meshterm/ui/channels.py b/meshterm/ui/channels.py index 573d9cd..74d9797 100644 --- a/meshterm/ui/channels.py +++ b/meshterm/ui/channels.py @@ -40,6 +40,7 @@ from dataclasses import dataclass from functools import lru_cache from typing import TYPE_CHECKING +from rich.cells import cell_len from rich.console import Group from rich.text import Text @@ -62,9 +63,20 @@ from ..core.channels import ( from ..core.connection import Device from ..core.models import Conversation from ..persistence.repository import ACTIVITY_DRAWN_BUCKETS +from ..platforms import get_platform from .braillechart import activity_peak, activity_sparkline -from .menus import command_label, fit_cells, menu_rows, run_steps, section_heading +from .menus import ( + Lane, + column_header, + fit_cells, + icon_lane, + marked_label, + menu_rows, + run_steps, + section_heading, +) from .qr import share_popup +from .theme import glyph from .tui import CANCEL, Choice, SelectScreen, Separator from .widgets import _age_seconds, _format_age, channel_glyph, format_ago @@ -72,6 +84,9 @@ if TYPE_CHECKING: from ..context import AppContext from ..persistence.repository import ChannelStats +#: The manager's footer sentence: navigation, then the filter, then the action, Esc last. +_MANAGER_HINT = "↑↓ move Β· type to filter Β· Enter open Β· Esc back" + # Top-menu action sentinels (distinct from a plain slot index, which selects that channel). _CREATE = "__create__" _PUBLIC = "__public__" @@ -265,7 +280,10 @@ async def manage_channels(ctx: AppContext) -> int: if not keep: return changes title, items = await settle(moved=ctx.devstate.channels_epoch != epoch) - menu = SelectScreen(title, items) + # ``Enter open``, not the list default: every slot row pushes the channel's detail + # screen and Reorder pushes the reorder screen, which is what the chat picker β€” the same + # channels, one screen over β€” has always said. + menu = SelectScreen(title, items, footer_hint=_MANAGER_HINT) async with session.stay(menu) as visit: while True: choice = await visit.result() @@ -607,24 +625,36 @@ def _activity_sparkline(histogram: tuple[int, ...], peak: float) -> Text: # --- menus ------------------------------------------------------------------- -def _lanes_header(name_w: int) -> str: +def _lanes_header(name_w: int, width: int) -> str: """Column headers over the channel list's fixed lanes (see :func:`_slot_text`). - The leading spaces cover the select screen's pointer column (2 cells) plus the glyph - lane (3 cells), so each header lands exactly over its column. ``UNREAD`` borrows its - lane's trailing gap β€” the badge lane itself is one cell too narrow for the word β€” which - still leaves a space before the message count. (No TYPE or HASH lane: the glyph already - carries the openness and the hash lives in Show key, which buys the activity sparkline - its room on a 72-column terminal.) + The indent covers the select screen's pointer column (2 cells, drawn on choice rows but + not separators) plus the glyph lane, *measured* rather than assumed: a channel's glyph + is two cells on the desktop and one on the console, so a hard-coded indent put every + label a column off there. ``UNREAD`` borrows its lane's trailing gap β€” the badge lane + itself is one cell too narrow for the word β€” which still leaves a space before the + message count. (No TYPE or HASH lane: the glyph already carries the openness and the + hash lives in Show key, which buys the activity sparkline its room on a 72-column + terminal.) + + Resolved against the render width, because the header row is pinned and must stay one + row: at 53 columns the full line ran to 54 and wrapped, costing a content row out of + twenty-six and leaving the landmark drawn twice over. ``ACTIVITY`` gives its cells back + first (see :func:`~meshterm.ui.menus.column_header`). """ - return ( - " " - + "CHANNEL".ljust(name_w + 2) - + "UNREAD".ljust(_BADGE_WIDTH + 2) - + f"{'MSGS':>{_COUNT_WIDTH}}" - + " " - + f"{'LAST':>{_AGE_WIDTH}}" - + " ACTIVITY" + return column_header( + [ + Lane("CHANNEL", name_w + 2), + Lane("UNREAD", _BADGE_WIDTH + 2), + # Right-aligned, because the values under them are: a count and an age are + # padded to the right edge of their lane, so a left-aligned label would sit + # off the digits it names. + Lane(f"{'MSGS':>{_COUNT_WIDTH}}", _COUNT_WIDTH + 2), + Lane(f"{'LAST':>{_AGE_WIDTH}}", _AGE_WIDTH + 2), + Lane(("ACTIVITY", "ACT")), + ], + width, + indent=2 + cell_len(channel_glyph("Public", None)) + 1, ) @@ -661,8 +691,9 @@ def _slot_text(ctx: AppContext, slot: ChannelSlot, stats: _LiveStats, name_w: in text.append(fit_cells(slot.name, name_w)) text.append(" ") if muted: - text.append("πŸ”•", style="muted") # 2 cells; pad the rest of the lane - text.append(" " * (_BADGE_WIDTH - 2)) + mark = glyph("πŸ”•") # two cells on the desktop, one on the console + text.append(mark, style="muted") + text.append(" " * (_BADGE_WIDTH - cell_len(mark))) elif unread: text.append("●", style="err") text.append(f" {unread}".ljust(_BADGE_WIDTH - 1), style="warn") @@ -674,7 +705,11 @@ def _slot_text(ctx: AppContext, slot: ChannelSlot, stats: _LiveStats, name_w: in # Clamped so a pathological backlog can't push the row out of its lanes. text.append(f"{min(total, 99999):>{_COUNT_WIDTH}}") else: - text.append(f"{'Β·':>{_COUNT_WIDTH}}", style="muted") + # ``β—‹`` is the app's empty mark. ``Β·`` here was three things at once: the separator + # that chains status atoms, the picker's unknown-sender stand-in, and β€” two cells + # away in this very row β€” what the console folds ``πŸ”•`` to, so a muted channel with + # no messages drew the same glyph twice meaning different things. + text.append(f"{'β—‹':>{_COUNT_WIDTH}}", style="muted") text.append(" ") age = _format_age(_age_seconds(st.last_at)) if st is not None and st.last_at else "" text.append(f"{age:>{_AGE_WIDTH}}", style="muted") @@ -699,56 +734,99 @@ def _menu_items( name_w = min(_NAME_WIDTH_MAX, max(len("CHANNEL"), *(len(s.name) for s in slots))) # The lane names are this block's only landmark (its section carries no ── heading ──), # so they pin overhead while the slots scroll and give way to Organize/Add a channel. - items.append(Separator(_lanes_header(name_w), heading=True)) + items.append(Separator(lambda w: _lanes_header(name_w, w), heading=True)) for slot in slots: items.append(Choice(title=_slot_row(ctx, slot, stats, name_w), value=slot.idx)) else: - items.append(Separator(" no channels configured yet")) + items.append(Separator(" no channels yet β€” add one below")) + # One measured icon column for every command row on this screen, so the one-cell marks + # (↕) start their labels in the same column as the two-cell ones (οΌ‹ οΌƒ 🌐 πŸ”‘ πŸ”—) instead + # of a column early β€” which the Organize row used to fix by hand, with two spaces and a + # four-line comment. Empty where the platform draws no icons, and the labels take the + # cells back. + lane = icon_lane(("↕", "🌐", "οΌ‹", "οΌƒ", "πŸ”‘", "πŸ”—")) if len(slots) > 1: items.append(section_heading("Organize")) - # Two spaces after the arrow: ↕ (East-Asian-ambiguous width) renders one cell where - # the sibling rows' glyphs (οΌ‹ οΌƒ πŸ”‘ πŸ”—) render two, so the extra space keeps this - # label's text column-aligned with theirs. Both go together where the platform - # draws no icon lane at all. - items.append(Choice(title=command_label("↕ Reorder channels"), value=_REORDER)) + reorder = marked_label("↕", "Reorder channels", "", lane=lane) + items.append(Choice(title=reorder, value=_REORDER)) items.append(section_heading("Add a channel")) + if _next_free_slot(slots, capacity) is None: + # The list already knows there is nowhere to put one, so it says so here rather than + # offering four rows that all dead-end in the same refusal. + items.append(Separator(" every slot is full β€” clear one first")) + return f"Channels Β· {len(slots)}/{capacity} slots", items rows = [] # The firmware's built-in fixed-key Public channel has one well-known secret, so it's the # same channel on every slot β€” offer to restore it only while no slot already holds it. if not any(s.secret == DEFAULT_PUBLIC_SECRET for s in slots): rows.append( - ("🌐 Standard Public channel", "MeshCore's built-in meshwide channel", _DEFAULT_PUBLIC) + ( + marked_label("🌐", "Standard Public channel", "", lane=lane), + "MeshCore's built-in meshwide channel", + _DEFAULT_PUBLIC, + ) ) rows.extend( [ - ("οΌ‹ New private channel…", "A fresh random key", _CREATE), - ("οΌƒ Public channel…", "Key derived from its name", _PUBLIC), - ("πŸ”‘ Join with a key…", "Paste a channel's 32-hex key", _JOIN), - ("πŸ”— Import a link…", "Paste a meshcore:// share link", _IMPORT), + ( + marked_label("οΌ‹", "New private channel…", "", lane=lane), + "A fresh random key", + _CREATE, + ), + ( + marked_label("οΌƒ", "Public channel…", "", lane=lane), + "Key derived from its name", + _PUBLIC, + ), + ( + marked_label("πŸ”‘", "Join with a key…", "", lane=lane), + "Paste a channel's 32-hex key", + _JOIN, + ), + ( + marked_label("πŸ”—", "Import a link…", "", lane=lane), + "Paste a meshcore:// share link", + _IMPORT, + ), ] ) items.extend(menu_rows(rows)) - return f"Channels β€” {len(slots)}/{capacity} slots", items + # ``Β·`` chains a status atom; ``β€”`` would introduce a subject, and the slot count is not + # what this screen is about (see Contacts, which reads "Contacts Β· 12 known"). + return f"Channels Β· {len(slots)}/{capacity} slots", items def _detail_summary(ctx: AppContext, slot: ChannelSlot, stats: _LiveStats) -> str: - """One line of vital signs for the detail screen: slot, totals, unread, mute, last activity.""" + """One line of vital signs for the detail screen: what it is, then how it has been used. + + The openness and the hash lead, having moved here out of the screen's title, which was + carrying them in a parenthesised blob and running to 42 cells β€” the whole of the + PicoCalc's borderless title bar, leaving no room for the one place that says Esc leaves. + + The line is one line. Atoms are shed from the right until it fits the platform's + readable width, because the ones on the left identify the channel and the ones on the + right describe traffic the reader can also see in the row they came from. + """ st = stats.get(slot.identity) unread = ctx.chat.unread(slot.conversation.key) - muted = _is_muted(ctx, slot) + kind = "public" if slot.is_public else "private" + parts = [kind, f"hash {slot.hash}", f"slot {slot.idx}"] if st is None or not st.total: - base = f"Slot {slot.idx} Β· no messages recorded yet" - return f"{base} Β· muted" if muted else base - parts = [f"Slot {slot.idx}", f"{st.total} message{'' if st.total == 1 else 's'}"] - if unread: - parts.append(f"{unread} unread") - if muted: + parts.append("no messages yet") + else: + parts.append(f"{st.total} msg{'' if st.total == 1 else 's'}") + if unread: + parts.append(f"{unread} unread") + if _is_muted(ctx, slot): parts.append("muted") - if st.last_at is not None: - parts.append(f"last message {format_ago(_age_seconds(st.last_at))}") + if st is not None and st.last_at is not None: + parts.append(f"last {format_ago(_age_seconds(st.last_at))}") + width = get_platform().readable_cols + while len(parts) > 1 and cell_len(" Β· ".join(parts)) > width: + parts.pop() return " Β· ".join(parts) @@ -763,31 +841,49 @@ def _detail_items(ctx: AppContext, slot: ChannelSlot) -> list: err-tinted label so it reads as such. """ unread = ctx.chat.unread(slot.conversation.key) - chat_label = Text("πŸ’¬ Open in chat") + # ✎ and πŸ—‘ are one cell where πŸ“± πŸ”‘ πŸ’¬ πŸ”” πŸ”• are two, so the column is measured once and + # every mark padded out to it β€” otherwise the edit and clear rows start their labels a + # column left of the rows above them. + lane = icon_lane(("πŸ“±", "πŸ”‘", "πŸ’¬", "πŸ””", "πŸ”•", "✎", "πŸ—‘")) + chat_label = marked_label("πŸ’¬", "Open in chat", "", lane=lane) if unread: chat_label.append(" ●", style="err") chat_label.append(f" {unread}", style="warn") if _is_muted(ctx, slot): mute_row = ( - "πŸ”” Unmute notifications", + marked_label("πŸ””", "Unmute notifications", "", lane=lane), "Show new messages here in the unread badge", _MUTE, ) else: mute_row = ( - "πŸ”• Mute notifications", + marked_label("πŸ”•", "Mute notifications", "", lane=lane), "Hide new messages here from the unread badge", _MUTE, ) items = menu_rows( [ - ("πŸ“± Show QR code", "Share this channel as a scannable code", _QR), - ("πŸ”‘ Show key", "The name, key, hash, and share link", _KEY), + ( + marked_label("πŸ“±", "Show QR code", "", lane=lane), + "Share this channel as a scannable code", + _QR, + ), + ( + marked_label("πŸ”‘", "Show key", "", lane=lane), + "The name, key, hash, and share link", + _KEY, + ), (chat_label, "Read and send messages on this channel", _CHAT), mute_row, - ("✎ Rename / change key…", "Edit the name or paste a different key", _EDIT), ( - Text("πŸ—‘ Clear this slot…", style="err"), + marked_label("✎", "Rename / change key…", "", lane=lane), + "Edit the name or paste a different key", + _EDIT, + ), + ( + # The tint goes on the mark, not the words β€” and falls back to the words only + # where the platform draws no mark at all (see menus.marked_label). + marked_label("πŸ—‘", "Clear this slot…", "err", lane=lane), "Remove the channel from this device", _CLEAR, ), @@ -810,8 +906,9 @@ async def _channel_detail( rename/re-key or clear closes the detail (the slot's occupant changed, so the caller re-reads the device); the read-only actions loop back here. """ - kind = "public" if slot.is_public else "private" - title = f"{slot.name} ({kind}, hash {slot.hash})" + # ``Feature β€” subject``: what the openness and the hash used to be doing in here is now + # the first two atoms of the summary line below the title (see :func:`_detail_summary`). + title = f"Channel β€” {slot.name}" async def handle(choice: object) -> int | None: """Run one action; an int closes the detail with that many changes, ``None`` stays.""" diff --git a/tests/test_channels.py b/tests/test_channels.py index 7ed08f9..fe1b701 100644 --- a/tests/test_channels.py +++ b/tests/test_channels.py @@ -13,6 +13,7 @@ from pathlib import Path from types import SimpleNamespace import pytest +from rich.cells import cell_len from rich.console import Console from meshterm.context import AppContext @@ -580,7 +581,7 @@ async def test_channel_rows_carry_stats_unread_and_lanes(ctx: AppContext) -> Non ctx.chat._unread[slot.conversation.key] = 2 # as the service would after two arrivals title, items = _menu_items(ctx, slots, 8, _LiveStats(ctx)) - assert title == "Channels β€” 1/8 slots" + assert title == "Channels Β· 1/8 slots" # a status atom chains with Β·, not an em dash row = next(it for it in items if isinstance(it, Choice) and it.value == 0) plain = row.label.plain # the title is a live callable; .label resolves it assert "Ops" in plain @@ -678,13 +679,47 @@ async def test_detail_summary_reads_slot_totals_and_unread(ctx: AppContext) -> N await device.set_channel(3, "Ops", bytes(range(16))) slot = next(s for s in await read_channel_slots(device) if s.idx == 3) - assert _detail_summary(ctx, slot, _LiveStats(ctx)) == "Slot 3 Β· no messages recorded yet" + # What the channel *is* leads β€” the openness and hash moved here out of a title that + # was 42 cells wide and left no room on the console for the bar that says Esc leaves. + assert _detail_summary(ctx, slot, _LiveStats(ctx)) == ( + "private Β· hash be Β· slot 3 Β· no messages yet" + ) ctx.repo.record_chat_message(ChatMessage(text="hi", is_channel=True, channel_id=slot.identity)) ctx.chat._unread[slot.conversation.key] = 1 summary = _detail_summary(ctx, slot, _LiveStats(ctx)) # A fresh age reads as bare "now" (the app-wide format_ago grammar β€” never "now ago"). - assert summary == "Slot 3 Β· 1 message Β· 1 unread Β· last message now" + assert summary == "private Β· hash be Β· slot 3 Β· 1 msg Β· 1 unread Β· last now" + + +async def test_the_detail_summary_sheds_atoms_rather_than_wrapping(ctx: AppContext) -> None: + """One line, on both platforms: the traffic atoms go before the line does. + + The identifying atoms are on the left and the describing ones on the right, so a line + too long for the console loses what the reader could already see in the row they opened + this screen from β€” never the channel's own identity. + """ + from meshterm.core.models import ChatMessage + from meshterm.platforms import PICOCALC, REGULAR, set_platform + from meshterm.ui.channels import _detail_summary, _LiveStats + + device = await ctx.device() + await device.set_channel(0, "Lakeside emergency net", bytes(range(16))) + slot = next(s for s in await read_channel_slots(device) if s.idx == 0) + for _ in range(3): + ctx.repo.record_chat_message( + ChatMessage(text="hi", is_channel=True, channel_id=slot.identity) + ) + ctx.chat._unread[slot.conversation.key] = 2 + + try: + for platform in (REGULAR, PICOCALC): + set_platform(platform) + line = _detail_summary(ctx, slot, _LiveStats(ctx)) + assert cell_len(line) <= platform.readable_cols, (platform.name, line) + assert line.startswith("private Β· hash be Β· slot 0") # identity always survives + finally: + set_platform(REGULAR) # -- muting channel notifications --------------------------------------------- diff --git a/tests/test_gallery.py b/tests/test_gallery.py index fb1f8e6..f6fe95a 100644 --- a/tests/test_gallery.py +++ b/tests/test_gallery.py @@ -29,12 +29,14 @@ from collections.abc import Callable from dataclasses import dataclass from datetime import datetime, timezone from pathlib import Path +from types import SimpleNamespace import pytest from rich.cells import cell_len from rich.text import Text from meshterm.core.advert_store import AdvertPolicy +from meshterm.core.channels import DEFAULT_PUBLIC_SECRET, derive_secret from meshterm.core.courier_store import CourierStore from meshterm.core.models import ( NODE_TYPE_REPEATER, @@ -417,6 +419,77 @@ def _chat_picker(cols: int, rows: int) -> Screen: ) +def _channels_manager(cols: int, rows: int) -> Screen: + """The channel manager at its widest: every lane populated, every action row drawn. + + Built through the feature's own row funnel, so the header line, the glyph lane, the + unread badge, the counts, the ages and the sparkline are laid out by the code the app + runs. The cases that matter are here: a name at the lane's ceiling, a muted channel (its + mark folds to a different glyph on the console), a channel with no messages at all, and + a three-digit unread badge. + """ + from meshterm.ui.channels import _MANAGER_HINT, ChannelSlot, _LiveStats, _menu_items + + slots = [ + ChannelSlot(idx=0, name="Public", secret=DEFAULT_PUBLIC_SECRET), + ChannelSlot(idx=1, name="Lakeside emergency", secret=bytes(range(16))), + ChannelSlot(idx=2, name="#montreal", secret=derive_secret("#montreal")), + ChannelSlot(idx=3, name="Ops", secret=bytes(range(16, 32))), + ] + ctx = _ChannelsCtx(muted={slots[3].identity}) + title, items = _menu_items(ctx, slots, 8, _LiveStats(ctx)) + return SelectScreen(title, items, footer_hint=_MANAGER_HINT) + + +def _channel_detail(cols: int, rows: int) -> Screen: + """One channel's action page, its vital-signs line above the rows.""" + from meshterm.ui.channels import ChannelSlot, _detail_items, _detail_summary, _LiveStats + + slot = ChannelSlot(idx=2, name="Lakeside emergency", secret=bytes(range(16))) + ctx = _ChannelsCtx(muted=set()) + stats = _LiveStats(ctx) + return SelectScreen( + f"Channel β€” {slot.name}", + _detail_items(ctx, slot), + prompt=_detail_summary(ctx, slot, stats), + footer_hint="↑↓ move Β· Enter select Β· Esc back", + ) + + +class _ChannelsChat: + """The chat service surface the channel rows read: unread counts and mute state.""" + + def __init__(self, muted: set[str]) -> None: + self._muted = muted + + def unread(self, key: str) -> int: + """A three-digit badge on one channel, so the lane is drawn at its widest.""" + return 128 if key.endswith("slot:1") or "Lakeside" in key else 0 + + +class _ChannelsCtx: + """The minimal AppContext surface the channel rows and detail page read.""" + + def __init__(self, muted: set[str]) -> None: + self.chat = _ChannelsChat(muted) + self.repo = _ChannelsRepo() + self.preferences = Preferences() + self._muted = muted + + @property + def mute_store(self): # noqa: ANN201 - a stand-in for the real store + """A store answering only the one question a row asks it.""" + return SimpleNamespace(is_muted=lambda identity: identity in self._muted) + + +class _ChannelsRepo: + """Channel statistics with one busy channel, one quiet one, and one never used.""" + + def channel_stats(self, *a, **k): # noqa: ANN002, ANN003, ANN201 + """Return nothing: the rows then draw their empty marks, which is the tighter case.""" + return {} + + class _PickerCtx: """The minimal AppContext surface :meth:`ChatTool._picker_items` reads.""" @@ -789,6 +862,8 @@ _ENTRIES: list[_Entry] = [ _Entry("map", _map), _Entry("chat", _chat), _Entry("chat_picker", _chat_picker), + _Entry("channels_manager", _channels_manager), + _Entry("channel_detail", _channel_detail), _Entry("livefeed", _livefeed), _Entry("walk", _walk), _Entry("timemachine", _timemachine),