From 1fea883d31ae0d02eb5f47d839bddb06338ff4cc Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Tue, 11 Aug 2026 14:41:19 -0500 Subject: [PATCH 01/19] docs(spec): find, the history chord, and an activity boundary that lasts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ⌃F becomes search and freeze moves to ⌥F; ⌥↑/⌥↓ recall history without the caret-row precondition; the activity boundary generalises from "you left the terminal" to "you left this window" and gains a dwell floor. Every chord was driven at a raw-mode reader with `kitten @ send-key` before it was spent — ⌃↑/⌃↓, one of the two the request offered, was never available. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN --- ...6-08-11-pane-search-and-activity-design.md | 320 ++++++++++++++++++ 1 file changed, 320 insertions(+) create mode 100644 docs/superpowers/specs/2026-08-11-pane-search-and-activity-design.md diff --git a/docs/superpowers/specs/2026-08-11-pane-search-and-activity-design.md b/docs/superpowers/specs/2026-08-11-pane-search-and-activity-design.md new file mode 100644 index 0000000..9171711 --- /dev/null +++ b/docs/superpowers/specs/2026-08-11-pane-search-and-activity-design.md @@ -0,0 +1,320 @@ +# Finding things: ⌃F search, the ⌥↑ history chord, and an activity boundary that means "since you were last here" + +**Date:** 2026-08-11 +**Status:** designed — implemented as a three-PR stack (`feat/find-chords` → `feat/window-activity-boundary` → `feat/pane-search`) + +## Problem + +Three complaints, one theme: the client holds text you cannot get back to. + +1. **There is no way to search output.** Every MU\* client has a find. A pose scrolled off ten + minutes ago, the room description with the exit you missed, who said the thing in `Chat` — the + text is in the buffer and there is no key that looks at it. `⌃F`, the chord every reader on + every platform reaches for, is spent on freeze. +2. **Command history has no dedicated key.** `↑`/`↓` recall only when the caret has nowhere further + to go, which is a rule you have to hold in your head and which stops being true the moment the + bar grows to a second row. A key that means *history* and nothing else is missing. +3. **The activity boundary only marks one kind of absence, and it vanishes too fast.** The away + divider marks where you were when you tabbed away from the *terminal*. Look away from a window + instead — switch tabs, switch characters, scroll a pane back — and the client counts the lines on + a badge but marks no boundary. Come back to a busy channel and the badge says `47` and nothing on + screen says where the 47 start. And the bar that does get drawn retires on the first keystroke + after you reach the tail, which on a shallow absence is a second or two. + +## Locked decisions + +Made explicitly during design; not to be relitigated without asking. + +| Decision | Chosen | Why not the alternative | +|---|---|---| +| Search UX | A modal results surface, the `⌃R` idiom | An in-pane `less`-style bar has nowhere to say *which pane* a hit is in, and multi-pane search is half the request | +| Search scope | Focused window, `⌥A` widens to every window | "Visible panes only" omits background tabs, which is where a busy channel's history is; "always everything" buries the pane you are looking at | +| Landing | A boundary bar above the hit | Painting the matched span would destroy the game's own colours — the reason the away bar marks rather than restyles | +| Activity boundary | Accrues whenever the window is not caught up | One rule feeding the badge *and* the bar, so a badge never shows a count with no bar to explain it | +| Bar lifetime | A dwell floor in seconds | The complaint is measured in time, so the fix is; a raised input count means an hour on a quiet character and three seconds on a busy one | + +## The chords, and why these ones + +Every chord below was driven at a raw-mode reader inside the target terminal with +`kitten @ send-key` before it was spent. A decode test is not an arrival test — the rule +`Alt+Shift+arrow` was bought with. + +| chord | bytes written to the pty | what the parser makes of it | +|---|---|---| +| `alt+up` / `alt+down` | `ESC [ 1;3 A` / `ESC [ 1;3 B` | `AnsiInputParser.ParseModifiers` reads `3-1 = 2`, bit 1 → **Alt**, and the key is `UpArrow`/`DownArrow` | +| `ctrl+up` / `ctrl+down` | `ESC [ 1;5 A` / `ESC [ 1;5 B` | Arrives, and is **already spent**: pane selection, and the ladder onto the second command line | +| `alt+f` | `ESC f` | `ProcessEscape` → Alt+F | +| `ctrl+f` | `0x06` | A control byte with no other meaning; free once freeze moves off it | +| `alt+g` / `alt+shift+g` | `ESC g` / `ESC G` | `ProcessEscape` sets `shift = char.IsUpper(c)`, so the pair is distinguishable | + +`⌃↑`/`⌃↓` was one of the two the request offered and it was never available. `⌥↑`/`⌥↓` is the other, +and it is also the *right* one on this keyboard: word movement is already `⌥←`/`⌥→`, so the four +arrows under Alt are one family. + +### `⌃F` becomes search; freeze becomes `⌥F` + +`⌃F` is in CLAUDE.md's "deliberately left on Ctrl" list, and it leaves it for the reason the others +stay: the convention is worth more than the pattern, and `⌃F` means *find* to everyone who has used +a computer. Freeze keeps its letter and changes its modifier, which is the smallest move that frees +the chord — `⌥F` is delivered as `ESC f` and claimed by nothing. + +Everything that says `⌃F` moves with it, and the list is the point: `FreezeBarRenderer.Bar` (the +`❄ FROZEN ⌃F` label a user reads *while frozen*), the `⌃P` command-surface entry, `MacroKeys.AppShortcuts`, +`docs/design/README.md`, and CLAUDE.md's own Ctrl list. A chord that moved in the handler and not in +the bar would be a client telling the user to press a key that no longer does anything. + +There is no `⌃F` alias left behind. The `⌃D` precedent: a second key for one action is either a +secret or a duplicate row on every surface that lists chords, and letting it go hands a clean chord +back to macros. + +### `⌥↑` / `⌥↓` recall history, unconditionally + +`TryRecallKey` today declines anything with a modifier and recalls on the bare arrows only when +`bar.TryMoveRow` reports the caret has nowhere further to go. That stays exactly as it is — it is +how the client has always behaved and nothing is taken away. The Alt pair is added beside it and +skips the caret test entirely: on a three-row draft, `⌥↑` recalls and `↑` moves the caret, which is +the distinction the request asked for. + +Matched on **exact** modifiers (`key.Modifiers == ConsoleModifiers.Alt`). Three separate defects in +this repository have been a handler looking at the key and not the modifiers; ordering is the second +line of defence and never the first. + +A macro bound to `Alt+Up` still wins, because `DispatchMacro` runs ahead of the recall keys in +`HandleWindowKey` — the same relationship `Ctrl+←/→` already has with pane selection, and +`MacroKeys.Verdict` continues to report it honestly. + +### `⌥G` / `⌥⇧G` repeat the last search + +Reachable without reopening the surface, so walking hits is one key rather than four. `⌥⇧G` is bound +only if `alt+shift+g` measures as a distinct arrival at a raw reader; if it does not, `⌥G` wraps +forward and there is no backward chord, and no surface advertises one. + +## Part 2: `⌃F` search + +### What gets searched, and what does not + +**The pane line buffer** (`SharpMUTermApp._lines`), which is markup plus a stamp per line. Not +`WorldSession.Scrollback` and not the file-backed spill — for `RestoreLog`'s reason, one layer over: +a spawn window's lines never reach a session's scrollback at all (`ProcessOutputLine` raises +`SpawnLine`, and a gagging capture rule keeps the line out of the transcript entirely), so a +session-keyed search would find nothing in exactly the windows people search hardest. + +That makes the searchable region *what the client is holding*, which is bounded and smaller than the +session's history. The surface says so rather than implying otherwise: the counter reads +`12 of 38 · 4,812 lines held`, so a reader who does not find a line from an hour ago can see why. + +**The visible text, with markup stripped.** A `[bold #ff0000]` in the middle of a word must not split +a match, and a user must not be able to search for `#ff0000` and hit every red line. This is the same +rule `UrlDetector` follows for the same reason — run over the *line*, never span by span. + +`PaneLine` therefore gains a third component: + +```csharp +internal readonly record struct PaneLine(string Markup, string? Stamp = null, string Plain = ""); +``` + +computed once, at append, by a new `MarkupText.Plain`. The alternative — stripping on demand — would +restrip every line of every window on every keystroke of the query, and the whole point of an +incremental surface is that it refilters as you type. The cost is one extra string per buffered line, +usually shorter than the markup beside it; the buffer is already capped per window, so the memory is +bounded by the same constant that bounds the buffer. + +The timestamp gutter is not searched. It is glued on at render time (`Compose`) and is not part of +the line — searching it would mean `12:` matching every line printed in the twelfth hour. + +### `Core.Search.OutputSearch` + +Beside `HistorySearch`, UI-free, and where the tests live: + +```csharp +internal readonly record struct OutputMatch(int LineIndex, int Start, int Length); + +internal static class OutputSearch +{ + internal static IReadOnlyList Find(IReadOnlyList lines, string query, bool regex); +} +``` + +- **Case-insensitive in both modes.** `HistorySearch` is (`StringComparison.OrdinalIgnoreCase`), and + two search surfaces in one client disagreeing about case would be a bug report. Regex mode says + `(?-i)` inline when it wants otherwise, which is a documented .NET feature rather than a rule we + invented. +- **An invalid pattern is a state, not an exception.** The query line says `invalid pattern` and the + list is empty. A regex is typed one character at a time, so *most* of the time a regex query is + being typed it is invalid; throwing, or listing stale results, are both worse than saying so. +- **A match timeout.** `new Regex(pattern, options, TimeSpan.FromMilliseconds(100))`. Catastrophic + backtracking on a user's own pattern must not wedge the UI thread, and the surface refilters on + every keystroke over every line of every window. +- **One match per line.** The result is a list of *lines* to jump to, not of every occurrence; the + first match's offsets are kept so the row can mark why it is listed, exactly as + `HistorySearchPrompt.Row` does. + +### The surface + +`SearchPrompt` (pure — keystroke → decision, and the renderer) and `SearchSurface` (framework calls +only). The split is `HistorySearchPrompt`/`HistorySurface` verbatim, and for its reason: the rules +and the wording are the part a headless test can pin. + +``` +┌ search ────────────────────────────────────────────────┐ +│ search goblin▌ 12 of 38 · 4,812 lines held │ +│ │ +│ main The goblin snarls at you. │ +│ ▸ Chat Ana: goblin room is bugged │ +│ main You hit the goblin for 12. │ +│ main A goblin corpse lies here. │ +│ │ +│ type to filter · ↑↓ pick · ⏎ go · ⌥E regex │ +│ ⌥A all windows · Esc cancel · ⌃F closes │ +└────────────────────────────────────────────────────────┘ +``` + +- Opens on the focused pane's active window. `⌥A` widens to every window in the workspace, including + background tabs and windows in other panes; the window column names each hit's home and only + appears when widened. +- `⌥E` toggles regex. Both toggles are named in the footer and both travel with the query while the + surface is open. +- `⏎` goes. `Esc` cancels. `⌃F` closes — the toggle answers its own chord, `HistorySurface`'s rule, + because a global shortcut runs before any window and the key never reaches the handler. +- Anything unrecognised is swallowed. A modal surface that let stray keys through would be typing + into a command line the user cannot see. +- The footer names exactly the keys `Interpret` honours, held to the honesty rule the settings + screens and the composer are held to, and pinned by a test that presses every key it names. +- It joins `AnyOverlayOpen`, and it **refuses over an open settings screen or the composer** — two + modal windows with two `PreviewKeyPressed` handlers cannot be driven headlessly, and the composer's + `MultilineEditControl` is a focusable `IPasteTarget` that would make `SettingsOverlay`'s + driver-level paste listener double-fire. + +### Landing on a hit + +`⏎` closes the surface and then, in order: + +1. **Activates the target window** through `SharpMUTermApp.Activate` — the one activation path. It + selects the pane, raises the tab, adopts the session and re-syncs the caret and NAWS. A second + route that "just scrolled the pane" is how `_active` and the focused pane came apart before. +2. **Inserts a search bar above the hit**, through the buffer rather than through `AppendLine`: it is + the client's own chrome, so it must not badge the window unread and must not reach the restore log. + + ``` + ── ⌕ goblin (12 of 38) ───────────────────────────── ⌥G next ── + ``` + +3. **Reveal-scrolls it**, with the measured tail height. A buffer index is not a viewport row: the + panel's offset counts *display* rows and a buffered line wraps into as many as it needs, which is + the defect `RevealAwayBar` was first written with and the reason the height is measured through + the framework's own `MarkupControl.MeasureDOM` at the pane's real width. + +At most one search bar exists at a time, client-wide — `⌥G` moves it. It is removed by the next +search, by the next `⌃F`, and by a trim that takes it. + +**Not by `Esc`**, tempting as that is. An Escape that is claimed does not set `_escapeAt`, and +`TryAltEnter` pairs an unclaimed Escape with an Enter arriving within 50 ms to reassemble `Alt+⏎` — +the newline chord. Binding Escape to "clear the search bar" would break inserting a newline for as +long as a search bar was on screen, which is a defect nobody would connect to the search feature. + +## Part 3: the activity boundary + +### One boundary, recorded where it actually happens + +Today the away boundary is reconstructed *backwards*, from the last input before the reader +disappeared, because focus-out is unrecoverable: `ESC [ O` has no case in `DispatchCsi` and is +dropped as an `UnknownSequenceEvent`, so a departure cannot be timestamped. That machinery stays +exactly as it is for the terminal case, because nothing better is available. + +The window case needs none of it, because it is **observable at the moment it happens**. A line is +appended; the window either is or is not caught up; if it is not, and no boundary is pending, the +boundary is the buffer count *before* this line. That is exact rather than approximate, and it is one +comparison in a method that already runs per line. + +``` +Watched(window) ≡ terminal has focus ∧ Workspace.IsCaughtUp(window) +``` + +`IsCaughtUp` and not `IsVisible`, which is already the rule the unread badge uses: a visible tab whose +output you have scrolled back off is exactly as blind as a tab you are not looking at. So the badge +and the bar answer to one fact, and a badge showing `47` always has a bar under it explaining where +the 47 begin. + +When a window becomes watched again and something is pending, the bar materialises, is revealed, and +the pending boundary clears. Where both a window boundary and a terminal-away boundary exist for one +window, the **older** wins: it is still one bar per window, and it marks the earlier of the two things +you missed. + +The bar's wording distinguishes the two absences, because they are different facts: + +``` +▾ AWAY 37 lines · 12 min ────────────── (terminal absence, today's wording) +▾ NEW 47 lines since you were here ─── (window absence) +``` + +### Lifetime + +Three conjuncts, the first two unchanged: + +1. The pane is back at its **live tail** (`ScrollablePanelControl.AutoScroll`, the framework's own + "showing the newest line" bit). This means something only because the reveal took the pane *off* + its tail whenever the bar was not on screen, so arriving at the bottom is having read down through + what you missed. +2. **One input** has landed since the bar was drawn, numbered rather than timestamped + (`AwayMark.DrawnAfter` against `InputCount`), because marshalling to the UI thread reorders and a + note raised before the frame that drew a bar can be delivered after it. +3. **New:** the bar has existed for at least the dwell floor. + +The floor is a wall-clock span held on the mark at draw time and compared in `ConsumeReadAwayBars`. +Default **30 seconds**, settable on F7 (Text & ANSI, where the other output-rendering settings live) +as `activity bar holds for`. `0` is today's behaviour exactly, so the change is reversible by the +person who disagrees with it. + +New defaulted field in the configuration, so **no schema bump and no migration** — the reasoning +`ConnectAtStartup` and `PaneTint.None` already establish: a default that describes the state most +clients are in does not need anybody marked. + +The clock is **injected**, a `Func` defaulting to `DateTimeOffset.Now`, joining the +`save:`/`logRoot:`/`restore:`/launcher family of caller-supplied seams. A dwell floor tested against +the wall clock is a test that fails on a loaded CI box, and a snapshot that waited 30 seconds would +be a snapshot nobody runs. + +## Part 4: the shared machinery + +By the end of this there are two kinds of client chrome inserted mid-buffer — the activity bar and +the search bar — and every index into that buffer has to survive both: the freeze point, the pending +boundary, the other bar, and the trim that reclaims the buffer's cap. That bookkeeping exists once +today, spread across `RemoveAwayBar`, `TrimWindow` and `MarkWhereTheReaderLeft`, and would exist +twice by the end. + +It is extracted into one `PaneMarks` type — insert, remove, and "fix up every index past N" — as part +of whichever PR lands first. Deliberately not a wider refactor: the freeze point stays where it is, +and nothing else moves. + +## Verification + +- **Core:** `OutputSearchTests` — plain and regex, case-insensitivity in both, `(?-i)`, invalid + patterns, a pathological pattern against the timeout, and the offsets a row marks itself with. +- **Tui, pure:** `SearchPromptTests` — every key the footer names, and no other key doing anything. +- **Tui, integration:** the chord moves (`⌥F` freezes, `⌃F` opens the surface, `⌥↑` recalls from a + grown bar's middle row, `⌥↑` declines to a macro), landing (`⏎` activates the right *window*, not + the focused one), and the boundary (a background tab accrues, a visible tab at its tail does not, a + bar survives the dwell floor and retires after it, against an injected clock). +- **Honesty pins:** `AdvertisedKeyHonestyTests` covers the new chords; `MacroKeys.Bindable` must not + hand out anything the app has just claimed. +- **Snapshots:** `search`, `search-regex`, `search-all`, `search-landed` (the bar in a pane, in a + split so the pane is narrower than the terminal — the geometry that catches a reveal landing at the + wrong row), and `activity-bar`. The existing `freeze` views' bar text changes to `⌥F`, which + `FreezeBarRendererTests` already pins. +- Primary signal as always: `dotnet build SharpMUTerm.slnx` plus all five suites green and + warning-free. + +## The stack + +Three PRs, each branched off the one before it, each independently reviewable and each green on its +own: + +1. **`feat/find-chords`** — `⌃F` → `⌥F` for freeze, `⌥↑`/`⌥↓` for history recall, and this document. + Frees `⌃F`. Small, and touches nothing the other two need to agree with. +2. **`feat/window-activity-boundary`** — the boundary generalisation, the dwell floor and its F7 + setting. Owns the extraction of `PaneMarks`, so the machinery is already general when the search + bar arrives. +3. **`feat/pane-search`** — `OutputSearch`, the surface, the landing bar, `⌥G`. + +The order is chosen so that each PR's hardest part is already built: `⌃F` is free before search wants +it, and the mid-buffer chrome bookkeeping is one type before there are two kinds of it. From 1e0565a24aa36fa5d0a61f53baff201df3f46990 Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Tue, 11 Aug 2026 14:51:17 -0500 Subject: [PATCH 02/19] =?UTF-8?q?docs(plan):=20PR=201=20of=20the=20stack?= =?UTF-8?q?=20=E2=80=94=20=E2=8C=A5F=20freeze=20and=20the=20=E2=8C=A5?= =?UTF-8?q?=E2=86=91/=E2=8C=A5=E2=86=93=20history=20chord?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN --- .../plans/2026-08-11-find-chords.md | 586 ++++++++++++++++++ 1 file changed, 586 insertions(+) create mode 100644 docs/superpowers/plans/2026-08-11-find-chords.md diff --git a/docs/superpowers/plans/2026-08-11-find-chords.md b/docs/superpowers/plans/2026-08-11-find-chords.md new file mode 100644 index 0000000..080442d --- /dev/null +++ b/docs/superpowers/plans/2026-08-11-find-chords.md @@ -0,0 +1,586 @@ +# PR 1 — Find chords: `⌥F` freeze, `⌥↑`/`⌥↓` history Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan +> task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Free `⌃F` for search by moving freeze to `⌥F`, and give command history a dedicated +`⌥↑`/`⌥↓` that works wherever the caret is. + +**Architecture:** Two independent keyboard changes in `SharpMUTerm.Tui`. Freeze moves by editing the +one claim list the app registers from (`MacroKeys.AppShortcuts`) and the matching arm of +`ShortcutAction`; everything that *prints* the chord moves with it. History recall gains an +Alt-modified path in `TryRecallKey` beside the bare-arrow path, which is untouched. + +**Tech Stack:** C# / .NET 10, SharpConsoleUI 2.5.14 (package only), TUnit on +Microsoft.Testing.Platform. + +**Spec:** `docs/superpowers/specs/2026-08-11-pane-search-and-activity-design.md`, parts 1 and 2. + +**Branch:** `feat/find-chords`, off `main`. First of a three-PR stack. + +## Global Constraints + +- Target framework `net10.0`. File-scoped namespaces, 4-space C#, LF endings (`.editorconfig`). +- `SharpMUTerm.Core` stays UI-agnostic; nothing in this PR touches it. +- Tests are TUnit `Exe` projects. `dotnet test` does **not** work. Run: + `dotnet run -c Release --project tests/SharpMUTerm.Tui.Tests +/// Freeze answers to ⌥F and not to ⌃F, which search takes in the PR after this one. Both halves are +/// the claim: a chord that moved in the handler and not in the bar would leave the client telling a +/// frozen reader to press a key that no longer thaws it. +/// +/// Serialised: constructing the app touches the process-global console streams. +[NotInParallel] +public class FreezeChordTests +{ + private const string Main = "main"; + + private static readonly TerminalCapabilities Headless = + new(GraphicsProtocol.None, supportsTrueColor: true, supportsKittyGraphics: false, supportsSixel: false); + + private static SharpMUTermApp App() + { + Console.SetIn(TextReader.Null); + var app = new SharpMUTermApp(DemoScene.Build(), Headless, new HeadlessConsoleDriver(120, 34)); + app.RenderSnapshot("default"); + return app; + } + + private static ConsoleKeyInfo Chord(ConsoleKey key, bool alt = false, bool control = false) => + new('\0', key, shift: false, alt: alt, control: control); + + [Test] + public async Task AltFFreezesTheFocusedPaneAndPressingItAgainResumes() + { + var app = App(); + await Assert.That(app.FrozenScrollbackOf(Main)).IsNull(); + + app.SimulateKey(Chord(ConsoleKey.F, alt: true)); + await Assert.That(app.FrozenScrollbackOf(Main)).IsNotNull(); + + app.SimulateKey(Chord(ConsoleKey.F, alt: true)); + await Assert.That(app.FrozenScrollbackOf(Main)).IsNull(); + } + + [Test] + public async Task CtrlFNoLongerFreezesAnything() + { + var app = App(); + + app.SimulateKey(Chord(ConsoleKey.F, control: true)); + + await Assert.That(app.FrozenScrollbackOf(Main)).IsNull(); + } + + [Test] + public async Task TheClaimListNamesAltFAndNoLongerNamesCtrlF() + { + var claims = MacroKeys.AppShortcuts; + + await Assert.That(claims.Any(c => c.Modifiers == ConsoleModifiers.Alt && c.Key == ConsoleKey.F)).IsTrue(); + await Assert.That(claims.Any(c => c.Modifiers == ConsoleModifiers.Control && c.Key == ConsoleKey.F)).IsFalse(); + } +} +``` + +- [ ] **Step 2: Run it and watch it fail** + +```bash +cd /home/grave/RiderProjects/SharpMUTerm-find +dotnet run -c Release --project tests/SharpMUTerm.Tui.Tests --treenode-filter "/*/*/FreezeChordTests/*" () => { ToggleFreeze(); return true; }, +``` + +and in the same method's `if (claim.Modifiers == ConsoleModifiers.Alt)` block, after the `⌥K` arm and +before the `WindowJumpNumber` check, add: + +```csharp + // ⌥F freezes and resumes the focused pane. Same delivery story as ⌥D and ⌥R: ESC + a + // printable byte, decoded as that letter with Alt set. + if (claim.Key == ConsoleKey.F) + { + return () => { ToggleFreeze(); return true; }; + } +``` + +- [ ] **Step 5: Move the label the frozen reader is looking at** + +In `src/SharpMUTerm.Tui/FreezeBarRenderer.cs`, line 18, change `FROZEN ⌃F` to `FROZEN ⌥F`, and the +comment on line 17 with it. In `tests/SharpMUTerm.Tui.Tests/FreezeBarRendererTests.cs`, line 13, change +the expected string to `$"[#c678dd]{Glyphs.Freeze} FROZEN ⌥F[/]"` and the comment on line 12 with it. + +- [ ] **Step 6: Run the whole Tui suite** + +```bash +dotnet run -c Release --project tests/SharpMUTerm.Tui.Tests +Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN" +``` + +--- + +### Task 2: `⌥↑` / `⌥↓` recall history unconditionally + +**Files:** +- Modify: `src/SharpMUTerm.Tui/SharpMUTermApp.cs` (`TryRecallKey`, ~3524) +- Create: `tests/SharpMUTerm.Tui.Tests/HistoryChordTests.cs` +- Modify: `docs/design/README.md` (the chord list), `CLAUDE.md` + +**Interfaces:** +- Consumes: `SharpMUTermApp.ActiveBar()` → `InputBarControl`; `BarKind(InputBarControl)` → `InputBar`; + `HistoryFor(InputBar)` → `InputHistory` with `Recall(string)`, `Forward()`, `IsRecalling`; + `InputBarControl.TryMoveRow(int)` → `bool`; `_drafts.Record(string windowId, InputBar kind, string text)`. + All private/internal members of `SharpMUTermApp` already used by the bare-arrow path. +- Produces: nothing later tasks depend on. + +- [ ] **Step 1: Write the failing test** + +Create `tests/SharpMUTerm.Tui.Tests/HistoryChordTests.cs`: + +```csharp +using SharpMUTerm.Graphics; +using SharpMUTerm.Tui; + +namespace SharpMUTerm.Tui.Tests; + +/// +/// ⌥↑/⌥↓ mean history and nothing else. The bare arrows still recall at the edges — nothing is taken +/// away — but they answer to the caret first, which stops being a usable rule the moment the command +/// line grows to a second row. That was the reported complaint. +/// +/// ⌃↑/⌃↓, the other chord the request offered, was never available: the terminal writes `ESC [ 1;5 A` +/// for it and this client already spends that on pane selection and the ladder onto the second command +/// line. `ESC [ 1;3 A` — Alt — is free, and was measured at a raw reader before it was spent. +/// +/// +/// Serialised: constructing the app touches the process-global console streams. +[NotInParallel] +public class HistoryChordTests +{ + private static readonly TerminalCapabilities Headless = + new(GraphicsProtocol.None, supportsTrueColor: true, supportsKittyGraphics: false, supportsSixel: false); + + private static SharpMUTermApp App() + { + Console.SetIn(TextReader.Null); + var app = new SharpMUTermApp(DemoScene.Build(), Headless, new HeadlessConsoleDriver(120, 34)); + app.RenderSnapshot("default"); + return app; + } + + private static ConsoleKeyInfo Chord(ConsoleKey key, bool alt = false) => + new('\0', key, shift: false, alt: alt, control: false); + + /// One printable character, as the command line's own tests spell it. + private static ConsoleKeyInfo Key(char c) => new(c, ConsoleKey.NoName, false, false, false); + + private static void Type(SharpMUTermApp app, string text) + { + foreach (var c in text) + { + app.SimulateKey(Key(c)); + } + } + + private static void Send(SharpMUTermApp app, string text) + { + Type(app, text); + app.SimulateKey(new ConsoleKeyInfo('\r', ConsoleKey.Enter, false, false, false)); + } + + /// Sends two lines so the history has something in it, and leaves the bar empty. + private static void Seed(SharpMUTermApp app) + { + Send(app, "look"); + Send(app, "say hello"); + } + + [Test] + public async Task AltUpRecallsTheNewestLineAndAltDownWalksBack() + { + var app = App(); + Seed(app); + + app.SimulateKey(Chord(ConsoleKey.UpArrow, alt: true)); + await Assert.That(app.ArmedInputText).IsEqualTo("say hello"); + + app.SimulateKey(Chord(ConsoleKey.UpArrow, alt: true)); + await Assert.That(app.ArmedInputText).IsEqualTo("look"); + + app.SimulateKey(Chord(ConsoleKey.DownArrow, alt: true)); + await Assert.That(app.ArmedInputText).IsEqualTo("say hello"); + } + + /// + /// The point of the chord. With a draft tall enough to have a row above the caret, the bare ↑ is the + /// caret's and ⌥↑ is history's — on the same keystroke, from the same position. + /// + [Test] + public async Task OnAGrownBarTheBareArrowMovesTheCaretAndTheAltArrowRecalls() + { + var app = App(); + Seed(app); + var draft = new string('x', 400); // wraps to several rows at 120 columns + Type(app, draft); + + app.SimulateKey(Chord(ConsoleKey.UpArrow)); + await Assert.That(app.ArmedInputText).IsEqualTo(draft); + + app.SimulateKey(Chord(ConsoleKey.UpArrow, alt: true)); + await Assert.That(app.ArmedInputText).IsEqualTo("say hello"); + } + + /// + /// ⌥↓ with nothing recalled is not ours: it must not clear the bar, and it must not be claimed on + /// the way past. Exactly the bare-arrow rule, which returns false rather than blanking a draft. + /// + [Test] + public async Task AltDownWithNothingRecalledLeavesTheDraftAlone() + { + var app = App(); + Seed(app); + Type(app, "half a thought"); + + app.SimulateKey(Chord(ConsoleKey.DownArrow, alt: true)); + + await Assert.That(app.ArmedInputText).IsEqualTo("half a thought"); + } +} +``` + +`ArmedInputText` (`SharpMUTermApp.cs:7754`) reads whichever bar is armed; `Type`/`Send` are lifted from +`InputAreaEndToEndTests` so the two suites drive the command line the same way. There is no setter for +the bar's text on purpose — assigning `Text` raises no change event, so a test that set it would skip +the draft recording the real path does. + +- [ ] **Step 2: Run it and watch it fail** + +```bash +dotnet run -c Release --project tests/SharpMUTerm.Tui.Tests --treenode-filter "/*/*/HistoryChordTests/*" RecallMode.AtTheEdges, + ConsoleModifiers.Alt => RecallMode.Always, + _ => RecallMode.No, + }; + + if (history == RecallMode.No) + { + return false; + } +``` + +and change the two `TryMoveRow` guards so they only apply to the bare arrows: + +```csharp + case ConsoleKey.UpArrow: + if (history == RecallMode.AtTheEdges && bar.TryMoveRow(-1)) + { + e.Handled = true; + return true; + } + + text = entries.Recall(bar.Text); + break; +``` + +Rename the existing local `history` (the `InputHistory` from `HistoryFor(kind)`) to `entries`, so the +mode local above can take the name `history`; both uses in the `DownArrow` arm move with it. The `DownArrow` arm changes the same way: + +```csharp + case ConsoleKey.DownArrow: + if (history == RecallMode.AtTheEdges && bar.TryMoveRow(1)) + { + e.Handled = true; + return true; + } + + if (!entries.IsRecalling) + { + return false; + } + + text = entries.Forward(); + break; +``` + +Add the enum beside the method: + +```csharp + /// Which of the two ways into history a keystroke is: the bare arrows, or ⌥↑/⌥↓. + private enum RecallMode + { + /// Not a recall key at all. + No, + + /// A bare arrow: the caret gets it first, and history only at the edges. + AtTheEdges, + + /// ⌥↑ or ⌥↓: history, wherever the caret is. + Always, + } +``` + +- [ ] **Step 4: Run the tests** + +```bash +dotnet run -c Release --project tests/SharpMUTerm.Tui.Tests --treenode-filter "/*/*/HistoryChordTests/*" +Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN" +``` + +--- + +### Task 3: Open the PR + +- [ ] **Step 1: Push and open** + +```bash +git push -u origin feat/find-chords +gh pr create --base main --title "feat(keys): ⌥F freezes, ⌥↑/⌥↓ recall, and ⌃F is free" --body "$(cat <<'BODY' +First of a three-PR stack (`feat/find-chords` → `feat/window-activity-boundary` → `feat/pane-search`). +Design: `docs/superpowers/specs/2026-08-11-pane-search-and-activity-design.md`. + +**Freeze moves to `⌥F`.** `⌃F` means *find* to everyone who has used a computer, and the search +surface in PR 3 takes it. Freeze keeps its letter and changes its modifier — the smallest move that +frees the chord. No `⌃F` alias is left behind (the `⌃D` rule). The `❄ FROZEN ⌥F` label a frozen +reader is actually looking at moves with it, as do the `⌃P` entry and the docs. + +**History gets its own chord, `⌥↑`/`⌥↓`.** The bare arrows still recall at the edges and are +unchanged; the Alt pair skips the caret test, so it works on a draft tall enough to have another row. +That was the report. + +**Every chord was measured** at a raw-mode reader with `kitten @ send-key` before it was spent: +`⌥F` is `ESC f`, `⌥↑`/`⌥↓` are `ESC [ 1;3 A`/`B`. `⌃↑`/`⌃↓` — the alternative the request offered — +arrives as `ESC [ 1;5 A` and was already spent on pane selection. + +🤖 Generated with [Claude Code](https://claude.com/claude-code) + +https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN +BODY +)" +``` + +- [ ] **Step 2: Branch the next PR off this one** + +```bash +git checkout -b feat/window-activity-boundary +``` + +The stack continues in `docs/superpowers/plans/2026-08-11-window-activity-boundary.md`, written after +this PR lands its first review. + +## Self-review notes + +- **Spec coverage:** this plan covers spec parts "The chords, and why these ones" and "`⌃F` becomes + search; freeze becomes `⌥F`" and "`⌥↑`/`⌥↓` recall history, unconditionally". `⌥G`/`⌥⇧G` is + deliberately *not* here — it repeats a search that does not exist yet, and belongs to PR 3. +- **Intermediate state:** between this PR and PR 3, `⌃F` is claimed by nobody and `MacroKeys.Verdict` + says a macro on it fires. That is true while it lasts and needs no stub. +- **Not in scope:** the `⌃P` command-surface entries for freeze carry no chord hint + (`CommandCatalog.cs:175-176` passes only a label and an id), so nothing there needs to change. From 61a9224888b5e39ae45e7c32f4a1817060294807 Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Tue, 11 Aug 2026 14:55:14 -0500 Subject: [PATCH 03/19] =?UTF-8?q?feat(keys):=20freeze=20moves=20to=20?= =?UTF-8?q?=E2=8C=A5F,=20freeing=20=E2=8C=83F=20for=20find?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ⌃F means find to everyone who has used a computer, and the search surface in the third PR of this stack takes it. Freeze keeps its letter and changes its modifier — ⌥F is `ESC f`, measured at a raw reader with `kitten @ send-key`, the same way ⌥D and ⌥R were. No ⌃F alias is left behind: the ⌃D rule, that a second key for one action is either a secret or a duplicate row on every surface listing chords. The label a *frozen* reader is looking at moves with the chord — a bar naming a key that no longer thaws the pane would be the worst place to leave a stale one. The chord test reads the frame rather than FrozenScrollbackOf: that viewport is created on first freeze and kept for the life of the window, so it answers non-null for ever after the first ⌥F and cannot say whether a pane is frozen now. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN --- CLAUDE.md | 9 ++- docs/design/README.md | 6 +- src/SharpMUTerm.Tui/FreezeBarRenderer.cs | 6 +- src/SharpMUTerm.Tui/MacroKeys.cs | 10 ++- src/SharpMUTerm.Tui/SharpMUTermApp.cs | 25 ++++--- .../FreezeBarRendererTests.cs | 4 +- .../SharpMUTerm.Tui.Tests/FreezeChordTests.cs | 70 +++++++++++++++++++ .../OutputScrollbackTests.cs | 2 +- .../TimestampGutterTests.cs | 2 +- 9 files changed, 113 insertions(+), 21 deletions(-) create mode 100644 tests/SharpMUTerm.Tui.Tests/FreezeChordTests.cs diff --git a/CLAUDE.md b/CLAUDE.md index 52ad05b..778c9f4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -544,8 +544,15 @@ markup (`[bold #rrggbb on #rrggbb]…[/]`, `[[`/`]]` escaping, `[link=url]…[/] - **Deliberately left on Ctrl**, because the convention is worth more than the pattern: `⌃R` (readline's reverse history search), `⌃P` (command surface), `⌃Q` (quit — and safe here because `TerminalRawMode` clears `IXON`, so it is not XON), `⌃B` (tmux's prefix), `⌃O` (pane cycle), - `⌃N`/`⌃W`/`⌃F`, and the command line's `⌃A`/`⌃E`/`⌃K`/`⌃U`/`⌃L`. A sweep that moved everything + `⌃N`/`⌃W`, and the command line's `⌃A`/`⌃E`/`⌃K`/`⌃U`/`⌃L`. A sweep that moved everything would be as wrong as one that moved nothing. + - **Freeze is `⌥F`, and `⌃F` is find.** Freeze was on `⌃F` and left it for exactly the reason the + keys above stay where they are: `⌃F` means *find* to everyone who has used a computer, and that + convention outweighs freeze's claim on the chord. Freeze kept its letter and changed its modifier — + the smallest move that frees it; `⌥F` is `ESC f`, measured. Nothing is left behind on `⌃F` as an + alias (the `⌃D` rule). The label a **frozen** reader is looking at (`FreezeBarRenderer`) moves with + the chord: a bar naming a key that no longer thaws the pane would be the worst possible place to + leave a stale chord. - **Known and not fixed here**: `⌃N` and `⌃O` have no reverse (the character cycle does — `⌥J`/`⌥K`), and `⌃W` and `⌃B x` are two chords for one action. Both are shape complaints rather than defects, and both are behaviour changes rather than modifier moves. diff --git a/docs/design/README.md b/docs/design/README.md index 80998c7..97a4f47 100644 --- a/docs/design/README.md +++ b/docs/design/README.md @@ -251,8 +251,8 @@ like any other output window. Which rule feeds a pane is F2's answer, not the pa Output view: timestamp column (optional), then styled spans. Trigger-highlighted lines get a 2-col left rule in the trigger's colour plus a tinted background. -Freezing (⌃F) splits the pane horizontally: frozen scrollback above under a -`▲ FROZEN ⌃F` bar, live tail below. +Freezing (⌥F) splits the pane horizontally: frozen scrollback above under a +`▲ FROZEN ⌥F` bar, live tail below. **Input** (grows, min 3 rows): prompt reads `Corvid@aetherfall ›` — bound to the focused **character**, not the focused pane. Right gutter shows the destination window (`→ main`), @@ -423,7 +423,7 @@ count on the tab, the rail character, and the rail world. ### Other keys -`⌃P` command surface · `⌃F` freeze/resume in focused pane · `⌃L` toggle logging · +`⌃P` command surface · `⌥F` freeze/resume in focused pane · `⌃L` toggle logging · `⌃Tab` next tab in pane · `⌃R` reconnect · `↑`/`↓` history · `F2`–`F9` config · `Esc` close overlay · `Shift+Enter` newline in input. diff --git a/src/SharpMUTerm.Tui/FreezeBarRenderer.cs b/src/SharpMUTerm.Tui/FreezeBarRenderer.cs index 735e904..a7d6149 100644 --- a/src/SharpMUTerm.Tui/FreezeBarRenderer.cs +++ b/src/SharpMUTerm.Tui/FreezeBarRenderer.cs @@ -1,7 +1,7 @@ namespace SharpMUTerm.Tui; /// -/// Renders the freeze split bar (▲ FROZEN ⌃F) that divides a frozen pane's pinned scrollback +/// Renders the freeze split bar (▲ FROZEN ⌥F) that divides a frozen pane's pinned scrollback /// (above) from its live tail (below), per the design's freeze interaction. The label takes the /// frozen-chrome accent (design token #c678dd / ANSI 5, resolved through the theme); the hint /// is dim. Pure so the markup is unit-testable without a terminal. @@ -14,7 +14,7 @@ public static string Bar(string accentHex) ArgumentException.ThrowIfNullOrEmpty(accentHex); // A single line that is both the label and the divider border between pinned scrollback and the - // live tail: the accented "❄ FROZEN ⌃F" label followed by a dim rule. - return $"[{accentHex}]{Glyphs.Freeze} FROZEN ⌃F[/] [dim]{new string('─', 48)}[/]"; + // live tail: the accented "❄ FROZEN ⌥F" label followed by a dim rule. + return $"[{accentHex}]{Glyphs.Freeze} FROZEN ⌥F[/] [dim]{new string('─', 48)}[/]"; } } diff --git a/src/SharpMUTerm.Tui/MacroKeys.cs b/src/SharpMUTerm.Tui/MacroKeys.cs index 0e04abd..8d5e3e7 100644 --- a/src/SharpMUTerm.Tui/MacroKeys.cs +++ b/src/SharpMUTerm.Tui/MacroKeys.cs @@ -116,7 +116,6 @@ private static AppShortcut[] BuildAppShortcuts() new(ConsoleModifiers.Control, ConsoleKey.O, "cycles the panes"), new(ConsoleModifiers.Control, ConsoleKey.P, "opens the command surface"), new(ConsoleModifiers.Control, ConsoleKey.B, "arms the pane prefix"), - new(ConsoleModifiers.Control, ConsoleKey.F, "freezes the pane"), new(ConsoleModifiers.Control, ConsoleKey.R, "searches the command history"), // The connection pair, and it is a pair: ⌥D disconnects, ⌥R reconnects. One modifier, two // letters that spell the two words, opposite actions that look opposite on the keyboard. @@ -138,6 +137,15 @@ private static AppShortcut[] BuildAppShortcuts() // Both act at once; ⌃Q is the only key in this client that asks anything. new(ConsoleModifiers.Alt, ConsoleKey.D, "disconnects the focused character"), new(ConsoleModifiers.Alt, ConsoleKey.R, "reconnects the focused character"), + // Freeze is ⌥F and not ⌃F, and it moved rather than gaining a second spelling. ⌃F means *find* + // to everyone who has used a computer, and the search surface takes it; freeze keeps its letter + // and changes its modifier, which is the smallest move that frees the chord. ⌥F is delivered as + // `ESC f` — measured at a raw reader with `kitten @ send-key`, the same way ⌥D and ⌥R were — and + // is claimed by nothing else here or in the framework. + // + // No ⌃F alias is left behind, for the reason ⌃D was released rather than kept: a second key for + // one action is either a secret or a duplicate row on every surface that lists chords. + new(ConsoleModifiers.Alt, ConsoleKey.F, "freezes the pane"), // The character cycle. Letters and not digits because the digit row is spent (⌥N windows, ⌃B N // panes) and there is no third digit-bearing modifier this terminal delivers: read off a pty, // kitty writes ⌥⇧1 as `CSI 49;4u` and ⌃⇧N as `CSI 110;6u` — kitty-keyboard-protocol sequences diff --git a/src/SharpMUTerm.Tui/SharpMUTermApp.cs b/src/SharpMUTerm.Tui/SharpMUTermApp.cs index 4d09432..4fa35ea 100644 --- a/src/SharpMUTerm.Tui/SharpMUTermApp.cs +++ b/src/SharpMUTerm.Tui/SharpMUTermApp.cs @@ -2321,7 +2321,7 @@ private string Compose(PaneLine line) => /// /// Appends one already-formatted markup line to a window: records it in the scrollback buffer and, /// if the window has a live control, paints it. A frozen pane's live control is its tail region, so - /// new lines land below the ▲ FROZEN ⌃F bar while the pinned scrollback stays put. + /// new lines land below the ▲ FROZEN ⌥F bar while the pinned scrollback stays put. /// /// is when the line arrived, and defaults to none: only a world's output /// passes one, because only a world's output is what the timestamp column describes. @@ -4097,7 +4097,7 @@ private void RefuseFocusMove(PaneDirection direction) /// The viewport the scrollback keys drive: the focused pane's active window. /// /// A frozen pane hands them its pinned half. Freeze and scrollback are two ways of looking - /// at the same history and this is where they compose: ⌃F holds a region still above the bar and + /// at the same history and this is where they compose: ⌥F holds a region still above the bar and /// keeps the tail live below it, and while that is up the region worth moving through is the pinned /// one — the live tail is by definition already showing its newest line. The tail keeps its own /// viewport regardless (see ), so a burst into a four-row tail still @@ -4690,6 +4690,14 @@ private void RegisterFocusReportTab() return () => { CycleCharacter(-1); return true; }; } + // ⌥F freezes and resumes the focused pane. It was ⌥F, and moved so that search could have the + // chord every reader on every platform reaches for. Same delivery story as ⌥D and ⌥R: ESC + a + // printable byte, decoded as that letter with Alt set. + if (claim.Key == ConsoleKey.F) + { + return () => { ToggleFreeze(); return true; }; + } + // ⌥1–⌥9 go to the numbered window. Same delivery story as Alt+R and one digit over: the // terminal writes ESC + the digit and the parser reads it as that digit with Alt set. if (MacroKeys.WindowJumpNumber(claim.Key) is { } number) @@ -4719,7 +4727,6 @@ private void RegisterFocusReportTab() ConsoleKey.O => () => { CyclePane(); return true; }, ConsoleKey.P => () => { ToggleMenu(); return true; }, ConsoleKey.B => () => { ArmPrefix(); return true; }, - ConsoleKey.F => () => { ToggleFreeze(); return true; }, // ⌃R is the readline/bash/zsh/fish reverse-history-search chord, which is why the surface is // on it. ⌃H — what a user reaching for "history" tries first — cannot be bound at all: the // framework's parser turns byte 0x08 into Backspace with no Control modifier, so binding it @@ -6272,7 +6279,7 @@ private void StopLogging() } /// - /// Freezes or resumes the focused pane (⌃F). Freezing records the active window's current scrollback + /// Freezes or resumes the focused pane (⌥F). Freezing records the active window's current scrollback /// length as the split point (pinned scrollback above, live tail below); resuming clears it and /// re-flows the whole buffer back into the single output control. /// @@ -7059,7 +7066,7 @@ private IWindowControl Divider() /// /// The pane's tab strip (or its move/drag stand-in). /// - /// Whether this is Layout.FocusedPane — the pane the scrollback keys, the ⌃B commands, ⌃F and + /// Whether this is Layout.FocusedPane — the pane the scrollback keys, the ⌃B commands, ⌥F and /// the Ctrl+arrows all act on. Nothing rendered it before, so the one pane every keystroke was aimed /// at looked exactly like the ones it was not; it is lit with , /// the same tone as the armed command line. @@ -7426,19 +7433,19 @@ private IWindowControl BuildTabContent(PaneNode pane, string windowId, Workspace /// /// Builds a frozen window's content: a vertical split of pinned scrollback (buffer up to the freeze - /// point), the ▲ FROZEN ⌃F bar, and the live tail (buffer since the freeze). The tail is the + /// point), the ▲ FROZEN ⌥F bar, and the live tail (buffer since the freeze). The tail is the /// window's real control, so incoming lines keep landing below the bar while the top stays pinned. /// /// Both halves get their own scroll viewport. The pinned half is the one a reader most wants /// to move through — it is the history freeze exists to hold still — and before this it could show - /// only the oldest screenful of it, which made ⌃F a way of pinning text you could not read. The + /// only the oldest screenful of it, which made ⌥F a way of pinning text you could not read. The /// live tail gets one too, for the same reason every other pane does: it is a tail, and a burst of /// output past its few rows would otherwise vanish under the bar. /// /// /// Both are ordinary viewports, with no /// freeze-specific rule. On the pinned half "the bottom" is the freeze point — the last line that was - /// on screen when ⌃F was pressed — so auto-scroll opens it exactly where the reader left off and + /// on screen when ⌥F was pressed — so auto-scroll opens it exactly where the reader left off and /// detaches the moment they scroll up, which is the behaviour a special case would have had to /// reproduce. It also keeps the half honest when the scrollback cap trims the buffer and /// walks the freeze point down: the pinned region shrinks and the @@ -7458,7 +7465,7 @@ private IWindowControl BuildFrozenContent(string windowId, string title) var live = PaneContentFor(windowId, title); FeedRange(live, buffer, split, buffer.Count - split); - // Pinned scrollback gets the lion's share; a single "❄ FROZEN ⌃F ───" line is both label and + // Pinned scrollback gets the lion's share; a single "❄ FROZEN ⌥F ───" line is both label and // border, with the live tail a few rows below it. var grid = Controls.Grid() .WithAlignment(HorizontalAlignment.Stretch) diff --git a/tests/SharpMUTerm.Tui.Tests/FreezeBarRendererTests.cs b/tests/SharpMUTerm.Tui.Tests/FreezeBarRendererTests.cs index 30b81bc..4a99a49 100644 --- a/tests/SharpMUTerm.Tui.Tests/FreezeBarRendererTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/FreezeBarRendererTests.cs @@ -9,8 +9,8 @@ public async Task Bar_CarriesFrozenLabelInTheAccentAndADimHint() { var bar = FreezeBarRenderer.Bar("#c678dd"); - // A single line: accented "❄ FROZEN ⌃F" label, then a dim rule serving as the border. - await Assert.That(bar).Contains($"[#c678dd]{Glyphs.Freeze} FROZEN ⌃F[/]"); + // A single line: accented "❄ FROZEN ⌥F" label, then a dim rule serving as the border. + await Assert.That(bar).Contains($"[#c678dd]{Glyphs.Freeze} FROZEN ⌥F[/]"); await Assert.That(bar).Contains("[dim]"); await Assert.That(bar).Contains("─"); } diff --git a/tests/SharpMUTerm.Tui.Tests/FreezeChordTests.cs b/tests/SharpMUTerm.Tui.Tests/FreezeChordTests.cs new file mode 100644 index 0000000..baf2ce5 --- /dev/null +++ b/tests/SharpMUTerm.Tui.Tests/FreezeChordTests.cs @@ -0,0 +1,70 @@ +using SharpConsoleUI.Drivers; +using SharpMUTerm.Graphics; +using SharpMUTerm.Tui; + +namespace SharpMUTerm.Tui.Tests; + +/// +/// Freeze answers to ⌥F and not to ⌃F, which search takes in the PR after this one. Both halves are +/// the claim: a chord that moved in the handler and not in the bar would leave the client telling a +/// frozen reader to press a key that no longer thaws it. +/// +/// Serialised: constructing the app touches the process-global console streams. +[NotInParallel] +public class FreezeChordTests +{ + private const string Main = "main"; + + private static readonly TerminalCapabilities Headless = + new(GraphicsProtocol.None, supportsTrueColor: true, supportsKittyGraphics: false, supportsSixel: false); + + private static SharpMUTermApp App() + { + Console.SetIn(TextReader.Null); + var app = new SharpMUTermApp(DemoScene.Build(), Headless, new HeadlessConsoleDriver(120, 34)); + app.RenderSnapshot(); + return app; + } + + private static ConsoleKeyInfo Chord(ConsoleKey key, bool alt = false, bool control = false) => + new('\0', key, shift: false, alt: alt, control: control); + + /// + /// Read off the frame rather than off FrozenScrollbackOf: the pinned-scrollback viewport is + /// created on first freeze and kept for the life of the window, so it is a poor oracle for + /// "is this pane frozen right now" and answers non-null for ever after the first ⌥F. The bar in the + /// paint is the thing the reader is actually looking at. + /// + [Test] + public async Task AltFFreezesTheFocusedPaneAndPressingItAgainResumes() + { + var app = App(); + await Assert.That(app.RenderSnapshot()).DoesNotContain("FROZEN"); + + app.SimulateKey(Chord(ConsoleKey.F, alt: true)); + await Assert.That(app.RenderSnapshot()).Contains("FROZEN ⌥F"); + + app.SimulateKey(Chord(ConsoleKey.F, alt: true)); + await Assert.That(app.RenderSnapshot()).DoesNotContain("FROZEN"); + } + + [Test] + public async Task CtrlFNoLongerFreezesAnything() + { + var app = App(); + + app.SimulateKey(Chord(ConsoleKey.F, control: true)); + + await Assert.That(app.RenderSnapshot()).DoesNotContain("FROZEN"); + await Assert.That(app.FrozenScrollbackOf(Main)).IsNull(); + } + + [Test] + public async Task TheClaimListNamesAltFAndNoLongerNamesCtrlF() + { + var claims = MacroKeys.AppShortcuts; + + await Assert.That(claims.Any(c => c.Modifiers == ConsoleModifiers.Alt && c.Key == ConsoleKey.F)).IsTrue(); + await Assert.That(claims.Any(c => c.Modifiers == ConsoleModifiers.Control && c.Key == ConsoleKey.F)).IsFalse(); + } +} diff --git a/tests/SharpMUTerm.Tui.Tests/OutputScrollbackTests.cs b/tests/SharpMUTerm.Tui.Tests/OutputScrollbackTests.cs index 423f17d..39e8ce9 100644 --- a/tests/SharpMUTerm.Tui.Tests/OutputScrollbackTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/OutputScrollbackTests.cs @@ -513,7 +513,7 @@ await Assert.That(Rows(app.RenderWholeFrame()) /// /// A frozen pane's pinned half scrolls, and its live tail keeps following. Freeze holds a region /// still above the bar and leaves the tail live below it; before this the pinned half could only ever - /// show its oldest screenful, which made ⌃F a way of pinning history you could not read. + /// show its oldest screenful, which made ⌥F a way of pinning history you could not read. /// [Test] public async Task AFrozenPanesPinnedHalfScrollsWhileItsTailStaysLive() diff --git a/tests/SharpMUTerm.Tui.Tests/TimestampGutterTests.cs b/tests/SharpMUTerm.Tui.Tests/TimestampGutterTests.cs index 3623626..db7c4e7 100644 --- a/tests/SharpMUTerm.Tui.Tests/TimestampGutterTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/TimestampGutterTests.cs @@ -193,7 +193,7 @@ public async Task TurningTheColumnOffTakesItBackOffWhatIsOnScreen() /// /// A frozen pane takes it on both halves, and stays a frozen pane. Freezing splits a window's - /// buffer at the length it had when ⌃F was pressed and draws the two sides into two controls, so a + /// buffer at the length it had when ⌥F was pressed and draws the two sides into two controls, so a /// repaint that fed one control the whole buffer would slide the pinned scrollback into the live /// tail and duplicate every line above the bar. The freeze view puts the demo scene above the /// split and two courier lines below it — one assertion for each side. From abff85ef8ab7436a71834d6251c632045bdd56a9 Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Tue, 11 Aug 2026 14:58:35 -0500 Subject: [PATCH 04/19] =?UTF-8?q?feat(input):=20=E2=8C=A5=E2=86=91/?= =?UTF-8?q?=E2=8C=A5=E2=86=93=20recall=20command=20history=20wherever=20th?= =?UTF-8?q?e=20caret=20is?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bare arrows recall only where the caret has nowhere further to go, which stops being a rule you can hold in your head once the command line grows to a second row. They are unchanged — the Alt pair is added beside them and skips the caret test. ⌃↑/⌃↓ was the other chord offered and was never available: the terminal writes ESC [ 1;5 A for it and this client already spends that on pane selection and on the ladder onto the second command line. ESC [ 1;3 A is Alt, is free, and was measured at a raw reader before it was spent. Matched on exact modifiers, so ⌥⇧↑ still reaches the pane resize. Also corrects four stale chords in the design README's key list — ⌃Tab (cannot arrive: a terminal writes 0x09), ⌃R as reconnect (it is the history surface), ⌃L as a logging toggle (it is the newline chord) and Shift+⏎ (collapses onto a bare Enter). MacroKeys.Verdict was already right about all four. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN --- CLAUDE.md | 8 ++ docs/design/README.md | 12 +- src/SharpMUTerm.Tui/SharpMUTermApp.cs | 65 +++++++-- .../HistoryChordTests.cs | 129 ++++++++++++++++++ 4 files changed, 196 insertions(+), 18 deletions(-) create mode 100644 tests/SharpMUTerm.Tui.Tests/HistoryChordTests.cs diff --git a/CLAUDE.md b/CLAUDE.md index 778c9f4..1d8ce56 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -553,6 +553,14 @@ markup (`[bold #rrggbb on #rrggbb]…[/]`, `[[`/`]]` escaping, `[link=url]…[/] alias (the `⌃D` rule). The label a **frozen** reader is looking at (`FreezeBarRenderer`) moves with the chord: a bar naming a key that no longer thaws the pane would be the worst possible place to leave a stale chord. + - **History recall has its own chord: `⌥↑`/`⌥↓`.** The bare arrows still recall where the caret has + nowhere further to go, and that is unchanged — but it is a rule that stops being usable the moment + the bar grows to a second row, which was the report. `⌃↑`/`⌃↓`, the alternative offered, was never + available: the terminal writes `ESC [ 1;5 A` and this client already spends it on pane selection + and on the ladder onto the second command line. `ESC [ 1;3 A` is Alt, is free, and is what + `TryRecallKey` now matches on — **exactly**, so `⌥⇧↑` (the pane resize) still reaches its own + handler. A macro bound to `Alt+Up` wins over recall, because `DispatchMacro` runs first: the same + relationship `Ctrl+←/→` has with pane selection. - **Known and not fixed here**: `⌃N` and `⌃O` have no reverse (the character cycle does — `⌥J`/`⌥K`), and `⌃W` and `⌃B x` are two chords for one action. Both are shape complaints rather than defects, and both are behaviour changes rather than modifier moves. diff --git a/docs/design/README.md b/docs/design/README.md index 97a4f47..4054d81 100644 --- a/docs/design/README.md +++ b/docs/design/README.md @@ -423,9 +423,15 @@ count on the tab, the rail character, and the rail world. ### Other keys -`⌃P` command surface · `⌥F` freeze/resume in focused pane · `⌃L` toggle logging · -`⌃Tab` next tab in pane · `⌃R` reconnect · `↑`/`↓` history · `F2`–`F9` config · `Esc` close overlay · -`Shift+Enter` newline in input. +`⌃P` command surface · `⌥F` freeze/resume in focused pane · `⌃R` command-history search · +`⌃N` next window · `⌥D`/`⌥R` disconnect/reconnect · `⌥↑`/`⌥↓` command history (`↑`/`↓` do it too, +where the caret has nowhere further to go) · `⌥⏎`, or `⌃L`, newline in input · `F1` composer · +`F2`–`F9` config · `Esc` close overlay. + +Four entries here were stale and have been corrected rather than left: `⌃Tab` cannot arrive at all (a +terminal writes `0x09` for it, which is a bare Tab), `⌃R` is the history surface and never was +reconnect, `⌃L` is the newline chord and not a logging toggle, and `Shift+⏎` collapses onto a bare +`Enter` with no modifier bits. `MacroKeys.Verdict` is the readable form of all of that. --- diff --git a/src/SharpMUTerm.Tui/SharpMUTermApp.cs b/src/SharpMUTerm.Tui/SharpMUTermApp.cs index 4fa35ea..e5736dd 100644 --- a/src/SharpMUTerm.Tui/SharpMUTermApp.cs +++ b/src/SharpMUTerm.Tui/SharpMUTermApp.cs @@ -3509,54 +3509,89 @@ private void RecallDrafts(string windowId) _second.Text = _drafts.Recall(windowId, InputBar.Secondary); } + /// Which of the two ways into history a keystroke is, or neither. + private enum RecallMode + { + /// Not a recall key at all. + No, + + /// A bare arrow: the caret gets it first, and history only where the caret cannot move. + AtTheEdges, + + /// ⌥↑ or ⌥↓: history, wherever the caret is. + Always, + } + /// - /// Handles ↑/↓ as draft-safe history recall. A command line tall enough to have another row keeps - /// the arrows for the caret — recall only happens where the caret has nowhere further to go, which - /// is the single-row case it has always been plus the top and bottom of a grown one. + /// Handles ↑/↓ and ⌥↑/⌥↓ as draft-safe history recall. + /// + /// Two ways in, and the difference is the caret. On the bare arrows a command line tall enough + /// to have another row keeps them for the caret, and recall happens only where the caret has nowhere + /// further to go — the single-row case it has always been, plus the top and bottom of a grown one. + /// That is unchanged. ⌥↑/⌥↓ mean history and nothing else, which is what makes them usable on a draft + /// several rows tall: "recall happens at the edges" stops being a rule you can hold in your head the + /// moment the bar grows, and that was the report. Both are kept, because the edge behaviour is how + /// this client has always worked and nothing is taken away by adding a chord beside it. + /// + /// + /// ⌥ and not ⌃, and that is the terminal's answer rather than a preference. ⌃↑/⌃↓ arrives as + /// ESC [ 1;5 A/B and is already spent twice over — pane selection, and the ladder from + /// the last pane onto the second command line. ⌥↑/⌥↓ is ESC [ 1;3 A/B, which + /// AnsiInputParser.ParseModifiers reads as Alt, and which nothing else here or in the + /// framework claims. Measured at a raw-mode reader with kitten @ send-key before it was spent. + /// /// - /// The bare arrows only. This used to look at the key and never at the modifiers, which is how - /// it came to swallow Shift+↑ from the scrollback work — repaired then by putting the scrollback keys - /// ahead of it, which fixes the one chord that had already been reported and leaves the next one to - /// be found the same way. Ordering still matters and is unchanged; declining what is not ours is the - /// half that was missing. + /// Exact modifiers, on both. This used to look at the key and never at the modifiers, which is + /// how it came to swallow Shift+↑ from the scrollback work — repaired then by putting the scrollback + /// keys ahead of it, which fixed the one chord that had been reported and left the next one to be + /// found the same way. Ordering still matters and is unchanged; declining what is not ours is the + /// half that was missing, and a switch on the whole modifier set is what keeps Alt+Shift+↑ + /// (the pane resize) out of here now that Alt alone means something. /// /// private bool TryRecallKey(KeyPressedEventArgs e) { - if (e.KeyInfo.Modifiers != 0) + var mode = e.KeyInfo.Modifiers switch + { + (ConsoleModifiers)0 => RecallMode.AtTheEdges, + ConsoleModifiers.Alt => RecallMode.Always, + _ => RecallMode.No, + }; + + if (mode == RecallMode.No) { return false; } var bar = ActiveBar(); var kind = BarKind(bar); - var history = HistoryFor(kind); + var entries = HistoryFor(kind); string? text; switch (e.KeyInfo.Key) { case ConsoleKey.UpArrow: - if (bar.TryMoveRow(-1)) + if (mode == RecallMode.AtTheEdges && bar.TryMoveRow(-1)) { e.Handled = true; return true; } - text = history.Recall(bar.Text); + text = entries.Recall(bar.Text); break; case ConsoleKey.DownArrow: - if (bar.TryMoveRow(1)) + if (mode == RecallMode.AtTheEdges && bar.TryMoveRow(1)) { e.Handled = true; return true; } - if (!history.IsRecalling) + if (!entries.IsRecalling) { return false; } - text = history.Forward(); + text = entries.Forward(); break; default: return false; diff --git a/tests/SharpMUTerm.Tui.Tests/HistoryChordTests.cs b/tests/SharpMUTerm.Tui.Tests/HistoryChordTests.cs new file mode 100644 index 0000000..3cd1bb4 --- /dev/null +++ b/tests/SharpMUTerm.Tui.Tests/HistoryChordTests.cs @@ -0,0 +1,129 @@ +using SharpConsoleUI.Drivers; +using SharpMUTerm.Graphics; +using SharpMUTerm.Tui; + +namespace SharpMUTerm.Tui.Tests; + +/// +/// ⌥↑/⌥↓ mean history and nothing else. The bare arrows still recall at the edges — nothing is taken +/// away — but they answer to the caret first, which stops being a usable rule the moment the command +/// line grows to a second row. That was the reported complaint. +/// +/// ⌃↑/⌃↓, the other chord the request offered, was never available: the terminal writes `ESC [ 1;5 A` +/// for it and this client already spends that on pane selection and on the ladder onto the second +/// command line. `ESC [ 1;3 A` — Alt — is free, and was measured at a raw reader before it was spent. +/// +/// +/// Serialised: constructing the app touches the process-global console streams. +[NotInParallel] +public class HistoryChordTests +{ + private static readonly TerminalCapabilities Headless = + new(GraphicsProtocol.None, supportsTrueColor: true, supportsKittyGraphics: false, supportsSixel: false); + + private static SharpMUTermApp App() + { + Console.SetIn(TextReader.Null); + var app = new SharpMUTermApp(DemoScene.Build(), Headless, new HeadlessConsoleDriver(120, 34)); + app.RenderSnapshot(); + return app; + } + + private static ConsoleKeyInfo Chord(ConsoleKey key, bool alt = false) => + new('\0', key, shift: false, alt: alt, control: false); + + /// One printable character, as the command line's own tests spell it. + private static ConsoleKeyInfo Key(char c) => new(c, ConsoleKey.NoName, false, false, false); + + private static void Type(SharpMUTermApp app, string text) + { + foreach (var c in text) + { + app.SimulateKey(Key(c)); + } + } + + private static void Send(SharpMUTermApp app, string text) + { + Type(app, text); + app.SimulateKey(new ConsoleKeyInfo('\r', ConsoleKey.Enter, false, false, false)); + } + + /// + /// Sends two lines so the history has something in it, and leaves the bar empty. The leading empty + /// send is what clears it: the demo scene seeds a draft into the command line, and typing on top of + /// that would send — and recall — a line nobody typed. + /// + private static void Seed(SharpMUTermApp app) + { + Send(app, string.Empty); + Send(app, "look"); + Send(app, "say hello"); + } + + [Test] + public async Task AltUpRecallsTheNewestLineAndAltDownWalksBack() + { + var app = App(); + Seed(app); + + app.SimulateKey(Chord(ConsoleKey.UpArrow, alt: true)); + await Assert.That(app.ArmedInputText).IsEqualTo("say hello"); + + app.SimulateKey(Chord(ConsoleKey.UpArrow, alt: true)); + await Assert.That(app.ArmedInputText).IsEqualTo("look"); + + app.SimulateKey(Chord(ConsoleKey.DownArrow, alt: true)); + await Assert.That(app.ArmedInputText).IsEqualTo("say hello"); + } + + /// + /// The point of the chord. With a draft tall enough to have a row above the caret, the bare ↑ is the + /// caret's and ⌥↑ is history's — on the same keystroke, from the same position. + /// + [Test] + public async Task OnAGrownBarTheBareArrowMovesTheCaretAndTheAltArrowRecalls() + { + var app = App(); + Seed(app); + var draft = new string('x', 400); // wraps to several rows at 120 columns + Type(app, draft); + + app.SimulateKey(Chord(ConsoleKey.UpArrow)); + await Assert.That(app.ArmedInputText).IsEqualTo(draft); + + app.SimulateKey(Chord(ConsoleKey.UpArrow, alt: true)); + await Assert.That(app.ArmedInputText).IsEqualTo("say hello"); + } + + /// + /// ⌥↓ with nothing recalled is not ours: it must not clear the bar. Exactly the bare-arrow rule, + /// which declines rather than blanking a draft. + /// + [Test] + public async Task AltDownWithNothingRecalledLeavesTheDraftAlone() + { + var app = App(); + Seed(app); + Type(app, "half a thought"); + + app.SimulateKey(Chord(ConsoleKey.DownArrow, alt: true)); + + await Assert.That(app.ArmedInputText).IsEqualTo("half a thought"); + } + + /// + /// The bare arrows are unchanged: on a single-row bar they still recall, because the caret has + /// nowhere further to go. Nothing about this feature takes that away. + /// + [Test] + public async Task TheBareArrowsStillRecallAtTheEdges() + { + var app = App(); + Seed(app); + + app.SimulateKey(Chord(ConsoleKey.UpArrow)); + + await Assert.That(app.ArmedInputText).IsEqualTo("say hello"); + } +} From 9715575dbb7d07605819f78b4699638dd67b8567 Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Tue, 11 Aug 2026 15:59:51 -0500 Subject: [PATCH 05/19] =?UTF-8?q?docs(keys):=20the=20=E2=8C=A5F=20comment?= =?UTF-8?q?=20says=20which=20chord=20freeze=20moved=20*from*?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review catch, and a self-inflicted one: the blanket ⌃F → ⌥F rewrite that moved the chord across this file also rewrote the words "It was ⌃F", leaving a comment that read "It was ⌥F, and moved" — the chord it moved *to*, which says nothing. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN --- src/SharpMUTerm.Tui/SharpMUTermApp.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SharpMUTerm.Tui/SharpMUTermApp.cs b/src/SharpMUTerm.Tui/SharpMUTermApp.cs index e5736dd..261dc3d 100644 --- a/src/SharpMUTerm.Tui/SharpMUTermApp.cs +++ b/src/SharpMUTerm.Tui/SharpMUTermApp.cs @@ -4725,7 +4725,7 @@ private void RegisterFocusReportTab() return () => { CycleCharacter(-1); return true; }; } - // ⌥F freezes and resumes the focused pane. It was ⌥F, and moved so that search could have the + // ⌥F freezes and resumes the focused pane. It was ⌃F, and moved so that search could have the // chord every reader on every platform reaches for. Same delivery story as ⌥D and ⌥R: ESC + a // printable byte, decoded as that letter with Alt set. if (claim.Key == ConsoleKey.F) From 00c66701a8b6e9e4908162e543b893af6e9049b3 Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Tue, 11 Aug 2026 15:04:28 -0500 Subject: [PATCH 06/19] =?UTF-8?q?docs(plan):=20PR=202=20of=20the=20stack?= =?UTF-8?q?=20=E2=80=94=20the=20window=20activity=20boundary=20and=20its?= =?UTF-8?q?=20dwell=20floor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN --- .../2026-08-11-window-activity-boundary.md | 787 ++++++++++++++++++ 1 file changed, 787 insertions(+) create mode 100644 docs/superpowers/plans/2026-08-11-window-activity-boundary.md diff --git a/docs/superpowers/plans/2026-08-11-window-activity-boundary.md b/docs/superpowers/plans/2026-08-11-window-activity-boundary.md new file mode 100644 index 0000000..5c40010 --- /dev/null +++ b/docs/superpowers/plans/2026-08-11-window-activity-boundary.md @@ -0,0 +1,787 @@ +# PR 2 — The activity boundary: "since you were last here", and a bar that lasts Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan +> task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Mark where the reader left off in *any* window they were not watching — not just when they +tabbed away from the terminal — and stop the bar retiring before it has been read. + +**Architecture:** One new per-window boundary (`_missedFrom`), recorded at the moment a line lands in +a window that is not caught up, and materialised into the existing away-bar machinery when that window +becomes caught up again. The terminal-away path is unchanged and still uses its input-proxy boundary, +because focus-out is unobservable; where both boundaries exist the older wins. Retirement gains a third +conjunct: a dwell floor from a new F7 setting, measured off the app's existing `TimeProvider`. + +**Tech Stack:** C# / .NET 10, SharpConsoleUI 2.5.14 (package only), TUnit. + +**Spec:** `docs/superpowers/specs/2026-08-11-pane-search-and-activity-design.md`, part 3. + +**Branch:** `feat/window-activity-boundary`, off `feat/find-chords`. Second of a three-PR stack. + +## Global Constraints + +- Target framework `net10.0`; file-scoped namespaces, 4-space C#, LF endings. +- `SharpMUTerm.Core` stays UI-agnostic. The only Core change here is one settings property. +- Run suites directly, never `dotnet test`, and keep the ` + /// The window-absence bar says what it can and no more. There is no duration on it: the terminal + /// bar measures from the last input before the reader vanished, which is a real (if approximate) + /// instant, while this one is made when a line lands in a window nobody is watching — a moment the + /// reader was not part of. A "3 min" on it would be timing the *output*, not the absence. + /// + [Test] + public async Task TheMissedBarCountsTheLinesAndClaimsNoDuration() + { + var bar = AwayBarRenderer.Missed(47, "#c678dd"); + + await Assert.That(bar).Contains($"[#c678dd]{Glyphs.Away} {AwayBarRenderer.MissedLabel}[/]"); + await Assert.That(bar).Contains("47 lines since you were here"); + await Assert.That(bar).DoesNotContain("min"); + } + + [Test] + public async Task TheMissedBarSaysOneLineRatherThanOneLines() + { + await Assert.That(AwayBarRenderer.Missed(1, "#c678dd")).Contains("1 line since you were here"); + } +``` + +- [ ] **Step 2: Run it and watch it fail** + +```bash +dotnet run -c Release --project tests/SharpMUTerm.Tui.Tests --treenode-filter "/*/*/AwayBarRendererTests/*" + /// The label on the bar marking a *window* the reader was not watching, as against + /// 's terminal absence. Two words rather than one bar with two meanings: "AWAY" + /// is about the reader, "NEW" is about the window, and a reader who sees both in one client should + /// be able to tell which absence they are looking at without counting the lines. + /// + internal const string MissedLabel = "NEW"; + + /// + /// The bar for lines that arrived in a window while the reader was not + /// watching it, on an already-resolved #rrggbb accent. + /// + /// It carries a count and no duration, which is the one way it differs from . The + /// terminal bar's span is measured from the last input before the reader vanished — approximate, + /// but an instant the reader was part of. This boundary is made when a line lands in a window + /// nobody is watching, so a duration on it would be timing the output rather than the absence. + /// + /// + public static string Missed(int lines, string accentHex) + { + ArgumentException.ThrowIfNullOrEmpty(accentHex); + + var count = lines == 1 ? "1 line" : $"{lines} lines"; + var rule = new string('─', RuleCells); + return $"[{accentHex}]{Glyphs.Away} {MissedLabel}[/] " + + $"[dim]{MarkupText.Escape($"{count} since you were here")} {rule}[/]"; + } +``` + +- [ ] **Step 4: Run the tests** + +Same command. Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add -A +git commit -m "feat(activity): a bar for the window you were not watching + +Beside the terminal-absence bar rather than replacing it: AWAY is about the +reader, NEW is about the window. No duration on this one — its boundary is made +when a line lands in a window nobody is watching, so a span on it would be +timing the output rather than the absence. + +Co-Authored-By: Claude Opus 5 (1M context) +Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN" +``` + +--- + +### Task 2: The boundary — record it where it happens, draw it when they come back + +**Files:** +- Modify: `src/SharpMUTerm.Tui/SharpMUTermApp.cs` +- Create: `tests/SharpMUTerm.Tui.Tests/WindowActivityBoundaryTests.cs` + +**Interfaces:** +- Consumes: `Workspace.IsCaughtUp(string)` → `bool` (visible **and** not scrolled back); + `AwayBarRenderer.Missed(int, string)` from Task 1; the existing `_awayMarks`, `RemoveAwayBar`, + `RevealAwayBar`, `RepaintPane`, `FrozenAccentHex()`. +- Produces: `SharpMUTermApp.AwayBarIndex(string windowId)` (already internal) now also answers for a + window bar; `MarkMissedLines()` private. Task 3 adds a conjunct to `ConsumeReadAwayBars`. + +- [ ] **Step 1: Write the failing test** + +Create `tests/SharpMUTerm.Tui.Tests/WindowActivityBoundaryTests.cs`: + +```csharp +using SharpConsoleUI.Drivers; +using SharpMUTerm.Graphics; +using SharpMUTerm.Tui; + +namespace SharpMUTerm.Tui.Tests; + +/// +/// The boundary that marks a window the reader was not watching. It is the same bar the terminal +/// absence draws and it answers to the same rule the unread badge does — IsCaughtUp, visible +/// *and* at the live tail — so a badge showing a count always has a bar under it saying where the +/// count begins. +/// +/// Serialised: constructing the app touches the process-global console streams. +[NotInParallel] +public class WindowActivityBoundaryTests +{ + private const string Main = "main"; + private const string Chat = "chat"; + + private static readonly TerminalCapabilities Headless = + new(GraphicsProtocol.None, supportsTrueColor: true, supportsKittyGraphics: false, supportsSixel: false); + + private static SharpMUTermApp App() + { + Console.SetIn(TextReader.Null); + var app = new SharpMUTermApp(DemoScene.Build(), Headless, new HeadlessConsoleDriver(120, 34)); + app.RenderSnapshot(); + return app; + } + + /// + /// A background tab gains lines, and going to it lands the reader on a bar with the missed lines + /// under it. This is the reported case: the badge said 3 and nothing said where the 3 began. + /// + [Test] + public async Task AWindowTheReaderCannotSeeAccruesABoundaryAndShowsItOnReturn() + { + var app = App(); + + app.SimulateLine(Chat, " Ana: anyone about?"); + app.SimulateLine(Chat, " Bo: here"); + await Assert.That(app.AwayBarIndex(Chat)).IsNull(); + + app.SimulateWindowChange(Chat); + + await Assert.That(app.AwayBarIndex(Chat)).IsNotNull(); + await Assert.That(app.RenderSnapshot()).Contains("2 lines since you were here"); + } + + /// + /// The window in front of the reader, at its live tail, accrues nothing — those lines went past + /// their eyes. Same rule as the unread badge, which is the point: the two must not disagree. + /// + [Test] + public async Task TheWindowInFrontOfTheReaderGetsNoBar() + { + var app = App(); + + app.SimulateLine(Main, "The goblin snarls at you."); + app.RenderSnapshot(); + + await Assert.That(app.AwayBarIndex(Main)).IsNull(); + await Assert.That(app.RenderSnapshot()).DoesNotContain("since you were here"); + } + + /// + /// A window the reader is looking at but has scrolled back off is exactly as blind as one they are + /// not looking at — the reasoning OnLine already uses for the badge — so it accrues, and the + /// bar appears when they come back down to the tail. + /// + [Test] + public async Task AVisibleWindowScrolledBackAccruesAndTheBarAppearsOnTheWayBackDown() + { + var app = App(); + app.LoadLongScene(); + app.SimulateScrollKey(new ConsoleKeyInfo('\0', ConsoleKey.PageUp, false, false, false)); + + app.SimulateLine(Main, "A goblin corpse lies here."); + await Assert.That(app.AwayBarIndex(Main)).IsNull(); + + app.SimulateScrollKey(new ConsoleKeyInfo('\0', ConsoleKey.End, false, false, true)); + + await Assert.That(app.AwayBarIndex(Main)).IsNotNull(); + } + + /// + /// One bar per window, and the terminal absence is the older of the two boundaries: a window that + /// had already missed lines before the reader left the terminal must not have its bar moved *down* + /// to where they left, which would hide the lines it was drawn for. + /// + [Test] + public async Task TheOlderBoundaryWins() + { + var app = App(); + + app.SimulateLine(Chat, "first, missed while the tab was in the background"); + app.SimulateReturnFromAway(TimeSpan.FromMinutes(5)); + + await Assert.That(app.AwayBarIndex(Chat)).IsEqualTo(0); + } +} +``` + +- [ ] **Step 2: Add the test seam if it is missing** + +`SimulateLine(windowId, text)` must append one line of *world* output to a window, through the same +path `OnLine` uses (`AppendWindowLine` + `NoteActivity`). If `SharpMUTermApp` has no such internal +method, add one beside `SimulateWindowChange`: + +```csharp + /// + /// Appends one line of a world's output to a window, the way does — including + /// the unread accounting, because that is the fact the activity boundary shares a rule with. + /// + internal void SimulateLine(string windowId, string text) + { + AppendWindowLine(windowId, MarkupText.Escape(text), StampNow()); + if (!_workspace.IsCaughtUp(windowId)) + { + _workspace.NoteActivity(windowId); + RefreshTabTitles(); + } + } +``` + +- [ ] **Step 3: Run it and watch it fail** + +```bash +dotnet run -c Release --project tests/SharpMUTerm.Tui.Tests --treenode-filter "/*/*/WindowActivityBoundaryTests/*" + /// Where the lines the reader has not seen start, per window, for the windows that have any — the + /// buffer index of the first line that landed while the window was not caught up. + /// + /// Recorded forwards, unlike the terminal boundary beside it. `_awayBoundary` has to be + /// reconstructed from the last input before the reader vanished, because a terminal reports focus-in + /// and this client cannot see focus-out at all. This one needs none of that: a line arrives, the + /// window either is or is not caught up, and if it is not then *this* is the boundary. Exact, and + /// one comparison in a method that already runs per line. + /// + /// + /// and not IsVisible, which is already the rule the unread + /// badge answers to: a visible tab whose output the reader has scrolled back off is exactly as blind + /// as a tab they are not looking at. One fact behind both, so a badge showing a count always has a + /// bar under it saying where the count begins. + /// + /// + private readonly Dictionary _missedFrom = new(StringComparer.Ordinal); + + /// + /// Whether a line landing out of sight is an absence yet. False until the constructor has + /// finished, because until the workspace is laid out "not visible" means "no pane has been built", + /// which is not something the reader missed — and because the restore replay pours a previous + /// session's lines through into windows that are not visible yet. + /// Every one of those already sits under a bar saying exactly what + /// it is; a NEW bar over the top would be the client marking its own startup as news. + /// + private bool _watching; +``` + +Set it at the very end of the constructor, after `_system.AddWindow(_window);`: + +```csharp + _watching = true; +``` + +In `AppendWindowLine`, immediately before `buffer.Add(...)`: + +```csharp + // The moment a boundary is made: a line landing in a window the reader is not watching. Before + // the Add, so the index is the first line they missed rather than the one after it. + if (_watching && !_missedFrom.ContainsKey(windowId) && !_workspace.IsCaughtUp(windowId)) + { + _missedFrom[windowId] = buffer.Count; + } +``` + +and in the trim block, beside the other index fixups: + +```csharp + if (_missedFrom.TryGetValue(windowId, out var missed)) + { + _missedFrom[windowId] = Math.Max(0, missed - excess); + } +``` + +- [ ] **Step 5: Draw it when the window is watched again** + +Add, beside `MarkWhereTheReaderLeft`: + +```csharp + /// + /// Draws the boundary in every window that has missed lines and is now caught up again — the + /// reader has come back to it, so this is the moment to say where they left off. + /// + /// Called from the two places a window can *become* caught up: , which raises + /// a tab and focuses its pane, and , which is where every scroll + /// route lands. There is no third: Workspace.IsCaughtUp is visibility and scroll position, + /// and nothing else moves either. + /// + /// + /// The bar is client chrome, so it is inserted through the buffer rather than through + /// : it must not badge the window unread, and it must not reach the + /// restore log. Recursion is not a risk — the entry is removed before the reveal, and the reveal's + /// scroll comes back through with nothing pending. + /// + /// + private void MarkMissedLines() + { + foreach (var windowId in _missedFrom.Keys.ToArray()) + { + if (!_workspace.IsCaughtUp(windowId) + || !_lines.TryGetValue(windowId, out var buffer) + || string.Equals(windowId, WebWindowId, StringComparison.Ordinal)) + { + continue; + } + + var at = Math.Clamp(_missedFrom[windowId], 0, buffer.Count); + _missedFrom.Remove(windowId); + + var missed = buffer.Count - at; + if (missed <= 0) + { + continue; + } + + // At most one bar per window: whichever absence drew the last one, this replaces it. It goes + // first, because removing it shifts every index after it — the boundary included. + RemoveAwayBar(windowId); + at = Math.Clamp(at, 0, buffer.Count); + + buffer.Insert(at, new PaneLine(AwayBarRenderer.Missed(buffer.Count - at, FrozenAccentHex()))); + if (_freezePoints.TryGetValue(windowId, out var freeze) && freeze > at) + { + _freezePoints[windowId] = freeze + 1; + } + + var mark = new AwayMark { Index = at, DrawnAfter = _focus.InputCount, DrawnAt = _time.GetUtcNow() }; + _awayMarks[windowId] = mark; + RepaintPane(windowId); + RevealAwayBar(windowId, mark); + } + } +``` + +Call it from `SyncScrollbackState`, immediately before `ConsumeReadAwayBars()`: + +```csharp + MarkMissedLines(); +``` + +and from `Activate`, immediately after `SyncToFocusedPane();`. + +- [ ] **Step 6: Let the older boundary win** + +In `MarkWhereTheReaderLeft`, replace + +```csharp + var at = Math.Clamp(_awayBoundary.GetValueOrDefault(windowId), 0, buffer.Count); +``` + +with + +```csharp + // Two boundaries can exist for one window: the terminal absence's, reconstructed from the + // last input before the reader vanished, and the window's own, recorded exactly when a line + // landed out of sight. The *older* wins — one bar per window, marking the earlier of the two + // things they missed. Taking the terminal one unconditionally would move the bar down past + // lines it had already been made for. + var boundary = _awayBoundary.GetValueOrDefault(windowId); + if (_missedFrom.TryGetValue(windowId, out var missedFrom)) + { + boundary = Math.Min(boundary, missedFrom); + _missedFrom.Remove(windowId); + } + + var at = Math.Clamp(boundary, 0, buffer.Count); +``` + +- [ ] **Step 7: Run the tests** + +```bash +dotnet run -c Release --project tests/SharpMUTerm.Tui.Tests --treenode-filter "/*/*/WindowActivityBoundaryTests/*" +Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN" +``` + +--- + +### Task 3: The dwell floor + +**Files:** +- Modify: `src/SharpMUTerm.Core/Configuration/PreferenceSettings.cs` +- Modify: `src/SharpMUTerm.Tui/OptionsScreenRenderer.cs` +- Modify: `src/SharpMUTerm.Tui/SharpMUTermApp.cs` (`AwayMark`, `ConsumeReadAwayBars`, the two draw sites) +- Create: `tests/SharpMUTerm.Tui.Tests/ActivityBarDwellTests.cs` + +**Interfaces:** +- Consumes: `SharpMUTermApp._time` (the constructor's existing `TimeProvider`, defaulting to + `TimeProvider.System`) — no new constructor parameter; tests pass a `FakeTimeProvider`-alike. +- Produces: `TextSettings.ActivityBarSeconds` (`int`, default `TextSettings.DefaultActivityBarSeconds` + = 30, max `TextSettings.MaxActivityBarSeconds` = 600). + +- [ ] **Step 1: Write the failing test** + +Create `tests/SharpMUTerm.Tui.Tests/ActivityBarDwellTests.cs`: + +```csharp +using SharpConsoleUI.Drivers; +using SharpMUTerm.Core.Configuration; +using SharpMUTerm.Graphics; +using SharpMUTerm.Tui; + +namespace SharpMUTerm.Tui.Tests; + +/// +/// The bar does not retire the instant it has been read past. Two conjuncts were not enough: on a +/// shallow absence the pane is already at its live tail and the very next keystroke retired the bar, +/// which is a second or two after it appeared. The third conjunct is a floor in *time*, because that +/// is the unit the complaint was in — a raised input count would be an hour on a quiet character and +/// three seconds on a busy one. +/// +/// Serialised: constructing the app touches the process-global console streams. +[NotInParallel] +public class ActivityBarDwellTests +{ + private const string Chat = "chat"; + + private static readonly TerminalCapabilities Headless = + new(GraphicsProtocol.None, supportsTrueColor: true, supportsKittyGraphics: false, supportsSixel: false); + + /// A clock the test moves by hand, so the floor is not a race against a loaded CI box. + private sealed class Clock : TimeProvider + { + private DateTimeOffset _now = new(2026, 8, 11, 12, 0, 0, TimeSpan.Zero); + + public override DateTimeOffset GetUtcNow() => _now; + + public void Advance(TimeSpan by) => _now += by; + } + + private static (SharpMUTermApp App, Clock Time) App(int seconds) + { + Console.SetIn(TextReader.Null); + var config = DemoScene.Build(); + config.Text.ActivityBarSeconds = seconds; + var clock = new Clock(); + var app = new SharpMUTermApp(config, Headless, new HeadlessConsoleDriver(120, 34), clock); + app.RenderSnapshot(); + return (app, clock); + } + + /// Draws a bar in the Chat window and reads it past: at the tail, one input since. + private static void DrawAndRead(SharpMUTermApp app) + { + app.SimulateLine(Chat, " Ana: anyone about?"); + app.SimulateWindowChange(Chat); + app.SimulateKey(new ConsoleKeyInfo('x', ConsoleKey.NoName, false, false, false)); + app.SimulateScrollKey(new ConsoleKeyInfo('\0', ConsoleKey.End, false, false, true)); + } + + [Test] + public async Task TheBarSurvivesBeingReadPastUntilTheFloorHasElapsed() + { + var (app, clock) = App(30); + DrawAndRead(app); + + await Assert.That(app.AwayBarIndex(Chat)).IsNotNull(); + + clock.Advance(TimeSpan.FromSeconds(29)); + app.SimulateKey(new ConsoleKeyInfo('y', ConsoleKey.NoName, false, false, false)); + await Assert.That(app.AwayBarIndex(Chat)).IsNotNull(); + + clock.Advance(TimeSpan.FromSeconds(2)); + app.SimulateKey(new ConsoleKeyInfo('z', ConsoleKey.NoName, false, false, false)); + await Assert.That(app.AwayBarIndex(Chat)).IsNull(); + } + + /// Zero is a real answer, and it is exactly the behaviour this feature replaced. + [Test] + public async Task AFloorOfZeroRetiresTheBarAsSoonAsItHasBeenReadPast() + { + var (app, _) = App(0); + DrawAndRead(app); + + await Assert.That(app.AwayBarIndex(Chat)).IsNull(); + } +} +``` + +- [ ] **Step 2: Run it and watch it fail** + +```bash +dotnet run -c Release --project tests/SharpMUTerm.Tui.Tests --treenode-filter "/*/*/ActivityBarDwellTests/*" + /// How long the activity bar stays put after the reader has read past it, in seconds. Thirty by + /// default; zero retires it as soon as the pane is back at its live tail and one input has landed, + /// which is what this client did before the floor existed. + /// + /// A floor in *time* rather than in keystrokes, because the complaint was in time: the bar went + /// before it had been read. A raised input count would be an hour on a quiet character and three + /// seconds on a busy one. + /// + /// + /// It is only a floor. The bar still has to be read past — the pane back at its tail, one input + /// since it was drawn — and it retires on the first of those checks *after* the floor, which is the + /// next keystroke or scroll rather than a timer firing on its own. A client with nothing happening + /// in it keeps the bar, which is the right answer for a reader who has walked away again. + /// + /// + public int ActivityBarSeconds { get; set; } = DefaultActivityBarSeconds; + + /// Thirty seconds — long enough to read a screenful, short enough not to become furniture. + public const int DefaultActivityBarSeconds = 30; + + /// Ten minutes. Past this the bar is not a boundary marker, it is a pin. + public const int MaxActivityBarSeconds = 600; +``` + +- [ ] **Step 4: Add the F7 row** + +In `OptionsScreenRenderer.TextAnsiScreen`, after the UNICODE section: + +```csharp + new(string.Empty, null, null), + new("├ ACTIVITY", null, null), + + // Seconds, and its own section: it is the only row here that describes the client's own + // chrome rather than how a world's text is drawn. Zero is a real answer — it retires the + // bar as soon as it has been read past, which is what this client did before the floor. + new("activity bar holds for (seconds)", + settings.ActivityBarSeconds.ToString(CultureInfo.InvariantCulture), null, null, null, + ScreenField.Integer( + "activity bar holds for (seconds)", + () => settings.ActivityBarSeconds, v => settings.ActivityBarSeconds = v, + 0, TextSettings.MaxActivityBarSeconds)), +``` + +- [ ] **Step 5: Add the conjunct** + +In `AwayMark`: + +```csharp + /// + /// When this bar was drawn, off the app's own TimeProvider. The dwell floor is measured + /// from here — see . + /// + public DateTimeOffset DrawnAt; +``` + +Set it at both draw sites (`MarkWhereTheReaderLeft` and `MarkMissedLines`): +`DrawnAt = _time.GetUtcNow()`. + +In `ConsumeReadAwayBars`, replace the condition: + +```csharp + // Three conjuncts, and the third is the one the reader asked for. At the live tail, which + // means something because the reveal took the pane *off* its tail whenever the bar was not + // on screen; one input since it was drawn, which stops a shallow absence clearing in the + // frame it appears in; and drawn long enough ago to have been read. + var held = TimeSpan.FromSeconds(Math.Max(0, _config.Text.ActivityBarSeconds)); + if (mark.InputSince && panel.AutoScroll && _time.GetUtcNow() - mark.DrawnAt >= held) +``` + +- [ ] **Step 6: Run the tests** + +```bash +dotnet run -c Release --project tests/SharpMUTerm.Tui.Tests --treenode-filter "/*/*/ActivityBarDwellTests/*" +Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN" +``` + +--- + +### Task 4: The frame, the brief, and the PR + +**Files:** +- Modify: `src/SharpMUTerm.Tui/SharpMUTermApp.cs` (`RenderSnapshot`'s view table) +- Modify: `CLAUDE.md` + +- [ ] **Step 1: Add the `activity-bar` view** + +In `RenderSnapshot`, beside the `away` view: + +```csharp + // The window absence, as against `away`'s terminal one: lines arrive in a background tab and + // the reader goes to it. The one frame that shows the NEW bar with the missed lines under it, + // and the only one that would catch the boundary landing at the wrong index. + if (string.Equals(view, "activity-bar", StringComparison.OrdinalIgnoreCase)) + { + LoadScene(); + foreach (var line in new[] + { + " Ana: anyone seen the vault key?", + " Bo: try the east store room", + " Ana: found it, thanks", + }) + { + SimulateLine("chat", line); + } + + Activate("chat"); + return RenderWholeFrame(); + } +``` + +Match the surrounding views' exact idiom for scene loading and returning a frame — copy the `away` +view's shape rather than the sketch above where they differ. + +- [ ] **Step 2: Render it and look at it** + +```bash +dotnet build SharpMUTerm.slnx +dotnet run -c Release --project src/SharpMUTerm.Tui --no-build -- \ + --snapshot --demo-config --view activity-bar --size 120x32 --out /tmp/activity.ansi +python3 tools/ansi_frame_to_image.py /tmp/activity.ansi /tmp/activity.html +``` + +Open the `.html` (not the `.svg` — Chromium clips a bare SVG's bottom). Expected: the Chat pane shows +`▾ NEW 3 lines since you were here ───` with the three lines under it. + +- [ ] **Step 3: Update the brief** + +In `CLAUDE.md`, the away-bar entry gains the generalisation — the two boundaries, which one wins, the +`IsCaughtUp` rule shared with the badge, the `_watching` gate, and the dwell floor with its F7 setting. +Add `activity-bar` to the snapshot views list. + +- [ ] **Step 4: Full verification** + +```bash +dotnet build SharpMUTerm.slnx +for p in Core Graphics Scripting Web Tui; do + printf "%-10s " "$p" + dotnet run -c Release --project tests/SharpMUTerm.$p.Tests &1 | grep -E "^ (total|failed):" | tr '\n' ' ' + echo +done +``` + +Expected: build clean and warning-free, five suites green. + +- [ ] **Step 5: Push, open the PR against `feat/find-chords`, and branch PR 3** + +```bash +git push -u origin feat/window-activity-boundary +gh pr create --base feat/find-chords --title "feat(activity): a boundary for the window you were not watching, and one that lasts" +git checkout -b feat/pane-search +``` + +## Self-review notes + +- **Spec coverage:** part 3 in full — the `IsCaughtUp` rule, the exact forward-recorded boundary, the + older-wins composition with the terminal absence, the two wordings, and the dwell floor with its + injected clock and F7 setting. +- **Not in scope:** `PaneMarks`, the shared mid-buffer bookkeeping extraction. The spec assigns it to + "whichever PR lands first", and after this one there is still exactly *one* kind of inserted chrome — + `MarkMissedLines` and `MarkWhereTheReaderLeft` share `RemoveAwayBar`/`RevealAwayBar`/`AwayMark` + already. The extraction earns itself in PR 3, where the search bar becomes a second kind; doing it + here would be a refactor with one caller. From f7d89341920336e3d18699a9605ce0b30ff467a6 Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Tue, 11 Aug 2026 15:26:52 -0500 Subject: [PATCH 07/19] =?UTF-8?q?docs(plan):=20PR=203=20of=20the=20stack?= =?UTF-8?q?=20=E2=80=94=20=E2=8C=83F=20search=20across=20the=20panes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN --- .../plans/2026-08-11-pane-search.md | 235 ++++++++++++++++++ 1 file changed, 235 insertions(+) create mode 100644 docs/superpowers/plans/2026-08-11-pane-search.md diff --git a/docs/superpowers/plans/2026-08-11-pane-search.md b/docs/superpowers/plans/2026-08-11-pane-search.md new file mode 100644 index 0000000..a04f407 --- /dev/null +++ b/docs/superpowers/plans/2026-08-11-pane-search.md @@ -0,0 +1,235 @@ +# PR 3 — `⌃F` search across the panes Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan +> task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** `⌃F` opens a modal results surface over the panes; typing filters the output the client is +holding, `⌥E` switches to regex, `⌥A` widens from the focused window to every window, `⏎` goes to a +hit and marks it with a bar, `⌥G` walks to the next one. + +**Architecture:** Matching is a pure Core function over plain text. The pane buffer carries that plain +text per line, computed once at append. The surface is the `HistorySearchPrompt`/`HistorySurface` split +verbatim — a pure class owning what a keystroke means and what the surface says, and a host owning +nothing but framework calls. Landing reuses the activity bar's machinery, which this PR finishes +extracting into two shared operations. + +**Tech Stack:** C# / .NET 10, SharpConsoleUI 2.5.14 (package only), TUnit. + +**Spec:** `docs/superpowers/specs/2026-08-11-pane-search-and-activity-design.md`, part 2. + +**Branch:** `feat/pane-search`, off `feat/window-activity-boundary`. Third of a three-PR stack. + +## Global Constraints + +- Target framework `net10.0`; file-scoped namespaces, 4-space C#, LF endings. +- `SharpMUTerm.Core` stays UI-agnostic — `OutputSearch` takes plain strings and knows nothing of panes. +- Run suites directly, never `dotnet test`, keep the ` Matches, string? Error); +public static class OutputSearch +{ + public const int MaxQueryLength = 200; + public static OutputSearchResult Match(IReadOnlyList lines, string query, bool regex); +} +``` + +**Decisions, all of which get a test:** + +- **Case-insensitive in both modes.** `HistorySearch` is (`OrdinalIgnoreCase`) and two search surfaces + in one client disagreeing about case would be a bug report. Regex mode says `(?-i)` inline when it + wants otherwise — a documented .NET feature rather than one we invented. +- **One match per line**, the first: the result is a list of *lines to go to*, and the offsets exist so + a row can show why it is listed. `HistorySearchPrompt.Row` does the same. +- **An empty query matches nothing**, unlike `HistorySearch`, where an empty query is the opening + chronological list. A pane buffer is thousands of lines and "everything, oldest first" is not a + result set anybody asked for; the surface says what to type instead. +- **An invalid pattern is a state.** `Error` carries the message, `Matches` is empty, nothing throws. A + regex is typed one character at a time, so most of the time a regex query is being typed it is + invalid. +- **A match timeout** (`TimeSpan.FromMilliseconds(100)`), and a timeout is an `Error` rather than an + exception: this runs on the UI thread on every keystroke, over every line of every window. +- **Order is oldest-first**, the buffer's own. The rows are a transcript, not a ranking, and the reader + is looking for a place in it. + +- [ ] **Step 1:** Write `OutputSearchTests` first — plain substring; case-insensitivity both ways; + `(?-i)` honoured in regex mode; a regex metacharacter treated *literally* in plain mode (`a.c` does + not match `abc`); empty query → no matches, no error; invalid pattern → empty + non-null `Error`; + `(a+)+$` against a long non-matching line → `Error` rather than a hang; over-long query rejected; + offsets point at the matched run; line indices are the caller's own. +- [ ] **Step 2:** Run: `dotnet run -c Release --project tests/SharpMUTerm.Core.Tests --treenode-filter "/*/*/OutputSearchTests/*" Render(IReadOnlyList rows, string query, string? error, bool regex, bool all, int held, int width = 0, int listRows = 0, int first = 0); + internal static int Scroll(int first, int selected, int count, int listRows); + internal static int MaxWidth(IReadOnlyList lines); +} +internal readonly record struct SearchRow(string WindowId, string WindowLabel, int LineIndex, string Text, int MatchStart, int MatchLength); +``` + +- `⌥E` and `⌥A` flip their flags and redraw. `⌃F` cancels (the toggle answers it in the running client; + spelt out here so a test can read the rule back). Escape cancels. `⏎` with nothing listed does + nothing and leaves the surface up — the query is what needs fixing. +- Printable characters filter; `⌫` un-filters; everything else is swallowed, because a modal that let + keys through would be typing into a command line the reader cannot see. +- The window column is drawn **only when `all`** — one window's results do not need a column saying + which window. +- The header carries the counted state and the searched bound: `12 of 38 · 4,812 lines held`. The bound + is on the frame rather than implied, because the search sees the pane buffer and not the session's + whole scrollback. +- An `Error` replaces the count with the message; the list is empty. +- **The footer names exactly the keys `Interpret` honours** — the honesty rule the settings screens and + the composer are held to, pinned by a test that presses every key it names and asserts each does + something. + +- [ ] **Step 1:** Write `SearchPromptTests` — one per rule above, plus the footer-honesty test. +- [ ] **Step 2:** Run, watch it fail. **Step 3:** Implement. **Step 4:** Re-run, PASS. **Step 5:** Commit. + +--- + +### Task 4: `SearchSurface`, the chord, and the scope + +**Files:** create `SearchSurface.cs`; modify `MacroKeys.cs`, `SharpMUTermApp.cs`; create +`tests/SharpMUTerm.Tui.Tests/SearchEndToEndTests.cs`. + +- `SearchSurface` is `HistorySurface` with a different prompt: same window construction, same + `PreviewKeyPressed` wiring, same `SimulateKey`/`SimulateTyping` seams, same size-once rule. +- It is handed **a function that returns the corpus** — for each searchable window, its id, its label + and its plain lines — read at the moment a key changes the query, so a hit's index is an index into + the buffer as it is *now*. +- `MacroKeys`: `new(ConsoleModifiers.Control, ConsoleKey.F, "searches the output")`, plus `⌥G` /`⌥⇧G`. + `ShortcutAction` gains the three arms. `⌃F` refuses over a settings screen or the composer and says + so (`ComposerIsInTheWay`), for the paste reason and because two modal `PreviewKeyPressed` handlers + cannot be driven headlessly. +- `AnyOverlayOpen` gains `_search.IsOpen`; `OpenOverlayName` gains "the search surface". +- Scope: focused window only, or every window in `_lines` except the web view (whose pane is not fed + from the buffer). The window label is `WindowTitle(id)`, `Snippet`-bounded — a window title can be a + *world's* text. + +- [ ] **Step 1:** `SearchEndToEndTests`: ⌃F opens it; typing lists hits from the focused window only; + `⌥A` brings in a background window's hits; `⌃F` again closes; ⌃F over an open settings screen refuses + and says so; nothing reaches the wire (a recording transport, `HistorySearchEndToEndTests`' shape). +- [ ] **Step 2:** Run, watch it fail. **Step 3:** Implement. **Step 4:** Whole Tui suite. **Step 5:** Commit. + +--- + +### Task 5: Landing — the bar, the jump, and `⌥G` + +**Files:** create `SearchBarRenderer.cs`; modify `SharpMUTermApp.cs`; extend `SearchEndToEndTests`. + +- `SearchBarRenderer.Bar(query, ordinal, total, accentHex)` → `⌕ goblin (12 of 38) ─── ⌥G next ──`, + fourth of the boundary bars, `Glyphs`-based, pure, unit-tested. The query is `MarkupText.Escape`d — + it is user text going into markup. +- **The chrome-row extraction the spec assigns to whichever PR needs it second.** There are now two + kinds of inserted row, so `InsertChromeRow(windowId, at, markup)` and `RemoveChromeRow(windowId, at)` + become the one place that fixes up everything indexing into a buffer: the freeze point, the pending + boundary, the away mark, and the search mark. `RemoveAwayBar` and the trim block call through them. +- `⏎` → `Activate(windowId)` (the one activation path — it selects the pane, raises the tab, adopts the + session), then insert the bar above the hit, `RepaintPane`, and reveal with the *measured* tail + height (`RevealAwayBar`'s arithmetic, generalised: a buffer index is not a viewport row). +- **One search bar client-wide**; `⌥G` moves it to the next hit, wrapping, and re-activates that + window. `⌥⇧G` goes back. With no search yet, both refuse out loud. +- **Not cleared by `Esc`.** A claimed Escape does not set `_escapeAt`, and `TryAltEnter` pairs an + unclaimed one with a following Enter to make `Alt+⏎`. Binding Escape here would break the newline + chord for as long as a search bar was on screen — a defect nobody would connect to search. It goes on + the next search, the next `⌃F`, or a trim that takes it. + +- [ ] **Step 1:** Tests — `⏎` on a hit in a *background* window activates that window (not the focused + one) and leaves the bar directly above the hit; the bar names the ordinal; `⌥G` moves it and wraps; + `⌥G` with no search refuses; a trim that passes the bar drops it; `Esc` in a pane leaves it alone and + `Alt+⏎` still makes a newline with a bar up. +- [ ] **Step 2:** Run, watch it fail. **Step 3:** Implement. **Step 4:** Whole Tui suite. **Step 5:** Commit. + +--- + +### Task 6: Frames, brief, PR + +- [ ] **Step 1:** Snapshot views `search`, `search-regex`, `search-all`, `search-landed` (the last over + a **split**, so the pane is narrower than the terminal — the geometry that catches a reveal landing at + the wrong row). Drive real keys through the surface's own handler, as `history-search-filter` does. +- [ ] **Step 2:** `dotnet build -c Release SharpMUTerm.slnx`, render each, look at the `.html`. +- [ ] **Step 3:** CLAUDE.md: the search entry (corpus, the plain-text field, the scheme-free bar, the + scope rule, the Escape reasoning), the new chords, and the four views. +- [ ] **Step 4:** Five suites green, build warning-free. +- [ ] **Step 5:** Push; `gh pr create --base feat/window-activity-boundary`. + +## Self-review notes + +- **Spec coverage:** part 2 in full — corpus and its bound, plain text at append, the Core matcher and + its five decisions, the surface and its footer, the scope toggle, the landing bar, `⌥G`. +- **Deliberately not here:** searching the file-backed spill or a session's `WorldSession.Scrollback`. + The spec rules both out — a spawn window's lines exist in neither — and the surface states the bound + it does search rather than implying a bigger one. From 242bb3be2aed12c6e29aec6f473eaeb766ca5a48 Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Tue, 11 Aug 2026 15:05:57 -0500 Subject: [PATCH 08/19] feat(activity): a bar for the window you were not watching MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Beside the terminal-absence bar rather than replacing it: AWAY is about the reader, NEW is about the window, and a reader who meets both in one client should be able to tell them apart without counting the lines. No duration on this one. The terminal bar's span is measured from the last input before the reader vanished — approximate, but an instant they were part of. This boundary is made when a line lands in a window nobody is watching, so a span on it would be timing the output rather than the absence. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN --- src/SharpMUTerm.Tui/AwayBarRenderer.cs | 28 +++++++++++++ .../AwayBarRendererTests.cs | 39 +++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/src/SharpMUTerm.Tui/AwayBarRenderer.cs b/src/SharpMUTerm.Tui/AwayBarRenderer.cs index 7d7dd78..a8aa943 100644 --- a/src/SharpMUTerm.Tui/AwayBarRenderer.cs +++ b/src/SharpMUTerm.Tui/AwayBarRenderer.cs @@ -28,6 +28,14 @@ internal static class AwayBarRenderer /// The label, kept as a constant so a test can look for the exact words a reader will see. internal const string Label = "AWAY"; + /// + /// The label on the bar marking a window the reader was not watching, as against + /// 's terminal absence. Two words rather than one bar with two meanings: AWAY is + /// about the reader, NEW is about the window, and a reader who meets both in one client should be + /// able to tell which absence they are looking at without counting the lines. + /// + internal const string MissedLabel = "NEW"; + /// /// The bar for lines that arrived over an absence of , /// on an already-resolved #rrggbb accent. @@ -46,6 +54,26 @@ public static string Bar(int lines, TimeSpan away, string accentHex) + $"[dim]{MarkupText.Escape($"{count} since you left · {Duration(away)}")} {rule}[/]"; } + /// + /// The bar for lines that arrived in a window while the reader was not + /// watching it, on an already-resolved #rrggbb accent. + /// + /// It carries a count and no duration, which is the one way it differs from . The + /// terminal bar's span is measured from the last input before the reader vanished — approximate, but + /// an instant they were part of. This boundary is made at the moment a line lands in a window nobody + /// is watching, so a duration on it would be timing the output rather than the absence. + /// + /// + public static string Missed(int lines, string accentHex) + { + ArgumentException.ThrowIfNullOrEmpty(accentHex); + + var count = lines == 1 ? "1 line" : $"{lines} lines"; + var rule = new string('─', RuleCells); + return $"[{accentHex}]{Glyphs.Away} {MissedLabel}[/] " + + $"[dim]{MarkupText.Escape($"{count} since you were here")} {rule}[/]"; + } + /// /// An absence in the coarsest unit that still says something useful. Deliberately not seconds past /// the first minute and not minutes past the first day: the number is read at a glance to decide how diff --git a/tests/SharpMUTerm.Tui.Tests/AwayBarRendererTests.cs b/tests/SharpMUTerm.Tui.Tests/AwayBarRendererTests.cs index 265e62d..31625e5 100644 --- a/tests/SharpMUTerm.Tui.Tests/AwayBarRendererTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/AwayBarRendererTests.cs @@ -34,6 +34,45 @@ public void Bar_RejectsAnEmptyAccent() Assert.Throws(() => AwayBarRenderer.Bar(4, TimeSpan.FromMinutes(2), string.Empty)); } + /// + /// The window-absence bar says what it can and no more. There is no duration on it: the terminal + /// bar measures from the last input before the reader vanished, which is a real (if approximate) + /// instant, while this one is made when a line lands in a window nobody is watching — a moment the + /// reader was not part of. A "3 min" on it would be timing the *output*, not the absence. + /// + [Test] + public async Task Missed_CountsTheLinesAndClaimsNoDuration() + { + var bar = AwayBarRenderer.Missed(47, "#c678dd"); + + await Assert.That(bar).Contains($"[#c678dd]{Glyphs.Away} {AwayBarRenderer.MissedLabel}[/]"); + await Assert.That(bar).Contains("47 lines since you were here"); + await Assert.That(bar).DoesNotContain("min"); + await Assert.That(bar).Contains("[dim]"); + } + + [Test] + public async Task Missed_CountsOneLineInTheSingular() + { + await Assert.That(AwayBarRenderer.Missed(1, "#c678dd")).Contains("1 line since you were here"); + } + + /// + /// The two labels are different words, so a reader who sees both in one client can tell which + /// absence they are looking at without counting the lines. + /// + [Test] + public async Task TheTwoAbsencesDoNotWearTheSameWord() + { + await Assert.That(AwayBarRenderer.MissedLabel).IsNotEqualTo(AwayBarRenderer.Label); + } + + [Test] + public void Missed_RejectsAnEmptyAccent() + { + Assert.Throws(() => AwayBarRenderer.Missed(4, string.Empty)); + } + /// /// The coarsest unit that still decides something. The anchor is the last input event rather than /// the moment of departure — focus-out is not observable — so a sub-minute gap must not be dressed From f632dc6f006585cd2d5973abbe9807c9436b9e13 Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Tue, 11 Aug 2026 15:28:39 -0500 Subject: [PATCH 09/19] =?UTF-8?q?feat(search):=20OutputSearch=20=E2=80=94?= =?UTF-8?q?=20what=20=E2=8C=83F=20matches,=20in=20Core?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Plain text in, matching lines out: nothing here knows about panes, markup or windows, which is what keeps it in Core and what makes a match mean what it looks like. A colour tag mid-word must not split a match, and nobody should be able to search for #ff0000 and find every red line. Five decisions, each pinned: case ignored in both modes (HistorySearch already is, and (?-i) is the way back, which is why there is no third toggle); one match per line, the first, with offsets so a row can show why it is listed; an empty query matches nothing, unlike history, because a pane buffer is thousands of lines and "everything, oldest first" is the pane you are already looking at; an invalid pattern is a state, because a regex is unparseable most of the time it is being typed; and a match timeout, because this runs on the UI thread on every keystroke over every line of every window. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN --- src/SharpMUTerm.Core/Text/OutputSearch.cs | 149 +++++++++++++++ .../OutputSearchTests.cs | 171 ++++++++++++++++++ 2 files changed, 320 insertions(+) create mode 100644 src/SharpMUTerm.Core/Text/OutputSearch.cs create mode 100644 tests/SharpMUTerm.Core.Tests/OutputSearchTests.cs diff --git a/src/SharpMUTerm.Core/Text/OutputSearch.cs b/src/SharpMUTerm.Core/Text/OutputSearch.cs new file mode 100644 index 0000000..f72ec68 --- /dev/null +++ b/src/SharpMUTerm.Core/Text/OutputSearch.cs @@ -0,0 +1,149 @@ +using System.Text.RegularExpressions; + +namespace SharpMUTerm.Core.Text; + +/// +/// One line the search surface lists: where it sits in the buffer it came from, its text, and where the +/// query landed inside it. +/// +/// Its index in the list handed to . +/// The line, verbatim. +/// Where the query matched. +/// How long the matched run is. +public readonly record struct OutputMatch(int LineIndex, string Text, int MatchStart, int MatchLength); + +/// +/// What one search came to: the lines it found, and — when the query could not be used at all — why. +/// +/// The matching lines, in the buffer's own order. +/// Why nothing could be searched for, or null when the query was usable. +public readonly record struct OutputSearchResult(IReadOnlyList Matches, string? Error); + +/// +/// Finds a query in a window's output. The matching half of ⌃F, and the whole of it that can be +/// reasoned about without a terminal. +/// +/// It takes plain text. Nothing here knows about panes, markup or windows: the caller strips its +/// own lines and hands over strings. That is what keeps this in Core, and it is also the rule that makes +/// a match mean what it looks like — a colour tag in the middle of a word must not split a match, and a +/// reader must not be able to search for #ff0000 and find every red line. +/// +/// +/// Case is ignored, in both modes. ignores it too, and two +/// search surfaces in one client disagreeing about case is a bug report waiting to happen. Regex mode +/// says (?-i) inline when it wants otherwise, which is a documented .NET feature rather than one +/// invented here — and is why there is no third toggle on the surface. +/// +/// +/// One match per line, the first. The result is a list of lines to go to; the offsets +/// exist so a row can show why it is listed, exactly as HistorySearchPrompt.Row uses them. +/// +/// +/// An empty query matches nothing, which is where this parts company with +/// : there an empty query is the opening chronological list, and a +/// command history is short enough to be one. A pane buffer is thousands of lines, and "everything, +/// oldest first" is not a result set anybody asked for — it is the pane they are already looking at. +/// +/// +public static class OutputSearch +{ + /// + /// The longest query this will look for. A search box is not a text editor, and an unbounded query + /// is an unbounded pattern compiled on every keystroke. + /// + public const int MaxQueryLength = 200; + + /// + /// How long one regex is given against one line. This runs on the UI thread, on every keystroke, + /// over every line of every window — so a pattern that backtracks catastrophically has to come back + /// as an error rather than wedge the client. + /// + private static readonly TimeSpan MatchTimeout = TimeSpan.FromMilliseconds(100); + + /// + /// The lines of matching , in the order they were + /// given — the buffer's own, because these rows are a transcript rather than a ranking and the reader + /// is looking for a place in it. + /// + /// The plain text of each line, oldest first. + /// What to look for; empty finds nothing. + /// Whether is a pattern rather than literal text. + public static OutputSearchResult Match(IReadOnlyList lines, string query, bool regex) + { + ArgumentNullException.ThrowIfNull(lines); + ArgumentNullException.ThrowIfNull(query); + + if (query.Length == 0) + { + return new OutputSearchResult(Array.Empty(), null); + } + + if (query.Length > MaxQueryLength) + { + return new OutputSearchResult( + Array.Empty(), $"query is longer than {MaxQueryLength} characters"); + } + + return regex ? ByPattern(lines, query) : ByText(lines, query); + } + + private static OutputSearchResult ByText(IReadOnlyList lines, string query) + { + var matches = new List(); + for (var index = 0; index < lines.Count; index++) + { + var text = lines[index] ?? string.Empty; + var at = text.IndexOf(query, StringComparison.OrdinalIgnoreCase); + if (at >= 0) + { + matches.Add(new OutputMatch(index, text, at, query.Length)); + } + } + + return new OutputSearchResult(matches, null); + } + + private static OutputSearchResult ByPattern(IReadOnlyList lines, string query) + { + Regex pattern; + try + { + pattern = new Regex(query, RegexOptions.IgnoreCase | RegexOptions.CultureInvariant, MatchTimeout); + } + catch (ArgumentException error) + { + // The whole of what "invalid pattern" means here. A regex is typed one character at a time, + // so most of the time a regex query is being typed it is unparseable; the surface says so + // and lists nothing, which is a state rather than a failure. + return new OutputSearchResult(Array.Empty(), error.Message); + } + + var matches = new List(); + for (var index = 0; index < lines.Count; index++) + { + var text = lines[index] ?? string.Empty; + try + { + var found = pattern.Match(text); + + // A zero-width match marks nothing, so it lists nothing: `x*` matches at position 0 of + // every line in the buffer, and a result set of "everything, highlighted nowhere" is + // worse than no result at all. + if (found.Success && found.Length > 0) + { + matches.Add(new OutputMatch(index, text, found.Index, found.Length)); + } + } + catch (RegexMatchTimeoutException) + { + // Reported against the whole search rather than skipping the line: a pattern that can + // take this long on one line will take it on the next, and a partial result set that + // silently omitted the expensive lines would be a search lying about what it found. + return new OutputSearchResult( + Array.Empty(), "pattern took too long — try a simpler one"); + } + } + + return new OutputSearchResult(matches, null); + } +} diff --git a/tests/SharpMUTerm.Core.Tests/OutputSearchTests.cs b/tests/SharpMUTerm.Core.Tests/OutputSearchTests.cs new file mode 100644 index 0000000..b1e3cc8 --- /dev/null +++ b/tests/SharpMUTerm.Core.Tests/OutputSearchTests.cs @@ -0,0 +1,171 @@ +using SharpMUTerm.Core.Text; + +namespace SharpMUTerm.Core.Tests; + +/// +/// What ⌃F matches. The rules are few and each one is a decision the surface above it depends on, so +/// they are pinned here rather than inferred from the surface's behaviour. +/// +public class OutputSearchTests +{ + private static readonly string[] Lines = + { + "The goblin snarls at you.", + " Ana: Goblin room is bugged", + "You hit the goblin for 12 damage.", + "A town guard stands watch by the northern gate.", + }; + + [Test] + public async Task APlainQueryFindsEveryLineHoldingIt() + { + var result = OutputSearch.Match(Lines, "goblin", regex: false); + + await Assert.That(result.Error).IsNull(); + await Assert.That(result.Matches.Select(m => m.LineIndex)).IsEquivalentTo(new[] { 0, 1, 2 }); + } + + /// + /// Case-insensitive, which is what HistorySearch does — two search surfaces in one client + /// disagreeing about case is a bug report waiting to happen. + /// + [Test] + public async Task CaseIsIgnoredInBothModes() + { + await Assert.That(OutputSearch.Match(Lines, "GOBLIN", regex: false).Matches.Count).IsEqualTo(3); + await Assert.That(OutputSearch.Match(Lines, "GOBLIN", regex: true).Matches.Count).IsEqualTo(3); + } + + /// + /// The way back for a reader who wants case to matter: an inline option, which is a documented .NET + /// feature rather than one this client invented. It is why there is no third toggle on the surface. + /// + [Test] + public async Task RegexModeHonoursAnInlineCaseOption() + { + var result = OutputSearch.Match(Lines, "(?-i)Goblin", regex: true); + + await Assert.That(result.Matches.Select(m => m.LineIndex)).IsEquivalentTo(new[] { 1 }); + } + + /// + /// Plain mode is plain: a query full of metacharacters is text, not a pattern. Anything else and a + /// reader searching for $5.00 or (OOC) gets a silent misfire or an error they did not + /// ask for. + /// + [Test] + public async Task PlainModeTreatsMetacharactersLiterally() + { + var lines = new[] { "abc", "a.c" }; + + var result = OutputSearch.Match(lines, "a.c", regex: false); + + await Assert.That(result.Matches.Select(m => m.LineIndex)).IsEquivalentTo(new[] { 1 }); + } + + [Test] + public async Task RegexModeMatchesAsAPattern() + { + var result = OutputSearch.Match(Lines, @"\d+ damage", regex: true); + + await Assert.That(result.Matches.Select(m => m.LineIndex)).IsEquivalentTo(new[] { 2 }); + } + + /// + /// An empty query matches nothing, which is where this parts company with HistorySearch: there + /// an empty query is the opening chronological list, and a command history is short. A pane buffer is + /// thousands of lines, and "everything, oldest first" is not a result set anybody asked for. + /// + [Test] + public async Task AnEmptyQueryMatchesNothingAndIsNotAnError() + { + var result = OutputSearch.Match(Lines, string.Empty, regex: false); + + await Assert.That(result.Matches).IsEmpty(); + await Assert.That(result.Error).IsNull(); + } + + /// + /// An invalid pattern is a state, not an exception. A regex is typed one character at a time, so + /// most of the time a regex query is being typed it is invalid — throwing, or listing stale results, + /// are both worse than saying so. + /// + [Test] + public async Task AnInvalidPatternIsReportedRatherThanThrown() + { + var result = OutputSearch.Match(Lines, "goblin(", regex: true); + + await Assert.That(result.Error).IsNotNull(); + await Assert.That(result.Matches).IsEmpty(); + } + + /// + /// The same characters in plain mode are a query, not a pattern, so they cannot be invalid. + /// + [Test] + public async Task ThatSameQueryIsFineInPlainMode() + { + await Assert.That(OutputSearch.Match(new[] { "goblin(x)" }, "goblin(", regex: false).Error).IsNull(); + } + + /// + /// This runs on the UI thread, on every keystroke, over every line of every window. A pattern that + /// backtracks catastrophically must come back as an error rather than wedge the client. + /// + [Test] + public async Task ARunawayPatternTimesOutIntoAnError() + { + var lines = new[] { new string('a', 4000) + "b" }; + + var result = OutputSearch.Match(lines, "(a+)+$", regex: true); + + await Assert.That(result.Error).IsNotNull(); + await Assert.That(result.Matches).IsEmpty(); + } + + [Test] + public async Task AnOverLongQueryIsRefused() + { + var result = OutputSearch.Match(Lines, new string('x', OutputSearch.MaxQueryLength + 1), regex: false); + + await Assert.That(result.Error).IsNotNull(); + await Assert.That(result.Matches).IsEmpty(); + } + + /// + /// One match per line, the first, and its offsets — the result is a list of lines to go to, + /// and the offsets are there so a row can show why it is listed. + /// + [Test] + public async Task AMatchCarriesTheLineAndWhereTheQueryLandedInIt() + { + var result = OutputSearch.Match(new[] { "You hit the goblin, and the goblin falls." }, "goblin", regex: false); + + var match = result.Matches.Single(); + await Assert.That(match.Text).IsEqualTo("You hit the goblin, and the goblin falls."); + await Assert.That(match.MatchStart).IsEqualTo(12); + await Assert.That(match.MatchLength).IsEqualTo(6); + } + + /// + /// Oldest first, the buffer's own order. The rows are a transcript rather than a ranking, and the + /// reader is looking for a place in it. + /// + [Test] + public async Task MatchesKeepTheBuffersOwnOrder() + { + var result = OutputSearch.Match(Lines, "the", regex: false); + + await Assert.That(result.Matches.Select(m => m.LineIndex).ToArray()) + .IsEquivalentTo(result.Matches.Select(m => m.LineIndex).OrderBy(i => i).ToArray()); + } + + /// A zero-width regex match must not produce a row claiming to mark nothing. + [Test] + public async Task AZeroWidthPatternMatchesNoLines() + { + var result = OutputSearch.Match(Lines, "x*", regex: true); + + await Assert.That(result.Matches).IsEmpty(); + } +} From 246257445dc9366b798e37208d2e6215cdccafdd Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Tue, 11 Aug 2026 15:13:48 -0500 Subject: [PATCH 10/19] feat(activity): mark where you left off in any window, not just the terminal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A window accrues a boundary from the first line that lands while it is not caught up — visible *and* at its live tail, the rule the unread badge already answers to — so a badge showing a count always has a bar under it saying where the count begins. The badge said 3 and nothing said which 3; that was the report. Recorded forwards, unlike the terminal boundary beside it, which has to be reconstructed from the input before the last because focus-out is unobservable. Where both exist the older wins: a reader who typed after lines landed in a window they could not see has a terminal boundary at the end of that buffer, saying they missed nothing, and the window's own boundary knows better. The reveal is only for *arriving* at a window. Coming back to the tail by ⌃End is an explicit "take me to the live tail", and a client that answered it by scrolling somewhere else would move the reader off the place they just asked for — CtrlEndGoesBackToFollowingTheLiveTail is the test that says so. The pane is re-pinned after the insert instead, because a whole-buffer re-feed leaves the offset a frame behind and the newest line would blink off the screen. Nothing accrues before the constructor finishes: until the workspace is laid out "not visible" means "no pane built yet", and the restore replay pours a previous session through the same seam into windows already under a restore bar. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN --- src/SharpMUTerm.Tui/SharpMUTermApp.cs | 162 +++++++++++++++++- .../WindowActivityBoundaryTests.cs | 148 ++++++++++++++++ 2 files changed, 309 insertions(+), 1 deletion(-) create mode 100644 tests/SharpMUTerm.Tui.Tests/WindowActivityBoundaryTests.cs diff --git a/src/SharpMUTerm.Tui/SharpMUTermApp.cs b/src/SharpMUTerm.Tui/SharpMUTermApp.cs index 261dc3d..1936cce 100644 --- a/src/SharpMUTerm.Tui/SharpMUTermApp.cs +++ b/src/SharpMUTerm.Tui/SharpMUTermApp.cs @@ -374,6 +374,38 @@ private sealed class SizeReport /// private readonly Dictionary _awayMarks = new(StringComparer.Ordinal); + /// + /// Where the lines the reader has not seen begin, per window, for the windows that have any: the + /// buffer index of the first line that landed while the window was not caught up. + /// + /// Recorded forwards, unlike beside it. The terminal boundary has + /// to be reconstructed from the last input before the reader vanished, because a terminal reports + /// focus-in and this client cannot see focus-out at all. This one needs none of that + /// machinery: a line arrives, the window either is or is not caught up, and if it is not then this is + /// the boundary. Exact rather than approximate, and one comparison in a method that already runs per + /// line. + /// + /// + /// and not IsVisible, which is already the rule the unread + /// badge answers to (): a visible tab whose output the reader has scrolled back + /// off is exactly as blind as a tab they are not looking at. One fact behind both, so a badge showing + /// a count always has a bar under it saying where the count begins — the badge said 3 and nothing + /// said which 3, which is what got reported. + /// + /// + private readonly Dictionary _missedFrom = new(StringComparer.Ordinal); + + /// + /// Whether a line landing out of sight is an absence yet. False until the constructor has + /// finished, for two reasons that are really the same one: until the workspace has been laid out, + /// "not visible" means "no pane has been built", which is nothing the reader missed; and + /// pours a previous run's lines through + /// into windows that are not visible yet. Every one of those already + /// sits under a bar saying exactly what it is, and a second bar over + /// the top would be the client marking its own startup as news. + /// + private bool _watching; + /// How the configuration is written back, or null for an app that owns no file. private readonly Action? _save; @@ -711,6 +743,11 @@ public SharpMUTermApp( // also the last moment at which no live line has arrived to be restored *above*. RestorePreviousSession(); _system.AddWindow(_window); + + // From here on, a line landing in a window the reader cannot see is something they missed. Not + // before: see _watching — the restore replay above goes through the same seam, into windows that + // have no pane yet. + _watching = true; } /// Captures the current workspace (panes/windows/focus) so it can be persisted and resumed. @@ -2343,6 +2380,15 @@ private void AppendWindowLine(string windowId, string markup, string? stamp = nu _awayBoundary[windowId] = 0; } + // The moment a boundary is made: a line landing in a window the reader is not watching. Before + // the Add, so the index is the first line they missed rather than the one after it — and only + // the first such line records it, because every one after that is on the same side of the same + // boundary. + if (_watching && !_missedFrom.ContainsKey(windowId) && !_workspace.IsCaughtUp(windowId)) + { + _missedFrom[windowId] = buffer.Count; + } + buffer.Add(new PaneLine(markup, stamp)); @@ -2373,6 +2419,11 @@ private void AppendWindowLine(string windowId, string markup, string? stamp = nu _awayBoundary[windowId] = Math.Max(0, boundary - excess); } + if (_missedFrom.TryGetValue(windowId, out var missed)) + { + _missedFrom[windowId] = Math.Max(0, missed - excess); + } + if (_awayMarks.TryGetValue(windowId, out var mark)) { mark.Index -= excess; @@ -2624,7 +2675,21 @@ private void MarkWhereTheReaderLeft(TimeSpan away) var removed = RemoveAwayBar(windowId); var buffer = _lines[windowId]; - var at = Math.Clamp(_awayBoundary.GetValueOrDefault(windowId), 0, buffer.Count); + + // Two boundaries can exist for one window, and the *older* wins. This one is reconstructed + // from the input before the last — so a reader who typed after lines landed in a window they + // could not see has a terminal boundary at the end of it, saying they missed nothing. The + // window's own boundary was recorded when the first of those lines arrived and knows better. + // Taking this one unconditionally moved the bar down past the very lines it was made for, or + // dropped it entirely; still one bar per window, marking the earlier of the two absences. + var boundary = _awayBoundary.GetValueOrDefault(windowId); + if (_missedFrom.TryGetValue(windowId, out var missedFrom)) + { + boundary = Math.Min(boundary, missedFrom); + _missedFrom.Remove(windowId); + } + + var at = Math.Clamp(boundary, 0, buffer.Count); var missed = buffer.Count - at; if (missed <= 0) { @@ -2662,6 +2727,90 @@ private void MarkWhereTheReaderLeft(TimeSpan away) } } + /// + /// Draws the boundary in every window that has missed lines and is caught up again — the reader has + /// come back to it, so this is the moment to say where they left off. + /// + /// Called from the two places a window can become caught up: , which + /// raises a tab and focuses its pane, and , where every scroll route + /// lands — the keys, the wheel and the scrollbar alike. There is no third, because + /// is visibility and scroll position and nothing else moves either. + /// + /// + /// The bar is the client's own chrome, so it goes in through the buffer rather than through + /// : it must not badge the window unread, and it must not reach the + /// restore log. Re-entrancy is not a risk — the pending entry is dropped before the reveal, and the + /// reveal's scroll comes back through with nothing left pending. + /// + /// + /// is false when the reader's own scroll is what caught the window up, + /// and that is not a detail. Arriving at a window is a jump: the pane bottom-anchors, so a bar + /// for a deep absence is drawn far above the fold and nothing on screen would change without a + /// scroll to it. Pressing ⌃End is the opposite — an explicit "take me to the live tail" — and a + /// client that answered it by scrolling somewhere else would be moving the reader's attention away + /// from the place they just asked for. The bar is still drawn either way; only the jump is withheld. + /// + /// + /// Whether to scroll each pane so its new bar is on screen. + private void MarkMissedLines(bool reveal) + { + foreach (var windowId in _missedFrom.Keys.ToArray()) + { + // The web view's pane is not fed from the line buffer, so there is nothing there to mark. + if (!_workspace.IsCaughtUp(windowId) + || string.Equals(windowId, WebWindowId, StringComparison.Ordinal) + || !_lines.TryGetValue(windowId, out var buffer)) + { + continue; + } + + var at = Math.Clamp(_missedFrom[windowId], 0, buffer.Count); + _missedFrom.Remove(windowId); + if (buffer.Count - at <= 0) + { + continue; + } + + // At most one bar per window, so a previous one goes first — and first rather than last, + // because removing it shifts every index after it, this boundary included. + if (RemoveAwayBar(windowId)) + { + at = Math.Clamp(at, 0, buffer.Count); + } + + buffer.Insert(at, new PaneLine(AwayBarRenderer.Missed(buffer.Count - at, FrozenAccentHex()))); + if (_freezePoints.TryGetValue(windowId, out var freeze) && freeze > at) + { + _freezePoints[windowId] = freeze + 1; + } + + var mark = new AwayMark + { + Index = at, + DrawnAfter = _focus.InputCount, + }; + + _awayMarks[windowId] = mark; + RepaintPane(windowId); + if (reveal) + { + RevealAwayBar(windowId, mark); + continue; + } + + // No jump — but the pane still gained a row, and a whole-buffer re-feed leaves the panel's + // offset a frame behind (auto-scroll re-pins during paint, after the children were arranged). + // On the frame in between, a pane that is following its tail would be showing the row above + // its newest line, which is a reader pressing ⌃End and watching the newest line leave the + // screen. Re-pinning here is the same pairing BackToLive makes, and touches AutoScroll not + // at all: ScrollToBottom is a bare offset write. + if (_paneScrolls.GetValueOrDefault(windowId) is { AutoScroll: true } following) + { + following.ScrollToBottom(); + } + } + } + /// /// Puts a freshly drawn away bar on screen, by scrolling its pane so the bar sits at the top of the /// viewport and the first line the reader has not seen is directly under it. @@ -4296,6 +4445,12 @@ private void SyncScrollbackState(string? windowId = null) // A viewport that moved is the gesture an away bar is read by, so this is where "has it been on // screen, and are we back at the tail" gets asked. Every scroll route reaches here — the keys, // the wheel and the scrollbar alike. + // + // Coming back down to the tail is also how a window stops being one the reader is missing, so the + // boundary is drawn here before it is consumed: a pane scrolled back while output arrived has + // both things to do, in that order, and the bar it has just been given is not one it has read. + // Without the reveal — the reader is here because they scrolled here. + MarkMissedLines(reveal: false); ConsumeReadAwayBars(); RefreshStatusRow(); } @@ -9062,6 +9217,11 @@ private bool Activate(string id) _activating = true; try { + // Before the syncs, and with the reveal: the window has just become caught up (ActivateWindow + // above raised its tab and focused its pane), so this is the moment its boundary is drawn — + // and arriving at a window is exactly the case that needs the jump, because a pane + // bottom-anchors and a deep absence's bar is drawn far above the fold. + MarkMissedLines(reveal: true); AdoptSessionOf(id); SelectTabFor(id); SyncToFocusedPane(); diff --git a/tests/SharpMUTerm.Tui.Tests/WindowActivityBoundaryTests.cs b/tests/SharpMUTerm.Tui.Tests/WindowActivityBoundaryTests.cs new file mode 100644 index 0000000..e4b4215 --- /dev/null +++ b/tests/SharpMUTerm.Tui.Tests/WindowActivityBoundaryTests.cs @@ -0,0 +1,148 @@ +using SharpConsoleUI.Drivers; +using SharpMUTerm.Core.Session; +using SharpMUTerm.Graphics; +using SharpMUTerm.Tui; + +namespace SharpMUTerm.Tui.Tests; + +/// +/// The boundary marking a window the reader was not watching — the generalisation of +/// ' terminal absence to the one that happens many times an hour: a tab +/// behind another tab, or a pane you have scrolled back in. +/// +/// It answers to Workspace.IsCaughtUp, which is the rule the unread badge already uses, and that +/// is the point rather than a coincidence: a badge showing a count must always have a bar under it +/// saying where the count begins. The badge said 3 and nothing said which 3 — that was the report. +/// +/// +/// +/// Serialised for the reason every file that renders a frame is: rendering redirects the process-global +/// Console.Out, and the harness redirects Console.In. +/// +[NotInParallel] +public class WindowActivityBoundaryTests +{ + private const int Width = 120; + private const int Height = 32; + + /// The window the demo scene's own output lands in. + private const string Main = "main"; + + private static readonly TerminalCapabilities Headless = + new(GraphicsProtocol.None, supportsTrueColor: true, supportsKittyGraphics: false, supportsSixel: false); + + /// + /// The demo configuration with the scrollback spill off — these tests print more than a session's + /// in-memory ring holds, and a spilling session writes into the developer's own cache directory, + /// which UserDirectoryGuard fails the run for. + /// + private static (SharpMUTermApp App, WorldSession Session) Bound() + { + Console.SetIn(TextReader.Null); + var config = DemoScene.Build(); + config.ScrollbackSpill.Enabled = false; + var app = new SharpMUTermApp(config, Headless, new HeadlessConsoleDriver(Width, Height)); + var session = app.BindWorldWithoutConnecting(config.Worlds[0]); + app.RenderSnapshot(); + return (app, session); + } + + private static ConsoleKeyInfo Key(ConsoleKey key, bool ctrl = false) => new('\0', key, false, false, ctrl); + + /// + /// The reported case. Lines land in a window sitting behind another tab; going to it puts the reader + /// on a boundary with exactly those lines under it. + /// + [Test] + public async Task AWindowBehindAnotherTabAccruesABoundaryAndShowsItOnReturn() + { + var (app, session) = Bound(); + app.SimulateWindowChange(DemoScene.ChatWindowId); + + session.PrintSystem("*** while you were reading Chat"); + session.PrintSystem("*** and again"); + await Assert.That(app.AwayBarIndex(Main)).IsNull(); + + app.SimulateWindowChange(Main); + + var index = app.AwayBarIndex(Main); + await Assert.That(index).IsNotNull(); + + var rows = app.PaneLines(Main); + await Assert.That(rows[index!.Value]).Contains(AwayBarRenderer.MissedLabel); + await Assert.That(rows[index.Value]).Contains("2 lines since you were here"); + + // The bar marks the boundary, so what was missed is under it and what was already read is above. + await Assert.That(rows[index.Value + 1]).Contains("while you were reading Chat"); + } + + /// + /// The window in front of the reader, at its live tail, accrues nothing: those lines went past their + /// eyes. The same rule the unread badge answers to, which is why the two cannot disagree. + /// + [Test] + public async Task TheWindowInFrontOfTheReaderGetsNoBar() + { + var (app, session) = Bound(); + + session.PrintSystem("*** the goblin snarls at you"); + app.RenderSnapshot(); + + await Assert.That(app.AwayBarIndex(Main)).IsNull(); + await Assert.That(app.PaneLines(Main).Any(row => row.Contains("since you were here"))).IsFalse(); + } + + /// + /// A window the reader is looking at but has scrolled back off is exactly as blind as one they are + /// not looking at — OnLine's reasoning for the badge, one field over — so it accrues, and the + /// bar is there when they come back down to the tail. + /// + [Test] + public async Task AVisibleWindowScrolledBackAccruesAndTheBarIsThereOnTheWayBackDown() + { + var (app, session) = Bound(); + for (var i = 1; i <= 80; i++) + { + session.PrintSystem($"*** line {i}"); + } + + app.RenderSnapshot(); + app.SimulateScrollKey(Key(ConsoleKey.PageUp)); + app.RenderSnapshot(); + + session.PrintSystem("*** arrived while you were reading back"); + await Assert.That(app.AwayBarIndex(Main)).IsNull(); + + app.SimulateScrollKey(Key(ConsoleKey.End, ctrl: true)); + + var index = app.AwayBarIndex(Main); + await Assert.That(index).IsNotNull(); + await Assert.That(app.PaneLines(Main)[index!.Value]).Contains(AwayBarRenderer.MissedLabel); + } + + /// + /// Two boundaries can exist for one window, and the older wins. A window that had already missed + /// lines before the reader left the terminal must not have its bar moved down to where they + /// left: that would hide the very lines it was made for. + /// + [Test] + public async Task TheOlderOfTheTwoBoundariesWins() + { + var (app, session) = Bound(); + app.SimulateWindowChange(DemoScene.ChatWindowId); + + session.PrintSystem("*** missed while the tab was behind Chat"); + + // Two keystrokes with a line between them, so the terminal absence's own boundary — which is + // reconstructed from the input before the last one — lands *after* the window's. + app.SimulateKey(Key(ConsoleKey.End)); + session.PrintSystem("*** and this one too"); + app.SimulateKey(Key(ConsoleKey.End)); + + app.SimulateReturnFromAway(TimeSpan.FromMinutes(12)); + + var index = app.AwayBarIndex(Main); + await Assert.That(index).IsNotNull(); + await Assert.That(app.PaneLines(Main)[index!.Value + 1]).Contains("missed while the tab was behind Chat"); + } +} From b56b42aab284bf6d45016391e23a9958f4c3de05 Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Tue, 11 Aug 2026 15:30:51 -0500 Subject: [PATCH 11/19] =?UTF-8?q?feat(search):=20the=20pane=20buffer=20car?= =?UTF-8?q?ries=20the=20text=20=E2=8C=83F=20will=20search?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MarkupText.Plain is what a markup line actually puts on the screen, and PaneLine holds it beside the markup, computed once at append. On demand would restrip every line of every window on every keystroke of a query, and an incremental surface is the whole point. Plain rather than markup because a match has to mean what it looks like: a world may change colour mid-word, and matching the markup would find neither half — the same defect a URL split by a colour change has, one layer down — while a query for #ff0000 must not find every red line. Chrome rows carry no plain text, so a search cannot find its own boundary bars. Plain and VisibleLength are pinned against each other over a table of inputs: they share a protect-then-strip shape, and a divergence would put a match's offsets in a different coordinate system from the width renderers measure with. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN --- src/SharpMUTerm.Tui/MarkupText.cs | 24 +++++++ src/SharpMUTerm.Tui/PaneLine.cs | 16 ++++- src/SharpMUTerm.Tui/SharpMUTermApp.cs | 4 +- .../MarkupTextPlainTests.cs | 64 +++++++++++++++++++ 4 files changed, 106 insertions(+), 2 deletions(-) create mode 100644 tests/SharpMUTerm.Tui.Tests/MarkupTextPlainTests.cs diff --git a/src/SharpMUTerm.Tui/MarkupText.cs b/src/SharpMUTerm.Tui/MarkupText.cs index a2f7244..892b185 100644 --- a/src/SharpMUTerm.Tui/MarkupText.cs +++ b/src/SharpMUTerm.Tui/MarkupText.cs @@ -25,6 +25,30 @@ internal static int VisibleLength(string markup) return TagPattern.Replace(protectedText, string.Empty).Length; } + /// + /// The text a markup string actually puts on the screen: [tag] wrappers removed, and escaped + /// brackets ([[/]]) back to the single characters they stand for. + /// + /// This is what ⌃F searches, and it searches this rather than the markup because a match has to mean + /// what it looks like: a colour tag in the middle of a word must not split one, and a reader must not + /// be able to search for #ff0000 and find every red line. The same rule UrlDetector + /// follows one layer down — run over the line, never over its pieces. + /// + /// + /// It shares 's protect-then-strip shape deliberately, and the two are + /// held together by test: Plain(x).Length equals VisibleLength(x) for every input. A + /// divergence would put a match's offsets in a different coordinate system from the width every + /// renderer measures with. + /// + /// + internal static string Plain(string markup) + { + var protectedText = markup.Replace("[[", "\u0001").Replace("]]", "\u0002"); + return TagPattern.Replace(protectedText, string.Empty) + .Replace('\u0001', '[') + .Replace('\u0002', ']'); + } + /// Pads a markup string to a target *visible* column width, ignoring markup tags. internal static string PadVisible(string markup, int width) { diff --git a/src/SharpMUTerm.Tui/PaneLine.cs b/src/SharpMUTerm.Tui/PaneLine.cs index 7fe13e8..dbeeaf6 100644 --- a/src/SharpMUTerm.Tui/PaneLine.cs +++ b/src/SharpMUTerm.Tui/PaneLine.cs @@ -32,4 +32,18 @@ namespace SharpMUTerm.Tui; /// /// The line's own Spectre-style markup, with no gutter attached. /// When the line arrived, pre-formatted for the gutter, or null for no gutter. -internal readonly record struct PaneLine(string Markup, string? Stamp = null); +/// +/// The text the markup puts on the screen () — what ⌃F searches. +/// +/// Held rather than derived, because the search surface refilters on every keystroke over every line of +/// every window: stripping thousands of lines eight times while somebody types a word is the difference +/// between a search that feels instant and one that does not. The cost is one string per buffered line, +/// bounded by the same cap the buffer already has. +/// +/// +/// Empty for the client's own chrome — the away and search bars, which are inserted into the +/// buffer rather than appended through it. A search that could find its own boundary markers would let +/// ⌥G walk between them, and they are not output. +/// +/// +internal readonly record struct PaneLine(string Markup, string? Stamp = null, string Plain = ""); diff --git a/src/SharpMUTerm.Tui/SharpMUTermApp.cs b/src/SharpMUTerm.Tui/SharpMUTermApp.cs index c11f360..ea338c0 100644 --- a/src/SharpMUTerm.Tui/SharpMUTermApp.cs +++ b/src/SharpMUTerm.Tui/SharpMUTermApp.cs @@ -2403,7 +2403,9 @@ private void AppendWindowLine(string windowId, string markup, string? stamp = nu _missedFrom[windowId] = buffer.Count; } - buffer.Add(new PaneLine(markup, stamp)); + // The plain text is computed here, once, and not on demand: ⌃F refilters on every keystroke over + // every line of every window (see PaneLine.Plain). + buffer.Add(new PaneLine(markup, stamp, MarkupText.Plain(markup))); // Cap the UI-side buffer at the configured scrollback so a long session doesn't grow without diff --git a/tests/SharpMUTerm.Tui.Tests/MarkupTextPlainTests.cs b/tests/SharpMUTerm.Tui.Tests/MarkupTextPlainTests.cs new file mode 100644 index 0000000..b391d99 --- /dev/null +++ b/tests/SharpMUTerm.Tui.Tests/MarkupTextPlainTests.cs @@ -0,0 +1,64 @@ +using SharpMUTerm.Tui; + +namespace SharpMUTerm.Tui.Tests; + +/// +/// — the text a markup line actually puts on the screen, and what ⌃F +/// searches. Its offsets are the ones the surface marks a row's match with, so they have to be in the +/// same coordinate system as the width every renderer measures with. +/// +public class MarkupTextPlainTests +{ + [Test] + public async Task TagsAreRemovedAndTheTextIsLeftAlone() + { + await Assert.That(MarkupText.Plain("[bold #ff0000]The goblin[/] snarls.")) + .IsEqualTo("The goblin snarls."); + } + + /// + /// The point of searching this rather than the markup: a world may change colour mid-word, and a + /// query for the word has to find it. Same defect as a URL split by a colour change, one layer down. + /// + [Test] + public async Task AColourChangeInsideAWordDoesNotSplitIt() + { + await Assert.That(MarkupText.Plain("gob[#00ff00]lin[/]")).IsEqualTo("goblin"); + } + + /// And the other half: a tag's own text is not searchable. + [Test] + public async Task ATagsContentsAreNotPartOfTheText() + { + await Assert.That(MarkupText.Plain("[#ff0000]red[/]")).DoesNotContain("ff0000"); + } + + [Test] + public async Task EscapedBracketsComeBackAsTheOneCharacterTheyStandFor() + { + await Assert.That(MarkupText.Plain("[[OOC]] Ana: hello")).IsEqualTo("[OOC] Ana: hello"); + } + + [Test] + public async Task ALinkSpanKeepsItsVisibleTextAndLosesItsTarget() + { + await Assert.That(MarkupText.Plain("see [link=https://example.com/map]the map[/] here")) + .IsEqualTo("see the map here"); + } + + /// + /// The invariant that keeps a match's offsets meaningful: this and + /// must agree on every input, or a row would mark a run at a column the renderer measures differently. + /// + [Test] + [Arguments("plain text")] + [Arguments("[bold]bold[/] and [dim]dim[/]")] + [Arguments("[[escaped]] and [#00ff00]coloured[/]")] + [Arguments("[link=https://example.com]a link[/]")] + [Arguments("")] + [Arguments("[dim]12:04[/] [bold]0001[/] · the courier's road")] + public async Task ItAgreesWithVisibleLength(string markup) + { + await Assert.That(MarkupText.Plain(markup).Length).IsEqualTo(MarkupText.VisibleLength(markup)); + } +} From 7fbf92cab0aaa2218e580e39df6a60c418477dc9 Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Tue, 11 Aug 2026 15:18:07 -0500 Subject: [PATCH 12/19] feat(activity): the bar holds for 30s after it has been read past MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two conditions were not enough. On a shallow absence the pane is already at its live tail when the reader returns, so the very next keystroke retired the bar a second or two after it appeared — the bar going before it could be read. The third is a floor in time, because that is the unit the complaint was in: a raised input count would be an hour on a quiet character and three seconds on a busy one. It is a floor and not a timer — nothing fires on its own, so the bar goes on the first of the existing checks after the floor has passed, and a client sitting untouched keeps its bar. F7 ▸ ACTIVITY, default 30, and 0 restores the previous behaviour exactly. New defaulted field, so no schema bump and no migration. Measured off the app's existing TimeProvider, so the tests move the clock instead of racing it. AwayDividerTests takes the floor at zero: those tests are about where the bar is drawn and what removes it, and several turn on "one keystroke is all it takes". Each suite asserts one rule rather than both at once. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN --- .../Configuration/PreferenceSettings.cs | 28 +++++ src/SharpMUTerm.Tui/OptionsScreenRenderer.cs | 13 ++ src/SharpMUTerm.Tui/SharpMUTermApp.cs | 19 ++- .../ActivityBarDwellTests.cs | 114 ++++++++++++++++++ .../SharpMUTerm.Tui.Tests/AwayDividerTests.cs | 7 ++ .../SharpMUTerm.Tui.Tests/ScreenModelTests.cs | 11 +- 6 files changed, 185 insertions(+), 7 deletions(-) create mode 100644 tests/SharpMUTerm.Tui.Tests/ActivityBarDwellTests.cs diff --git a/src/SharpMUTerm.Core/Configuration/PreferenceSettings.cs b/src/SharpMUTerm.Core/Configuration/PreferenceSettings.cs index bc9edf0..be90c08 100644 --- a/src/SharpMUTerm.Core/Configuration/PreferenceSettings.cs +++ b/src/SharpMUTerm.Core/Configuration/PreferenceSettings.cs @@ -82,6 +82,34 @@ public sealed class TextSettings /// public bool EmojiSubstitution { get; set; } = true; + /// + /// How long the activity bar stays put after the reader has read past it, in seconds. + /// + /// It exists because two conditions were not enough. The bar retires when its pane is back at its + /// live tail and one input has landed since it was drawn — and on a shallow absence the pane + /// is already at its tail the moment the reader returns, so the very next keystroke took the bar away + /// a second or two after it appeared. That was reported as the bar going before it could be read. + /// + /// + /// A floor in time, because that is the unit the complaint was in. A raised input count + /// would be an hour on a quiet character and three seconds on a busy one — the same bug with a bigger + /// number in it. + /// + /// + /// It is only a floor: the other two conditions still have to hold, and the bar goes on the first + /// check after it — the next keystroke or scroll, rather than a timer firing on its own. A + /// client sitting untouched keeps its bar, which is the right answer for a reader who has walked away + /// again. Zero restores the behaviour this replaced, exactly. + /// + /// + public int ActivityBarSeconds { get; set; } = DefaultActivityBarSeconds; + + /// Thirty seconds: long enough to read a screenful, short enough not to become furniture. + public const int DefaultActivityBarSeconds = 30; + + /// Ten minutes. Past this a boundary marker is not a marker, it is a pin. + public const int MaxActivityBarSeconds = 600; + // There is deliberately no "ambiguous width" here. It was a setting with nothing behind it: every // column measurement in this app is SharpConsoleUI's (Helpers/UnicodeWidth.cs), which asks the // Wcwidth tables and offers no East-Asian-ambiguous policy to set. Honouring it needs an upstream diff --git a/src/SharpMUTerm.Tui/OptionsScreenRenderer.cs b/src/SharpMUTerm.Tui/OptionsScreenRenderer.cs index 2fae090..b376f82 100644 --- a/src/SharpMUTerm.Tui/OptionsScreenRenderer.cs +++ b/src/SharpMUTerm.Tui/OptionsScreenRenderer.cs @@ -285,6 +285,19 @@ internal static OptionsScreen TextAnsiScreen(TextSettings? text = null) new("├ UNICODE", null, null), new("emoji substitution", null, settings.EmojiSubstitution, null, ScreenToggle.Bind(() => settings.EmojiSubstitution, v => settings.EmojiSubstitution = v)), + new(string.Empty, null, null), + new("├ ACTIVITY", null, null), + + // Seconds, and its own section: it is the only row on this screen describing the client's own + // chrome rather than how a world's text is drawn. Zero is a real answer — it retires the bar + // as soon as it has been read past, which is what this client did before the floor existed — + // so the field's floor is 0, as the tab width's is. + new("activity bar holds for (seconds)", + settings.ActivityBarSeconds.ToString(CultureInfo.InvariantCulture), null, null, null, + ScreenField.Integer( + "activity bar holds for (seconds)", + () => settings.ActivityBarSeconds, v => settings.ActivityBarSeconds = v, + 0, TextSettings.MaxActivityBarSeconds)), }); } diff --git a/src/SharpMUTerm.Tui/SharpMUTermApp.cs b/src/SharpMUTerm.Tui/SharpMUTermApp.cs index 1936cce..3bea9da 100644 --- a/src/SharpMUTerm.Tui/SharpMUTermApp.cs +++ b/src/SharpMUTerm.Tui/SharpMUTermApp.cs @@ -2619,6 +2619,13 @@ private sealed class AwayMark /// would set and retire the bar to the keystroke that produced it. /// public long DrawnAfter; + + /// + /// When this bar was drawn, off the app's own TimeProvider. The dwell floor is measured + /// from here — see , which explains why a floor in + /// time and not in keystrokes. + /// + public DateTimeOffset DrawnAt; } /// @@ -2709,7 +2716,7 @@ private void MarkWhereTheReaderLeft(TimeSpan away) _freezePoints[windowId] = freeze + 1; } - var mark = new AwayMark { Index = at, DrawnAfter = _focus.InputCount }; + var mark = new AwayMark { Index = at, DrawnAfter = _focus.InputCount, DrawnAt = _time.GetUtcNow() }; _awayMarks[windowId] = mark; RepaintPane(windowId); RevealAwayBar(windowId, mark); @@ -2788,6 +2795,7 @@ private void MarkMissedLines(bool reveal) { Index = at, DrawnAfter = _focus.InputCount, + DrawnAt = _time.GetUtcNow(), }; _awayMarks[windowId] = mark; @@ -2944,7 +2952,14 @@ private void ConsumeReadAwayBars() // AutoScroll is the framework's own "showing the live tail" bit — the same fact // SyncScrollbackState mirrors, rather than a second one kept in step with it. - if (mark.InputSince && panel.AutoScroll) + // + // The third conjunct is the dwell floor. Without it a shallow absence — the bar and + // everything under it on screen at once, the pane never moved — retired on the very next + // keystroke, a second or two after the bar appeared, which is the bar going before it could + // be read. It is a floor and not a timer: nothing fires on its own, so the bar goes on the + // first of these checks after the floor has passed. + var held = TimeSpan.FromSeconds(Math.Max(0, _config.Text.ActivityBarSeconds)); + if (mark.InputSince && panel.AutoScroll && _time.GetUtcNow() - mark.DrawnAt >= held) { RemoveAwayBar(windowId); RepaintPane(windowId); diff --git a/tests/SharpMUTerm.Tui.Tests/ActivityBarDwellTests.cs b/tests/SharpMUTerm.Tui.Tests/ActivityBarDwellTests.cs new file mode 100644 index 0000000..331cc67 --- /dev/null +++ b/tests/SharpMUTerm.Tui.Tests/ActivityBarDwellTests.cs @@ -0,0 +1,114 @@ +using SharpConsoleUI.Drivers; +using SharpMUTerm.Core.Session; +using SharpMUTerm.Graphics; +using SharpMUTerm.Tui; + +namespace SharpMUTerm.Tui.Tests; + +/// +/// The bar does not retire the instant it has been read past. Two conditions were not enough: on a +/// shallow absence the pane is already at its live tail, so the next keystroke took the bar away a +/// second or two after it appeared. The third is a floor in time, because that is the unit the +/// complaint was in — a raised input count would be an hour on a quiet character and three seconds on a +/// busy one. +/// +/// +/// Serialised for the reason every file that renders a frame is: rendering redirects the process-global +/// Console.Out, and the harness redirects Console.In. +/// +[NotInParallel] +public class ActivityBarDwellTests +{ + private const int Width = 120; + private const int Height = 32; + private const string Main = "main"; + + private static readonly TerminalCapabilities Headless = + new(GraphicsProtocol.None, supportsTrueColor: true, supportsKittyGraphics: false, supportsSixel: false); + + private static (SharpMUTermApp App, WorldSession Session, ManualTimeProvider Time) Bound(int seconds) + { + Console.SetIn(TextReader.Null); + var config = DemoScene.Build(); + config.ScrollbackSpill.Enabled = false; + config.Text.ActivityBarSeconds = seconds; + + var time = new ManualTimeProvider(); + var app = new SharpMUTermApp(config, Headless, new HeadlessConsoleDriver(Width, Height), time); + var session = app.BindWorldWithoutConnecting(config.Worlds[0]); + app.RenderSnapshot(); + return (app, session, time); + } + + private static ConsoleKeyInfo Key(ConsoleKey key) => new('\0', key, false, false, false); + + /// + /// Leaves a bar in the main window that has been read past on both of the original counts: the pane + /// is at its live tail (a shallow absence never takes it off) and an input has landed since. + /// + private static void DrawAndRead(SharpMUTermApp app, WorldSession session) + { + session.PrintSystem("*** before you left"); + app.SimulateKey(Key(ConsoleKey.End)); + session.PrintSystem("*** while you were away"); + app.SimulateReturnFromAway(TimeSpan.FromMinutes(2)); + app.SimulateKey(Key(ConsoleKey.End)); + } + + [Test] + public async Task TheBarSurvivesBeingReadPastUntilTheFloorHasElapsed() + { + var (app, session, time) = Bound(30); + DrawAndRead(app, session); + + await Assert.That(app.AwayBarIndex(Main)).IsNotNull(); + + time.Advance(TimeSpan.FromSeconds(29)); + app.SimulateKey(Key(ConsoleKey.End)); + await Assert.That(app.AwayBarIndex(Main)).IsNotNull(); + + time.Advance(TimeSpan.FromSeconds(2)); + app.SimulateKey(Key(ConsoleKey.End)); + await Assert.That(app.AwayBarIndex(Main)).IsNull(); + } + + /// + /// Zero is a real answer, and it is exactly the behaviour the floor replaced — so a reader who + /// disagrees with the default can have the old client back rather than a compromise. + /// + [Test] + public async Task AFloorOfZeroRetiresTheBarAsSoonAsItHasBeenReadPast() + { + var (app, session, _) = Bound(0); + DrawAndRead(app, session); + + await Assert.That(app.AwayBarIndex(Main)).IsNull(); + } + + /// + /// The floor is a floor and not a clock the bar goes by on its own: with the other two conditions + /// unmet — here, the pane taken off its live tail — waiting does not retire it. + /// + [Test] + public async Task TimePassingIsNotEnoughOnItsOwn() + { + var (app, session, time) = Bound(30); + + // Enough output to have a scrollback at all. Without it PageUp clamps at offset zero, which *is* + // the bottom, so auto-scroll re-attaches and the pane never leaves its live tail — the test would + // then be asserting the opposite of what it says. + for (var i = 1; i <= 80; i++) + { + session.PrintSystem($"*** line {i}"); + } + + app.RenderSnapshot(); + DrawAndRead(app, session); + + app.SimulateScrollKey(Key(ConsoleKey.PageUp)); + time.Advance(TimeSpan.FromMinutes(5)); + app.SimulateKey(Key(ConsoleKey.End)); + + await Assert.That(app.AwayBarIndex(Main)).IsNotNull(); + } +} diff --git a/tests/SharpMUTerm.Tui.Tests/AwayDividerTests.cs b/tests/SharpMUTerm.Tui.Tests/AwayDividerTests.cs index 8126b89..61046ee 100644 --- a/tests/SharpMUTerm.Tui.Tests/AwayDividerTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/AwayDividerTests.cs @@ -365,6 +365,13 @@ private static Core.Configuration.AppConfiguration Quiet() { var config = DemoScene.Build(); config.ScrollbackSpill.Enabled = false; + + // And no dwell floor. These tests are about where the bar is drawn, what puts it on screen, and + // what takes it away again — the two conditions that were always there — and several of them turn + // on "one keystroke is all it takes", which is only the answer when the floor is zero. The floor + // is a third condition with a suite of its own (ActivityBarDwellTests); setting it here keeps each + // suite asserting one rule rather than both at once. + config.Text.ActivityBarSeconds = 0; return config; } diff --git a/tests/SharpMUTerm.Tui.Tests/ScreenModelTests.cs b/tests/SharpMUTerm.Tui.Tests/ScreenModelTests.cs index 52ba201..008436d 100644 --- a/tests/SharpMUTerm.Tui.Tests/ScreenModelTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/ScreenModelTests.cs @@ -364,12 +364,13 @@ public async Task Options_NavigableRowsSkipSectionHeadersAndSpacers() var screen = OptionsScreenRenderer.TextAnsiScreen(); var model = OptionsScreenRenderer.Model(screen); - // 11 display rows: 3 section headers + 2 spacers + 6 options. It was 7/4 before WHITESPACE and - // its "tab width (spaces)" row, which brought a header and a spacer with it, and 10/5 before - // "detect links in output" joined the COLOUR section. - await Assert.That(screen.Rows.Count).IsEqualTo(11); + // 14 display rows: 4 section headers + 3 spacers + 7 options. It was 7/4 before WHITESPACE and + // its "tab width (spaces)" row, which brought a header and a spacer with it, 10/5 before + // "detect links in output" joined the COLOUR section, and 11/6 before ACTIVITY and its + // "activity bar holds for (seconds)" row brought a header and a spacer of their own. + await Assert.That(screen.Rows.Count).IsEqualTo(14); await Assert.That(model.PaneCount).IsEqualTo(1); - await Assert.That(model.Sizes[0]).IsEqualTo(6); + await Assert.That(model.Sizes[0]).IsEqualTo(7); } /// From 34bf9760acfb74f3ffb8650b7629bfaff49ad2f0 Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Tue, 11 Aug 2026 15:33:12 -0500 Subject: [PATCH 13/19] =?UTF-8?q?feat(search):=20SearchPrompt=20=E2=80=94?= =?UTF-8?q?=20what=20=E2=8C=83F=20means=20by=20a=20keystroke,=20and=20what?= =?UTF-8?q?=20it=20says?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The HistorySearchPrompt/HistorySurface split verbatim: the rules and the wording are exactly the part a headless test can pin. ⌥E and ⌥A both re-search from the top, because each changes what the list is and a pointer kept at row 12 of a different result set points at nothing the reader chose. The header states the bound it searched — '12 found · 4,812 lines held' — because ⌃F sees the pane buffer and not a session's whole history, and a reader who cannot find an old line should be able to see why rather than concluding the search is broken. The window column is drawn only when every window is searched; with one window it would be the same word on every row. The footer names only keys that work, pinned by a test that presses every one. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN --- src/SharpMUTerm.Tui/SearchPrompt.cs | 334 ++++++++++++++++++ .../SearchPromptTests.cs | 238 +++++++++++++ 2 files changed, 572 insertions(+) create mode 100644 src/SharpMUTerm.Tui/SearchPrompt.cs create mode 100644 tests/SharpMUTerm.Tui.Tests/SearchPromptTests.cs diff --git a/src/SharpMUTerm.Tui/SearchPrompt.cs b/src/SharpMUTerm.Tui/SearchPrompt.cs new file mode 100644 index 0000000..da1f5c8 --- /dev/null +++ b/src/SharpMUTerm.Tui/SearchPrompt.cs @@ -0,0 +1,334 @@ +using static SharpMUTerm.Tui.MarkupText; +using static SharpMUTerm.Tui.ScreenPalette; + +namespace SharpMUTerm.Tui; + +/// What one keystroke means to the open search surface. +internal enum SearchAction +{ + /// Nothing. The key is swallowed and the surface stays exactly as it is. + None, + + /// The query, a toggle or the pointed-at row changed; re-search and redraw. + Redraw, + + /// Go to the pointed-at line: activate its window, mark it, and close. + Go, + + /// Close, changing nothing. + Cancel, +} + +/// The answer to one keystroke: what to do, and the state it leaves behind. +internal readonly record struct SearchDecision( + SearchAction Action, string Query, int Selected, bool Regex, bool AllWindows); + +/// +/// One row of results: which window the line is in, where it sits in that window's buffer, and where the +/// query landed inside it. +/// +internal readonly record struct SearchRow( + string WindowId, string WindowLabel, int LineIndex, string Text, int MatchStart, int MatchLength); + +/// +/// The search surface (⌃F), minus the window: what a keystroke means to it, and what it says. Pure, for +/// the same reason is — the rules and the wording are exactly the part +/// a headless test can pin, and is left with nothing but framework calls. +/// +/// A results surface and not an in-pane bar. A less-style bar under the pane has nowhere to +/// say which pane a hit is in, and searching more than one window is half of what was asked for. +/// This shape also inherits ⌃R's interaction, which the client already teaches. +/// +/// +/// The header states the bound it searched. ⌃F sees the lines the client is holding — the pane +/// buffer — and not a session's whole scrollback or the file-backed spill, because a spawn window's lines +/// exist in neither. So the count reads 12 of 38 · 4,812 lines held: a reader who cannot find +/// something from an hour ago can see why on the frame rather than guessing. +/// +/// +/// The footer names only keys that work. ↑↓ move (wrapping, as ⌃P's and ⌃R's do), ⏎ goes, ⌥E +/// switches regex on and off, ⌥A widens to every window, Esc cancels and ⌃F closes. Printable characters +/// filter and ⌫ un-filters. Nothing else is bound, so nothing else is advertised — see +/// SearchPromptTests, which presses every key this string names. +/// +/// +internal static class SearchPrompt +{ + /// + /// The keys, and the whole set of them. Every one is honoured by . ⌫ is + /// deliberately absent for 's reason: it is named on the + /// no matches line instead, which is the only moment a reader needs telling that the query can + /// be widened. + /// + internal const string Hints = + "type to search · ↑↓ pick · ⏎ go · ⌥E regex · ⌥A all windows · Esc cancel · ⌃F closes"; + + /// The longest an entry is drawn before it is elided, when no width is supplied. + private const int DefaultEntryWidth = 72; + + /// + /// 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. + /// + /// Anything unrecognised is swallowed rather than passed down: a modal surface that let stray keys + /// through would be typing into the command line the reader cannot currently see. + /// + /// + internal static SearchDecision Interpret( + ConsoleKeyInfo key, string query, int selected, int count, bool regex, bool all) + { + var state = new SearchDecision(SearchAction.None, query, selected, regex, all); + + if (key.Modifiers.HasFlag(ConsoleModifiers.Control)) + { + // ⌃F is spelled out here as well as in the surface's toggle, for QuitPrompt's reason: this is + // where "what does this keystroke mean" is answered, and a rule living only in the app's + // shortcut table could not be read back by a test. In the running client a global shortcut + // runs before any window, so the chord never reaches this handler — the toggle answers it, + // the same way. + return key.Key == ConsoleKey.F ? state with { Action = SearchAction.Cancel } : state; + } + + if (key.Modifiers.HasFlag(ConsoleModifiers.Alt)) + { + // The two toggles. Both re-search from the top, because both change what the list *is* — a + // pointer kept at row 12 of a different result set points at nothing the reader chose. + return key.Key switch + { + ConsoleKey.E => state with { Action = SearchAction.Redraw, Regex = !regex, Selected = 0 }, + ConsoleKey.A => state with { Action = SearchAction.Redraw, AllWindows = !all, Selected = 0 }, + _ => state, + }; + } + + switch (key.Key) + { + case ConsoleKey.Escape: + return state with { Action = SearchAction.Cancel }; + + case ConsoleKey.Enter: + // With nothing listed there is nowhere to go, so ⏎ does nothing and leaves the surface + // up: the query is what needs fixing, and Esc is the advertised way out. + return count == 0 ? state : state with { Action = SearchAction.Go }; + + case ConsoleKey.UpArrow: + return state with { Action = SearchAction.Redraw, Selected = Step(selected, -1, count) }; + + case ConsoleKey.DownArrow: + return state with { Action = SearchAction.Redraw, Selected = Step(selected, 1, count) }; + + case ConsoleKey.Backspace: + // The pointer goes back to the first match: widening the query changes what row 0 is. + return query.Length == 0 + ? state + : state with { Action = SearchAction.Redraw, Query = query[..^1], Selected = 0 }; + } + + // Typing searches. Control characters are excluded so an undecoded sequence cannot end up in the + // query, and Tab (which arrives as one) is swallowed rather than becoming whitespace. + if (!char.IsControl(key.KeyChar) && key.KeyChar != '\0') + { + return state with { Action = SearchAction.Redraw, Query = query + key.KeyChar, Selected = 0 }; + } + + return state; + } + + /// Moves the pointer, wrapping — the same behaviour ⌃P's and ⌃R's lists have. + private static int Step(int selected, int delta, int count) => + count == 0 ? -1 : ((selected + delta) % count + count) % count; + + /// + /// The whole surface as markup lines: the query line with its state, the rows, and the keys. + /// + /// is how many rows the list area holds and which + /// match is at the top of it, which is what makes this a viewport rather than a dump. The area is + /// always drawn at full height — padded with blank rows — so the footer does not walk up and down the + /// screen on every keystroke. + /// + /// + /// The matches, in buffer order. + /// The query as typed so far. + /// Why the query could not be used, or null. + /// Whether the query is being read as a pattern. + /// Whether every window is being searched, rather than the focused one. + /// What the focused window is called, for the one-window case. + /// How many lines were searched — the bound, stated rather than implied. + /// Which row the pointer is on. + /// The surface's content width, when known. + /// How many rows the list area holds; zero means "as many as there are". + /// Which match is drawn at the top of the list area. + internal static List Render( + IReadOnlyList rows, + string query, + string? error, + bool regex, + bool all, + string scope, + int held, + int selected, + int width = 0, + int listRows = 0, + int first = 0) + { + ArgumentNullException.ThrowIfNull(rows); + ArgumentNullException.ThrowIfNull(query); + + var labelWidth = all ? rows.Select(r => VisibleLength(Escape(r.WindowLabel))).DefaultIfEmpty(0).Max() : 0; + var entryWidth = (width > 0 ? width : DefaultEntryWidth) - 4 - (labelWidth > 0 ? labelWidth + 2 : 0); + + var lines = new List + { + SpreadLR($"[{Label}]search[/] {QueryMarkup(query)}", $"[{Label}]{Escape(State(regex, all, scope))}[/]", width), + SpreadLR(string.Empty, $"[{Label}]{Escape(Counted(rows.Count, error, held))}[/]", width), + }; + + var body = new List(); + if (error is not null) + { + body.Add($"[{Muted}] {Escape(error)}[/]"); + } + else if (query.Length == 0) + { + body.Add($"[{Muted}] type to search {Escape(all ? "every window" : scope)}[/]"); + } + else if (rows.Count == 0) + { + body.Add($"[{Muted}] no matches — ⌫ widens the query[/]"); + } + + var top = Math.Clamp(first, 0, Math.Max(0, rows.Count - 1)); + var last = listRows > 0 ? Math.Min(rows.Count, top + listRows - body.Count) : rows.Count; + for (var i = top; i < last; i++) + { + body.Add(Row(rows[i], i == selected, entryWidth, labelWidth, width)); + } + + while (body.Count < listRows) + { + body.Add(string.Empty); + } + + lines.AddRange(body); + lines.Add(string.Empty); + lines.Add($"[{Label}]{Hints}[/]"); + return lines; + } + + /// + /// Where the list area's top must sit for to be inside it, moving as + /// little as possible from . + /// + internal static int Scroll(int first, int selected, int count, int listRows) + { + if (listRows <= 0 || count <= listRows || selected < 0) + { + return 0; + } + + var top = Math.Clamp(first, selected - listRows + 1, selected); + 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 + /// is visible in the results themselves. + /// + private static string State(bool regex, bool all, string scope) => + $"{(regex ? "regex" : "text")} · {(all ? "every window" : scope)}"; + + /// + /// How many lines matched out of how many were looked at. The second figure is the bound this search + /// actually had — the pane buffer, not a session's whole history — and it is on the frame so a reader + /// who cannot find an old line can see why rather than concluding the search is broken. + /// + private static string Counted(int shown, string? error, int held) + { + var searched = $"{held:n0} line{(held == 1 ? string.Empty : "s")} held"; + return error is not null ? searched : $"{shown} found · {searched}"; + } + + /// + /// The query, with the accent block caret after it — the same caret the settings screens draw on an + /// open field, so this reads as something being typed into. + /// + private static string QueryMarkup(string query) => + query.Length == 0 + ? $"[{Ink} on {Accent}] [/]" + : $"[{Value}]{Escape(query)}[/][{Ink} on {Accent}] [/]"; + + 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; + + if (selected) + { + // One continuous accent bar across the row, padded to the surface width. The matched run is + // not separately marked here: it would be a highlight inside a highlight, and the pointer is + // the fact this row is drawing. + var body = $" ▸ {label}{Escape(text)} "; + var pad = width > VisibleLength(body) ? new string(' ', width - VisibleLength(body)) : string.Empty; + return $"[{Ink} on {Accent}]{body}{pad}[/]"; + } + + var prefix = labelWidth > 0 ? $"[{Muted}] {label}[/]" : " "; + if (matchStart < 0 || matchLength == 0) + { + return $"{prefix}[{Value}]{Escape(text)}[/]"; + } + + // Mark where the query matched, so a row shows *why* it is in the list. + var before = Escape(text[..matchStart]); + var hit = Escape(text.Substring(matchStart, matchLength)); + var after = Escape(text[(matchStart + matchLength)..]); + return $"{prefix}[{Value}]{before}[/][bold {Accent}]{hit}[/][{Value}]{after}[/]"; + } + + /// + /// 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 + /// found. + /// + private static (string Text, int MatchStart, int MatchLength) Elide(SearchRow row, int entryWidth) + { + const string Ellipsis = "…"; + var text = row.Text; + if (entryWidth <= Ellipsis.Length || text.Length <= entryWidth) + { + return (text, row.MatchStart, row.MatchLength); + } + + var matchEnd = row.MatchStart < 0 ? 0 : row.MatchStart + row.MatchLength; + var start = Math.Max(0, Math.Min(matchEnd - entryWidth + Ellipsis.Length, text.Length - entryWidth)); + var length = Math.Min(entryWidth - Ellipsis.Length, text.Length - start); + + var head = start > 0 ? Ellipsis : string.Empty; + var tail = start + length < text.Length ? Ellipsis : string.Empty; + var shown = head + text.Substring(start, length) + tail; + + if (row.MatchStart < 0) + { + return (shown, -1, 0); + } + + var shifted = row.MatchStart - start + head.Length; + var visible = Math.Max(0, Math.Min(row.MatchLength, shown.Length - tail.Length - shifted)); + return shifted < 0 ? (shown, -1, 0) : (shown, shifted, visible); + } +} diff --git a/tests/SharpMUTerm.Tui.Tests/SearchPromptTests.cs b/tests/SharpMUTerm.Tui.Tests/SearchPromptTests.cs new file mode 100644 index 0000000..8cb9daf --- /dev/null +++ b/tests/SharpMUTerm.Tui.Tests/SearchPromptTests.cs @@ -0,0 +1,238 @@ +using SharpMUTerm.Tui; + +namespace SharpMUTerm.Tui.Tests; + +/// +/// What the ⌃F surface means by a keystroke, and what it says. The client wiring is +/// ' job; this is the part that needs no terminal. +/// +public class SearchPromptTests +{ + private static ConsoleKeyInfo Key(char c) => new(c, ConsoleKey.NoName, false, false, false); + + private static ConsoleKeyInfo Bare(ConsoleKey key) => new('\0', key, false, false, false); + + private static ConsoleKeyInfo Alt(ConsoleKey key) => new('\0', key, false, true, false); + + private static ConsoleKeyInfo Ctrl(ConsoleKey key) => new('\0', key, false, false, true); + + private static SearchDecision Interpret( + ConsoleKeyInfo key, string query = "gob", int selected = 0, int count = 3, + bool regex = false, bool all = false) => + SearchPrompt.Interpret(key, query, selected, count, regex, all); + + private static readonly SearchRow[] Rows = + { + new("main", "main", 12, "The goblin snarls at you.", 4, 6), + new("spawn:chat", "Chat", 3, " Ana: goblin room is bugged", 11, 6), + }; + + [Test] + public async Task TypingBuildsTheQueryAndPointsAtTheFirstRow() + { + var decision = Interpret(Key('x'), query: "gob", selected: 2); + + await Assert.That(decision.Action).IsEqualTo(SearchAction.Redraw); + await Assert.That(decision.Query).IsEqualTo("gobx"); + await Assert.That(decision.Selected).IsEqualTo(0); + } + + [Test] + public async Task BackspaceWidensTheQuery() + { + var decision = Interpret(Bare(ConsoleKey.Backspace), query: "gob"); + + await Assert.That(decision.Action).IsEqualTo(SearchAction.Redraw); + await Assert.That(decision.Query).IsEqualTo("go"); + } + + [Test] + public async Task BackspaceOnAnEmptyQueryDoesNothing() + { + await Assert.That(Interpret(Bare(ConsoleKey.Backspace), query: string.Empty).Action) + .IsEqualTo(SearchAction.None); + } + + [Test] + public async Task TheArrowsWalkTheRowsAndWrap() + { + await Assert.That(Interpret(Bare(ConsoleKey.DownArrow), selected: 2, count: 3).Selected).IsEqualTo(0); + await Assert.That(Interpret(Bare(ConsoleKey.UpArrow), selected: 0, count: 3).Selected).IsEqualTo(2); + } + + [Test] + public async Task EnterGoes() + { + await Assert.That(Interpret(Bare(ConsoleKey.Enter)).Action).IsEqualTo(SearchAction.Go); + } + + /// + /// With nothing listed there is nowhere to go, so ⏎ leaves the surface up: the query is what needs + /// fixing, and Esc is the advertised way out. + /// + [Test] + public async Task EnterWithNothingListedLeavesTheSurfaceUp() + { + await Assert.That(Interpret(Bare(ConsoleKey.Enter), count: 0).Action).IsEqualTo(SearchAction.None); + } + + [Test] + public async Task EscapeAndCtrlFBothClose() + { + await Assert.That(Interpret(Bare(ConsoleKey.Escape)).Action).IsEqualTo(SearchAction.Cancel); + await Assert.That(Interpret(Ctrl(ConsoleKey.F)).Action).IsEqualTo(SearchAction.Cancel); + } + + /// + /// Both toggles re-search from the top: each changes what the list *is*, and a pointer kept at row + /// 12 of a different result set points at nothing the reader chose. + /// + [Test] + public async Task AltEAndAltAFlipTheirTogglesAndResetThePointer() + { + var regex = Interpret(Alt(ConsoleKey.E), selected: 2, regex: false); + await Assert.That(regex.Action).IsEqualTo(SearchAction.Redraw); + await Assert.That(regex.Regex).IsTrue(); + await Assert.That(regex.Selected).IsEqualTo(0); + + var all = Interpret(Alt(ConsoleKey.A), selected: 2, all: false); + await Assert.That(all.AllWindows).IsTrue(); + await Assert.That(all.Selected).IsEqualTo(0); + + // And back again — they are toggles, not switches that only turn on. + await Assert.That(Interpret(Alt(ConsoleKey.E), regex: true).Regex).IsFalse(); + await Assert.That(Interpret(Alt(ConsoleKey.A), all: true).AllWindows).IsFalse(); + } + + /// + /// Anything unrecognised is swallowed. A modal over the workspace that let keys through would be + /// typing into the command line it is covering. + /// + [Test] + public async Task UnrecognisedKeysAreSwallowed() + { + await Assert.That(Interpret(Ctrl(ConsoleKey.K)).Action).IsEqualTo(SearchAction.None); + await Assert.That(Interpret(Alt(ConsoleKey.Z)).Action).IsEqualTo(SearchAction.None); + await Assert.That(Interpret(Bare(ConsoleKey.Tab)).Action).IsEqualTo(SearchAction.None); + await Assert.That(Interpret(Bare(ConsoleKey.F5)).Action).IsEqualTo(SearchAction.None); + } + + /// + /// The honesty rule the settings screens and the composer are held to: every key the footer names + /// does something. Pressed here rather than read, so a hint that outlived its binding fails. + /// + [Test] + public async Task EveryKeyTheFooterNamesDoesSomething() + { + var named = new (string Name, ConsoleKeyInfo Key)[] + { + ("↑", Bare(ConsoleKey.UpArrow)), + ("↓", Bare(ConsoleKey.DownArrow)), + ("⏎", Bare(ConsoleKey.Enter)), + ("⌥E", Alt(ConsoleKey.E)), + ("⌥A", Alt(ConsoleKey.A)), + ("Esc", Bare(ConsoleKey.Escape)), + ("⌃F", Ctrl(ConsoleKey.F)), + ("type", Key('g')), + }; + + foreach (var (name, key) in named) + { + await Assert.That(Interpret(key).Action).IsNotEqualTo(SearchAction.None).Because($"{name} is advertised"); + } + + // And the footer names each of them, so the two halves cannot drift apart. + foreach (var fragment in new[] { "↑↓", "⏎", "⌥E", "⌥A", "Esc", "⌃F", "type to search" }) + { + await Assert.That(SearchPrompt.Hints).Contains(fragment); + } + } + + /// + /// The surface says which way both toggles are set. Neither is visible in the results, and both + /// change what a query finds. + /// + [Test] + public async Task TheHeaderSaysWhichWayTheTogglesAreSet() + { + var text = string.Join('\n', SearchPrompt.Render(Rows, "gob", null, false, false, "main", 4812, 0)); + var pattern = string.Join('\n', SearchPrompt.Render(Rows, "gob", null, true, true, "main", 4812, 0)); + + await Assert.That(text).Contains("text"); + await Assert.That(text).Contains("main"); + await Assert.That(pattern).Contains("regex"); + await Assert.That(pattern).Contains("every window"); + } + + /// + /// The bound is on the frame. ⌃F searches the lines the client is holding, not a session's whole + /// history, and a reader who cannot find something from an hour ago should be able to see why. + /// + [Test] + public async Task TheHeaderStatesHowMuchWasSearched() + { + var lines = string.Join('\n', SearchPrompt.Render(Rows, "gob", null, false, false, "main", 4812, 0)); + + await Assert.That(lines).Contains("2 found"); + await Assert.That(lines).Contains("4,812 lines held"); + } + + [Test] + public async Task AnInvalidPatternIsSaidRatherThanShownAsNoMatches() + { + var lines = string.Join( + '\n', SearchPrompt.Render(Array.Empty(), "gob(", "unterminated group", true, false, "main", 10, -1)); + + await Assert.That(lines).Contains("unterminated group"); + await Assert.That(lines).DoesNotContain("no matches"); + } + + [Test] + public async Task AnEmptyQuerySaysWhatToDoRatherThanNothing() + { + var lines = string.Join( + '\n', SearchPrompt.Render(Array.Empty(), string.Empty, null, false, false, "main", 10, -1)); + + await Assert.That(lines).Contains("type to search main"); + } + + [Test] + public async Task NoMatchesNamesTheKeyThatWidensTheQuery() + { + var lines = string.Join( + '\n', SearchPrompt.Render(Array.Empty(), "zzz", null, false, false, "main", 10, -1)); + + await Assert.That(lines).Contains("⌫ widens"); + } + + /// + /// The window column is drawn only when there is more than one window in the results — with one + /// window it would be the same word on every row, saying nothing. + /// + [Test] + public async Task TheWindowColumnAppearsOnlyWhenEveryWindowIsSearched() + { + var one = string.Join('\n', SearchPrompt.Render(Rows, "gob", null, false, false, "main", 10, -1)); + var all = string.Join('\n', SearchPrompt.Render(Rows, "gob", null, false, true, "main", 10, -1)); + + await Assert.That(one).DoesNotContain("Chat"); + await Assert.That(all).Contains("Chat"); + } + + /// A row shows why it is listed: the matched run is marked in the accent. + [Test] + public async Task AnUnselectedRowMarksWhereTheQueryLanded() + { + var lines = SearchPrompt.Render(Rows, "goblin", null, false, false, "main", 10, selected: 1); + + await Assert.That(lines.Any(l => l.Contains("[bold ") && l.Contains("goblin"))).IsTrue(); + } + + [Test] + public async Task ScrollKeepsThePointedAtRowInsideTheListArea() + { + await Assert.That(SearchPrompt.Scroll(0, 9, 40, 5)).IsEqualTo(5); + await Assert.That(SearchPrompt.Scroll(5, 5, 40, 5)).IsEqualTo(5); + await Assert.That(SearchPrompt.Scroll(20, 0, 40, 5)).IsEqualTo(0); + } +} From 7659a87b82a0e213141dc09e5ab4e7b11c4056a5 Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Tue, 11 Aug 2026 15:24:03 -0500 Subject: [PATCH 14/19] docs(activity): the activity-bar frame, the brief, and a snapshot trap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the `activity-bar` view — three lines land in the main window while Chat is in front of it, and picking main back lands on the NEW bar with those three under it. Separate from `away` because the two are separate facts with separate wording, and this is the one that happens many times an hour. Also corrects the snapshot recipe, which cost this change real time: it said `dotnet build SharpMUTerm.slnx`, which builds *Debug*, while `--no-build` runs the *Release* output. The render then comes from a binary predating your change and a new view comes out byte-identical to the default frame — indistinguishable from a feature that does not work. The trap hides whenever you have just run the suites, because those refresh Release. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN --- CLAUDE.md | 46 +++++++++++++++++++++++++-- src/SharpMUTerm.Tui/SharpMUTermApp.cs | 14 ++++++++ 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 1d8ce56..f07a8f1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -103,8 +103,39 @@ fallbacks) for inline images/maps. Restored content is closed off by one `RestoreBarRenderer` row and the lines themselves are left alone. Restoring 3,000 lines costs ~18 ms before the first frame. `restore:` is the third member of the `save:`/`logRoot:` family — **null by default, so no test and no snapshot owns one**. +- **Coming back to a window you were not watching leaves a bar where you left off, and that covers two + different absences.** The *window* one is `NEW` and is the common case: a line lands while the window + is not `Workspace.IsCaughtUp` — visible **and** at its live tail — and `_missedFrom` records the index + of that first line, exactly, at the moment it happens. `MarkMissedLines` draws it when the window is + caught up again, from the only two places that can make it so (`Activate`, and `SyncScrollbackState`, + where every scroll route lands). Three things not to relitigate. **`IsCaughtUp` and not `IsVisible`**, + because that is already the unread badge's rule and one fact behind both means a badge showing a count + always has a bar under it saying where the count begins — "the badge said 3 and nothing said which 3" + was the report. **The reveal is only for *arriving*:** a pane bottom-anchors, so a deep absence's bar + is drawn far above the fold and nothing on screen would change without the jump — but ⌃End is an + explicit *take me to the live tail*, and scrolling somewhere else in answer to it is the "attention on + one pane, keystrokes to another" defect wearing a different hat (`CtrlEndGoesBackToFollowingTheLiveTail` + is the pin; the pane is re-pinned after the insert instead, because a whole-buffer re-feed leaves the + offset a frame behind and the newest line would blink off screen). **Nothing accrues before the + constructor finishes** (`_watching`): until the workspace is laid out, "not visible" means "no pane + built yet", and `RestorePreviousSession` pours a previous run through the same seam into windows that + already sit under a `RestoreBarRenderer` bar. + - **The bar retires on three conjuncts, and the third is a floor in time.** At the live tail, one input + since it was drawn, **and** `TextSettings.ActivityBarSeconds` elapsed (F7 ▸ ACTIVITY, default 30, `0` + is the old behaviour exactly). Two were not enough: a shallow absence leaves the pane at its tail, so + the next keystroke took the bar a second or two after it appeared. Time and not keystrokes because + that is the unit the complaint was in — a raised input count is an hour on a quiet character and + three seconds on a busy one. It is a **floor, not a timer**: nothing fires on its own, so the bar + goes on the first of these checks after it passes, and an untouched client keeps its bar. Measured + off the app's existing `TimeProvider`, so tests move the clock rather than racing it, and + `AwayDividerTests` sets the floor to zero so each suite asserts one rule. - **Coming back to the terminal leaves a bar where you were** (`AwayBarRenderer` + `TerminalFocusWatcher`, - Tui). Third of the boundary bars, and it earns its row the same way `FreezeBarRenderer` and + Tui). The *other* absence, and the harder one, because a terminal reports focus-in and not focus-out — + so this boundary is reconstructed from the input before the last, where the window one above is + recorded forwards. Where a window has both, the **older wins**: a reader who typed after lines landed + in a window they could not see has a terminal boundary at the end of that buffer, claiming they missed + nothing, and the window's own boundary knows better. Still one bar per window. + Third of the boundary bars, and it earns its row the same way `FreezeBarRenderer` and `RestoreBarRenderer` do: mark the *boundary*, never restyle the content. The signal is real terminal focus reporting (`CSI ?1004h`) and **both halves of getting it are workarounds**, which is why they are in one file. No released SharpConsoleUI asks for focus (verified against 2.5.18's string heap: `?2004` @@ -329,11 +360,19 @@ A headless environment can't run `NetConsoleDriver` or render Kitty graphics, bu therefore unverifiable: it renders real frames headlessly. ```bash +dotnet build -c Release SharpMUTerm.slnx # -c Release, or --no-build lies to you dotnet run -c Release --project src/SharpMUTerm.Tui --no-build -- \ --snapshot --demo-config --view --size 120x32 --out frame.ansi python3 tools/ansi_frame_to_image.py frame.ansi frame.html # or .svg ``` +- **`-c Release` on the build, and it is not a formality.** A bare `dotnet build` produces *Debug*, + `--no-build` runs the *Release* output, and nothing warns you: the snapshot renders happily from a + binary that predates your change, so a new view comes out byte-identical to the default frame and a + changed one shows the old behaviour. That reads exactly like a feature that does not work, and it + has cost real time. Either build Release first, or drop `--no-build` (`--no-build` is only there to + keep the render fast). Running the test suites also refreshes Release, which is why the trap hides + whenever you happen to have just run them. - **`--demo-config` is not optional for verification work.** Without it the snapshot renders whatever config is on the machine, and a saved `~/.config/SharpMUTerm/` quietly replaces the demo worlds — you end up checking your own data and calling it the demo. @@ -380,7 +419,10 @@ python3 tools/ansi_frame_to_image.py frame.ansi frame.html # or .svg pane is left on its live tail, and the deep one, where more arrived than the pane holds and the client has scrolled the pane to the bar itself; the second is the only frame that can show a bottom-anchored pane being "caught up" while nothing has been read, and the only one that would catch a scroll landing - at the wrong row), `prefix-panel` (the ⌃B which-key + at the wrong row), `activity-bar` (the *other* absence — a window the reader was not watching: three + lines land in the main window while Chat is in front of it, and picking main back lands on the `NEW` + bar with those three under it. Separate from `away` because the two are separate facts with separate + 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 diff --git a/src/SharpMUTerm.Tui/SharpMUTermApp.cs b/src/SharpMUTerm.Tui/SharpMUTermApp.cs index 3bea9da..c11f360 100644 --- a/src/SharpMUTerm.Tui/SharpMUTermApp.cs +++ b/src/SharpMUTerm.Tui/SharpMUTermApp.cs @@ -1023,6 +1023,20 @@ public string RenderSnapshot(string? view = null) ReArmWholeFrame(); } + // The other absence: a window the reader was not watching. `away` above is the terminal one, and + // the two are worth separate frames because they are separate facts with separate wording — this + // is the one that happens many times an hour, and the only frame where a NEW bar can be seen with + // the lines it divides above and below it. It is reached the way a reader reaches it: lines land + // in a background tab, and the tab is picked. + if (string.Equals(view, "activity-bar", StringComparison.OrdinalIgnoreCase)) + { + SimulateWindowChange(DemoScene.ChatWindowId); + LoadLongScene(MainWindowId, 3); + SettleScroll(); + SimulateWindowChange(MainWindowId); + ReArmWholeFrame(); + } + // A frozen pane whose pinned half holds far more than its three-quarters of the pane, scrolled // up inside it — the two features composing, which is the frame that says whether they do. if (string.Equals(view, "freeze-scrollback", StringComparison.OrdinalIgnoreCase)) From 5920c55df2087ea913c77e6569b32066bfce8953 Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Tue, 11 Aug 2026 15:43:28 -0500 Subject: [PATCH 15/19] =?UTF-8?q?feat(search):=20=E2=8C=83F=20opens=20the?= =?UTF-8?q?=20surface,=20=E2=8C=A5A=20widens=20it,=20=E2=8F=8E=20goes,=20?= =?UTF-8?q?=E2=8C=A5G=20walks?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The chord, the scope and the landing. ⏎ activates the window the line is really in — through Activate, the one activation path, so the pane, the tab and the session move together rather than a pane being scrolled behind the reader's back — and marks the line with a bar, fourth of the boundary bars, which points rather than repaints: the line is worth having because it is the game's own text in the game's own colours. Two things measured rather than chosen. ⌥⇧G is *not* bound: kitty writes it as CSI 103;4u, a kitty-keyboard-protocol sequence AnsiInputParser drops, so it could never arrive — ⌥⇧1's story one letter over. And Escape does not clear the bar: a claimed Escape does not set _escapeAt, and TryAltEnter pairs an unclaimed one with a following Enter to make Alt+⏎, so binding it would break the newline chord for as long as a bar was on screen. ⌥G removes the bar *before* re-running the search. The bar is itself a row, so a search run around it returns indices in a buffer about to lose one and every hit below it lands a row early — which is what the test caught. Two kinds of inserted chrome now, so the index bookkeeping every buffer mark depends on is one pair of methods (InsertChromeRow/RemoveChromeRow) rather than written out at each site: the freeze point, the pending boundary, the activity bar and the search bar all move together or none of them do. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN --- src/SharpMUTerm.Tui/Glyphs.cs | 6 + src/SharpMUTerm.Tui/MacroKeys.cs | 16 + src/SharpMUTerm.Tui/SearchBarRenderer.cs | 43 ++ src/SharpMUTerm.Tui/SearchSurface.cs | 265 ++++++++++++ src/SharpMUTerm.Tui/SharpMUTermApp.cs | 401 ++++++++++++++++-- .../SharpMUTerm.Tui.Tests/FreezeChordTests.cs | 14 +- .../SearchEndToEndTests.cs | 252 +++++++++++ 7 files changed, 968 insertions(+), 29 deletions(-) create mode 100644 src/SharpMUTerm.Tui/SearchBarRenderer.cs create mode 100644 src/SharpMUTerm.Tui/SearchSurface.cs create mode 100644 tests/SharpMUTerm.Tui.Tests/SearchEndToEndTests.cs diff --git a/src/SharpMUTerm.Tui/Glyphs.cs b/src/SharpMUTerm.Tui/Glyphs.cs index 2c16948..d16c73d 100644 --- a/src/SharpMUTerm.Tui/Glyphs.cs +++ b/src/SharpMUTerm.Tui/Glyphs.cs @@ -38,6 +38,12 @@ internal static class Glyphs /// public const string Away = "\uf070"; // nf-fa-eye_slash + /// + /// The bar marking the line \u2303F sent you to \u2014 see . A magnifying + /// glass, the one icon in this set that needs no explaining. + /// + public const string Search = "\uf002"; // nf-fa-search + /// /// The focused pane's marker, drawn on the active tab of the pane every workspace key acts on. Box /// drawing rather than a Nerd Font icon, deliberately: it is the one glyph here whose job is to be diff --git a/src/SharpMUTerm.Tui/MacroKeys.cs b/src/SharpMUTerm.Tui/MacroKeys.cs index 8d5e3e7..43f688b 100644 --- a/src/SharpMUTerm.Tui/MacroKeys.cs +++ b/src/SharpMUTerm.Tui/MacroKeys.cs @@ -117,6 +117,10 @@ private static AppShortcut[] BuildAppShortcuts() new(ConsoleModifiers.Control, ConsoleKey.P, "opens the command surface"), new(ConsoleModifiers.Control, ConsoleKey.B, "arms the pane prefix"), new(ConsoleModifiers.Control, ConsoleKey.R, "searches the command history"), + // ⌃F is find, which is what it means to everyone who has used a computer — the convention is + // worth the Ctrl chord, and freeze moved to ⌥F to make room. ⌃R searches what *you* typed; this + // searches what the *worlds* said, and the two chords sit one letter apart under one modifier. + new(ConsoleModifiers.Control, ConsoleKey.F, "searches the output"), // The connection pair, and it is a pair: ⌥D disconnects, ⌥R reconnects. One modifier, two // letters that spell the two words, opposite actions that look opposite on the keyboard. // @@ -146,6 +150,18 @@ private static AppShortcut[] BuildAppShortcuts() // No ⌃F alias is left behind, for the reason ⌃D was released rather than kept: a second key for // one action is either a secret or a duplicate row on every surface that lists chords. new(ConsoleModifiers.Alt, ConsoleKey.F, "freezes the pane"), + // The search repeat, so walking hits is one key rather than reopening the surface for each. + // + // It wraps forward and there is *no backward chord*, which was measured rather than chosen. The + // obvious partner is ⌥⇧G, and the parser would decode it — `ProcessEscape` reads the Shift flag + // out of `char.IsUpper`, so `ESC G` is Alt+Shift+G. The terminal does not send `ESC G`: read off + // a pty with `kitten @ send-key`, kitty writes ⌥⇧G as `CSI 103;4u`, a kitty-keyboard-protocol + // sequence `AnsiInputParser.DispatchCsi` has no case for and `UnixStdinReader` drops. It is + // ⌥⇧1's story exactly (`CSI 49;4u`), one letter over. A decode test is not an arrival test. + // + // So ⌥G is the whole repeat, and no surface advertises a chord that goes back — going back is + // ⌃F again, which is one keystroke more and is a key that exists. + new(ConsoleModifiers.Alt, ConsoleKey.G, "goes to the next search hit"), // The character cycle. Letters and not digits because the digit row is spent (⌥N windows, ⌃B N // panes) and there is no third digit-bearing modifier this terminal delivers: read off a pty, // kitty writes ⌥⇧1 as `CSI 49;4u` and ⌃⇧N as `CSI 110;6u` — kitty-keyboard-protocol sequences diff --git a/src/SharpMUTerm.Tui/SearchBarRenderer.cs b/src/SharpMUTerm.Tui/SearchBarRenderer.cs new file mode 100644 index 0000000..ffae197 --- /dev/null +++ b/src/SharpMUTerm.Tui/SearchBarRenderer.cs @@ -0,0 +1,43 @@ +namespace SharpMUTerm.Tui; + +/// +/// Renders the bar drawn above the line ⌃F sent you to: the one row saying which hit this is, out of how +/// many, and which key goes to the next. +/// +/// Fourth of the boundary bars, and it earns its row the same way , +/// RestoreBarRenderer and do: mark the boundary, never +/// restyle the content. Painting the matched span itself was the obvious alternative and is the wrong +/// one — the line is worth having because it is the game's own text in the game's own colours, and a +/// highlight over it would destroy the thing being pointed at. It also costs nothing in cells inside the +/// line, so a pane's rectangle does not move and no server is told its terminal changed size. +/// +/// +/// The query is escaped, because it is the reader's own text going into markup: a search for +/// [public] must appear on the bar rather than be eaten as a tag. +/// +/// Pure, so the markup is unit-testable without a terminal. +/// +internal static class SearchBarRenderer +{ + /// How long the trailing rule is. The same 48 cells the other three bars draw. + private const int RuleCells = 48; + + /// The chord the bar names, and the only one it can: see MacroKeys for why ⌥⇧G is not here. + internal const string NextChord = "⌥G next"; + + /// + /// The bar for hit of for + /// , on an already-resolved #rrggbb accent. + /// + public static string Bar(string query, int ordinal, int total, string accentHex) + { + ArgumentNullException.ThrowIfNull(query); + ArgumentException.ThrowIfNullOrEmpty(accentHex); + + // The ordinal is what makes ⌥G legible: without it the bar moves and nothing says whether you + // are getting closer to the end of the results or going round in circles. + var counted = MarkupText.Escape($"{query} ({ordinal} of {total})"); + var rule = new string('─', RuleCells); + return $"[{accentHex}]{Glyphs.Search} {counted}[/] [dim]{rule} {NextChord}[/]"; + } +} diff --git a/src/SharpMUTerm.Tui/SearchSurface.cs b/src/SharpMUTerm.Tui/SearchSurface.cs new file mode 100644 index 0000000..74121db --- /dev/null +++ b/src/SharpMUTerm.Tui/SearchSurface.cs @@ -0,0 +1,265 @@ +using SharpConsoleUI; +using SharpConsoleUI.Builders; +using SharpConsoleUI.Controls; +using SharpMUTerm.Core.Text; + +namespace SharpMUTerm.Tui; + +/// One window the search can look in: what it is called, and the plain text it holds. +/// The window's id — what hands back so the app can activate it. +/// What to call it in the window column. +/// Its lines' plain text, oldest first, indexed as the buffer indexes them. +internal readonly record struct SearchCorpus(string WindowId, string Label, IReadOnlyList Lines); + +/// +/// The search surface (⌃F): a modal list of the lines matching a query, which typing narrows. It is the +/// host and nothing else — owns what a keystroke means and what the surface +/// says, and owns the matching. The same split +/// uses, and the only reason any of it is testable without a terminal. +/// +/// Keys arrive on PreviewKeyPressed, before any control sees them, as they do for the quit prompt, +/// the settings screens and the history surface. There is no framework input control here for the same +/// reason those have none: the query is a buffer this class owns and the whole body is markup redrawn on +/// every key, so there is nothing for focus to land on and nothing to keep in step. +/// +/// +/// The corpus is read on every keystroke, not snapshotted at open. A window's buffer is trimmed +/// from the front as it grows, so an index taken a minute ago points at a different line — and ⏎ hands an +/// index back. Re-reading is also what makes a search over a live connection show what is there now. +/// +/// +internal sealed class SearchSurface +{ + /// + /// The narrowest the surface goes. Set by the footer: a surface too narrow for its own key hints + /// would wrap them onto a second row and push the list up. Checked by test against + /// rather than trusted. + /// + internal const int MinimumWidth = 78; + + /// + /// The rows the surface spends on something other than results: the query line, the count line, the + /// blank above the footer, and the footer. Keep in step with . + /// + private const int ChromeRows = 4; + + private readonly ConsoleWindowSystem _system; + private readonly Func> _corpus; + private readonly Action _go; + + private Window? _window; + private MarkupControl? _body; + private IReadOnlyList _rows = Array.Empty(); + private string _query = string.Empty; + private string? _error; + private string _scope = string.Empty; + private bool _regex; + private bool _all; + private int _held; + private int _selected = -1; + private int _contentWidth; + private int _listRows; + private int _first; + + /// + /// is asked for the windows to search — all of them, or just the focused + /// one — and is read afresh on every keystroke. is handed the chosen row, the + /// query, and its ordinal out of the total, which is what the bar above the landed line says. + /// + public SearchSurface( + ConsoleWindowSystem system, + Func> corpus, + Action go) + { + _system = system; + _corpus = corpus; + _go = go; + } + + public bool IsOpen => _window is not null; + + /// What the surface is currently showing, for a headless test to read back. + internal IReadOnlyList Lines => SearchPrompt.Render( + _rows, _query, _error, _regex, _all, _scope, _held, _selected, _contentWidth - 1, _listRows, _first); + + /// The query as typed so far. + internal string Query => _query; + + /// The rows currently listed, in buffer order — what ↑↓ walk and ⏎ picks from. + internal IReadOnlyList Rows => _rows; + + /// Whether the query is being read as a pattern. + internal bool Regex => _regex; + + /// Whether every window is being searched rather than the focused one. + internal bool AllWindows => _all; + + /// Opens the surface, or closes it when ⌃F arrives a second time. + public void Toggle() + { + if (_window is not null) + { + Close(); + return; + } + + Open(); + } + + /// Opens the surface into a headless frame (used by the search snapshot views). + public void OpenForSnapshot() => Open(); + + /// + /// Feeds one key to the very handler PreviewKeyPressed raises, for the same reason + /// exists: the framework only pumps keys inside + /// Run(), which a headless test or snapshot never enters. + /// + public void SimulateKey(ConsoleKeyInfo key) => OnKey(this, new KeyPressedEventArgs(key, false)); + + /// Types a whole query in, one real keystroke at a time. + public void SimulateTyping(string text) + { + foreach (var c in text) + { + SimulateKey(new ConsoleKeyInfo(c, ConsoleKey.NoName, false, false, false)); + } + } + + private void Open() + { + _query = string.Empty; + _error = null; + _regex = false; + _all = false; + _selected = -1; + _first = 0; + Refilter(_query, -1); + + var desktop = _system.DesktopDimensions; + + // 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. + _listRows = Math.Max(3, desktop.Height - ChromeRows - 6); + _contentWidth = Math.Clamp( + SearchPrompt.MaxWidth(Lines) + 2, MinimumWidth, 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)); + + _body = new MarkupControl(new List()); + + // Centred *after* WithSize, because the builder reads the bounds set so far and falls back to 80x25. + _window = new WindowBuilder(_system) + .WithTitle("Search output") + .AsModal() + .WithBorderStyle(BorderStyle.Single) + .WithBackgroundColor(new Color(ScreenPalette.MenuBg)) + .HideTitleButtons() + .Resizable(false) + .WithSize(width, height) + .Centered() + .AddControl(_body) + .OnClosed((_, _) => Reset()) + .Build(); + + _window.PreviewKeyPressed += OnKey; + _system.AddWindow(_window); + Paint(); + } + + private void OnKey(object? sender, KeyPressedEventArgs e) + { + if (_window is null) + { + return; + } + + var decision = SearchPrompt.Interpret(e.KeyInfo, _query, _selected, _rows.Count, _regex, _all); + e.Handled = true; + + switch (decision.Action) + { + case SearchAction.Go: + // Closed first: the reader is about to be looking at the pane this row is in, and a modal + // still painted over it would hide the thing they asked to see. + var row = _rows[_selected]; + var ordinal = _selected + 1; + var total = _rows.Count; + var query = _query; + Close(); + _go(row, query, ordinal, total); + break; + + case SearchAction.Cancel: + Close(); + break; + + case SearchAction.Redraw: + _regex = decision.Regex; + _all = decision.AllWindows; + Refilter(decision.Query, decision.Selected); + Paint(); + _window.Invalidate(redrawAll: true); + break; + + case SearchAction.None: + default: + break; + } + } + + /// + /// Re-reads the corpus and re-runs the search. The pointer is clamped to what the query actually + /// matched: a narrowing must not leave ⏎ aimed past the end of the list. + /// + private void Refilter(string query, int selected) + { + _query = query; + + var windows = _corpus(_all); + _scope = windows.Count == 1 ? windows[0].Label : "the focused window"; + _held = windows.Sum(w => w.Lines.Count); + + var rows = new List(); + string? error = null; + foreach (var window in windows) + { + var result = OutputSearch.Match(window.Lines, query, _regex); + if (result.Error is not null) + { + // One bad pattern is bad for every window, so it is reported once and the list is empty + // — a result set holding whatever the earlier windows managed would be a search that + // half-worked without saying which half. + error = result.Error; + rows.Clear(); + break; + } + + rows.AddRange(result.Matches.Select(m => + new SearchRow(window.WindowId, window.Label, m.LineIndex, m.Text, m.MatchStart, m.MatchLength))); + } + + _error = error; + _rows = rows; + _selected = _rows.Count == 0 ? -1 : Math.Clamp(selected, 0, _rows.Count - 1); + _first = SearchPrompt.Scroll(_first, _selected, _rows.Count, _listRows); + } + + private void Paint() => _body?.SetContent(new List(Lines)); + + private void Close() + { + if (_window is { } window) + { + _system.CloseModalWindow(window); + } + } + + private void Reset() + { + _window = null; + _body = null; + } +} diff --git a/src/SharpMUTerm.Tui/SharpMUTermApp.cs b/src/SharpMUTerm.Tui/SharpMUTermApp.cs index ea338c0..c3d7ada 100644 --- a/src/SharpMUTerm.Tui/SharpMUTermApp.cs +++ b/src/SharpMUTerm.Tui/SharpMUTermApp.cs @@ -265,6 +265,26 @@ private sealed class SizeReport /// private readonly HistorySurface _historySearch; + /// + /// The ⌃F search surface: the lines this client is holding, filtered by typing. ⏎ there goes to a + /// line and marks it; it never sends anything. Its sibling one letter away, ⌃R, searches what + /// you typed — this searches what the worlds said. + /// + private readonly SearchSurface _search; + + /// + /// The last search's terms, kept so ⌥G can walk to the next hit without reopening the surface: the + /// query, whether it was a pattern, and whether it covered every window. Null until something has + /// been searched for, which is what ⌥G refuses on. + /// + private (string Query, bool Regex, bool All)? _lastSearch; + + /// + /// Where the search bar sits, or null when no hit has been landed on. One client-wide, not one per + /// window: it marks the hit you went to, and ⌥G moves it rather than leaving a trail. + /// + private (string WindowId, int Index)? _searchMark; + /// Whether a confirmed quit has asked the loop to end — the headless view of the exit. private bool _exiting; @@ -684,6 +704,8 @@ public SharpMUTermApp( HistoryBarLabel, InsertHistoryEntry); + _search = new SearchSurface(_system, SearchableWindows, GoToSearchHit); + _window.OnResize += (_, _) => { // NAWS is deliberately not reported from here. At this moment the panes still carry the @@ -2448,6 +2470,14 @@ private void AppendWindowLine(string windowId, string markup, string? stamp = nu _awayMarks.Remove(windowId); } } + + // Same rule for the search bar: a bar trimmed off the top is gone, and a mark left pointing + // at row zero would have the next removal take a line of the game's output instead. + if (_searchMark is { } search && string.Equals(search.WindowId, windowId, StringComparison.Ordinal)) + { + var moved = search.Index - excess; + _searchMark = moved < 0 ? null : (windowId, moved); + } } if (_panes.TryGetValue(windowId, out var control)) @@ -2726,11 +2756,7 @@ private void MarkWhereTheReaderLeft(TimeSpan away) continue; } - buffer.Insert(at, new PaneLine(AwayBarRenderer.Bar(missed, away, accent))); - if (_freezePoints.TryGetValue(windowId, out var freeze) && freeze > at) - { - _freezePoints[windowId] = freeze + 1; - } + InsertChromeRow(windowId, at, AwayBarRenderer.Bar(missed, away, accent)); var mark = new AwayMark { Index = at, DrawnAfter = _focus.InputCount, DrawnAt = _time.GetUtcNow() }; _awayMarks[windowId] = mark; @@ -2801,11 +2827,7 @@ private void MarkMissedLines(bool reveal) at = Math.Clamp(at, 0, buffer.Count); } - buffer.Insert(at, new PaneLine(AwayBarRenderer.Missed(buffer.Count - at, FrozenAccentHex()))); - if (_freezePoints.TryGetValue(windowId, out var freeze) && freeze > at) - { - _freezePoints[windowId] = freeze + 1; - } + InsertChromeRow(windowId, at, AwayBarRenderer.Missed(buffer.Count - at, FrozenAccentHex())); var mark = new AwayMark { @@ -2984,42 +3006,271 @@ private void ConsumeReadAwayBars() } /// - /// Takes a window's away bar out of its line buffer, moving everything that indexes into that buffer - /// past it — the freeze point and the pending boundary — down by the row it freed. Does not repaint: - /// the callers either follow with one or are about to insert a replacement. + /// Puts one row of the client's own chrome into a window's line buffer, and moves everything that + /// indexes into that buffer past it up by the row it took. + /// + /// There are two kinds of inserted chrome now — the activity bar and the search bar — and every index + /// into a buffer has to survive both: the freeze point, the pending boundary, the other bar, and this + /// one. That bookkeeping lives here and in rather than being written out + /// at each site, because a site that forgot one of them would leave a mark pointing at a line of the + /// game's output, and the next removal would take that line instead. + /// + /// + /// It inserts, and does not repaint — the callers do, because they also have a reveal to sequence. + /// The row carries no plain text, so a search cannot find it (see ). + /// /// - /// Whether there was a bar to remove. - private bool RemoveAwayBar(string windowId) + private void InsertChromeRow(string windowId, int at, string markup) { - if (!_awayMarks.TryGetValue(windowId, out var mark) - || !_lines.TryGetValue(windowId, out var buffer) - || mark.Index < 0 - || mark.Index >= buffer.Count) + if (!_lines.TryGetValue(windowId, out var buffer)) { - return _awayMarks.Remove(windowId); + return; } - buffer.RemoveAt(mark.Index); - _awayMarks.Remove(windowId); + at = Math.Clamp(at, 0, buffer.Count); + buffer.Insert(at, new PaneLine(markup)); + + if (_freezePoints.TryGetValue(windowId, out var freeze) && freeze > at) + { + _freezePoints[windowId] = freeze + 1; + } - if (_freezePoints.TryGetValue(windowId, out var freeze) && freeze > mark.Index) + if (_awayMarks.TryGetValue(windowId, out var mark) && mark.Index >= at) + { + mark.Index++; + } + + if (_searchMark is { } search + && string.Equals(search.WindowId, windowId, StringComparison.Ordinal) + && search.Index >= at) + { + _searchMark = (windowId, search.Index + 1); + } + } + + /// + /// Takes one row of the client's own chrome out of a window's line buffer, moving everything that + /// indexes into that buffer past it down by the row it freed. The other half of + /// , and the same reason for existing. + /// + private void RemoveChromeRow(string windowId, int at) + { + if (!_lines.TryGetValue(windowId, out var buffer) || at < 0 || at >= buffer.Count) + { + return; + } + + buffer.RemoveAt(at); + + if (_freezePoints.TryGetValue(windowId, out var freeze) && freeze > at) { _freezePoints[windowId] = freeze - 1; } - if (_awayPending.TryGetValue(windowId, out var pending) && pending > mark.Index) + if (_awayPending.TryGetValue(windowId, out var pending) && pending > at) { _awayPending[windowId] = pending - 1; } - if (_awayBoundary.TryGetValue(windowId, out var boundary) && boundary > mark.Index) + if (_awayBoundary.TryGetValue(windowId, out var boundary) && boundary > at) { _awayBoundary[windowId] = boundary - 1; } + if (_missedFrom.TryGetValue(windowId, out var missed) && missed > at) + { + _missedFrom[windowId] = missed - 1; + } + + if (_awayMarks.TryGetValue(windowId, out var mark) && mark.Index > at) + { + mark.Index--; + } + + if (_searchMark is { } search + && string.Equals(search.WindowId, windowId, StringComparison.Ordinal) + && search.Index > at) + { + _searchMark = (windowId, search.Index - 1); + } + } + + /// + /// Takes a window's away bar out of its line buffer. Does not repaint: the callers either follow with + /// one or are about to insert a replacement. The index bookkeeping is 's, + /// which is the one place that knows everything pointing into a buffer. + /// + /// Whether there was a bar to remove. + private bool RemoveAwayBar(string windowId) + { + if (!_awayMarks.TryGetValue(windowId, out var mark)) + { + return false; + } + + var at = mark.Index; + _awayMarks.Remove(windowId); + RemoveChromeRow(windowId, at); return true; } + /// + /// Takes the search bar off whichever window is carrying it, and forgets where it was. Does not + /// repaint — every caller either repaints that pane or is about to put a new bar somewhere else. + /// + /// The window the bar was in, or null when there was no bar. + private string? RemoveSearchBar() + { + if (_searchMark is not { } mark) + { + return null; + } + + _searchMark = null; + RemoveChromeRow(mark.WindowId, mark.Index); + return mark.WindowId; + } + + /// + /// Goes to one search hit: activates its window, marks the line with a bar, and scrolls the pane so + /// the bar is on screen. + /// + /// Activation goes through , the one activation path — it selects the + /// pane, raises the tab and adopts the session, so ⏎ on a hit in a background pane leaves the client + /// in a consistent state rather than merely scrolling something the reader is not looking at. That is + /// the same reasoning that keeps every other "bring this window forward" gesture on that method. + /// + /// + /// One bar client-wide. It marks the hit you went to; ⌥G moves it rather than leaving + /// a trail behind, and a second search replaces it. It is not cleared by Escape: a claimed + /// Escape does not set _escapeAt, and pairs an unclaimed one with a + /// following Enter to make Alt+⏎ — so binding Escape here would break the newline chord for as long as + /// a search bar was on screen, which is a defect nobody would connect to search. + /// + /// + private void GoToSearchHit(SearchRow row, string query, int ordinal, int total) + { + _lastSearch = (query, _search.Regex, _search.AllWindows); + + var previous = RemoveSearchBar(); + if (previous is { } cleared && !string.Equals(cleared, row.WindowId, StringComparison.Ordinal)) + { + RepaintPane(cleared); + } + + if (!_lines.TryGetValue(row.WindowId, out var buffer)) + { + RefuseCommand("that window is gone"); + return; + } + + Activate(row.WindowId); + + var at = Math.Clamp(row.LineIndex, 0, buffer.Count); + InsertChromeRow(row.WindowId, at, SearchBarRenderer.Bar(query, ordinal, total, FrozenAccentHex())); + _searchMark = (row.WindowId, at); + + RepaintPane(row.WindowId); + RevealSearchBar(row.WindowId, at); + SyncScrollbackState(); + } + + /// + /// Goes to the hit after the one the bar is on, wrapping — the whole of ⌥G. It re-runs the last + /// search rather than keeping a result list, because the buffers move underneath one: lines arrive, + /// and a trim takes them off the front. Re-running also means ⌥G finds a hit that arrived since. + /// + private void NextSearchHit() + { + if (_lastSearch is not { } last) + { + RefuseCommand("nothing has been searched for yet — ⌃F searches the output"); + return; + } + + // The bar comes off *before* the search is re-run, and that is not tidiness: the bar is itself a + // row in the buffer, so a search run around it returns indices in a buffer that is about to lose + // one — every hit below the bar would be off by one, and the next ⌥G would land a row early. With + // it gone, the hit it was marking sits at exactly the index the bar had, which is also how "the + // one after this" is found below. + var was = _searchMark; + if (RemoveSearchBar() is { } cleared) + { + RepaintPane(cleared); + } + + var rows = new List(); + foreach (var window in SearchableWindows(last.All)) + { + var result = OutputSearch.Match(window.Lines, last.Query, last.Regex); + if (result.Error is not null) + { + RefuseCommand($"that search no longer works: {result.Error}"); + return; + } + + rows.AddRange(result.Matches.Select(m => + new SearchRow(window.WindowId, window.Label, m.LineIndex, m.Text, m.MatchStart, m.MatchLength))); + } + + if (rows.Count == 0) + { + RefuseCommand($"no lines hold “{Snippet(last.Query)}” any more"); + return; + } + + // The one after the hit the bar was on, wrapping. Found in the flat list rather than by index + // within a window, so with ⌥A on it walks out of one window and into the next in the order the + // surface listed them. A bar whose line has since been trimmed away is simply not found, and the + // walk starts again from the first hit — which is the only answer left, and a defensible one. + var next = 0; + if (was is { } mark) + { + var current = rows.FindIndex(r => + string.Equals(r.WindowId, mark.WindowId, StringComparison.Ordinal) && r.LineIndex == mark.Index); + next = current >= 0 ? (current + 1) % rows.Count : 0; + } + + GoToSearchHit(rows[next], last.Query, next + 1, rows.Count); + } + + /// + /// Scrolls a pane so a freshly drawn search bar is on screen, with the line it marks under it. The + /// away bar's arithmetic, and for its reason: a buffer index is not a viewport row — the panel's + /// offset counts display rows and a buffered line wraps into as many as it needs, so in a + /// narrow pane scrolling to the index lands hundreds of rows adrift. + /// + /// Unlike the away bar's reveal, this one runs whether or not the bar is already in view: the reader + /// asked to be taken to this line, so leaving the pane where it was would be answering "go there" + /// with "it is already roughly there". + /// + /// + private void RevealSearchBar(string windowId, int index) + { + if (_paneScrolls.GetValueOrDefault(windowId) is not { } panel + || !_lines.TryGetValue(windowId, out var buffer) + || panel.ViewportWidth <= 0 + || panel.ViewportHeight <= 0) + { + return; + } + + var origin = _freezePoints.TryGetValue(windowId, out var split) ? Math.Max(0, split) : 0; + if (index < origin) + { + return; + } + + var tailRows = MeasureRows(buffer, index, panel.ViewportWidth, _panes.GetValueOrDefault(windowId)); + if (tailRows <= 0) + { + return; + } + + var target = Math.Max(0, panel.TotalContentHeight - tailRows); + panel.ScrollVerticalBy(target - panel.VerticalScrollOffset); + } + /// /// Drives the return the terminal's focus report would have driven. The seam a headless test uses: /// is false for a headless driver by design, so the @@ -3450,7 +3701,68 @@ private void ToggleHistorySearch() /// private bool AnyOverlayOpen => _palette.IsOpen || _settings.IsOpen || _quit.IsOpen || _messageLog.IsOpen || _historySearch.IsOpen - || _prefixPanel.IsOpen || _composer.IsOpen; + || _prefixPanel.IsOpen || _composer.IsOpen || _search.IsOpen; + + /// + /// Opens the ⌃F search surface, or closes it when the chord arrives again. + /// + /// It refuses over any other overlay, and says so over the composer for the reason the composer + /// refuses over a settings screen: two modal windows with two PreviewKeyPressed handlers + /// cannot be driven headlessly, and `SettingsOverlay` takes paste off the *driver* because its + /// screens have no focusable target — a second modal in front of it would make both fire. + /// + /// + private void ToggleSearch() + { + if (_search.IsOpen) + { + _search.Toggle(); + return; + } + + if (_composer.IsOpen) + { + RefuseCommand("close the composer first — search cannot open over it"); + return; + } + + if (AnyOverlayOpen || _moveMode) + { + return; + } + + _search.Toggle(); + } + + /// + /// The windows ⌃F looks in: the focused one, or every window holding output when + /// — which is what ⌥A switches. + /// + /// What is searched is the pane buffer, and not a session's Scrollback or the + /// file-backed spill. RestoreLog's reasoning, one layer over: a spawn window's lines never + /// reach a session's scrollback at all (a gagging capture rule keeps them out of the transcript + /// entirely), so a session-keyed search would find nothing in exactly the windows people search + /// hardest. The surface states the bound it did search rather than implying a bigger one. + /// + /// + /// The web view is excluded because its pane is not fed from this buffer — the same exclusion the + /// 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). + /// + /// + private IReadOnlyList SearchableWindows(bool all) + { + var ids = all + ? _lines.Keys.Where(id => !string.Equals(id, WebWindowId, StringComparison.Ordinal)) + : new[] { ActiveWindowId() }.Where(_lines.ContainsKey); + + return ids + .Select(id => new SearchCorpus( + id, + Snippet(WindowTitle(id)), + _lines[id].Select(line => line.Plain).ToArray())) + .ToArray(); + } /// /// Refuses a surface that would open over the composer, naming what is in the way. The @@ -3491,6 +3803,7 @@ private string OpenOverlayName() => : _quit.IsOpen ? "the quit prompt" : _messageLog.IsOpen ? "the client messages" : _historySearch.IsOpen ? "the history search" + : _search.IsOpen ? "the search surface" : _prefixPanel.IsOpen ? "the pane keys panel" : "what is open"; @@ -4911,6 +5224,14 @@ private void RegisterFocusReportTab() return () => { CycleCharacter(-1); return true; }; } + // ⌥G walks to the next hit of the last search, so a reader following a name down a transcript + // presses one key rather than reopening the surface for each. There is no backward chord — + // see MacroKeys, where the measurement that rules ⌥⇧G out is recorded. + if (claim.Key == ConsoleKey.G) + { + return () => { NextSearchHit(); return true; }; + } + // ⌥F freezes and resumes the focused pane. It was ⌃F, and moved so that search could have the // chord every reader on every platform reaches for. Same delivery story as ⌥D and ⌥R: ESC + a // printable byte, decoded as that letter with Alt set. @@ -4953,6 +5274,10 @@ private void RegisterFocusReportTab() // framework's parser turns byte 0x08 into Backspace with no Control modifier, so binding it // would take the command line's erase key and the app could not even tell the two apart. ConsoleKey.R => () => { ToggleHistorySearch(); return true; }, + // ⌃F is find. Freeze had it and moved to ⌥F: the convention is what every reader on every + // platform reaches for, and ⌃R one letter away searches the other half of a session — what + // you typed, where this is what the worlds said. + ConsoleKey.F => () => { ToggleSearch(); return true; }, _ => null, }; } @@ -8275,6 +8600,30 @@ private bool RouteToInput(ConsoleKeyInfo key) /// internal void SimulateHistorySearchKey(ConsoleKeyInfo key) => _historySearch.SimulateKey(key); + /// Whether the ⌃F search surface is up. + internal bool SearchIsOpen => _search.IsOpen; + + /// What the search surface is currently listing — what ↑↓ walk and ⏎ picks from. + internal IReadOnlyList SearchRows => _search.Rows; + + /// Feeds one key to the search surface's own handler, as the framework's pump would. + internal void SimulateSearchKey(ConsoleKeyInfo key) => _search.SimulateKey(key); + + /// Types a whole query into the search surface, one real keystroke at a time. + internal void SimulateSearchTyping(string text) => _search.SimulateTyping(text); + + /// Opens the search surface for a snapshot frame, bypassing the chord's overlay guards. + internal void OpenSearchForSnapshot() => _search.OpenForSnapshot(); + + /// + /// Where the search bar sits in a window's line buffer, or null when that window is not carrying it. + /// There is only ever one, client-wide. + /// + internal int? SearchBarIndex(string windowId) => + _searchMark is { } mark && string.Equals(mark.WindowId, windowId, StringComparison.Ordinal) + ? mark.Index + : null; + /// Types a filter into the open ⌃R surface, one real keystroke at a time. internal void SimulateHistorySearchTyping(string text) => _historySearch.SimulateTyping(text); diff --git a/tests/SharpMUTerm.Tui.Tests/FreezeChordTests.cs b/tests/SharpMUTerm.Tui.Tests/FreezeChordTests.cs index baf2ce5..94d986e 100644 --- a/tests/SharpMUTerm.Tui.Tests/FreezeChordTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/FreezeChordTests.cs @@ -59,12 +59,20 @@ public async Task CtrlFNoLongerFreezesAnything() await Assert.That(app.FrozenScrollbackOf(Main)).IsNull(); } + /// + /// Both halves of the move: ⌥F is freeze, and ⌃F is claimed by something that is not freeze. The + /// second half matters because the whole point of the move was to hand ⌃F to find — a claim list + /// where it had simply gone missing would mean the chord had been spent on nothing. + /// [Test] - public async Task TheClaimListNamesAltFAndNoLongerNamesCtrlF() + public async Task TheClaimListNamesAltFForFreezeAndCtrlFForSomethingElse() { var claims = MacroKeys.AppShortcuts; - await Assert.That(claims.Any(c => c.Modifiers == ConsoleModifiers.Alt && c.Key == ConsoleKey.F)).IsTrue(); - await Assert.That(claims.Any(c => c.Modifiers == ConsoleModifiers.Control && c.Key == ConsoleKey.F)).IsFalse(); + var alt = claims.Single(c => c.Modifiers == ConsoleModifiers.Alt && c.Key == ConsoleKey.F); + await Assert.That(alt.Does).Contains("freeze"); + + var ctrl = claims.Single(c => c.Modifiers == ConsoleModifiers.Control && c.Key == ConsoleKey.F); + await Assert.That(ctrl.Does).DoesNotContain("freeze"); } } diff --git a/tests/SharpMUTerm.Tui.Tests/SearchEndToEndTests.cs b/tests/SharpMUTerm.Tui.Tests/SearchEndToEndTests.cs new file mode 100644 index 0000000..f4a9082 --- /dev/null +++ b/tests/SharpMUTerm.Tui.Tests/SearchEndToEndTests.cs @@ -0,0 +1,252 @@ +using SharpConsoleUI.Drivers; +using SharpMUTerm.Core.Session; +using SharpMUTerm.Graphics; +using SharpMUTerm.Tui; + +namespace SharpMUTerm.Tui.Tests; + +/// +/// The ⌃F surface driven through the chord the app actually registers. +/// pins what the surface means and says; these pin that the client is wired to it — that the chord opens +/// it, that ⌥A widens what it looks at, that ⏎ takes the reader to the window the line is really in, and +/// that nothing it does reaches the wire. +/// +/// +/// Serialised for the reason every file that renders a frame is: rendering redirects the process-global +/// Console.Out, and the harness redirects Console.In. +/// +[NotInParallel] +public class SearchEndToEndTests +{ + private const int Width = 140; + private const int Height = 40; + private const string Main = "main"; + + private static readonly TerminalCapabilities Headless = + new(GraphicsProtocol.None, supportsTrueColor: true, supportsKittyGraphics: false, supportsSixel: false); + + private static (SharpMUTermApp App, WorldSession Session) Bound() + { + Console.SetIn(TextReader.Null); + var config = DemoScene.Build(); + config.ScrollbackSpill.Enabled = false; + var app = new SharpMUTermApp(config, Headless, new HeadlessConsoleDriver(Width, Height)); + var session = app.BindWorldWithoutConnecting(config.Worlds[0]); + app.RenderSnapshot(); + return (app, session); + } + + private static ConsoleKeyInfo Ctrl(ConsoleKey key) => new('\0', key, false, false, true); + + private static ConsoleKeyInfo Alt(ConsoleKey key) => new('\0', key, false, true, false); + + private static ConsoleKeyInfo Bare(ConsoleKey key) => new('\0', key, false, false, false); + + [Test] + public async Task CtrlFOpensTheSurfaceAndCtrlFAgainClosesIt() + { + var (app, _) = Bound(); + + app.SimulateKey(Ctrl(ConsoleKey.F)); + await Assert.That(app.SearchIsOpen).IsTrue(); + + app.SimulateKey(Ctrl(ConsoleKey.F)); + await Assert.That(app.SearchIsOpen).IsFalse(); + } + + [Test] + public async Task TypingListsTheLinesHoldingTheQuery() + { + var (app, session) = Bound(); + session.PrintSystem("*** The goblin snarls at you."); + session.PrintSystem("*** A town guard stands watch."); + session.PrintSystem("*** You hit the goblin."); + + app.SimulateKey(Ctrl(ConsoleKey.F)); + app.SimulateSearchTyping("goblin"); + + await Assert.That(app.SearchRows.Count).IsEqualTo(2); + await Assert.That(app.SearchRows.All(r => r.WindowId == Main)).IsTrue(); + } + + /// + /// The scope toggle, and the reason it exists: the hit that matters is usually in a pane you are not + /// looking at. Narrow first — the focused window — and ⌥A widens. + /// + [Test] + public async Task AltAWidensFromTheFocusedWindowToEveryWindow() + { + var (app, session) = Bound(); + session.PrintSystem("*** the goblin snarls at you"); + app.SimulateWindowChange(DemoScene.ChatWindowId); + app.SimulateWindowChange(Main); + + app.SimulateKey(Ctrl(ConsoleKey.F)); + app.SimulateSearchTyping("the"); + var focused = app.SearchRows.Count; + + app.SimulateSearchKey(Alt(ConsoleKey.A)); + + await Assert.That(app.SearchRows.Count).IsGreaterThan(focused); + await Assert.That(app.SearchRows.Any(r => r.WindowId != Main)).IsTrue(); + } + + /// + /// ⌥E is the difference between a query and a pattern, and the frame says which way it is set — this + /// is the behaviour behind that label. + /// + [Test] + public async Task AltESwitchesTheQueryToAPattern() + { + var (app, session) = Bound(); + session.PrintSystem("*** You hit the goblin for 12 damage."); + session.PrintSystem("*** You hit the goblin for no damage."); + + app.SimulateKey(Ctrl(ConsoleKey.F)); + app.SimulateSearchTyping(@"\d+ damage"); + await Assert.That(app.SearchRows).IsEmpty(); + + app.SimulateSearchKey(Alt(ConsoleKey.E)); + + await Assert.That(app.SearchRows.Count).IsEqualTo(1); + } + + /// + /// The headline: ⏎ on a hit in a window the reader is not looking at takes them to that + /// window, not merely to that line. Activation is the app's one path, so the pane, the tab and the + /// session all move together. + /// + [Test] + public async Task EnterGoesToTheWindowTheLineIsActuallyIn() + { + var (app, session) = Bound(); + session.PrintSystem("*** the vault key is behind the bar"); + app.SimulateWindowChange(DemoScene.ChatWindowId); + + app.SimulateKey(Ctrl(ConsoleKey.F)); + app.SimulateSearchKey(Alt(ConsoleKey.A)); + app.SimulateSearchTyping("vault key"); + await Assert.That(app.SearchRows.Count).IsEqualTo(1); + + app.SimulateSearchKey(Bare(ConsoleKey.Enter)); + + await Assert.That(app.SearchIsOpen).IsFalse(); + await Assert.That(app.ActiveWindowId()).IsEqualTo(Main); + } + + /// + /// And it marks where it took them: a bar directly above the line, saying which hit this is out of + /// how many and which key goes to the next. + /// + [Test] + public async Task TheBarSitsDirectlyAboveTheLineItSentYouTo() + { + var (app, session) = Bound(); + session.PrintSystem("*** the vault key is behind the bar"); + + app.SimulateKey(Ctrl(ConsoleKey.F)); + app.SimulateSearchTyping("vault key"); + app.SimulateSearchKey(Bare(ConsoleKey.Enter)); + + var index = app.SearchBarIndex(Main); + await Assert.That(index).IsNotNull(); + + var rows = app.PaneLines(Main); + await Assert.That(rows[index!.Value]).Contains("(1 of 1)"); + await Assert.That(rows[index.Value]).Contains(SearchBarRenderer.NextChord); + await Assert.That(rows[index.Value + 1]).Contains("vault key"); + } + + /// ⌥G walks to the next hit and wraps, without the surface being reopened. + [Test] + public async Task AltGWalksToTheNextHitAndWraps() + { + var (app, session) = Bound(); + session.PrintSystem("*** the goblin snarls"); + session.PrintSystem("*** a quiet line"); + session.PrintSystem("*** the goblin falls"); + + app.SimulateKey(Ctrl(ConsoleKey.F)); + app.SimulateSearchTyping("goblin"); + app.SimulateSearchKey(Bare(ConsoleKey.Enter)); + var first = app.SearchBarIndex(Main)!.Value; + + app.SimulateKey(Alt(ConsoleKey.G)); + var second = app.SearchBarIndex(Main)!.Value; + await Assert.That(second).IsGreaterThan(first); + await Assert.That(app.PaneLines(Main)[second + 1]).Contains("goblin falls"); + + // And round again, rather than stopping at the end with nothing said. + app.SimulateKey(Alt(ConsoleKey.G)); + await Assert.That(app.PaneLines(Main)[app.SearchBarIndex(Main)!.Value + 1]).Contains("goblin snarls"); + } + + [Test] + public async Task AltGWithNothingSearchedForYetRefusesOutLoud() + { + var (app, _) = Bound(); + + app.SimulateKey(Alt(ConsoleKey.G)); + + await Assert.That(app.Messages.Entries.Any(m => m.Text.Contains("nothing has been searched for"))).IsTrue(); + } + + /// One bar, client-wide: a second landing moves it rather than leaving a trail. + [Test] + public async Task ASecondLandingMovesTheBarRatherThanAddingOne() + { + var (app, session) = Bound(); + session.PrintSystem("*** the goblin snarls"); + session.PrintSystem("*** the goblin falls"); + + app.SimulateKey(Ctrl(ConsoleKey.F)); + app.SimulateSearchTyping("goblin"); + app.SimulateSearchKey(Bare(ConsoleKey.Enter)); + app.SimulateKey(Alt(ConsoleKey.G)); + + var bars = app.PaneLines(Main).Count(l => l.Contains(Glyphs.Search)); + await Assert.That(bars).IsEqualTo(1); + } + + /// + /// The surface is modal chrome and sends nothing. A connected recording transport is the point: with + /// an unconnected session every "nothing reached the wire" assertion passes whatever the surface did. + /// + [Test] + public async Task NothingTheSurfaceDoesReachesTheWire() + { + Console.SetIn(TextReader.Null); + var config = DemoScene.Build(); + config.ScrollbackSpill.Enabled = false; + var app = new SharpMUTermApp(config, Headless, new HeadlessConsoleDriver(Width, Height)); + var telnet = new RecordingTelnetSession(); + app.TelnetFactory = _ => telnet; + var session = app.BindWorldWithoutConnecting(config.Worlds[0]); + await session.ConnectAsync(); + app.RenderSnapshot(); + session.PrintSystem("*** the goblin snarls"); + var before = telnet.Lines.Count; + + app.SimulateKey(Ctrl(ConsoleKey.F)); + app.SimulateSearchTyping("goblin"); + app.SimulateSearchKey(Bare(ConsoleKey.Enter)); + + await Assert.That(telnet.Lines.Count).IsEqualTo(before); + } + + /// + /// It refuses over the composer and says which surface is in the way — the composer's own guard is the + /// other half, and together they make the pair mutually exclusive rather than one-sided. + /// + [Test] + public async Task ItRefusesOverTheComposerAndSaysSo() + { + var (app, _) = Bound(); + app.SimulateKey(Bare(ConsoleKey.F1)); + + app.SimulateKey(Ctrl(ConsoleKey.F)); + + await Assert.That(app.SearchIsOpen).IsFalse(); + await Assert.That(app.Messages.Entries.Any(m => m.Text.Contains("composer"))).IsTrue(); + } +} From bee70319b336f440a2dfdb157bc411aea544d404 Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Tue, 11 Aug 2026 15:48:22 -0500 Subject: [PATCH 16/19] docs(search): the four frames, the brief, and the demo's own scene MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four views, because four things about ⌃F are only visible in a frame: a plain query with its hits marked, the same query read as a pattern (the header is the only place either state is said — compose/compose-literal's reasoning), the widened scope where the window column appears, and what ⏎ leaves behind. The last is over a split, so the pane is narrower than the terminal: that is the geometry that catches a landing scrolled to the wrong row. The demo scene now loads with _watching off. It pours a spawn window's whole history in before the first frame, so every line counted as missed and any frame that later made such a window visible carried an activity bar reporting the client's own setup as news — which is exactly what the first cut of the landed frame showed. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN --- CLAUDE.md | 49 ++++++++++-- docs/design/README.md | 3 +- src/SharpMUTerm.Tui/SharpMUTermApp.cs | 74 +++++++++++++++++++ .../SearchBarRendererTests.cs | 60 +++++++++++++++ 4 files changed, 180 insertions(+), 6 deletions(-) create mode 100644 tests/SharpMUTerm.Tui.Tests/SearchBarRendererTests.cs diff --git a/CLAUDE.md b/CLAUDE.md index f07a8f1..dab94d1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -103,6 +103,39 @@ fallbacks) for inline images/maps. Restored content is closed off by one `RestoreBarRenderer` row and the lines themselves are left alone. Restoring 3,000 lines costs ~18 ms before the first frame. `restore:` is the third member of the `save:`/`logRoot:` family — **null by default, so no test and no snapshot owns one**. +- **`⌃F` searches the output the client is holding** (`OutputSearch`, Core; `SearchPrompt`/`SearchSurface` + + `SearchBarRenderer`, Tui). A modal results surface — the `⌃R` idiom, and the same pure-prompt/host + split — over the pane buffers, with `⌥E` for regex, `⌥A` to widen from the focused window to every + window, `⏎` to go, and `⌥G` to walk to the next hit. Decisions worth not relitigating: + - **What is searched is the pane buffer**, not `WorldSession.Scrollback` and not the file-backed spill. + `RestoreLog`'s reasoning one layer over: a spawn window's lines reach neither, so a session-keyed + 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. + - **`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 + every window on every keystroke. Chrome rows carry none, so a search cannot find its own bars. + - **Case is ignored in both modes**, matching `HistorySearch`; `(?-i)` is the way back, which is why + there is no third toggle. An invalid pattern is a *state* (a regex is unparseable most of the time it + is being typed), and there is a match timeout, because this runs on the UI thread per keystroke. + - **`⏎` goes through `Activate`**, the one activation path, so a hit in a background pane brings the + pane, the tab and the session forward together rather than scrolling something nobody is looking at. + One bar client-wide; `⌥G` moves it rather than leaving a trail. + - **`⌥G` removes the bar before re-running the search.** The bar is itself a row, so a search run + around it returns indices in a buffer about to lose one and every hit below it lands a row early. + - **Escape does not clear the bar.** A claimed Escape does not set `_escapeAt`, and `TryAltEnter` pairs + an unclaimed one with a following Enter to make `⌥⏎` — binding it here would break the newline chord + for as long as a bar was on screen, which nobody would connect to search. + - **There is no backward chord, and that was measured.** `⌥⇧G` would decode (`ProcessEscape` reads + Shift out of `char.IsUpper`), but kitty writes it as `CSI 103;4u` — a kitty-keyboard-protocol + sequence `DispatchCsi` drops. `⌥⇧1`'s story one letter over; a decode test is not an arrival test. + - **Two kinds of client chrome now live in the line buffers**, so `InsertChromeRow`/`RemoveChromeRow` + are the one place that fixes up everything indexing into one — the freeze point, the pending + boundary, the activity bar and the search bar move together or none of them do. + - **The demo scene loads with `_watching` off.** It pours a spawn window's whole history in before the + first frame, and every line would otherwise count as missed — so any frame that later made such a + window visible carried an activity bar reporting the client's own setup as news. - **Coming back to a window you were not watching leaves a bar where you left off, and that covers two different absences.** The *window* one is `NEW` and is the common case: a line lands while the window is not `Workspace.IsCaughtUp` — visible **and** at its live tail — and `_missedFrom` records the index @@ -419,7 +452,13 @@ python3 tools/ansi_frame_to_image.py frame.ansi frame.html # or .svg pane is left on its live tail, and the deep one, where more arrived than the pane holds and the client has scrolled the pane to the bar itself; the second is the only frame that can show a bottom-anchored pane being "caught up" while nothing has been read, and the only one that would catch a scroll landing - at the wrong row), `activity-bar` (the *other* absence — a window the reader was not watching: three + at the wrong row), `search`/`search-regex`/`search-all`/`search-landed` (the ⌃F surface: a plain + query with its hits marked, the same query read as a *pattern* — the header is the only place + either state is said, which is why they are a pair, `compose`/`compose-literal`'s reasoning — the + widened scope, where the window column appears and a background pane's hit is listed under it, and + what ⏎ leaves behind. The last is over a **split**, so the pane is narrower than the terminal: + that is the geometry that catches a landing scrolled to the wrong row, since a buffer index is not + a viewport row), `activity-bar` (the *other* absence — a window the reader was not watching: three lines land in the main window while Chat is in front of it, and picking main back lands on the `NEW` bar with those three under it. Separate from `away` because the two are separate facts with separate wording, and this is the one that happens many times an hour), `prefix-panel` (the ⌃B which-key @@ -586,11 +625,11 @@ markup (`[bold #rrggbb on #rrggbb]…[/]`, `[[`/`]]` escaping, `[link=url]…[/] - **Deliberately left on Ctrl**, because the convention is worth more than the pattern: `⌃R` (readline's reverse history search), `⌃P` (command surface), `⌃Q` (quit — and safe here because `TerminalRawMode` clears `IXON`, so it is not XON), `⌃B` (tmux's prefix), `⌃O` (pane cycle), - `⌃N`/`⌃W`, and the command line's `⌃A`/`⌃E`/`⌃K`/`⌃U`/`⌃L`. A sweep that moved everything + `⌃N`/`⌃W`/`⌃F` (find), and the command line's `⌃A`/`⌃E`/`⌃K`/`⌃U`/`⌃L`. A sweep that moved everything would be as wrong as one that moved nothing. - - **Freeze is `⌥F`, and `⌃F` is find.** Freeze was on `⌃F` and left it for exactly the reason the - keys above stay where they are: `⌃F` means *find* to everyone who has used a computer, and that - convention outweighs freeze's claim on the chord. Freeze kept its letter and changed its modifier — + - **Freeze is `⌥F`, and `⌃F` is find** — the search surface, above. Freeze was on `⌃F` and left it for + exactly the reason the keys above stay where they are: `⌃F` means *find* to everyone who has used a + computer, and that convention outweighs freeze's claim on the chord. Freeze kept its letter and changed its modifier — the smallest move that frees it; `⌥F` is `ESC f`, measured. Nothing is left behind on `⌃F` as an alias (the `⌃D` rule). The label a **frozen** reader is looking at (`FreezeBarRenderer`) moves with the chord: a bar naming a key that no longer thaws the pane would be the worst possible place to diff --git a/docs/design/README.md b/docs/design/README.md index 4054d81..6947542 100644 --- a/docs/design/README.md +++ b/docs/design/README.md @@ -423,7 +423,8 @@ count on the tab, the rail character, and the rail world. ### Other keys -`⌃P` command surface · `⌥F` freeze/resume in focused pane · `⌃R` command-history search · +`⌃P` command surface · `⌃F` search the output (`⌥G` next hit) · `⌥F` freeze/resume in focused pane · +`⌃R` command-history search · `⌃N` next window · `⌥D`/`⌥R` disconnect/reconnect · `⌥↑`/`⌥↓` command history (`↑`/`↓` do it too, where the caret has nowhere further to go) · `⌥⏎`, or `⌃L`, newline in input · `F1` composer · `F2`–`F9` config · `Esc` close overlay. diff --git a/src/SharpMUTerm.Tui/SharpMUTermApp.cs b/src/SharpMUTerm.Tui/SharpMUTermApp.cs index c3d7ada..a6ac50e 100644 --- a/src/SharpMUTerm.Tui/SharpMUTermApp.cs +++ b/src/SharpMUTerm.Tui/SharpMUTermApp.cs @@ -845,7 +845,14 @@ public string RenderSnapshot(string? view = null) ShowTimestamps = true; } + // The demo scene is a scene, not an absence. It pours lines into windows the reader has never + // been in front of — a spawn window's whole history arrives before the first frame — and every + // one of them would count as missed, so any frame that later made such a window visible (a split, + // a tab change) would carry an activity bar reporting the client's own setup as news. Same + // reasoning as `_watching` itself, which the restore replay needs for the same shape of reason. + _watching = false; LoadDemoScene(); + _watching = true; // The reported bug, as a frame: the scene is already on screen with the column off, and *then* // the real ⌃P entry is dispatched. Under the old append-time gutter this frame was identical to @@ -1045,6 +1052,73 @@ public string RenderSnapshot(string? view = null) ReArmWholeFrame(); } + // The ⌃F search surface, over a client that has something to find. Four views, because four + // things about it are only visible in a frame: `search` is a plain query with its hits marked, + // `search-regex` is the same query read as a pattern (the header is the only place either state + // is said, which is why they are a pair — the same reasoning as compose/compose-literal), + // `search-all` is the widened scope, where the window column appears and hits from a pane the + // reader is not looking at are listed under it, and `search-landed` is what ⏎ leaves behind. + // + // Driven through the surface's own key handler, as `history-search-filter` is: the frame shows + // what the real filter produced rather than an impression of it. + if (view is not null && view.StartsWith("search", StringComparison.OrdinalIgnoreCase)) + { + // A split first, so the pane is narrower than the terminal. That is the geometry that catches + // a landing scrolled to the wrong row — a buffer index is not a viewport row, and in a narrow + // pane almost every line wraps. + if (string.Equals(view, "search-landed", StringComparison.OrdinalIgnoreCase)) + { + PaneCommands.Apply(_workspace.Layout, PaneCommand.SplitRight); + RebuildPaneArea(); + + // Laid out *before* the scene is loaded, so the windows are visible and at their live + // tails as the lines arrive. Without it the split's own rebuild leaves them uncaught-up + // for the length of the load, every line counts as missed, and the frame comes out + // carrying an activity bar — a true report of a state this view is not about. + RenderFrame(); + SettleScroll(); + } + + LoadLongScene(MainWindowId, 30); + foreach (var line in new[] + { + "The goblin snarls at you and misses.", + "You hit the goblin for 12 damage.", + "A goblin corpse lies here, still twitching.", + }) + { + AppendWindowLine(MainWindowId, MarkupText.Escape(line)); + } + + AppendWindowLine(DemoScene.ChatWindowId, MarkupText.Escape(" Ana: the goblin room is bugged")); + SettleScroll(); + + OpenSearchForSnapshot(); + if (string.Equals(view, "search-all", StringComparison.OrdinalIgnoreCase)) + { + SimulateSearchKey(new ConsoleKeyInfo('\0', ConsoleKey.A, false, true, false)); + } + + if (string.Equals(view, "search-regex", StringComparison.OrdinalIgnoreCase)) + { + SimulateSearchKey(new ConsoleKeyInfo('\0', ConsoleKey.E, false, true, false)); + SimulateSearchTyping(@"gobl\w+ (corpse|for)"); + } + else + { + SimulateSearchTyping("goblin"); + } + + if (string.Equals(view, "search-landed", StringComparison.OrdinalIgnoreCase)) + { + SimulateSearchKey(new ConsoleKeyInfo('\0', ConsoleKey.DownArrow, false, false, false)); + SimulateSearchKey(new ConsoleKeyInfo('\r', ConsoleKey.Enter, false, false, false)); + SettleScroll(); + } + + ReArmWholeFrame(); + } + // The other absence: a window the reader was not watching. `away` above is the terminal one, and // the two are worth separate frames because they are separate facts with separate wording — this // is the one that happens many times an hour, and the only frame where a NEW bar can be seen with diff --git a/tests/SharpMUTerm.Tui.Tests/SearchBarRendererTests.cs b/tests/SharpMUTerm.Tui.Tests/SearchBarRendererTests.cs new file mode 100644 index 0000000..abfc8d9 --- /dev/null +++ b/tests/SharpMUTerm.Tui.Tests/SearchBarRendererTests.cs @@ -0,0 +1,60 @@ +using SharpMUTerm.Tui; + +namespace SharpMUTerm.Tui.Tests; + +/// +/// The bar drawn above the line ⌃F sent you to. Fourth of the boundary bars, and held to their rule: +/// mark the boundary, never restyle the content. +/// +public class SearchBarRendererTests +{ + [Test] + public async Task ItCarriesTheQueryTheOrdinalAndTheChordThatGoesToTheNextHit() + { + var bar = SearchBarRenderer.Bar("goblin", 12, 38, "#c678dd"); + + await Assert.That(bar).Contains($"[#c678dd]{Glyphs.Search} goblin (12 of 38)[/]"); + await Assert.That(bar).Contains(SearchBarRenderer.NextChord); + await Assert.That(bar).Contains("[dim]"); + await Assert.That(bar).Contains("─"); + } + + /// + /// The ordinal is what makes ⌥G legible: without it the bar moves and nothing says whether you are + /// getting closer to the end of the results or going round in circles. + /// + [Test] + public async Task TheOrdinalSaysWhereInTheResultsThisIs() + { + await Assert.That(SearchBarRenderer.Bar("key", 1, 1, "#c678dd")).Contains("(1 of 1)"); + } + + /// + /// The query is the reader's own text going into markup. A search for [public] must appear on + /// the bar rather than be eaten as a tag — the rule every renderer here follows for text it did not + /// write. + /// + [Test] + public async Task TheQueryIsEscapedRatherThanParsedAsMarkup() + { + var bar = SearchBarRenderer.Bar("[public]", 1, 2, "#c678dd"); + + await Assert.That(bar).Contains("[[public]]"); + } + + [Test] + public void ItRejectsAnEmptyAccent() + { + Assert.Throws(() => SearchBarRenderer.Bar("goblin", 1, 1, string.Empty)); + } + + /// + /// The chord it names is the only one there is. ⌥⇧G would be the obvious partner and cannot arrive — + /// kitty writes it as a CSI-u sequence this parser drops — so the bar must not offer it. + /// + [Test] + public async Task ItNamesNoChordThatCannotArrive() + { + await Assert.That(SearchBarRenderer.Bar("goblin", 1, 3, "#c678dd")).DoesNotContain("⇧"); + } +} From 8f571a144b0c02545eff1347ad6a01e1cba4d5dd Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Tue, 11 Aug 2026 15:24:05 -0500 Subject: [PATCH 17/19] fix(triggers): a rewrite no longer throws away its own rule's highlight MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The reported defect was "highlight colours don't seem to actually work". They work alone — `Highlight_RecoloursMatchedRegion` has always passed — and they were destroyed by the rule's *own* rewrite. `Process` applied the highlight to the matched region and then, four lines later, replaced the whole line with `StyledLine.FromText(text, TextStyle.Default)`: no colour, no attributes, no left rule. That combination is not exotic, it is what a channel rule looks like — route it, tidy it to `» $1`, colour it — and it is the shape of the demo configuration's own headline rule. The F2 screen badged such a rule `H` and painted both swatches, so the client promised a highlight it then threw away, and the only way to find one was to discover that deleting the rewrite brought it back. The rewrite now runs first and the highlight covers the whole of what it produced. It cannot cover the match's own offsets, because after a rewrite those address a string that no longer exists; the rewritten text is the rule's product in its entirety, so colouring all of it is the only reading that means anything. Without a rewrite nothing moves: the highlight covers the match and only the match, as it always has. A *later* rule's rewrite still replaces an earlier rule's highlighted text, and that is correct rather than the same bug one rule over — those characters are gone. Pinned, so the ordering fix is not later generalised into re-colouring text the first rule never saw. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN --- src/SharpMUTerm.Core/Automation/Trigger.cs | 5 +- .../Automation/TriggerEngine.cs | 38 +++-- .../Automation/HighlightRewriteTests.cs | 148 ++++++++++++++++++ 3 files changed, 181 insertions(+), 10 deletions(-) create mode 100644 tests/SharpMUTerm.Core.Tests/Automation/HighlightRewriteTests.cs diff --git a/src/SharpMUTerm.Core/Automation/Trigger.cs b/src/SharpMUTerm.Core/Automation/Trigger.cs index e512ca4..d11a44b 100644 --- a/src/SharpMUTerm.Core/Automation/Trigger.cs +++ b/src/SharpMUTerm.Core/Automation/Trigger.cs @@ -29,7 +29,10 @@ public sealed class TriggerActions /// /// Replace the whole line's text with this template (supports $1..$9 and - /// ${name} capture references). Rewritten text renders with the default style. Null — which + /// ${name} capture references). Rewritten text starts from the default style — a rewrite is + /// how a server's own colour is dropped as well as how its wording is changed — and then wears + /// whatever , and + /// this same rule asks for, across the whole of it. Null — which /// is what the F2 screen writes for a blank field — means the rule rewrites nothing; settable for /// the same reason is, and with the same absence of any cached state. /// diff --git a/src/SharpMUTerm.Core/Automation/TriggerEngine.cs b/src/SharpMUTerm.Core/Automation/TriggerEngine.cs index d01c5e9..8a97313 100644 --- a/src/SharpMUTerm.Core/Automation/TriggerEngine.cs +++ b/src/SharpMUTerm.Core/Automation/TriggerEngine.cs @@ -253,17 +253,31 @@ public TriggerResult Process(StyledLine line) suppress = true; } - if (actions.HighlightForeground is not null || - actions.HighlightBackground is not null || - actions.AddAttributes != TextAttributes.None) + // The rewrite runs *before* the highlight, and that order is the whole of a reported defect. + // It used to run after, and a rewrite replaces the line wholesale with an unstyled one — so a + // rule that both rewrote and highlighted threw its own colours, attributes and left rule away + // on the very next statement. That is not an exotic combination: it is what a channel rule + // looks like (route it, tidy it to `» $1`, colour it), and it is the shape of the demo + // configuration's own headline rule. The F2 screen badged such a rule `H` and painted its + // swatches, so the client promised a highlight that could never appear. + var rewritten = false; + if (actions.Rewrite is not null) { - current = ApplyHighlight(current, match, actions); + current = StyledLine.FromText(match.Result(actions.Rewrite), TextStyle.Default); + rewritten = true; } - if (actions.Rewrite is not null) + if (actions.HighlightForeground is not null || + actions.HighlightBackground is not null || + actions.AddAttributes != TextAttributes.None) { - var text = match.Result(actions.Rewrite); - current = StyledLine.FromText(text, TextStyle.Default); + // A rewrite makes the match's own offsets meaningless — they described the string the + // rewrite replaced — so the highlight covers the whole of what the rule produced, which + // is the only region of the new line the rule can be said to be talking about. Without a + // rewrite it covers the match and only the match, exactly as it always has. + current = rewritten + ? ApplyHighlight(current, 0, current.Length, actions) + : ApplyHighlight(current, match.Index, match.Length, actions); } if (!string.IsNullOrEmpty(actions.SendResponse)) @@ -382,9 +396,15 @@ actions.HighlightBackground is not null || /// public const int MaxTargetLength = 64; - private static StyledLine ApplyHighlight(StyledLine line, Match match, TriggerActions actions) + /// + /// Recolours characters from and carries the + /// rule's colour onto the whole line so the output pane can draw its left rule. The region is a + /// parameter rather than a because a rewritten line has no match offsets left to + /// speak of — see the call site. + /// + private static StyledLine ApplyHighlight(StyledLine line, int start, int length, TriggerActions actions) { - var restyled = StyledText.Restyle(line, match.Index, match.Length, style => + var restyled = StyledText.Restyle(line, start, length, style => { if (actions.HighlightForeground is not null) { diff --git a/tests/SharpMUTerm.Core.Tests/Automation/HighlightRewriteTests.cs b/tests/SharpMUTerm.Core.Tests/Automation/HighlightRewriteTests.cs new file mode 100644 index 0000000..f39bd1c --- /dev/null +++ b/tests/SharpMUTerm.Core.Tests/Automation/HighlightRewriteTests.cs @@ -0,0 +1,148 @@ +using SharpMUTerm.Core.Automation; +using SharpMUTerm.Core.Text; + +namespace SharpMUTerm.Core.Tests.Automation; + +/// +/// The reported defect: highlight colours don't seem to actually work. They work on their own — +/// has always passed — and they were +/// destroyed by the rule's own rewrite. TriggerEngine.Process applied the highlight to +/// the matched region and then, four lines later, replaced the whole line with +/// StyledLine.FromText(text, TextStyle.Default), which is a line with no colour, no attributes +/// and no left rule on it. +/// +/// That combination is not exotic; it is what a channel rule looks like. Route the line to a capture +/// pane, tidy it up (» $1) and colour it — which is exactly the shape of the demo +/// configuration's own headline rule (DemoScene's public: teal, bold, and +/// Rewrite = "» $1"). The F2 screen badges such a rule H and paints both swatches, so the +/// client promised a highlight it then threw away, and the only way to get one was to discover that +/// deleting the rewrite brought it back. +/// +/// +/// The fix is an ordering one: the rewrite runs first, and the highlight is then applied to +/// the whole rewritten line. It cannot be applied to the match's own offsets, because after a rewrite +/// those address a string that no longer exists — the rewritten text is the rule's product in its +/// entirety, so colouring all of it is the only reading that means anything. +/// +/// +public class HighlightRewriteTests +{ + private static readonly TerminalColor Gold = TerminalColor.FromRgb(0xff, 0xd7, 0x00); + + private static StyledLine Line(string text) => StyledLine.FromText(text, TextStyle.Default); + + private static TriggerResult Run(TriggerActions actions, string pattern, string text) + { + var engine = new TriggerEngine(); + engine.Add(new Trigger { Pattern = pattern, Actions = actions }); + return engine.Process(Line(text)); + } + + /// The headline: a rule that rewrites and highlights does both. + [Test] + public async Task ARewrittenLineStillWearsItsRulesHighlight() + { + var result = Run( + new TriggerActions { HighlightForeground = Gold, Rewrite = "» $1" }, + @"^\[public\] (.+)$", + "[public] hello there"); + + await Assert.That(result.Line.Text).IsEqualTo("» hello there"); + await Assert.That(result.Line.Spans.All(s => s.Style.Foreground == Gold)).IsTrue(); + } + + /// + /// The whole rewritten line, not a fragment of it. The match's offsets described the line the + /// rewrite replaced, so re-using them would colour an arbitrary prefix of the new text — which is + /// the same defect wearing a different mask, and harder to spot. + /// + [Test] + public async Task TheHighlightCoversTheWholeRewrittenLine() + { + // The rewrite is far longer than the region that matched, so a highlight still keyed to + // match.Index/Length would leave the tail of the new text unstyled. + var result = Run( + new TriggerActions { HighlightBackground = Gold, Rewrite = "$1 — and a great deal more text besides" }, + @"^\[(\w+)\]", + "[public] hello there"); + + await Assert.That(result.Line.Spans.All(s => s.Style.Background == Gold)).IsTrue(); + } + + /// Attributes are part of the same promise, and were lost with the colours. + [Test] + public async Task ARewrittenLineKeepsTheAttributesItsRuleAdded() + { + var result = Run( + new TriggerActions { AddAttributes = TextAttributes.Bold, Rewrite = "» $1" }, + @"^\[public\] (.+)$", + "[public] hello there"); + + await Assert.That(result.Line.Spans.All(s => s.Style.HasAttribute(TextAttributes.Bold))).IsTrue(); + } + + /// + /// And the left rule, which is the marker the output pane draws to say a trigger touched this line + /// at all. It went with the colours, so a rewritten line was indistinguishable from an untouched one. + /// + [Test] + public async Task ARewrittenLineKeepsItsLeftRule() + { + var result = Run( + new TriggerActions { HighlightForeground = Gold, Rewrite = "» $1" }, + @"^\[public\] (.+)$", + "[public] hello there"); + + await Assert.That(result.Line.RuleColor).IsEqualTo(Gold); + } + + /// + /// A rule that only rewrites still produces unstyled text. Reordering the two actions must not smuggle + /// a style onto a line whose rule asked for none — the rewritten text is deliberately the default + /// style, so that a rewrite is a way to drop a server's colour as well as to reword it. + /// + [Test] + public async Task ARewriteWithNoHighlightIsStillPlain() + { + var result = Run(new TriggerActions { Rewrite = "» $1" }, @"^\[public\] (.+)$", "[public] hello there"); + + await Assert.That(result.Line.Text).IsEqualTo("» hello there"); + await Assert.That(result.Line.RuleColor).IsNull(); + await Assert.That(result.Line.Spans.All(s => s.Style.Equals(TextStyle.Default))).IsTrue(); + } + + /// + /// Without a rewrite nothing moves: the highlight still covers the matched region and only that. This + /// is the property the reordering could most easily have broken, and it is the behaviour every rule + /// that does not rewrite depends on. + /// + [Test] + public async Task WithoutARewriteTheHighlightStillCoversOnlyTheMatch() + { + var result = Run(new TriggerActions { HighlightForeground = Gold }, "gold", "you find gold today"); + + var gold = result.Line.Spans.Single(s => s.Text == "gold"); + await Assert.That(gold.Style.Foreground).IsEqualTo(Gold); + await Assert.That(result.Line.Spans.Where(s => s.Text != "gold").All(s => + s.Style.Foreground == TerminalColor.Default)).IsTrue(); + } + + /// + /// A later rule's rewrite still replaces an earlier rule's highlighted text, and that is + /// correct rather than the same bug one rule over: the characters the first rule coloured are gone. + /// Pinned so the ordering fix is not later "generalised" into carrying styles across rules, where it + /// would be re-colouring text the first rule never saw. + /// + [Test] + public async Task ALaterRulesRewriteStillReplacesAnEarlierRulesHighlight() + { + var engine = new TriggerEngine(); + engine.Add(new Trigger { Pattern = "gold", Actions = new TriggerActions { HighlightForeground = Gold } }); + engine.Add(new Trigger { Pattern = "^you find (.+)$", Actions = new TriggerActions { Rewrite = "found: $1" } }); + + var result = engine.Process(Line("you find gold today")); + + await Assert.That(result.Line.Text).IsEqualTo("found: gold today"); + await Assert.That(result.Line.Spans.All(s => s.Style.Foreground == TerminalColor.Default)).IsTrue(); + } +} From 4ce384b8ce903bbf61d8ad9cc486ada288a1197a Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Tue, 11 Aug 2026 15:24:18 -0500 Subject: [PATCH 18/19] feat(triggers): a rule routes to a window that already exists, not only to a spawn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "Triggers should not be restricted to only be able to go to spawns." They were, and the restriction was one method deep: `Workspace.RouteSpawn` was the only destination resolver a matched rule had, and it computes `SpawnWindowId(sessionKey, target)` and registers a new `WindowKind.Spawn` window when nothing answers to that id. There is no branch in it that can reach a window that already exists under any other name, so a route naming a window on the screen opened a second one beside it wearing the same label, and the one the user was looking at stayed empty. `Workspace.RouteLine` is the resolver now: a window the target already names wins, and creating a capture pane is what happens when nothing does. `FindRouteTarget` is the same lookup without the side effects, so the shell can still tell "this line opened a pane" from "this line went to one that was already there" without routing twice. What a target may reach is deliberately narrower than "any window with that title": 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, because two characters running one capture rule get a pane each and a bare title lookup would collapse them back into one, which is the exact defect `SpawnWindowId` was given an owner to fix. Only a placed window is a destination: appending to a window no pane holds is indistinguishable from the rule not firing. And finding never creates, which is the property that bounds a capture-derived target — that arm can only ever land in a window the user already has. Two consequences in the shell. `OwnerLabel` is stamped on this session's own capture panes only; writing our name onto a window somebody else owns would rename their pane after whoever last routed a line into it. And the F2 `route` field now suggests the workspace's own windows as well as the other rules' targets — while the list was the rules' targets alone, the one place a user reads what a route may say could not name any window they had open. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN --- src/SharpMUTerm.Core/Workspace/Workspace.cs | 92 +++++++ src/SharpMUTerm.Tui/SharpMUTermApp.cs | 79 ++++-- src/SharpMUTerm.Tui/TriggersScreenRenderer.cs | 38 +-- src/SharpMUTerm.Tui/TriggersScreenView.cs | 6 +- .../Workspace/RouteToExistingWindowTests.cs | 195 +++++++++++++++ .../TriggerRouteDestinationTests.cs | 225 ++++++++++++++++++ .../TriggersScreenRendererTests.cs | 16 +- 7 files changed, 607 insertions(+), 44 deletions(-) create mode 100644 tests/SharpMUTerm.Core.Tests/Workspace/RouteToExistingWindowTests.cs create mode 100644 tests/SharpMUTerm.Tui.Tests/TriggerRouteDestinationTests.cs diff --git a/src/SharpMUTerm.Core/Workspace/Workspace.cs b/src/SharpMUTerm.Core/Workspace/Workspace.cs index fea867b..52316de 100644 --- a/src/SharpMUTerm.Core/Workspace/Workspace.cs +++ b/src/SharpMUTerm.Core/Workspace/Workspace.cs @@ -155,11 +155,103 @@ public WorkspaceWindow OpenWindow( return window; } + /// + /// Routes a matched trigger's line to the window names, on behalf of + /// : a window that already exists wins, and a spawn window is what + /// 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. + /// + /// + /// 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. + /// + /// + public WorkspaceWindow RouteLine(string target, string? sessionKey = null) + { + ArgumentException.ThrowIfNullOrEmpty(target); + if (FindRouteTarget(target, sessionKey) is not { } existing) + { + return RouteSpawn(target, sessionKey); + } + + NoteActivity(existing.Id); + return existing; + } + + /// + /// The window already names for , or null when + /// nothing does — the finding half of , with no side effects, so a caller can + /// tell "this line opened a pane" from "this line went to one that was already there" without + /// routing twice. See for what a target may and may not reach. + /// + public WorkspaceWindow? FindRouteTarget(string target, string? sessionKey = null) + { + ArgumentException.ThrowIfNullOrEmpty(target); + + // Preference order, and it has to be total: several windows may carry one title, and a route that + // resolved differently from one line to the next would scatter a channel across panes. This + // session's own first, then the unowned, then another character's main; ties inside a group go to + // the older window, which is the same creation order everything else here numbers windows in. + var best = _windows.Values + .Where(w => string.Equals(w.Title, target, StringComparison.Ordinal)) + .Where(w => Layout.FindWindow(w.Id) is not null) + .Select(w => (Window: w, Rank: RouteRank(w, sessionKey))) + .Where(candidate => candidate.Rank >= 0) + .OrderBy(candidate => candidate.Rank) + .ThenBy(candidate => candidate.Window.Sequence) + .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. + return best ?? _windows.GetValueOrDefault(SpawnWindowId(sessionKey, target)); + } + + /// + /// How willingly takes a line routed by — + /// lower is better, and negative means never. + /// + private static int RouteRank(WorkspaceWindow window, string? sessionKey) => window switch + { + _ when window.SessionKey is not null && string.Equals(window.SessionKey, sessionKey, StringComparison.Ordinal) => 0, + _ when window.SessionKey is null => 1, + _ when window.Kind == WindowKind.Main => 2, + _ => -1, + }; + /// /// Routes trigger-spawned output to 's spawn window named /// , creating and placing the window on first use, and counts the line as /// unread unless the window is currently visible. Returns the destination window. /// + /// This is the creating half only; is what a routed line goes through, + /// 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 diff --git a/src/SharpMUTerm.Tui/SharpMUTermApp.cs b/src/SharpMUTerm.Tui/SharpMUTermApp.cs index a6ac50e..d788e32 100644 --- a/src/SharpMUTerm.Tui/SharpMUTermApp.cs +++ b/src/SharpMUTerm.Tui/SharpMUTermApp.cs @@ -3390,7 +3390,11 @@ private void OnLine(WorldSession session, string windowId, StyledLine line) } /// - /// Routes a trigger-spawned line to its spawn window (creating the tab on first use). + /// Routes a trigger-matched line to the window its rule names — one that already exists when the + /// target names one, and a spawn window created on the spot when it does not + /// (). Routing used to be RouteSpawn and nothing else, so + /// every destination a rule could have was a spawn pane of its own session's; a route naming a window + /// on the screen opened a second one beside it wearing the same label. /// /// The owner recorded on a first-seen window is the whose trigger fired, /// not _active. It used to be the latter, so a background world's capture opened a window @@ -3399,6 +3403,12 @@ private void OnLine(WorldSession session, string windowId, StyledLine line) /// which world a link clicked in a spawn window sends to by it. /// /// + /// It is stamped on this session's own capture panes only. The label prefixes a tab as + /// Owner: Name so a spawn scattered into another pane stays tied to its character, and a + /// destination this session does not own is somebody else's window or nobody's — writing our name + /// onto another character's main window would relabel their pane after whoever routed into it. + /// + /// /// The same session key also picks the window (), /// which is what gives two characters running one capture rule a pane each. While the id was the /// target alone there was one window per workspace: the first session to match created it with its @@ -3409,12 +3419,19 @@ private void OnLine(WorldSession session, string windowId, StyledLine line) /// private void OnSpawnLine(WorldSession session, string target, StyledLine line) { - var existed = _workspace.FindWindow(Workspace.SpawnWindowId(session.SessionKey, target)) is not null; - var window = _workspace.RouteSpawn(target, session.SessionKey); + // Asked before routing rather than after, because routing is what makes the answer false: this is + // "was there already somewhere for this line to go", which is what decides between adding a tab + // and merely refreshing the badges. + var existed = _workspace.FindRouteTarget(target, session.SessionKey) is not null; + var window = _workspace.RouteLine(target, session.SessionKey); // Its owner's own name, which for a session with no character is its world's. It used to fall back on // the *main window's* title, which is a different session's name as soon as more than one is open. - window.OwnerLabel ??= SessionTitle(session); + if (window.Kind == WindowKind.Spawn && + string.Equals(window.SessionKey, session.SessionKey, StringComparison.Ordinal)) + { + window.OwnerLabel ??= SessionTitle(session); + } PaneContentFor(window.Id, window.Title); // ensure the live control exists before buffering // The restore log is fed here as well as in OnLine, and that is the crux of the whole feature: @@ -3424,7 +3441,7 @@ private void OnSpawnLine(WorldSession session, string target, StyledLine line) AppendWindowLine(window.Id, _formatter.ToMarkup(line), stamp); RecordForRestore(session, window.Id, window.Title, line, stamp); - // A first-seen spawn adds a tab to its pane, so rebuild; otherwise just refresh badges. + // A first-seen destination adds a tab to its pane, so rebuild; otherwise just refresh badges. if (existed) { RefreshTabTitles(); @@ -5565,14 +5582,41 @@ private void PersistConfiguration() } } - /// Distinct spawn-window targets referenced by any trigger (for the F2 route-to list). - private IReadOnlyList SpawnTargets() => - _config.TriggerSets.SelectMany(s => s.Triggers) - .Select(t => t.Actions.SpawnTarget) - .Where(t => !string.IsNullOrEmpty(t)) - .Select(t => t!) - .Distinct(StringComparer.Ordinal) - .ToList(); + /// + /// The destinations the F2 route field offers: every window some trigger already routes to, + /// then the windows this workspace actually holds. + /// + /// The second half is what makes routing to an existing window expressible. A rule's + /// destination is resolved by name against the windows that are open + /// (), and while the list was the other rules' targets alone the + /// only names it could offer were spawn panes — so the one place a user reads what a route may say + /// could not name the character's own window, another character's, or any window they had opened. The + /// list is suggestions and not the permitted set, so this widens what is discoverable rather than + /// what is legal. + /// + /// + /// Trigger targets lead, because a rule that has not opened its pane yet names a window nothing else + /// can offer, and because that is the order this list has always been read in. + /// + /// + private IReadOnlyList RouteTargets() + { + var targets = new List(); + foreach (var name in _config.TriggerSets.SelectMany(s => s.Triggers) + .Select(t => t.Actions.SpawnTarget) + .Concat(_workspace.Windows + .Where(w => _workspace.Layout.FindWindow(w.Id) is not null) + .OrderBy(w => w.Sequence) + .Select(w => w.Title))) + { + if (!string.IsNullOrEmpty(name) && !targets.Contains(name, StringComparer.Ordinal)) + { + targets.Add(name); + } + } + + return targets; + } /// Every configured macro across all trigger sets (for the F4 keypad/hotkey list). private IReadOnlyList Macros() => _config.TriggerSets.SelectMany(s => s.Macros).ToList(); @@ -5753,13 +5797,13 @@ private ScreenBinding WorldsScreen(string fkey, bool onCharacters) private ScreenBinding TriggersScreen() { var session = new SettingsSession(selection => - TriggersScreenRenderer.Model(_config.TriggerSets, selection.SelectionIn(0), SpawnTargets()), + TriggersScreenRenderer.Model(_config.TriggerSets, selection.SelectionIn(0), RouteTargets()), SaveConfiguration); return new ScreenBinding(session, () => TriggersScreenView.Build( _config.TriggerSets, session.Selection.SelectionIn(0), - SpawnTargets(), + RouteTargets(), _system.DesktopDimensions.Width, session.Focus(), _system.DesktopDimensions.Height)); @@ -8462,6 +8506,11 @@ internal void OpenUnownedWindowForTest(string id, string title) /// , and it goes stale silently, so it is worth asserting directly. internal string? WindowOwnerOf(string windowId) => _workspace.FindWindow(windowId)?.SessionKey; + /// A window's owner label — the Owner: Name prefix its tab wears. A different + /// fact from and worth asserting separately: a line routed into a window + /// somebody else owns must not stamp the routing character's name onto it. + internal string? WindowOwnerLabelOf(string windowId) => _workspace.FindWindow(windowId)?.OwnerLabel; + /// A pane's visible tab, so a test can say which window a click on a tab strip brought up. internal string? PaneActiveTab(string paneId) => _workspace.Layout.FindPane(paneId)?.ActiveTab; diff --git a/src/SharpMUTerm.Tui/TriggersScreenRenderer.cs b/src/SharpMUTerm.Tui/TriggersScreenRenderer.cs index e7f8abb..f5f5c25 100644 --- a/src/SharpMUTerm.Tui/TriggersScreenRenderer.cs +++ b/src/SharpMUTerm.Tui/TriggersScreenRenderer.cs @@ -119,21 +119,23 @@ internal static class TriggersScreenRenderer private static string Route(Trigger trigger) => trigger.Actions.SpawnTarget ?? MainWindow; /// - /// The windows offered as ↑↓ suggestions on the route field: the main output, every spawn window - /// the workspace knows about, and — always — the one this rule already points at, so a rule - /// routed somewhere the current workspace has no window for still shows its own value. + /// The destinations offered as ↑↓ suggestions on the route field: the main output, every window a + /// rule or the workspace can name (SharpMUTermApp.RouteTargets), and — always — the one this + /// rule already points at, so a rule routed somewhere the current workspace has no window for still + /// shows its own value. /// /// These are suggestions, not the permitted set. Typing a name that isn't here is how a new spawn - /// window comes into existence: the workspace's spawn windows are defined by what triggers route - /// to, so a closed list could only ever re-use one that already exists. + /// window comes into existence: a name nothing answers to is created as a capture pane + /// (Workspace.RouteLine), so a closed list could only ever re-use a destination that already + /// exists. /// /// - internal static IReadOnlyList Routes(Trigger trigger, IReadOnlyList? spawnTargets) + internal static IReadOnlyList Routes(Trigger trigger, IReadOnlyList? routeTargets) { ArgumentNullException.ThrowIfNull(trigger); var routes = new List { MainWindow }; - foreach (var target in (spawnTargets ?? Array.Empty()).Append(Route(trigger))) + foreach (var target in (routeTargets ?? Array.Empty()).Append(Route(trigger))) { if (!string.IsNullOrEmpty(target) && !routes.Contains(target, StringComparer.Ordinal)) { @@ -210,15 +212,15 @@ private static List NamedCallbacks(IReadOnlyList sets) public static List Render( IReadOnlyList sets, int selectedTrigger, - IReadOnlyList spawnTargets) + IReadOnlyList routeTargets) { ArgumentNullException.ThrowIfNull(sets); - ArgumentNullException.ThrowIfNull(spawnTargets); + ArgumentNullException.ThrowIfNull(routeTargets); var left = RulesColumn(sets, selectedTrigger); - var right = EditorColumn(sets, selectedTrigger, spawnTargets); + var right = EditorColumn(sets, selectedTrigger, routeTargets); - var lines = new List { HeaderLine(0, Model(sets, selectedTrigger, spawnTargets)), string.Empty }; + var lines = new List { HeaderLine(0, Model(sets, selectedTrigger, routeTargets)), string.Empty }; var rowCount = Math.Max(left.Count, right.Count); for (var i = 0; i < rowCount; i++) @@ -261,7 +263,7 @@ internal static string HeaderLine(int width, ScreenModel? model = null, ScreenFo /// and a palette are — while the editor keeps drawing them where they are read. /// /// - /// + /// /// The spawn windows a rule may route to, beyond main and its own current target. Optional /// so a caller that only wants the navigable shape (the header hints, the tests) need not know the /// workspace's windows. @@ -269,7 +271,7 @@ internal static string HeaderLine(int width, ScreenModel? model = null, ScreenFo internal static ScreenModel Model( IReadOnlyList sets, int selectedTrigger, - IReadOnlyList? spawnTargets = null) + IReadOnlyList? routeTargets = null) { ArgumentNullException.ThrowIfNull(sets); @@ -284,7 +286,7 @@ internal static ScreenModel Model( "route", () => Route(entry.Trigger), v => entry.Trigger.Actions.SpawnTarget = v == MainWindow ? null : v.Trim(), - Routes(entry.Trigger, spawnTargets)), + Routes(entry.Trigger, routeTargets)), ScreenField.Colour( "highlight fg", () => entry.Trigger.Actions.HighlightForeground, @@ -517,13 +519,13 @@ private static IEnumerable FlagLegend(Trigger? trigger, int width) internal static List EditorColumn( IReadOnlyList sets, int selectedTrigger, - IReadOnlyList spawnTargets, + IReadOnlyList routeTargets, ScreenFocus? focus = null, int width = ColumnWidth, int height = 0) { ArgumentNullException.ThrowIfNull(sets); - ArgumentNullException.ThrowIfNull(spawnTargets); + ArgumentNullException.ThrowIfNull(routeTargets); var cursor = focus ?? ScreenFocus.None; var flattened = Flatten(sets); @@ -531,7 +533,7 @@ internal static List EditorColumn( ? BuildEditor( flattened[selectedTrigger].Trigger, flattened[selectedTrigger].SetName, - spawnTargets, + routeTargets, cursor, selectedTrigger, width, @@ -609,7 +611,7 @@ private static string Flags(TriggerActions actions) private static List BuildEditor( Trigger trigger, string setName, - IReadOnlyList spawnTargets, + IReadOnlyList routeTargets, ScreenFocus cursor, int index, int width = ColumnWidth, diff --git a/src/SharpMUTerm.Tui/TriggersScreenView.cs b/src/SharpMUTerm.Tui/TriggersScreenView.cs index 469684d..90cb703 100644 --- a/src/SharpMUTerm.Tui/TriggersScreenView.cs +++ b/src/SharpMUTerm.Tui/TriggersScreenView.cs @@ -18,14 +18,14 @@ internal static class TriggersScreenView public static IWindowControl Build( IReadOnlyList sets, int selectedTrigger, - IReadOnlyList spawnTargets, + IReadOnlyList routeTargets, int width, ScreenFocus? focus = null, int height = 0) { var header = ScreenChrome.Band( TriggersScreenRenderer.HeaderLine( - width, TriggersScreenRenderer.Model(sets, selectedTrigger, spawnTargets), focus), + width, TriggersScreenRenderer.Model(sets, selectedTrigger, routeTargets), focus), ScreenPalette.HeaderBg); var footer = ScreenChrome.Band( TriggersScreenRenderer.FooterLine(sets, selectedTrigger, width, focus), ScreenPalette.FooterBg); @@ -41,7 +41,7 @@ public static IWindowControl Build( var body = ScreenChrome.Rows(height); var left = TriggersScreenRenderer.RulesColumn(sets, selectedTrigger, focus, rules); var right = TriggersScreenRenderer.EditorColumn( - sets, selectedTrigger, spawnTargets, focus, width <= 0 ? rules : width - rules - ScreenChrome.ColumnDivider, body); + sets, selectedTrigger, routeTargets, focus, width <= 0 ? rules : width - rules - ScreenChrome.ColumnDivider, body); var rulesCol = ScreenChrome.Stretch(new MarkupControl(ScreenChrome.Window(left, body))); var editorCol = ScreenChrome.Stretch(new MarkupControl( diff --git a/tests/SharpMUTerm.Core.Tests/Workspace/RouteToExistingWindowTests.cs b/tests/SharpMUTerm.Core.Tests/Workspace/RouteToExistingWindowTests.cs new file mode 100644 index 0000000..2284791 --- /dev/null +++ b/tests/SharpMUTerm.Core.Tests/Workspace/RouteToExistingWindowTests.cs @@ -0,0 +1,195 @@ +using SharpMUTerm.Core.Workspaces; + +namespace SharpMUTerm.Core.Tests.Workspaces; + +/// +/// The reported defect: a trigger's routing could only ever go to a spawn window. +/// Workspace.RouteSpawn is the one destination resolver a matched rule has, and it computes +/// SpawnWindowId(sessionKey, target) and — when nothing answers to that id — registers a brand +/// new window. There is no branch in it that can reach a window that +/// already exists under any other name, so "route to the window I already have open" was not a thing a +/// rule could ask for however it was spelt. +/// +/// is the resolver now: an existing window the target names wins, and +/// creating a spawn is what happens when nothing does. What that buys is a rule feeding a window +/// somebody opened deliberately — a character's own main window, another character's, or any named +/// auxiliary window — rather than a fourth pane appearing beside them. +/// +/// +/// What it deliberately cannot reach is another session's spawn window, and that is the whole +/// reason resolution is scoped rather than a bare title lookup over the registry. Two characters running +/// one capture rule get a pane each; a title that crossed between them would collapse the two back into +/// one and file the second character's channel under the first, which is exactly the defect +/// SpawnWindowId was given an owner to fix. +/// +/// +public class RouteToExistingWindowTests +{ + private const string Ann = "Convergence.Ann"; + private const string Bob = "Convergence.Bob"; + + /// A workspace whose main window belongs to Ann and is titled the way the shell titles it. + private static Workspace AnnsWorkspace() => new("main", "Ann", Ann); + + // ---- The report ------------------------------------------------------------------------- + + /// + /// The headline. A rule routing to the title of a window that already exists lands in that + /// window; before the fix it opened a second one beside it, of a different kind, with the same name on + /// its tab. + /// + [Test] + public async Task ARuleRoutesToAnExistingWindowRatherThanOpeningASpawnBesideIt() + { + var workspace = AnnsWorkspace(); + workspace.OpenWindow("notes", "Notes", WindowKind.Auxiliary, Ann); + + var destination = workspace.RouteLine("Notes", Ann); + + await Assert.That(destination.Id).IsEqualTo("notes"); + await Assert.That(workspace.Windows.Count(w => w.Title == "Notes")).IsEqualTo(1); + } + + /// + /// Including the character's own main window, which is the destination the F2 route list has always + /// named and the one a rule could least express: main there means do not route, which + /// only reaches the main window for a line the rule does not also gag. + /// + [Test] + public async Task ARuleCanRouteToItsOwnCharactersMainWindow() + { + var workspace = AnnsWorkspace(); + + var destination = workspace.RouteLine("Ann", Ann); + + await Assert.That(destination.Id).IsEqualTo("main"); + await Assert.That(destination.Kind).IsEqualTo(WindowKind.Main); + } + + /// + /// And another character's main window — one alt's channel collected into the pane you actually read. + /// A main window is the one window another session owns that this may reach, because it is a window + /// the user opened by connecting rather than one a capture rule conjured. + /// + [Test] + public async Task ARuleCanRouteToAnotherCharactersMainWindow() + { + var workspace = AnnsWorkspace(); + workspace.OpenWindow("main:bob", "Bob", WindowKind.Main, Bob); + + var destination = workspace.RouteLine("Bob", Ann); + + await Assert.That(destination.Id).IsEqualTo("main:bob"); + } + + /// An unowned window — the web view is the one in this client — is in everybody's reach. + [Test] + public async Task ARuleCanRouteToAnUnownedWindow() + { + var workspace = AnnsWorkspace(); + workspace.OpenWindow("web", "Scratch", WindowKind.Auxiliary); + + await Assert.That(workspace.RouteLine("Scratch", Ann).Id).IsEqualTo("web"); + } + + // ---- What must not move ------------------------------------------------------------------ + + /// + /// The per-session guarantee, which this resolution is scoped to preserve: Bob's rule may not land in + /// Ann's capture pane just because they chose the same channel name. Bob gets his own, as before. + /// + [Test] + public async Task ARuleCannotRouteIntoAnotherSessionsSpawnWindow() + { + var workspace = AnnsWorkspace(); + var anns = workspace.RouteLine("Public", Ann); + + var bobs = workspace.RouteLine("Public", Bob); + + await Assert.That(bobs.Id).IsNotEqualTo(anns.Id); + await Assert.That(bobs.Id).IsEqualTo(Workspace.SpawnWindowId(Bob, "Public")); + await Assert.That(bobs.SessionKey).IsEqualTo(Bob); + } + + /// + /// Nor into another session's auxiliary window. Only a main window crosses the owner + /// boundary: everything else another character owns was created for them, and the two cases a route + /// must never conflate are "the window you meant" and "somebody else's window with the same label". + /// + [Test] + public async Task ARuleCannotRouteIntoAnotherSessionsAuxiliaryWindow() + { + var workspace = AnnsWorkspace(); + workspace.OpenWindow("bobs-notes", "Notes", WindowKind.Auxiliary, Bob); + + var destination = workspace.RouteLine("Notes", Ann); + + await Assert.That(destination.Id).IsEqualTo(Workspace.SpawnWindowId(Ann, "Notes")); + } + + /// + /// Nothing found is still a spawn window, created and placed exactly as it always was. This is the + /// path every existing capture rule takes and it must be untouched. + /// + [Test] + public async Task ATargetNothingAnswersToStillOpensASpawnWindow() + { + var workspace = AnnsWorkspace(); + + var destination = workspace.RouteLine("Chat", Ann); + + await Assert.That(destination.Id).IsEqualTo(Workspace.SpawnWindowId(Ann, "Chat")); + await Assert.That(destination.Kind).IsEqualTo(WindowKind.Spawn); + await Assert.That(destination.Title).IsEqualTo("Chat"); + await Assert.That(workspace.Layout.FindWindow(destination.Id)).IsNotNull(); + } + + /// + /// A rule feeding its own capture pane goes on feeding the same one — the second line of a channel + /// must not find the window by a different route than the first did and end up somewhere else. + /// + [Test] + public async Task TheSecondLineOfACaptureLandsInTheSamePaneAsTheFirst() + { + var workspace = AnnsWorkspace(); + + var first = workspace.RouteLine("Chat", Ann); + var second = workspace.RouteLine("Chat", Ann); + + await Assert.That(second.Id).IsEqualTo(first.Id); + } + + /// + /// A window no pane holds is not a destination. Routing there would append to a buffer nothing can + /// draw, which is indistinguishable from the rule not firing — so a closed window is passed over and + /// the line goes to a spawn pane that can actually be seen. + /// + [Test] + public async Task AClosedWindowIsNotADestination() + { + var workspace = AnnsWorkspace(); + workspace.OpenWindow("notes", "Notes", WindowKind.Auxiliary, Ann); + workspace.CloseWindow("notes"); + + var destination = workspace.RouteLine("Notes", Ann); + + await Assert.That(destination.Id).IsEqualTo(Workspace.SpawnWindowId(Ann, "Notes")); + } + + /// + /// Routing badges the destination unread when it is not the window being read, whichever kind it + /// turned out to be. The badge is the only thing that says a background pane gained a line, and a + /// resolution that reached a new kind of window without it would make the feature silent. + /// + [Test] + public async Task RoutingToAnExistingWindowStillBadgesItUnread() + { + var workspace = AnnsWorkspace(); + workspace.OpenWindow("notes", "Notes", WindowKind.Auxiliary, Ann); + workspace.ActivateWindow("main"); // Notes shares the pane as a tab, and is now the hidden one + + var destination = workspace.RouteLine("Notes", Ann); + + await Assert.That(destination.Unread).IsGreaterThan(0); + } +} diff --git a/tests/SharpMUTerm.Tui.Tests/TriggerRouteDestinationTests.cs b/tests/SharpMUTerm.Tui.Tests/TriggerRouteDestinationTests.cs new file mode 100644 index 0000000..03e0f64 --- /dev/null +++ b/tests/SharpMUTerm.Tui.Tests/TriggerRouteDestinationTests.cs @@ -0,0 +1,225 @@ +using SharpConsoleUI.Drivers; +using SharpMUTerm.Core.Automation; +using SharpMUTerm.Core.Commands; +using SharpMUTerm.Core.Configuration; +using SharpMUTerm.Core.Text; +using SharpMUTerm.Core.Workspaces; +using SharpMUTerm.Graphics; + +namespace SharpMUTerm.Tui.Tests; + +/// +/// The two reported defects, from the shell's side and over a live connection: a trigger could only +/// route to a spawn window, and a highlight colour did not survive the same rule's rewrite. +/// +/// Both are asserted end to end rather than on the engine alone, because both have a second half here. +/// A destination is only a destination if the shell appends to it (OnSpawnLine), and a highlight +/// is only a highlight if the markup a pane is fed carries the colour — a pane holds Spectre markup and +/// nothing else, so a StyledLine that was right on the way in proves nothing about the frame. +/// +/// +/// +/// Serialised with the other end-to-end suites: constructing the app and rendering a frame both touch +/// the process-global console streams. +/// +[NotInParallel] +public class TriggerRouteDestinationTests +{ + private const int Width = 160; + private const int Height = 40; + + private const string Ann = "Convergence.Ann"; + private const string Bob = "Convergence.Bob"; + + private static readonly TerminalCapabilities Headless = + new(GraphicsProtocol.None, supportsTrueColor: true, supportsKittyGraphics: false, supportsSixel: false); + + // ---- Routing somewhere that is not a fresh spawn window ----------------------------------- + + /// + /// The headline. Ann's rule routes to Bob — a window that already exists, of a kind no rule + /// could reach — and the line lands in it. Before the fix routing went through + /// Workspace.RouteSpawn, which can only ever answer with a spawn id it owns, so this opened a + /// fourth pane called Bob beside Bob's own and left his empty. + /// + [Test] + public async Task ARuleRoutesIntoAnotherCharactersWindowThatAlreadyExists() + { + var app = await Two(RouteTo("Bob")); + + Receive(app, AnnWire, " Ann says, \"hello\"\n"); + + await Assert.That(string.Join("\n", app.PaneLines(MainWindowOf(app, Bob)))).Contains("hello"); + await Assert.That(app.WindowIds()).DoesNotContain(Workspace.SpawnWindowId(Ann, "Bob")); + } + + /// + /// And into a character's own main window, which the F2 route list has always been able to name and a + /// rule could not reach: main there means do not route, so it only ever described a + /// line the rule did not also gag. A gagging rule aimed at the main window used to delete the line. + /// + [Test] + public async Task AGaggingRuleCanStillPutItsLineInItsOwnMainWindow() + { + var app = await Two(RouteTo("Ann")); + + Receive(app, AnnWire, " Ann says, \"hello\"\n"); + + await Assert.That(string.Join("\n", app.PaneLines(MainWindowOf(app, Ann)))).Contains("hello"); + } + + /// + /// A target nothing answers to is still a capture pane, created and owned by the matching session — + /// the path every capture rule that ships takes, and the one that must not have moved. + /// + [Test] + public async Task AnUnknownTargetStillOpensACapturePane() + { + var app = await Two(RouteTo("Chat")); + + Receive(app, AnnWire, " Ann says, \"hello\"\n"); + + var id = Workspace.SpawnWindowId(Ann, "Chat"); + await Assert.That(app.WindowIds()).Contains(id); + await Assert.That(app.WindowOwnerOf(id)).IsEqualTo(Ann); + await Assert.That(string.Join("\n", app.PaneLines(id))).Contains("hello"); + } + + /// + /// The per-session guarantee is untouched, and this is the fixture it was bought with: two characters + /// running one capture rule still get a pane each. Resolution admits another character's main + /// window and never their capture panes, so a shared channel name cannot collapse the two back into + /// one and file the second character's channel under the first. + /// + [Test] + public async Task TwoCharactersCapturingOneNameStillGetAPaneEach() + { + var app = await Two(RouteTo("Public")); + + Receive(app, AnnWire, " Ann says, \"first\"\n"); + Receive(app, BobWire, " Bob says, \"second\"\n"); + + var ann = string.Join("\n", app.PaneLines(Workspace.SpawnWindowId(Ann, "Public"))); + var bob = string.Join("\n", app.PaneLines(Workspace.SpawnWindowId(Bob, "Public"))); + + await Assert.That(ann).Contains("first"); + await Assert.That(ann).DoesNotContain("second"); + await Assert.That(bob).Contains("second"); + await Assert.That(bob).DoesNotContain("first"); + } + + /// + /// A window this session does not own is not relabelled by routing into it. OwnerLabel prefixes + /// a tab as Owner: Name to tie a capture pane scattered into another pane back to its + /// character; stamping it on a destination somebody else owns would rename their pane after whoever + /// last routed a line into it. + /// + [Test] + public async Task RoutingIntoAWindowDoesNotRelabelItAfterTheRoutingCharacter() + { + var app = await Two(RouteTo("Bob")); + + Receive(app, AnnWire, " Ann says, \"hello\"\n"); + + await Assert.That(app.WindowOwnerLabelOf(MainWindowOf(app, Bob))).IsNull(); + } + + // ---- The highlight, in the markup a pane is actually fed ------------------------------------ + + /// + /// A rule that rewrites and highlights: the pane's markup carries the colour. On the unfixed build + /// the rewrite ran after the highlight and replaced the line with an unstyled one, so the pane was + /// fed plain text while the F2 screen went on badging that rule H and painting its swatch. + /// + [Test] + public async Task ARewrittenLineReachesThePaneWearingItsHighlight() + { + var app = await Two(Configuration(new TriggerActions + { + HighlightForeground = TerminalColor.FromRgb(0xff, 0xd7, 0x00), + Rewrite = "» $1", + })); + + Receive(app, AnnWire, " Ann says, \"hello\"\n"); + + var line = app.PaneLines(MainWindowOf(app, Ann)).Last(); + await Assert.That(line).Contains("» Ann says"); + await Assert.That(line).Contains("#ffd700"); + } + + // ---- Harness ------------------------------------------------------------------------------ + + private RecordingTelnetSession AnnWire { get; set; } = new(); + + private RecordingTelnetSession BobWire { get; set; } = new(); + + /// The window a character's own output goes to — found by its owner rather than assumed. + private static string MainWindowOf(SharpMUTermApp app, string sessionKey) => + app.WindowIds().Single(id => + app.WindowOwnerOf(id) == sessionKey && !id.StartsWith(Workspace.SpawnPrefix, StringComparison.Ordinal)); + + private static AppConfiguration RouteTo(string target) => + Configuration(new TriggerActions { SpawnTarget = target, Gag = true }); + + private static AppConfiguration Configuration(TriggerActions actions) + { + var config = new AppConfiguration(); + config.TriggerSets.Add(new TriggerSet + { + Name = "Comms", + Triggers = + { + new Trigger { Name = "Public", Pattern = "^ (.+)$", Actions = actions }, + }, + }); + + config.Worlds.Add(new WorldDefinition + { + Name = "Convergence", + Host = "convergence.example.org", + Port = 4201, + Characters = + { + new CharacterDefinition { Name = "Ann", Logging = new LoggingSettings(), TriggerSets = { "Comms" } }, + new CharacterDefinition { Name = "Bob", Logging = new LoggingSettings(), TriggerSets = { "Comms" } }, + }, + }); + + return config; + } + + /// + /// Both characters open and connected. Two, because every destination this is about is a window + /// somebody else has — and because a session that was never connected never runs its receive path, + /// which would make any routing assertion true whatever the code does. + /// + private async Task Two(AppConfiguration config) + { + Console.SetIn(TextReader.Null); + AnnWire = new RecordingTelnetSession(); + BobWire = new RecordingTelnetSession(); + + var app = new SharpMUTermApp(config, Headless, new HeadlessConsoleDriver(Width, Height)); + await Open(app, Ann, AnnWire); + await Open(app, Bob, BobWire); + app.RenderNextFrame(); + return app; + } + + private static async Task Open(SharpMUTermApp app, string sessionKey, RecordingTelnetSession wire) + { + app.TelnetFactory = _ => wire; + if (!app.DispatchCommand(CommandIds.Character(sessionKey))) + { + throw new InvalidOperationException($"the app would not switch to {sessionKey}"); + } + + await app.FindSession(sessionKey)!.ConnectAsync(); + } + + private static void Receive(SharpMUTermApp app, RecordingTelnetSession wire, string text) + { + wire.Receive(text); + app.RenderNextFrame(); + } +} diff --git a/tests/SharpMUTerm.Tui.Tests/TriggersScreenRendererTests.cs b/tests/SharpMUTerm.Tui.Tests/TriggersScreenRendererTests.cs index b87c423..9b42091 100644 --- a/tests/SharpMUTerm.Tui.Tests/TriggersScreenRendererTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/TriggersScreenRendererTests.cs @@ -54,7 +54,7 @@ private static IReadOnlyList Scene() => new[] [Test] public async Task Render_RuleListShowsNamePatternOwningSetAndRoute() { - var lines = TriggersScreenRenderer.Render(Scene(), selectedTrigger: 0, spawnTargets: new[] { "Chat", "Combat log" }); + var lines = TriggersScreenRenderer.Render(Scene(), selectedTrigger: 0, routeTargets: new[] { "Chat", "Combat log" }); var rowIndex = lines.FindIndex(l => l.Contains("Tell") && l.Contains(@"^(\w+) tells you")); await Assert.That(lines[rowIndex]).Contains("→ Chat"); @@ -66,7 +66,7 @@ public async Task Render_RuleListShowsNamePatternOwningSetAndRoute() [Test] public async Task Render_FlagsSummariseGagHighlightAndSpawn() { - var lines = TriggersScreenRenderer.Render(Scene(), selectedTrigger: 0, spawnTargets: new[] { "Chat" }); + var lines = TriggersScreenRenderer.Render(Scene(), selectedTrigger: 0, routeTargets: new[] { "Chat" }); var tellRowIndex = lines.FindIndex(l => l.Contains("Tell") && l.Contains(@"^(\w+) tells you")); var tellSub = lines[tellRowIndex + 1]; @@ -82,7 +82,7 @@ public async Task Render_FlagsSummariseGagHighlightAndSpawn() [Test] public async Task Render_SelectedTriggerEditorShowsPatternAndRoute() { - var lines = TriggersScreenRenderer.Render(Scene(), selectedTrigger: 0, spawnTargets: new[] { "Chat", "Combat log" }); + var lines = TriggersScreenRenderer.Render(Scene(), selectedTrigger: 0, routeTargets: new[] { "Chat", "Combat log" }); await Assert.That(lines.Any(l => l.Contains("match pattern"))).IsTrue(); await Assert.That(lines.Any(l => l.Contains(@"^(\w+) tells you"))).IsTrue(); @@ -99,10 +99,10 @@ public async Task Render_SelectedTriggerEditorShowsPatternAndRoute() [Test] public async Task Render_GagToggleReflectsActionsGag() { - var gagged = TriggersScreenRenderer.Render(Scene(), selectedTrigger: 1, spawnTargets: Array.Empty()); + var gagged = TriggersScreenRenderer.Render(Scene(), selectedTrigger: 1, routeTargets: Array.Empty()); await Assert.That(gagged.Any(l => l.Contains("[[x]] gag line") || l.Contains("#00f5b7][[x]][/] gag line"))).IsTrue(); - var notGagged = TriggersScreenRenderer.Render(Scene(), selectedTrigger: 0, spawnTargets: Array.Empty()); + var notGagged = TriggersScreenRenderer.Render(Scene(), selectedTrigger: 0, routeTargets: Array.Empty()); await Assert.That(notGagged.Any(l => l.Contains("[dim][[ ]] gag line[/]"))).IsTrue(); } @@ -115,7 +115,7 @@ public async Task Render_GagToggleReflectsActionsGag() [Test] public async Task Render_HighlightCaptionAndSwatchAppearWhenColourSet() { - var lines = TriggersScreenRenderer.Render(Scene(), selectedTrigger: 0, spawnTargets: Array.Empty()); + var lines = TriggersScreenRenderer.Render(Scene(), selectedTrigger: 0, routeTargets: Array.Empty()); var heading = lines.Single(l => l.Contains("highlight") && !l.Contains("fg") && !l.Contains("bg")); await Assert.That(heading).Contains("recoloured"); @@ -129,7 +129,7 @@ public async Task Render_HighlightCaptionAndSwatchAppearWhenColourSet() [Test] public async Task Render_EmptySetsShowsNoTriggers() { - var lines = TriggersScreenRenderer.Render(Array.Empty(), selectedTrigger: -1, spawnTargets: Array.Empty()); + var lines = TriggersScreenRenderer.Render(Array.Empty(), selectedTrigger: -1, routeTargets: Array.Empty()); await Assert.That(lines.Any(l => l.Contains("no triggers"))).IsTrue(); await Assert.That(lines.Any(l => l.Contains("Triggers & spawn routing"))).IsTrue(); @@ -150,7 +150,7 @@ public async Task Render_EscapesMarkupBracketsInNamesAndPatterns() }, }; - var lines = TriggersScreenRenderer.Render(sets, selectedTrigger: 0, spawnTargets: Array.Empty()); + var lines = TriggersScreenRenderer.Render(sets, selectedTrigger: 0, routeTargets: Array.Empty()); await Assert.That(lines.Any(l => l.Contains("Br[[acket]]"))).IsTrue(); await Assert.That(lines.Any(l => l.Contains("x[[1]]"))).IsTrue(); await Assert.That(lines.Any(l => l.Contains("Weird[[Set]]"))).IsTrue(); From ee02d8d7e52e435858d2e2513aa20f7bea09c4fa Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Tue, 11 Aug 2026 15:59:05 -0500 Subject: [PATCH 19/19] fix(triggers): a closed spawn window is placed again, not fed invisibly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review catch. The title lookup already required a *placed* window, and the renamed-spawn fallback beside it did not — so a spawn window whose pane the user closed could be returned as a destination and the channel written into a buffer nothing draws. That is reachable rather than theoretical: the registry outlives the layout, and a restored workspace registers windows a saved layout no longer places. It also contradicted RouteLine's own doc comment, which says a closed window is passed over and the line goes somewhere visible. Both halves, because guarding only the fallback moves the same defect one call deeper: RouteSpawn returned a registered-but-unplaced window untouched, since it only placed one it had just created. It now places on the way past whether or not the window is new — which is what lets FindRouteTarget decline a closed window and fall through, reopening the pane under the same id with its history in it. The regression test was checked against the unfixed code and fails there. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN --- src/SharpMUTerm.Core/Workspace/Workspace.cs | 22 +++++++++++++++--- .../Workspace/RouteToExistingWindowTests.cs | 23 +++++++++++++++++++ 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/src/SharpMUTerm.Core/Workspace/Workspace.cs b/src/SharpMUTerm.Core/Workspace/Workspace.cs index 52316de..85a4cb1 100644 --- a/src/SharpMUTerm.Core/Workspace/Workspace.cs +++ b/src/SharpMUTerm.Core/Workspace/Workspace.cs @@ -228,7 +228,15 @@ public WorkspaceWindow RouteLine(string target, string? sessionKey = null) // 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. - return best ?? _windows.GetValueOrDefault(SpawnWindowId(sessionKey, target)); + // + // 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. + var renamed = _windows.GetValueOrDefault(SpawnWindowId(sessionKey, target)); + return best ?? (renamed is not null && Layout.FindWindow(renamed.Id) is not null ? renamed : null); } /// @@ -265,9 +273,17 @@ public WorkspaceWindow RouteSpawn(string target, string? sessionKey = null) { ArgumentException.ThrowIfNullOrEmpty(target); var id = SpawnWindowId(sessionKey, target); - if (!_windows.TryGetValue(id, out var window)) + var window = _windows.TryGetValue(id, out var existing) + ? 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. + if (Layout.FindWindow(id) is null) { - window = Register(new WorkspaceWindow(id, target, WindowKind.Spawn, sessionKey)); Layout.AddWindow(id, activate: false); // spawns open in the background and accrue unread } diff --git a/tests/SharpMUTerm.Core.Tests/Workspace/RouteToExistingWindowTests.cs b/tests/SharpMUTerm.Core.Tests/Workspace/RouteToExistingWindowTests.cs index 2284791..7671909 100644 --- a/tests/SharpMUTerm.Core.Tests/Workspace/RouteToExistingWindowTests.cs +++ b/tests/SharpMUTerm.Core.Tests/Workspace/RouteToExistingWindowTests.cs @@ -176,6 +176,29 @@ public async Task AClosedWindowIsNotADestination() await Assert.That(destination.Id).IsEqualTo(Workspace.SpawnWindowId(Ann, "Notes")); } + /// + /// The same rule through the renamed-spawn fallback, which is the one arm that does not go by title. + /// A spawn window whose pane the user closed is still in the registry — the registry outlives the + /// layout, and a restored workspace can register windows a saved layout no longer places — so the + /// fallback would hand back a window nothing draws. It has to place the pane again instead, under + /// the same id, so the channel comes back with its history rather than going somewhere invisible. + /// + [Test] + public async Task ARenamedSpawnWindowWhosePaneWasClosedIsPlacedAgainRatherThanFedInvisibly() + { + var workspace = AnnsWorkspace(); + var spawned = workspace.RouteLine("Chat", Ann); + spawned.Title = "Tells"; // the user renames it, so no title answers to "Chat" any more + workspace.Layout.RemoveWindow(spawned.Id); // and closes its pane, leaving it registered + + await Assert.That(workspace.FindRouteTarget("Chat", Ann)).IsNull(); + + var destination = workspace.RouteLine("Chat", Ann); + + await Assert.That(destination.Id).IsEqualTo(spawned.Id); + await Assert.That(workspace.Layout.FindWindow(destination.Id)).IsNotNull(); + } + /// /// Routing badges the destination unread when it is not the window being read, whichever kind it /// turned out to be. The badge is the only thing that says a background pane gained a line, and a