From 6a4ea173b2752805ca4dda7c49172ff8c30ff02a Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Wed, 12 Aug 2026 14:07:58 -0500 Subject: [PATCH 1/3] fix(tabs): a background pane says which tab it is showing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PaintTabChips computed an unfocused pane's selected chip and its siblings from the same expression, background and ink alike, so every chip in a background pane was painted byte-identically to the pane's own surface tone. The ▌ was no help: TabTitles only emits it on the focused pane. A chip now states one fact relative to its own strip — the selected tab is painted the plane its page is painted on, its siblings are recessed one BackdropScale step below it. Pane focus is not a parameter; it arrives folded into PaneSurfaceTone, so the cue is one ratio in every strip, on every theme, under every tint, and hue stays free to carry identity. Selection is also emboldened, in every pane, so it survives a terminal that flattens the chips; it composes with the unread tint into one markup tag, and costs no cells. The new `tabs` view is the frame: two tabs in the pane that does not hold the focus, which no existing view had — and is why no snapshot caught it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN --- CLAUDE.md | 31 ++- src/SharpMUTerm.Tui/DemoScene.cs | 8 + src/SharpMUTerm.Tui/SharpMUTermApp.cs | 91 +++++---- src/SharpMUTerm.Tui/TabTitles.cs | 23 ++- src/SharpMUTerm.Tui/WorkspacePalette.cs | 19 +- .../TabActivityIndicatorTests.cs | 6 +- .../TabSelectionTests.cs | 182 ++++++++++++++++++ 7 files changed, 308 insertions(+), 52 deletions(-) create mode 100644 tests/SharpMUTerm.Tui.Tests/TabSelectionTests.cs diff --git a/CLAUDE.md b/CLAUDE.md index 31ca561..20ee8ad 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -539,7 +539,11 @@ python3 tools/ansi_frame_to_image.py frame.ansi frame.html # or .svg wording, and this is the one that happens many times an hour), `prefix-panel` (the ⌃B which-key panel — the state `prefix` becomes a few hundred milliseconds later, if no key has arrived), `focus`/`focus-moved` (a split *and* a second command line — the one geometry showing a focused pane - beside an unfocused one and an armed bar above an idle one, before and after a real ⌃→), plus the + beside an unfocused one and an armed bar above an idle one, before and after a real ⌃→), + `tabs` (**two tabs in the pane that does *not* hold the focus** — the one geometry that can show a + strip saying which tab is in front, since every other view has at most one tab in an unfocused pane. + It opens a third window for itself alone, and re-activates the main window before splitting, because + opening brings a window to the front and a split carries the tabs that are not), plus the default workspace (no `--view`). Any settings screen also takes a `-edit` suffix, which opens it and drives real keys in so the frame shows a field mid-edit. State toggles: `collapsed`, `prefix`, `timestamps`, @@ -854,10 +858,25 @@ markup (`[bold #rrggbb on #rrggbb]…[/]`, `[[`/`]]` escaping, `[link=url]…[/] `TabTitles` claimed the opposite for as long as it existed, and the claim had two costs: the unread count went out untinted, and a window title was never escaped — a window called `[Chat]`, or a web view titled from the page it loaded, had that eaten as a tag by the parser *and* by the hit test. Titles are - `MarkupText.Escape`d now. The tint covers the name and the count only; the `▌` stays outside it, because - focus and activity are independent facts and a marker that changed colour on an incoming line would be - reporting the wrong one. **The two cues are different channels on purpose** — focus is said entirely in - *backgrounds* from the theme's chrome family, activity in a *foreground* no plane is painted in. + `MarkupText.Escape`d now. The tint and the selection **bold** cover the name and the count only; the `▌` + stays outside them, because focus, selection and activity are independent facts and a marker that + changed colour on an incoming line would be reporting the wrong one. **The cues are different channels + on purpose** — focus in *background luminance*, activity in a *foreground* no plane is painted in, + selection in *weight* and in a within-strip step. A selected tab can be unread, so the two are one tag + (`[bold #rrggbb]`) rather than two nested ones. +- **A tab chip says which tab you are viewing, relative to its own strip — and pane focus is not a term in + it.** The selected chip is painted `PaneSurfaceTone` exactly, the plane its own page is painted on, and + its siblings are `WorkspacePalette.Recessed` one step below it (`BackdropScale`, the backdrop's own + step). Focus arrives already folded into that plane — tinted for the character, lifted for focus — so + one ratio inside every strip reads whether or not the pane holds the keyboard, and hue is untouched. + Deriving the selected chip *from* focus is what made this unreadable: `PaintTabChips` computed an + unfocused pane's selected chip and its siblings from the same expression, background **and** ink, so + every chip in a background pane was byte-identical to the pane's surface and the `▌` was no help + because `TabTitles` only emits it on the focused pane. `TabSelectionTests`, and the `tabs` view is the + frame — no other view has two tabs in an unfocused pane, which is why nothing caught it. + **Not fixed here**: the `│` between chips and the `─` filling the rest of the strip are hardcoded + `Color.Grey` in `TabControl.Rendering.cs`, unthemeable in all three header styles. That is an upstream + change. - **One unread count, one spelling: `UnreadBadge`.** The sidebar and the tab strip are two views of `WorkspaceWindow.Unread`, and they had two formatters — the rail capped at `99+`, the tab printed the raw integer, so a busy channel read `99+` in one place and `(4127)` in the other. Cap, field width and @@ -889,7 +908,7 @@ markup (`[bold #rrggbb on #rrggbb]…[/]`, `[[`/`]]` escaping, `[link=url]…[/] is derived from the pane rectangle (`PaneOutputRects`), so a border, gutter or marker column that only the focused pane has would re-announce a different terminal size to every connected server on every focus change and reflow the game's own output. The cues are the pane's own plane - (`WorkspacePalette.Focus`), the active tab's chip colour (`TabControl.Active*BackgroundColor`), and a + (`WorkspacePalette.Focus`), the tab chips it carries that plane into, and a `▌` in the tab *title* — all zero-cost. `FocusIndicationTests.MovingFocusDoesNotMoveAnyPaneRectangle` is the test that stops this being "improved" into a border. Colours live in `WorkspacePalette`, whose constants are all derived from a `ScreenPalette` pair so the workspace and the settings screens share diff --git a/src/SharpMUTerm.Tui/DemoScene.cs b/src/SharpMUTerm.Tui/DemoScene.cs index f821d27..4ee012e 100644 --- a/src/SharpMUTerm.Tui/DemoScene.cs +++ b/src/SharpMUTerm.Tui/DemoScene.cs @@ -34,6 +34,14 @@ internal static class DemoScene /// public static string ChatWindowId => Workspace.SpawnWindowId(ActiveSessionKey, "Chat"); + /// + /// A second routed window, opened only by the tabs view. It is deliberately not in + /// : a third window there would put a third row on every frame's rail + /// and a second tab in every frame's main pane, and the whole gallery would move to answer one + /// view's question. + /// + public static string ScenesWindowId => Workspace.SpawnWindowId(ActiveSessionKey, "Scenes"); + public static AppConfiguration Build() { var config = new AppConfiguration(); diff --git a/src/SharpMUTerm.Tui/SharpMUTermApp.cs b/src/SharpMUTerm.Tui/SharpMUTermApp.cs index 19d723c..a9ac5b6 100644 --- a/src/SharpMUTerm.Tui/SharpMUTermApp.cs +++ b/src/SharpMUTerm.Tui/SharpMUTermApp.cs @@ -950,6 +950,35 @@ public string RenderSnapshot(string? view = null) RebuildPaneArea(); } + // The tab strip's own question — *which tab am I looking at* — in the one geometry that can + // answer it: a pane that does not hold the focus, holding two tabs. Every other view has at most + // one tab in an unfocused pane, which is exactly why no frame ever caught an unfocused pane + // painting its selected and unselected chips the same colour. A third window is opened for this + // view alone (see DemoScene.ScenesWindowId) and the split then carries both non-active tabs + // across, leaving the focus on the left pane and two tabs on the right. The unread on the tab + // that is *not* selected is there so the frame also shows the activity tint and the selection + // weight in one strip, which is the pair that has to stay legible together. + if (string.Equals(view, "tabs", StringComparison.OrdinalIgnoreCase)) + { + var scenes = _workspace.OpenWindow( + DemoScene.ScenesWindowId, "Scenes", WindowKind.Spawn, DemoScene.ActiveSessionKey); + scenes.OwnerLabel = DemoScene.MainCharacterName; + + // Opening a window brings it to the front, and a split carries the tabs that are *not* in + // front. Put the character's own window back in front first, or the split strands Scenes + // alone in the focused pane and the frame answers a different question. + _workspace.ActivateWindow(MainWindowId); + PaneCommands.Apply(_workspace.Layout, PaneCommand.SplitRight); + + for (var i = 0; i < 4; i++) + { + _workspace.NoteActivity(DemoScene.ScenesWindowId); + } + + RebuildPaneArea(); + RefreshRail(); + } + // The focus indication, in the one geometry that shows all of it at once: two panes and two // command lines, so a frame carries a focused pane beside an unfocused one *and* an armed bar // above an idle one. `focus` leaves the focus where a split leaves it (the left pane, primary bar @@ -7868,7 +7897,7 @@ private void RefreshPaneFocus() { foreach (var (paneId, tabs) in _paneTabs) { - PaintTabChips(tabs, IsFocusedPane(paneId), PanePlane(paneId)); + PaintTabChips(tabs, PaneSurfaceTone(paneId)); } } @@ -8020,45 +8049,28 @@ private bool IsFocusedPane(string paneId) => string.Equals(paneId, _workspace.Layout.FocusedPaneId, StringComparison.Ordinal); /// - /// Colours a pane's tab chips by whether the pane holds the focus. This is the cue that carries the - /// signal: the focused pane's active tab is painted in the very band the armed command line is painted - /// in, so one colour means "you are here" in both of the places it can be said, and the strip is - /// already drawn — so, like the plane behind it, this consumes no cells and cannot move the pane's - /// rectangle or the NAWS size derived from it. - /// - /// The framework's four properties split on its own keyboard focus, which in this app is - /// never the tab strip: focus is pinned to the armed command line, so the …Focused… variants - /// would never be reached. Both are therefore set to the same value and driven by our pane - /// focus, which is the fact the user is asking about. - /// + /// Colours a pane's tab chips: the selected tab is painted — the plane its + /// own page is painted on — and its siblings are recessed from it. /// - /// Elevation alone was not enough. A pane's plane is what the game's own colours are read against, so - /// it can only be lifted so far before it starts competing with them — and a lift small enough to be - /// safe there reads, in a rendered frame, as a gentle elevation rather than as "super obvious", which - /// is what was asked for. The chip has no such constraint: nothing is read on it but its own label. + /// Pane focus is not a parameter, and that is the design. It is already in + /// , which PaneSurfaceTone has lifted for focus and tinted for its + /// character, so the selection cue is one ratio inside every strip and reads whether or not the pane + /// holds the focus. Deriving the selected chip from focus is what left an unfocused pane + /// painting all of its chips the same colour. /// /// - /// The inactive chips are painted on — the pane's own plane, tint - /// and all — rather than on the palette's plain surface. They sit inside the rectangle that plane - /// fills, so a strip drawn on the untinted tone would be a differently-coloured band across the top - /// of every tinted pane. The active chip keeps the chrome family's armed band, because that one is - /// saying where the keyboard is and not whose pane this is. + /// The framework's four properties split on its own keyboard focus, which is never the tab + /// strip here — focus is pinned to the armed command line — so each pair is set to one value. /// /// - private void PaintTabChips(TabControl tabs, bool focused, Rgb plane) + private void PaintTabChips(TabControl tabs, Rgb surface) { - // Each ink is held to the floor against the chip it actually lands on, and the two chips are - // different planes. Solarized is the theme that showed why: its foreground on its own armed band - // measured 2.43:1, so the active tab and the command line — the two places this pair is used — - // were the least readable text in the client on the theme most likely to be chosen for comfort. - var activeBgRgb = focused ? WorkspacePalette.ArmedBand(_theme) : plane; - var restBgRgb = focused ? WorkspacePalette.Focus(plane) : plane; - var activeBg = ToColor(activeBgRgb); + // Each ink is held to the floor against the chip it actually lands on; the two are different + // planes, and the recessed one is the harder of them. + var restBgRgb = WorkspacePalette.Recessed(surface); + var activeBg = ToColor(surface); var activeFg = ToColor(Contrast.Legible( - focused - ? _theme.Resolve(TerminalColor.Default, isBackground: false) - : WorkspacePalette.IdleInk(_theme), - activeBgRgb)); + _theme.Resolve(TerminalColor.Default, isBackground: false), surface)); var restBg = ToColor(restBgRgb); var restFg = ToColor(Contrast.Legible(WorkspacePalette.IdleInk(_theme), restBgRgb)); @@ -8085,11 +8097,11 @@ private IWindowControl BuildPaneTabs(PaneNode pane) continue; } - // The marker rides the focused pane's *active* tab: the strip is plain text to the framework, - // so a glyph is the only per-pane cue the strip can carry, and it is the shape half of the - // focus signal — it reads on a monochrome terminal, where a luminance step does not. + // ▌ is the focus half of the signal and bold is the selection half; both read on a monochrome + // terminal, where a luminance step does not. + var selected = string.Equals(pane.ActiveTab, windowId, StringComparison.Ordinal); builder.AddTab( - TabTitles.For(window, ActiveCharacterKey(), focused && pane.ActiveTab == windowId, _ink), + TabTitles.For(window, ActiveCharacterKey(), focused && selected, selected, _ink), BuildTabContent(pane, windowId, window)); ids.Add(windowId); } @@ -8108,7 +8120,7 @@ private IWindowControl BuildPaneTabs(PaneNode pane) tabs.ActiveTabIndex = pane.ActiveIndex; } - PaintTabChips(tabs, focused, PanePlane(pane.Id)); + PaintTabChips(tabs, PaneSurfaceTone(pane.Id)); var paneId = pane.Id; tabs.TabChanged += (_, e) => OnTabChanged(paneId, e.NewTab); @@ -9909,8 +9921,9 @@ private void RefreshTabTitles() { if (page.Tag is string id && _workspace.FindWindow(id) is { } window) { + var selected = string.Equals(activeTab, id, StringComparison.Ordinal); page.Title = TabTitles.For( - window, focusedCharacter, IsFocusedPane(paneId) && activeTab == id, _ink); + window, focusedCharacter, IsFocusedPane(paneId) && selected, selected, _ink); // The × follows the active tab, so keep it in step with every title refresh. page.IsClosable = CanCloseTab(id, activeTab); } diff --git a/src/SharpMUTerm.Tui/TabTitles.cs b/src/SharpMUTerm.Tui/TabTitles.cs index d7df90e..1772c9f 100644 --- a/src/SharpMUTerm.Tui/TabTitles.cs +++ b/src/SharpMUTerm.Tui/TabTitles.cs @@ -43,10 +43,17 @@ internal static class TabTitles /// two planes flatten together. It leads rather than trails because that is the edge of the strip a /// reader's eye starts at, and it is on the active tab only, so a pane never shows two. /// + /// + /// Whether this is the tab its pane is showing. It is emboldened in every pane, focused or + /// not, so the selection reads on a terminal that flattens the chips behind it — the shape half of + /// what the chip colours say. Independent of : an unfocused pane still + /// has a tab in front of it. + /// public static string For( WorkspaceWindow window, string? focusedCharacterKey = null, bool focusedPane = false, + bool selected = false, ChromeInk? ink = null) { ArgumentNullException.ThrowIfNull(window); @@ -76,11 +83,19 @@ public static string For( var focus = focusedPane ? Glyphs.FocusedPane + " " : string.Empty; - // The activity tint. It covers the window's name and its count and stops there: the ▌ ahead of it - // is the focus marker and the ✎ / ⌁ behind it are other facts, and a signal that recoloured them - // would be claiming they had changed too. Zero cells — see the remarks on this class. + // The activity tint and the selection weight cover the window's name and its count and stop + // there: the ▌ ahead of them is the focus marker and the ✎ / ⌁ behind them are other facts, and + // a signal that recoloured those would be claiming they had changed too. One tag rather than two + // nested ones, because a selected tab can also be unread. Zero cells — see the class remarks. var named = owner + MarkupText.Escape(window.Title) + unread; - var body = window.Unread > 0 ? $"[{UnreadBadge.TintFor(ink)}]{named}[/]" : named; + var style = (selected, window.Unread > 0) switch + { + (true, true) => $"bold {UnreadBadge.TintFor(ink)}", + (true, false) => "bold", + (false, true) => UnreadBadge.TintFor(ink), + _ => null, + }; + var body = style is null ? named : $"[{style}]{named}[/]"; return focus + body + pen + cross; } diff --git a/src/SharpMUTerm.Tui/WorkspacePalette.cs b/src/SharpMUTerm.Tui/WorkspacePalette.cs index ede121d..d4810fe 100644 --- a/src/SharpMUTerm.Tui/WorkspacePalette.cs +++ b/src/SharpMUTerm.Tui/WorkspacePalette.cs @@ -356,7 +356,19 @@ internal static Rgb IdleInk(Theme theme) /// gaps a split leaves between panes. Recessed relative to , so an empty pane /// is still a visible rectangle and a workspace of many panes reads as cards on a desk. /// - internal static Rgb Backdrop(Theme theme) => Scale(Surface(theme), BackdropScale); + internal static Rgb Backdrop(Theme theme) => Recessed(Surface(theme)); + + /// + /// One step behind a plane — the backdrop's own step, reused for the chips of a tab strip's + /// unselected tabs. + /// + /// A chip states one fact, and states it relative to its own strip: the selected tab is painted the + /// plane its page is painted on, its siblings are recessed from it. Pane focus is not a term, because + /// it is already in the plane — so the selection cue is one ratio in every strip, on every theme, + /// under every tint, and the two questions a strip answers stay on separate channels. + /// + /// + internal static Rgb Recessed(Rgb plane) => Scale(plane, BackdropScale); /// The one-cell hairline a split draws between two panes, and beside the rail. internal static Rgb Rule(Theme theme) => Mix(Surface(theme), theme.Border, RuleLift); @@ -432,7 +444,10 @@ internal static Rgb ReadingPlane(Theme theme) internal static Rgb ChromePlane(Theme theme) { ArgumentNullException.ThrowIfNull(theme); - return Extreme(PanePlanes(theme).Append(Backdrop(theme)), Surface(theme)); + + // The recessed chips belong in the set: an unread tint is chrome ink and lands on one of them. + var planes = PanePlanes(theme).ToList(); + return Extreme(planes.Concat(planes.Select(Recessed)).Append(Backdrop(theme)), Surface(theme)); } /// diff --git a/tests/SharpMUTerm.Tui.Tests/TabActivityIndicatorTests.cs b/tests/SharpMUTerm.Tui.Tests/TabActivityIndicatorTests.cs index 3ea04c8..2a6cd96 100644 --- a/tests/SharpMUTerm.Tui.Tests/TabActivityIndicatorTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/TabActivityIndicatorTests.cs @@ -117,7 +117,11 @@ await Assert.That(MarkupParser.StripLength(MainTabLabel(app))) app.RenderNextFrame(); await Assert.That(app.UnreadOf(Main)).IsEqualTo(0); - await Assert.That(MainTabLabel(app)).IsEqualTo($"{Glyphs.FocusedPane} Corvid"); + + // On the visible text, not the markup: the label is still emboldened for being the tab its pane + // is showing, which is a different fact from having unread. + await Assert.That(FrameGrid.Visible(MainTabLabel(app))) + .IsEqualTo($"{Glyphs.FocusedPane} Corvid"); } /// diff --git a/tests/SharpMUTerm.Tui.Tests/TabSelectionTests.cs b/tests/SharpMUTerm.Tui.Tests/TabSelectionTests.cs new file mode 100644 index 0000000..1571a1a --- /dev/null +++ b/tests/SharpMUTerm.Tui.Tests/TabSelectionTests.cs @@ -0,0 +1,182 @@ +using SharpConsoleUI.Drivers; +using SharpConsoleUI.Parsing; +using SharpMUTerm.Core.Configuration; +using SharpMUTerm.Core.Text; +using SharpMUTerm.Core.Theming; +using SharpMUTerm.Core.Workspaces; +using SharpMUTerm.Graphics; +using SharpMUTerm.Tui; + +namespace SharpMUTerm.Tui.Tests; + +/// +/// Which tab a pane is showing, and how the strip says so. +/// +/// A chip states one fact — is this the tab you are viewing — relative to its own strip. Pane focus is +/// not a term in the arithmetic; it arrives folded into the plane, so the cue is one ratio in every +/// strip, on every theme, under every tint. Deriving the selected chip from focus instead is what left +/// an unfocused pane painting every chip its own surface tone, ink included. +/// +/// +/// is a frame test rather than +/// an arithmetic one deliberately: the old arithmetic was internally consistent while the screen was +/// wrong, so anything agreeing with the expression would have passed. +/// +/// +/// Serialised: rendering redirects the process-global Console.Out. +[NotInParallel] +public class TabSelectionTests +{ + private const int Width = 120; + private const int Height = 32; + + private static readonly TerminalCapabilities Headless = + new(GraphicsProtocol.None, supportsTrueColor: true, supportsKittyGraphics: false, supportsSixel: false); + + private static SharpMUTermApp Demo() + { + Console.SetIn(TextReader.Null); + return new SharpMUTermApp(DemoScene.Build(), Headless, new HeadlessConsoleDriver(Width, Height)); + } + + /// + /// The row carrying both tab labels of the tabs view's right-hand pane, with the column each + /// starts at. Read off the decoded rows, since the strip sits above the pane rectangles. + /// + private static (int Row, int Chat, int Scenes) Strip(string frame) + { + var rows = FrameGrid.Decode(frame, Width, Height); + for (var y = 0; y < rows.Count; y++) + { + var chat = rows[y].IndexOf("Chat", StringComparison.Ordinal); + var scenes = rows[y].IndexOf("Scenes", StringComparison.Ordinal); + if (chat >= 0 && scenes >= 0) + { + return (y, chat, scenes); + } + } + + throw new InvalidOperationException("No row carried both of the right pane's tab labels."); + } + + /// + /// The reported bug as a frame: the right pane holds no focus and two tabs, and the chip under the + /// one in front has to differ from the chip under its sibling. + /// + [Test] + public async Task AnUnfocusedPanesSelectedTabIsDistinguishableFromItsSiblings() + { + var frame = Demo().RenderSnapshot("tabs"); + var (row, chat, scenes) = Strip(frame); + var cells = FrameGrid.Cells(frame, Width, Height); + + // A cell inside each label, not the padding between chips. + await Assert.That(cells[(row, chat)].Background) + .IsNotEqualTo(cells[(row, scenes)].Background); + } + + /// + /// Within a strip the selected chip is the brighter of the two. Across panes that ordering belongs to + /// focus, which is why the step is taken inside the strip and not against a fixed tone. + /// + [Test] + public async Task ASelectedChipOutshinesItsSiblingsOnEveryPlane() + { + foreach (var theme in ThemeLibrary.Names.Select(ThemeLibrary.Get)) + { + foreach (var tint in Enum.GetValues()) + { + foreach (var lit in new[] { false, true }) + { + var plane = WorkspacePalette.Tint(theme, tint); + var surface = lit ? WorkspacePalette.Focus(plane) : plane; + + await Assert.That(Contrast.RelativeLuminance(WorkspacePalette.Recessed(surface))) + .IsLessThan(Contrast.RelativeLuminance(surface)); + } + } + } + } + + /// + /// Focus still orders the panes: both of a focused strip's chips clear their unfocused counterparts, + /// so the two steps cannot be read for each other. + /// + [Test] + public async Task FocusStillOrdersTheStripsAcrossPanes() + { + foreach (var theme in ThemeLibrary.Names.Select(ThemeLibrary.Get)) + { + foreach (var tint in Enum.GetValues()) + { + var plane = WorkspacePalette.Tint(theme, tint); + var lit = WorkspacePalette.Focus(plane); + + await Assert.That(Contrast.RelativeLuminance(plane)) + .IsLessThan(Contrast.RelativeLuminance(lit)); + await Assert.That(Contrast.RelativeLuminance(WorkspacePalette.Recessed(plane))) + .IsLessThan(Contrast.RelativeLuminance(WorkspacePalette.Recessed(lit))); + } + } + } + + /// + /// Selection is said in weight as well as colour, and costs no cells: every width the strip is + /// measured by is MarkupParser.StripLength. + /// + [Test] + public async Task SelectionIsBoldAndCostsNoCells() + { + var window = new WorkspaceWindow("w", "Chat", WindowKind.Spawn); + var selected = TabTitles.For(window, selected: true); + + await Assert.That(selected).Contains("[bold]"); + await Assert.That(MarkupParser.StripLength(selected)) + .IsEqualTo(MarkupParser.StripLength(TabTitles.For(window))); + } + + /// + /// A background pane's front tab collects lines like any other, so the two cues have to compose into + /// one tag rather than nest. + /// + [Test] + public async Task ASelectedTabThatIsAlsoUnreadKeepsBothCues() + { + var workspace = new Workspace(); + var window = workspace.OpenWindow("w", "Chat", WindowKind.Spawn); + workspace.ActivateWindow("main"); + for (var i = 0; i < 4; i++) + { + workspace.NoteActivity("w"); + } + + var markup = TabTitles.For(window, selected: true); + + await Assert.That(markup).Contains($"[bold {UnreadBadge.TintFor(null)}]"); + await Assert.That(MarkupParser.StripLength(markup)) + .IsEqualTo(MarkupParser.StripLength(TabTitles.For(window))); + } + + /// + /// Per-pane NAWS is derived from the pane rectangles, so a selection cue that cost a cell would + /// re-announce a terminal size on every tab change. Counterpart of + /// TabActivityIndicatorTests.ActivityMovesNoPaneRectangle. + /// + [Test] + public async Task SelectingATabMovesNoPaneRectangle() + { + var app = Demo(); + app.RenderSnapshot("tabs"); + var before = app.PaneOutputRects().ToDictionary(p => p.Key, p => p.Value, StringComparer.Ordinal); + + app.SimulateWindowChange(DemoScene.ScenesWindowId); + app.RenderNextFrame(); + + var after = app.PaneOutputRects(); + await Assert.That(after.Count).IsEqualTo(before.Count); + foreach (var (paneId, rect) in after) + { + await Assert.That(rect).IsEqualTo(before[paneId]); + } + } +} From a826a97a4e6cf03a13cacd59019692add3ef7435 Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Wed, 12 Aug 2026 14:14:20 -0500 Subject: [PATCH 2/3] docs: trim comment narration in the palette/rail/workspace cluster MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Comments here had grown into changelog: what the code used to do, which report a line answers, what a previous set of colours measured. None of that is recoverable value in pre-release software, and it buried the rationale that is. Keeps present-tense why a maintainer could not recover from the code — the TintDepth bound, the AtLuma direction split, why fields are reserved, why a route may not reach another session's spawn window — and drops the history around it. Also fixes a malformed doc block in UnreadBadge that closed twice. WorkspacePalette 338 -> 140 comment lines, RailRenderer 175 -> 112, Workspace 246 -> 176, TabTitles and UnreadBadge likewise. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN --- src/SharpMUTerm.Core/Workspace/Workspace.cs | 157 +++------ src/SharpMUTerm.Tui/RailRenderer.cs | 151 +++----- src/SharpMUTerm.Tui/TabTitles.cs | 62 +--- src/SharpMUTerm.Tui/UnreadBadge.cs | 28 +- src/SharpMUTerm.Tui/WorkspacePalette.cs | 368 ++++++-------------- 5 files changed, 223 insertions(+), 543 deletions(-) diff --git a/src/SharpMUTerm.Core/Workspace/Workspace.cs b/src/SharpMUTerm.Core/Workspace/Workspace.cs index 85a4cb1..ad2c621 100644 --- a/src/SharpMUTerm.Core/Workspace/Workspace.cs +++ b/src/SharpMUTerm.Core/Workspace/Workspace.cs @@ -32,14 +32,10 @@ public Workspace(string mainWindowId = "main", string mainTitle = "Main", string /// Rebuilds a workspace from a restored set of windows and a pre-built layout (session resume). /// The two are assumed consistent — every window id referenced by a pane tab should have a window. /// - /// A window restored without a creation sequence is given one here, from the order it arrived - /// in. Windows are numbered by and a configuration - /// written before that field existed carries none, so without this every restored window would sort - /// equal and the numbering would be whatever the sort happened to do. The saved order is the - /// numbering such a workspace was saved under, which is why it is the right seed. Any window that - /// does carry a sequence keeps it, and unsequenced ones are numbered after the highest - /// already taken, so a half-migrated set cannot produce two windows with one number. Same shape, - /// and the same reasoning, as 's restoring constructor. + /// A window restored without a is seeded from the order it + /// arrived in — the numbering it was saved under. Sequenced windows keep theirs and unsequenced ones + /// are numbered after the highest taken, so a half-migrated set cannot produce two windows with one + /// number. Same shape as 's restoring constructor. /// /// public Workspace(IEnumerable windows, WorkspaceLayout layout) @@ -76,38 +72,21 @@ public Workspace(IEnumerable windows, WorkspaceLayout layout) /// Go to … entry for it names. Those are three spellings of this index and there is /// deliberately no second ordering for any of them to drift onto. /// - /// Scoped to the active character, and re-based from 1 for each. It was global — every window - /// in the workspace in one sequence — and that failed on a real client the first day it was used: - /// with three characters sharing pane 1 as tabs, every character's row read ⌥1 because their - /// windows happened to be numbered from the same run. Nine digits also do not stretch across - /// everybody's windows; six windows over three characters already crowds them. Scoped, ⌥1 is - /// this character's main window whoever you are, ⌥2 their first capture, and the digits mean - /// the same thing wherever you stand. + /// Scoped to the active character, re-based from 1. Global numbering gives every character + /// sharing a pane the same ⌥1, and nine digits do not stretch across everybody's windows. + /// Scoped, ⌥1 is this character's main window whoever you are. /// /// - /// An unowned window is in everybody's list. The web view belongs to no session and is - /// reachable from wherever you are, so it takes a digit under each character — a different one under - /// each, since it sits after that character's own windows. That is not a second numbering: this - /// method is exactly the set the rail draws window rows for (its owner filter admits a character's - /// own windows plus the unowned ones), so the digit on the screen and the digit in the chord are the - /// same list read twice. + /// An unowned window is in everybody's list — the web view is reachable from anywhere, so it + /// takes a digit under each character. Not a second numbering: this is exactly the set the rail draws + /// window rows for, so screen and chord are one list read twice. /// /// - /// Creation order, for the reason panes are in creation order. Any ordering that is a function - /// of where a window sits — its tab index, its pane's position — moves when something is - /// inserted before it, so dragging a channel one slot left would renumber every window after it and - /// ⌥4 would stop meaning what it meant while the user was doing something else entirely. A window's - /// number is fixed for as long as it is open, and a new one always appears at the end. - /// - /// - /// The number is the index, not the sequence. Sequences are never reused, so reading them - /// directly would leave holes — close the second of three windows and the survivors would be 1 and - /// 3, with ⌥2 doing nothing while two windows sat on the screen. - /// - /// - /// Placed, because ⌥N has to land somewhere. A window the registry still knows and no pane - /// holds is drawn in the rail as closed; giving it a number would spend a digit on a place - /// there is no way to go, and would shift every window after it for a row that names nothing. + /// Creation order, because any ordering that is a function of where a window sits + /// moves when something is inserted before it, and a digit must not stop meaning what it meant. + /// The number is the index, not the sequence — sequences are never reused, so reading them + /// directly would leave a digit that does nothing with windows still on the screen. Placed only, + /// since a window no pane holds is drawn closed and a digit for it names nowhere to go. /// /// /// @@ -161,33 +140,16 @@ public WorkspaceWindow OpenWindow( /// happens when nothing answers. Counts the line as unread unless the destination is currently /// being read, and returns it. /// - /// This is the resolver a routed line goes through, and the finding half of it is the point. A rule's - /// destination used to be and nothing else, which computes a spawn id and - /// registers a new window when nothing answers to it — so "put this in - /// the window I already have open" was not a thing a rule could ask for however it was spelt, and a - /// route naming a window on the screen opened a second one beside it wearing the same label. - /// - /// - /// What a target may reach is deliberately narrower than "any window with that title". It is - /// this session's own windows, the windows nobody owns, and another character's main window - /// — one alt's channel collected into the pane you actually read. It is not another - /// session's spawn or auxiliary window: two characters running one capture rule get a pane each, and - /// a bare title lookup would collapse them back into one and file the second character's channel - /// under the first, which is the exact defect was given - /// an owner to fix. A main window is admitted across that boundary because it is a window the user - /// opened by connecting, rather than one a rule conjured out of a capture. - /// - /// - /// Only a placed window is a destination. Appending to a window no pane holds writes into a - /// buffer nothing can draw, which from the reader's side is indistinguishable from the rule not - /// firing at all; a closed window is passed over and the line goes somewhere visible. + /// What a target may reach is narrower than "any window with that title": this session's own + /// windows, the unowned ones, and another character's main window — one alt's channel + /// collected into the pane you read. Never another session's spawn window, or two characters running + /// one capture rule would collapse into a single pane filed under whoever matched first. /// /// - /// Finding never creates. A target is often a template with capture groups in it - /// (Channel $1), so the name can be the server's text — and the security property that keeps - /// that bounded is that this arm can only ever land in a window the user already has. Making one out - /// of a captured name still goes through , which puts the matching session's - /// own key on it. + /// Only a placed window is a destination — appending to a window no pane holds is + /// indistinguishable from the rule not firing. Finding never creates: a target may be a + /// template filled from the server's own text, and what bounds that is this arm only ever landing in + /// a window the user already has. /// /// public WorkspaceWindow RouteLine(string target, string? sessionKey = null) @@ -226,15 +188,10 @@ public WorkspaceWindow RouteLine(string target, string? sessionKey = null) .Select(candidate => candidate.Window) .FirstOrDefault(); - // A spawn window the user has since renamed answers to no title, and its rule must go on feeding - // it rather than opening a second pane beside it under the old name. - // - // Placed, like the title lookup above it, and for the same reason: a window the registry still - // knows and no pane holds is *closed* (see the numbering remarks), and routing to one writes the - // channel into a buffer nobody can see. The registry outlives the layout in two ways — a restored - // workspace registers windows a saved layout no longer places — so this is reachable rather than - // theoretical. Falling through instead is not a loss: RouteLine then reaches RouteSpawn, which - // places this very window again under the same id, so the pane reopens with its history in it. + // A renamed spawn window answers to no title, and its rule must go on feeding it rather than + // opening a second pane under the old name. Placed only, like the lookup above: the registry + // outlives the layout, and routing to a closed window writes into a buffer nobody can see. + // Falling through costs nothing — RouteSpawn re-places this same id, history and all. var renamed = _windows.GetValueOrDefault(SpawnWindowId(sessionKey, target)); return best ?? (renamed is not null && Layout.FindWindow(renamed.Id) is not null ? renamed : null); } @@ -260,13 +217,9 @@ public WorkspaceWindow RouteLine(string target, string? sessionKey = null) /// and it reaches here when no window the target names already exists. /// /// - /// The destination is per session, not per workspace. Two connected characters running the - /// same capture rule each get a window of their own; the id carries the owner, so the second - /// session to match cannot land in the first's window. It used to: the id was the target alone, so - /// whoever matched first created the window with their own session key on it and everybody - /// else's lines were appended to somebody else's pane. That was not merely a mixed-up channel — the - /// rail draws window rows for the active character only, so the second character's own channel was - /// filed under the first and was invisible from the character it belonged to. + /// The destination is per session, not per workspace. The id carries the owner, so two + /// characters running one capture rule get a window each and the second cannot land in the first's — + /// where it would also be invisible, since the rail draws window rows for the active character only. /// /// public WorkspaceWindow RouteSpawn(string target, string? sessionKey = null) @@ -277,11 +230,9 @@ public WorkspaceWindow RouteSpawn(string target, string? sessionKey = null) ? existing : Register(new WorkspaceWindow(id, target, WindowKind.Spawn, sessionKey)); - // Placed on the way past, and *not* only when the window is new. The registry outlives the layout - // — a restored workspace registers windows a saved layout no longer places — so a window can be - // known and closed at once, and returning that from a route writes the channel into a buffer - // nobody can see. Making this total is what lets FindRouteTarget decline a closed window and fall - // through here: the pane reopens under the same id, with its history already in it. + // Placed on the way past, not only when the window is new: the registry outlives the layout, so a + // window can be known and closed at once. Making this total is what lets FindRouteTarget decline + // a closed window and fall through here. if (Layout.FindWindow(id) is null) { Layout.AddWindow(id, activate: false); // spawns open in the background and accrue unread @@ -295,9 +246,8 @@ public WorkspaceWindow RouteSpawn(string target, string? sessionKey = null) public const string SpawnPrefix = "spawn:"; /// - /// The owner field of a spawn window that belongs to nobody. A single -, which is not a - /// decimal length and so can never be mistaken for one — that is the whole reason it is not the - /// empty string. + /// The owner field of a spawn window that belongs to nobody. A single - rather than the empty + /// string, because it is not a decimal length and so cannot be mistaken for one. /// private const string Unowned = "-"; @@ -306,23 +256,14 @@ public WorkspaceWindow RouteSpawn(string target, string? sessionKey = null) /// routes to. Unique per (owner, target) and stable for ever, so a reconnect or a restart /// comes back to the pane it left. /// - /// Why the length prefix. A session key and a target are both user-controlled strings that may - /// hold any character, colons included — a world or character can be called a:b and a - /// trigger's SpawnTarget is free text. Joining them with a separator is therefore not - /// injective: (a, b:c) and (a:b, c) would produce one id and collapse two characters' - /// panes into one, which is this defect again in a rarer shape. Writing the owner's length in front - /// of it makes the encoding total and reversible: the digits up to the first colon give the length, - /// exactly that many characters are the owner, one more colon is consumed, and everything left is - /// the target — so distinct pairs cannot produce equal ids, whatever is in them. + /// Why the length prefix. Both halves are user-controlled and may contain colons, so joining + /// them with a separator is not injective — (a, b:c) and (a:b, c) would collapse two + /// characters' panes into one. The owner's length in front makes the encoding total and reversible. /// /// - /// It is legible on purpose rather than hashed. This id is a dictionary key, a value in - /// config.json, and the stem of a RestoreLog file name; a digest would be unambiguous - /// too and would make every one of those unreadable to whoever has to look at them, for no property - /// a reversible encoding does not already have. (The file name's own collision handling is - /// unchanged and unaffected: RestoreLog stores the full id in each file's header and refuses - /// a file whose header names a different window, so a CRC-32 clash on the stem costs one window's - /// log rather than mixing two.) + /// Legible rather than hashed: this id is a dictionary key, a config.json value and the stem + /// of a RestoreLog file name, and a digest would buy no property a reversible encoding lacks + /// while making all three unreadable. /// /// /// The owning world.character session, or null for a window nobody owns. @@ -451,12 +392,9 @@ public void SetUnsentInput(string windowId, bool hasUnsent) /// Records who owns a window, for the case where a session takes over one that already exists. /// Unknown ids are ignored. /// - /// A window's owner is otherwise fixed at creation, which was fine while the first session always - /// created its own: the main window is opened before any session exists and the first session - /// simply adopts it, so without this its keeps naming - /// whoever held it before — and anything that reads ownership (the connection rail listing a - /// character's windows, the command surface subtitling them with their owner) is then reading a - /// stale answer. + /// Ownership is otherwise fixed at creation, and the main window is opened before any session + /// exists — so without this the rail and the command surface would keep naming whoever held it + /// before the adopting session arrived. /// /// public void SetWindowOwner(string windowId, string? sessionKey) @@ -484,10 +422,9 @@ public bool IsVisible(string windowId) => Layout.FindWindow(windowId) is { } pane && pane.ActiveTab == windowId; /// - /// True when a new line arriving in this window would land somewhere the reader can see it: the - /// window is the visible tab of its pane and its output is not scrolled back. This is the - /// condition unread badging turns on — alone is not it, which is why a client - /// that only asked that question badged nothing while the reader sat in their scrollback. + /// True when a line arriving here would land somewhere the reader can see it: the visible tab of its + /// pane and not scrolled back. This is the condition unread badging turns on; + /// alone would badge nothing while the reader sits in their scrollback. /// public bool IsCaughtUp(string windowId) => IsVisible(windowId) && _windows.TryGetValue(windowId, out var window) && !window.ScrolledBack; diff --git a/src/SharpMUTerm.Tui/RailRenderer.cs b/src/SharpMUTerm.Tui/RailRenderer.cs index 5c99503..3534fbc 100644 --- a/src/SharpMUTerm.Tui/RailRenderer.cs +++ b/src/SharpMUTerm.Tui/RailRenderer.cs @@ -11,13 +11,10 @@ namespace SharpMUTerm.Tui; /// unread/unsent detail and the chord that goes to them. Pure so the rail layout is unit-testable. /// /// A row carrying a is wrapped in a [link=…] span, which is how -/// clicking it switches. The span is invisible chrome: [link=…] emits no cell, so the rail -/// looks exactly as it did and — the part that matters beyond looks — its measured width is -/// unchanged, because SharpMUTermApp.RailWidth derives the sidebar's column count from the -/// widest row's visible width. A link that added a cell would resize the sidebar and, through -/// per-pane NAWS, misreport every connected session's pane size. The span covers the row's content -/// but never its leading indent or the empty tail out to the column edge, so a click aimed at the -/// splitter beside the rail lands on nothing. +/// clicking it switches. The span emits no cell, so it cannot change the sidebar's width — which the +/// panes, and through per-pane NAWS every connected server, are sized from. It covers the row's content +/// but not its indent or the tail out to the column edge, so a click aimed at the splitter beside the +/// rail lands on nothing. /// /// internal static class RailRenderer @@ -25,16 +22,12 @@ internal static class RailRenderer /// The rail's rows, as projects them. /// - /// The widest a row may be, in visible cells — the sidebar's own cap. A row longer than that is elided - /// rather than left to wrap, because the sidebar's width is the widest row's clamped - /// (SharpMUTermApp.RailWidth), so any name past the clamp — a web page's title is the easy one — - /// would run onto a second line. A wrapped rail row is the thing the report was about. + /// The widest a row may be in visible cells. The sidebar's width is the widest row's clamped + /// width, so a name past the clamp — a web page's title, most easily — would wrap; elide instead. /// /// - /// The client's own voice for the active theme. A parameter rather than the constants it replaced, - /// because these land on the : measured against the plane they - /// are painted on, the old accent is 1.42:1 and the old draft pen 1.26:1 on the Light theme. Null - /// means the unmeasured base hues, which is what a unit test with no theme wants. + /// The client's own voice for the active theme, held to the legibility floor against the plane the + /// rail is drawn on. Null means the unmeasured base hues, which is what a themeless unit test wants. /// public static List Render( IReadOnlyList rows, int maxWidth = int.MaxValue, ChromeInk? ink = null) @@ -42,19 +35,10 @@ public static List Render( ArgumentNullException.ThrowIfNull(rows); var voice = ink ?? ChromeInk.Default; - // Whether the chord column is drawn, decided once for the whole rail and **per row kind**. - // - // Reserved, because a field that costs a cell only when it has something in it resizes the - // sidebar — and the sidebar's width comes out of the pane area, which every connected server is - // told over NAWS (see UnsentFieldWidth for the reported instance of that bug). Within a kind the - // width therefore does not move as unread arrives, as a draft is typed, or as the ⌥J/⌥K pair - // travels from row to row on a character switch. - // - // Per kind, because the two kinds carry different mechanics and one of them is often empty: a - // window row's chord is the ⌥N numbering, a character row's is the cycle, and with fewer than two - // characters open no character row can have one at all. Reserving across both spent three cells - // on every character row of a client with one character open — which is the common case, and the - // opposite of the complaint this layout change exists to answer. + // Decided once for the whole rail and per row kind. Reserved, so the width does not move as + // unread arrives or the ⌥J/⌥K pair travels between rows; per kind, because a window row's chord + // is the ⌥N numbering and a character row's is the cycle, and with one character open no + // character row can have one — reserving across both would spend three cells on the common case. var reserveWindow = rows.Any(r => r.Kind == RailRowKind.Window && r.Chord is { Length: > 0 }); var reserveCharacter = rows.Any(r => r.Kind == RailRowKind.Character && r.Chord is { Length: > 0 }); @@ -79,20 +63,13 @@ public static List Render( }; /// - /// Renders a row, and if it does not fit, renders it again with its shortened - /// by however much it overran. The label is the only part that may give ground: the accent spine, the - /// connected dot, the unread count, the ✎ pen and the chord column are all information. Measured with the - /// app's own , because that is the measure the sidebar's width is - /// derived from — anything else could agree here and disagree where it matters. + /// Renders a row, and if it overruns, renders it again with the label shortened by the overrun. The + /// label is the only part that may give ground — everything else is information. Measured with + /// , the same measure the sidebar's width is derived from. /// - /// Only the label may vary in width, and only when it changes. Everything else on a row is either - /// one cell whatever it says (the spine, the ● / ○ dot, the ▸ active marker, the ▪ bullet) or occupies a - /// reserved field that is blank when it has nothing to say (, - /// ). That is what stops a keystroke or a line of output resizing the - /// sidebar and, through it, every connected server's terminal size. The chord column is the one - /// remaining variable part and it is deliberately left so: it is absent only while the workspace holds - /// a single window, and it appears when a second one opens — which is a structural change that - /// rebuilds the pane area and re-reports every pane anyway. + /// Only the label varies in width. Everything else is one cell whatever it says or sits in a + /// reserved field that is blank when empty, which is what stops a keystroke or a line of output + /// resizing the sidebar and every connected server's terminal size with it. /// /// private static string Fit(RailRow row, int maxWidth, Func render) @@ -112,8 +89,7 @@ private static string Fit(RailRow row, int maxWidth, Func rende /// /// Renders the collapsed rail (⌃B b): a ~6-col strip of per-world accent separators and, under /// each, its characters as a status dot + initial + unread count. Both stay clickable — an initial - /// is the only handle a collapsed rail offers, so if it did not switch character the strip would be - /// decoration. (It was decoration, and this comment said otherwise, until the rail was wired up.) + /// is the only handle a collapsed rail offers. /// public static List RenderCollapsed(IReadOnlyList rows, ChromeInk? ink = null) { @@ -133,9 +109,8 @@ public static List RenderCollapsed(IReadOnlyList rows, ChromeIn var dot = row.Connected ? "●" : "○"; var name = row.Active ? $"[bold]{initial}[/]" : initial; - // Reserved here too. The collapsed strip is clamped to 4–10 cells, so it moves less — - // but it moves, and a strip that widens when a background world says something is the - // same reflow as the expanded rail's, on a rail chosen for taking no space. + // Reserved here too: the strip is clamped to 4–10 cells, so it moves less, but a + // strip that widens when a background world speaks is the same reflow. lines.Add(Link(row, $"[{Accent(row, voice)}]{dot}[/]{name}{UnreadField(row.Unread, voice)}")); break; } @@ -147,21 +122,11 @@ public static List RenderCollapsed(IReadOnlyList rows, ChromeIn /// /// A character row: its chord, the active marker, the connected dot, the name and its unread total. /// - /// The chord leads, in the same column the window rows put theirs. It is ⌥J on the - /// character one step forward in the cycle and ⌥K one step back — the only two that are a - /// single keystroke away — and blank on everybody else, including the row you are standing on, whose - /// ▸ already says so. It used to be the chord of that character's own window, from - /// when window numbering ran across the whole workspace; scoped to the active character, that printed - /// ⌥1 against every character on the screen, which is the confusion this replaced. - /// - /// - /// Reserved, and on the left. The field is cells whether or not - /// there is a chord, so a row does not change width when the cycle moves — the same rule the pen and - /// the unread count follow, and for the same reason: the rail's width is its widest row, the sidebar - /// takes its columns out of the pane area, and every connected server is told its pane's size. And it - /// is on the left because the reader's complaint was the gap: with the chord at the end of the row it - /// sat behind two blank status fields, five cells of nothing between a window's name and the key that - /// goes to it. + /// The chord leads, in the column the window rows use. It is ⌥J on the character one step + /// forward in the cycle and ⌥K one step back — the only two a single keystroke away — and + /// blank on everybody else, including the row you are on, whose ▸ already says so. Reserved + /// () so a row does not change width as the cycle moves, and leading + /// rather than trailing so no blank status field separates it from the name it names. /// /// private static string Character(RailRow row, bool reserve, ChromeInk ink) @@ -174,15 +139,10 @@ private static string Character(RailRow row, bool reserve, ChromeInk ink) } /// - /// Cells the sidebar keeps for a row's chord, whether or not it has one. Three: the sigil, one - /// digit or letter, and the space that separates it from the row's own glyph. - /// - /// Reserved for the reason and are — a - /// cell that appears only when there is something to say resizes the sidebar, and the sidebar's width - /// comes out of the panes, which every connected server is told over NAWS. This one moves on events a - /// reader does not think of as structural: a capture window opening past the ninth loses its chord, and - /// the ⌥J/⌥K pair moves from row to row on every character switch. - /// + /// Cells kept for a row's chord whether or not it has one: the sigil, one digit or letter, and a + /// separating space. Reserved for the reason is, and this one moves on + /// events a reader would not call structural — a window past the ninth loses its chord, and the + /// ⌥J/⌥K pair moves on every character switch. /// private const int ChordFieldWidth = 3; @@ -210,17 +170,10 @@ private static string ChordField(string? chord, bool reserve) } /// - /// Cells the sidebar keeps for a row's unsent-draft pen, whether or not there is one. Two: the - /// glyph and the space that separates it from the label. - /// - /// This is the reported bug. The pen used to be emitted only when there was a draft, so the row grew by - /// two cells on the first keystroke of every line — and SharpMUTermApp.RailWidth takes - /// the sidebar's column count from its widest row, so the column grew, the panes shrank, and per-pane - /// NAWS re-announced a new terminal size to every connected server, which reflowed the game's own - /// output. Starting to type made the screen jump. The same reasoning is why focus is indicated by - /// recolouring and never by spending a cell; here the cell has to be spent, so it is spent - /// unconditionally. - /// + /// Cells kept for a row's unsent-draft pen whether or not there is one. Nothing volatile on a row + /// may cost a cell only when it has something to say: the sidebar's width is its widest row, the + /// panes are what is left over, and per-pane NAWS re-announces that size to every connected server — + /// so a pen appearing on the first keystroke of a line reflows the game's own output. /// private const int UnsentFieldWidth = 2; @@ -236,16 +189,10 @@ private static string Unsent(bool unsent, ChromeInk ink) => unsent ? $" [{ink.Draft}]{Glyphs.Draft}[/]" : new string(' ', UnsentFieldWidth); /// - /// An unread count in a fixed-width field, right-aligned, blank at zero. Reserved for the same reason - /// the pen is () and with more urgency: unread arrives unbidden from - /// the wire, so an unreserved badge resizes the sidebar — and every connected server's idea of its - /// terminal — on a line of output the reader did not ask for, and again at 9 → 10 when it takes a - /// second digit. The cap is what makes the field finite: a count past - /// reads 99+, which is the same three cells and the same information at a glance. - /// - /// Both the wording and the colour come from , which the pane tab labels draw - /// from as well, so the sidebar and the strip cannot come to say different things about one count. - /// + /// An unread count in a fixed-width field, right-aligned, blank at zero. Reserved for + /// 's reason and more urgently: unread arrives unbidden from the wire. + /// The cap is what makes the field finite. Wording and colour both come from + /// , so the sidebar and the tab strip cannot disagree about one count. /// private static string UnreadField(int unread, ChromeInk ink) => unread <= 0 @@ -255,22 +202,13 @@ private static string UnreadField(int unread, ChromeInk ink) => /// /// A window row: how you get to it, then what it is, then its badges. /// - /// The chord leads, in the same reserved column the character rows use. That is the reported - /// complaint — "there is still way too much room after a window's name before it hits 'alt-1'". The - /// gap was the two badge fields, which are blank far more often than not and sat between the name and - /// the key. They cannot be removed (see : a field that costs a cell only - /// when it has something to say resizes the sidebar on a keystroke or a line of output, and the - /// sidebar's width comes out of the panes, which every connected server is told over NAWS) — so the - /// chord moved to the front instead, where nothing blank separates it from the name it belongs to, - /// and the badges ended up at the right edge where status belongs. The row's measured width is - /// unchanged by the move; only the order is. + /// The chord leads, in the reserved column the character rows use, so no blank status field sits + /// between a name and the key that reaches it; the badges trail, where status belongs. /// /// - /// The column earns its place only once the character holds a second window: with one, there is one - /// place to be, so the model leaves null and the field is not drawn at - /// all. A window past the ninth has no chord and shows blanks, which is the honest reading — the row - /// is still clickable and still reachable by ⌃N and the tab strip, and a column claiming a key that - /// would go somewhere else is the one thing this numbering exists to prevent. + /// The column is drawn only once the character holds a second window — with one there is one place to + /// be. A window past the ninth shows blanks: it is still clickable and still reachable by ⌃N, and a + /// column claiming a key that would go elsewhere is what this numbering exists to prevent. /// /// /// closed is a state rather than a destination, so it is drawn where the badges are rather than @@ -302,8 +240,7 @@ private static string Link(RailRow row, string content) => /// /// A row's own accent as a markup hex, or the client's when it has none — either way held to the /// legibility floor against the plane the sidebar is drawn on. A world's accent is a colour a user - /// picked in F5 and this is where it meets a plane: unlifted, the demo's own #ff9f1c measures - /// 1.03:1 on the Light theme's backdrop. + /// picked in F5, and this is where it meets a plane it was not chosen against. /// private static string Accent(RailRow row, ChromeInk ink) => row.Accent.Kind == TerminalColorKind.Rgb diff --git a/src/SharpMUTerm.Tui/TabTitles.cs b/src/SharpMUTerm.Tui/TabTitles.cs index 1772c9f..3bfa1a3 100644 --- a/src/SharpMUTerm.Tui/TabTitles.cs +++ b/src/SharpMUTerm.Tui/TabTitles.cs @@ -9,26 +9,13 @@ namespace SharpMUTerm.Tui; /// the one currently focused. Pure so it can be unit-tested without a terminal. /// /// -/// A tab title is markup, not plain text. This file used to say the opposite, and the claim -/// had a cost: it is why the unread count went out untinted and why a window title was never escaped. -/// TabControl.Rendering runs each label through MarkupParser.Parse, and every width it is -/// measured by — the header paint, the strip's desired width, and the click hit test that decides which -/// tab and which × a press landed on — is MarkupParser.StripLength. So a colour tag here -/// costs no cells and moves no hit test, which is what makes the activity tint affordable on a -/// surface where a cell may not be spent. -/// It also means configured and world-supplied text has to be escaped on the way in -/// (): a window titled [Chat] — or a web view titled from the page -/// it loaded — would otherwise have that eaten as a tag by the parser and by the hit test alike. -/// The focused pane is marked by the ▌ this emits plus the lit plane it is painted -/// on (), because a pane cannot be given a border without changing -/// its rectangle and so the per-pane NAWS size it reports. The ▌ is deliberately left -/// outside the activity tint: focus and activity are independent, a tab can have both, and a -/// marker that changed colour when a line arrived would be reporting the wrong fact. -/// The close affordance is deliberately not here. A ✕ written into the label is -/// just text: the framework's tab hit test sees it as part of the title and a click on it merely -/// selects the tab. The real close button is TabPage.IsClosable, which the framework draws -/// itself and hit-tests into TabControl.TabCloseRequested — see -/// SharpMUTermApp.BuildPaneTabs. +/// A tab title is markup. Every width the strip is measured by — the header paint, its +/// desired width, and the click hit test that picks the tab and its × — is +/// MarkupParser.StripLength, so a tag here costs no cells and moves no hit test. It also means +/// configured and world-supplied text must be escaped on the way in: a window titled [Chat] +/// would otherwise be eaten as a tag by the parser and by the hit test alike. +/// The close affordance is not here. A ✕ in the label is just text the hit test +/// reads as part of the title; the real button is TabPage.IsClosable. /// internal static class TabTitles { @@ -37,17 +24,13 @@ internal static class TabTitles /// The session key of the character in focus, so a window belonging to another one can be marked. /// /// - /// Whether this tab is the active tab of the focused pane — the one pane the - /// scrollback keys, the ⌃B commands and the Ctrl+arrows all act on. It gets a leading ▌: the - /// pane's own plane is lit as well, and the glyph is what carries the signal on a terminal where the - /// two planes flatten together. It leads rather than trails because that is the edge of the strip a - /// reader's eye starts at, and it is on the active tab only, so a pane never shows two. + /// Whether this is the focused pane's tab, which earns a leading ▌ — the shape half of the + /// focus cue, for a terminal where the lit plane behind it flattens. /// /// - /// Whether this is the tab its pane is showing. It is emboldened in every pane, focused or - /// not, so the selection reads on a terminal that flattens the chips behind it — the shape half of - /// what the chip colours say. Independent of : an unfocused pane still - /// has a tab in front of it. + /// Whether this is the tab its pane is showing, emboldened in every pane so the selection + /// survives a flattened palette. Independent of : an unfocused pane + /// still has a tab in front of it. /// public static string For( WorkspaceWindow window, @@ -58,23 +41,18 @@ public static string For( { ArgumentNullException.ThrowIfNull(window); - // A child window (spawn/aux) carries its owning connection as an "Owner - " prefix so it stays - // traceable to its character once dragged into another pane. A character's own main window - // needs no prefix — the focused-character context already identifies it. + // A child window carries its owner as a prefix so it stays traceable once dragged into another + // pane; a character's own main window needs none, the focused-character context names it. var owner = window.Kind != WindowKind.Main && !string.IsNullOrEmpty(window.OwnerLabel) ? MarkupText.Escape(window.OwnerLabel) + " - " : string.Empty; - // Capped the way the sidebar's badge is, from the same formatter. Not for the sidebar's reason — - // a tab strip is laid out along a row the framework fills to the pane's edge, so a label that grows - // moves the tabs beside it and never the pane's own rectangle. The cap is here so the two surfaces - // reading one number cannot print different answers, and so an unbounded count arriving from the - // wire cannot push a pane's other tabs off the end of a narrow strip. + // Capped through the sidebar's own formatter, so the two surfaces reading one number cannot print + // different answers and a count from the wire cannot push a narrow strip's later tabs off the end. var unread = window.Unread > 0 ? $" ({UnreadBadge.Format(window.Unread)})" : string.Empty; var pen = window.HasUnsentInput ? $" {Glyphs.Draft}" : string.Empty; - // ⌁ marks a window owned by a character other than the focused one, so a pane holding - // borrowed windows stays traceable to their owners. + // ⌁ marks a window owned by a character other than the focused one. var cross = focusedCharacterKey is not null && window.SessionKey is not null && !string.Equals(window.SessionKey, focusedCharacterKey, StringComparison.Ordinal) @@ -83,10 +61,8 @@ public static string For( var focus = focusedPane ? Glyphs.FocusedPane + " " : string.Empty; - // The activity tint and the selection weight cover the window's name and its count and stop - // there: the ▌ ahead of them is the focus marker and the ✎ / ⌁ behind them are other facts, and - // a signal that recoloured those would be claiming they had changed too. One tag rather than two - // nested ones, because a selected tab can also be unread. Zero cells — see the class remarks. + // Tint and weight cover the name and count only: the ▌ ahead and the ✎ / ⌁ behind are other + // facts. One tag rather than two nested, because a selected tab can also be unread. var named = owner + MarkupText.Escape(window.Title) + unread; var style = (selected, window.Unread > 0) switch { diff --git a/src/SharpMUTerm.Tui/UnreadBadge.cs b/src/SharpMUTerm.Tui/UnreadBadge.cs index 1e00c29..c1b91ed 100644 --- a/src/SharpMUTerm.Tui/UnreadBadge.cs +++ b/src/SharpMUTerm.Tui/UnreadBadge.cs @@ -4,13 +4,8 @@ namespace SharpMUTerm.Tui; /// /// The one spelling of an unread count, shared by the two surfaces that draw one: the connection rail's -/// per-row badge () and a pane's tab label (). -/// -/// It is shared because the sidebar and the tab strip are two views of a single number — -/// WorkspaceWindow.Unread — and two formatters would eventually disagree about it. They already -/// did: the rail capped at while the tab printed the raw integer, so a busy channel -/// read 99+ in the sidebar and (4127) on its tab, which is two answers to one question. -/// +/// per-row badge () and a pane's tab label (). Shared +/// because both are views of a single number, and two formatters would eventually disagree about it. /// internal static class UnreadBadge { @@ -21,21 +16,12 @@ internal static class UnreadBadge internal const int FieldWidth = 3; /// - /// The markup colour a count is drawn in — the app accent, on both surfaces. + /// The markup colour a count is drawn in — the client's accent for the active theme, resolved rather + /// than fixed because both surfaces it lands on move with the theme. /// - /// One colour on purpose. It is not the focus colour and cannot be mistaken for it: focus is said - /// entirely in backgrounds drawn from the theme's own chrome family - /// ( behind a pane, - /// behind the focused pane's active tab chip), while this is a foreground and the one hue in - /// the workspace that no plane is ever painted in. So the two cues are orthogonal — a tab can be - /// focused, unread, both or neither, and each of the four states reads distinctly. - /// - /// - /// - /// It is the client's own accent for the active theme rather than a constant, because both surfaces - /// it appears on move with the theme: a rail badge lands on the backdrop and a tab badge lands in a - /// pane. As a fixed #00f5b7 it measured 1.41:1 on the Light theme's backdrop and 1.42:1 on its - /// focused pane — a count the reader was shown and could not read. + /// A foreground, and the one hue in the workspace no plane is painted in, so it cannot be + /// mistaken for focus (a background) or selection (weight): a tab can be any combination of the + /// three and each reads distinctly. /// /// internal static string TintFor(ChromeInk? ink) => (ink ?? ChromeInk.Default).Accent; diff --git a/src/SharpMUTerm.Tui/WorkspacePalette.cs b/src/SharpMUTerm.Tui/WorkspacePalette.cs index d4810fe..5e76eaa 100644 --- a/src/SharpMUTerm.Tui/WorkspacePalette.cs +++ b/src/SharpMUTerm.Tui/WorkspacePalette.cs @@ -5,100 +5,53 @@ namespace SharpMUTerm.Tui; /// -/// The three tones the main workspace is drawn on — the output a pane paints, -/// the everything that is not a pane sits on, and the -/// hairline between panes. +/// The tones the main workspace is drawn on, derived from the active theme rather than written as +/// hexes so every theme gets the same relationships: under a pane, +/// under everything else, between panes. /// -/// They are derived from the active theme rather than written down as hexes, because the -/// theme is the user's (Dark / Light / Solarized Dark / a hand-written one) and a fixed pair would be -/// right for exactly one of them. What is fixed is the relationship, and it is the one the -/// settings screens already use: sits a little over three quarters -/// of the way from black to , so a card reads as raised off its -/// backdrop without either tone leaving the family. The workspace uses that same step, which is why -/// F5 and the pane behind it look like two views of one application. -/// -/// -/// The surface is deliberately not the theme's plain text background: it is that background -/// nudged a quarter of the way toward the theme's own chrome tone, so the output area carries a hint -/// of the colour the header and input bands are already painted in. MU* text is unaffected — a span -/// with the default background emits no background at all (see ), so it -/// takes whatever surface it is drawn on. -/// -/// -/// A pane's plane can also carry whose pane it is (), and the two facts a -/// plane states are kept on separate channels so neither can be mistaken for the other: identity is -/// hue and focus is luminance. moves the surface's colour, -/// multiplies its brightness, and because the focus step is a ratio it lands the same relative -/// distance above every plane it is applied to. -/// -/// -/// The command line wears the same hue ( / -/// ), so the bar under a pane says whose connection ⏎ is aimed -/// at. It takes the hue and not the step the pane takes: on the input -/// band luminance is already spoken for — it is the whole armed-versus-idle cue — and a colour that -/// moved it too would put a second fact on a channel that already carries one. On the pane, luminance -/// carries focus as a ratio, which a step applied equally to all six tints leaves untouched. +/// A plane says two things and they are kept on separate channels: identity is hue +/// () and focus is luminance (). Focus is a +/// multiplication, so it lands the same ratio above every plane it is applied to and neither cue can be +/// read as the other. /// /// internal static class WorkspacePalette { /// - /// How far the surface moves from the theme's text background toward its chrome background. Small - /// on purpose: this tints the plane the game's own colours are read against, and anything the eye - /// can name as a colour would be competing with them. + /// How far the surface moves from the theme's text background toward its chrome background. Small: + /// this is the plane the game's own colours are read against. /// private const double ChromeTint = 0.25; /// - /// The backdrop as a fraction of the surface, taken from 's own pair — - /// the mean of PanelBg ÷ EditBg across the three channels. Sharing the settings screens' - /// step is the whole point: one application, one idea of how far a card floats. + /// The backdrop as a fraction of the surface — the mean of PanelBg ÷ EditBg from + /// , so the workspace and the settings screens agree on how far a card + /// floats. /// private const double BackdropScale = 0.757; /// - /// How far the hairline between two panes moves from the surface toward the theme's border - /// colour. Lifted off the surface rather than off the backdrop, because the rule has both planes - /// beside it — the backdrop where it runs down the side of the rail, the surface where it separates - /// two panes — and only the surface end guarantees a step away from each. (On a dark theme that - /// lands it lighter than both, exactly where sits on the settings - /// screens; on a light one it lands between them, which is where a hairline belongs there.) Short of - /// the border itself: a rule on reads fine once and shouts at four panes, - /// and a divider's job is to be found, not noticed. + /// How far the hairline moves from the surface toward . Measured + /// off the surface because the rule has both planes beside it and only that end guarantees a step + /// away from each; short of the border itself, since a divider's job is to be found, not noticed. /// private const double RuleLift = 0.45; /// - /// How far the focused plane is lifted off the unfocused one, as a fraction of it. Like - /// this is not a number somebody liked: it is the mean of - /// CursorBg ÷ EditBg across the three channels — the step the settings screens already take - /// to say "the keyboard is here", measured off the card the cursor bar sits on - /// ( is documented as exactly that). Reusing it means the - /// workspace and F5 do not have two different ideas of what focus looks like, and it is a step of - /// about three fifths rather than the thirteen points per channel the two input bands used to - /// differ by — which is the whole complaint. - /// - /// It is a scale, not a mix toward a hue, deliberately: multiplying keeps the theme's own - /// colour and changes only its luminance, so the cue survives a monochrome terminal and a - /// colour-blind reader, and a light theme lifts the same way a dark one does. - /// + /// How far a focused plane is lifted off an unfocused one — the mean of + /// CursorBg ÷ EditBg, the step the settings screens take to say "the keyboard is here". + /// A scale rather than a mix toward a hue, so the cue is pure luminance and survives a monochrome + /// terminal. /// private const double FocusScale = 1.595; - /// - /// How far the idle input band is recessed from the theme's chrome tone. Like the other constants - /// here it is measured off what the design already chose: the mean of the old hardcoded - /// #262b3a over the default theme's , across the three - /// channels. The idle band therefore lands where it always did — the tone was never the complaint — - /// while moves away from it by a step the eye can actually find. - /// + /// How far the idle input band is recessed from the theme's chrome tone. private const double IdleBandScale = 0.814; /// - /// How far the armed input band leans toward . Enough to read as a - /// different colour and not merely a brighter one, which is the cue that survives a reader - /// who sees luminance but not hue being given the opposite problem; small enough that the band is - /// still the theme's chrome rather than a stripe of accent across the bottom of the window. + /// How far the armed input band leans toward — enough to read as a + /// different colour rather than only a brighter one, so the pair survives a reader who sees + /// luminance but not hue. /// private const double PromptTint = 0.28; @@ -110,65 +63,30 @@ internal static Rgb Surface(Theme theme) } /// - /// The plane the focused pane's output is painted on, and the band behind the command line - /// ⏎ sends from. One tone for both on purpose: "this is where you are" should be one thing to learn, - /// not two, and the two questions a user has — which pane am I acting on, which line am I typing - /// into — are then answered by the same colour in the two places it can appear. - /// - /// It costs no cells. A focused pane is the same rectangle as an unfocused one, repainted; that - /// matters because per-pane NAWS is derived from the pane rectangle, so a border or a marker column - /// would re-announce a different terminal size to the server on every focus change and reflow the - /// game's own output. See SharpMUTermApp.PaneOutputRects. - /// + /// The plane the focused pane is painted on, and the band behind the command line ⏎ sends from. One + /// tone for both, so "you are here" is one thing to learn. /// internal static Rgb Focus(Theme theme) => Focus(Surface(theme)); /// - /// The same focus step, taken off whatever plane a pane is actually painted on — which is - /// for a character with no tint and for one that has chosen - /// a colour. It is the one step either way: focus is a multiplication, so it lands the same - /// distance above every plane it is applied to, and a tinted pane is therefore exactly as visibly - /// focused as an untinted one. - /// - /// This overload is what keeps the two cues from fighting. A tint changes only the plane's - /// hue (see ); focus changes only its luminance. Neither can be - /// read as the other, and no combination of the two produces a pane that is ambiguous about which - /// question it is answering. - /// + /// The focus step taken off whatever plane a pane is actually painted on, tinted or not. It is a + /// multiplication, so a tinted pane is exactly as visibly focused as an untinted one. /// internal static Rgb Focus(Rgb plane) => Scale(plane, FocusScale); /// - /// The plane a character's pane is painted on: taken one - /// step down and pushed toward the tint's hue at that luminance, so a workspace holding - /// several characters says whose pane is whose. returns the surface - /// itself, byte for byte — an untinted client is painted exactly as it was before this existed. - /// - /// All six tints sit at exactly one luminance, and it is not the surface's. The first half is - /// the legibility guarantee and is by construction: the plane is re-lit to the target - /// () before the anchor is mixed into it, and luma is linear in the - /// channels, so both ends of the blend share that luma and so does every point between them — for - /// any and on any theme. No character's pane is brighter than another's, - /// and the focus step therefore lands the same ratio above each. - /// + /// The plane a character's pane is painted on, so a workspace holding several characters says whose + /// pane is whose. returns the surface byte for byte. /// - /// The second half is a deliberate departure from the first cut of this feature, which held the - /// tinted plane at the untinted surface's exact luma. MU* servers are written for black - /// terminals and their own bright ANSI is what is read on this plane, so the pane wants to be - /// darker than the client's chrome rather than level with it. What is given up is the claim that a - /// tint changes no contrast ratio the theme was designed around; what is kept is the reason that - /// claim was made — the game's text sits on a plane one step away from it on a dark theme, - /// which raises contrast rather than lowering it. The step is bounded and the bound is not a - /// preference: see . + /// All six tints sit at one luminance, by construction: the plane is re-lit to + /// before the anchor is mixed in, and luma is linear in the channels, so both + /// ends of the blend share it and so does every point between. No character's pane is brighter than + /// another's. That luminance sits below the untinted surface because MU* servers are written + /// for black terminals and their bright ANSI is what is read here. /// /// - /// Two consequences worth stating rather than discovering. The tint carries no information a - /// monochrome terminal can show, and it must not: the cue that has to survive a lost hue is - /// focus, and focus is luminance. A reader who cannot use the tint loses nothing they did not - /// already have — the sidebar and the tab title still name the character in words. And it is a - /// truecolor cue: a 256-colour terminal quantises these planes onto a handful of entries, which - /// degrades to a pane that is merely dark rather than to a wrong answer. That is why - /// WorkspacePaletteTests.FocusSurvivesA256ColourTerminal exists and has no tint counterpart. + /// The cue is truecolor and says nothing on a monochrome terminal, deliberately: what has to survive + /// a lost hue is focus, and the rail and the tab title still name the character in words. /// /// internal static Rgb Tint(Theme theme, PaneTint tint) @@ -178,10 +96,10 @@ internal static Rgb Tint(Theme theme, PaneTint tint) } /// - /// One plane wearing one character's colour: re-lit to - /// of its own luminance, then mixed toward the tint's hue at that same - /// luminance. The two callers hand it two different depths and the difference is the whole - /// composition rule — see the type's own summary. + /// One plane wearing one character's colour: re-lit to of its own + /// luminance, then mixed toward the tint's hue at that same luminance. The two callers pass + /// different depths, which is the whole composition rule — a pane takes the step down, the command + /// line takes hue only. /// private static Rgb Tinted(Rgb plane, PaneTint tint, double depth) { @@ -195,45 +113,26 @@ private static Rgb Tinted(Rgb plane, PaneTint tint, double depth) } /// - /// How far below the untinted surface a tinted pane sits, as a fraction of its luminance. It is - /// bounded from below by the focus step and the bound is arithmetic, not taste. A client may - /// hold tinted and untinted characters at once, so the four planes on screen are - /// tinted, surface, tinted·, surface·FocusScale — and if the depth - /// ever reached 1 ÷ FocusScale a focused tinted pane would be no brighter than an - /// unfocused untinted one, which is the focus cue reporting the wrong fact for the reason - /// the tint work exists to prevent. The value is the geometric mean of that floor and no darkening - /// at all, so the untinted surface sits exactly midway — in ratio, √FocusScale ≈ 1.26 either way — - /// between a tinted pane and a focused tinted one. Every focused pane on the screen is then brighter - /// than every unfocused one, whatever colours are in play. + /// How far below the untinted surface a tinted pane sits. Bounded from below by the focus step, + /// arithmetically: a client may hold tinted and untinted characters at once, and a depth reaching + /// 1 ÷ would leave a focused tinted pane no brighter than an + /// unfocused untinted one. The geometric mean of that floor and no darkening puts the untinted + /// surface midway in ratio, so every focused pane outshines every unfocused one whatever the colours. /// private static readonly double TintDepth = 1.0 / Math.Sqrt(FocusScale); /// - /// How far a tinted plane travels from its own tone toward the tint's hue. It is a pure chroma - /// control — has already taken the brightness question away — so this is the - /// answer to "how coloured", and nothing else. It went up a little with , and - /// only a little: chroma is bounded by luminance, so the same fraction of a darker plane is a fainter - /// colour, but the work of keeping the six apart is the s' and not this constant's. - /// Still well short of the whole way, because this is the plane the game's own colours are read - /// against and the thing being identified is a character, not an alarm — pushed to the anchor itself - /// the pane reads as a coloured panel, which is a client shouting a fact nobody asked it to repeat. + /// How far a tinted plane travels toward its hue — pure chroma, since has + /// already settled the brightness. Short of the whole way: the thing being identified is a character, + /// not an alarm, and this is the plane the game's colours are read against. /// private const double TintStrength = 0.75; /// - /// The hue each named tint stands for, as a reference colour. These are never painted: - /// re-lights each one to the plane it is going onto, so what is fixed here is - /// the hue and the saturation, and the brightness is the theme's. - /// - /// Six, spread around the wheel at roughly even spacing (blue → blue-green → green → amber → - /// red-orange → violet), because the failure this feature has is two characters whose colours a - /// reader has to compare rather than recognise. They are saturated, and that is what the darker - /// target bought: re-lit down to a dark theme's pane a muted anchor has almost no chroma left, and - /// the first set — muted, at the surface's own brightness — left the two closest of the six ΔE 8.2 - /// apart on the default theme, with the nearest only ΔE 7.7 from the untinted plane. Measured the - /// same way, these are ΔE 14.4 from each other and ΔE 12.1 from the untinted plane, on a plane that - /// is also a fifth darker. - /// + /// The hue each named tint stands for. Never painted — re-lights each to the + /// plane it is going onto, so what is fixed here is hue and saturation. Six, spread around the wheel, + /// so two characters' colours are recognised rather than compared; saturated, because chroma is + /// bounded by luminance and these are re-lit onto a dark plane. /// private static Rgb? Anchor(PaneTint tint) => tint switch { @@ -247,12 +146,10 @@ private static Rgb Tinted(Rgb plane, PaneTint tint, double depth) }; /// - /// The same colour at a different brightness — luma, keeping the hue. It - /// takes the two directions separately because only one of them is safe in each: a colour brighter - /// than the target is scaled down (a multiplication can never leave the byte range), and one - /// darker is blended toward white, which reaches any luminance up to 255 without clipping. Scaling - /// upward would clip the strongest channel first and so would silently change the hue — on a light - /// theme, where every anchor has to travel up, it would change it beyond recognition. + /// The same colour at luma, keeping the hue. The two directions are taken + /// separately because only one is safe in each: darkening is a scale (which cannot leave the byte + /// range), brightening is a blend toward white. Scaling upward would clip the strongest channel and + /// silently change the hue. /// private static Rgb AtLuma(Rgb rgb, double target) { @@ -268,37 +165,25 @@ private static Rgb AtLuma(Rgb rgb, double target) } /// - /// Perceived brightness, on the same weights the rest of this codebase measures a colour by (ITU-R - /// BT.601, which is what WorkspacePaletteTests and FocusIndicationTests already use). - /// Being linear in the channels is the property leans on, not the - /// particular weights. + /// Perceived brightness (ITU-R BT.601). Being linear in the channels is the property + /// leans on, not the particular weights. /// private static double Luma(Rgb rgb) => ((rgb.R * 299.0) + (rgb.G * 587.0) + (rgb.B * 114.0)) / 1000.0; /// - /// The chrome band a command line is drawn on when ⏎ will not send from it. It is the theme's - /// status/chrome tone recessed by — the input area belongs to the chrome - /// family, not to the pane surface, which is why it is measured off - /// and not off . - /// - /// It sits where the design's own idle band sat; the tone was never the complaint. What was wrong was - /// the distance to the armed one — the two hardcoded hexes were a ratio of about 1.33 apart, - /// thirteen points per channel, which is genuinely close to invisible. now - /// takes the same focus step everything else does, and picks up the theme's prompt hue on the way. - /// + /// The band a command line is drawn on when ⏎ will not send from it. Measured off + /// rather than , because the input area + /// belongs to the chrome family. /// internal static Rgb IdleBand(Theme theme) => IdleBand(theme, PaneTint.None); /// - /// The same band wearing a character's colour — the bar under a tinted pane, so a glance at the - /// command line says whose connection ⏎ is aimed at without reading the prompt. + /// The same band wearing a character's colour, so a glance at the command line says whose connection + /// ⏎ is aimed at. /// - /// Hue only: the band keeps its luminance exactly. On a pane the tint takes a - /// step down as well, and it must not here, because luminance on this row is - /// already the armed-versus-idle cue — the one thing the input area says with brightness. Leaving it - /// alone means the step between the two bands is the step it has always been, in every colour and on - /// every theme, and that the ink chosen to be read on these bands () keeps the - /// contrast it was picked with. + /// Hue only — the band keeps its luminance exactly. On this row luminance is already the + /// armed-versus-idle cue, so a colour that moved it too would put a second fact on a channel that + /// carries one, and would lose the contrast it was picked with. /// /// internal static Rgb IdleBand(Theme theme, PaneTint tint) @@ -308,26 +193,16 @@ internal static Rgb IdleBand(Theme theme, PaneTint tint) } /// - /// The band behind the command line ⏎ does send from: the idle band lifted by the same - /// a focused pane is lifted by, and then pushed a little toward - /// — the theme's own colour for a prompt, which is what this band is. - /// - /// The hue is affordable here and not on a pane: a pane's plane is what the game's own colours are - /// read against, so stays a pure luminance lift, while the input band is chrome - /// and was already tinted. Between them the armed and idle bands now differ in luminance and - /// hue, on top of the bold-versus-dim prompt and the bright-versus-dim ink — four cues, of which - /// three survive a terminal that cannot render the fourth. - /// + /// The band behind the command line ⏎ does send from: the idle band lifted by + /// and leaned toward . The hue is affordable here + /// and not on a pane, because a pane's plane is what the game's own colours are read against. /// internal static Rgb ArmedBand(Theme theme) => ArmedBand(theme, PaneTint.None); /// /// The armed band over a tinted idle one. Derived from rather - /// than tinted in its own right, and that ordering is the point: the lift and the lean toward - /// are applied after the character's hue, so both cues survive a - /// tint — the armed bar is brighter than the idle one by the step it always was, and still bluer than - /// it by the theme's own prompt colour. Tinting the armed band directly would have overwritten that - /// lean with the character's hue and left the pair differing in brightness alone. + /// than tinted itself, so the lift and the prompt lean land after the character's hue and + /// both cues survive a tint. /// internal static Rgb ArmedBand(Theme theme, PaneTint tint) { @@ -336,14 +211,10 @@ internal static Rgb ArmedBand(Theme theme, PaneTint tint) } /// - /// Text on an idle band: the theme's foreground pulled most of the way down to that band. Dimmer - /// than the armed bar's ink, so the pair still reads apart if a terminal flattens both backgrounds. - /// - /// Measured against the untinted band, and safely so: a tint moves that band's hue and not - /// its luminance (), so this ink keeps the contrast it was - /// picked with whatever colour the bar is wearing. One ink for every tint also keeps the tab chips, - /// which share it, from acquiring a per-character text colour nobody asked for. - /// + /// Text on an idle band, dimmer than the armed bar's ink so the pair reads apart if a terminal + /// flattens both backgrounds. Measured against the untinted band — safely, since a tint moves that + /// band's hue and not its luminance — which also keeps the tab chips that share it from acquiring a + /// per-character text colour. /// internal static Rgb IdleInk(Theme theme) { @@ -352,21 +223,16 @@ internal static Rgb IdleInk(Theme theme) } /// - /// The plane everything that is not a pane sits on: the connection rail, the status line, and the - /// gaps a split leaves between panes. Recessed relative to , so an empty pane - /// is still a visible rectangle and a workspace of many panes reads as cards on a desk. + /// The plane everything that is not a pane sits on: the rail, the status line, and the gaps a split + /// leaves between panes. /// internal static Rgb Backdrop(Theme theme) => Recessed(Surface(theme)); /// - /// One step behind a plane — the backdrop's own step, reused for the chips of a tab strip's - /// unselected tabs. - /// - /// A chip states one fact, and states it relative to its own strip: the selected tab is painted the - /// plane its page is painted on, its siblings are recessed from it. Pane focus is not a term, because - /// it is already in the plane — so the selection cue is one ratio in every strip, on every theme, - /// under every tint, and the two questions a strip answers stay on separate channels. - /// + /// One step behind a plane — the backdrop's step, reused for the chips of a tab strip's unselected + /// tabs. A chip states one fact relative to its own strip: the selected tab is painted the plane its + /// page is painted on, its siblings are recessed from it. Pane focus is not a term, because it is + /// already in the plane. /// internal static Rgb Recessed(Rgb plane) => Scale(plane, BackdropScale); @@ -374,13 +240,8 @@ internal static Rgb IdleInk(Theme theme) internal static Rgb Rule(Theme theme) => Mix(Surface(theme), theme.Border, RuleLift); /// - /// The dim chrome the header ribbon's character segment sits on — the theme's chrome band lifted - /// toward its own foreground, so the segment reads as a distinct chip against the band it ends on. - /// - /// Derived rather than the fixed #3f4859 it was. That literal was picked against a dark theme - /// and is a dark chip whatever the theme, so under Light the world's accent — text on this - /// chip — was drawn at 1.53:1 while everything around it had been resolved for a light plane. - /// + /// The chrome the header ribbon's character segment sits on — the chrome band lifted toward its own + /// foreground, so the segment reads as a chip against the band it ends on. /// internal static Rgb HeaderChip(Theme theme) { @@ -389,18 +250,15 @@ internal static Rgb HeaderChip(Theme theme) } /// - /// How far the header chip is lifted off the chrome band toward the theme's ink. Enough that the - /// wedge between the two is visible as a shape — the segment boundary is the only thing that says - /// where one part of the ribbon ends — and no further, because this is a background for a name and - /// not a highlight. + /// How far the header chip is lifted toward the theme's ink: enough that the wedge between the two + /// reads as a shape, no further, since this is a background for a name and not a highlight. /// private const double ChipLift = 0.22; /// - /// Every plane a pane's output can be painted on: the untinted surface and all six tints, each - /// focused and not. Fourteen colours, and what matters about them is that they form a band - /// — the whole set sits on one side of mid-scale, because every one of them is one theme background - /// put through a darkening and a brightening. + /// Every plane a pane's output can be painted on — the untinted surface and all six tints, each + /// focused and not. They form a band: every one is a theme background put through a darkening and a + /// brightening. /// private static IEnumerable PanePlanes(Theme theme) { @@ -413,20 +271,13 @@ private static IEnumerable PanePlanes(Theme theme) } /// - /// The one plane a foreground has to clear for it to be legible on every pane — the extreme - /// of in the direction a foreground on this theme is moved. - /// - /// It is the worst case rather than an approximation of one. A lift pushes a foreground away - /// from the band; once it is past the band the contrast ratio is monotone in the background's - /// luminance, so the plane hardest to clear is the one furthest in the direction of travel — the - /// brightest on a dark theme, the darkest on a light one. Clearing it clears all fourteen. - /// + /// The one plane a foreground must clear to be legible on every pane: the extreme of + /// in the direction a foreground on this theme travels. It is the true worst + /// case rather than an approximation — past the band the contrast ratio is monotone in the + /// background's luminance, so clearing the furthest clears them all. /// - /// It is per theme, and that is the whole reason it exists. A pane's actual plane depends on - /// its character's tint and on whether it holds focus, and resolving a colour against that - /// would mean re-formatting a whole buffer on every focus move — the expensive whole-buffer path - /// this codebase reserves for one deliberate keystroke. One plane per theme means a lifted colour is - /// decided once, when the line is formatted, and never revisited. + /// Per theme rather than per pane, because resolving against a pane's actual plane would mean + /// re-formatting a whole buffer on every focus move. /// /// internal static Rgb ReadingPlane(Theme theme) @@ -436,10 +287,8 @@ internal static Rgb ReadingPlane(Theme theme) } /// - /// The same worst case for the colours the client paints in its own voice, which land on - /// the — the status line, the rail — as well as on panes. It is - /// 's band with the backdrop added, so one ink is legible wherever the - /// chrome puts it: a status-line segment and a pane overlay must not need two different teals. + /// The same worst case for the colours the client paints in its own voice, which land on the + /// backdrop and on tab chips as well as on panes — so one ink is legible wherever the chrome puts it. /// internal static Rgb ChromePlane(Theme theme) { @@ -451,9 +300,8 @@ internal static Rgb ChromePlane(Theme theme) } /// - /// The member of hardest for a foreground to clear, given that the - /// direction of travel is decided by : brightest when the theme is dark, - /// darkest when it is light. + /// The member of hardest for a foreground to clear: brightest when + /// is dark, darkest when it is light. /// private static Rgb Extreme(IEnumerable planes, Rgb reference) => Contrast.RelativeLuminance(reference) < LightPlaneLuminance @@ -461,17 +309,15 @@ private static Rgb Extreme(IEnumerable planes, Rgb reference) => : planes.MinBy(Contrast.RelativeLuminance); /// - /// Mid-scale in relative luminance — 0.18, the sRGB middle grey. A theme whose planes sit below it - /// is dark and its text is lifted; above it and the text is darkened. Measured in luminance and not - /// in bytes because #808080 looks like half way when written down and is not: its relative - /// luminance is 0.216, and a byte pivot would call it dark and push text toward it. + /// Mid-scale in relative luminance. Measured in luminance and not in bytes: #808080 reads as + /// half way when written down but is 0.216, so a byte pivot would call it dark and push text + /// toward it. /// private const double LightPlaneLuminance = 0.18; /// /// The colours this client paints in its own voice, resolved against the theme and held to - /// on the plane they land on. See for what each - /// one is for, and what it measured before it was measured against anything. + /// on the plane they land on. /// internal static ChromeInk Chrome(Theme theme) { @@ -479,11 +325,9 @@ internal static ChromeInk Chrome(Theme theme) var plane = ChromePlane(theme); - // The marker's hue is the theme's own index 5, so a theme that overrides the base palette - // (Solarized does) contributes its violet rather than xterm's. What is *not* the theme's to - // decide is whether that colour can be read: on the default dark theme index 5 is #800080 - // against a #36363d pane, which is 1.27:1 — the reported "freeze is purple on a blue - // background", and very nearly the same colour twice. + // The marker's hue is the theme's own index 5, so a theme overriding the base palette contributes + // its violet. Whether that colour can be *read* is not the theme's to decide: on the default dark + // theme index 5 is #800080 against a #36363d pane, 1.27:1. return new ChromeInk( Contrast.Legible(ChromeInk.BaseAccent, plane).ToHex(), Contrast.Legible(ChromeInk.BaseNotice, plane).ToHex(), @@ -492,7 +336,7 @@ internal static ChromeInk Chrome(Theme theme) plane); } - /// Linear blend of two colours, of the way from to . + /// Linear blend, of the way from to . private static Rgb Mix(Rgb from, Rgb to, double t) => new( Channel(from.R + ((to.R - from.R) * t)), Channel(from.G + ((to.G - from.G) * t)), From d522c7bd793cf4eb3ff12cdb93ae9b5f5d244348 Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Wed, 12 Aug 2026 14:39:02 -0500 Subject: [PATCH 3/3] docs: trim comment narration on the settings-screen chrome Same pass as the palette cluster, over the settings screens: keeps the present-tense rule a maintainer needs (why a targeted button is not a cursor stop, why F5 states its own pane layout, why the password note names the file it names) and drops the account of which report each line answers and what the row said before it said this. ScreenChrome 417 -> 337 comment lines, WorldsScreenRenderer 599 -> 494, ScreenModel 279 -> 243. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN --- src/SharpMUTerm.Tui/ScreenChrome.cs | 312 ++++++--------- src/SharpMUTerm.Tui/ScreenModel.cs | 184 ++++----- src/SharpMUTerm.Tui/WorldsScreenRenderer.cs | 421 ++++++++------------ 3 files changed, 348 insertions(+), 569 deletions(-) diff --git a/src/SharpMUTerm.Tui/ScreenChrome.cs b/src/SharpMUTerm.Tui/ScreenChrome.cs index 71b4e59..aa4af07 100644 --- a/src/SharpMUTerm.Tui/ScreenChrome.cs +++ b/src/SharpMUTerm.Tui/ScreenChrome.cs @@ -20,9 +20,8 @@ internal static class ScreenChrome /// is the F-key that also toggles the screen (F6/Esc close). /// /// comes from the screen's , never from the - /// screen itself: a header may only claim ⏎ opens an editor when a row actually offers one. While - /// an edit *is* open the hints change wholesale — Esc no longer closes the screen, it abandons the - /// buffer, and saying otherwise would be the same lie in the other direction. + /// screen itself: a header may only claim ⏎ opens an editor when a row offers one. While an edit is + /// open the hints change wholesale — Esc abandons the buffer rather than closing the screen. /// /// internal static string Hints( @@ -64,15 +63,11 @@ internal static string Hints( } /// - /// The keyboard hints every screen with a list and a checkbox pane shares. Kept in one place so a - /// screen can't advertise a key its doesn't actually offer. - /// - /// It names ←→ as well as ⇥ because on a multi-pane screen both change pane, and ←→ was the one - /// movement these screens offered that nothing on them mentioned. The single-pane form below - /// deliberately does not: with one pane there is nowhere sideways to go, and a screen may not - /// advertise a key its model doesn't answer. pins the pair against - /// every screen's real pane count. - /// + /// The keyboard hints every screen with a list and a checkbox pane shares, kept in one place so a + /// screen cannot advertise a key its does not offer. It names ←→ as well as + /// ⇥ because on a multi-pane screen both change pane; the single-pane form below does not, since + /// there is nowhere sideways to go. pins the pair against every + /// screen's real pane count. /// internal const string ListHints = "↑↓ select · ←→ ⇥ pane · Space toggle"; @@ -85,20 +80,16 @@ internal static string Hints( internal const string EditHint = " · ⏎ edit"; /// - /// What a screen adds to its hints when — and only when — a pane offers a way to remove a row. Delete - /// is now the only way to run a removal: reaching the drawn row with ↑↓ meant walking the - /// cursor past the whole list, which dragged the selection to its last item, so that row is no longer a - /// cursor stop at all (). Delete acts on the row the cursor is already - /// on, which is the row the eye is on, and this is where the screen says so. + /// What a screen adds to its hints when a pane offers a way to remove a row. Delete is the only way + /// to run a removal — the drawn row is not a cursor stop () — so it + /// acts on the row the cursor is already on, and this is where the screen says so. /// internal const string DeleteHint = " · Del remove"; /// - /// What a screen adds to its hints when — and only when — a pane offers a read-only report on the - /// selected row. It matters more than that this is derived rather than - /// written: i is an ordinary letter, so a screen that answered it without saying so would be - /// a hidden feature, and one that said so without answering it would look broken on the one pane - /// where the key does nothing. + /// What a screen adds when a pane offers a read-only report on the selected row. Derived rather than + /// written, because i is an ordinary letter: a screen answering it silently is a hidden + /// feature, and one claiming it without answering looks broken. /// internal const string DetailHint = " · i info"; @@ -115,24 +106,17 @@ internal static string Hints( internal const string NextFieldHint = " · ⇥ next field"; /// - /// Added to only while the open field's dropdown actually has entries in - /// it — because ↑↓ walk exactly those entries and nothing else. It says pick from list rather - /// than the older choose because the list is now on screen: the keys move through what the - /// user can see, and put the entry they land on into the field. - /// - /// It is derived from , not from whether the field has - /// choices at all, so it disappears the moment a typed value narrows the list to nothing — the - /// point at which ↑↓ genuinely stop doing anything (see ). A hint - /// that stayed up over an empty list would be the same lie the ⏎ edit rule already forbids. - /// + /// Added to only while the open field's dropdown has entries in it, since + /// ↑↓ walk exactly those entries. Derived from rather + /// than from whether the field has choices at all, so it disappears the moment a typed value narrows + /// the list to nothing — the point at which the keys genuinely stop doing anything. /// internal const string ChoiceHint = " · ↑↓ pick from list"; /// - /// What the footer's Esc chip does while the screen is navigating. It said Cancel until the - /// key stopped cancelling: closing keeps every committed edit, so the footer now agrees with the - /// header's Esc close instead of contradicting it in the one place a user looks for the - /// consequences of a key. pins the two against each other. + /// What the footer's Esc chip does while the screen is navigating. Closing keeps every committed + /// edit, so it agrees with the header's Esc close; pins the + /// two against each other. /// internal const string CloseAction = "[[Esc]] Close"; @@ -163,14 +147,12 @@ internal static string Hints( internal const string BindAction = "[[any key]] Bind"; /// - /// The right-hand actions of a footer bar. lets a screen with a - /// context colour (F5's per-world accent) tint the ⏎ chip; it defaults to the app accent. + /// The right-hand actions of a footer bar. lets a screen with a context + /// colour (F5's per-world accent) tint the ⏎ chip; it defaults to the app accent. /// - /// is read for the same reason reads it: while a - /// field edit is open, ⏎ commits that field and Esc abandons its buffer — neither closes the - /// screen — so an action bar still offering Save and Cancel names two keys that do - /// something else at that moment. The footer is the more visible of the two claims, so it has to - /// change with the header rather than being left behind. + /// is read for 's reason: while a field edit is open ⏎ + /// commits that field and Esc abandons its buffer, so a bar still offering Save and + /// Cancel would name two keys that do something else. /// /// internal static string Actions(string? accent = null, ScreenFocus? focus = null) @@ -206,10 +188,9 @@ internal static string Cursor(string row, bool focused, int width) => focused ? $"[on {ScreenPalette.CursorBg}]{MarkupText.PadVisible(row, width)}[/]" : row; /// - /// The cursor band paints, which is what scrolls to. It is - /// found the same way finds the block caret, and for the same reason: exactly - /// one row of one pane carries it, so a column can locate its own focused row without every renderer - /// having to hand back the line number it drew it on. + /// The cursor band paints, which is what scrolls to. Found + /// the way finds the block caret: exactly one row of one pane carries it, so a + /// column can locate its own focused row without every renderer handing back a line number. /// private static readonly string CursorMark = $"[on {ScreenPalette.CursorBg}]"; @@ -217,18 +198,10 @@ internal static string Cursor(string row, bool focused, int width) => internal const int ColumnDivider = 2; /// - /// How wide a two-column screen's list column actually runs, given the width the screen was handed. - /// The split used to be a constant on every one of them, which was right at the width they were - /// designed at and wrong at every other: at 100 columns the list kept its full share while the - /// column beside it — the one carrying the editor, or the binding rows — lost its tail off the - /// right-hand edge. - /// - /// The rule is " unless that would starve the other column": the list gets - /// what it wants when there is room, gives cells back when there isn't, and never drops below - /// , because past that point both columns are unreadable rather than one. - /// A caller with no width to spend (the merged Render the unit tests go through) gets the - /// desired width unchanged, so the width-agnostic form is exactly what it always was. - /// + /// How wide a two-column screen's list column runs, given the width the screen was handed: + /// unless that would starve the column beside it, never below + /// , past which both columns are unreadable rather than one. A caller with + /// no width to spend gets the desired width unchanged. /// /// The whole screen's width, or 0 when the caller has none. /// What the list column takes when the screen can afford it. @@ -238,15 +211,10 @@ internal static int SplitWidth(int width, int desired, int minimum, int companio width <= 0 ? desired : Math.Clamp(width - ColumnDivider - companion, minimum, desired); /// - /// Drops a block's blank separator rows until it fits in rows, and hands - /// it back. The separators are the first thing a short pane can spare: they carry no content at all, - /// and every row they cost at the top is a row the pane loses off the bottom — where the checkboxes, - /// the buttons and the rest of the cursor's stops live. - /// - /// They go from the top down, so the section that compacts is the one already on screen rather than - /// the one about to fall off it. A block that already fits, or a caller with no height to fit it - /// into, comes back untouched — which is what keeps the wide case looking exactly as it did. - /// + /// Drops a block's blank separator rows until it fits in rows. They are + /// the first thing a short pane can spare — they carry no content, and every row they cost at the top + /// is one the pane loses off the bottom, where the cursor's stops live. They go from the top down, so + /// the section that compacts is the one already on screen. /// internal static List Compact(List block, int height) { @@ -275,19 +243,12 @@ internal static List Compact(List block, int height) /// /// Slices a block down to rows around the row carrying the cursor band, so - /// a pane taller than the screen still shows the row the keyboard is on. Without it a cursor can be - /// moved onto a row that was never drawn — F5 at 100×24 put its whole CHARACTERS list, and the - /// add/duplicate/remove buttons under it, below the fold while ↑↓ walked happily through them. - /// - /// The window is centred on the focused row rather than scrolled minimally into view, because these - /// blocks are rebuilt from scratch on every keystroke and there is no previous offset to scroll from - /// — a stateless rule has to be a function of the cursor alone. A block with no cursor in it (the - /// keyboard is in another pane) shows its top, which is where its own heading is. - /// + /// a pane taller than the screen still shows the row the keyboard is on — otherwise the cursor walks + /// through rows that were never drawn. /// - /// The edges say what they are hiding. A row silently missing from a pane is the same failure as a - /// cursor stop that was never drawn, one level up: the screen would be showing part of a list and - /// claiming it was the list. + /// Centred on the focused row rather than scrolled minimally into view: these blocks are rebuilt from + /// scratch on every keystroke, so a stateless rule has to be a function of the cursor alone. A block + /// with no cursor in it shows its top, where its heading is, and the edges say what they are hiding. /// /// internal static List Window(List block, int height) @@ -324,29 +285,20 @@ private static string More(string arrow, int count) => $" [{ScreenPalette.Muted}]{arrow} {count.ToString(CultureInfo.InvariantCulture)} more[/]"; /// - /// Draws a row's editable value: its committed text in a field well when nothing is being typed, - /// or — when is the open edit for that field — the buffer in that same - /// well, a block caret sitting inside it, and the reason the last commit was refused. Every screen - /// draws fields, so the affordance lives here rather than being re-invented (and drifting) per - /// renderer. - /// - /// The resting well is the whole point of 's existence: until it was drawn, - /// host aetherfall.mux and security TLS on · certs strict were the same row to look - /// at, and the only way to find out which one the keyboard could change was to walk the cursor into - /// it. A screen may not advertise a key its model doesn't offer; a row may not advertise an editor - /// it hasn't got, which is the same rule one level down. - /// + /// Draws a row's editable value: its committed text in a field well, or — when + /// is the open edit for that field — the buffer in that same well with a + /// block caret in it and the reason the last commit was refused. /// - /// is already markup, because a screen decides for itself how a - /// committed value reads (a null log directory shows as (default)); the buffer is escaped - /// here, since what has been typed is raw text. + /// The resting well is why exists: a well means "the keyboard can change this + /// here" and its absence means it cannot, so a row may not advertise an editor it has not got. /// /// - /// A buffer is replaced by before any of that - /// happens, so a secret has no route into markup at all — not even the character under the caret. - /// The mask is per-character rather than fixed-width while typing, because the caret has to - /// land where the keys say it does; a resting secret is drawn at a fixed width by its own renderer - /// (see ) so a screen nobody is editing doesn't publish its length. + /// is already markup, since a screen decides how a committed value reads; + /// the buffer is escaped here, being raw text. A buffer is + /// replaced by first, so a secret has no route into markup at all — not even the + /// character under the caret. The mask is per-character while typing so the caret lands where + /// the keys say; a resting secret is fixed-width () so an unedited screen + /// does not publish its length. /// /// internal static string Field(string display, ScreenFieldEdit? edit) @@ -388,9 +340,8 @@ internal static string Field(string display, ScreenFieldEdit? edit) /// /// An armed key capture: the value is replaced outright by the prompt, in the accent block the caret - /// is drawn in, because there is no buffer to show a caret inside — the next keystroke is - /// the value. A refused key keeps the capture armed and says why beside it, exactly as a refused - /// buffer does, so "that key cannot be bound" and "press another" are one state and not two. + /// is drawn in, because there is no buffer to put a caret inside — the next keystroke is the + /// value. A refused key keeps the capture armed and says why beside it. /// private static string Capture(ScreenFieldEdit open) { @@ -401,18 +352,16 @@ private static string Capture(ScreenFieldEdit open) } /// - /// The block caret paints, which is what hangs the - /// dropdown off. Exactly one field of one row can be open at a time, and only the column that draws - /// that field paints this — so finding it is how a column knows the open edit is *its* edit, without - /// every renderer having to hand back the line number it drew the value on. + /// The block caret paints, which is what hangs the dropdown + /// off. One field of one row can be open at a time and only the column drawing it paints this, so + /// finding it is how a column knows the open edit is its own. /// private static readonly string CaretMark = $"[{ScreenPalette.Ink} on {ScreenPalette.Accent}]"; /// - /// The most candidates a dropdown lists at once. Seventeen colour names is more rows than F2's - /// editor pane has to spare beside the pattern, the highlight rows and the three action templates, - /// so the list is capped and the caption says what it is capped to (6 of 17) — a list that - /// silently showed a third of itself would be worse than no list. + /// The most candidates a dropdown lists at once. Seventeen colour names is more rows than F2's editor + /// pane can spare, so the list is capped and the caption says what it is capped to (6 of 17) — + /// a list silently showing a third of itself would be worse than no list. /// internal const int MaxChoiceRows = 6; @@ -423,18 +372,16 @@ private static string Capture(ScreenFieldEdit open) internal const string ClosedChoicesCaption = "these values only"; /// - /// What an open field's dropdown says when the buffer matches none of its entries. It names the - /// state as *legal*, because on these fields it is: the spawn windows are defined by what routes to - /// them, so a name matching nothing is how the next one is created. An empty list with nothing - /// written beside it would read as a refusal. + /// What an open field's dropdown says when the buffer matches none of its entries. It names the state + /// as legal, because on these fields it is: spawn windows are defined by what routes to them, so a + /// name matching nothing is how the next one is created. /// internal const string NoMatchOpen = "nothing matches — a new value is allowed"; /// - /// What a closed field's dropdown says instead. It states the fact and stops there: the value is - /// refused at ⏎ by the field's own validator, which reports it against the row in - /// , and a second warning drawn before the user has finished typing - /// would spend that colour on a value they may still be halfway through. + /// What a closed field's dropdown says instead. It states the fact and stops: the value is refused at + /// ⏎ by the field's validator, and a second warning before the user has finished typing would spend + /// that colour on a value they may be halfway through. /// internal const string NoMatchClosed = "nothing matches"; @@ -442,24 +389,19 @@ private static string Capture(ScreenFieldEdit open) private const string ChoiceIndent = " "; /// - /// Draws an open field's candidate list into , and hands the column back. - /// Every screen calls this once on each block that draws fields; a block that isn't drawing the open - /// edit has no caret in it and comes back untouched, so the wiring is one line per column and cannot - /// be pointed at the wrong field. + /// Draws an open field's candidate list into and hands the column back. A + /// block not drawing the open edit has no caret in it and comes back untouched, so the wiring is one + /// line per column and cannot be pointed at the wrong field. /// - /// The list is an overlay: it replaces the rows next to the field instead of pushing them - /// down. Pushing was the obvious shape and is the wrong one here. F5's character form is a grid row - /// sized to its own line count, so a list that grew it would resize the whole screen the instant ⏎ - /// was pressed; F2's editor pane already runs to two dozen rows, so on a short terminal the rows - /// pushed off the bottom would include the three checkboxes the cursor can still reach. An overlay - /// changes no geometry at all — the rows it covers are visible again the moment the field closes, - /// and none of them can be scrolled out of existence in the meantime. + /// The list is an overlay, replacing the rows beside the field rather than pushing them down. + /// F5's character form is a grid row sized to its own line count, so a list that grew it would resize + /// the screen on ⏎; F2's editor pane is long enough that pushing would shove reachable checkboxes off + /// a short terminal. An overlay changes no geometry. /// /// - /// It opens downward, and upward when there aren't enough rows below it — F5's log format is the - /// second-to-last line of its form, and a list that ran off the end of the block would simply not be - /// drawn. The caption keeps its edge against the field either way (▾ below, ▴ above), - /// so the block reads as attached to the well rather than as content that happens to be near it. + /// It opens downward, and upward when there are not enough rows below — F5's log format sits second + /// from the end of its form. The caption keeps its edge against the field either way (▾ below, + /// ▴ above), so the block reads as attached to the well. /// /// /// The block's lines, as the renderer has just built them. @@ -523,10 +465,9 @@ internal static List Choices(List column, ScreenFieldEdit? edit, } /// - /// The dropdown's caption and its drawn entries. The entries are the choices the buffer narrows to - /// () — the very list ↑↓ walk — windowed to - /// around the one the buffer names, so the marked entry is always on - /// screen however far down a seventeen-colour palette it sits. + /// The dropdown's caption and its drawn entries: the choices the buffer narrows to + /// (), windowed to around the one the + /// buffer names, so the marked entry is always on screen. /// private static (string Caption, List<(string Content, string Background)> Entries) ChoiceContent( ScreenFieldEdit open) @@ -561,10 +502,9 @@ private static (string Caption, List<(string Content, string Background)> Entrie } /// - /// One row of the floating block: its own markup, inset from the column's edge, padded to the - /// block's shared inner width on a raised background. The block hugs its content rather than - /// spanning the pane, because a full-width band is what the pane's own rows look like and the one - /// thing this block must not be mistaken for is a row. + /// One row of the floating block: its markup, inset from the column's edge, padded to the block's + /// inner width on a raised background. It hugs its content rather than spanning the pane, because a + /// full-width band is what the pane's own rows look like and this must not be mistaken for one. /// private static string MenuLine(string content, string bg, int inner) => $"{ChoiceIndent}[on {bg}] {MarkupText.PadVisible(content, Math.Max(0, inner))} [/]"; @@ -578,14 +518,11 @@ private static string Shadow(int inner) => $"{ChoiceIndent} [on {ScreenPalette.MenuShadow}]{new string(' ', Math.Max(0, inner + 1))}[/]"; /// - /// Draws a value the keyboard cannot change where it is drawn — a world's TLS/certificate line, a - /// character's password or session state, a numpad cell mirroring a binding elsewhere. It gets the - /// muted ink and, decisively, *no* field well, which is what tells it apart from an editable value - /// at rest and without focus. The pair of them is one rule with one implementation: a well means - /// "you can change this here", its absence means "you cannot". + /// Draws a value the keyboard cannot change where it is drawn. It gets the muted ink and, decisively, + /// no field well: a well means "you can change this here", its absence means "you cannot". /// - /// The rule is scoped to rows that read label value, which is where the ambiguity lives. A - /// checkbox and a radio group already carry an affordance of their own and are left alone. + /// Scoped to rows reading label value, where the ambiguity lives. A checkbox and a radio + /// group carry an affordance of their own and are left alone. /// /// internal static string ReadOnly(string text) => $"[{ScreenPalette.Muted}]{MarkupText.Escape(text)}[/]"; @@ -605,9 +542,7 @@ private static string Shadow(int inner) => /// /// A set secret at rest: glyphs in the ordinary value ink, so the row - /// reads as holding something. It is drawn in a well like any other editable value, because it - /// is one — the well is this project's one promise that the keyboard can change a value - /// here, and a password that can be typed but is drawn without one would be the same lie in reverse. + /// reads as holding something. Drawn in a well like any other editable value, because it is one. /// internal static string RestingMask() => $"[{ScreenPalette.Value}]{Mask(RestingMaskWidth)}[/]"; @@ -616,16 +551,13 @@ private static string Shadow(int inner) => /// /// Spells out the glyphs a list's rows are written in, at the foot of the column that draws them. - /// The list screens compress a rule down to single cells — a tick, a set marker, and on F2 a strip - /// of action letters — and a compressed value is the one thing that cannot say what its own words - /// mean. Nothing on these screens said, anywhere: H could as easily have been "hidden" as - /// "highlight". + /// These screens compress a rule into single cells, and a compressed value cannot say what its own + /// marks mean — H could as easily be "hidden" as "highlight". /// - /// It goes at the foot of the list, not beside the header: the header names the row's columns - /// (on name / pattern → window) and these are the marks inside them, and the slack - /// in a list column is at the bottom — which is exactly the dead space a key is worth spending. - /// Entries wrap to rather than to a fixed count, because the column is a - /// function of the screen's width now (see ). + /// At the foot rather than beside the header: the header names the row's columns and these are the + /// marks inside them, and the slack in a list column is at the bottom. Entries wrap to + /// , since the column is a function of the screen's width + /// (). /// /// /// What the block is called, drawn on its first row only. @@ -667,16 +599,12 @@ internal static string LegendEntry(string glyph, string meaning, bool lit) => li : $"[{ScreenPalette.Label}]{glyph} {MarkupText.Escape(meaning)}[/]"; /// - /// The one row an empty trigger set gets in a flattened pane. F2, F3, F4 and F6 each draw - /// one column of every set's rules, so a set holding none of that kind is drawn nowhere at all — and - /// once sets can be created, the very first thing you would look for after making one is the thing - /// the screen cannot show you. This says it is there and has nothing in it. + /// The one row an empty trigger set gets in a flattened pane. F2, F3, F4 and F6 each draw one + /// column of every set's rules, so a set holding none of that kind would be drawn nowhere at all. /// - /// It is a readout and not a row: the cursor cannot reach it, because it stands for a set rather - /// than for an item, and giving it a cursor stop would put a row in the pane that [[- del]], - /// Space and ⏎ would all have to make an exception for. Moving an item into the set — the - /// set field on any row () — is what replaces it with real - /// rows. + /// A readout and not a row: the cursor cannot reach it, because it stands for a set rather than an + /// item, and a cursor stop here would be a row that [[- del]], Space and ⏎ all had to except. + /// Moving an item into the set is what replaces it with real rows. /// /// /// The set with nothing in it. @@ -685,19 +613,15 @@ internal static string EmptySet(string set, string noun) => $" [{ScreenPalette.Muted}]▪ {MarkupText.Escape(set)} — no {MarkupText.Escape(noun)}[/]"; /// - /// Draws a pane's button rows, appended after its list. The rows come *from* the pane's own - /// s rather than being written out again per screen, so the label the - /// cursor lands on and the command ⏎ runs cannot drift apart — and every screen paints them the - /// same, which is the whole reason this lives here rather than in five renderers. + /// Draws a pane's button rows, appended after its list. They come from the pane's own + /// s rather than being written out per screen, so the label the cursor lands + /// on and the command ⏎ runs cannot drift apart. /// - /// The two kinds are drawn differently because they are no longer the same kind of thing. A button - /// that builds is a chip in the accent — a place the cursor goes and ⏎ presses, which is what - /// brackets mean everywhere else on these screens. A removal is not a cursor stop at all any - /// more ( explains why), so drawing it as a chip would be an - /// affordance for something the keyboard cannot reach. It is drawn as what it now is: a reading of - /// what Delete would take, naming the key and its victim — Del removes Aetherfall. The row - /// still earns its place, because the target is the one thing a destructive key must not leave - /// off-screen. + /// The two kinds are drawn differently because they are different things. A button that builds + /// is a chip in the accent — somewhere the cursor goes and ⏎ presses. A removal is not a cursor + /// stop (), so a chip would be an affordance for something the keyboard + /// cannot reach; it is drawn as a reading of what Delete would take — Del removes Aetherfall — + /// because the target is the one thing a destructive key must not leave off-screen. /// /// /// The pane's button rows, in the order the model appends them. @@ -756,19 +680,17 @@ private static string KeyHintRow(ScreenButton button, string verb) => /// /// Where the cursor is within one of a screen's lists — trigger 1/4, world 2/2. Every - /// footer's context line opens with one of these, so the eight screens answer the same question in - /// the same words instead of each reporting whatever its author found interesting (F9 used to count - /// its own section headers). + /// footer's context line opens with one, so the eight screens answer the same question in the same + /// words. /// internal static string Position(string noun, int index, int count) => $"{noun} {(index + 1).ToString(CultureInfo.InvariantCulture)}" + $"/{count.ToString(CultureInfo.InvariantCulture)}"; /// - /// A footer's context line: a , then whatever identifies the thing it points - /// at (the set a trigger belongs to, the section an option sits under, the name a binding carries). - /// Null and empty parts are dropped, so a screen with nothing selected renders an empty context - /// rather than a stranded separator. + /// A footer's context line: a , then whatever identifies the thing it points at. + /// Null and empty parts are dropped, so a screen with nothing selected renders an empty context rather + /// than a stranded separator. /// internal static string Context(params string?[] parts) { @@ -793,14 +715,12 @@ internal static string Context(params string?[] parts) internal static int Rows(int height) => height <= 0 ? 0 : Math.Max(1, height - 2); /// - /// The frame every two-column settings screen shares: a header band on the first row, an action bar - /// on the last, and between them a body of two columns divided by a hairline. + /// The frame every two-column settings screen shares: a header band on the first row, an action bar on + /// the last, and between them two columns divided by a hairline. /// - /// The body is sized to its content rather than stretched to fill, which is the single - /// change that stops F3, F6 and F4 drawing a thirty-row empty pane under four rows of rules. The - /// hairline stops where the columns stop, exactly as F7/F8's options card ends where its options do, - /// and the slack below it belongs to the backdrop instead of pretending to be part of a list. A - /// caller with no height falls back to the old fill, since there is nothing to size against. + /// The body is sized to its content rather than stretched, so a screen with four rules does + /// not draw a thirty-row empty pane under them. The hairline stops where the columns stop and the + /// slack below belongs to the backdrop. A caller with no height falls back to filling. /// /// /// The header band. diff --git a/src/SharpMUTerm.Tui/ScreenModel.cs b/src/SharpMUTerm.Tui/ScreenModel.cs index 163dc5a..e7c3451 100644 --- a/src/SharpMUTerm.Tui/ScreenModel.cs +++ b/src/SharpMUTerm.Tui/ScreenModel.cs @@ -2,13 +2,8 @@ namespace SharpMUTerm.Tui; /// /// A checkbox row on a settings screen, bound to the config it shows: how to read the flag, and how to -/// flip it. -/// -/// There is nothing here about putting the old value back, and deliberately so. A flipped checkbox is a -/// committed edit the moment Space presses it — see for the scope rule -/// — so nothing ever asks a toggle to undo itself, and a snapshot nothing replayed would be a mechanism -/// with no driver. -/// +/// flip it. Nothing here restores the old value — a flipped checkbox is a committed edit the +/// moment Space presses it (see ), so nothing ever asks a toggle to undo itself. /// /// Reads the flag as the renderer draws it. /// Inverts the flag. @@ -30,49 +25,41 @@ internal static ScreenToggle Bind(Func get, Action set) /// /// /// Puts the list back exactly as it was, position included — or null when the press destroyed nothing. -/// A button that built a row returns null: an addition loses no work by being kept, so it is -/// never reviewed and never replayed, and an undo action nothing could run would be dead weight -/// carried by every press. Only the destructive buttons hand one back, which is what makes -/// ' log a log of deletions rather than of everything. +/// A button that built a row returns null, which is what makes ' log a +/// log of deletions rather than of everything. /// /// -/// The row of the button's own pane the cursor should move to — the row just added, so a new world -/// opens ready to be named. Null leaves the cursor where it was. +/// The row of the button's own pane the cursor should move to — the row just added, so a new world opens +/// ready to be named. Null leaves the cursor where it was. /// internal readonly record struct ScreenPress(Action? Undo, int? Select = null); /// -/// A command on a settings screen. The building ones are rows the cursor lands on and ⏎ presses — -/// [+ world], [⧉ duplicate] — since ⏎ is already "activate the focused row". A -/// removal is run by Delete on the row it would take, and its own drawn row is not a cursor stop -/// (see ). +/// A command on a settings screen. The building ones are rows the cursor lands on and ⏎ presses, since ⏎ +/// is already "activate the focused row". A removal is run by Delete on the row it would take, +/// and its own drawn row is not a cursor stop (see ). /// -/// performs the change and *returns* how to undo it, rather than being handed a -/// snapshot taken beforehand. That is forced by what a removal has to capture: the item and its -/// index, so the closing review's "put them back" restores a deleted world where it was in the list -/// rather than on the end — the list's order is what the screen navigates by, and silently reordering it -/// would be a second, invisible edit. A building press returns no undo at all: an addition destroys -/// nothing, so nothing reviews or replays it. +/// performs the change and returns how to undo it rather than being handed a +/// snapshot beforehand, because a removal has to capture the item and its index — restoring a +/// deleted world onto the end of the list would be a second, invisible edit to the order the screen +/// navigates by. /// /// /// What the button is called, for the row the renderer draws. /// Performs the change and returns the undo plus where to leave the cursor. /// -/// Whether the button builds or destroys, which is what decides how the row is drawn and whether -/// Delete runs it. It is carried here rather than inferred from , because a -/// renderer comparing label strings to decide how to paint a row is one rename away from painting a -/// deletion in the "add" accent. +/// Whether the button builds or destroys, which decides how the row is drawn and whether Delete runs it. +/// Carried rather than inferred from , because a renderer comparing label strings +/// is one rename away from painting a deletion in the "add" accent. /// /// -/// The row this button would act on, named on the button's own row so the screen says what is about to -/// happen. Null for a button that acts on nothing in particular. +/// The row this button would act on, named on the button's own row. Null for a button that acts on +/// nothing in particular. /// /// -/// What this press would destroy, in words, asked before — which is the -/// only moment the answer can still be counted (a world's characters are gone by the time the press -/// returns). It is what the closing review names, so the question reads Aetherfall and its 2 -/// characters rather than 1 deletion; see . Null on a button -/// that destroys nothing. +/// What this press would destroy, in words, asked before — the only +/// moment the answer can still be counted, since a world's characters are gone by the time the press +/// returns. It is what the closing review names. Null on a button that destroys nothing. /// internal readonly record struct ScreenButton( string Label, @@ -90,14 +77,11 @@ internal readonly record struct ScreenButton( internal const string RemoveKeyLabel = "Del"; /// - /// Appends a new item and leaves the cursor on it — a new row is worth nothing if the next - /// keystroke has to go and find it. + /// Appends a new item and leaves the cursor on it. /// - /// is how many of the pane's list rows precede . - /// It is zero when the pane is the list (F5's worlds), and non-zero when the pane flattens - /// several lists into one (F2/F3/F4/F6 show every set's rules in one column, but a rule is added to - /// one particular set), because the cursor is asked for a row of the pane, not an index into the - /// list that happens to hold the new item. + /// is how many of the pane's list rows precede : + /// zero when the pane is the list, non-zero when the pane flattens several lists into one, + /// because the cursor is asked for a row of the pane and not an index into the list holding the item. /// /// internal static ScreenButton Add( @@ -126,12 +110,11 @@ internal static ScreenButton Add( internal const string DetailKeyLabel = "i"; /// - /// Opens a read-only report about the selected row. It is a so that the - /// row the screen draws and the key that runs it come from one place, exactly as a removal does — - /// but it changes nothing, so it returns no undo and moves no cursor, and - /// runs it outside . Navigation is - /// not an edit: routing it through the edit log would persist the configuration and re-periodise - /// every running timer every time somebody looked at a world. + /// Opens a read-only report about the selected row. A so the drawn row and + /// the key that runs it come from one place — but it changes nothing, so it returns no undo, moves no + /// cursor, and runs it outside : + /// routing navigation through the edit log would persist the configuration and re-periodise every + /// running timer whenever somebody looked at a world. /// /// The row this would report on, named on the drawn key-hint row. /// Puts the report on the screen. @@ -151,20 +134,18 @@ internal static ScreenButton Detail(string target, Action open) } /// - /// Removes the item at , restoring it *at that index* on undo. The cursor - /// stays on the same ordinal, which is now whatever followed the deleted row — the same place the - /// eye is. means what it does on . + /// Removes the item at , restoring it at that index on undo. The + /// cursor stays on the same ordinal, which is now whatever followed the deleted row. + /// means what it does on . /// - /// It takes no label. A removal is drawn as the key that runs it - /// () rather than as a chip, because its row is no longer - /// somewhere the cursor can go — see for why, and - /// for what that row now reads as. + /// It takes no label: a removal is drawn as the key that runs it + /// () rather than as a chip, because its row is not somewhere + /// the cursor can go. /// /// /// - /// What this removal would destroy, in words, for the closing review to name. See - /// ; it defaults to , which is the honest - /// answer for every row that takes nothing else with it. + /// What this removal would destroy, for the closing review to name. Defaults to + /// , the honest answer for a row that takes nothing else with it. /// internal static ScreenButton Remove( IList list, int index, int offset = 0, string? target = null, Func? describe = null) @@ -218,16 +199,13 @@ internal enum ScreenButtonKind } /// -/// One row of a settings screen's navigable shape. A row is a plain stop (neither a checkbox nor -/// anything to type into), a checkbox, a row of editable fields, a button, or both a checkbox and -/// fields at once — the keypad's bindings are the last case, where Space enables the macro and ⏎ edits -/// the command it sends. +/// One row of a settings screen's navigable shape: a plain stop, a checkbox, a row of editable fields, a +/// button, or a checkbox and fields at once — the keypad's bindings are the last case, where Space +/// enables the macro and ⏎ edits the command it sends. /// -/// Fields are an ordered list rather than a single value because a row is a *record*, not a cell: one -/// world row carries its name, host, port, encoding, and keepalive. ⏎ opens the first, ⇥ steps to the -/// next, and the renderers draw the open one wherever that field's labelled value already appears. It -/// also keeps a row's identity stable — giving a row fields never renumbers the rows around it, so a -/// pane's cursor indices mean the same thing before and after this feature. +/// Fields are an ordered list rather than a single value because a row is a record: one world row +/// carries name, host, port, encoding and keepalive. ⏎ opens the first, ⇥ steps to the next. It also keeps +/// a row's identity stable — giving a row fields never renumbers the rows around it. /// /// /// The checkbox Space flips, or null when the row has none. @@ -288,39 +266,31 @@ internal ScreenModel(params IReadOnlyList[] panes) /// /// How many rows of each pane the cursor may occupy, in pane order — what - /// navigates by. It is not how many rows the pane draws: a - /// pane's destructive button is drawn and is not a stop. + /// navigates by. Not how many rows the pane draws: a pane's + /// destructive button is drawn and is not a stop. /// - /// That asymmetry is the point, and it is the fix for "only the last world can be deleted". A pane is - /// a list followed by its buttons, and reaching a button with ↑↓ means walking the cursor over every - /// row of the list on the way — which drags the selection to the last one - /// (), so a [[- del]] arrived at that way could only ever - /// delete the final item. It cannot be fixed by remembering where the cursor has been, because the - /// last list row visited on the way to the buttons is *always* the last row of the list; the button - /// block structurally cannot know which item was meant. + /// That asymmetry is the point. Reaching a button with ↑↓ means walking the cursor over every row of + /// the list on the way, which drags the selection to the last one + /// () — so a removal arrived at that way could only ever delete + /// the final item, and no amount of remembering where the cursor has been can fix it. /// /// - /// So the rule is: an action with no target needs a cursor stop; an action with a target must not - /// steal the cursor from the thing it acts on. [[+ world]] stays a stop, because "add" has - /// nothing to point at and needs somewhere to be pressed from. A removal has a target — the selected - /// row — and is run by Delete on that very row, so its drawn row stops being a place you go and - /// becomes a true reading of what Delete would take (). Nothing is - /// lost: advertises the key, and it is derived from - /// so it cannot advertise it where there is none. + /// The rule: an action with no target needs a cursor stop; an action with a target must not steal + /// the cursor from the thing it acts on. [[+ world]] stays a stop; a removal is run by + /// Delete on the selected row, and its drawn row becomes a reading of what Delete would take. + /// advertises the key and is derived from + /// , so it cannot advertise it where there is none. /// /// internal IReadOnlyList Sizes { get; } /// - /// How many of a pane's rows the cursor may occupy: all of them but the targeted buttons at - /// the end. Those are appended last on every screen (a pane reads list → add → duplicate → info → - /// remove), so this is a count and not a set of holes — the cursor never has to skip a row in the - /// middle, and stays a plain clamp. ScreenModelTests pins the - /// count against the drawn rows, and MsspScreenTests.TheInfoRowIsDrawnAndIsNotSomewhereTheCursorCanGo - /// pins it for the half. + /// How many of a pane's rows the cursor may occupy: all but the targeted buttons at the end. + /// Those are appended last on every screen (list → add → duplicate → info → remove), so this is a + /// count and not a set of holes and stays a plain clamp. /// - /// Both non-stop kinds must stay trailing. A row put - /// anywhere but among them gives the cursor a hole, and the clamp becomes a skip list. + /// Both non-stop kinds must stay trailing. A row anywhere + /// but among them gives the cursor a hole, and the clamp becomes a skip list. /// /// private static int Stops(IReadOnlyList rows) @@ -337,21 +307,17 @@ private static int Stops(IReadOnlyList rows) /// /// Where each pane is drawn, which is what the arrow keys and ⇥ navigate by. It defaults to - /// — one pane per column, in index order — because that is what every - /// screen but F5 actually is: a list beside an editor, or a single card. Under that default - /// reading order and index order are the same thing, so ⇥ walks exactly the panes it always did. + /// — one pane per column, in index order — because that is what every screen + /// but F5 is, and under that default reading order and index order are the same thing. /// - /// A screen states its own layout only when the two disagree. F5 is the one that does: its - /// security pane is appended (a pane index is a cursor coordinate, so inserting one would - /// renumber every stop the screen and its tests navigate by) while being drawn above the - /// characters list, in the same column. Left as index order, ⇥ went left-top → right-middle → - /// bottom-right and then jumped back up the screen, which is the awkwardness this coordinate - /// exists to fix. + /// A screen states its own layout only when the two disagree. F5 does: its security pane is + /// appended (a pane index is a cursor coordinate, so inserting one would renumber every stop + /// the screen and its tests navigate by) while being drawn above the characters list, in the + /// same column. /// /// - /// A layout that doesn't name every pane is ignored rather than half-applied: a missing - /// coordinate is a pane the arrows could never reach, and silently dropping one stop is worse than - /// falling back to a rule that reaches them all. + /// A layout not naming every pane is ignored rather than half-applied: a missing coordinate is a pane + /// the arrows could never reach. /// /// internal IReadOnlyList Layout @@ -379,15 +345,13 @@ internal static IReadOnlyList SideBySide(int panes) } /// - /// How many rows of each pane are *list* rows rather than the buttons appended after them. A pane - /// is a list followed by its own buttons, and the two mean different things to the cursor: moving - /// onto [[+ world]] must not change which world is selected, or the detail column beside it - /// would blank the moment the cursor reached for the add button. - /// anchors the selection with this. + /// How many rows of each pane are list rows rather than the buttons appended after them. + /// Moving onto [[+ world]] must not change which world is selected, or the detail column would + /// blank the moment the cursor reached for the add button; anchors the + /// selection with this. /// - /// The anchor is not what keeps a removal pointed at the right row — it cannot be, since - /// walking down to a button leaves it on the last list row. That is what answers, - /// by keeping the cursor out of the removal's row altogether. + /// The anchor is not what keeps a removal pointed at the right row — walking down to a button + /// leaves it on the last list row. That is what answers. /// /// internal IReadOnlyList ListSizes diff --git a/src/SharpMUTerm.Tui/WorldsScreenRenderer.cs b/src/SharpMUTerm.Tui/WorldsScreenRenderer.cs index df539df..3611ace 100644 --- a/src/SharpMUTerm.Tui/WorldsScreenRenderer.cs +++ b/src/SharpMUTerm.Tui/WorldsScreenRenderer.cs @@ -64,15 +64,10 @@ internal static class WorldsScreenRenderer /// /// The character row's field ordinals, in the order ⇥ steps through them — which is the order the - /// CHARACTER form draws them in, top to bottom. The name leads, as it does on every list screen. - /// - /// The password and the connect line are inserted after the name rather than appended past - /// the log fields, and the ordinals below them moved to make room. Appending would have left the - /// numbering untouched at the cost of ⇥ walking name → on connect → log → log folder and then back - /// up the form to the password — the same "three hops forward and one backwards" that - /// exists to have fixed one level up. Every renderer and every test - /// addresses these by name, so the shift is a rename and not a silent renumbering. - /// + /// CHARACTER form draws them, top to bottom. The name leads, as on every list screen. The password + /// and connect line are inserted after it rather than appended, so ⇥ does not walk down the + /// form and then back up to reach them; every renderer and test addresses these by name, so the shift + /// is a rename rather than a silent renumbering. /// internal const int CharacterNameField = 0; @@ -83,14 +78,13 @@ internal static class WorldsScreenRenderer internal const int OnConnectField = 3; /// - /// . Inserted here rather than appended for the - /// reason above, and here in particular because it belongs with the connection rows it is about: the - /// connect line, the on-connect lines and the login readout are all answers to "what happens when - /// this character connects", and this one answers "does it, unasked". + /// , placed with the connection rows it belongs to: + /// the connect line, the on-connect lines and the login readout all answer "what happens when this + /// character connects", and this one answers "does it, unasked". /// - /// It stays a field, and that is now load-bearing. A character row draws no checkbox — see - /// — so a value moved onto the row's toggle is a value with no affordance - /// at all. This one has a well, and ⏎ opens it, which is the only reachable way to set it. + /// It stays a field, and that is load-bearing. A character row draws no checkbox (see + /// ), so a value moved onto the row's toggle would have no affordance at + /// all. This one has a well, and ⏎ opens it. /// /// internal const int StartupField = 4; @@ -102,10 +96,8 @@ internal static class WorldsScreenRenderer /// /// — whether this character's panes come back holding their /// previous session's content. Appended after the two log rows rather than inserted among them, - /// because it is the third answer to "what does this client write down about me" and reads in that - /// order: what a transcript is, where it goes, and then whether the panes remember. It is a field - /// and not a toggle for the same reason is — a character row draws no - /// checkbox, so a value on the row's toggle would have no affordance at all. + /// because it is the third answer to "what does this client write down about me". A field and not a + /// toggle for 's reason. /// internal const int RestoreLogField = 7; @@ -133,17 +125,13 @@ internal static class WorldsScreenRenderer /// /// Which item of a list a pane's cursor has selected. The cursor also visits the pane's - /// [[+ …]] row, which sits past the end of the list, and a cursor parked there must not read - /// as "no world selected" — that would blank the detail column and empty the character pane while - /// the user was reaching for the add button. A cursor past the end therefore keeps the last item - /// selected. A negative cursor still means nothing is selected, which is how a caller says so - /// deliberately. + /// [[+ …]] row past the end of the list, and a cursor parked there must not read as "nothing + /// selected" — that would blank the detail column while the user reached for the add button — so a + /// cursor past the end keeps the last item. A negative cursor still means nothing is selected. /// - /// This clamp is a display rule and never decides what a removal acts on. The live screens - /// hand these blocks — the anchored selection, which never - /// runs past its list — and a removal is run by Delete on the selected row, whose own row is not a - /// cursor stop at all (see ). It was reading this clamp as the cause of - /// "only the last world can be deleted" that pointed one diagnosis at the wrong mechanism. + /// A display rule only: it never decides what a removal acts on. The live screens hand these + /// blocks , which never runs past its list, and a removal is + /// run by Delete on the selected row. /// /// private static int Selected(int count, int cursor) => cursor >= count ? count - 1 : cursor; @@ -231,14 +219,13 @@ internal static bool HasCharacter(IReadOnlyList worlds, int sel /// /// The screen title on the left, the keyboard hints right-aligned to . The - /// hints are derived from and rather than - /// written here, so the header cannot advertise an edit the screen doesn't offer. + /// hints are derived from and rather than written + /// here, so the header cannot advertise an edit the screen does not offer. /// /// - /// The key that opened the screen, which is the key the header offers to close it with. It is a - /// parameter rather than a constant because this screen has two doors: F5, and F9 straight onto the - /// selected character's log. A header that always said F5 would name a key that, pressed from - /// an F9-opened screen, re-opens it somewhere else instead of closing it. + /// The key that opened the screen, which is the key the header offers to close it with. A parameter + /// because this screen has two doors — F5, and F9 straight onto the selected character's log — and a + /// header always saying F5 would name a key that re-opens rather than closes. /// internal static string HeaderLine( int width, ScreenModel? model = null, ScreenFocus? focus = null, string fkey = FKey) @@ -281,14 +268,12 @@ internal static string HeaderLine( /// /// The row for . Two cells short, because the - /// CHARACTER panel is 48 wide and CharField has already spent fourteen of them on the indent - /// and the label column — a row that wraps costs the form a line it was never measured for, which is - /// how log folder once fell out of the band (see ). + /// CHARACTER panel is 48 wide and CharField has spent fourteen on the indent and label column; + /// a row that wraps costs the form a line it was never measured for. /// - /// It says when, and the login row directly under it says what is typed; - /// they are adjacent so the difference is read rather than guessed, and neither label contains the - /// other's word. at start in particular avoids "auto-": the one thing this must not be called - /// is a second kind of login setting. + /// It says when and the login row under it says what is typed, adjacent so + /// the difference is read rather than guessed. at start avoids "auto-": the one thing this must + /// not be called is a second kind of login setting. /// /// internal const string StartupLabel = "at start"; @@ -356,55 +341,43 @@ internal static string TintDetail(PaneTint tint) => tint == PaneTint.None ? $"[{Label}]{PaneTint.None}[/]" : $"[{Value}]{tint}[/]"; /// - /// The screen's four navigable panes, in ⇥ order: the WORLDS list (no checkbox on a world's row, - /// but ⏎ opens the world's own fields — the ones the detail column lists), the selected world's - /// characters (no checkbox on a character's row either — ⏎ edits its name, password, connect line, - /// on-connect line, at start and log), the - /// trigger sets (Space assigns the selected character to one, ⏎ renames it, and the pane's - /// own buttons make and unmake them), and the selected world's two security checkboxes. All but the - /// first collapse to empty when there is nothing selected above them, and ⇥ skips empty panes, so - /// the cursor never lands somewhere with no rows. + /// The screen's four navigable panes, in ⇥ order: the WORLDS list (⏎ opens the world's own fields), + /// the selected world's characters (⏎ edits name, password, connect line, on-connect, at start + /// and log), the trigger sets (Space assigns the selected character to one, ⏎ renames it, and + /// the pane's buttons make and unmake them), and the world's two security checkboxes. All but the + /// first collapse to empty when nothing is selected above them, and ⇥ skips empty panes. /// - /// The trigger-set pane is where sets are managed, not merely assigned — it is the only - /// view in the app of sets as objects, because F2, F3, F4 and F6 each flatten one kind of a - /// set's contents across all of them and so cannot show a set that holds none of that kind. It still - /// needs a selected character, because the other half of every row is that character's opt-in; on a - /// configuration with no characters yet there is also nothing a set could apply to. + /// The trigger-set pane is where sets are managed, not merely assigned — it is the only view + /// of sets as objects, because F2, F3, F4 and F6 each flatten one kind of a set's contents + /// and so cannot show a set holding none of that kind. It still needs a selected character, since the + /// other half of every row is that character's opt-in. /// /// - /// A world's *typed* values hang off its list row rather than becoming a pane of their own: the - /// detail column is a projection of whatever the WORLDS list has selected, so those values already - /// belong to that row. Its two booleans cannot — a row carries at most one checkbox, and there are - /// two of them — so they are the one thing on this screen that needs a pane, and it is appended - /// (see ) rather than slotted in beside the world it describes. + /// A world's typed values hang off its list row rather than forming a pane, because the detail column + /// is a projection of whatever WORLDS has selected. Its two booleans cannot — a row carries at most + /// one checkbox — so they are appended as . /// /// - /// A character's logging is two fields of the character's own row, drawn in the character form. It - /// lives here because this is where the character it applies to is: F9 used to edit it on a - /// screen of its own that resolved "the active character, or else the first one configured" and - /// never said which — so the same screen edited a different character's log depending on what was - /// connected. There is no Space-to-start checkbox because a character row draws none at all, and - /// already spells "off" as one of the format's own choices — one - /// control over one stored value rather than two over one. + /// A character's logging is two fields of the character's own row, drawn in the character form, so + /// the character it applies to is on screen beside it. There is no Space-to-start checkbox because a + /// character row draws none, and already spells "off" as one of the + /// format's own choices — one control over one stored value. /// /// - /// Each list pane ends in its own buttons, because a button acts on the list it is drawn under and - /// the cursor is already there. A button that would act on nothing is left out rather than drawn - /// dead: a world with no characters offers + add character and nothing else, so ⏎ never - /// lands on a row that silently does nothing. + /// Each list pane ends in its own buttons, which act on the list they are drawn under. A button that + /// would act on nothing is left out rather than drawn dead, so ⏎ never lands on a row that silently + /// does nothing. /// /// /// - /// Which trigger set the third pane has selected — what [[- del]] would remove. It defaults to - /// the first, the way every other pane's cursor starts on its first row, so a caller that only wants - /// the navigable shape still gets the pane's real buttons. + /// Which trigger set the third pane has selected — what [[- del]] would remove. Defaults to the + /// first, so a caller wanting only the navigable shape still gets the pane's real buttons. /// /// /// Opens the read-only MSSP report for the world at the given index, or null when this projection has - /// nowhere to open one — which is every caller but the live app: the renderer is pure and a screen is - /// not something a markup block can put on the screen by itself. Null means the WORLDS pane grows no - /// i row, so the header hint (derived from ) does not - /// advertise a key that would do nothing. + /// nowhere to open one — every caller but the live app, since the renderer is pure. Null means the + /// WORLDS pane grows no i row, so the header hint does not advertise a key that would do + /// nothing. /// internal static ScreenModel Model( IReadOnlyList worlds, @@ -484,21 +457,17 @@ internal static ScreenModel Model( } /// - /// Where the four panes actually sit, which is not the order they are numbered in. The WORLDS list - /// is the left column on its own; the detail column beside it runs security → characters → - /// trigger sets down the screen, with the sets drawn in the editing band across the bottom. + /// Where the four panes sit, which is not the order they are numbered in: WORLDS is the left column + /// alone, and the detail column beside it runs security → characters → trigger sets down the screen. /// - /// The mismatch is deliberate and predates this: is appended, - /// because a pane index is a cursor coordinate and slotting one in beside the world it describes - /// would renumber every stop the screen and its tests navigate by. Numbering and drawing therefore - /// disagree, and until the panes said where they were, ⇥ believed the numbering: it went WORLDS → - /// CHARACTERS → TRIGGER SETS and then jumped back up the screen to the security - /// checkboxes. Three hops forward and one backwards is what "the tabbing feels awkward" was. + /// The mismatch is deliberate. is appended, because a pane index + /// is a cursor coordinate and slotting one in beside the world it describes would renumber every stop + /// the screen and its tests navigate by. ⇥ therefore follows this layout and not the numbering, or it + /// would run down the screen and jump back up to the security checkboxes. /// /// /// Stacking the three detail panes in one column is also what lets ↓ run from the TLS checkbox - /// straight through the characters list and on into the trigger sets, which is how the character's - /// own row is now arrived at rather than hunted for. + /// straight through the characters list and into the trigger sets. /// /// private static readonly ScreenPanePlace[] PaneLayout = @@ -527,15 +496,14 @@ internal static ScreenModel Model( }); /// - /// Renames a set and every reference to it. This is the one rename on these screens that - /// reaches outside the object it is on: a character selects automation by name, so a set renamed on - /// its own would leave every character that used it pointing at a set that no longer exists — the - /// automation would simply stop, silently, at the next connect. Each reference keeps its position in - /// the character's list, because that order is what decides which set wins a conflict. + /// Renames a set and every reference to it — the one rename on these screens that reaches + /// outside the object it is on. A character selects automation by name, so a set renamed alone would + /// leave every character using it pointing at nothing and the automation would stop silently at the + /// next connect. Each reference keeps its position, because that order decides which set wins a + /// conflict. /// - /// The references are found by the old name before anything is written, and undo comes for - /// free: 's snapshot replays this same setter with the old name, which - /// walks the references back the way it walked them here. + /// References are found by the old name before anything is written, which is also what makes + /// undo free: 's snapshot replays this setter with the old name. /// /// private static void RenameSet(IReadOnlyList worlds, TriggerSet set, string name) @@ -546,11 +514,9 @@ private static void RenameSet(IReadOnlyList worlds, TriggerSet } /// - /// The trigger-set list's buttons — the only place in the app that makes or unmakes a set. They live - /// here because this pane is the only one that shows sets as things in their own right rather than - /// as a column of somebody's rules: F2, F3, F4 and F6 each flatten one kind of the set's - /// contents across every set, so a set holding none of that kind is invisible on all four, and a set - /// you have just made holds nothing at all. + /// The trigger-set list's buttons — the only place in the app that makes or unmakes a set, because + /// this pane is the only one showing sets as things in their own right rather than as a column of + /// somebody's rules. /// /// A new set is empty and named apart from its neighbours, because its name is a key rather than a /// label () and two called New Set could not both be @@ -583,19 +549,13 @@ private static List SetButtons( } /// - /// Deleting a set, which is the destructive edit on these screens with the longest reach: the set - /// goes, and so does every character's opt-in to it — a character left holding the name of a set - /// that no longer exists would show an assignment that resolves to nothing. - /// - /// It is a hand-built button rather than because its undo is - /// two restorations, not one: the set back at its index, and each stripped reference back at - /// its index inside the character that held it. Restoring the set alone would be the - /// quieter half of a change nobody agreed to — and the closing review offers exactly that undo, so - /// this is the half of it that has to be complete. - /// + /// Deleting a set: the set goes and so does every character's opt-in to it, since a character holding + /// the name of a set that no longer exists would show an assignment resolving to nothing. /// - /// It is also why the review's question names more than a set's name: what the deletion costs is the - /// rules inside it and the characters that used it, and neither is visible once it is gone. + /// Hand-built rather than because its undo is two restorations, + /// not one: the set back at its index, and each stripped reference back at its index inside + /// the character that held it. The closing review offers exactly that undo, so this half has to be + /// complete — and it is why the review's question names more than the set's name. /// /// private static ScreenButton RemoveSet( @@ -659,16 +619,14 @@ private static string DescribeWorld(WorldDefinition world) => private static string Plural(int count) => count == 1 ? string.Empty : "s"; /// - /// The selected world's two security booleans, in the order the WORLD block draws them. They are - /// checkboxes rather than one typed value because that is what they are — two independent flags, - /// which no single field could offer without inventing a vocabulary for the four combinations of - /// them — and they are two rows because a carries at most one checkbox. + /// The selected world's two security booleans, in the order the WORLD block draws them. Two + /// checkboxes rather than one typed value because they are two independent flags, and two rows + /// because a carries at most one checkbox. /// - /// Certificate validation is a plain bool here and is deliberately still bound in the - /// direction config stores it: the checkbox reads accept invalid certificates, so the - /// dangerous state is the *checked* one and the screen can paint it as such (see - /// ). A checkbox showing the inverse of its stored value would read - /// more comfortably and would be one negation away from silently turning verification off. + /// Certificate validation is bound in the direction config stores it: the checkbox reads + /// accept invalid certificates, so the dangerous state is the checked one and the + /// screen can paint it as such. A checkbox showing the inverse would read more comfortably and would + /// be one negation away from silently turning verification off. /// /// private static ScreenRow[] SecurityRows(WorldDefinition world) => new[] @@ -679,16 +637,15 @@ private static ScreenRow[] SecurityRows(WorldDefinition world) => new[] }; /// - /// The WORLDS pane's buttons, in the order they are drawn and — decisively — in the order - /// needs them: the cursor stop first, then the two targeted key - /// hints. [+ world] has no target and so needs somewhere to be pressed from; i and - /// Del both act on the selected row and must not steal the cursor from it, so they trail and - /// are trimmed out of the pane's stop count. Put either of them above [+ world] and the + /// The WORLDS pane's buttons, in the order needs them: the cursor + /// stop first, then the two targeted key hints. [+ world] needs somewhere to be pressed from; + /// i and Del act on the selected row and must not steal the cursor from it, so they + /// trail and are trimmed out of the pane's stop count. Put either above [+ world] and the /// cursor gains a hole. /// - /// Both targeted keys are offered only when there is a world under the cursor for them to act on. A - /// brand-new world is a blank template, because a world's whole identity is its host and a - /// "helpfully" prefilled one would be a guess the user then has to notice and undo. + /// Both targeted keys are offered only with a world under the cursor. A new world is a blank + /// template, since a world's identity is its host and a prefilled one would be a guess the user has + /// to notice and undo. /// /// private static List WorldButtons( @@ -840,12 +797,10 @@ internal static List WorldsColumn( } /// - /// How many rows the column has, or 0 when the caller has none. This is the one pane on these - /// screens that draws rows belonging to three of its four cursor panes, so on a short terminal it - /// has to give its blanks up and then scroll — see and - /// . At 100×24 it ran to nineteen rows in twelve, and the twelve - /// it drew stopped just above the CHARACTERS list, so every character row and every button under - /// them was a cursor stop that had never been drawn. + /// How many rows the column has, or 0 when the caller has none. This is the one pane drawing rows + /// belonging to three of its four cursor panes, so on a short terminal it gives up its blanks and then + /// scrolls — see and . Without + /// that, character rows and the buttons under them are cursor stops that were never drawn. /// internal static List DetailColumn( IReadOnlyList worlds, @@ -866,14 +821,9 @@ internal static List DetailColumn( var world = worlds[selectedWorld]; // The world's own fields are the WORLDS-list row's fields, in this order — the detail column is - // where they are displayed, so it is where an open edit draws its caret. - // - // There is no title strip above them any more. It read - // `Aetherfall aetherfall.mux:4201 TLS on · UTF-8` — and every token of it was repeated in the - // five rows immediately underneath, twice over for the address, which the WORLDS list beside it - // also carries. A summary directly above the thing it summarises is not a summary; it is the - // same row drawn again in a shape you cannot edit, and the two cells it cost were the two rows - // the CHARACTERS list needed at 100×24. + // where they are displayed, so it is where an open edit draws its caret. No title strip above + // them: every token of one would be repeated in the rows underneath and in the WORLDS list beside + // it, and the rows it cost are rows the CHARACTERS list needs at 100x24. var right = new List { $"[{accent}]├ WORLD[/]", @@ -934,16 +884,12 @@ internal static List DetailColumn( } /// - /// The world's security block: two checkbox rows where a read-only security TLS on · certs - /// strict summary used to sit. The summary said everything and offered nothing — the two flags - /// behind it had no UI at all — so it is replaced by the rows themselves rather than kept above - /// them. The column's title strip used to repeat the answer a third time (TLS on) and has - /// since gone with the rest of its duplications; the checkbox is the one-glance answer, and - /// it is the only one that can also be pressed. + /// The world's security block: two checkbox rows, in place of a read-only summary of the same two + /// flags. The checkbox is the one-glance answer and it is the only one that can also be + /// pressed. /// - /// They keep the security label column so the block still reads as one setting with two - /// switches, and so the checkboxes line up under the field wells above them rather than starting at - /// the margin. + /// They keep the security label column so the block reads as one setting with two switches, and + /// so the checkboxes line up under the field wells above them rather than at the margin. /// /// private static List SecurityColumn(WorldDefinition world, ScreenFocus cursor) => new() @@ -957,15 +903,14 @@ internal static List DetailColumn( }; /// - /// The certificate checkbox. It is the one row on these screens that can switch off a check the user - /// is otherwise entitled to assume is running, so — unlike the encoding beside it — it does not - /// stay quiet about it: checked *and* encrypting, it is drawn in - /// with the same ▲ a refused value gets, and says what the state actually costs rather than - /// restating its own label. + /// The certificate checkbox — the one row on these screens that can switch off a check the user is + /// otherwise entitled to assume is running. Checked and encrypting, it is drawn in + /// with the ▲ a refused value gets, and says what the state + /// costs rather than restating its label. /// - /// With TLS off it is drawn plainly whatever it holds, and says so: nothing is being validated - /// either way, and a warning that fires on a connection carrying no certificates would train the - /// eye to ignore the one that matters. + /// With TLS off it is drawn plainly whatever it holds: nothing is being validated either way, and a + /// warning firing on a connection carrying no certificates would train the eye to ignore the one that + /// matters. /// /// private static string CertificateRow(WorldDefinition world) @@ -994,49 +939,30 @@ private static string Checkbox(string label, bool value, string hint) /// /// The character form — labels left-aligned with their values, one field per row. The editable ones - /// are the character row's own fields (name, password, connect line, on-connect, at start, - /// the two log values, then restore) and are the only eight drawn in a field well. The other - /// two deliberately are not: login is derived from the fields above it and there is - /// nothing there to set, and the session line is a report of what the connection is doing rather - /// than a setting. + /// are the character row's own fields (name, password, connect line, on-connect, at start, the + /// two log values, then restore) and are the only eight drawn in a field well. login is + /// derived from the fields above it and the session line is a report, so neither gets one. /// /// A row here draws no checkbox at all (), so every settable thing - /// about a character is a field on this form and there is nothing Space can reach. That used not to - /// be true in the model — the row bound a toggle over autoLogin which the renderer never drew - /// — and the defect it caused is the one this whole change is about: the form said auto-login - /// no with no well beside it, correctly meaning "not editable here", and there was no "here" to - /// go to. A saved password sat inert behind a control that did not exist. The toggle is gone with the - /// setting; do not put another one on this row without also drawing it. - /// - /// - /// at start is therefore a field and must stay one. A closed two-value choice is the - /// next-nearest control the screen already knows, and it lands the setting beside the rows it has to - /// be told apart from. It is drawn directly above login on purpose: at start decides - /// whether a connection is opened at launch, login reports what gets typed once one exists, - /// and neither implies the other. + /// about a character is a field on this form and there is nothing Space can reach. Do not bind a + /// toggle on this row without also drawing it: the value would be unreachable behind a control that + /// does not exist. /// /// - /// The password was a seventh readout until it had somewhere to go. It was drawn without a - /// well and labelled keychain — an affordance-free row advertising a credential store that - /// does not exist anywhere in this codebase. It is now a real field, masked - /// (), and its note says what is actually true: the value is - /// written to secrets.json in plaintext, and not into config.json (see - /// ). The mask is about shoulders and screenshots, not about storage, and the - /// note is what stops it being read as a claim about storage. + /// at start is therefore a field and must stay one, drawn directly above login: it + /// decides whether a connection is opened at launch, login reports what gets typed once one + /// exists, and neither implies the other. /// /// - /// The connect line is drawn here for the first time, because it is only now a thing worth - /// looking at: it is a template (), it holds the two tokens by - /// default, and a user who can see it can move their password out of it. While it was an - /// interpolated string built in C#, the screen had nothing to show and no way to let anyone edit it, - /// which is exactly why the plaintext-in-the-connect-string workaround was the only option on offer. + /// The password is masked () and its note says where the + /// value goes (). The mask is about shoulders and screenshots, not storage, + /// and the note is what stops it being read as a claim about storage. /// /// - /// The log rows are here, under a heading that names the character, because logging is per - /// character: LoggingSettings hangs off . On its own screen - /// it had to guess whose settings to show and said nothing about the answer; here the question - /// cannot come up, and the row says this character only anyway for the reader arriving from - /// the F9 it used to live on. + /// The log rows are here, under a heading naming the character, because logging is per + /// character — LoggingSettings hangs off — so a screen of + /// its own would have to guess whose settings it was showing. The row says this character only + /// for the reader arriving from F9. /// /// internal static List FormColumn( @@ -1135,37 +1061,24 @@ internal static List FormColumn( /// /// What the password row says about where the value goes: into secrets.json, as plaintext. - /// is saved — just not into config.json, which carries - /// only a meaningless GUID (). This row is the only place a - /// user could learn any of that. + /// is saved — just not into config.json, which + /// carries only a meaningless GUID (). This row is the + /// only place a user could learn any of that. /// - /// It has said four different things, and the sequence is worth keeping legible because each was true of - /// the design it shipped with. It said keychain — a credential store that exists nowhere in this - /// codebase, which is the exact class of claim these screens' tests are written to prevent. It said - /// this session only — never saved, honest while the field was [JsonIgnore] and a lie the - /// moment passwords were persisted. It briefly said saved in config.json, plain text, which was - /// honest about a design that put the secret in the file people paste. It now names the file the secret - /// is actually in. + /// Plainly, not glossed. "Encrypted" would be false, "stored securely" would gloss a plaintext + /// file, and saying only saved would let a reader supply the reassuring half themselves. Naming + /// the file is what makes the note actionable — someone excluding credentials from a backup knows what + /// to open. The owner-only mode and the safety of sharing config.json are in + /// docs/design/README.md and --help; neither is something a reader of this row must act + /// on. /// /// - /// Plainly, not glossed. "Encrypted" would be false, "stored securely" would be a gloss over a - /// plaintext file, and saying only saved would let a reader supply the reassuring half themselves. - /// Naming the file is what makes the note actionable — a user who wants to look after their credentials, - /// exclude them from a backup or check what is in them now knows what to open. The two things it does not - /// have room for are the owner-only mode and the fact that config.json is consequently safe to - /// share; both are in docs/design/README.md and in --help, and neither is something a reader - /// of this row has to act on. - /// - /// - /// It takes a row of its own, with a blank label, the way the certificate checkbox hangs off the - /// security row above it. Beside the value it would have had to share a 48-cell panel - /// () with a field whose drawn width is the buffer's own — so it wrapped - /// the moment a password was set, and a wrapped row costs the form a line it was not measured for and - /// pushed log folder out of the band. On its own row it cannot collide with a value of any - /// length, and it is drawn while the field is open as well as at rest, which is when it is most worth - /// reading. CharField spends 14 of the panel's 47 usable cells on the indent and the label - /// column, so this string has 33 to fit in and - /// ScreenPasswordFieldTests.EveryFormRowFitsThePanel… is what stops the next edit forgetting it. + /// It takes a row of its own with a blank label, the way the certificate checkbox hangs off the + /// security row. Beside the value it would share a 48-cell panel with a field whose drawn width + /// is the buffer's own, so it would wrap as soon as a password was set — and a wrapped row costs the + /// form a line it was not measured for. CharField spends 14 of the panel's 47 usable cells on + /// the indent and label column, leaving this string 33; + /// ScreenPasswordFieldTests.EveryFormRowFitsThePanel… is the pin. /// /// internal const string StorageNote = "saved in secrets.json, plaintext"; @@ -1178,23 +1091,18 @@ internal static List FormColumn( /// /// The one line on this screen that says how to get into it. The CHARACTER form draws four field - /// wells — the affordance these screens use to mean "the keyboard can change this here" — but the - /// cursor stop that opens them is the character's row in the CHARACTERS list, a column and a band - /// away. So the wells were honest about being editable and silent about being editable - /// from somewhere else, and the way in was reachable, correct and unguessable: the row you - /// arrive on by ⇥ is a bare selector with no well of its own, which by this project's own rule reads - /// as "nothing to type into here". + /// wells — this project's affordance for "the keyboard can change this here" — but the cursor stop + /// that opens them is the character's row in the CHARACTERS list, a column and a band away, and that + /// row is a bare selector with no well of its own. /// - /// It is derived from the cursor rather than written once, so it names the key at the moment the key - /// would work: ⏎ opens these while the cursor is on that character's row, and where to go - /// otherwise. Mid-edit it says nothing at all — the header hints have already swapped wholesale to - /// ⏎ commit · Esc revert, and a second, staler claim beside them would be the disagreement - /// those hints exist to prevent. + /// Derived from the cursor rather than written once, so it names the key at the moment the key works: + /// ⏎ opens these while the cursor is on that character's row, and where to go otherwise. + /// Mid-edit it says nothing — the header hints have already swapped to ⏎ commit · Esc revert, + /// and a second, staler claim beside them is the disagreement those hints exist to prevent. /// /// /// It takes the blank row under the heading rather than sitting beside it, so it costs no row and - /// cannot be pushed off the form's 48-cell column by a long character name — the heading already - /// spends that width on the name itself. It still reads as a separator when it is empty. + /// cannot be pushed off the 48-cell column by a long character name. /// /// private static string FormDoor(ScreenFocus cursor, int selectedCharacter) @@ -1303,18 +1211,12 @@ private static int Width(IReadOnlyList sets, Func VisibleLength(part(s))); /// - /// One row of the CHARACTERS list: which character, and nothing else. It is a selector — - /// the row you move the cursor onto to bring a character up in the CHARACTER form below — and it - /// used to be a four-column table (name state login trigger sets) whose other three - /// columns were all drawn again, on the same screen, at the same time: session and - /// login are rows of the form directly underneath, and the sets are the pane beside it, - /// with checkboxes, descriptions and counts. A list that restates the detail pane is a list you have - /// to read to discover it says nothing new — and its column header cost a row that the list itself - /// needed on a short screen. - /// - /// So: the list says which characters there are and which one is selected, and the form owns - /// everything about the one that is. - /// + /// One row of the CHARACTERS list: which character, and nothing else. It is a selector — the + /// row you move the cursor onto to bring a character up in the CHARACTER form below — so it carries no + /// state, login or trigger-set columns: all three are drawn on the same screen at the same time, by + /// the form underneath and the pane beside it. A list restating the detail pane is one you have to + /// read to discover it says nothing new, and its header would cost a row the list needs on a short + /// screen. /// private static string CharacterRow(CharacterDefinition character, bool selected) { @@ -1340,20 +1242,13 @@ internal static string StartupDetail(bool connectAtStartup) => /// /// The login cell: what this character will actually type when it connects, read off - /// rather than off a setting. - /// - /// This row is the whole reason the screen changed. It replaced an auto-login readout - /// that said yes or no about a stored boolean — and a character could sit there with a - /// saved password, that flag at its default, and a row cheerfully reading no that never - /// explained it was the reason the password did nothing. Two of this repo's own characters were in - /// exactly that state. The flag is gone; this row reports the derived answer, so there is no longer a - /// setting the screen can agree with while the client disagrees. - /// + /// rather than off a setting — so there is no stored flag the + /// screen can agree with while the client disagrees. /// /// is drawn in , the ink the - /// only other misconfiguration on this screen uses (a world trusting invalid certificates). It is the - /// one state here where the user has done something that cannot work: the password field above is - /// filled in and the connect line beside it has nowhere to put it. + /// only other misconfiguration on this screen uses. It is the one state where the user has done + /// something that cannot work: the password field above is filled in and the connect line beside it + /// has nowhere to put it. /// /// internal static string LoginDetail(CharacterDefinition character) => character.Login() switch