From 5c4ccd4733b03a3c554b896fcdaa631f573191a7 Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Mon, 10 Aug 2026 14:37:29 -0500 Subject: [PATCH 1/6] docs: spec the away divider A boundary row drawn where the reader left off when they tab away from the terminal, so returning does not mean guessing which lines are already read. The signal is terminal focus reporting (CSI ?1004h), which SharpConsoleUI neither asks for nor decodes. Both halves of the workaround are recorded: WriteClipboardOsc52 is a raw-escape writer wearing its first customer's name, and focus-in reaches us disguised as a Tab keypress because DispatchCsi reads a trailing I as Tab. Focus-out is discarded upstream and is not recoverable, so the boundary anchors to the last input event instead. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0195EnLstmb6tAgpbUHjHBEX --- .../specs/2026-08-10-away-divider-design.md | 223 ++++++++++++++++++ 1 file changed, 223 insertions(+) create mode 100644 docs/superpowers/specs/2026-08-10-away-divider-design.md diff --git a/docs/superpowers/specs/2026-08-10-away-divider-design.md b/docs/superpowers/specs/2026-08-10-away-divider-design.md new file mode 100644 index 0000000..0959f1d --- /dev/null +++ b/docs/superpowers/specs/2026-08-10-away-divider-design.md @@ -0,0 +1,223 @@ +# The away divider: where you were when you left the terminal + +**Date:** 2026-08-10 +**Status:** proposed — design only, nothing implemented + +## Problem + +Tab away from the terminal, come back later, and there is no way to tell which of the lines on +screen you have already read. Every pane has bottom-anchored through your absence, so what you land +on is the newest output with no boundary in it. The client already knows how to say "the rows above +this are not live" twice over — `FreezeBarRenderer` divides pinned scrollback from the live tail, +`RestoreBarRenderer` closes off content carried over from a previous run — and has nothing to say +about the one absence that happens many times a day. + +The unread counts on the rail and the tab strip do not answer it. `Workspace.NoteActivity` only +counts a line when the window is *not* `IsCaughtUp` — not the visible tab, or scrolled off its live +tail (`Workspace.cs:278`). The window you were looking at when you alt-tabbed away is visible and at +its tail the whole time you are gone, so it accrues nothing. The count is a "which pane should I +look at" signal for background windows and is silent about the foreground one, which is exactly the +one you were reading. + +## What the reader gets + +One row, drawn inline in each window at the point where you left: + +``` +▾ AWAY 37 lines · 12 min ──────────────────────────────────────── +``` + +Everything below it arrived while you were away. It is a boundary marker and not a restyling of the +content, for the reason `RestoreBarRenderer` gives: the lines are worth having because they are the +game's own text in the game's own colours, and recolouring them to prove they are new would destroy +the thing being marked. The restored bar sits *below* its content because a pane bottom-anchors and +the boundary is what you land on; this one sits *above* its content for the same reason read the +other way — what you land on is the newest line, and the thing you are looking for is up. + +## Why this is hard: the terminal will not tell us, quite + +A terminal reports focus only if asked. `CSI ?1004h` turns it on, after which the terminal writes +`ESC [ I` when its window gains focus and `ESC [ O` when it loses it, down the same pipe as +keystrokes. Neither half is reachable through SharpConsoleUI as shipped: + +- **No released version asks.** Checked against 2.5.18, the newest on nuget (we are pinned at + 2.5.14): the assembly's UTF-16 string heap holds `[?2004h`/`[?2004l` for bracketed paste and no + `?1004` in any version. The `FocusChanged` symbols in it are `FocusManager.FocusChanged` — + control focus, which the app already hooks for `PinFocusToArmedBar`. +- **`ESC [ O` is discarded.** `AnsiInputParser.DispatchCsi` has no case for it, so it becomes an + `UnknownSequenceEvent`, and `UnixStdinReader` dispatches only key, paste and mouse events + (`UnixStdinReader.cs:147`). Nothing downstream can see it. +- **`ESC [ I` arrives disguised as a Tab keypress.** `DispatchCsi` reads a trailing `I` as Tab + (`AnsiInputParser.cs:511`), which is right for the forms carrying modifiers — `ESC [ 1;5 I` is + genuinely Ctrl+Tab in xterm — and wrong for the bare form, which is focus-in. That is an upstream + bug, and it is also the only way the message reaches us. + +`IConsoleDriver` exposes no focus event and `NetConsoleDriverOptions` no hook, so there is no +supported seam. The input-stack wall in CLAUDE.md holds: owning this properly means either an +upstream PR or a from-scratch `IConsoleDriver`. + +Two decisions follow, and both are exploiting an implementation detail rather than a contract. Both +are contained in one file so the blast radius is one file. + +### Asking for focus reports + +`IConsoleDriver.WriteClipboardOsc52(string)` is named for its first customer and does not do what +its name says: its body takes the console lock and writes the string verbatim, with no validation, +wrapping or encoding (`NetConsoleDriver.cs:667`). It is a raw-escape emitter. + +We need one. Escape sequences go to stdout, the framework owns stdout and paints whole frames +through it, and a `Console.Out.Write` of our own can land mid-frame and corrupt a paint. This is the +only public write that is serialised against the renderer, because it takes the same `_consoleLock`. + +The risk is that we are depending on a body rather than a signature: a future version that validates +the payload is OSC 52 breaks us. Mitigated by putting both writes behind one `EmitTerminalMode` +method, so the change is one line, and by filing the upstream ask for a `WriteRaw` or a focus option +regardless. + +### Recognising the disguised focus-in + +`ConsoleWindowSystem.RegisterGlobalShortcut(modifiers, key, Func)` registers a handler that +decides whether the key is consumed — return `false` and it continues down the normal pipeline +(`ConsoleWindowSystem.cs:1683`). Global shortcuts are tried before any window sees the key, so this +also covers the case where a settings or quit overlay is open, which a `PreviewKeyPressed` hook on +the main window would miss. + +So: claim bare Tab, consume it only when it is a focus-in, decline otherwise. A declined Tab reaches +`InputBarControl.cs:278` (cycle to the sibling command bar) and the overlays exactly as it does +today. + +**Telling the two apart is a question about time, not about the key.** A Tab arriving after a long +quiet gap is a return; a Tab arriving while you are demonstrably at the keyboard is a Tab. The +misfire is benign in the direction it fires: a genuine Tab pressed after ten minutes of silence is +*also* a return, because you had to come back to press it. `Ctrl+I`, which the terminal spells as a +bare Tab too (`MacroKeys` records this), is covered by the same gap test. + +The threshold is a debounce, not an idle timer — nothing is drawn without an actual focus-in — so it +can be short. **30 seconds**, not configurable in the first cut. + +### What we do not get + +`ESC [ O` is unreachable, so we cannot timestamp your departure. The boundary is anchored to the +last input event we saw instead (see below), which is seconds off at worst: you stop typing, then +you leave. + +**Unix only.** The Windows branch of `NetConsoleDriver` is a `Console.ReadKey` loop with its own +ad-hoc sequence reassembly, not `AnsiInputParser`, so `?1004` must not be enabled there — the +reassembler would see `ESC [ I` and make something else of it. On Windows the feature is inert. It +is also inert on any terminal that does not implement `?1004`; kitty, WezTerm and Ghostty all do, +and tmux passes them through only with `focus-events on`. + +## Design + +### 1. `TerminalFocusWatcher` (Tui) + +One file, owning the whole trick. + +- `Start()` emits `\x1b[?1004h` through `EmitTerminalMode`; `Stop()` emits `\x1b[?1004l`. Both are + no-ops off Unix. +- Subscribes to the **driver's** `KeyPressed`, `MouseEvent` and `Paste` — driver level, so it sees + input routed to overlays as well as to the workspace — and keeps `LastInputAt`. +- Registers bare Tab as a declining global shortcut. On a Tab, if the gap since the *previous* input + exceeds `ReturnThreshold`, raise `Returned(awaySince)` and return `true`; otherwise `false`. +- **Ordering trap:** the disguised focus-in *is* a `KeyPressed`, and the driver raises that before + `InputCoordinator` reaches the global shortcuts. The watcher must therefore compare against the + input before this one, not the timestamp it has just written. +- Takes an `Func` clock, so the timing is unit-testable with no terminal. + +Enabled only by `Program`, the same gate `save`, `logRoot` and `restore` use: an app that is not the +live entry point holds no watcher and writes nothing to any terminal. That keeps the snapshot +pipeline and the test suite from emitting mode changes into whatever console is attached. + +### 2. Where the boundary comes from + +`PaneLine` is `(string Markup, string? Stamp)` and the stamp is *formatted text*, not a time +(`PaneLine.cs:35`), so the boundary cannot be found retroactively by walking the buffer for lines +newer than some instant. Widening `PaneLine` to carry an arrival time would touch every append and +the restore codec. + +Track it forward instead: on every input event, record `PendingMark[windowId] = _lines[windowId].Count` +for each window. Input events are at human rate and the map is a few entries, so this is free. When +the return arrives, that index is where you left. + +### 3. Drawing it + +`AwayBarRenderer`, a sibling of `FreezeBarRenderer` and `RestoreBarRenderer`, sharing their 48-cell +rule and taking a resolved `#rrggbb` accent so it is pure and testable without a terminal. Both +figures are on the bar for the reason the restore bar carries two: the count answers "is this a +glance or a session's worth", the duration answers "how far behind am I". + +Inserted into `_lines` at the recorded index, which is a mid-buffer insert and therefore costs one +`RepaintPanes` for that window — the expensive whole-buffer re-feed. That is affordable here for the +same reason it is affordable for the timestamp toggle: it is bounded by one deliberate event, a +return, and not by lines or frames. + +Two things it must not do: + +- **Not count as unread.** It is the client's own chrome; `NoteActivity` is not called for it. +- **Not reach the restore log.** Already true for free — `RestoreLog` is fed at `OnLine`/`OnSpawnLine` + and deliberately not at `AppendWindowLine`, so client chrome has never gone into it. + +A window with no lines at all gets no bar: there is no boundary to mark. + +### 4. Consumption + +The divider is cleared when you have read past it, and "read past it" needs care, because the +obvious test does not survive contact. A bottom-anchored pane is *already* `IsCaughtUp` the instant +you return — that predicate is "visible and not scrolled back" (`Workspace.cs:384`) and says nothing +about how much arrived. Clearing on it would clear the divider before you had read a word of the two +hundred lines above the fold. + +The rule is therefore three conjuncts: + +1. the divider row has been **inside the viewport** — computable per frame from the panel's + `VerticalScrollOffset`, `ViewportHeight` and the row's index; and +2. the pane is at its **live tail**; and +3. at least **one input event** has landed since the bar was drawn. + +Read together: you saw the marker, and you are now at the bottom, so you crossed the gap between +them. (1) is what makes a deep absence keep its divider until you scroll up and find it. (3) is what +stops a shallow absence — a handful of lines, all on screen with the marker — clearing in the very +frame it appears. + +Clearing is a removal from `_lines` and so costs the same single-window re-feed the insertion did. + +Each window carries at most one. A return while a previous divider is still unconsumed replaces it, +because two boundaries in one pane cannot both be "where you left". + +## Verification + +- `AwayBarRendererTests` — the markup, both figures, escaping. +- `TerminalFocusWatcherTests` — on a fake clock: a Tab inside the threshold declines and is passed + through; a Tab outside it consumes and raises `Returned`; the disguised focus-in's own `KeyPressed` + does not move the baseline it is about to be compared against; nothing is emitted off Unix or + without a live driver. +- `AwayDividerTests` — insertion at the recorded index; the three consumption conjuncts, each failing + alone; replacement on a second return; no bar for an empty window; the bar is not counted as unread. +- Snapshot views `away` (divider on screen) and `away-scrollback` (divider above the fold, over + `LoadLongScene`). CLAUDE.md is explicit that the three `scroll*` views are the only ones with more + output than a pane holds, and that anything touching the output area needs one. +- The whole suite and `dotnet build SharpMUTerm.slnx` warning-free. + +## Out of scope + +- **Focus-out.** Unreachable without an upstream change; the last-input anchor is the answer until + then. +- **Windows.** Inert, deliberately. An idle-time fallback there is a separate decision and would + bring back the misfire this design exists to avoid. +- **A jump-to-divider chord, and a "while you were away" digest across windows.** Both were + considered and dropped from the first cut; the divider is the thing that was asked for. +- **Configurability.** One threshold, one appearance, no settings-screen entry until there is a + reason. + +## Upstream + +Worth filing against `nickprotop/ConsoleEx` whatever we ship, because both tricks here are working +around it: + +1. Bare, parameterless `CSI I` is focus-in, not Tab — a bug on its own terms. +2. A `FocusInputEvent`, a case for it in `UnixStdinReader`'s dispatch switch, a `FocusChanged` event + on `IConsoleDriver`, and `?1004h`/`?1004l` paired where `?2004` already is + (`NetConsoleDriver.cs:446`, `:531`). Roughly 60 lines on the Unix path. +3. A `WriteRaw` that means what it says, so `WriteClipboardOsc52` stops being one by accident. + +If that lands, this design's §1 swaps its signal and nothing else in the feature moves. From 4bdc289fd3aef4571738de2fc580161ea8a34d07 Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Mon, 10 Aug 2026 14:43:25 -0500 Subject: [PATCH 2/6] feat(tui): the away bar and the focus watcher it is drawn from MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AwayBarRenderer is a sibling of the freeze and restore bars and carries the same two figures the restore bar does, for the same reason: a returning reader asks both how much is in front of them and how far behind they are. TerminalFocusWatcher is the whole workaround, in one file so it is one file to delete. It asks for focus reporting through WriteClipboardOsc52 — named for its first customer, but a verbatim raw write under the renderer's own lock — and recognises focus-in in the Tab keypress the framework's parser mistranslates it into. Telling that Tab from a real one is a question about time, and the comparison has to be against the input before it: the disguised focus-in is itself a KeyPressed, raised before the global shortcuts run. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0195EnLstmb6tAgpbUHjHBEX --- src/SharpMUTerm.Tui/AwayBarRenderer.cs | 84 +++++++ src/SharpMUTerm.Tui/Glyphs.cs | 6 + src/SharpMUTerm.Tui/TerminalFocusWatcher.cs | 220 ++++++++++++++++++ .../AwayBarRendererTests.cs | 67 ++++++ .../TerminalFocusWatcherTests.cs | 211 +++++++++++++++++ 5 files changed, 588 insertions(+) create mode 100644 src/SharpMUTerm.Tui/AwayBarRenderer.cs create mode 100644 src/SharpMUTerm.Tui/TerminalFocusWatcher.cs create mode 100644 tests/SharpMUTerm.Tui.Tests/AwayBarRendererTests.cs create mode 100644 tests/SharpMUTerm.Tui.Tests/TerminalFocusWatcherTests.cs diff --git a/src/SharpMUTerm.Tui/AwayBarRenderer.cs b/src/SharpMUTerm.Tui/AwayBarRenderer.cs new file mode 100644 index 0000000..7d7dd78 --- /dev/null +++ b/src/SharpMUTerm.Tui/AwayBarRenderer.cs @@ -0,0 +1,84 @@ +using System.Globalization; + +namespace SharpMUTerm.Tui; + +/// +/// Renders the bar marking where a reader was when they left the terminal: the one row saying that +/// everything below it arrived while they were away. +/// +/// Why a bar and not a restyling. The same reasoning sets out — +/// mark the boundary, not the content. The lines below this bar are worth having because they +/// are the game's own text in the game's own colours, so tinting them to prove they are new would +/// destroy the thing being marked. One row, drawn like the freeze bar and the restore bar, which divide +/// the same pane for the same kind of reason. +/// +/// +/// It sits above its content, where the restore bar sits below its own, and the two are the +/// same rule read from opposite ends: a pane bottom-anchors, so what you land on is the newest line. +/// For restored content that newest line is the boundary itself; here the boundary is behind you, and +/// what you are looking for is up. +/// +/// Pure, so the markup is unit-testable without a terminal. +/// +internal static class AwayBarRenderer +{ + /// How long the trailing rule is. The same 48 cells the freeze and restore bars draw. + private const int RuleCells = 48; + + /// 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 bar for lines that arrived over an absence of , + /// on an already-resolved #rrggbb accent. + /// + public static string Bar(int lines, TimeSpan away, string accentHex) + { + ArgumentException.ThrowIfNullOrEmpty(accentHex); + + // Both figures, for the reason the restore bar carries two: they answer different questions and a + // returning reader asks both. The count is "is this a glance or a session's worth" — how much + // scrolling is in front of me. The duration is "how far behind am I" — five minutes is a + // conversation you can still join, two hours is a different evening. + var count = lines == 1 ? "1 line" : $"{lines} lines"; + var rule = new string('─', RuleCells); + return $"[{accentHex}]{Glyphs.Away} {Label}[/] " + + $"[dim]{MarkupText.Escape($"{count} since you left · {Duration(away)}")} {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 + /// much scrolling is ahead, and "2 h 14 min" and "2 h" lead to the same decision. + /// + /// A negative or sub-minute span reads as "a moment", not as "0 min" — the anchor is the last input + /// event rather than the moment of departure (focus-out is not observable, see + /// ), so the figure is approximate by construction and should not + /// wear a precision it does not have. + /// + /// + internal static string Duration(TimeSpan away) + { + if (away < TimeSpan.FromMinutes(1)) + { + return "a moment"; + } + + if (away < TimeSpan.FromHours(1)) + { + return string.Create(CultureInfo.CurrentCulture, $"{(int)away.TotalMinutes} min"); + } + + if (away < TimeSpan.FromDays(1)) + { + var hours = (int)away.TotalHours; + var minutes = away.Minutes; + return minutes == 0 + ? string.Create(CultureInfo.CurrentCulture, $"{hours} h") + : string.Create(CultureInfo.CurrentCulture, $"{hours} h {minutes} min"); + } + + var days = (int)away.TotalDays; + return days == 1 ? "1 day" : string.Create(CultureInfo.CurrentCulture, $"{days} days"); + } +} diff --git a/src/SharpMUTerm.Tui/Glyphs.cs b/src/SharpMUTerm.Tui/Glyphs.cs index 7b8a901..2c16948 100644 --- a/src/SharpMUTerm.Tui/Glyphs.cs +++ b/src/SharpMUTerm.Tui/Glyphs.cs @@ -32,6 +32,12 @@ internal static class Glyphs /// public const string Restored = "\uf1da"; // nf-fa-history + /// + /// The bar marking where the reader was when they left the terminal — see . + /// A struck-through eye, because what the rows below it have in common is that nobody was looking. + /// + public const string Away = "\uf070"; // nf-fa-eye_slash + /// /// 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/TerminalFocusWatcher.cs b/src/SharpMUTerm.Tui/TerminalFocusWatcher.cs new file mode 100644 index 0000000..659e4b3 --- /dev/null +++ b/src/SharpMUTerm.Tui/TerminalFocusWatcher.cs @@ -0,0 +1,220 @@ +using System.Drawing; +using SharpConsoleUI.Drivers; + +namespace SharpMUTerm.Tui; + +/// +/// Notices that the reader has come back to the terminal after being away from it, so the client can +/// mark where they were (see ). +/// +/// This whole type is a workaround, and it is one file so that it is one file to delete. A +/// terminal reports focus only if asked — CSI ?1004h turns it on, after which the terminal writes +/// ESC [ I on focus gain and ESC [ O on focus loss, down the same pipe as keystrokes. +/// SharpConsoleUI neither asks nor decodes: no released version emits ?1004 (checked against +/// 2.5.18, the newest published; we are pinned at 2.5.14), IConsoleDriver carries no focus event, +/// and NetConsoleDriverOptions no hook. The input-stack wall in CLAUDE.md is real — owning this +/// properly means an upstream PR or a from-scratch driver. +/// +/// +/// How the two halves get in. is named for its +/// first customer and does not do what its name says: its body takes the console lock and writes the +/// string verbatim, with no validation or wrapping. It is a raw-escape emitter, and it is the only +/// public write serialised against the renderer — the framework paints whole frames through stdout, so +/// a Console.Out.Write of our own could land mid-frame. Emissions go through +/// alone, so if a future version starts validating that payload there is +/// one line to change. +/// +/// +/// And focus-in arrives disguised as a Tab keypress: AnsiInputParser.DispatchCsi reads a +/// trailing I as Tab, which is right for the forms carrying modifiers (ESC [ 1;5 I is +/// genuinely Ctrl+Tab in xterm) and wrong for the bare form, which is focus-in. So this type does not +/// choose to concern itself with Tab; Tab is the shape the message arrives in. +/// +/// +/// Focus-out is not recoverable. ESC [ O has no case in DispatchCsi and becomes an +/// UnknownSequenceEvent, which UnixStdinReader never dispatches. So we cannot timestamp a +/// departure, and reports the gap since the last input event instead — you stop +/// typing, then you leave, so it is seconds long at worst. +/// +/// +internal sealed class TerminalFocusWatcher : IDisposable +{ + /// Turns focus reporting on. + private const string EnableFocusReporting = "\x1b[?1004h"; + + /// Turns focus reporting off again. + private const string DisableFocusReporting = "\x1b[?1004l"; + + /// + /// How long a quiet gap has to be before an arriving Tab is read as a return rather than as a Tab. + /// + /// This is a debounce and not an idle timer: nothing is drawn without an actual focus-in, so the + /// number only has to be longer than the pauses inside ordinary typing. It can therefore be short, + /// and short is what keeps a real Tab a real Tab. The misfire is benign in the direction it fires — + /// a genuine Tab pressed after half a minute of silence is also a return, because you had + /// to come back to the terminal to press it. Ctrl+I, which the terminal spells as a bare Tab + /// as well (see ), is covered by the same test. + /// + /// + public static readonly TimeSpan DefaultReturnThreshold = TimeSpan.FromSeconds(30); + + private readonly IConsoleDriver _driver; + private readonly TimeProvider _time; + private readonly TimeSpan _threshold; + private readonly bool _enabled; + + private DateTimeOffset _lastInputAt; + private DateTimeOffset _previousInputAt; + private bool _started; + private bool _disposed; + + public TerminalFocusWatcher( + IConsoleDriver driver, + TimeProvider time, + bool enabled, + TimeSpan? threshold = null) + { + _driver = driver; + _time = time; + _enabled = enabled; + _threshold = threshold ?? DefaultReturnThreshold; + _lastInputAt = _previousInputAt = time.GetUtcNow(); + } + + /// + /// Raised when the reader has come back, carrying how long they were away for. Runs on whichever + /// thread delivered the Tab, which is the UI thread: a global shortcut is dispatched from the + /// framework's own input pump. + /// + public event Action? Returned; + + /// + /// Raised for every input event the driver saw — keys, mouse and paste alike, and including input + /// routed to an overlay rather than to the workspace. It is what the client hangs "where was the + /// reader last looking" off, and it is here rather than on the app's own key handler because that + /// handler does not see a key an overlay consumed. + /// + public event Action? Input; + + /// + /// Whether this watcher will do anything at all. False leaves the terminal untouched and leaves + /// every Tab alone, which is what asks for off Unix. + /// + public bool IsEnabled => _enabled; + + /// + /// Whether focus reporting can be used with at all. + /// + /// Not on Windows. The Windows branch of NetConsoleDriver is a + /// Console.ReadKey loop with its own ad-hoc sequence reassembly rather than + /// AnsiInputParser, so an ESC [ I arriving there would be reassembled into something + /// else entirely. The feature is inert on Windows, deliberately, rather than wrong. + /// + /// + /// Not headless. A snapshot or a test has no terminal to report focus and no reader to have + /// left one. WriteClipboardOsc52 is a default interface method with an empty body, so the + /// emission would already be a no-op there — but the Tab interpretation would not be, and a + /// headless harness pressing Tab must get a Tab. + /// + /// + public static bool ShouldEnable(IConsoleDriver driver) => + !OperatingSystem.IsWindows() && driver is not HeadlessConsoleDriver; + + /// Asks the terminal to report focus and starts listening for input. Idempotent. + public void Start() + { + if (!_enabled || _started) + { + return; + } + + _started = true; + _driver.KeyPressed += OnKeyPressed; + _driver.Paste += OnPaste; + _driver.MouseEvent += OnMouse; + EmitTerminalMode(EnableFocusReporting); + } + + /// + /// Stops the terminal reporting focus and stops listening. Idempotent, and safe to call on a + /// watcher that never started — which is what a disposal after a failed launch does. + /// + public void Stop() + { + if (!_started) + { + return; + } + + _started = false; + _driver.KeyPressed -= OnKeyPressed; + _driver.Paste -= OnPaste; + _driver.MouseEvent -= OnMouse; + EmitTerminalMode(DisableFocusReporting); + } + + /// + /// Offers this watcher a bare Tab, and reports whether it was the terminal's focus-in rather than + /// the reader's Tab key. True means the key is consumed and has been raised; + /// false means it is an ordinary Tab and must carry on down the pipeline untouched. + /// + /// The comparison is against the input before this one. The disguised focus-in is a + /// KeyPressed, and the driver raises that before InputCoordinator reaches the global + /// shortcuts — so by the time this runs, has already been moved to now by + /// this very keystroke. Measuring from it would find a gap of zero every time and this would never + /// fire once. + /// + /// + public bool TryTakeAsReturn() + { + if (!_enabled) + { + return false; + } + + var away = _lastInputAt - _previousInputAt; + if (away < _threshold) + { + return false; + } + + Returned?.Invoke(away); + return true; + } + + /// + /// Records that the reader did something. Public because a headless harness drives keys straight + /// into the app rather than through a driver that could raise them (see SimulateKey), and the + /// two paths must reach the same state or a test would be exercising a different rule from the one + /// that ships. + /// + public void NoteInput() + { + _previousInputAt = _lastInputAt; + _lastInputAt = _time.GetUtcNow(); + Input?.Invoke(); + } + + public void Dispose() + { + if (_disposed) + { + return; + } + + _disposed = true; + Stop(); + } + + private void OnKeyPressed(object? sender, ConsoleKeyInfo key) => NoteInput(); + + private void OnPaste(object? sender, string text) => NoteInput(); + + private void OnMouse(object sender, List flags, Point point) => NoteInput(); + + /// + /// The one place a terminal mode is written. See the type remarks for why this goes through the + /// clipboard-named writer and why it is worth having exactly one line that does. + /// + private void EmitTerminalMode(string sequence) => _driver.WriteClipboardOsc52(sequence); +} diff --git a/tests/SharpMUTerm.Tui.Tests/AwayBarRendererTests.cs b/tests/SharpMUTerm.Tui.Tests/AwayBarRendererTests.cs new file mode 100644 index 0000000..265e62d --- /dev/null +++ b/tests/SharpMUTerm.Tui.Tests/AwayBarRendererTests.cs @@ -0,0 +1,67 @@ +using SharpMUTerm.Tui; + +namespace SharpMUTerm.Tui.Tests; + +public class AwayBarRendererTests +{ + [Test] + public async Task Bar_CarriesTheLabelInTheAccentAndBothFiguresDim() + { + var bar = AwayBarRenderer.Bar(37, TimeSpan.FromMinutes(12), "#c678dd"); + + await Assert.That(bar).Contains($"[#c678dd]{Glyphs.Away} {AwayBarRenderer.Label}[/]"); + + // Both figures, because a returning reader asks two questions: how much is in front of me, and + // how far behind am I. A bar carrying one of them answers half. + await Assert.That(bar).Contains("37 lines"); + await Assert.That(bar).Contains("12 min"); + await Assert.That(bar).Contains("[dim]"); + await Assert.That(bar).Contains("─"); + } + + [Test] + public async Task Bar_CountsOneLineInTheSingular() + { + var bar = AwayBarRenderer.Bar(1, TimeSpan.FromMinutes(3), "#c678dd"); + + await Assert.That(bar).Contains("1 line "); + await Assert.That(bar).DoesNotContain("1 lines"); + } + + [Test] + public void Bar_RejectsAnEmptyAccent() + { + Assert.Throws(() => AwayBarRenderer.Bar(4, TimeSpan.FromMinutes(2), 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 + /// up as "0 min", which would claim a precision the figure does not have. + /// + [Test] + [Arguments(0, "a moment")] + [Arguments(59, "a moment")] + [Arguments(60, "1 min")] + [Arguments(12 * 60, "12 min")] + [Arguments(59 * 60, "59 min")] + [Arguments(60 * 60, "1 h")] + [Arguments((2 * 60 + 14) * 60, "2 h 14 min")] + [Arguments(24 * 60 * 60, "1 day")] + [Arguments(3 * 24 * 60 * 60, "3 days")] + public async Task Duration_ReadsInTheCoarsestUsefulUnit(int seconds, string expected) + { + await Assert.That(AwayBarRenderer.Duration(TimeSpan.FromSeconds(seconds))).IsEqualTo(expected); + } + + /// + /// A negative span is reachable: the anchor is the last input event, and a clock that steps + /// backwards (an NTP correction, a suspend) can put it after the moment the return arrives. It must + /// read as "a moment" rather than produce a negative count of minutes. + /// + [Test] + public async Task Duration_TreatsATimeGoingBackwardsAsAMoment() + { + await Assert.That(AwayBarRenderer.Duration(TimeSpan.FromMinutes(-5))).IsEqualTo("a moment"); + } +} diff --git a/tests/SharpMUTerm.Tui.Tests/TerminalFocusWatcherTests.cs b/tests/SharpMUTerm.Tui.Tests/TerminalFocusWatcherTests.cs new file mode 100644 index 0000000..5f3a962 --- /dev/null +++ b/tests/SharpMUTerm.Tui.Tests/TerminalFocusWatcherTests.cs @@ -0,0 +1,211 @@ +using SharpConsoleUI.Drivers; +using SharpMUTerm.Tui; + +namespace SharpMUTerm.Tui.Tests; + +/// +/// The rule that tells the terminal's focus report apart from the reader's Tab key. +/// +/// It is a rule about time and not about the key, because the key is indistinguishable: +/// SharpConsoleUI's parser turns the bare ESC [ I a terminal writes on focus-in into a +/// ConsoleKey.Tab, identical in every field to a Tab the reader pressed. See +/// for why that is the only channel available. +/// +/// +public class TerminalFocusWatcherTests +{ + private static readonly TimeSpan Threshold = TimeSpan.FromSeconds(30); + + [Test] + public async Task ATabWhileTheReaderIsAtTheKeyboardIsAnOrdinaryTab() + { + var (watcher, time, returns) = Watcher(); + + watcher.NoteInput(); + time.Advance(TimeSpan.FromSeconds(2)); + watcher.NoteInput(); // the Tab's own KeyPressed, which the driver raises first + + await Assert.That(watcher.TryTakeAsReturn()).IsFalse(); + await Assert.That(returns).IsEmpty(); + } + + [Test] + public async Task ATabAfterAQuietGapIsTheTerminalReportingFocus() + { + var (watcher, time, returns) = Watcher(); + + watcher.NoteInput(); + time.Advance(TimeSpan.FromMinutes(12)); + watcher.NoteInput(); + + await Assert.That(watcher.TryTakeAsReturn()).IsTrue(); + await Assert.That(returns).Count().IsEqualTo(1); + await Assert.That(returns[0]).IsEqualTo(TimeSpan.FromMinutes(12)); + } + + /// + /// The ordering trap, pinned. The disguised focus-in is a KeyPressed and the driver + /// raises that before InputCoordinator reaches the global shortcuts, so the watcher must + /// measure from the input before the Tab. Measuring from the latest one finds a gap of zero + /// on every return and the feature never fires at all — which is a bug that looks exactly like the + /// terminal not supporting focus reporting. + /// + [Test] + public async Task TheGapIsMeasuredFromTheInputBeforeTheTabsOwn() + { + var (watcher, time, _) = Watcher(); + + watcher.NoteInput(); + time.Advance(TimeSpan.FromMinutes(12)); + watcher.NoteInput(); + + // No time passes between the Tab's KeyPressed and the shortcut running: a watcher reading the + // latest timestamp would see nothing at all here. + await Assert.That(watcher.TryTakeAsReturn()).IsTrue(); + } + + [Test] + public async Task ASecondTabStraightAfterAReturnIsAnOrdinaryTab() + { + var (watcher, time, returns) = Watcher(); + + watcher.NoteInput(); + time.Advance(TimeSpan.FromMinutes(12)); + watcher.NoteInput(); + await Assert.That(watcher.TryTakeAsReturn()).IsTrue(); + + // You are back, and now you press Tab to cycle command bars. The baseline moved with the + // return, so this must reach InputBarControl rather than being eaten as a second focus-in. + time.Advance(TimeSpan.FromSeconds(1)); + watcher.NoteInput(); + await Assert.That(watcher.TryTakeAsReturn()).IsFalse(); + await Assert.That(returns).Count().IsEqualTo(1); + } + + [Test] + public async Task ADisabledWatcherLeavesEveryTabAlone() + { + var (watcher, time, returns) = Watcher(enabled: false); + + watcher.NoteInput(); + time.Advance(TimeSpan.FromHours(3)); + watcher.NoteInput(); + + await Assert.That(watcher.TryTakeAsReturn()).IsFalse(); + await Assert.That(returns).IsEmpty(); + } + + [Test] + public async Task StartAsksTheTerminalToReportFocusAndStopTurnsItOff() + { + var driver = new RecordingConsoleDriver(); + using var watcher = new TerminalFocusWatcher(driver, new ManualTimeProvider(), enabled: true); + + watcher.Start(); + await Assert.That(driver.Written).IsEquivalentTo(new[] { "\x1b[?1004h" }); + + watcher.Stop(); + await Assert.That(driver.Written).IsEquivalentTo(new[] { "\x1b[?1004h", "\x1b[?1004l" }); + } + + [Test] + public async Task StartingTwiceAsksOnce() + { + var driver = new RecordingConsoleDriver(); + using var watcher = new TerminalFocusWatcher(driver, new ManualTimeProvider(), enabled: true); + + watcher.Start(); + watcher.Start(); + + await Assert.That(driver.Written).Count().IsEqualTo(1); + } + + /// + /// A watcher that is off writes nothing at all. This is the same guarantee save, logRoot + /// and restore carry, one layer out: an app that is not the live entry point must not reach + /// the developer's terminal any more than it reaches their configuration. + /// + [Test] + public async Task ADisabledWatcherWritesNothingToTheTerminal() + { + var driver = new RecordingConsoleDriver(); + using var watcher = new TerminalFocusWatcher(driver, new ManualTimeProvider(), enabled: false); + + watcher.Start(); + watcher.Stop(); + + await Assert.That(driver.Written).IsEmpty(); + } + + [Test] + public async Task StopOnAWatcherThatNeverStartedWritesNothing() + { + var driver = new RecordingConsoleDriver(); + var watcher = new TerminalFocusWatcher(driver, new ManualTimeProvider(), enabled: true); + + watcher.Dispose(); + + await Assert.That(driver.Written).IsEmpty(); + } + + /// + /// Headless means no terminal to report focus and no reader to have left one — and, more sharply, + /// a harness pressing Tab must get a Tab. The emission would already be a no-op there + /// (WriteClipboardOsc52 is a default interface method with an empty body); the Tab + /// interpretation would not be. + /// + [Test] + public async Task ShouldEnable_IsFalseForAHeadlessDriver() + { + using var headless = new HeadlessConsoleDriver(80, 24); + + await Assert.That(TerminalFocusWatcher.ShouldEnable(headless)).IsFalse(); + } + + [Test] + public async Task NoteInputAnnouncesEveryInputEvent() + { + var (watcher, _, _) = Watcher(); + var seen = 0; + watcher.Input += () => seen++; + + watcher.NoteInput(); + watcher.NoteInput(); + + await Assert.That(seen).IsEqualTo(2); + } + + private static (TerminalFocusWatcher Watcher, ManualTimeProvider Time, List Returns) Watcher( + bool enabled = true) + { + var time = new ManualTimeProvider(); + var watcher = new TerminalFocusWatcher(new RecordingConsoleDriver(), time, enabled, Threshold); + var returns = new List(); + watcher.Returned += away => returns.Add(away); + return (watcher, time, returns); + } + + /// + /// A headless driver that remembers what was written through the raw-escape seam. It subclasses + /// rather than reimplementing IConsoleDriver from scratch, because that interface is wide and + /// none of the rest of it matters here. + /// + /// It names IConsoleDriver in its base list even though HeadlessConsoleDriver already + /// does, and that is load-bearing rather than noise. WriteClipboardOsc52 is a default + /// interface method that HeadlessConsoleDriver does not override, so the interface mapping is + /// fixed at the base class and a matching public method on a derived type would never be reached + /// through an IConsoleDriver reference — which is exactly how the watcher holds it. Re-listing + /// the interface is what re-runs the mapping. + /// + /// + private sealed class RecordingConsoleDriver : HeadlessConsoleDriver, IConsoleDriver + { + public RecordingConsoleDriver() : base(80, 24) + { + } + + public List Written { get; } = new(); + + public void WriteClipboardOsc52(string sequence) => Written.Add(sequence); + } +} From 575b705166a912cd756f6e83e4eeb23cafeba753 Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Mon, 10 Aug 2026 14:55:22 -0500 Subject: [PATCH 3/6] feat(tui): draw a bar where the reader was when they left the terminal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tabbing away and coming back meant guessing which lines had already been read: every pane bottom-anchors through an absence, so what you land on is the newest output with no boundary in it, and the unread badges are silent about it — NoteActivity only counts a line while a window is *not* caught up, and the window you were looking at stays visible and at its tail the whole time. The boundary is tracked forward, on every input event, because it cannot be found afterwards: a PaneLine's stamp is formatted text, not a time. It is kept one input back, for the reason the watcher's clock is — the focus report is itself a keypress and has already moved the newer of the two by the time anything recognises it as a return. Consumption is not Workspace.IsCaughtUp. A bottom-anchored pane satisfies that the instant you return with two hundred unread lines above the fold. The bar goes when it has been inside the viewport, the pane is at its live tail, and one input has landed since it was drawn. Also fixes SimulateKey discarding a global shortcut's result: it swallowed the key whether or not the handler claimed it, which was invisible while every claim returned true and is not any more. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0195EnLstmb6tAgpbUHjHBEX --- CLAUDE.md | 45 +- src/SharpMUTerm.Tui/SharpMUTermApp.cs | 419 +++++++++++++++++- .../SharpMUTerm.Tui.Tests/AwayDividerTests.cs | 298 +++++++++++++ 3 files changed, 746 insertions(+), 16 deletions(-) create mode 100644 tests/SharpMUTerm.Tui.Tests/AwayDividerTests.cs diff --git a/CLAUDE.md b/CLAUDE.md index c2b0690..28011cc 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -103,6 +103,44 @@ 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 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 + `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` + is there, `?1004` is in no version), `IConsoleDriver` has no focus event, and `UnixStdinReader` + dispatches only key/paste/mouse. So we ask through `IConsoleDriver.WriteClipboardOsc52`, which is named + for its first customer and is really a verbatim raw write under the renderer's own `_consoleLock` — the + only public write serialised against frame painting — funnelled through one `EmitTerminalMode` so a + version that starts validating that payload is one line to fix. And focus-**in** is recognised in the + **Tab keypress** `AnsiInputParser.DispatchCsi` mistranslates it into (`:511` reads a trailing `I` as + Tab, right for `ESC [ 1;5 I` = Ctrl+Tab, wrong for the bare form). Tab is claimed through + `RegisterGlobalShortcut`'s **declining** overload, deliberately *not* through `MacroKeys.AppShortcuts`: + it declines nearly every Tab it sees, and listing it would tell F4's readers a key was gone that is not. + - **Telling that Tab from a real one is a question about time, and the comparison must be against the + input *before* it.** The disguised focus-in is itself a `KeyPressed`, raised before `InputCoordinator` + reaches the global shortcuts, so measuring from the latest timestamp finds a gap of zero on every + return and the feature never fires — a bug indistinguishable from the terminal not supporting `?1004`. + **The same trap bit the boundary**, one field over: that keypress had already moved `_awayPending` to + the end of a buffer full of unseen lines, so `_awayBoundary` keeps the value from the input before it. + `SimulateReturnFromAway` notes an input first for that reason — a seam that skipped it would read a + boundary the shipping path never reads. + - **Focus-out is not recoverable.** `ESC [ O` has no case and is dropped as an `UnknownSequenceEvent`, + so a departure cannot be timestamped; the boundary is the last input event instead, which is seconds + off. **Unix only** — the Windows branch is a `Console.ReadKey` loop with its own reassembly, so + `?1004` must not be enabled there — and inert headless, because a harness pressing Tab must get a Tab. + - **Consumption is three conjuncts, and `Workspace.IsCaughtUp` is not one of them.** A pane + bottom-anchors, so it is already "visible and not scrolled back" the instant you return with two + hundred unread lines above the fold; clearing on it clears the bar before a word is read. It goes when + the bar has been *inside the viewport*, the pane is at its *live tail*, and *one input* has landed + since it was drawn — the last of which is what stops a shallow absence clearing in the frame it + appears in. Insert and remove are mid-buffer, so each costs one `RepaintPane`; affordable for the + timestamp toggle's reason, bounded by a deliberate event rather than by lines or frames. + - The bar is chrome: it never badges unread, never reaches the restore log (already free — that is fed + from the session's line handlers, not the append seam), and a trim that takes it drops the mark with + it. A window that gained nothing gets no bar. + - **`SimulateKey` used to discard a global shortcut's result** and swallow the key either way. Harmless + while every claim returned true; wrong the moment one declined, and it now honours the decline. - **Every server's MSSP report is kept, and the INFO screen reads it** (`MsspCache`, Core; `mssp.json` beside `config.json`, keyed by `host:port`; F5 ▸ `i`). Fourth of the `save:`/`logRoot:`/`restore:` family with **one deliberate difference**: the constructor parameter is null by default like the @@ -191,7 +229,12 @@ python3 tools/ansi_frame_to_image.py frame.ansi frame.html # or .svg a report, a server that answered and publishes none, and a world nothing has dialled; all three reached by driving the real `i` into a real F5, and all three needed because the two empty ones are the pair it is easy to conflate), `web`, - `rail-long`, `scrollback`, `scrollback-up`, `freeze-scrollback`, `prefix-panel` (the ⌃B which-key + `rail-long`, `scrollback`, `scrollback-up`, `freeze-scrollback`, + `away`/`away-scrollback` (the bar marking where the reader was when they tabbed away from the + *terminal* — the shallow absence, where the bar and everything below it are on screen at once, and the + deep one, where more arrived than the pane holds and the reader has scrolled back to find it; the + second is the only frame that can show a bottom-anchored pane being "caught up" while nothing has been + read), `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 765c9b0..5cfb3e0 100644 --- a/src/SharpMUTerm.Tui/SharpMUTermApp.cs +++ b/src/SharpMUTerm.Tui/SharpMUTermApp.cs @@ -317,6 +317,47 @@ private sealed class SizeReport /// Assembles pane drag-and-drop out of the driver's raw mouse frames (see PaneDragTracker). private readonly PaneDragTracker _paneDrag = new(); + /// + /// Notices the reader coming back to the terminal after being away from it, so the panes can be + /// marked where they left. Always constructed and usually inert: see + /// for what it can and cannot see, and the focusReporting constructor parameter for who turns + /// it on. + /// + private readonly TerminalFocusWatcher _focus; + + /// + /// Where each window's newest line was when the reader last did anything, by window id. + /// + /// It is tracked forward, on every input event, rather than found retroactively when the return + /// arrives, because there is nothing in the buffer to find it by: a 's stamp is + /// formatted text and not a time, so the buffer cannot be searched for "the first line newer + /// than this instant". Widening PaneLine to carry an arrival time would touch every append and + /// the restore codec; this costs a dictionary write per keystroke over a handful of entries. + /// + /// + private readonly Dictionary _awayPending = new(StringComparer.Ordinal); + + /// + /// Where each window's newest line was at the input before the last one — which is the + /// boundary an away bar is actually drawn at. + /// + /// The same ordering trap the clock has, one field over. The terminal's focus report arrives + /// disguised as a Tab keypress, so by the time the return is recognised that keypress has already + /// been through and moved to the end of a + /// buffer full of lines the reader never saw. Reading it there finds nothing missed and draws no bar + /// at all. measures its gap from the input before + /// the Tab for exactly this reason; so does this. + /// + /// + private readonly Dictionary _awayBoundary = new(StringComparer.Ordinal); + + /// + /// The away bar each window is currently carrying, by window id. At most one per window: a return + /// while a previous bar is still unread replaces it, because two rows in one pane cannot both be + /// where the reader left. + /// + private readonly Dictionary _awayMarks = new(StringComparer.Ordinal); + /// How the configuration is written back, or null for an app that owns no file. private readonly Action? _save; @@ -414,6 +455,17 @@ private sealed class SizeReport /// needs no "is there a cache" branch, and a snapshot can seed a report through the same writer a /// live session uses without any of it reaching disk. /// + /// + /// Whether to ask the terminal to report focus, so the panes can be marked where the reader was when + /// they tabbed away (see ). Null, the default, decides from the + /// driver — off on Windows, whose input path cannot decode the reports, and off headless, where + /// there is no terminal to have been left and where a harness pressing Tab must get a Tab. + /// + /// It is a parameter rather than only that decision because the interesting half of the feature is + /// what happens to a declined Tab, and a test can only exercise that against a driver the + /// automatic answer says no to. + /// + /// public SharpMUTermApp( AppConfiguration config, TerminalCapabilities capabilities, @@ -423,7 +475,8 @@ public SharpMUTermApp( Action? save = null, string? logRoot = null, RestoreLog? restore = null, - MsspCache? mssp = null) + MsspCache? mssp = null, + bool? focusReporting = null) { _config = config; _save = save; @@ -476,6 +529,17 @@ public SharpMUTermApp( ExitKey: null); _system = new ConsoleWindowSystem(driver ?? new NetConsoleDriver(RenderMode.Buffer), options); + // Built here so the app always has one to ask, and inert unless something turns it on. Nothing + // reaches the terminal until Run() starts it, which is the same shape as the save/logRoot/restore + // family one layer out: an app that is not the live entry point must not write to the developer's + // terminal any more than it writes to their configuration. + _focus = new TerminalFocusWatcher( + _system.ConsoleDriver, + _time, + focusReporting ?? TerminalFocusWatcher.ShouldEnable(_system.ConsoleDriver)); + _focus.Input += NoteReaderInput; + _focus.Returned += MarkWhereTheReaderLeft; + _header = Controls.Markup(HeaderMarkup()).StickyTop().Build(); _header.LinkClicked += (_, e) => OnChromeLinkClicked(e.Url); _header.BackgroundColor = ToColor(_theme.StatusBackground); // the menu bar is a distinct chrome band @@ -627,6 +691,10 @@ public SharpMUTermApp( public int Run(IReadOnlyList startup) { ScheduleStartup(startup ?? Array.Empty()); + + // Here and not in the constructor: this is the one method that means "there is a live terminal + // in front of this app", and asking a terminal to report focus is a write to it. + _focus.Start(); return _system.Run(); } @@ -814,6 +882,33 @@ public string RenderSnapshot(string? view = null) } } + // The bar marking where the reader was when they tabbed away from the terminal. Two views, + // because the two states it has are the two ends of the consumption rule. `away` is a shallow + // absence: the bar and everything below it are on screen at once, which is the frame where the + // bar has to be legible without being loud. `away-scrollback` is a deep one, where more arrived + // than the pane can hold — the bar is above the fold, the reader has gone back to look for it, + // and the frame carries it with the lines it divides on both sides of it *and* the status row's + // scrollback segment. Only the second can show that a bottom-anchored pane being "caught up" + // says nothing about whether the lines have been read, which is the mistake the obvious + // consumption rule makes. + if (string.Equals(view, "away", StringComparison.OrdinalIgnoreCase) || + string.Equals(view, "away-scrollback", StringComparison.OrdinalIgnoreCase)) + { + var deep = string.Equals(view, "away-scrollback", StringComparison.OrdinalIgnoreCase); + SimulateKey(new ConsoleKeyInfo('\0', ConsoleKey.End, false, false, false)); // the reader is here + LoadLongScene(MainWindowId, deep ? 40 : 4); + SimulateReturnFromAway(TimeSpan.FromMinutes(deep ? 143 : 12)); + SettleScroll(); + + if (deep) + { + SimulateKey(new ConsoleKeyInfo('\0', ConsoleKey.PageUp, false, false, false)); + SimulateKey(new ConsoleKeyInfo('\0', ConsoleKey.PageUp, false, false, false)); + } + + 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)) @@ -2085,6 +2180,15 @@ private void AppendWindowLine(string windowId, string markup, string? stamp = nu if (!_lines.TryGetValue(windowId, out var buffer)) { _lines[windowId] = buffer = new List(); + + // A window the reader has never been offered an input event over starts at the beginning: + // everything about to land in it is content they have not seen. Seeded here rather than + // left to default, because "no entry" would otherwise have to mean two different things — + // a window that is new, and a window whose boundary happens to be zero — and the arm that + // read it as "the reader has seen everything" silently drew no bar for any window that + // opened while they were away, which is every spawn window a busy absence creates. + _awayPending[windowId] = 0; + _awayBoundary[windowId] = 0; } buffer.Add(new PaneLine(markup, stamp)); @@ -2101,6 +2205,30 @@ private void AppendWindowLine(string windowId, string markup, string? stamp = nu { _freezePoints[windowId] = Math.Max(0, point - excess); } + + // Everything else that indexes into this buffer moves with it. The boundary is clamped at + // zero — a reader whose position has been trimmed away was, as far as this buffer can now + // say, at the beginning of it. The away bar is not clamped: 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 (_awayPending.TryGetValue(windowId, out var pending)) + { + _awayPending[windowId] = Math.Max(0, pending - excess); + } + + if (_awayBoundary.TryGetValue(windowId, out var boundary)) + { + _awayBoundary[windowId] = Math.Max(0, boundary - excess); + } + + if (_awayMarks.TryGetValue(windowId, out var mark)) + { + mark.Index -= excess; + if (mark.Index < 0) + { + _awayMarks.Remove(windowId); + } + } } if (_panes.TryGetValue(windowId, out var control)) @@ -2224,37 +2352,252 @@ private void SetTimestamps(bool on) /// /// private void RepaintPanes() + { + foreach (var windowId in _lines.Keys) + { + RepaintPane(windowId); + } + } + + /// + /// Re-draws one output pane from its line buffer. The single-window half of + /// , and it carries the same warning: this is the whole-buffer feed, so it + /// belongs only to changes bounded by a deliberate event. Inserting and removing an away bar + /// () is one — it happens when the reader comes back and when they have + /// read what they missed, not per line and not per frame. + /// + private void RepaintPane(string windowId) + { + // Skipped for the reason RepaintPanes gives: the web view's pane is not fed from this buffer at + // all, so re-feeding it would replace the page with whatever last printed into that window. + if (string.Equals(windowId, WebWindowId, StringComparison.Ordinal) + || !_lines.TryGetValue(windowId, out var buffer)) + { + return; + } + + if (_freezePoints.TryGetValue(windowId, out var point)) + { + var split = Math.Clamp(point, 0, buffer.Count); + if (_frozenPanes.TryGetValue(windowId, out var frozen)) + { + FeedRange(frozen, buffer, 0, split); + } + + if (_panes.TryGetValue(windowId, out var tail)) + { + FeedRange(tail, buffer, split, buffer.Count - split); + } + + return; + } + + if (_panes.TryGetValue(windowId, out var control)) + { + FeedRange(control, buffer, 0, buffer.Count); + } + } + + /// + /// An away bar a window is currently carrying: where it sits in the line buffer, and the two + /// observations that between them mean the reader has read past it. + /// + private sealed class AwayMark + { + /// The bar's own index in the window's line buffer. Moves with an insert or a trim. + public int Index; + + /// Whether the bar has been inside the pane's viewport since it was drawn. + public bool Seen; + + /// Whether the reader has done anything at all since it was drawn. + public bool InputSince; + } + + /// + /// Records that the reader did something: moves every window's boundary up to its newest line, and + /// re-checks whether an away bar already on screen has now been read. + /// + /// This is where the boundary comes from. It cannot be found retroactively — see + /// — so it is kept current, and the last value it held before the reader + /// vanished is where they were. + /// + /// + private void NoteReaderInput() { foreach (var (windowId, buffer) in _lines) + { + _awayBoundary[windowId] = _awayPending.GetValueOrDefault(windowId); + _awayPending[windowId] = buffer.Count; + } + + foreach (var mark in _awayMarks.Values) + { + mark.InputSince = true; + } + + ConsumeReadAwayBars(); + } + + /// + /// Draws an away bar in every window that gained lines while the reader was gone. + /// + /// A window that gained nothing gets nothing: there is no boundary to mark, and a bar sitting on the + /// newest line of a quiet pane would be pure furniture. Neither does the web view, whose pane is not + /// fed from the line buffer. + /// + /// + /// The bar is the client's own chrome, so it is appended through the buffer rather than through + /// : it must not badge the window unread, and it must not reach the restore + /// log — which it cannot anyway, because that is fed from the session's own line handlers and + /// deliberately not from the append seam. + /// + /// + private void MarkWhereTheReaderLeft(TimeSpan away) + { + var accent = FrozenAccentHex(); + foreach (var windowId in _lines.Keys.ToArray()) { if (string.Equals(windowId, WebWindowId, StringComparison.Ordinal)) { continue; } - if (_freezePoints.TryGetValue(windowId, out var point)) + // At most one per window, so the previous bar goes first — and it goes first rather than + // last because removing it shifts every index after it, the pending boundary included. + RemoveAwayBar(windowId); + + var buffer = _lines[windowId]; + var at = Math.Clamp(_awayBoundary.GetValueOrDefault(windowId), 0, buffer.Count); + var missed = buffer.Count - at; + if (missed <= 0) { - var split = Math.Clamp(point, 0, buffer.Count); - if (_frozenPanes.TryGetValue(windowId, out var frozen)) - { - FeedRange(frozen, buffer, 0, split); - } + continue; + } - if (_panes.TryGetValue(windowId, out var tail)) - { - FeedRange(tail, buffer, split, buffer.Count - split); - } + buffer.Insert(at, new PaneLine(AwayBarRenderer.Bar(missed, away, accent))); + if (_freezePoints.TryGetValue(windowId, out var freeze) && freeze > at) + { + _freezePoints[windowId] = freeze + 1; + } + + _awayMarks[windowId] = new AwayMark { Index = at }; + RepaintPane(windowId); + } + // The reader is back and this is where they are now, so the next absence measures from here + // rather than from the keystroke before the last one. + foreach (var (windowId, buffer) in _lines) + { + _awayPending[windowId] = _awayBoundary[windowId] = buffer.Count; + } + } + + /// + /// Clears every away bar the reader has now read past, which is three things at once and not one. + /// + /// The obvious test — Workspace.IsCaughtUp — does not survive contact: a pane bottom-anchors, + /// so it is already "visible and not scrolled back" the instant the reader returns, however many + /// hundred lines are above the fold. Clearing on it would clear the bar before a word of them had + /// been read. + /// + /// + /// So: the bar has been inside the viewport (which is what makes a deep absence keep its + /// bar until the reader scrolls up and finds it), the pane is at its live tail, and the + /// reader has done something since it was drawn (which is what stops a shallow absence — + /// a handful of lines, all on screen with the bar — clearing in the very frame it appears). + /// + /// + private void ConsumeReadAwayBars() + { + foreach (var windowId in _awayMarks.Keys.ToArray()) + { + if (!_awayMarks.TryGetValue(windowId, out var mark) + || _paneScrolls.GetValueOrDefault(windowId) is not { } panel) + { continue; } - if (_panes.TryGetValue(windowId, out var control)) + // A frozen window's live control starts at the freeze point, so the bar's row within that + // control is its buffer index less the split. A bar in the *frozen* half is pinned on screen + // above the divider and has been seen by construction. + var origin = _freezePoints.TryGetValue(windowId, out var split) ? Math.Max(0, split) : 0; + var row = mark.Index - origin; + var top = panel.VerticalScrollOffset; + if (row < 0 || (row >= top && row < top + panel.ViewportHeight)) { - FeedRange(control, buffer, 0, buffer.Count); + mark.Seen = true; } + + // 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.Seen && mark.InputSince && panel.AutoScroll) + { + RemoveAwayBar(windowId); + RepaintPane(windowId); + } + } + } + + /// + /// 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. + /// + /// Whether there was a bar to remove. + private bool RemoveAwayBar(string windowId) + { + if (!_awayMarks.TryGetValue(windowId, out var mark) + || !_lines.TryGetValue(windowId, out var buffer) + || mark.Index < 0 + || mark.Index >= buffer.Count) + { + return _awayMarks.Remove(windowId); } + + buffer.RemoveAt(mark.Index); + _awayMarks.Remove(windowId); + + if (_freezePoints.TryGetValue(windowId, out var freeze) && freeze > mark.Index) + { + _freezePoints[windowId] = freeze - 1; + } + + if (_awayPending.TryGetValue(windowId, out var pending) && pending > mark.Index) + { + _awayPending[windowId] = pending - 1; + } + + if (_awayBoundary.TryGetValue(windowId, out var boundary) && boundary > mark.Index) + { + _awayBoundary[windowId] = boundary - 1; + } + + return true; + } + + /// + /// 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 + /// rule that recognises a return is tested against the watcher directly and what the client *does* + /// with one is tested here. + /// + /// It notes an input first because the real thing does: a focus report arrives as a Tab keypress, + /// which has been through before anything recognises it as a return. + /// A seam that skipped that step would read a different boundary from the one that ships, which is + /// the whole failure mode exists to describe. + /// + /// + internal void SimulateReturnFromAway(TimeSpan away) + { + _focus.NoteInput(); + MarkWhereTheReaderLeft(away); } + /// The away bar a window is carrying, by its index in that window's line buffer, or null. + internal int? AwayBarIndex(string windowId) => + _awayMarks.TryGetValue(windowId, out var mark) ? mark.Index : null; + /// /// Appends a line to a window's pane and badges it unread when the reader cannot see where it landed. /// @@ -3498,6 +3841,10 @@ private void SyncScrollbackState(string? windowId = null) RefreshTabTitles(); } + // 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. + ConsumeReadAwayBars(); RefreshStatusRow(); } @@ -3759,6 +4106,36 @@ private void RegisterGlobalShortcuts() $"the {key} settings screen is not claimed in MacroKeys.AppShortcuts"); } } + + RegisterFocusReportTab(); + } + + /// + /// Registers bare Tab, because a terminal reporting that its window regained focus arrives here as + /// one — see for why that is the only channel there is. + /// + /// It is deliberately not in . That table is what F4 reads + /// to tell a user which chords the application has taken, and every entry in it is taken outright. + /// This one is not: it declines all but a vanishing fraction of the Tabs it sees, and a real Tab + /// still reaches 's sibling cycle and the settings screens exactly as it + /// did before. Listing it would tell users a key was gone that is not gone, which is the same class + /// of lie the ⌃Tab claim was — a chord advertised as claimed that could never have matched. + /// + /// + /// Registered only when the watcher is live, so on Windows and headless nothing is claimed at all + /// and the framework's Tab pipeline is untouched. + /// + /// + private void RegisterFocusReportTab() + { + if (!_focus.IsEnabled) + { + return; + } + + Func action = _focus.TryTakeAsReturn; + _system.RegisterGlobalShortcut((ConsoleModifiers)0, ConsoleKey.Tab, action); + _shortcuts[((ConsoleModifiers)0, ConsoleKey.Tab)] = action; } /// @@ -6584,12 +6961,23 @@ private void ConsumePrefixKey(ConsoleKeyInfo key) /// internal string? SimulateKey(ConsoleKeyInfo key) { + // Every key the reader presses, so the pending away marks move with them and a Tab can be told + // from the terminal's focus report by the gap in front of it. In the live client this arrives + // from the driver, which a headless harness never runs. + _focus.NoteInput(); + // The framework runs a global shortcut before the window sees the key at all, so the harness // does too — otherwise a test pressing ⌃B would find it typed into the command line, which is // the opposite of what the running app does with it. - if (_shortcuts.TryGetValue((key.Modifiers, key.Key), out var shortcut)) + // + // A handler that returns false has *declined* the key, and the framework then carries on down + // the normal pipeline (ConsoleWindowSystem.cs:1683). This used to discard the result and swallow + // the key either way, which was invisible while every claim returned true and stopped being so + // the moment one did not: the focus-report Tab declines almost every Tab it sees, and a harness + // that ate them would have every command-bar cycle test passing against a client that no longer + // cycles. + if (_shortcuts.TryGetValue((key.Modifiers, key.Key), out var shortcut) && shortcut()) { - shortcut(); return null; } @@ -8781,6 +9169,7 @@ private static Theme ResolveTheme(AppConfiguration config) public async ValueTask DisposeAsync() { _system.ConsoleDriver.MouseEvent -= OnDriverMouseEvent; + _focus.Dispose(); // and the terminal is told to stop reporting focus at nobody _sizeFlushTimer?.Dispose(); // nothing left to tell a server we are shutting down to _noticeTimer?.Dispose(); // and no row left to put a notice back on _prefixTimer?.Dispose(); // and no window left to float a which-key panel over diff --git a/tests/SharpMUTerm.Tui.Tests/AwayDividerTests.cs b/tests/SharpMUTerm.Tui.Tests/AwayDividerTests.cs new file mode 100644 index 0000000..5ac7d5c --- /dev/null +++ b/tests/SharpMUTerm.Tui.Tests/AwayDividerTests.cs @@ -0,0 +1,298 @@ +using SharpConsoleUI.Drivers; +using SharpMUTerm.Core.Session; +using SharpMUTerm.Graphics; +using SharpMUTerm.Tui; + +namespace SharpMUTerm.Tui.Tests; + +/// +/// The bar marking where the reader was when they left the terminal: where it is drawn, and what makes +/// it go away again. +/// +/// The rule that recognises a return lives in , because +/// it is a rule about time and needs no client around it. These are about what the client does with one. +/// Both halves are exercised here at least once, through the real global-shortcut registration, so the +/// seam between them is not left to inspection. +/// +/// +/// +/// 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 AwayDividerTests +{ + 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); + + [Test] + public async Task AReturnDrawsTheBarWhereTheReaderLeft() + { + var (app, session, _) = Bound(); + session.PrintSystem("*** before you left"); + app.SimulateKey(Key(ConsoleKey.End)); // the reader is here; this is the boundary + + session.PrintSystem("*** while you were away"); + session.PrintSystem("*** and again"); + app.SimulateReturnFromAway(TimeSpan.FromMinutes(12)); + + var index = app.AwayBarIndex(Main); + await Assert.That(index).IsNotNull(); + + var rows = app.PaneLines(Main); + await Assert.That(rows[index!.Value]).Contains(AwayBarRenderer.Label); + await Assert.That(rows[index.Value]).Contains("2 lines"); + await Assert.That(rows[index.Value]).Contains("12 min"); + + // The boundary is where they left, so what they had already read is above it and what they + // missed is below. A bar in the wrong place is worse than no bar. + await Assert.That(rows[index.Value - 1]).Contains("before you left"); + await Assert.That(rows[index.Value + 1]).Contains("while you were away"); + } + + [Test] + public async Task AWindowThatGainedNothingGetsNoBar() + { + var (app, session, _) = Bound(); + session.PrintSystem("*** before you left"); + app.SimulateKey(Key(ConsoleKey.End)); + + app.SimulateReturnFromAway(TimeSpan.FromHours(2)); + + // Nothing arrived, so there is no boundary. A bar on the newest line of a quiet pane would be + // furniture that says only "time passed", which the reader already knew. + await Assert.That(app.AwayBarIndex(Main)).IsNull(); + await Assert.That(app.PaneLines(Main).Any(r => r.Contains(AwayBarRenderer.Label))).IsFalse(); + } + + [Test] + public async Task ASecondReturnReplacesTheFirstBar() + { + var (app, session, _) = Bound(); + app.SimulateKey(Key(ConsoleKey.End)); + session.PrintSystem("*** first absence"); + app.SimulateReturnFromAway(TimeSpan.FromMinutes(5)); + + app.SimulateKey(Key(ConsoleKey.End)); + session.PrintSystem("*** second absence"); + app.SimulateReturnFromAway(TimeSpan.FromMinutes(9)); + + // Two boundaries in one pane cannot both be where the reader left. + var bars = app.PaneLines(Main).Count(r => r.Contains(AwayBarRenderer.Label)); + await Assert.That(bars).IsEqualTo(1); + + var index = app.AwayBarIndex(Main); + await Assert.That(app.PaneLines(Main)[index!.Value]).Contains("9 min"); + } + + /// + /// The third consumption conjunct. A shallow absence puts the bar on screen with everything below + /// it, so "seen" and "at the tail" are both true in the frame it is drawn — and clearing there would + /// remove it before the reader had looked at it. + /// + [Test] + public async Task TheBarSurvivesTheFrameItIsDrawnIn() + { + var (app, session, _) = Bound(); + app.SimulateKey(Key(ConsoleKey.End)); + session.PrintSystem("*** while you were away"); + app.SimulateReturnFromAway(TimeSpan.FromMinutes(12)); + app.RenderWholeFrame(); + + await Assert.That(app.AwayBarIndex(Main)).IsNotNull(); + } + + [Test] + public async Task TheBarGoesOnceItHasBeenSeenAndTheReaderTouchesSomething() + { + var (app, session, _) = Bound(); + app.SimulateKey(Key(ConsoleKey.End)); + session.PrintSystem("*** while you were away"); + app.SimulateReturnFromAway(TimeSpan.FromMinutes(12)); + app.RenderWholeFrame(); + + app.SimulateKey(Key(ConsoleKey.End)); + + await Assert.That(app.AwayBarIndex(Main)).IsNull(); + await Assert.That(app.PaneLines(Main).Any(r => r.Contains(AwayBarRenderer.Label))).IsFalse(); + + // What it marked is still there. The bar goes; the lines it pointed at do not. + await Assert.That(app.PaneLines(Main).Any(r => r.Contains("while you were away"))).IsTrue(); + } + + /// + /// The first consumption conjunct, and the one the obvious rule gets wrong. A bottom-anchored pane + /// is already "caught up" the instant the reader returns, however much is above the fold — so a bar + /// they have not scrolled up to must survive their typing. + /// + [Test] + public async Task ADeepAbsenceKeepsItsBarUntilTheReaderGoesAndLooks() + { + var (app, session, _) = Bound(); + app.SimulateKey(Key(ConsoleKey.End)); + for (var i = 0; i < 200; i++) + { + session.PrintSystem($"*** while you were away {i}"); + } + + app.SimulateReturnFromAway(TimeSpan.FromHours(2)); + app.RenderWholeFrame(); + + app.SimulateKey(Key(ConsoleKey.End)); + await Assert.That(app.AwayBarIndex(Main)).IsNotNull(); + + // Now go and find it, then come back to the bottom: that is the gap being crossed. + for (var i = 0; i < 40; i++) + { + app.SimulateKey(Key(ConsoleKey.PageUp)); + } + + app.RenderWholeFrame(); + app.SimulateKey(Key(ConsoleKey.End, ctrl: true)); + app.RenderWholeFrame(); + app.SimulateKey(Key(ConsoleKey.End)); + + await Assert.That(app.AwayBarIndex(Main)).IsNull(); + } + + /// + /// The bar is the client's own chrome, so it goes into the line buffer directly rather than through + /// the append seam. A reader who was away and is now reading has enough to do without the badge + /// counting the client's own furniture as something else they missed. + /// + [Test] + public async Task TheBarIsNotCountedAsUnread() + { + var (app, session, _) = Bound(); + app.SimulateKey(Key(ConsoleKey.End)); + session.PrintSystem("*** while you were away"); + + var before = app.UnreadOf(Main); + app.SimulateReturnFromAway(TimeSpan.FromMinutes(12)); + + await Assert.That(app.UnreadOf(Main)).IsEqualTo(before); + } + + /// + /// A bar trimmed off the top of the buffer is gone, and the mark has to go with it — a mark left + /// pointing at row zero would have the next removal take a line of the game's output instead. + /// + [Test] + public async Task ABarTrimmedOffTheTopOfTheBufferIsForgotten() + { + var config = Quiet(); + config.ScrollbackLines = 40; + var (app, session, _) = Bound(config); + + app.SimulateKey(Key(ConsoleKey.End)); + session.PrintSystem("*** while you were away"); + app.SimulateReturnFromAway(TimeSpan.FromMinutes(12)); + await Assert.That(app.AwayBarIndex(Main)).IsNotNull(); + + for (var i = 0; i < 80; i++) + { + session.PrintSystem($"*** and life went on {i}"); + } + + // The buffer is the assertion and the control is not: MarkupControl accumulates every line it + // was ever appended and is only pruned by a re-feed, so its text still holds rows the buffer has + // dropped. That is true of the game's own trimmed output as much as of this bar. + await Assert.That(app.AwayBarIndex(Main)).IsNull(); + } + + /// + /// The whole feature, end to end, through the Tab the terminal's focus report actually arrives as — + /// the real global-shortcut registration, not SimulateReturnFromAway. This is the only test + /// that crosses the seam between recognising a return and drawing one. + /// + [Test] + public async Task ATabAfterAQuietGapDrawsTheBar() + { + var (app, session, time) = Bound(focusReporting: true); + app.SimulateKey(Key(ConsoleKey.End)); + session.PrintSystem("*** while you were away"); + + time.Advance(TimeSpan.FromMinutes(12)); + app.SimulateKey(Key(ConsoleKey.Tab, '\t')); + + await Assert.That(app.AwayBarIndex(Main)).IsNotNull(); + } + + /// + /// And the other half, which is the one that has to hold every single day: a Tab pressed by a reader + /// who is sitting right there is a Tab. It must not be consumed, and it must not mark anything. + /// + /// This also pins the harness itself. SimulateKey used to run a global shortcut and discard + /// its result, swallowing the key either way — invisible while every claim returned true, and wrong + /// the moment one declined. + /// + /// + [Test] + public async Task ATabFromAReaderWhoIsSittingThereIsATab() + { + var (app, session, _) = Bound(focusReporting: true); + app.SimulateKey(Key(ConsoleKey.End)); + session.PrintSystem("*** a line"); + + app.SimulateKey(Key(ConsoleKey.Tab, '\t')); + + await Assert.That(app.AwayBarIndex(Main)).IsNull(); + } + + [Test] + public async Task AnAppWithNoFocusReportingClaimsNoTabAtAll() + { + var (app, session, time) = Bound(); + app.SimulateKey(Key(ConsoleKey.End)); + session.PrintSystem("*** while you were away"); + + time.Advance(TimeSpan.FromHours(3)); + app.SimulateKey(Key(ConsoleKey.Tab, '\t')); + + // Headless is not a terminal that can report focus, so nothing here is a return and Tab is + // nobody's but the command line's. + await Assert.That(app.AwayBarIndex(Main)).IsNull(); + } + + private static (SharpMUTermApp App, WorldSession Session, ManualTimeProvider Time) Bound( + bool focusReporting = false) => Bound(Quiet(), focusReporting); + + /// + /// 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 segment files into the developer's own cache + /// directory — which UserDirectoryGuard fails the run for, correctly. + /// + private static Core.Configuration.AppConfiguration Quiet() + { + var config = DemoScene.Build(); + config.ScrollbackSpill.Enabled = false; + return config; + } + + private static (SharpMUTermApp App, WorldSession Session, ManualTimeProvider Time) Bound( + Core.Configuration.AppConfiguration config, + bool focusReporting = false) + { + Console.SetIn(TextReader.Null); + var time = new ManualTimeProvider(); + var app = new SharpMUTermApp( + config, + Headless, + new HeadlessConsoleDriver(Width, Height), + time, + focusReporting: focusReporting); + var session = app.BindWorldWithoutConnecting(config.Worlds[0]); + return (app, session, time); + } + + private static ConsoleKeyInfo Key(ConsoleKey key, char character = '\0', bool ctrl = false) => + new(character, key, false, false, ctrl); + + private static ConsoleKeyInfo Key(ConsoleKey key, bool ctrl) => Key(key, '\0', ctrl); +} From 0e2fad78a24b9d2d76165ac1fef69fa0821935c3 Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Mon, 10 Aug 2026 15:45:02 -0500 Subject: [PATCH 4/6] fix(tui): scroll the pane to the away bar instead of leaving it above the fold MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported as "I saw no change". The mechanism was working — proven against a real kitty: the client emits ?1004h, a DECRQM probe answers ?1004;1 after the alternate-screen switch and every mode set behind it, the terminal writes ESC [ O and ESC [ I, and a 42-second absence was recognised. What was missing was any way to tell. Come back to more lines than the pane holds and the bar is drawn far above the viewport, so nothing on screen changes. Nothing else covers for it either: a window that is visible and at its live tail throughout an absence accrues no unread badge, because NoteActivity counts only what arrives while not caught up. The reader got no signal at all, which is indistinguishable from the terminal not reporting focus. RevealAwayBar scrolls each pane that gained a bar so the bar is at the top of its viewport. A bar already in view is left alone — scrolling a shallow absence would take a pane off its tail to reveal what is already on it. The first cut of that scroll was itself wrong, and the frame is what caught it: a buffer index is not a viewport row, a wrapped line occupies several, and in a narrow pane the scroll landed hundreds of rows adrift in content from a previous session. The tail height is measured through the framework's own MeasureDOM, so it wraps the way the real control will, and only the tail is measured. Consumption drops to two conjuncts. "At the live tail" now means something on its own, because the reveal has taken the pane off its tail whenever the bar was not on screen. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0195EnLstmb6tAgpbUHjHBEX --- CLAUDE.md | 42 +++-- .../specs/2026-08-10-away-divider-design.md | 85 +++++++--- src/SharpMUTerm.Tui/SharpMUTermApp.cs | 157 ++++++++++++++---- .../SharpMUTerm.Tui.Tests/AwayDividerTests.cs | 70 ++++++-- 4 files changed, 276 insertions(+), 78 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 28011cc..5cdcbee 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -129,13 +129,32 @@ fallbacks) for inline images/maps. so a departure cannot be timestamped; the boundary is the last input event instead, which is seconds off. **Unix only** — the Windows branch is a `Console.ReadKey` loop with its own reassembly, so `?1004` must not be enabled there — and inert headless, because a harness pressing Tab must get a Tab. - - **Consumption is three conjuncts, and `Workspace.IsCaughtUp` is not one of them.** A pane - bottom-anchors, so it is already "visible and not scrolled back" the instant you return with two - hundred unread lines above the fold; clearing on it clears the bar before a word is read. It goes when - the bar has been *inside the viewport*, the pane is at its *live tail*, and *one input* has landed - since it was drawn — the last of which is what stops a shallow absence clearing in the frame it - appears in. Insert and remove are mid-buffer, so each costs one `RepaintPane`; affordable for the - timestamp toggle's reason, bounded by a deliberate event rather than by lines or frames. + - **A bar off the fold is scrolled to** (`RevealAwayBar`), and without that the feature is invisible in + the case that matters most — the reported defect. Come back to more lines than the pane holds and the + bar is drawn far above the viewport, so *nothing on screen changes*; nothing else covers for it either, + because a window visible and at its live tail throughout an absence accrues no unread badge. A bar + already in view is left alone: scrolling a shallow absence would take a pane off its tail to reveal + what is already on it. `ScrollVerticalBy` and not `ScrollToTop` — it re-syncs metrics from the arranged + bounds before clamping (so a scroll straight after mutating content is not clamped against a stale + viewport) and detaches `AutoScroll` on the way up, which a jump that left it armed would have undone + on the next repaint. + - **A buffer index is not a viewport row, and conflating them is a bug this has already had.** The + panel's offset counts *display* rows and a buffer line wraps into as many as it needs, so in a narrow + pane scrolling to the index landed hundreds of rows adrift, in content from a previous session. The + height is **measured**, by the framework's own `MarkupControl.MeasureDOM` through a throwaway control + at the pane's `ViewportWidth`, so it wraps the way the real control will — and only the *tail* is + measured, from the bar to the newest line, then subtracted from the panel's authoritative + `TotalContentHeight`. Never re-derive wrapping by counting characters; word breaks, zero-width markup + tags and wide characters all change the answer. + - **Consumption is two conjuncts, and `Workspace.IsCaughtUp` is not one of them.** A pane bottom-anchors, + so it is already "visible and not scrolled back" the instant you return with two hundred unread lines + above the fold; clearing on it clears the bar before a word is read. It goes when the pane is at its + *live tail* and *one input* has landed since it was drawn. What makes the first mean anything is the + reveal: the pane was taken **off** its tail whenever the bar was not on screen, so arriving back at the + bottom is having read down through what you missed rather than never having left. The second is what + stops a shallow absence clearing in the frame it appears in. Insert and remove are mid-buffer, so each + costs one `RepaintPane`; affordable for the timestamp toggle's reason, bounded by a deliberate event + rather than by lines or frames. - The bar is chrome: it never badges unread, never reaches the restore log (already free — that is fed from the session's line handlers, not the append seam), and a trim that takes it drops the mark with it. A window that gained nothing gets no bar. @@ -231,10 +250,11 @@ python3 tools/ansi_frame_to_image.py frame.ansi frame.html # or .svg the pair it is easy to conflate), `web`, `rail-long`, `scrollback`, `scrollback-up`, `freeze-scrollback`, `away`/`away-scrollback` (the bar marking where the reader was when they tabbed away from the - *terminal* — the shallow absence, where the bar and everything below it are on screen at once, and the - deep one, where more arrived than the pane holds and the reader has scrolled back to find it; the - second is the only frame that can show a bottom-anchored pane being "caught up" while nothing has been - read), `prefix-panel` (the ⌃B which-key + *terminal* — the shallow absence, where the bar and everything below it are on screen at once and the + 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 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/docs/superpowers/specs/2026-08-10-away-divider-design.md b/docs/superpowers/specs/2026-08-10-away-divider-design.md index 0959f1d..d405313 100644 --- a/docs/superpowers/specs/2026-08-10-away-divider-design.md +++ b/docs/superpowers/specs/2026-08-10-away-divider-design.md @@ -159,25 +159,51 @@ Two things it must not do: A window with no lines at all gets no bar: there is no boundary to mark. -### 4. Consumption - -The divider is cleared when you have read past it, and "read past it" needs care, because the -obvious test does not survive contact. A bottom-anchored pane is *already* `IsCaughtUp` the instant -you return — that predicate is "visible and not scrolled back" (`Workspace.cs:384`) and says nothing -about how much arrived. Clearing on it would clear the divider before you had read a word of the two -hundred lines above the fold. - -The rule is therefore three conjuncts: - -1. the divider row has been **inside the viewport** — computable per frame from the panel's - `VerticalScrollOffset`, `ViewportHeight` and the row's index; and -2. the pane is at its **live tail**; and -3. at least **one input event** has landed since the bar was drawn. - -Read together: you saw the marker, and you are now at the bottom, so you crossed the gap between -them. (1) is what makes a deep absence keep its divider until you scroll up and find it. (3) is what -stops a shallow absence — a handful of lines, all on screen with the marker — clearing in the very -frame it appears. +### 4. Making sure you can see it + +A bar drawn above the fold is not a feature. Come back to more lines than the pane holds and the +divider is drawn far above the viewport, so **nothing on screen changes** — and nothing else covers +for it, because a window that is visible and at its live tail throughout the absence accrues no +unread badge either (`NoteActivity` counts only what arrives while *not* `IsCaughtUp`). The reader +gets no signal whatsoever, which is indistinguishable from the terminal not reporting focus at all. +This was the reported defect and it is the reason `RevealAwayBar` exists. + +On a return, each pane that gained a bar is scrolled so the bar sits at the top of its viewport and +the first line you have not read is under it. A bar **already in view is left alone**: a shallow +absence has the bar and everything below it on screen at once, and scrolling then would take a pane +off its live tail to reveal what is already on it. + +`ScrollVerticalBy` rather than `ScrollToTop`: it re-syncs its metrics from the arranged bounds before +clamping — which is what makes a scroll immediately after mutating the content land where it was +asked rather than against a stale viewport — and it detaches `AutoScroll` on the way up, where a jump +that left auto-scroll armed would be undone by the very next repaint. + +**A buffer index is not a viewport row.** The panel's offset counts *display* rows, and a buffer line +wraps into as many of them as it needs; in a narrow pane almost every line wraps, so scrolling to the +index lands hundreds of rows adrift. The tail height is therefore **measured**, through the +framework's own `MarkupControl.MeasureDOM` on a throwaway control at the pane's `ViewportWidth`, so it +wraps the way the real control will. Only the tail is measured — from the bar to the newest line, +bounded by what arrived during the absence — and it is subtracted from the panel's authoritative +`TotalContentHeight`, so nothing walks the whole scrollback. Re-deriving wrapping by counting +characters would be a second implementation of the renderer's arithmetic to keep in step, and word +breaks, zero-width markup tags and wide characters all change the answer. + +### 5. Consumption + +The divider is cleared when you have read past it, and "read past it" needs care, because the obvious +test does not survive contact. A bottom-anchored pane is *already* `IsCaughtUp` the instant you +return — that predicate is "visible and not scrolled back" (`Workspace.cs:384`) and says nothing +about how much arrived. + +Two conjuncts: + +1. the pane is at its **live tail**; and +2. at least **one input event** has landed since the bar was drawn. + +(1) means something only because of the reveal above: the pane was taken *off* its tail whenever the +bar was not on screen, so arriving back at the bottom is you having come down through the lines you +missed rather than never having left. (2) is what stops a shallow absence — a handful of lines, all +on screen with the marker — clearing in the very frame it appears. Clearing is a removal from `_lines` and so costs the same single-window re-feed the insertion did. @@ -191,11 +217,22 @@ because two boundaries in one pane cannot both be "where you left". through; a Tab outside it consumes and raises `Returned`; the disguised focus-in's own `KeyPressed` does not move the baseline it is about to be compared against; nothing is emitted off Unix or without a live driver. -- `AwayDividerTests` — insertion at the recorded index; the three consumption conjuncts, each failing - alone; replacement on a second return; no bar for an empty window; the bar is not counted as unread. -- Snapshot views `away` (divider on screen) and `away-scrollback` (divider above the fold, over - `LoadLongScene`). CLAUDE.md is explicit that the three `scroll*` views are the only ones with more - output than a pane holds, and that anything touching the output area needs one. +- `AwayDividerTests` — insertion at the recorded index; both consumption conjuncts; replacement on a + second return; no bar for an empty window; the bar is not counted as unread; and one test that + crosses the whole seam through the real global-shortcut registration rather than the simulation seam. +- **The deep-absence tests assert on the painted frame, not on a scroll offset**, because the offset is + where the reveal's own bug lived: an index-as-row scroll lands hundreds of rows adrift and only the + frame tells the two apart. Both fail without `RevealAwayBar`. +- Snapshot views `away` (shallow: divider on screen, pane left on its live tail) and `away-scrollback` + (deep: the client has scrolled the pane to the divider, over `LoadLongScene`). CLAUDE.md is explicit + that anything touching the output area needs a view with more output than a pane holds. +- **A real terminal, because none of the above can see the signal itself.** The headless harness cannot + produce a focus report — `ShouldEnable` is false for a headless driver by design. Driving a real kitty + proved the rest: the client emits `?1004h`; a DECRQM probe answers `?1004;1` after the alternate-screen + switch and after every mode the framework sets behind it; the terminal writes `ESC [ O` and `ESC [ I`; + and the client recognised a 42-second absence and drew the bar. Note that a test which moves the client + to a separate **OS window** and drives focus with `kitten @ focus-window` proves nothing — programmatic + OS-focus stealing is refused under Wayland, so no focus transition happens. Use a tab or a split. - The whole suite and `dotnet build SharpMUTerm.slnx` warning-free. ## Out of scope diff --git a/src/SharpMUTerm.Tui/SharpMUTermApp.cs b/src/SharpMUTerm.Tui/SharpMUTermApp.cs index 5cfb3e0..8b8f09f 100644 --- a/src/SharpMUTerm.Tui/SharpMUTermApp.cs +++ b/src/SharpMUTerm.Tui/SharpMUTermApp.cs @@ -897,15 +897,8 @@ public string RenderSnapshot(string? view = null) var deep = string.Equals(view, "away-scrollback", StringComparison.OrdinalIgnoreCase); SimulateKey(new ConsoleKeyInfo('\0', ConsoleKey.End, false, false, false)); // the reader is here LoadLongScene(MainWindowId, deep ? 40 : 4); - SimulateReturnFromAway(TimeSpan.FromMinutes(deep ? 143 : 12)); SettleScroll(); - - if (deep) - { - SimulateKey(new ConsoleKeyInfo('\0', ConsoleKey.PageUp, false, false, false)); - SimulateKey(new ConsoleKeyInfo('\0', ConsoleKey.PageUp, false, false, false)); - } - + SimulateReturnFromAway(TimeSpan.FromMinutes(deep ? 143 : 12)); ReArmWholeFrame(); } @@ -2407,9 +2400,6 @@ private sealed class AwayMark /// The bar's own index in the window's line buffer. Moves with an insert or a trim. public int Index; - /// Whether the bar has been inside the pane's viewport since it was drawn. - public bool Seen; - /// Whether the reader has done anything at all since it was drawn. public bool InputSince; } @@ -2481,10 +2471,16 @@ private void MarkWhereTheReaderLeft(TimeSpan away) _freezePoints[windowId] = freeze + 1; } - _awayMarks[windowId] = new AwayMark { Index = at }; + var mark = new AwayMark { Index = at }; + _awayMarks[windowId] = mark; RepaintPane(windowId); + RevealAwayBar(windowId, mark); } + // The scroll above takes panes off their live tail, and the status row's scrollback segment and + // the unread badges are both read off that fact. + SyncScrollbackState(); + // The reader is back and this is where they are now, so the next absence measures from here // rather than from the keystroke before the last one. foreach (var (windowId, buffer) in _lines) @@ -2494,18 +2490,124 @@ private void MarkWhereTheReaderLeft(TimeSpan away) } /// - /// Clears every away bar the reader has now read past, which is three things at once and not one. + /// 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. + /// + /// Without this the feature is invisible in the case it matters most. Come back to more lines + /// than the pane holds and the bar is drawn far above the fold, so nothing on screen changes — and + /// nothing else covers for it either: while the reader is away the window is visible and at its live + /// tail, so NoteActivity counts none of those lines and no unread badge appears. A returning + /// reader got no signal whatsoever, which is indistinguishable from the terminal not reporting focus. + /// This was the reported defect. + /// + /// + /// A bar already in view is left exactly where it is. A shallow absence puts the bar and + /// everything below it on screen at once, and scrolling then would take a pane off its live tail to + /// show something already visible — turning a glance into a gesture the reader has to undo. + /// + /// + /// and not ScrollToTop: it re-syncs its + /// metrics from the arranged bounds before clamping, which is what makes a scroll immediately after + /// mutating the content land where it was asked to rather than against a stale viewport, and it + /// detaches AutoScroll on the way up — a jump that left auto-scroll armed would be undone by + /// the very next repaint. + /// + /// + private void RevealAwayBar(string windowId, AwayMark mark) + { + if (_paneScrolls.GetValueOrDefault(windowId) is not { } panel + || !_lines.TryGetValue(windowId, out var buffer) + || panel.ViewportWidth <= 0 + || panel.ViewportHeight <= 0) + { + return; + } + + // A frozen window's live control starts at the freeze point; a bar above that is in the pinned + // half, which is on screen already and is not this control's to scroll to. + var origin = _freezePoints.TryGetValue(windowId, out var split) ? Math.Max(0, split) : 0; + if (mark.Index < origin) + { + return; + } + + // A buffer index is NOT a viewport row, and conflating them is the defect this method was first + // written with: the panel's offset counts *display* rows, and a buffer line wraps into as many of + // them as it needs. In a narrow pane almost every line wraps, so scrolling to the index landed + // hundreds of rows adrift — far above what the reader missed, in content from a previous session. + // + // The height is therefore measured rather than computed, and measured by the framework's own + // layout so it wraps the way the real control will. Only the *tail* is measured — from the bar to + // the newest line, which is bounded by what arrived during the absence — and it is subtracted + // from the panel's own authoritative total, so nothing walks the whole scrollback. + var tailRows = MeasureRows(buffer, mark.Index, panel.ViewportWidth, _panes.GetValueOrDefault(windowId)); + if (tailRows <= 0) + { + return; + } + + var target = Math.Max(0, panel.TotalContentHeight - tailRows); + var delta = target - panel.VerticalScrollOffset; + if (delta < 0) + { + // Only ever upwards. A shallow absence leaves the bar and everything under it on screen + // already, and scrolling then would take a pane off its live tail to reveal something the + // reader can see — turning a glance into a gesture they have to undo. + panel.ScrollVerticalBy(delta); + } + } + + /// + /// How many display rows a window's buffer occupies from to its end, at + /// cells, as the framework will wrap it. /// - /// The obvious test — Workspace.IsCaughtUp — does not survive contact: a pane bottom-anchors, - /// so it is already "visible and not scrolled back" the instant the reader returns, however many - /// hundred lines are above the fold. Clearing on it would clear the bar before a word of them had - /// been read. + /// Measured through a throwaway and its public MeasureDOM rather + /// than by counting characters here: word wrapping, markup tags that occupy no cells and wide + /// characters all change the answer, and a second implementation of that arithmetic would be a second + /// thing to keep in step with the renderer. The wrap setting is copied off the real pane control for + /// the same reason — measuring with a different one would measure a different control. + /// + /// + private int MeasureRows(List buffer, int from, int width, MarkupControl? like) + { + var start = Math.Clamp(from, 0, buffer.Count); + var markup = new List(buffer.Count - start); + for (var i = start; i < buffer.Count; i++) + { + markup.Add(Compose(buffer[i])); + } + + if (markup.Count == 0) + { + return 0; + } + + var probe = new MarkupControl(markup); + if (like is not null) + { + probe.Wrap = like.Wrap; + probe.Padding = like.Padding; + } + + // Unbounded height, tight width: exactly how a scroll viewport measures the child it will scroll. + return probe.MeasureDOM(new LayoutConstraints(width, width, 0, int.MaxValue)).Height; + } + + /// + /// Clears every away bar the reader has now read past: the pane is back at its live tail, + /// and one input has landed since the bar was drawn. + /// + /// The obvious test — Workspace.IsCaughtUp — does not survive contact, and this is not it. A + /// pane bottom-anchors, so it is "visible and not scrolled back" the instant the reader returns, + /// however many hundred lines are above the fold; clearing on that would clear the bar before a word + /// of them had been read. What makes "at the live tail" mean something here is that + /// has already taken the pane off its tail whenever the bar was + /// not on screen, so getting back to the bottom is the reader having come down through the lines + /// they missed rather than never having left it. /// /// - /// So: the bar has been inside the viewport (which is what makes a deep absence keep its - /// bar until the reader scrolls up and finds it), the pane is at its live tail, and the - /// reader has done something since it was drawn (which is what stops a shallow absence — - /// a handful of lines, all on screen with the bar — clearing in the very frame it appears). + /// The input conjunct is what stops a shallow absence — a handful of lines, all on screen with the + /// bar, pane never moved — clearing in the very frame it appears. /// /// private void ConsumeReadAwayBars() @@ -2518,20 +2620,9 @@ private void ConsumeReadAwayBars() continue; } - // A frozen window's live control starts at the freeze point, so the bar's row within that - // control is its buffer index less the split. A bar in the *frozen* half is pinned on screen - // above the divider and has been seen by construction. - var origin = _freezePoints.TryGetValue(windowId, out var split) ? Math.Max(0, split) : 0; - var row = mark.Index - origin; - var top = panel.VerticalScrollOffset; - if (row < 0 || (row >= top && row < top + panel.ViewportHeight)) - { - mark.Seen = true; - } - // 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.Seen && mark.InputSince && panel.AutoScroll) + if (mark.InputSince && panel.AutoScroll) { RemoveAwayBar(windowId); RepaintPane(windowId); diff --git a/tests/SharpMUTerm.Tui.Tests/AwayDividerTests.cs b/tests/SharpMUTerm.Tui.Tests/AwayDividerTests.cs index 5ac7d5c..4aabde7 100644 --- a/tests/SharpMUTerm.Tui.Tests/AwayDividerTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/AwayDividerTests.cs @@ -127,12 +127,19 @@ public async Task TheBarGoesOnceItHasBeenSeenAndTheReaderTouchesSomething() } /// - /// The first consumption conjunct, and the one the obvious rule gets wrong. A bottom-anchored pane - /// is already "caught up" the instant the reader returns, however much is above the fold — so a bar - /// they have not scrolled up to must survive their typing. + /// The reported defect. Come back to more lines than the pane holds and the bar is drawn far above + /// the fold, so nothing on screen changes — and nothing else covers for it, because a window that is + /// visible and at its live tail throughout an absence accrues no unread badge either. The reader got + /// no signal at all, which is indistinguishable from the terminal not reporting focus. + /// + /// This asserts on the frame and not on the scroll offset, because the offset is where the + /// second bug lived: a buffer index is not a viewport row, and a wrapped line occupies several rows, + /// so scrolling to the index landed hundreds of rows adrift in content from a previous session. Only + /// the painted frame can tell the two apart. + /// /// [Test] - public async Task ADeepAbsenceKeepsItsBarUntilTheReaderGoesAndLooks() + public async Task ADeepAbsenceScrollsThePaneSoTheBarIsOnScreen() { var (app, session, _) = Bound(); app.SimulateKey(Key(ConsoleKey.End)); @@ -141,19 +148,40 @@ public async Task ADeepAbsenceKeepsItsBarUntilTheReaderGoesAndLooks() session.PrintSystem($"*** while you were away {i}"); } - app.SimulateReturnFromAway(TimeSpan.FromHours(2)); + // A live app has painted frames by now, which is what gives the pane an arranged viewport to + // measure against. Without one there is nothing to scroll and nothing to scroll within. app.RenderWholeFrame(); - app.SimulateKey(Key(ConsoleKey.End)); - await Assert.That(app.AwayBarIndex(Main)).IsNotNull(); + app.SimulateReturnFromAway(TimeSpan.FromHours(2)); + var frame = app.RenderWholeFrame(); + + await Assert.That(frame).Contains(AwayBarRenderer.Label); + } - // Now go and find it, then come back to the bottom: that is the gap being crossed. - for (var i = 0; i < 40; i++) + /// + /// And the scroll is what makes "back at the live tail" mean something: the pane has been taken off + /// its tail, so returning to the bottom is the reader having come down through what they missed + /// rather than never having left it. + /// + [Test] + public async Task ADeepAbsenceKeepsItsBarUntilTheReaderReadsDownToTheTail() + { + var (app, session, _) = Bound(); + app.SimulateKey(Key(ConsoleKey.End)); + for (var i = 0; i < 200; i++) { - app.SimulateKey(Key(ConsoleKey.PageUp)); + session.PrintSystem($"*** while you were away {i}"); } app.RenderWholeFrame(); + app.SimulateReturnFromAway(TimeSpan.FromHours(2)); + app.RenderWholeFrame(); + + // Typing where you landed does not clear it — you have read the top of what you missed, not all. + app.SimulateKey(Key(ConsoleKey.End)); + await Assert.That(app.AwayBarIndex(Main)).IsNotNull(); + + // ⌃End is the way back to live output, and arriving there is the gap being crossed. app.SimulateKey(Key(ConsoleKey.End, ctrl: true)); app.RenderWholeFrame(); app.SimulateKey(Key(ConsoleKey.End)); @@ -161,6 +189,28 @@ public async Task ADeepAbsenceKeepsItsBarUntilTheReaderGoesAndLooks() await Assert.That(app.AwayBarIndex(Main)).IsNull(); } + /// + /// A shallow absence must not be scrolled. The bar and everything under it are on screen already, so + /// moving the pane off its live tail would turn a glance into a gesture the reader has to undo. + /// + [Test] + public async Task AShallowAbsenceLeavesThePaneOnItsLiveTail() + { + var (app, session, _) = Bound(); + app.SimulateKey(Key(ConsoleKey.End)); + session.PrintSystem("*** while you were away"); + app.RenderWholeFrame(); + + app.SimulateReturnFromAway(TimeSpan.FromMinutes(12)); + var frame = app.RenderWholeFrame(); + + await Assert.That(frame).Contains(AwayBarRenderer.Label); + + // Still at the tail, so one keystroke is all it takes to be done with it. + app.SimulateKey(Key(ConsoleKey.End)); + await Assert.That(app.AwayBarIndex(Main)).IsNull(); + } + /// /// The bar is the client's own chrome, so it goes into the line buffer directly rather than through /// the append seam. A reader who was away and is now reading has enough to do without the badge From cae5697c9e31a96419ff89c9446c5d26658a97e9 Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Mon, 10 Aug 2026 15:49:26 -0500 Subject: [PATCH 5/6] fix(telnet): ask no server to enable an option it has not offered TelnetSession wrote IAC DO MSSP straight to the transport the moment it connected, so that a server which supports MSSP but never volunteers it would answer. It is legal telnet - RFC 854 has either party initiating, and requires a response even to a refusal - and it cost the auto-login. Refusing an option means consuming its three bytes. A server that implements neither leaves them in its line buffer, where they are prepended to the next line the client sends, and that line is always the login. The server reads \xFF\xFD\x46connect Name password, does not recognise it, redisplays its connect screen and logs nobody in - while the transcript shows the welcome screen twice with no reason for it, because the login line is deliberately never echoed or logged. Only the first line after the request dies, which is why typing the login by hand always worked and the auto-login never did. Measured on a live game rather than reasoned about. With the request the login line was never evaluated; without it the same line reached the game and was answered. IAC DO GMCP and IAC WILL/DONT MSSP reproduce it there; NAWS and TTYPE, options that server implements, do not. The mechanism is deleted rather than left empty - RequestOptions, MsspOption and RequestOptionsAsync all go - so there is no seam to reach for. Nothing is lost upstream: TelnetNegotiationCore would never have sent that DO, because its client-side MSSP answers a server's WILL and initiates nothing, and the bytes were written around the library only to keep IAC from being escaped as data. Negotiation is the library's to conduct. UnsolicitedNegotiationTests pins both halves, and the second one dials a real loopback socket. An injected sessionFactory replaces the exact arm that carried the bug - WorldSession.DefaultSessionFactory - so a test built that way would have agreed with the code while every real connection carried the bytes, which is why nothing here caught it. MSSP now arrives the way its own specification writes the handshake: the server offers IAC WILL MSSP and the library answers DO. Servers that never offer are what the INFO screen's "publishes none" state is for. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0195EnLstmb6tAgpbUHjHBEX --- CLAUDE.md | 38 ++++-- src/SharpMUTerm.Core/Session/WorldSession.cs | 18 ++- src/SharpMUTerm.Core/Telnet/TelnetSession.cs | 45 +------ .../Telnet/LoopbackServer.cs | 83 ++++++++++++ .../Telnet/MsspParsingTests.cs | 13 +- .../Telnet/UnsolicitedNegotiationTests.cs | 123 ++++++++++++++++++ 6 files changed, 253 insertions(+), 67 deletions(-) create mode 100644 tests/SharpMUTerm.Core.Tests/Telnet/LoopbackServer.cs create mode 100644 tests/SharpMUTerm.Core.Tests/Telnet/UnsolicitedNegotiationTests.cs diff --git a/CLAUDE.md b/CLAUDE.md index c2b0690..5fc7ad4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -119,10 +119,23 @@ fallbacks) for inline images/maps. as of…" are three different screens. Report capture is bounded at the door (`MaxVariables`/`MaxValuesPerVariable`/`MaxValueLength`), not only at the renderer — a value only the screen trimmed would still be full size on disk and in memory on every later launch. -- **`IAC DO MSSP` is sent on connect** (`TelnetSessionOptions.RequestOptions`, set by `WorldSession`'s - session factory). The library opens with `IAC WILL NAWS` and nothing else, so a server that supports - MSSP but waits to be asked is never asked — and the INFO screen would then report it as publishing - none, which is a claim about the server made out of our own silence. +- **This client asks no server to enable an option the server has not offered, and that rule was bought + with a login** (`UnsolicitedNegotiationTests`, Core). `TelnetSession` used to write `IAC DO MSSP` + straight to the transport on connect — legal telnet (RFC 854 has either party initiating, and requires + a response even to a refusal), and the way to reach the many servers that support MSSP but never + volunteer it. **Refusing an option means consuming its three bytes, and a server that implements + neither leaves them in its line buffer, where they are prepended to the next line the client sends — + which is always the auto-login.** The server reads `\xFF\xFD\x46connect Name password`, redisplays its + connect screen and logs nobody in; the transcript shows the welcome screen twice, with no reason for it, + because the login line is deliberately not echoed or logged. Measured on a live game: with the request + the login line was never evaluated, without it the same line reached the game, and only the *first* + line after the request dies — which is why typing the login by hand always worked and the auto-login + never did. Two things follow. **We are not in a position to know which servers parse telnet properly**, + and the one that does not is exactly the one whose login we break. And **negotiation is the library's + to conduct**: TelnetNegotiationCore would never have sent that `DO` — its client-side MSSP answers a + server's `WILL` and initiates nothing (`MSSPProtocol.OnWillMSSPAsync`) — so a hand-written negotiation + byte, written around it to avoid `IAC` being escaped as data, is a negotiation nothing keeps state for. + The `RequestOptions` mechanism is deleted rather than left empty, so there is no seam to reach for. - **A launch connects nothing unless it is told to** (`StartupConnections.Resolve`, Core). A host on the command line wins outright; otherwise it is every character with `ConnectAtStartup` (F5's `at start`), in configuration order; otherwise none, and the client says which of the two empty states it is in. @@ -667,13 +680,16 @@ markup (`[bold #rrggbb on #rrggbb]…[/]`, `[[`/`]]` escaping, `[link=url]…[/] `MsspParsingTests` now pins the fixed behaviour by name rather than the bugs. MSSP still has no payload size cap upstream — `SubnegotiationBuffer` guards GMCP, MSDP and CHARSET's TTABLE, but not this — so a hostile server can make a session buffer as much as it likes. -- **MSSP is asked for, not waited for, and the client surfaces it** (`TelnetSessionOptions.RequestOptions` - / `MsspOption`; `MsspCache`; the F5 ▸ `i` INFO screen). The library's opening negotiation is - `IAC WILL NAWS` and nothing else, so MSSP is only ever reached if the server volunteers it — and a - great many servers that fully support MSSP answer `IAC DO MSSP` and volunteer nothing, which is why - the protocol's own reference client asks. `WorldSession`'s session factory therefore sets - `RequestOptions = [MsspOption]`. Do not "simplify" that away: without it the INFO screen is empty - against most of the servers that have the data. +- **MSSP is waited for, never asked for, and the client surfaces what arrives** (`MsspCache`; the F5 ▸ `i` + INFO screen). The MSSP specification writes one handshake and only one: the server "should send + IAC WILL MSSP", the client answers `IAC DO MSSP` or `IAC DONT MSSP`. It says nothing about a client + opening with `DO` — crawlers do that on RFC 854's authority, not MSSP's — and this client used to, which + cost it the auto-login on any server whose telnet parser leaks an unknown option into its command + buffer (see the entry above; the mechanism is gone). The library's opening negotiation is + `IAC WILL NAWS` and nothing else, so MSSP is reached only when the server volunteers it, and the servers + that never do are exactly what the INFO screen's *dialled, publishes none* state is for. **Do not + re-add the ask** — not as an option, not per world: the cost lands on the login, silently, on the users + least able to diagnose it. - **Text encoding is CHARSET's answer, not a setting** (`SessionEncoding`, `TelnetSession.CurrentEncoding`). A world's `encoding` is `auto` by default — state the app's `CharsetOrder`, decode with whatever RFC 2066 settles on — and naming one is an *override*: still offered at the head of the order so a diff --git a/src/SharpMUTerm.Core/Session/WorldSession.cs b/src/SharpMUTerm.Core/Session/WorldSession.cs index 87f7a92..f077be2 100644 --- a/src/SharpMUTerm.Core/Session/WorldSession.cs +++ b/src/SharpMUTerm.Core/Session/WorldSession.cs @@ -655,11 +655,18 @@ private void SetState(ConnectionState state, Exception? error) /// misbehaves, and it now reaches the client's diagnostics pipeline. /// /// - /// MSSP is asked for rather than waited for (). - /// The library opens with IAC WILL NAWS and nothing else, so a server that supports MSSP but - /// waits to be asked — a great many of them — is never asked, and the INFO screen would report it as - /// publishing no MSSP. That is a claim about the server made out of our own silence. Three bytes, - /// once per connection, and a server without the option answers IAC WONT. + /// MSSP is waited for and never asked for, and that is the fix to a bug this factory caused. + /// It used to set RequestOptions so the session wrote IAC DO MSSP the moment it + /// connected, on the reasoning that a server which supports MSSP but waits to be asked is otherwise + /// never asked. The reasoning was sound and the cost was not: refusing an option means + /// consuming its three bytes, and a server that does not implement it and does not consume + /// them prepends them to the next line the client sends — which is always the auto-login. Measured + /// against a live game: with the request, the login line was never evaluated and the connect screen + /// came back instead; without it, the same line reached the game. The client is not in a position to + /// know which servers parse telnet properly, and the one that does not is exactly the one whose + /// login it breaks, silently. MSSP now arrives the way its own specification writes the handshake — + /// the server offers IAC WILL MSSP, the library answers DO — and the INFO screen's + /// "publishes none" covers the servers that never offer. /// /// private ITelnetSession DefaultSessionFactory(ConnectionOptions options) @@ -675,7 +682,6 @@ private ITelnetSession DefaultSessionFactory(ConnectionOptions options) CharsetOrder = order, EncodingOverride = encodingOverride, KeepaliveInterval = TelnetSessionOptions.ResolveKeepalive(World.KeepaliveSeconds), - RequestOptions = [TelnetSessionOptions.MsspOption], }); } diff --git a/src/SharpMUTerm.Core/Telnet/TelnetSession.cs b/src/SharpMUTerm.Core/Telnet/TelnetSession.cs index 2c4fd0c..29d4d08 100644 --- a/src/SharpMUTerm.Core/Telnet/TelnetSession.cs +++ b/src/SharpMUTerm.Core/Telnet/TelnetSession.cs @@ -85,30 +85,6 @@ public sealed class TelnetSessionOptions public static IReadOnlyList SharpMUTermTerminalTypes { get; } = ["SHARPMUTERM", "XTERM", "MTTS 2333"]; - /// - /// Telnet options to request outright on connect, by sending IAC DO <option>, rather - /// than waiting for the server to offer them. - /// - /// This exists because waiting does not work. The MSSP specification says the server "should - /// send IAC WILL MSSP" when a client connects, and TelnetNegotiationCore is built on that reading: - /// its whole opening negotiation is IAC WILL NAWS and nothing else, so MSSP is only ever - /// reached if the server volunteers it. A great many servers that fully support MSSP do not - /// volunteer anything — they answer IAC DO MSSP and are otherwise silent, which is why the - /// protocol's own reference client (TinTin++'s #session mssp) asks rather than listens. A - /// client that only listens simply does not see those servers, and its INFO screen reports them as - /// publishing no MSSP — which is a different and wronger claim than "we never asked". - /// - /// - /// This is negotiation, not traffic: IAC DO is the client half of the option handshake, it - /// costs three bytes once per connection, and a server that does not implement the option answers - /// IAC WONT and is no worse off. - /// - /// - public IReadOnlyList RequestOptions { get; init; } = []; - - /// The MSSP telnet option, 70. - public const byte MsspOption = 70; - /// /// How long the connection may sit silent before a keepalive goes out, or null for none — a /// world's , resolved @@ -411,8 +387,8 @@ public async Task ConnectAsync(CancellationToken cancellationToken = default) _loopCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); _readLoop = Task.Run(() => ReadLoopAsync(_loopCts.Token), CancellationToken.None); - // After the read loop is running, so a server that answers instantly is heard. - await RequestOptionsAsync(cancellationToken).ConfigureAwait(false); + // Nothing else goes out here. The opening negotiation is the interpreter's, and every option + // this client turns on is one the server offered first — see UnsolicitedNegotiationTests. } private Task BuildInterpreterAsync() @@ -472,23 +448,6 @@ private void SeedInterpreterEncoding(TelnetInterpreter interpreter) _seeded = seed; } - /// - /// Sends IAC DO <option> for each of . - /// - /// Written straight to the transport rather than through , because that - /// escapes IAC as data — which is exactly right for a command line and exactly wrong for a - /// negotiation. This is the same door the interpreter's own negotiation output goes through. - /// - /// - private async ValueTask RequestOptionsAsync(CancellationToken cancellationToken) - { - foreach (var option in _options.RequestOptions) - { - _logger.LogDebug("Requesting telnet option {Option}.", option); - await _transport.SendAsync(new byte[] { 255, 253, option }, cancellationToken).ConfigureAwait(false); - } - } - /// /// Tells the terminal-type plugin what to answer with, when this session has an opinion. /// diff --git a/tests/SharpMUTerm.Core.Tests/Telnet/LoopbackServer.cs b/tests/SharpMUTerm.Core.Tests/Telnet/LoopbackServer.cs new file mode 100644 index 0000000..d5c7a98 --- /dev/null +++ b/tests/SharpMUTerm.Core.Tests/Telnet/LoopbackServer.cs @@ -0,0 +1,83 @@ +using System.Net; +using System.Net.Sockets; + +namespace SharpMUTerm.Core.Tests.Telnet; + +/// +/// A socket on loopback that accepts one connection and records every byte written to it, saying +/// nothing back. It exists for the one thing cannot do: let a +/// build its own transport and telnet session, so a +/// test reads what a server would have received rather than what an injected double was handed. +/// +/// Silent on purpose. The server that exposed the unsolicited-DO bug negotiated nothing at all, +/// which is the commonest MU* server there is and the case a scripted greeting cannot stand in for. +/// +/// +internal sealed class LoopbackServer : IDisposable +{ + private readonly TcpListener _listener; + private readonly Lock _gate = new(); + private readonly List _received = []; + private readonly CancellationTokenSource _cts = new(); + + public LoopbackServer() + { + _listener = new TcpListener(IPAddress.Loopback, 0); + _listener.Start(); + Port = ((IPEndPoint)_listener.LocalEndpoint).Port; + _ = Task.Run(AcceptAsync); + } + + /// The ephemeral port the listener was given. + public int Port { get; } + + /// Everything the client has written, in order. + public byte[] Received + { + get + { + lock (_gate) + { + return [.. _received]; + } + } + } + + private async Task AcceptAsync() + { + try + { + using var client = await _listener.AcceptTcpClientAsync(_cts.Token); + var stream = client.GetStream(); + var buffer = new byte[4096]; + while (!_cts.IsCancellationRequested) + { + var read = await stream.ReadAsync(buffer, _cts.Token); + if (read <= 0) + { + return; + } + + lock (_gate) + { + _received.AddRange(buffer.AsSpan(0, read)); + } + } + } + catch (OperationCanceledException) + { + // The test finished first, which is the ordinary way this ends. + } + catch (Exception) + { + // A socket torn down under the reader is likewise the test being over. + } + } + + public void Dispose() + { + _cts.Cancel(); + _listener.Stop(); + _cts.Dispose(); + } +} diff --git a/tests/SharpMUTerm.Core.Tests/Telnet/MsspParsingTests.cs b/tests/SharpMUTerm.Core.Tests/Telnet/MsspParsingTests.cs index fad51ce..d3b646b 100644 --- a/tests/SharpMUTerm.Core.Tests/Telnet/MsspParsingTests.cs +++ b/tests/SharpMUTerm.Core.Tests/Telnet/MsspParsingTests.cs @@ -17,10 +17,11 @@ namespace SharpMUTerm.Core.Tests.Telnet; /// and say why. /// /// -/// The session is built the way a world's is — -/// carrying , so the client asks rather than waits — and -/// the scripted server answers only a client that asked. These are therefore the pins for the INFO -/// screen's supply as much as for the model. +/// The session is built the way a world's is, and the scripted server runs the handshake MSSP's own +/// specification describes: it offers IAC WILL MSSP, and it answers only a client that replied +/// DO. The client never opens with a DO of its own — see +/// for the login that cost. These are therefore the pins for +/// the INFO screen's supply as much as for the model. /// /// public class MsspParsingTests @@ -52,9 +53,7 @@ private static async Task ReadRaw(byte[] payload, bool fragmented = fa private static async Task ReadFrom(ScriptedTransport transport, TimeSpan? patience = null) { var received = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - await using var session = new TelnetSession( - transport, - options: new TelnetSessionOptions { RequestOptions = [TelnetSessionOptions.MsspOption] }); + await using var session = new TelnetSession(transport); session.MsspReceived += (_, e) => received.TrySetResult(e.Data); await session.ConnectAsync(); diff --git a/tests/SharpMUTerm.Core.Tests/Telnet/UnsolicitedNegotiationTests.cs b/tests/SharpMUTerm.Core.Tests/Telnet/UnsolicitedNegotiationTests.cs new file mode 100644 index 0000000..451d52c --- /dev/null +++ b/tests/SharpMUTerm.Core.Tests/Telnet/UnsolicitedNegotiationTests.cs @@ -0,0 +1,123 @@ +using System.Text; +using Microsoft.Extensions.Logging.Abstractions; +using SharpMUTerm.Core.Configuration; +using SharpMUTerm.Core.Session; +using SharpMUTerm.Core.Telnet; + +namespace SharpMUTerm.Core.Tests.Telnet; + +/// +/// The client asks no server to enable an option the server has not offered. +/// +/// This is the shape of a real failure, and the damage was invisible. The session used to write +/// IAC DO MSSP to the transport the moment it connected, ahead of everything, so that a server +/// which supports MSSP but waits to be asked would answer. It is legal telnet — RFC 854 has either +/// party initiating, and requires a response even to a refusal — but a server that does not implement +/// the option has to consume those three bytes to refuse them, and one that does not leaves +/// them in its line buffer, where they are prepended to the next line the client sends. That line is +/// always the auto-login. The server sees \xFF\xFD\x46connect Name password, does not recognise +/// it, redisplays its connect screen, and the login silently never happens — while the transcript shows +/// a welcome screen twice and no reason for it, because the login line is not echoed or logged. +/// Measured against a live server: with the request, the login line was never evaluated; without it, +/// the same line reached the game. +/// +/// +/// The narrower lesson is where the bytes went. wrote them straight to the +/// transport, around TelnetNegotiationCore, because an option request must not be IAC-escaped as data. +/// The library would never have sent that DO on its own: its client-side MSSP answers a server's +/// WILL and initiates nothing. Negotiation is the library's to conduct, and a hand-written +/// negotiation byte is a negotiation nothing is keeping state for. +/// +/// +public class UnsolicitedNegotiationTests +{ + private const byte Iac = 255; + private const byte Do = 253; + + /// + /// A server that offers nothing at all — the case that broke, and the commonest MU* server there + /// is. Nothing the client writes may ask it to turn anything on. + /// + [Test] + public async Task ConnectingToASilentServerRequestsNoOption() + { + var transport = new ScriptedTransport(); + await using var session = new TelnetSession(transport, NullLogger.Instance); + await session.ConnectAsync(); + await Task.Delay(100); + + await Assert.That(Requests(transport.Sent)).IsEmpty() + .Because("a DO the peer has to consume in order to refuse is a DO a broken peer feeds to its parser"); + } + + /// + /// And at the seam it cost, over a real socket. + /// + /// An injected session factory cannot pin this and it is worth saying why. The request was + /// configured in WorldSession.DefaultSessionFactory — the arm every world uses and the one a + /// test that passes its own sessionFactory replaces wholesale. Such a test would have agreed + /// with the code while every real connection carried the bytes, which is the exact reason this + /// shipped. So this one takes a loopback listener and lets the session dial it: real + /// , real factory, and the assertion is on the bytes a + /// server actually received before the login line. + /// + /// + [Test] + public async Task NoOptionRequestPrecedesTheLoginLine() + { + using var server = new LoopbackServer(); + var world = new WorldDefinition { Name = "Convergence MUSH", Host = "127.0.0.1", Port = server.Port }; + var character = new CharacterDefinition { Name = "Mannaz", Password = "hunter2" }; + world.Characters.Add(character); + + await using var session = new WorldSession(world, character); + await session.ConnectAsync(); + + const string login = "connect Mannaz hunter2"; + var expected = Encoding.ASCII.GetBytes(login); + for (var i = 0; i < 50 && IndexOf(server.Received, expected) < 0; i++) + { + await Task.Delay(20); + } + + var wire = server.Received; + var loginAt = IndexOf(wire, expected); + await Assert.That(loginAt).IsGreaterThanOrEqualTo(0).Because("the login line has to have been sent at all"); + await Assert.That(Requests(wire[..loginAt])).IsEmpty() + .Because("three unconsumed bytes in front of the login line are three bytes the server reads as part of it"); + } + + /// Every option the client asked the peer to enable, in order. + private static IReadOnlyList Requests(byte[] wire) + { + var options = new List(); + for (var i = 0; i + 2 < wire.Length; i++) + { + if (wire[i] == Iac && wire[i + 1] == Do) + { + options.Add(wire[i + 2]); + } + } + + return options; + } + + private static int IndexOf(byte[] haystack, byte[] needle) + { + for (var i = 0; i + needle.Length <= haystack.Length; i++) + { + var match = true; + for (var j = 0; j < needle.Length && match; j++) + { + match = haystack[i + j] == needle[j]; + } + + if (match) + { + return i; + } + } + + return -1; + } +} From b0b675fa91b56258ee414ec0a41e3a450030e9ad Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Mon, 10 Aug 2026 16:41:44 -0500 Subject: [PATCH 6/6] fix(tui): keep the away bar's state on the UI thread, and take a removed bar off the screen Two defects from review, both in the away divider. TerminalFocusWatcher subscribes to the driver's KeyPressed, Paste and MouseEvent directly, and the driver raises those from the reader task it starts - not the UI thread. The framework's own key path does not run there: its driver handler only enqueues, and InputCoordinator drains on the main loop. So every other key handler in this client is on the UI thread and this one was not, while its handler iterated the pane buffers and mutated the away boundaries, the marks and the controls they repaint. A window opening during that foreach throws. The subscription cannot move: a queued key arrives too late to measure the gap in front of a focus-in Tab, which is the whole rule. NoteInput therefore keeps its timestamps on the reader thread and both handlers go through OnUiThread - which runs inline when it already is one, so headless and the harness are unchanged. Marshalling reorders, and that has a consequence worth naming. An input raised before the return that drew a bar can be delivered after it, and would then set InputSince on a bar nobody has looked at - retiring it to the very keystroke that produced it, which is precisely what the third consumption conjunct exists to prevent. Input now carries a count and AwayMark records the count it was drawn after, so a late note is recognisable as an early input. The second: MarkWhereTheReaderLeft removes the previous bar first (it has to, removal shifts every index after it) and then returns early for a window that gained nothing, without the repaint that the insert branch does. RemoveAwayBar deliberately does not repaint, on the understanding that its caller either follows with one or inserts over it - and this arm did neither, so the row left the buffer and stayed on the control, with no mark left to consume it. Reachable without touching the keyboard: a deep absence, then a second return to a quiet window. Tests: the ghost bar has one of its own. Two more stopped being able to pass for the wrong reason - TheBarIsNotCountedAsUnread never asserted a bar was drawn, and neither Tab test could tell a Tab that travelled on from one that was swallowed and drew nothing, which they now do by raising the second command line and watching the Tab cycle it. Frame assertions go through FrameGrid rather than searching the escape stream, and the enable/disable writes are asserted in order, since IsEquivalentTo ignores it. Docs: the design spec said nothing was implemented; the stale "three scroll* views" claim already missed freeze-scrollback and now misses away-scrollback; two sentences were broken. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0195EnLstmb6tAgpbUHjHBEX --- CLAUDE.md | 26 +++++-- .../specs/2026-08-10-away-divider-design.md | 6 +- src/SharpMUTerm.Core/Telnet/TelnetSession.cs | 5 +- src/SharpMUTerm.Tui/SharpMUTermApp.cs | 28 ++++++-- src/SharpMUTerm.Tui/TerminalFocusWatcher.cs | 16 ++++- .../SharpMUTerm.Tui.Tests/AwayDividerTests.cs | 69 +++++++++++++++++-- .../TerminalFocusWatcherTests.cs | 19 +++-- 7 files changed, 139 insertions(+), 30 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index c35edef..0d66e25 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -112,11 +112,22 @@ fallbacks) for inline images/maps. dispatches only key/paste/mouse. So we ask through `IConsoleDriver.WriteClipboardOsc52`, which is named for its first customer and is really a verbatim raw write under the renderer's own `_consoleLock` — the only public write serialised against frame painting — funnelled through one `EmitTerminalMode` so a - version that starts validating that payload is one line to fix. And focus-**in** is recognised in the - **Tab keypress** `AnsiInputParser.DispatchCsi` mistranslates it into (`:511` reads a trailing `I` as - Tab, right for `ESC [ 1;5 I` = Ctrl+Tab, wrong for the bare form). Tab is claimed through + version that starts validating that payload is one line to fix. And focus-**in** is recognised as the + **bare Tab keypress that `AnsiInputParser.DispatchCsi` mistranslates it into** — `:511` reads a + trailing `I` as Tab, which is right for `ESC [ 1;5 I` = Ctrl+Tab and wrong for the bare form. Tab is + claimed through `RegisterGlobalShortcut`'s **declining** overload, deliberately *not* through `MacroKeys.AppShortcuts`: it declines nearly every Tab it sees, and listing it would tell F4's readers a key was gone that is not. + - **The watcher listens to the driver, so its `Input` runs on the driver's reader thread — and the app + marshals.** The framework's own key path does not run there (its driver handler only enqueues, + `ConsoleWindowSystem.cs:970-973`, and `InputCoordinator.ProcessInput` drains on the main loop), so + every other key handler here is on the UI thread and this one cannot be: a queued key arrives too + late to measure the gap in front of a focus-in Tab. `NoteInput` keeps its timestamps on the reader + thread; both subscriptions go through `OnUiThread`, because everything past them — pane buffers, + away marks, the controls they repaint — is the UI thread's. **Marshalling reorders, so the input is + numbered** (`InputCount`, carried by `Input`; `AwayMark.DrawnAfter` is the stamp): a note raised + before the return that drew a bar can be delivered after it, and would otherwise set `InputSince` on + a bar nobody has seen, retiring it to the keystroke that produced it. - **Telling that Tab from a real one is a question about time, and the comparison must be against the input *before* it.** The disguised focus-in is itself a `KeyPressed`, raised before `InputCoordinator` reaches the global shortcuts, so measuring from the latest timestamp finds a gap of zero on every @@ -304,9 +315,12 @@ python3 tools/ansi_frame_to_image.py frame.ansi frame.html # or .svg `LogFolder` or a fake sink would have passed all along. It also let a pile of per-test `character.Logging = new LoggingSettings()` workarounds be deleted — with them gone the suite exercises the gate, and an unfixed build leaks seven files a run instead of three. -- **The three `scroll*` views are the only ones with more output than a pane holds.** Every other view - fits, which is exactly why no snapshot caught the panes being unable to scroll at all. Reach for one - of these (or `LoadLongScene`) whenever a change touches the output area. +- **Four views have more output than a pane holds** — `scrollback`, `scrollback-up`, + `freeze-scrollback` and `away-scrollback`. Name them, rather than saying "the `scroll*` views": the + prefix has now twice lagged behind the set it claimed to describe, and a reader looking for a + long-output view goes by the list. (`away` is the shallow one and fits.) Every other view fits too, + which is exactly why no snapshot caught the panes being unable to scroll at all. Reach for one of + these (or `LoadLongScene`) whenever a change touches the output area. - **Send the user the `.svg`.** For your *own* inspection render the `.html` — Chromium clips the bottom of a bare `.svg` through aspect-ratio scaling, which will make you chase a layout bug that isn't there. diff --git a/docs/superpowers/specs/2026-08-10-away-divider-design.md b/docs/superpowers/specs/2026-08-10-away-divider-design.md index d405313..d10cabd 100644 --- a/docs/superpowers/specs/2026-08-10-away-divider-design.md +++ b/docs/superpowers/specs/2026-08-10-away-divider-design.md @@ -1,13 +1,13 @@ # The away divider: where you were when you left the terminal **Date:** 2026-08-10 -**Status:** proposed — design only, nothing implemented +**Status:** implemented — `TerminalFocusWatcher`, `AwayBarRenderer`, and the away-marker lifecycle in `SharpMUTermApp` ## Problem Tab away from the terminal, come back later, and there is no way to tell which of the lines on -screen you have already read. Every pane has bottom-anchored through your absence, so what you land -on is the newest output with no boundary in it. The client already knows how to say "the rows above +screen you have already read. Every pane stayed bottom-anchored through your absence, so what you +land on is the newest output with no boundary in it. The client already knows how to say "the rows above this are not live" twice over — `FreezeBarRenderer` divides pinned scrollback from the live tail, `RestoreBarRenderer` closes off content carried over from a previous run — and has nothing to say about the one absence that happens many times a day. diff --git a/src/SharpMUTerm.Core/Telnet/TelnetSession.cs b/src/SharpMUTerm.Core/Telnet/TelnetSession.cs index 29d4d08..a9ddd57 100644 --- a/src/SharpMUTerm.Core/Telnet/TelnetSession.cs +++ b/src/SharpMUTerm.Core/Telnet/TelnetSession.cs @@ -387,8 +387,9 @@ public async Task ConnectAsync(CancellationToken cancellationToken = default) _loopCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); _readLoop = Task.Run(() => ReadLoopAsync(_loopCts.Token), CancellationToken.None); - // Nothing else goes out here. The opening negotiation is the interpreter's, and every option - // this client turns on is one the server offered first — see UnsolicitedNegotiationTests. + // Nothing else goes out from here. The interpreter's own opening WILL NAWS offers an option of + // ours; what this client never sends is an IAC DO the server has not offered — see + // UnsolicitedNegotiationTests for the login that paid for the rule. } private Task BuildInterpreterAsync() diff --git a/src/SharpMUTerm.Tui/SharpMUTermApp.cs b/src/SharpMUTerm.Tui/SharpMUTermApp.cs index 8b8f09f..e41e65e 100644 --- a/src/SharpMUTerm.Tui/SharpMUTermApp.cs +++ b/src/SharpMUTerm.Tui/SharpMUTermApp.cs @@ -537,8 +537,10 @@ public SharpMUTermApp( _system.ConsoleDriver, _time, focusReporting ?? TerminalFocusWatcher.ShouldEnable(_system.ConsoleDriver)); - _focus.Input += NoteReaderInput; - _focus.Returned += MarkWhereTheReaderLeft; + // The watcher listens to the driver, which raises input on its own reader thread; everything past + // these two handlers is UI-thread state. OnUiThread runs inline when it already is one. + _focus.Input += at => OnUiThread(() => NoteReaderInput(at)); + _focus.Returned += away => OnUiThread(() => MarkWhereTheReaderLeft(away)); _header = Controls.Markup(HeaderMarkup()).StickyTop().Build(); _header.LinkClicked += (_, e) => OnChromeLinkClicked(e.Url); @@ -2402,6 +2404,13 @@ private sealed class AwayMark /// Whether the reader has done anything at all since it was drawn. public bool InputSince; + + /// + /// The watcher's input count when this bar was drawn. Both paths are marshalled, so an input + /// raised before the return can be delivered after it; without the stamp that late note + /// would set and retire the bar to the keystroke that produced it. + /// + public long DrawnAfter; } /// @@ -2413,7 +2422,7 @@ private sealed class AwayMark /// vanished is where they were. /// /// - private void NoteReaderInput() + private void NoteReaderInput(long at) { foreach (var (windowId, buffer) in _lines) { @@ -2421,7 +2430,7 @@ private void NoteReaderInput() _awayPending[windowId] = buffer.Count; } - foreach (var mark in _awayMarks.Values) + foreach (var mark in _awayMarks.Values.Where(mark => mark.DrawnAfter < at)) { mark.InputSince = true; } @@ -2455,13 +2464,20 @@ private void MarkWhereTheReaderLeft(TimeSpan away) // At most one per window, so the previous bar goes first — and it goes first rather than // last because removing it shifts every index after it, the pending boundary included. - RemoveAwayBar(windowId); + var removed = RemoveAwayBar(windowId); var buffer = _lines[windowId]; var at = Math.Clamp(_awayBoundary.GetValueOrDefault(windowId), 0, buffer.Count); var missed = buffer.Count - at; if (missed <= 0) { + // No replacement is going in, and RemoveAwayBar leaves the repaint to its caller — so + // without this the row leaves the buffer and stays on the control. + if (removed) + { + RepaintPane(windowId); + } + continue; } @@ -2471,7 +2487,7 @@ private void MarkWhereTheReaderLeft(TimeSpan away) _freezePoints[windowId] = freeze + 1; } - var mark = new AwayMark { Index = at }; + var mark = new AwayMark { Index = at, DrawnAfter = _focus.InputCount }; _awayMarks[windowId] = mark; RepaintPane(windowId); RevealAwayBar(windowId, mark); diff --git a/src/SharpMUTerm.Tui/TerminalFocusWatcher.cs b/src/SharpMUTerm.Tui/TerminalFocusWatcher.cs index 659e4b3..8f0a793 100644 --- a/src/SharpMUTerm.Tui/TerminalFocusWatcher.cs +++ b/src/SharpMUTerm.Tui/TerminalFocusWatcher.cs @@ -65,6 +65,7 @@ internal sealed class TerminalFocusWatcher : IDisposable private DateTimeOffset _lastInputAt; private DateTimeOffset _previousInputAt; + private long _inputs; private bool _started; private bool _disposed; @@ -93,8 +94,19 @@ public TerminalFocusWatcher( /// routed to an overlay rather than to the workspace. It is what the client hangs "where was the /// reader last looking" off, and it is here rather than on the app's own key handler because that /// handler does not see a key an overlay consumed. + /// + /// It runs on the driver's own input thread, and does not. This watcher + /// subscribes to the driver's events directly — it has to, because the framework's own key path only + /// enqueues there and would arrive too late to measure the gap in front of a focus-in Tab — so a + /// subscriber touching anything the UI thread owns must marshal. The argument counts the inputs seen, + /// which is what lets a subscriber that defers its work order it against a in + /// between. + /// /// - public event Action? Input; + public event Action? Input; + + /// How many inputs this watcher has seen. Monotonic, and the value carries. + public long InputCount => Interlocked.Read(ref _inputs); /// /// Whether this watcher will do anything at all. False leaves the terminal untouched and leaves @@ -192,7 +204,7 @@ public void NoteInput() { _previousInputAt = _lastInputAt; _lastInputAt = _time.GetUtcNow(); - Input?.Invoke(); + Input?.Invoke(Interlocked.Increment(ref _inputs)); } public void Dispose() diff --git a/tests/SharpMUTerm.Tui.Tests/AwayDividerTests.cs b/tests/SharpMUTerm.Tui.Tests/AwayDividerTests.cs index 4aabde7..8126b89 100644 --- a/tests/SharpMUTerm.Tui.Tests/AwayDividerTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/AwayDividerTests.cs @@ -153,9 +153,8 @@ public async Task ADeepAbsenceScrollsThePaneSoTheBarIsOnScreen() app.RenderWholeFrame(); app.SimulateReturnFromAway(TimeSpan.FromHours(2)); - var frame = app.RenderWholeFrame(); - await Assert.That(frame).Contains(AwayBarRenderer.Label); + await Assert.That(Painted(app.RenderWholeFrame())).IsTrue(); } /// @@ -202,15 +201,45 @@ public async Task AShallowAbsenceLeavesThePaneOnItsLiveTail() app.RenderWholeFrame(); app.SimulateReturnFromAway(TimeSpan.FromMinutes(12)); - var frame = app.RenderWholeFrame(); - await Assert.That(frame).Contains(AwayBarRenderer.Label); + await Assert.That(Painted(app.RenderWholeFrame())).IsTrue(); // Still at the tail, so one keystroke is all it takes to be done with it. app.SimulateKey(Key(ConsoleKey.End)); await Assert.That(app.AwayBarIndex(Main)).IsNull(); } + /// + /// A second return to a window that gained nothing takes the old bar away, and it has to leave the + /// screen with it: the removal and the repaint were separated by an early exit, so the row left the + /// buffer and stayed on the control with no mark left to consume it. + /// + /// Asserted on the painted frame, because the buffer was always right. The absence has to be a deep + /// one — a shallow one is at the live tail, where ConsumeReadAwayBars removes and repaints + /// before this path is reached. + /// + /// + [Test] + public async Task ASecondReturnToAQuietWindowTakesTheOldBarOffTheScreen() + { + var (app, session, _) = Bound(); + app.SimulateKey(Key(ConsoleKey.End)); + for (var i = 0; i < 200; i++) + { + session.PrintSystem($"*** while you were away {i}"); + } + + app.RenderWholeFrame(); + app.SimulateReturnFromAway(TimeSpan.FromHours(2)); + await Assert.That(Painted(app.RenderWholeFrame())).IsTrue().Because("the first absence draws one"); + + // Away again with nothing arriving: the same bar asked to go, not one being replaced. + app.SimulateReturnFromAway(TimeSpan.FromHours(2)); + + await Assert.That(app.AwayBarIndex(Main)).IsNull(); + await Assert.That(Painted(app.RenderWholeFrame())).IsFalse(); + } + /// /// The bar is the client's own chrome, so it goes into the line buffer directly rather than through /// the append seam. A reader who was away and is now reading has enough to do without the badge @@ -226,6 +255,10 @@ public async Task TheBarIsNotCountedAsUnread() var before = app.UnreadOf(Main); app.SimulateReturnFromAway(TimeSpan.FromMinutes(12)); + // Without this the test passes against a client that stopped drawing bars at all — and at the + // live tail both counts are zero, so the equality alone says little. + await Assert.That(app.AwayBarIndex(Main)).IsNotNull(); + await Assert.That(before).IsEqualTo(0); await Assert.That(app.UnreadOf(Main)).IsEqualTo(before); } @@ -283,15 +316,23 @@ public async Task ATabAfterAQuietGapDrawsTheBar() /// the moment one declined. /// /// + /// + /// Both raise the second command line first, so the Tab has something to do: the no-bar + /// assertion alone holds just as well for a Tab that was swallowed and drew nothing, which is the + /// failure these exist to exclude. With a sibling bar up, an arriving Tab cycles the armed one. + /// [Test] public async Task ATabFromAReaderWhoIsSittingThereIsATab() { var (app, session, _) = Bound(focusReporting: true); + ToggleSecondBar(app); app.SimulateKey(Key(ConsoleKey.End)); session.PrintSystem("*** a line"); + await Assert.That(app.SecondBarArmed).IsTrue().Because("raising the second bar arms it"); app.SimulateKey(Key(ConsoleKey.Tab, '\t')); + await Assert.That(app.SecondBarArmed).IsFalse().Because("a real Tab reaches the bar's sibling cycle"); await Assert.That(app.AwayBarIndex(Main)).IsNull(); } @@ -299,14 +340,16 @@ public async Task ATabFromAReaderWhoIsSittingThereIsATab() public async Task AnAppWithNoFocusReportingClaimsNoTabAtAll() { var (app, session, time) = Bound(); + ToggleSecondBar(app); app.SimulateKey(Key(ConsoleKey.End)); session.PrintSystem("*** while you were away"); time.Advance(TimeSpan.FromHours(3)); + await Assert.That(app.SecondBarArmed).IsTrue(); app.SimulateKey(Key(ConsoleKey.Tab, '\t')); - // Headless is not a terminal that can report focus, so nothing here is a return and Tab is - // nobody's but the command line's. + // The gap in front of this Tab is three hours, so a live watcher would certainly have taken it. + await Assert.That(app.SecondBarArmed).IsFalse(); await Assert.That(app.AwayBarIndex(Main)).IsNull(); } @@ -341,6 +384,20 @@ private static (SharpMUTermApp App, WorldSession Session, ManualTimeProvider Tim return (app, session, time); } + /// + /// Whether the away bar is on the painted frame, read off the decoded cells. A frame is + /// cursor-addressed SGR, so a substring search can miss a label split by a cursor move or a style run. + /// + private static bool Painted(string frame) => + FrameGrid.Decode(frame, Width, Height).Any(row => row.Contains(AwayBarRenderer.Label, StringComparison.Ordinal)); + + /// Raises the second command line the way ⌃B i does, which also arms it. + private static void ToggleSecondBar(SharpMUTermApp app) + { + app.SimulateKey(Key(ConsoleKey.B, ctrl: true)); + app.SimulateKey(new ConsoleKeyInfo('i', ConsoleKey.I, false, false, false)); + } + private static ConsoleKeyInfo Key(ConsoleKey key, char character = '\0', bool ctrl = false) => new(character, key, false, false, ctrl); diff --git a/tests/SharpMUTerm.Tui.Tests/TerminalFocusWatcherTests.cs b/tests/SharpMUTerm.Tui.Tests/TerminalFocusWatcherTests.cs index 5f3a962..0cef253 100644 --- a/tests/SharpMUTerm.Tui.Tests/TerminalFocusWatcherTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/TerminalFocusWatcherTests.cs @@ -104,8 +104,12 @@ public async Task StartAsksTheTerminalToReportFocusAndStopTurnsItOff() watcher.Start(); await Assert.That(driver.Written).IsEquivalentTo(new[] { "\x1b[?1004h" }); + // Ordered: IsEquivalentTo ignores order by default, so a watcher that disabled reporting before + // enabling it would leave the terminal reporting focus for ever and still pass. watcher.Stop(); - await Assert.That(driver.Written).IsEquivalentTo(new[] { "\x1b[?1004h", "\x1b[?1004l" }); + await Assert.That(driver.Written).IsEquivalentTo( + new[] { "\x1b[?1004h", "\x1b[?1004l" }, + TUnit.Assertions.Enums.CollectionOrdering.Matching); } [Test] @@ -162,17 +166,22 @@ public async Task ShouldEnable_IsFalseForAHeadlessDriver() await Assert.That(TerminalFocusWatcher.ShouldEnable(headless)).IsFalse(); } + /// + /// Every input is announced, with a count that only goes up — which is what lets a subscriber that + /// defers its work (the app does; these are raised on the driver's thread) order it against a return. + /// [Test] - public async Task NoteInputAnnouncesEveryInputEvent() + public async Task NoteInputAnnouncesEveryInputEventWithARisingCount() { var (watcher, _, _) = Watcher(); - var seen = 0; - watcher.Input += () => seen++; + var seen = new List(); + watcher.Input += at => seen.Add(at); watcher.NoteInput(); watcher.NoteInput(); - await Assert.That(seen).IsEqualTo(2); + await Assert.That(seen).IsEquivalentTo(new long[] { 1, 2 }, TUnit.Assertions.Enums.CollectionOrdering.Matching); + await Assert.That(watcher.InputCount).IsEqualTo(2L); } private static (TerminalFocusWatcher Watcher, ManualTimeProvider Time, List Returns) Watcher(