From d521917d91245624f55e35b27b026a03044cf815 Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Fri, 14 Aug 2026 10:09:04 -0500 Subject: [PATCH] =?UTF-8?q?fix(search):=20give=20=E2=8C=83F's=20results=20?= =?UTF-8?q?the=20room=20the=20terminal=20has?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three defects, all visible in one reported frame: the results were squeezed into twenty-odd cells while a sixty-cell window column of blank sat beside them. `_lines` is wider than the workspace on purpose — RestorePreviousSession buffers a restore log the workspace cannot place under its own id, so its pane refills if that channel speaks again — and those ids have no WorkspaceWindow to be titled from, so WindowTitle handed back the raw `spawn:24:World|Character:Target` id. Every row was padded to it. Those rows were dead as well: ActivateWindow refuses a window no pane holds, so ⏎ inserted its bar into a buffer nothing paints. The corpus is now Workspace.WindowsFor's rule, and GoToSearchHit honours Activate's answer. The window column is bounded at eighteen cells and elided past it: a title is not this client's text to trust, and every row pads to the widest one. It also pads by visible width now, so a window called `[Chat]` no longer sits a cell adrift of every other row. And the surface opens at the room the terminal has. It has no unfiltered list to size to — an empty query matches nothing — so the width was measured against the only content an empty surface has, which is its own footer: eighty-odd cells on any terminal, with every result elided to fit a window sized by a key hint. The height always took the desktop. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 14 +++++ src/SharpMUTerm.Tui/SearchPrompt.cs | 41 ++++++++----- src/SharpMUTerm.Tui/SearchSurface.cs | 10 ++-- src/SharpMUTerm.Tui/SharpMUTermApp.cs | 21 ++++++- .../SearchEndToEndTests.cs | 23 +++++++ .../SearchPromptTests.cs | 60 +++++++++++++++++++ .../SpawnWindowIdUpgradeTests.cs | 42 +++++++++++++ 7 files changed, 190 insertions(+), 21 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 9f9200c3..1486a1ee 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -112,6 +112,20 @@ fallbacks) for inline images/maps. search would find nothing in exactly the windows people search hardest. The bound is *stated* — `12 found · 4,812 lines held` — so a reader who cannot find an old line sees why rather than concluding the search is broken. + - **Only windows a pane holds are searched**, `Workspace.WindowsFor`'s rule and for its reason: `⏎` + takes the reader to the hit, and `ActivateWindow` refuses a window no pane holds. `_lines` is wider + than the workspace deliberately — `RestorePreviousSession` buffers a restore log the workspace cannot + place under its own id, so its pane refills if that channel speaks again — and such a window has no + `WorkspaceWindow` to be titled from, so `WindowTitle` handed back the raw + `spawn:24:World|Character:Target` id. That was the reported "the results take up a small amount of + room": a sixty-cell window column of blank against a twenty-cell result. `GoToSearchHit` now honours + `Activate`'s answer as well, rather than inserting its bar into a buffer nothing paints. + - **Both dimensions are the room there is.** There is no unfiltered list to size to — an empty query + matches nothing — so the width used to be measured against the only content an empty surface has, + which is its own footer, and the surface opened at eighty-odd cells on any terminal while eliding + every result to fit a window sized by a key hint. The height always took the desktop; the width does + now too. `SearchPrompt.MaxLabelWidth` bounds the window column on top of that, because a title is not + this client's text to trust and every row is padded to the widest one. - **`PaneLine.Plain` is held, not derived.** Matching runs over the visible text so a colour change mid-word cannot split a match and `#ff0000` cannot find every red line (`UrlDetector`'s rule, one layer down) — and it is computed once at append, because the surface refilters over every line of diff --git a/src/SharpMUTerm.Tui/SearchPrompt.cs b/src/SharpMUTerm.Tui/SearchPrompt.cs index da1f5c85..63cc7a99 100644 --- a/src/SharpMUTerm.Tui/SearchPrompt.cs +++ b/src/SharpMUTerm.Tui/SearchPrompt.cs @@ -66,6 +66,15 @@ internal static class SearchPrompt /// The longest an entry is drawn before it is elided, when no width is supplied. private const int DefaultEntryWidth = 72; + /// + /// The widest the window column is drawn in ⌥A scope. Every row is padded to the widest label + /// in the list, so an unbounded column is a column one long title can spend the whole row on — and a + /// title is not this client's text to trust: Snippet caps it at sixty cells, which was sixty + /// cells of blank against a twenty-cell result. A label past this is elided, which is the rail's rule + /// (RailRenderer) one surface over. + /// + internal const int MaxLabelWidth = 18; + /// /// What one keystroke does, and the state it leaves behind. is how many rows /// are listed, so the pointer wraps within what is actually on screen. @@ -174,7 +183,9 @@ internal static List Render( ArgumentNullException.ThrowIfNull(rows); ArgumentNullException.ThrowIfNull(query); - var labelWidth = all ? rows.Select(r => VisibleLength(Escape(r.WindowLabel))).DefaultIfEmpty(0).Max() : 0; + var labelWidth = all + ? Math.Min(MaxLabelWidth, rows.Select(r => r.WindowLabel.Length).DefaultIfEmpty(0).Max()) + : 0; var entryWidth = (width > 0 ? width : DefaultEntryWidth) - 4 - (labelWidth > 0 ? labelWidth + 2 : 0); var lines = new List @@ -230,18 +241,6 @@ internal static int Scroll(int first, int selected, int count, int listRows) return Math.Clamp(top, 0, count - listRows); } - /// The visible width of the widest rendered line — used to size the surface to its content. - internal static int MaxWidth(IReadOnlyList lines) - { - var max = 0; - foreach (var line in lines) - { - max = Math.Max(max, VisibleLength(line)); - } - - return max; - } - /// /// The two toggles and what they currently mean, in words rather than in glyphs: the surface has to /// be able to say which way they are set, because both change what a query finds and neither @@ -273,9 +272,7 @@ private static string QueryMarkup(string query) => private static string Row(SearchRow row, bool selected, int entryWidth, int labelWidth, int width) { var (text, matchStart, matchLength) = Elide(row, entryWidth); - var label = labelWidth > 0 - ? Escape(row.WindowLabel).PadRight(labelWidth) + " " - : string.Empty; + var label = labelWidth > 0 ? LabelCell(row.WindowLabel, labelWidth) : string.Empty; if (selected) { @@ -300,6 +297,18 @@ private static string Row(SearchRow row, bool selected, int entryWidth, int labe return $"{prefix}[{Value}]{before}[/][bold {Accent}]{hit}[/][{Value}]{after}[/]"; } + /// + /// One window-column cell: the label elided to the column and padded out to it, then the two-cell + /// gap. Padded by visible width, because a window may be called [Chat] and an escaped + /// bracket is two characters standing for one cell — PadRight shortened the column by one per + /// bracket and left that row's text a cell adrift of every other row's. + /// + private static string LabelCell(string label, int labelWidth) + { + var shown = label.Length <= labelWidth ? label : label[..(labelWidth - 1)] + "…"; + return PadVisible(Escape(shown), labelWidth) + " "; + } + /// /// Shortens an over-long line to the surface's width, keeping the matched run visible: a pose is /// hundreds of cells long, and a row clipped at the left edge would hide the very text the query diff --git a/src/SharpMUTerm.Tui/SearchSurface.cs b/src/SharpMUTerm.Tui/SearchSurface.cs index 74121db9..fbf71c7f 100644 --- a/src/SharpMUTerm.Tui/SearchSurface.cs +++ b/src/SharpMUTerm.Tui/SearchSurface.cs @@ -139,11 +139,13 @@ private void Open() // Sized once and never again, HistorySurface's rule: narrowing must pad the list area rather than // shrink the window, so the rows and the footer stay where the eye left them. There is no - // unfiltered list to size to here — an empty query matches nothing — so the height is the room - // there is rather than the room the results need. + // unfiltered list to size to here — an empty query matches nothing — so *both* dimensions are the + // room there is rather than the room the results need. The height always was; the width was + // measured against the only content an empty surface has, which is its own footer, so it opened + // at eighty-odd cells on any terminal and every result was elided to fit a window sized by a key + // hint. What the surface holds is a game's own lines, and they are wider than that by design. _listRows = Math.Max(3, desktop.Height - ChromeRows - 6); - _contentWidth = Math.Clamp( - SearchPrompt.MaxWidth(Lines) + 2, MinimumWidth, Math.Max(MinimumWidth, desktop.Width - 6)); + _contentWidth = Math.Max(MinimumWidth, desktop.Width - 6); var width = _contentWidth + 2; // + the 1-cell left/right border var height = Math.Min(_listRows + ChromeRows + 2, Math.Max(ChromeRows + 3, desktop.Height - 2)); diff --git a/src/SharpMUTerm.Tui/SharpMUTermApp.cs b/src/SharpMUTerm.Tui/SharpMUTermApp.cs index b2ab11f1..09bf4b71 100644 --- a/src/SharpMUTerm.Tui/SharpMUTermApp.cs +++ b/src/SharpMUTerm.Tui/SharpMUTermApp.cs @@ -3326,7 +3326,14 @@ private void GoToSearchHit(SearchRow row, string query, int ordinal, int total) return; } - Activate(row.WindowId); + // Honoured, not fired and forgotten: a window no pane holds cannot be activated, and going on + // would insert the bar into a buffer nothing paints and then scroll a pane that is not there. + // SearchableWindows already keeps such a window out of the corpus; this is the second line. + if (!Activate(row.WindowId)) + { + RefuseCommand($"{Snippet(WindowTitle(row.WindowId))} is not in any pane"); + return; + } var at = Math.Clamp(row.LineIndex, 0, buffer.Count); InsertChromeRow(row.WindowId, at, SearchBarRenderer.Bar(query, ordinal, total, FrozenAccentHex())); @@ -3946,6 +3953,17 @@ private void ToggleSearch() /// activity boundary and RepaintPanes make. Labels go through : a window /// title can be a world's text (the web view is titled from the page it loaded). /// + /// + /// Only windows a pane actually holds are searched — 's rule, + /// and for its reason: ⏎ takes the reader to the hit, and + /// refuses a window no pane holds, so a hit in one is a row that can never be shown. _lines is + /// wider than the workspace on purpose — buffers a restore log + /// the workspace cannot place under its own id, so its pane refills if that channel speaks again — + /// and those ids have no to be titled from, so + /// gave back the raw spawn:24:World|Character:Target id. That is + /// what padded the window column to sixty cells and squeezed every result into the twenty that were + /// left: the reported "the results take up a small amount of room". + /// /// private IReadOnlyList SearchableWindows(bool all) { @@ -3954,6 +3972,7 @@ private IReadOnlyList SearchableWindows(bool all) : new[] { ActiveWindowId() }.Where(_lines.ContainsKey); return ids + .Where(id => _workspace.Layout.FindWindow(id) is not null) .Select(id => new SearchCorpus( id, Snippet(WindowTitle(id)), diff --git a/tests/SharpMUTerm.Tui.Tests/SearchEndToEndTests.cs b/tests/SharpMUTerm.Tui.Tests/SearchEndToEndTests.cs index f4a90826..a975a991 100644 --- a/tests/SharpMUTerm.Tui.Tests/SearchEndToEndTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/SearchEndToEndTests.cs @@ -42,6 +42,29 @@ private static (SharpMUTermApp App, WorldSession Session) Bound() private static ConsoleKeyInfo Bare(ConsoleKey key) => new('\0', key, false, false, false); + /// + /// The surface opens at the room the terminal has, not at the width of its own footer. It has no + /// unfiltered list to size to — an empty query matches nothing — so it used to measure the only + /// content it had, which is the key hints, and open at eighty-odd cells on any terminal; the results + /// were then elided to fit a window sized by a hint. The height has always taken the room there is, + /// and this is the other half of that rule. + /// + [Test] + public async Task TheSurfaceOpensAtTheRoomTheTerminalHasRatherThanAtItsFootersWidth() + { + var (app, session) = Bound(); + session.PrintSystem("*** the vault key is behind the bar"); + + app.SimulateKey(Ctrl(ConsoleKey.F)); + app.SimulateSearchTyping("vault"); + var rows = FrameGrid.Decode(app.RenderWholeFrame(), Width, Height); + + var footer = rows.Single(r => r.Contains("type to search", StringComparison.Ordinal)); + await Assert.That(footer.TrimEnd().Length).IsGreaterThan(Width - 8); + await Assert.That(rows.Any(r => r.Contains("vault key is behind the bar", StringComparison.Ordinal))) + .IsTrue(); + } + [Test] public async Task CtrlFOpensTheSurfaceAndCtrlFAgainClosesIt() { diff --git a/tests/SharpMUTerm.Tui.Tests/SearchPromptTests.cs b/tests/SharpMUTerm.Tui.Tests/SearchPromptTests.cs index 8cb9daf3..ca4321a3 100644 --- a/tests/SharpMUTerm.Tui.Tests/SearchPromptTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/SearchPromptTests.cs @@ -228,6 +228,66 @@ public async Task AnUnselectedRowMarksWhereTheQueryLanded() await Assert.That(lines.Any(l => l.Contains("[bold ") && l.Contains("goblin"))).IsTrue(); } + /// + /// Every row is padded to the widest label, so an unbounded window column is one long title spending + /// the whole row. The reported frame had a sixty-cell column of blank against a twenty-cell result: + /// a restore log the workspace could not place was searched under its raw + /// spawn:24:World|Character:Target id. keeps such a window + /// out of the corpus; this is the bound that holds whatever a title turns out to be. + /// + [Test] + public async Task ALongWindowNameIsElidedRatherThanSpendingTheRowOnItself() + { + var rows = new[] + { + new SearchRow("main", "main", 12, "The goblin snarls at you.", 4, 6), + new SearchRow( + "spawn:24:Convergence MUSH|Mannaz:O-Gatecrashers", + "spawn:24:Convergence MUSH|Mannaz:O-Gatecrashers", + 3, + " Ana: goblin room is bugged", + 11, + 6), + }; + + var listed = SearchPrompt.Render(rows, "gob", null, false, true, "main", 10, -1, width: 100) + .Where(l => l.Contains("goblin")) + .Select(MarkupText.Plain) + .ToArray(); + + await Assert.That(listed.Length).IsEqualTo(2); + await Assert.That(string.Join('\n', listed)).DoesNotContain("O-Gatecrashers"); + + // The column is the bound rather than the label: three cells of pointer, the column, two of gap. + await Assert.That(listed[0].IndexOf("The goblin", StringComparison.Ordinal)) + .IsEqualTo(SearchPrompt.MaxLabelWidth + 5); + await Assert.That(listed[1].IndexOf("", StringComparison.Ordinal)) + .IsEqualTo(SearchPrompt.MaxLabelWidth + 5); + } + + /// + /// A window may be called [Chat]. Escaping turns each bracket into two characters standing for + /// one cell, so a column padded by string.Length comes up a cell short per bracket and leaves + /// that row's text adrift of every other row's. + /// + [Test] + public async Task ABracketedWindowNamePadsToTheSameColumnAsEveryOtherRow() + { + var rows = new[] + { + new SearchRow("main", "[Chat]", 12, "The goblin snarls at you.", 4, 6), + new SearchRow("spawn:chat", "Ansible", 3, "The goblin room is bugged", 4, 6), + }; + + var listed = SearchPrompt.Render(rows, "gob", null, false, true, "main", 10, -1, width: 100) + .Where(l => l.Contains("goblin")) + .Select(l => MarkupText.Plain(l).IndexOf("The goblin", StringComparison.Ordinal)) + .ToArray(); + + await Assert.That(listed.Length).IsEqualTo(2); + await Assert.That(listed[0]).IsEqualTo(listed[1]); + } + [Test] public async Task ScrollKeepsThePointedAtRowInsideTheListArea() { diff --git a/tests/SharpMUTerm.Tui.Tests/SpawnWindowIdUpgradeTests.cs b/tests/SharpMUTerm.Tui.Tests/SpawnWindowIdUpgradeTests.cs index 77b7fd46..da311b40 100644 --- a/tests/SharpMUTerm.Tui.Tests/SpawnWindowIdUpgradeTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/SpawnWindowIdUpgradeTests.cs @@ -168,6 +168,48 @@ public async Task AnOldLogNoPaneClaimsIsLeftWhereItIs() await Assert.That(app.WindowIds().Any(id => id.EndsWith(":Tells", StringComparison.Ordinal))).IsFalse(); } + /// + /// …and ⌃F does not offer it. Those lines are buffered under an id no pane holds, so ⏎ on one has + /// nowhere to take the reader — Workspace.ActivateWindow refuses a window with no pane, and + /// the surface would insert its bar into a buffer nothing paints. It was also drawing them under the + /// raw spawn:24:World|Character:Target id, which padded the window column to sixty cells and + /// left every result squeezed into what was left: the reported "the results take up a small amount + /// of room". holds the rest of ⌥A; this is the corpus it looks in. + /// + [Test] + public async Task ABufferedWindowNoPaneHoldsIsNotSearched() + { + using var root = new TempRoot(); + SeedLegacyLog(root); + using (var seed = new RestoreLog(root.Path)) + { + seed.Append("spawn:Tells", "Tells", StyledLine.FromText("Rivane pages: hello", TextStyle.Default), "09:24"); + } + + var config = OldConfiguration(); + using var log = new RestoreLog(root.Path, config.RestoreLog); + Console.SetIn(TextReader.Null); + await using var app = new SharpMUTermApp( + config, Headless, new HeadlessConsoleDriver(Width, Height), restore: log); + + // The placed window's restored lines are found, so an empty result for the other one is the pane + // rule at work rather than a search that finds nothing restored at all. + app.SimulateKey(new ConsoleKeyInfo('\0', ConsoleKey.F, false, false, true)); + app.SimulateSearchKey(new ConsoleKeyInfo('\0', ConsoleKey.A, false, true, false)); + app.SimulateSearchTyping("crypt run"); + await Assert.That(app.SearchRows.Count).IsEqualTo(1); + + foreach (var _ in "crypt run") + { + app.SimulateSearchKey(new ConsoleKeyInfo('\0', ConsoleKey.Backspace, false, false, false)); + } + + app.SimulateSearchTyping("Rivane pages"); + + await Assert.That(app.SearchRows).IsEmpty(); + await Assert.That(log.Read().Any(w => w.WindowId == "spawn:Tells")).IsTrue(); + } + /// /// And the fix survives the round trip it is most likely to be undone by. Two characters capture one /// target, the workspace is saved and reopened, and each still has a pane of their own holding their