From 60e815a715df41753dc9dc72c4a3097111922571 Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Thu, 13 Aug 2026 22:08:20 -0500 Subject: [PATCH 1/5] fix(compose): a post you wrote is a post you can recall MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ⌃S reached the session directly and skipped OnCommandEntered, which holds the one seam that adds a history entry — so a composed post was echoed and alias-expanded (both free from SendUserInputAsync) and never recorded. SendComposed's own doc had claimed all three for as long as the composer existed. It keeps going direct: OnCommandEntered is the command *line's* seam and clears that window's bar draft, moves the unsent marker and owns the /web, /graphics and /triggers branches. It records the entry itself instead — the built line rather than the buffer, since history holds sendable commands, and through InputHistory.Add so a post carrying a connect line meets the same secret gate. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN --- src/SharpMUTerm.Tui/SharpMUTermApp.cs | 24 +++++++++++++-- .../ComposeWindowTests.cs | 30 +++++++++++++++++++ 2 files changed, 51 insertions(+), 3 deletions(-) diff --git a/src/SharpMUTerm.Tui/SharpMUTermApp.cs b/src/SharpMUTerm.Tui/SharpMUTermApp.cs index 203d373..1e97d01 100644 --- a/src/SharpMUTerm.Tui/SharpMUTermApp.cs +++ b/src/SharpMUTerm.Tui/SharpMUTermApp.cs @@ -3642,9 +3642,26 @@ private void ToggleComposer() ?? (_workspace.FindWindow(windowId)?.SessionKey is { Length: > 0 } owner ? owner : null); /// - /// Sends a composed post as one line, through the ordinary command path — so it is echoed, recorded - /// in history and alias-expanded exactly like the same text typed on the command line, which is what - /// the buffer is. The window closes on success and keeps the post on a refusal. + /// Sends a composed post as one line — echoed, alias-expanded and recorded in history exactly like the + /// same text typed on the command line, which is what the buffer is. The window closes on + /// success and keeps the post on a refusal. + /// + /// It reaches the session directly rather than through , and so records + /// the history entry itself. That seam is the command line's: it clears that window's bar + /// draft, moves the unsent-input marker and owns the /web, /graphics and /triggers + /// branches, none of which belong to a post written in a different window. The doc here claimed the + /// ordinary path for as long as the composer existed, and two thirds of the claim were true — the echo + /// and the alias expansion come free from SendUserInputAsync, and the history did not, which is + /// how a composed post became the one user-authored command in this client with no recall route at all. + /// + /// + /// The built line is what is kept, not : history holds + /// sendable commands, and a recalled entry lands on a one-command bar the raw multi-line buffer would + /// not fit. Through like every other entry, so a post carrying a connect + /// line meets the same secret gate — that gate lives inside Add precisely so no caller can get + /// round it. On the armed bar's list, because that is where ⌥↑ and ⌃R will look from where the user is + /// standing. + /// /// private void SendComposed(ComposeResult result) { @@ -3669,6 +3686,7 @@ private void SendComposed(ComposeResult result) // was opened, already sent. _composer.Close(); _composeDrafts.Remove(session.SessionKey); + HistoryFor(BarKind(ActiveBar())).Add(line); _ = session.SendUserInputAsync(line); } diff --git a/tests/SharpMUTerm.Tui.Tests/ComposeWindowTests.cs b/tests/SharpMUTerm.Tui.Tests/ComposeWindowTests.cs index d99876d..50b40bb 100644 --- a/tests/SharpMUTerm.Tui.Tests/ComposeWindowTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/ComposeWindowTests.cs @@ -108,6 +108,36 @@ public async Task AltLFlipsTheEscapingAndChangesWhatIsSent() await Assert.That(world.Telnet.Lines).IsEquivalentTo(new[] { "100%% \\[sure\\]" }); } + /// + /// A composed post is recallable afterwards, like anything else the user wrote and sent. It is the + /// built line that is kept, not the editor's buffer: history holds sendable commands, and a + /// recalled entry lands on a one-command bar that the raw multi-line buffer would not fit. + /// + [Test] + public async Task AComposedPostIsRecallableFromTheCommandHistory() + { + var world = await Connected(); + world.App.SimulateKey(Key(ConsoleKey.F1)); + world.App.Composer.SimulateTyping("+bbpost 12=Title\nfirst\nsecond"); + + world.App.Composer.SimulateKey(CtrlS); + + await Assert.That(world.App.HistoryEntries(InputBar.Primary)) + .Contains("+bbpost 12=Title%rfirst%rsecond"); + } + + /// A post that was refused is a post the user still has; nothing was sent, so nothing is recalled. + [Test] + public async Task ARefusedPostEntersNoHistory() + { + var world = await Connected(); + world.App.SimulateKey(Key(ConsoleKey.F1)); + + world.App.Composer.SimulateKey(CtrlS); + + await Assert.That(world.App.HistoryEntries(InputBar.Primary)).IsEmpty(); + } + [Test] public async Task SendingAnEmptyComposerSaysSoAndSendsNothing() { From a194039fd566e6760306c6396402fdfb60ea522b Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Thu, 13 Aug 2026 22:17:02 -0500 Subject: [PATCH 2/5] feat(tabs): the key that walks a pane's tabs can now be found MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ⌃N has cycled the focused pane's tab strip for as long as panes have held more than one window, and it was named on F4 and nowhere else. It is now a ⌃P entry (layout:next-tab) and a status-row segment (⌃N tab, shown while the focused pane has a second tab). The chord stays ⌃N because the familiar spellings do not arrive, measured at a raw reader rather than assumed: ⌃Tab is 09, byte-identical to Tab; ⌃⇧Tab is CSI Z, byte-identical to ⇧Tab; ⌥Tab is ESC + a control byte and so arrives as two key events, on a chord the compositor takes anyway. Listing a key obliges it to answer. NextWindow returned in silence on a single-tab pane — indistinguishable from a dead key — and now refuses out loud, beside the pane cycle's own wording. Every surface says tab rather than window; F4 and --help said window while the rest said tab, and ⌥N already owns the window noun. FocusHints is generated from a segment list instead of eight hand-written ladders, with reading order and drop order kept separate so the existing pane · size · line row is unchanged cell for cell. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN --- CLAUDE.md | 26 +++ docs/design/README.md | 2 +- .../Commands/CommandCatalog.cs | 11 ++ src/SharpMUTerm.Tui/MacroKeys.cs | 7 +- src/SharpMUTerm.Tui/Program.cs | 2 +- src/SharpMUTerm.Tui/SharpMUTermApp.cs | 81 ++++++-- .../Commands/CommandCatalogTests.cs | 15 ++ tests/SharpMUTerm.Tui.Tests/TabCycleTests.cs | 180 ++++++++++++++++++ 8 files changed, 304 insertions(+), 20 deletions(-) create mode 100644 tests/SharpMUTerm.Tui.Tests/TabCycleTests.cs diff --git a/CLAUDE.md b/CLAUDE.md index 9e549e1..036ce35 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -724,6 +724,32 @@ markup (`[bold #rrggbb on #rrggbb]…[/]`, `[[`/`]]` escaping, `[link=url]…[/] `TryRecallKey` now matches on — **exactly**, so `⌥⇧↑` (the pane resize) still reaches its own handler. A macro bound to `Alt+Up` wins over recall, because `DispatchMacro` runs first: the same relationship `Ctrl+←/→` has with pane selection. + - **`⌃N` walks the focused pane's tab strip, and it stays `⌃N` because the familiar spellings do not + exist here.** Asked for "an easy key-combination to tab through the tabs of the active pane", the + obvious candidates were measured at a raw reader with `kitten @ send-key` before anything was built: + `⌃Tab` is `09`, byte-identical to a bare Tab (already in `MacroKeys.ControlBytes`); `⌃⇧Tab` is + `CSI Z`, byte-identical to plain `⇧Tab`; and **`⌥Tab` is `ESC` + `09`**, which is `ESC` + a *control* + byte and so arrives as **two** key events rather than an Alt chord (`AnsiInputParser.ProcessEscape`). + A `TryAltEnter`-style reassembly could pair them, but Tab is already spent as + `TerminalFocusWatcher`'s disguised focus-in — and `send-key` writes into the pty, so it says nothing + about the *compositor*, which takes `⌥Tab` unconditionally on Windows, GNOME and KDE. `⌃PgUp`/`⌃PgDn` + is the one familiar pair that does arrive (`CSI 5;5~` / `CSI 6;5~`, decoded by `DispatchTilde`, and + free because `TryScrollKey` matches PageUp/PageDown only at `ctrl: false`) — kept in reserve rather + than spent, since the reported problem was that `⌃N` could not be *found*, not that it was wrong. + - **So the fix was discoverability, and the chord had to earn it by answering.** `⌃N` was named on F4 + and nowhere else. It is now a ⌃P entry (`layout:next-tab`, listed unconditionally like the + directional pane entries, because this surface is where a reader learns a pane holds tabs at all) and + a status-row segment (`⌃N tab`, shown exactly while the focused pane has a second tab, the same + contextual rule its neighbours follow). Listing a key obliges it to answer: `NextWindow` **returned + in silence** on a single-tab pane, which is indistinguishable from a dead key, and now refuses out + loud beside `PrefixPanel.NoCycleRefusal`'s wording. Every surface says **tab**, not "window" — F4 and + `--help` said window while everything else said tab, and `⌥N` already owns the window noun. + - **`FocusHints` separates reading order from drop order.** Three independent conditions is eight + cases, so the ladder is generated; but the row reads `pane · size · line` while *size* is the first + thing surrendered, so a generator that dropped from the end of the reading order would silently + reorder a row nobody asked to reorder. The tab segment is given up second, and that judgement is + written down: a pane's tabs are drawn as a strip you can see, so the hint names a shortcut to + something already visible, while nothing on screen says how to reach another pane or the second bar. - **Known and not fixed here**: `⌃N` and `⌃O` have no reverse (the character cycle does — `⌥J`/`⌥K`), and `⌃W` and `⌃B x` are two chords for one action. Both are shape complaints rather than defects, and both are behaviour changes rather than modifier moves. diff --git a/docs/design/README.md b/docs/design/README.md index 6947542..086b097 100644 --- a/docs/design/README.md +++ b/docs/design/README.md @@ -425,7 +425,7 @@ count on the tab, the rail character, and the rail world. `⌃P` command surface · `⌃F` search the output (`⌥G` next hit) · `⌥F` freeze/resume in focused pane · `⌃R` command-history search · -`⌃N` next window · `⌥D`/`⌥R` disconnect/reconnect · `⌥↑`/`⌥↓` command history (`↑`/`↓` do it too, +`⌃N` next tab in the focused pane · `⌥D`/`⌥R` disconnect/reconnect · `⌥↑`/`⌥↓` command history (`↑`/`↓` do it too, where the caret has nowhere further to go) · `⌥⏎`, or `⌃L`, newline in input · `F1` composer · `F2`–`F9` config · `Esc` close overlay. diff --git a/src/SharpMUTerm.Core/Commands/CommandCatalog.cs b/src/SharpMUTerm.Core/Commands/CommandCatalog.cs index 3bf66cf..2d16928 100644 --- a/src/SharpMUTerm.Core/Commands/CommandCatalog.cs +++ b/src/SharpMUTerm.Core/Commands/CommandCatalog.cs @@ -250,6 +250,17 @@ public static IReadOnlyList Build( items.Add(new CommandItem(CommandGroup.Layout, "Focus pane down", "layout:focus-down", "⌃↓")); items.Add(new CommandItem(CommandGroup.Layout, "Focus the next pane", "layout:cycle", "⌃O · ⌃B o")); + // The tab cycle, beside the pane cycle it rhymes with. Listed unconditionally for the same reason + // the four directional entries above are: this surface is where the keyboard is learnt, and a + // reader whose panes each hold one window has no other way to find out that a pane holds tabs at + // all. ⌃N has always done this and was named on F4 and nowhere else. + // + // Listing it obliges it to answer, which the directional entries pay for by refusing out loud and + // this one did not — it returned in silence on a pane with one tab, which is what a dead key looks + // like. The refusal is the host's (SharpMUTermApp.NextWindow); the entry is only allowed to exist + // because it is there. + items.Add(new CommandItem(CommandGroup.Layout, "Focus the next tab", "layout:next-tab", "⌃N")); + // Numbered pane jumps, one entry per pane that exists — the one group here that is *not* listed // unconditionally, because "Go to pane 4" on a workspace with two panes names a place there is no // way to make. The number is the one the move and drag overlays badge each pane with, so the entry diff --git a/src/SharpMUTerm.Tui/MacroKeys.cs b/src/SharpMUTerm.Tui/MacroKeys.cs index 43f688b..cc1b5fa 100644 --- a/src/SharpMUTerm.Tui/MacroKeys.cs +++ b/src/SharpMUTerm.Tui/MacroKeys.cs @@ -105,7 +105,12 @@ private static AppShortcut[] BuildAppShortcuts() private static AppShortcut[] Fixed() => new AppShortcut[] { new(ConsoleModifiers.Control, ConsoleKey.Q, "asks whether to quit"), - new(ConsoleModifiers.Control, ConsoleKey.N, "picks the next window"), + // "the next tab in this pane", not "the next window", and the wording is the point. A tab *is* a + // window — but ⌥N goes to a numbered window anywhere in the workspace, and this walks the strip of + // the pane in front of you, so two keys described in the same noun read as two spellings of one + // action. F4, --help, the ⌃P entry and the status row all say tab now; they said window here and + // tab everywhere else, which is the drift the numbering vocabularies are kept apart to avoid. + new(ConsoleModifiers.Control, ConsoleKey.N, "goes to the next tab in this pane"), // ⌃Tab is deliberately absent, and its absence is measured rather than assumed: a terminal writes // 0x09 for it, byte-identical to a bare Tab (read off a pty with `kitten @ send-key`), so the // parser reports ConsoleKey.Tab with no Control bit and this claim could never once have matched. diff --git a/src/SharpMUTerm.Tui/Program.cs b/src/SharpMUTerm.Tui/Program.cs index 4b00e07..fef155a 100644 --- a/src/SharpMUTerm.Tui/Program.cs +++ b/src/SharpMUTerm.Tui/Program.cs @@ -299,7 +299,7 @@ private static void WriteUsage(TextWriter usage) usage.WriteLine("'at start' only opens the connection. What is typed once one is open follows from the"); usage.WriteLine("character's saved password and connect line — F5's 'login' row says which."); usage.WriteLine(); - usage.WriteLine("In-app: Up/Down history · Ctrl+N next window · Ctrl+W close · Ctrl+P palette · Ctrl+Q quit."); + usage.WriteLine("In-app: Up/Down history · Ctrl+N next tab · Ctrl+W close · Ctrl+P palette · Ctrl+Q quit."); // The composer earns a line of its own because what it *sends* is not guessable from the window: // the buffer is one command and its line breaks are written %r, which is what a MUSH board or // mail body wants. Naming the send chord matters for the same reason — Ctrl+Enter is what a diff --git a/src/SharpMUTerm.Tui/SharpMUTermApp.cs b/src/SharpMUTerm.Tui/SharpMUTermApp.cs index 1e97d01..639e1dd 100644 --- a/src/SharpMUTerm.Tui/SharpMUTermApp.cs +++ b/src/SharpMUTerm.Tui/SharpMUTermApp.cs @@ -6757,6 +6757,9 @@ internal bool DispatchCommand(string id) CyclePane(); return true; + case "layout:next-tab": + NextWindow(); // refuses out loud on a pane with one tab, the same as the chord does + return true; case "term:newline": // The same edit Alt+⏎ makes, through the same key table, so the surface cannot drift from // the chord it advertises. @@ -8319,13 +8322,33 @@ private void RebuildPaneArea() /// The TabControl of the focused pane, or null if none is realised. private TabControl? FocusedTabs() => _paneTabs.GetValueOrDefault(_workspace.Layout.FocusedPaneId); - /// Cycles to the next window tab in the focused pane, wrapping (⌃N). + /// + /// Why ⌃N can refuse. Named beside the pane cycle's own wording () + /// and phrased to match it, because the two keys are one gesture at two scales and a reader who has met + /// one should recognise the other. + /// + private const string NoTabCycleRefusal = "nowhere to cycle to — this pane has one tab"; + + /// + /// Cycles to the next window tab in the focused pane, wrapping (⌃N, and ⌃P ▸ Focus the next tab). + /// + /// A pane holding a single tab is refused out loud. It used to return in silence, which was + /// tolerable only while the chord was advertised nowhere but F4 — the moment the ⌃P surface lists it, + /// the key is held to the same rule as the directional pane entries beside it, every one of which says + /// why nothing happened. A key that is dead and a key that has nowhere to go look identical otherwise, + /// and this one is a wrap: on two tabs it always moves, so the state it is silent in is the state a + /// first-time reader is most likely to try it in. + /// + /// private void NextWindow() { - if (FocusedTabs() is { TabCount: > 1 } tabs) + if (FocusedTabs() is not { TabCount: > 1 } tabs) { - tabs.ActiveTabIndex = (tabs.ActiveTabIndex + 1) % tabs.TabCount; + RefuseCommand(NoTabCycleRefusal); + return; } + + tabs.ActiveTabIndex = (tabs.ActiveTabIndex + 1) % tabs.TabCount; } /// @@ -10561,26 +10584,50 @@ private string HeaderMarkup() /// navigation one, instead of losing both because the pair no longer fitted. The chord is still named /// on the ⌃P surface and in --help either way. /// + /// + /// The ladder is generated, not written out per combination. Three independent conditions is + /// eight cases, each needing its own ordered candidates, and eight hand-written ladders is eight + /// chances for one of them to drop the wrong segment. + /// + /// + /// Reading order and drop order are separate, and have to be. The row reads + /// pane · size · line — the two pane chords together, then the bars — while size is the + /// first thing given up. A generator that dropped from the end of the reading order would have to put + /// size last, which reorders a row nobody asked to have reordered. + /// /// private string[] FocusHints() { var panes = _workspace.Layout.Panes.Count > 1 && _workspace.Layout.ZoomedPaneId is null; var bars = _second.Visible; - return (panes, bars) switch - { - (true, true) => new[] - { - "[dim]⌃←→↑↓ pane · ⌥⇧←→↑↓ size · ⇥ line[/]", - "[dim]⌃←→↑↓ pane · ⇥ line[/]", - }, - (true, false) => new[] - { - "[dim]⌃←→↑↓ pane · ⌥⇧←→↑↓ size[/]", - "[dim]⌃←→↑↓ pane[/]", - }, - (false, true) => new[] { "[dim]⇥ · ⌃↑↓ line[/]" }, - _ => Array.Empty(), + var tabs = FocusedTabs() is { TabCount: > 1 }; + + // In reading order, each with the rank it is surrendered at — lowest goes first. + // + // Size is rank 0 as it always was: the longest claim for the least urgent fact. The tab cycle + // follows it, and that is the one judgement here worth stating — a pane's tabs are drawn as a + // strip the reader can see, so this hint names a shortcut to something already visible, while + // nothing at all on the screen says how to move between panes or how to reach the second command + // line. Where you are outlives everything. + (string Text, int Rank)?[] ordered = + { + panes ? ("⌃←→↑↓ pane", 3) : null, + tabs ? ("⌃N tab", 1) : null, + panes ? ("⌥⇧←→↑↓ size", 0) : null, + + // ⌃↑↓ is only worth naming where the pane arrows have not already said it. + bars ? (panes ? "⇥ line" : "⇥ · ⌃↑↓ line", 2) : null, }; + + var segments = ordered.OfType<(string Text, int Rank)>().ToList(); + var candidates = new List(segments.Count); + while (segments.Count > 0) + { + candidates.Add($"[dim]{string.Join(" · ", segments.Select(s => s.Text))}[/]"); + segments.Remove(segments.MinBy(s => s.Rank)); + } + + return candidates.ToArray(); } /// diff --git a/tests/SharpMUTerm.Core.Tests/Commands/CommandCatalogTests.cs b/tests/SharpMUTerm.Core.Tests/Commands/CommandCatalogTests.cs index 14d67e9..d77b69d 100644 --- a/tests/SharpMUTerm.Core.Tests/Commands/CommandCatalogTests.cs +++ b/tests/SharpMUTerm.Core.Tests/Commands/CommandCatalogTests.cs @@ -48,6 +48,21 @@ public async Task StatefulCommands_ReadCurrentValue() await Assert.That(loggingOn.Any(c => c.Title == "Resume scrollback")).IsTrue(); } + /// + /// The tab cycle is listed, and it is listed on a workspace whose panes each hold one tab — the same + /// rule the directional pane entries follow, because this surface is where a reader learns that a pane + /// holds tabs at all. The chord it names is the one that runs it. + /// + [Test] + public async Task TheTabCycleIsListedWithItsChord() + { + var catalog = CommandCatalog.Build(new Workspace(), Characters, null, new CommandContext()); + + var entry = catalog.Single(c => c.Id == "layout:next-tab"); + await Assert.That(entry.Title).IsEqualTo("Focus the next tab"); + await Assert.That(entry.Subtitle).IsEqualTo("⌃N"); + } + /// /// The numbered pane entries: one per pane that exists, in Panes order (which is the order the /// move overlay badges them in), and only when there is more than one pane. The first nine carry diff --git a/tests/SharpMUTerm.Tui.Tests/TabCycleTests.cs b/tests/SharpMUTerm.Tui.Tests/TabCycleTests.cs new file mode 100644 index 0000000..d100000 --- /dev/null +++ b/tests/SharpMUTerm.Tui.Tests/TabCycleTests.cs @@ -0,0 +1,180 @@ +using SharpConsoleUI.Drivers; +using SharpMUTerm.Graphics; + +namespace SharpMUTerm.Tui.Tests; + +/// +/// ⌃N, and the three places it is now readable from. The chord itself is not new — it has cycled the +/// focused pane's tabs for as long as panes have held more than one window — but it was named on F4 and +/// nowhere else, which is the state ⌃L's newline sat in until it was reported missing. +/// +/// The refusal is the part that is genuinely new behaviour. Advertising a key on a surface obliges that +/// key to do something or say why not: every directional pane entry beside it refuses out loud, and this +/// one returned silently on a pane holding one tab. +/// +/// +/// Serialised: rendering redirects the process-global Console.Out. +[NotInParallel] +public class TabCycleTests +{ + private const int Width = 120; + private const int Height = 32; + + private static readonly TerminalCapabilities Headless = + new(GraphicsProtocol.None, supportsTrueColor: true, supportsKittyGraphics: false, supportsSixel: false); + + private static ConsoleKeyInfo CtrlN => + new('\0', ConsoleKey.N, shift: false, alt: false, control: true); + + private static SharpMUTermApp Demo() + { + Console.SetIn(TextReader.Null); + return new SharpMUTermApp(DemoScene.Build(), Headless, new HeadlessConsoleDriver(Width, Height)); + } + + /// + /// A fresh client: one pane holding one window. Not the demo scene, whose main pane already carries + /// the Chat capture as a second tab — which is the whole reason the chord had somewhere to go in every + /// frame anyone had looked at, and the silent refusal went unnoticed. + /// + private static SharpMUTermApp OneTab() + { + Console.SetIn(TextReader.Null); + return new SharpMUTermApp( + new SharpMUTerm.Core.Configuration.AppConfiguration(), + Headless, + new HeadlessConsoleDriver(Width, Height)); + } + + /// + /// An app whose clock a test can move past . The scenes + /// that put two tabs in front get there by switching character, and that raises a notice which sits + /// over the resting row — so a test reading the row's own content has to let it retire + /// rather than assert against the message that displaced it. + /// + private static (SharpMUTermApp App, ManualTimeProvider Clock) TimedDemo(int width = Width, int height = Height) + { + Console.SetIn(TextReader.Null); + var clock = new ManualTimeProvider(); + return (new SharpMUTermApp( + DemoScene.Build(), Headless, new HeadlessConsoleDriver(width, height), time: clock), clock); + } + + /// + /// tint-tabs is the one view where the focused pane holds two windows as tabs — every other + /// scene with two tabs puts them in a pane that does not hold the focus, and the cycle acts on the + /// focused one. + /// + private static SharpMUTermApp TwoTabsInFront() + { + var app = Demo(); + app.RenderSnapshot("tint-tabs"); + return app; + } + + [Test] + public async Task CtrlNMovesToTheNextTabOfTheFocusedPane() + { + var app = TwoTabsInFront(); + var before = app.ActiveWindowId(); + + app.SimulateKey(CtrlN); + + await Assert.That(app.ActiveWindowId()).IsNotEqualTo(before); + } + + /// + /// Wrapping, which is what makes one key enough: pressed round the strip it comes back rather than + /// stopping at the end. It is also why there is no backward chord to look for. The count is + /// discovered rather than written down — the demo pane holds however many windows the scene left in + /// it, and a literal here would be a test asserting on the fixture instead of on the cycle. + /// + [Test] + public async Task TheCycleVisitsEveryTabAndWrapsBackToWhereItStarted() + { + var app = TwoTabsInFront(); + var first = app.ActiveWindowId(); + + var visited = new List { first }; + for (var press = 0; press < 10; press++) + { + app.SimulateKey(CtrlN); + if (app.ActiveWindowId() == first) + { + break; + } + + visited.Add(app.ActiveWindowId()); + } + + await Assert.That(visited.Distinct().Count()).IsEqualTo(visited.Count).Because("no tab is visited twice"); + await Assert.That(visited.Count).IsGreaterThan(1); + await Assert.That(app.ActiveWindowId()).IsEqualTo(first); + } + + /// + /// The new behaviour. A pane holding one tab has nowhere to cycle to, and the chord said nothing at + /// all — which is exactly what a key that is broken looks like, and is not something the ⌃P surface + /// may list without an answer. + /// + [Test] + public async Task CtrlNOnAPaneHoldingOneTabRefusesOutLoud() + { + var app = OneTab(); + app.RenderSnapshot(); + + app.SimulateKey(CtrlN); + + await Assert.That(app.StatusMarkup).Contains("this pane has one tab"); + } + + /// The ⌃P entry and the chord are one action, so they must leave the same tab in front. + [Test] + public async Task TheCommandSurfaceEntryDoesWhatTheChordDoes() + { + var viaKey = TwoTabsInFront(); + viaKey.SimulateKey(CtrlN); + + var viaEntry = TwoTabsInFront(); + await Assert.That(viaEntry.DispatchCommand("layout:next-tab")).IsTrue(); + + await Assert.That(viaEntry.ActiveWindowId()).IsEqualTo(viaKey.ActiveWindowId()); + } + + /// + /// The status row names the chord exactly while the focused pane has somewhere to cycle to — the same + /// contextual rule the pane and second-bar hints beside it follow, and the reason a fresh client's row + /// is not carrying a key that would only refuse. + /// + [Test] + public async Task TheStatusRowNamesTheChordOnlyWhileThereAreTabsToCycle() + { + var (tabs, clock) = TimedDemo(); + tabs.RenderSnapshot("tint-tabs"); + clock.Advance(SharpMUTermApp.NoticeDuration); // the character switch's notice retires off the row + await Assert.That(tabs.StatusMarkup).Contains("⌃N tab"); + + var solo = OneTab(); + solo.RenderSnapshot(); + await Assert.That(solo.StatusMarkup).DoesNotContain("⌃N tab"); + } + + /// + /// The hint is a segment of a sticky row, and a row that overflows wraps and costs every pane + /// a line of output — which per-pane NAWS then re-announces to every connected server. The tab segment + /// must therefore give way on a narrow terminal like the resize hint does, and the pane hint must + /// survive both of them going. + /// + [Test] + public async Task ANarrowTerminalDropsTheTabHintBeforeTheOneThatSaysWhereYouAre() + { + var narrow = new SharpMUTermApp(DemoScene.Build(), Headless, new HeadlessConsoleDriver(76, 30)); + narrow.RenderSnapshot("split"); + + await Assert.That(narrow.StatusMarkup).Contains("⌃←→↑↓ pane"); + foreach (var row in FrameGrid.Decode(narrow.RenderSnapshot("split"), 76, 30)) + { + await Assert.That(row.TrimEnd().Length).IsLessThanOrEqualTo(76); + } + } +} From d10a94fdd053d5338c21984960abb6dfb91863cd Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Thu, 13 Aug 2026 23:01:24 -0500 Subject: [PATCH 3/5] docs: FocusHints' doc names the tab segment and its rank MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The paragraph stated the reading order as pane · size · line and explained only size's priority, while the code inserts ⌃N tab between the pane and size segments at rank 1. CLAUDE.md carried the reasoning; the doc a reader of the method actually sees did not. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN --- src/SharpMUTerm.Tui/SharpMUTermApp.cs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/SharpMUTerm.Tui/SharpMUTermApp.cs b/src/SharpMUTerm.Tui/SharpMUTermApp.cs index 639e1dd..d6f7d1b 100644 --- a/src/SharpMUTerm.Tui/SharpMUTermApp.cs +++ b/src/SharpMUTerm.Tui/SharpMUTermApp.cs @@ -10591,9 +10591,15 @@ private string HeaderMarkup() /// /// /// Reading order and drop order are separate, and have to be. The row reads - /// pane · size · line — the two pane chords together, then the bars — while size is the - /// first thing given up. A generator that dropped from the end of the reading order would have to put - /// size last, which reorders a row nobody asked to have reordered. + /// pane · tab · size · line — the pane chords together, then the bars — while size is + /// the first thing given up. A generator that dropped from the end of the reading order would have to + /// put size last, which reorders a row nobody asked to have reordered. + /// + /// + /// ⌃N tab is surrendered second, and that is the one ranking here worth arguing. A pane's + /// tabs are drawn as a strip the reader can see, so this hint names a shortcut to something already on + /// the screen — while nothing at all says how to reach another pane or the second command line. Where + /// you are outlives everything. /// /// private string[] FocusHints() From 9a1408b7eed2d673bff7e38f617b0b8751d720ea Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Thu, 13 Aug 2026 22:35:08 -0500 Subject: [PATCH 4/5] feat(panes): select a pane's output with the mouse and copy it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The terminal's own selection cannot do this job — UrlDetector's argument one layer over. Under ?1003, which this client needs for the wheel, tab and rail clicks and pane drag-and-drop, a plain drag belongs to the application; the emulator's escape hatch selects a terminal *row*, which on a vertical split crosses the divider into another pane's output, and since a pane is narrower than the row a logical line wraps and comes back with newlines injected at the wrap points. Almost all of it is the framework's, shipped in the pinned 2.5.14 and off by default: drag, double-click word, triple-click line, drag-autoscroll and a soft-wrap-aware copy. What had to be ours is the colour, the clipboard, and what happens when the buffer moves. - WorkspacePalette.SelectionBand/SelectionInk: one pair per theme, since a selection is not an identity or a focus fact. ReadingPlane pushed further in the direction of travel, leaned toward Theme.Prompt. Held to a fill floor against all fourteen planes and the ink to Contrast.Floor on the band — the highlight replaces the world's foreground too, so that ink is what all selected output is read in. - The clipboard writer is caller-supplied and null by default, the save:/logRoot:/openUrl: family. It also buys one copy path: the framework's ⌃C writes through a static helper no caller can substitute, so a test run would have replaced the developer's real clipboard. - ⌃C is claimed in the main window's key chain, not in AppShortcuts — a global shortcut would take it from the composer's editor as well. - RepaintPane drops any selection: chrome rows go in and out mid-buffer and a selection anchored to display rows would highlight text nobody dragged. - NewPaneControl is now the one place a pane control is made. Enabling selection on PaneContentFor alone left the main window unable to select anything, because that control is built in the constructor. SimulatePaneDrag is the test seam (the framework routes mouse only inside Run()), and the new `selection` view is the frame — in FrameContrastTests' list, because a colour nothing renders is a colour nobody checks. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN --- CLAUDE.md | 59 ++++- docs/design/README.md | 3 +- .../Commands/CommandCatalog.cs | 7 + src/SharpMUTerm.Tui/Program.cs | 10 +- src/SharpMUTerm.Tui/SharpMUTermApp.cs | 225 +++++++++++++++++- src/SharpMUTerm.Tui/WorkspacePalette.cs | 58 +++++ .../FrameContrastTests.cs | 5 + .../LegiblePaletteTests.cs | 58 +++++ .../PaneSelectionTests.cs | 187 +++++++++++++++ 9 files changed, 602 insertions(+), 10 deletions(-) create mode 100644 tests/SharpMUTerm.Tui.Tests/PaneSelectionTests.cs diff --git a/CLAUDE.md b/CLAUDE.md index 036ce35..f697e34 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -136,6 +136,58 @@ fallbacks) for inline images/maps. - **The demo scene loads with `_watching` off.** It pours a spawn window's whole history in before the first frame, and every line would otherwise count as missed — so any frame that later made such a window visible carried an activity bar reporting the client's own setup as news. +- **A pane's output can be selected with the mouse and copied with `⌃C`, and almost all of it is the + framework's** (`MarkupControl.EnableSelection`, shipped in the pinned 2.5.14 and **off by default**). + Drag, double-click word, triple-click line, drag-autoscroll through the `ScrollablePanelControl` each + pane already sits in, one-selection-per-window arbitration and a wrap-aware copy all come from the + package. This is the rare case where the answer was to *switch something on* rather than build it; what + had to be ours is the colour, the clipboard, and what happens to a selection when the buffer moves. + - **The terminal's own selection cannot do this job, which is `UrlDetector`'s argument one layer over.** + Under `?1003` — which this client needs for the wheel, tab and rail clicks and pane drag-and-drop — a + plain drag belongs to the application, and the emulator's escape hatch (kitty's ⇧-drag) selects a + terminal **row**: on a vertical split that row crosses the divider into another pane's output, and + since a pane is narrower than the row a logical line wraps and comes back with newlines injected at + the wrap points. The framework's copy walks the painted cells and breaks a line only where a row is + not a soft-wrap continuation. There is no partial retreat from `?1003` that keeps the wheel. + - **The selection band is one pair per theme, and for a different reason than `ReadingPlane`'s.** There + it is cost; here it is meaning. A pane's plane already says whose connection this is (hue) and where + the keyboard is (luminance), and a selection is neither — it is the client answering a gesture being + made *now*, in exactly one pane at a time, so a highlight that changed colour with whose pane it + landed in would report a fact nobody asked about. `WorkspacePalette.SelectionBand` is `ReadingPlane` + pushed *further* in the direction of travel — brighter on a dark theme, darker on a light one — then + leaned toward `Theme.Prompt`, the same anchor `ArmedBand` uses. It is held to a **fill** floor against + all fourteen planes (1.5:1; tightest measured 2.70:1) and `SelectionInk` to `Contrast.Floor` on the + band, which matters more here than anywhere: the highlight replaces the world's *foreground* too, so + that one ink is what all selected output is read in. + - **The clipboard writer is caller-supplied and null by default** — the `save:`/`logRoot:`/`openUrl:` + family, and here it also buys a *single* copy path. The framework's own ⌃C writes straight to the + system clipboard through a static helper no caller can substitute, so a test run would replace + whatever the developer had copied and the one path that could not be injected would be the one under + test. `CopyEnabled = false` on the pane controls and the chord is answered by + `SharpMUTermApp.CopyFocusedSelection` instead; `Program` supplies `ClipboardHelper.SetText`, which + covers OSC 52 *and* the platform tool, so a copy lands locally and over ssh alike. + - **⌃C is claimed in the main window's key chain, not in `MacroKeys.AppShortcuts`.** A global shortcut + runs ahead of *every* window including the composer, whose `MultilineEditControl` has its own ⌃C and + is a real editor. Being in the chain also puts it after `DispatchMacro`, so a macro bound to ⌃C wins — + the same relationship ⌃←/→ has with pane selection, and the reason `Verdict` needs no special case. + - **A selection is dropped whenever the rows under it move**, in `RepaintPane` — the one seam that + re-feeds a pane. Chrome rows go in and out mid-buffer (the freeze bar, the away bar, `NEW`, the search + bar) and the timestamp toggle re-feeds whole buffers; a selection is anchored to display rows, so one + left alone across an insert highlights text nobody dragged over. + - **`NewPaneControl` is the one place a pane's control is made**, and it exists because enabling + selection on `PaneContentFor` alone left the **main** window — the pane most people are looking at — + unable to select anything. That control is built in the constructor, before a workspace exists; every + other one is built on demand. Two creation paths, and the trap announced itself immediately. + - **`SimulatePaneDrag` is the test seam**, for `SimulatePaneClick`'s reason: the framework registers its + driver-mouse handler inside `Run()`, which no test calls. The drag flag rides *with* the button flag, + because SGR encodes motion-while-held as `Button1Pressed | Button1Dragged` and a seam sending the bare + form would exercise a path the terminal never produces. The `selection` view is the frame, and it is + in `FrameContrastTests`' list — a colour nothing renders is a colour nobody checks. + - **Not fixed, and known**: chrome rows live in the same buffer and are selectable (a terminal selection + would take them too); OSC 52 caps at ~74 KB and `Osc52.BuildSequence` returns null past it, so a very + large copy lands locally and **silently** does not travel over ssh; GNU screen has OSC 52 disabled + upstream; tmux needs `allow-passthrough on`. The Windows mouse path is a separate ad-hoc parser in + `NetConsoleDriver` and nothing here can verify it — treat Windows drag-select as unproven. - **Coming back to a window you were not watching leaves a bar where you left off, and that covers two different absences.** The *window* one is `NEW` and is the common case: a line lands while the window is not `Workspace.IsCaughtUp` — visible **and** at its live tail — and `_missedFrom` records the index @@ -539,7 +591,12 @@ python3 tools/ansi_frame_to_image.py frame.ansi frame.html # or .svg a viewport row), `activity-bar` (the *other* absence — a window the reader was not watching: three lines land in the main window while Chat is in front of it, and picking main back lands on the `NEW` bar with those three under it. Separate from `away` because the two are separate facts with separate - wording, and this is the one that happens many times an hour), `prefix-panel` (the ⌃B which-key + wording, and this is the one that happens many times an hour), + `selection` (a real ⌃-drag across the main window's output, through `SimulatePaneDrag` and the control's + own hit test rather than a highlight posed by hand — the only frame carrying `WorkspacePalette.SelectionBand`, + which is why it is in `FrameContrastTests`' list: the band is the one plane this client invents rather + than derives from a pane, and its ink replaces the world's own on every selected cell), + `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 ⌃→), diff --git a/docs/design/README.md b/docs/design/README.md index 086b097..5c8fe94 100644 --- a/docs/design/README.md +++ b/docs/design/README.md @@ -425,7 +425,8 @@ count on the tab, the rail character, and the rail world. `⌃P` command surface · `⌃F` search the output (`⌥G` next hit) · `⌥F` freeze/resume in focused pane · `⌃R` command-history search · -`⌃N` next tab in the focused pane · `⌥D`/`⌥R` disconnect/reconnect · `⌥↑`/`⌥↓` command history (`↑`/`↓` do it too, +`⌃N` next tab in the focused pane · `⌃C` copy the pane selection (drag to select) · +`⌥D`/`⌥R` disconnect/reconnect · `⌥↑`/`⌥↓` command history (`↑`/`↓` do it too, where the caret has nowhere further to go) · `⌥⏎`, or `⌃L`, newline in input · `F1` composer · `F2`–`F9` config · `Esc` close overlay. diff --git a/src/SharpMUTerm.Core/Commands/CommandCatalog.cs b/src/SharpMUTerm.Core/Commands/CommandCatalog.cs index 2d16928..4562b66 100644 --- a/src/SharpMUTerm.Core/Commands/CommandCatalog.cs +++ b/src/SharpMUTerm.Core/Commands/CommandCatalog.cs @@ -200,6 +200,13 @@ public static IReadOnlyList Build( "term:restore-purge", "deletes every pane's saved content")); + // Copying output. Listed unconditionally and subtitled with the *gesture* as well as the chord, + // because the gesture is the part nobody can guess: under mouse reporting a plain drag belongs to + // the application, so a user who has learnt that their terminal needs ⇧-drag has no reason to try + // dragging here. It refuses out loud with nothing selected, which is what earns it a row at all. + items.Add(new CommandItem( + CommandGroup.Terminal, "Copy the selection", "term:copy", "⌃C · drag across a pane to select")); + // The client's own messages — the status-line notices that dismiss themselves — kept out of the // output window (and so out of the session log) and readable here instead. items.Add(new CommandItem( diff --git a/src/SharpMUTerm.Tui/Program.cs b/src/SharpMUTerm.Tui/Program.cs index fef155a..5c41658 100644 --- a/src/SharpMUTerm.Tui/Program.cs +++ b/src/SharpMUTerm.Tui/Program.cs @@ -143,7 +143,14 @@ private static int Main(string[] args) logRoot: logRoot, restore: restore, mssp: mssp, - openUrl: ExternalBrowser.Open); + openUrl: ExternalBrowser.Open, + + // The one place a real clipboard is reached. SharpConsoleUI's helper writes OSC 52 *and* + // mirrors to the platform tool (wl-copy / xclip / pbcopy / Win32), so a copy lands whether the + // client is local or on the far end of an ssh session. Supplied here rather than reached for + // inside the app for the reason logRoot and the browser launcher are: nothing that is not this + // entry point may touch the developer's clipboard, least of all the test suite. + clipboard: SharpConsoleUI.Helpers.ClipboardHelper.SetText); var exitCode = liveApp.Run(startup); // blocks on the SharpConsoleUI main loop until exit // Persist the workspace so the next launch resumes where this one left off. @@ -300,6 +307,7 @@ private static void WriteUsage(TextWriter usage) usage.WriteLine("character's saved password and connect line — F5's 'login' row says which."); usage.WriteLine(); usage.WriteLine("In-app: Up/Down history · Ctrl+N next tab · Ctrl+W close · Ctrl+P palette · Ctrl+Q quit."); + usage.WriteLine("Selection: drag across a pane's output to select it, Ctrl+C to copy."); // The composer earns a line of its own because what it *sends* is not guessable from the window: // the buffer is one command and its line breaks are written %r, which is what a MUSH board or // mail body wants. Naming the send chord matters for the same reason — Ctrl+Enter is what a diff --git a/src/SharpMUTerm.Tui/SharpMUTermApp.cs b/src/SharpMUTerm.Tui/SharpMUTermApp.cs index d6f7d1b..9db621b 100644 --- a/src/SharpMUTerm.Tui/SharpMUTermApp.cs +++ b/src/SharpMUTerm.Tui/SharpMUTermApp.cs @@ -446,6 +446,19 @@ private sealed class SizeReport /// private readonly Action? _openUrl; + /// + /// Where a copied selection goes, or null when this app may not reach a clipboard at all. + /// + /// The save/logRoot/openUrl family, for the family's reason: only Program + /// knows it is the live client, so a snapshot and a test provably leave the developer's real clipboard + /// holding whatever it held. It also means there is one copy path — the framework's own ⌃C + /// handler writes straight to the system clipboard and is switched off on these controls, because a + /// second writer would be a second answer and the one that could not be injected would be the one that + /// ran under test. + /// + /// + private readonly Action? _clipboard; + /// /// The directory session transcripts are written under, or null for an app that owns no log /// directory — which is the default, and is what every test and every snapshot gets. See the @@ -562,11 +575,13 @@ public SharpMUTermApp( RestoreLog? restore = null, MsspCache? mssp = null, bool? focusReporting = null, - Action? openUrl = null) + Action? openUrl = null, + Action? clipboard = null) { _config = config; _save = save; _openUrl = openUrl; + _clipboard = clipboard; _logRoot = string.IsNullOrWhiteSpace(logRoot) ? null : logRoot; _restore = restore; _mssp = mssp ?? new MsspCache(); @@ -638,9 +653,7 @@ public SharpMUTermApp( _header.FocusedLinkBackgroundColor = ToColor(new Rgb(brand.R, brand.G, brand.B)); _header.FocusedLinkForegroundColor = ToColor(_theme.Resolve(TerminalColor.Default, isBackground: true)); - var main = new MarkupControl(new List()); - main.LinkClicked += (_, e) => OnLinkClicked(MainWindowId, e.Url); - _panes[MainWindowId] = main; + var main = NewPaneControl(MainWindowId); // The connection rail (worlds → characters → windows) sits left of the pane area, joined by // a splitter. RailModel/RailRenderer keep the projection + markup tested; this just hosts it. @@ -1021,6 +1034,18 @@ public string RenderSnapshot(string? view = null) } } + // A live selection over the main window's output. It exists because the selection band is the one + // plane this client invents rather than derives from a pane, and a colour nothing renders is a + // colour nobody checks — this frame is what puts the pair in front of FrameContrastTests and in + // front of a reader. The drag is the real gesture through the real control (SimulatePaneDrag), not + // a highlight painted in by hand, because the thing worth seeing is what a drag actually produces. + if (string.Equals(view, "selection", StringComparison.OrdinalIgnoreCase)) + { + RenderWholeFrame(); // the grid has to be painted before a hit test can land on it + SimulatePaneDrag(MainWindowId, 0, 1, 46, 3); + ReArmWholeFrame(); + } + // Move mode needs a split to have multiple target panes; set it up then arm move mode. if (string.Equals(view, "move", StringComparison.OrdinalIgnoreCase)) { @@ -2752,6 +2777,10 @@ private void RepaintPane(string windowId) return; } + // Every row this pane holds is about to be replaced, so a selection anchored to the old ones has + // nothing left to describe. + ClearPaneSelection(windowId); + if (_freezePoints.TryGetValue(windowId, out var point)) { var split = Math.Clamp(point, 0, buffer.Count); @@ -6604,6 +6633,45 @@ internal bool SimulatePaneClick(string windowId, int x, int y) new List { MouseFlags.Button1Clicked }, local, onWindow, onWindow, _window)); } + /// + /// Drags across a window's output pane, from one cell to another measured from the control's own + /// top-left — press, move, release, the three events a terminal really sends. + /// + /// It exists for the reason does, and it is the same limitation stated + /// for a gesture with more than one event in it: the framework subscribes its driver-mouse handler + /// inside Run(), which no test calls, so nothing reaches a control here unless it is handed + /// over directly. The drag flag rides with the button flag because that is how the SGR + /// encoding arrives — a move with the button still down is Button1Pressed | Button1Dragged, not + /// a bare drag, and a seam that sent the bare form would exercise a path the terminal never produces. + /// + /// + internal void SimulatePaneDrag(string windowId, int fromX, int fromY, int toX, int toY) + { + if (!_panes.TryGetValue(windowId, out var pane)) + { + return; + } + + Send(new List { MouseFlags.Button1Pressed }, fromX, fromY); + Send(new List { MouseFlags.Button1Pressed, MouseFlags.Button1Dragged }, toX, toY); + Send(new List { MouseFlags.Button1Released }, toX, toY); + + void Send(List flags, int x, int y) + { + var local = new System.Drawing.Point(x, y); + var onWindow = new System.Drawing.Point(pane.ActualX + x, pane.ActualY + y); + pane.ProcessMouseEvent(new MouseEventArgs(flags, local, onWindow, onWindow, _window)); + } + } + + /// + /// What a window's pane currently has selected, as the text a copy would put on the clipboard, or + /// empty when nothing is. Reads the control rather than any state of ours, because the selection + /// belongs to the framework and a second record of it would be a second answer. + /// + internal string PaneSelection(string windowId) => + _panes.TryGetValue(windowId, out var pane) && pane.HasSelection ? pane.GetSelectedText() : string.Empty; + /// /// The markup a window's output pane currently holds, one string per row. Internal so a test can read /// a link payload off the pane the app really drew instead of writing the expected one down — the @@ -6774,6 +6842,8 @@ internal bool DispatchCommand(string id) case "term:input2-off": ToggleSecondBar(); return true; + case "term:copy": + return CopyFocusedSelection(); case "term:messages": if (!ComposerIsInTheWay("the client messages")) { @@ -7632,9 +7702,7 @@ private MarkupControl PaneContentFor(string id, string title) return existing; } - var control = new MarkupControl(new List()); - control.LinkClicked += (_, e) => OnLinkClicked(id, e.Url); - _panes[id] = control; + var control = NewPaneControl(id); if (_lines.TryGetValue(id, out var buffer) && buffer.Count > 0) { @@ -7644,6 +7712,133 @@ private MarkupControl PaneContentFor(string id, string title) return control; } + /// + /// Builds a window's output control and records it — the one place a pane's + /// is made. + /// + /// It is a factory rather than two similar blocks because there really are two callers, and they are + /// not interchangeable: the main window's control is built in the constructor, before any + /// workspace exists, while every other one is built on demand. That split is why enabling selection + /// on the on-demand path alone left the main window — the pane most people are looking at — unable to + /// select anything, which is how this trap announced itself. + /// + /// + private MarkupControl NewPaneControl(string windowId) + { + var control = new MarkupControl(new List()); + control.LinkClicked += (_, e) => OnLinkClicked(windowId, e.Url); + EnableSelection(control); + _panes[windowId] = control; + return control; + } + + /// + /// Lets a pane's output be selected with the mouse and copied with ⌃C. + /// + /// The framework already does all of this and ships it switched off — + /// MarkupControl implements ISelectableControl, ICopyableControl and + /// IDragAutoScrollTarget, so drag, double-click word, triple-click line, autoscroll past the + /// pane's edge and the clipboard write (local tool and OSC 52) come from the pinned package. + /// What could not be inherited is the part that is this client's: which colours, and what happens to a + /// selection when the buffer under it moves. + /// + /// + /// Why not leave it to the terminal. Under ?1003 — which this app needs for the wheel, + /// the tab and rail clicks and pane drag-and-drop — a plain drag belongs to the application, and the + /// emulator's escape hatch (⇧-drag in kitty) selects a terminal row. On a vertical split that + /// row spans two panes and the divider between them, and since a pane is narrower than the row a + /// logical line wraps and comes back with newlines injected at the wrap points. The framework's copy + /// walks the painted cells and emits a newline only where a row is not a soft-wrap continuation, which + /// is the thing no terminal selection can do. + /// + /// + /// Rendered, not Source. Source returns the original markup lines — a reader who dragged + /// across a red word would be handed [bold #ff0000] and the tag it closes with. + /// + /// + private void EnableSelection(MarkupControl control) + { + control.EnableSelection = true; + control.CopyMode = MarkupCopyMode.Rendered; + control.SelectionBackgroundColor = ToColor(WorkspacePalette.SelectionBand(_theme)); + control.SelectionForegroundColor = ToColor(WorkspacePalette.SelectionInk(_theme)); + + // The framework's own ⌃C is switched off here and answered by this app instead (see _clipboard). + // Its handler writes straight to the system clipboard through a static helper, which is not + // something a caller can supply — so a test run would replace whatever the developer had copied, + // and the one code path that could not be injected would be the one running under test. Turning it + // off leaves one copy path rather than two. The composer keeps the framework's ⌃C: it is a + // separate modal window with its own key handling, and an editor's copy is the editor's. + control.CopyEnabled = false; + } + + /// + /// Puts the focused window's selected text on the clipboard (⌃C, and ⌃P ▸ Copy the selection). + /// + /// The focused window's, resolved the way everything else in this client resolves a window — + /// a selection lives in one pane at a time and the framework already arbitrates that, so this asks the + /// pane the keyboard is aimed at rather than hunting for whichever control happens to hold one. + /// A frozen pane is asked too: a pane someone has deliberately stopped is the one they are most likely + /// to be copying out of. + /// + /// + /// Both empty cases speak. Nothing selected is the state ⌃C is pressed in by accident; no writer at all + /// is a client that cannot copy, which is a fact about how it was built and must not look like a + /// gesture that failed. + /// + /// + private bool CopyFocusedSelection() + { + var windowId = ActiveWindowId(); + var text = PaneSelection(windowId); + if (text.Length == 0 + && _frozenPanes.TryGetValue(windowId, out var frozen) + && frozen.HasSelection) + { + text = frozen.GetSelectedText(); + } + + if (text.Length == 0) + { + RefuseCommand("nothing selected — drag across a pane's output to select it"); + return true; + } + + if (_clipboard is null) + { + RefuseCommand("no clipboard is configured, so nothing was copied"); + return true; + } + + _clipboard(text); + Notice($"copied {text.Length} characters", MessageSeverity.Info); + return true; + } + + /// + /// Drops any live selection in a pane. Called wherever the buffer under one moves: a chrome row going + /// in or coming out (the freeze bar, the away bar, the NEW divider) and the whole-buffer re-feed + /// behind the timestamp column. + /// + /// The selection is anchored to display rows, and this client mutates buffers mid-stream — so a + /// selection left alone across an insert describes rows that have shifted under it, and the highlight + /// on screen then marks text nobody dragged over. Dropping is the honest answer: a gesture whose + /// subject has moved is a gesture that is over. + /// + /// + private void ClearPaneSelection(string windowId) + { + if (_panes.TryGetValue(windowId, out var pane)) + { + pane.ClearSelection(); + } + + if (_frozenPanes.TryGetValue(windowId, out var frozen)) + { + frozen.ClearSelection(); + } + } + /// /// A window's output as the pane actually shows it: its markup control inside a scroll viewport. /// @@ -8292,6 +8487,7 @@ private MarkupControl FrozenContentFor(string windowId) var control = new MarkupControl(new List()); control.LinkClicked += (_, e) => OnLinkClicked(windowId, e.Url); + EnableSelection(control); // a frozen pane is the one people most want to copy out of _frozenPanes[windowId] = control; return control; } @@ -8991,6 +9187,21 @@ private bool RouteToInput(ConsoleKeyInfo key) return null; } + // ⌃C: copy the focused pane's selection. Here and not in MacroKeys.AppShortcuts, and the + // difference is load-bearing — a global shortcut runs ahead of *every* window, including the + // composer, whose MultilineEditControl has its own ⌃C and is a real editor. This chain belongs + // to the main window alone, so the composer keeps its copy and the panes get theirs. + // + // After DispatchMacro like everything else below it: a macro the user bound to ⌃C wins, which + // is the same relationship ⌃←/→ has with pane selection and is what lets MacroKeys.Verdict go + // on telling the truth about the chord without a special case. + if (e.KeyInfo.Modifiers == ConsoleModifiers.Control && e.KeyInfo.Key == ConsoleKey.C) + { + e.Handled = true; + CopyFocusedSelection(); + return null; + } + // Ctrl+arrows: move between panes, and at the bottom edge into the command lines. Ahead of // both the scrollback keys and recall because it is a workspace gesture rather than a move // inside one, and ahead of the command line because the bars would otherwise eat it — diff --git a/src/SharpMUTerm.Tui/WorkspacePalette.cs b/src/SharpMUTerm.Tui/WorkspacePalette.cs index 5e76eaa..a1f4087 100644 --- a/src/SharpMUTerm.Tui/WorkspacePalette.cs +++ b/src/SharpMUTerm.Tui/WorkspacePalette.cs @@ -286,6 +286,64 @@ internal static Rgb ReadingPlane(Theme theme) return Extreme(PanePlanes(theme), Surface(theme)); } + /// + /// The band a selected run of a pane's output is painted on. + /// + /// One pair per theme, not per pane, and for a reason of its own rather than + /// 's. There it is cost; here it is meaning. A pane's plane says two things + /// already — whose connection this is, in hue, and where the keyboard is, in luminance — and a + /// selection is neither. It is the client answering a gesture the user is making *now*, it exists in + /// exactly one pane at a time, and a highlight that changed colour depending on whose pane it landed in + /// would be reporting a fact nobody asked it about. + /// + /// + /// Built from — the extreme of the fourteen in the direction of travel — + /// pushed further that way, so it clears the whole band rather than the one plane it was + /// derived from, and then leaned toward . The lean is what makes it read as + /// the client's own mark instead of a pane that has gone brighter, and it is the same anchor + /// uses, which is the other place this client says "your keystrokes are + /// about this". + /// + /// + internal static Rgb SelectionBand(Theme theme) + { + ArgumentNullException.ThrowIfNull(theme); + + // Away from the panes, whichever way that is. On a dark theme the reading plane is the brightest of + // the fourteen and the selection goes brighter still; on a light one it is the darkest and the + // selection goes darker. Scaling the wrong way would land the band *inside* the band it has to + // stand clear of — the exact failure Extreme exists to describe, one step further out. + var plane = ReadingPlane(theme); + var dark = Contrast.RelativeLuminance(Surface(theme)) < LightPlaneLuminance; + return Mix(Scale(plane, dark ? SelectionScale : 1.0 / SelectionScale), theme.Prompt, SelectionTint); + } + + /// + /// The one colour every selected cell's text is painted in, held to on + /// . + /// + /// The framework's highlight replaces the foreground as well as the background on every selected cell, + /// so a world's own colours are gone for the duration and this single ink is what all selected output + /// is read in. That makes it the one place in this client where failing the floor would make text + /// unreadable rather than merely quiet. + /// + /// + internal static Rgb SelectionInk(Theme theme) + { + ArgumentNullException.ThrowIfNull(theme); + return Contrast.Legible(theme.Foreground, SelectionBand(theme)); + } + + /// + /// How far past the reading plane the selection band sits. Larger than on + /// purpose: the focus step separates a pane from its neighbours, and a selection has to separate itself + /// from a pane that may already have taken that step. + /// + private const double SelectionScale = 1.9; + + /// How much of the prompt hue the band carries — enough to be recognised, not a wash. + private const double SelectionTint = 0.35; + /// /// The same worst case for the colours the client paints in its own voice, which land on the /// backdrop and on tab chips as well as on panes — so one ink is legible wherever the chrome puts it. diff --git a/tests/SharpMUTerm.Tui.Tests/FrameContrastTests.cs b/tests/SharpMUTerm.Tui.Tests/FrameContrastTests.cs index ece0c9c..c67141c 100644 --- a/tests/SharpMUTerm.Tui.Tests/FrameContrastTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/FrameContrastTests.cs @@ -42,6 +42,11 @@ public class FrameContrastTests "", "freeze", "away", "highlight", "scrollback", "links", "connections", "tint", "tint-input", "characters", "compose", "mssp", "web", "spawn", "split", "menu", "quit", "worlds", "triggers", "logging", "startup", "history", "prefix-panel", "keypad", + + // The selection band is the one plane this client invents rather than derives from a pane, and + // its ink replaces the world's own colour on every selected cell — so it is the pair with the + // most to lose from a theme it was not measured against. + "selection", ]; public static IEnumerable<(string Theme, string View)> Cases() => diff --git a/tests/SharpMUTerm.Tui.Tests/LegiblePaletteTests.cs b/tests/SharpMUTerm.Tui.Tests/LegiblePaletteTests.cs index 1770c14..6868c82 100644 --- a/tests/SharpMUTerm.Tui.Tests/LegiblePaletteTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/LegiblePaletteTests.cs @@ -185,6 +185,64 @@ public async Task TheReadingPlaneIsTheWorstOfTheBandAndNotAMemberPickedByName() } } + /// + /// A selection has to be seen against the pane it is drawn in, and every pane is a different plane. + /// The band is one pair per theme rather than one per pane for 's + /// reason, so it has to stand clear of the whole band, not of the plane it happened to be derived from. + /// + /// The floor here is a fill against a fill and is deliberately not : + /// three to one is where text stops being invisible, and two backgrounds that differed that hard would + /// make a selected line shout. What is held to the text floor is the ink on it, below. + /// + /// + [Test] + public async Task TheSelectionBandStandsClearOfEveryPlaneAPaneCanWear() + { + var failures = new List(); + + foreach (var theme in Themes()) + { + var band = WorkspacePalette.SelectionBand(theme); + foreach (var (name, plane) in Planes(theme)) + { + var ratio = Contrast.Ratio(band, plane); + if (ratio < SelectionSeparation) + { + failures.Add($"{theme.Name}/{name}: {ratio:0.00}:1"); + } + } + } + + await Assert.That(failures).IsEmpty(); + } + + /// + /// The ink painted on that band, held to the text floor — the rule the whole file exists for, + /// applied to the one plane the client invents rather than inherits. It matters more here than + /// elsewhere: the highlight replaces the game's own foreground on every selected cell, so this single + /// colour is what all selected output is read in. + /// + [Test] + public async Task TheSelectionInkIsLegibleOnItsOwnBand() + { + foreach (var theme in Themes()) + { + var band = WorkspacePalette.SelectionBand(theme); + await Assert.That(Contrast.Ratio(WorkspacePalette.SelectionInk(theme), band)) + .IsGreaterThanOrEqualTo(Contrast.Floor) + .Because($"{theme.Name}'s selection ink must be readable on its own band"); + } + } + + /// + /// How far a selection's fill must sit from a pane's own fill to be seen as a band. Below the text + /// floor deliberately (see ) and far + /// enough above 1:1 that it cannot be a rounding artefact. The band as built clears it with room — + /// the tightest cell measured across the three themes is 2.70:1, on Dark's focused untinted pane — + /// so this is the property being asserted rather than the number that happens to hold today. + /// + private const double SelectionSeparation = 1.5; + private static Rgb ParseHex(string hex) => new( Convert.ToByte(hex.Substring(1, 2), 16), Convert.ToByte(hex.Substring(3, 2), 16), diff --git a/tests/SharpMUTerm.Tui.Tests/PaneSelectionTests.cs b/tests/SharpMUTerm.Tui.Tests/PaneSelectionTests.cs new file mode 100644 index 0000000..645a890 --- /dev/null +++ b/tests/SharpMUTerm.Tui.Tests/PaneSelectionTests.cs @@ -0,0 +1,187 @@ +using SharpConsoleUI.Drivers; +using SharpMUTerm.Graphics; + +namespace SharpMUTerm.Tui.Tests; + +/// +/// Selecting a pane's output with the mouse and copying it. +/// +/// The terminal's own selection cannot do this job: it selects a terminal row, so on a vertical +/// split a drag returns the left pane's text, the divider and the right pane's unrelated output +/// concatenated — and a pane is narrower than the row, so a logical line wraps and comes back with hard +/// newlines injected at the wrap points. Both are UrlDetector's problem one layer over: the +/// decision has to be made where the pane's line is known to end. +/// +/// +/// Every gesture here goes through the control's real ProcessMouseEvent, because the framework +/// only registers its driver-mouse handler inside Run() — which no test calls. That is the same +/// limitation SimulatePaneClick documents, and the reason a drag needs a seam of its own. +/// +/// +/// Serialised: rendering redirects the process-global Console.Out. +[NotInParallel] +public class PaneSelectionTests +{ + private const int Width = 120; + private const int Height = 32; + + private static readonly TerminalCapabilities Headless = + new(GraphicsProtocol.None, supportsTrueColor: true, supportsKittyGraphics: false, supportsSixel: false); + + private static SharpMUTermApp Demo() + { + Console.SetIn(TextReader.Null); + return new SharpMUTermApp(DemoScene.Build(), Headless, new HeadlessConsoleDriver(Width, Height)); + } + + /// The main window, rendered, with a known line of output to drag across. + private static SharpMUTermApp Rendered() + { + var app = Demo(); + app.RenderSnapshot(); + return app; + } + + /// + /// The same, with somewhere for a copy to land. The writer is caller-supplied and null by default — + /// the save/logRoot/browser-launcher family — so a test that does not ask for one + /// provably leaves the real system clipboard alone. + /// + private static (SharpMUTermApp App, List Copied) WithClipboard() + { + Console.SetIn(TextReader.Null); + var copied = new List(); + var app = new SharpMUTermApp( + DemoScene.Build(), Headless, new HeadlessConsoleDriver(Width, Height), clipboard: copied.Add); + app.RenderSnapshot(); + return (app, copied); + } + + private static ConsoleKeyInfo CtrlC => + new('\0', ConsoleKey.C, shift: false, alt: false, control: true); + + [Test] + public async Task CtrlCCopiesWhatWasSelected() + { + var (app, copied) = WithClipboard(); + app.SimulatePaneDrag(SharpMUTermApp.MainWindowId, 0, 0, 25, 1); + + app.SimulateKey(CtrlC); + + await Assert.That(copied).HasSingleItem(); + await Assert.That(copied[0]).IsEqualTo(app.PaneSelection(SharpMUTermApp.MainWindowId)); + } + + /// Nothing selected is not an error, but it is not silence either. + [Test] + public async Task CtrlCWithNothingSelectedSaysSo() + { + var (app, copied) = WithClipboard(); + + app.SimulateKey(CtrlC); + + await Assert.That(copied).IsEmpty(); + await Assert.That(app.StatusMarkup).Contains("nothing selected"); + } + + /// + /// The family rule, asserted rather than assumed: an app given no writer copies nowhere and says so. + /// A test that quietly reached the real clipboard would replace whatever the developer had on it. + /// + [Test] + public async Task AnAppWithNoClipboardWriterCopiesNothingAndSaysSo() + { + var app = Rendered(); + app.SimulatePaneDrag(SharpMUTermApp.MainWindowId, 0, 0, 25, 1); + + app.SimulateKey(CtrlC); + + await Assert.That(app.StatusMarkup).Contains("no clipboard"); + } + + /// The ⌃P entry and the chord are one action, and the entry is how the chord is found at all. + [Test] + public async Task TheCommandSurfaceCopiesTheSameText() + { + var (app, copied) = WithClipboard(); + app.SimulatePaneDrag(SharpMUTermApp.MainWindowId, 0, 0, 25, 1); + + await Assert.That(app.DispatchCommand("term:copy")).IsTrue(); + + await Assert.That(copied).HasSingleItem(); + } + + [Test] + public async Task DraggingAcrossAPaneSelectsTheTextUnderThePointer() + { + var app = Rendered(); + + app.SimulatePaneDrag(SharpMUTermApp.MainWindowId, 0, 0, 12, 0); + + await Assert.That(app.PaneSelection(SharpMUTermApp.MainWindowId)).IsNotEmpty(); + } + + /// + /// What is copied is what the pane shows, not the markup behind it. Source mode would + /// hand back [bold #ff0000]…[/], which is not what anyone dragged over. + /// + [Test] + public async Task TheSelectedTextCarriesNoMarkup() + { + var app = Rendered(); + + app.SimulatePaneDrag(SharpMUTermApp.MainWindowId, 0, 0, 40, 2); + var selected = app.PaneSelection(SharpMUTermApp.MainWindowId); + + await Assert.That(selected).DoesNotContain("[/]"); + await Assert.That(selected).DoesNotContain("[bold"); + } + + /// + /// A pane with nothing selected reports nothing — the state every pane is in until a drag happens, and + /// the one the copy shortcut must not fire in. + /// + [Test] + public async Task AFreshPaneHasNoSelection() + { + var app = Rendered(); + + await Assert.That(app.PaneSelection(SharpMUTermApp.MainWindowId)).IsEmpty(); + } + + /// + /// The pin that stops this being "improved" into something that spends a cell. A selection recolours + /// cells that are already painted; if it ever gained a gutter or a marker column the pane rectangle + /// would change, and per-pane NAWS is derived from that rectangle — so dragging in a pane would + /// announce a new terminal size to every connected server and reflow the game's own output. + /// + [Test] + public async Task SelectingTextMovesNoPaneRectangle() + { + var app = Rendered(); + var before = app.PaneOutputRects(); + + app.SimulatePaneDrag(SharpMUTermApp.MainWindowId, 0, 0, 30, 3); + app.RenderWholeFrame(); + + await Assert.That(app.PaneOutputRects()).IsEquivalentTo(before); + } + + /// + /// A buffer that shifts under a live selection leaves it pointing at rows that have moved — the client + /// inserts and removes chrome rows mid-buffer (the freeze bar, the away bar, the NEW divider) + /// and repaints whole buffers when the timestamp column is toggled. The selection is dropped on those + /// paths rather than left to describe a stale grid. + /// + [Test] + public async Task RepaintingAPaneDropsASelectionThatWouldNowPointAtTheWrongRows() + { + var app = Rendered(); + app.SimulatePaneDrag(SharpMUTermApp.MainWindowId, 0, 0, 20, 1); + await Assert.That(app.PaneSelection(SharpMUTermApp.MainWindowId)).IsNotEmpty(); + + app.DispatchCommand("term:timestamps-on"); // the whole-buffer re-feed + + await Assert.That(app.PaneSelection(SharpMUTermApp.MainWindowId)).IsEmpty(); + } +} From c26538d6ef0c390fe2a60ee039025d5d83944bbe Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Thu, 13 Aug 2026 23:08:34 -0500 Subject: [PATCH 5/5] fix(panes): copy the selection that exists, and drop it wherever rows move MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defects from review, both real and both confirmed by a test that failed first. The copy asked the *focused pane*. Pane selection moves on ⌃arrows, ⌃O and a tab click, and a press in a pane's body moves none of them — so a drag in the pane beside the focused one left ⌃C looking elsewhere and reporting "nothing selected", which reads as a feature that does not work. It asks the window's SelectionManager now, which owns the one active selection and clears the previous owner when a new one starts. That also retires the special case for a frozen pane: one selection, one owner. The clear was at RepaintPane, which is not the only thing that re-feeds a pane — BuildFrozenContent feeds both halves and ToggleFreeze's thaw branch pours the whole buffer back. It moves to FeedRange, the one function that actually replaces a control's content. MarkupControl.SetContent does not clear a selection; only its append path does, so this cannot be left to the framework. The thaw is the case with teeth: freezing leaves the live control empty so a stale anchor yields nothing, while after a thaw the rows exist again and the new test copied "The Grand Plaza…" before the fix. PaneWindows() is added because the first cut of the non-focused-pane test passed a *pane* id to a seam that takes a *window* id and selected nothing — indistinguishable from the bug it was written to catch. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN --- CLAUDE.md | 23 ++++- src/SharpMUTerm.Tui/SharpMUTermApp.cs | 89 +++++++++---------- .../PaneSelectionTests.cs | 52 +++++++++++ 3 files changed, 115 insertions(+), 49 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index f697e34..9f9200c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -170,10 +170,25 @@ fallbacks) for inline images/maps. runs ahead of *every* window including the composer, whose `MultilineEditControl` has its own ⌃C and is a real editor. Being in the chain also puts it after `DispatchMacro`, so a macro bound to ⌃C wins — the same relationship ⌃←/→ has with pane selection, and the reason `Verdict` needs no special case. - - **A selection is dropped whenever the rows under it move**, in `RepaintPane` — the one seam that - re-feeds a pane. Chrome rows go in and out mid-buffer (the freeze bar, the away bar, `NEW`, the search - bar) and the timestamp toggle re-feeds whole buffers; a selection is anchored to display rows, so one - left alone across an insert highlights text nobody dragged over. + - **A selection is dropped whenever the rows under it move, and the clear belongs in `FeedRange`** — the + one function that actually replaces a pane control's content. `RepaintPane` is *not* the only caller: + `BuildFrozenContent` feeds both halves of a frozen pane and `ToggleFreeze`'s thaw branch pours the whole + buffer back, so clearing at the repaint site alone left freeze and thaw re-feeding under a live + selection. The thaw is the case with teeth — freezing leaves the live control empty, so a stale anchor + merely yields nothing, while after a thaw the rows exist again and ⌃C hands over real text nobody + dragged across (`FreezingAndThawingDropsTheSelectionRatherThanReAnchoringIt` copied `The Grand Plaza…` + before this moved). **`MarkupControl.SetContent` does not clear a selection** — only its append path + does (`OnContentAppended`) — so this cannot be left to the framework. + - **The copy asks the window's `SelectionManager`, never the focused pane.** That manager owns the one + active selection and clears the previous owner when a new one starts. The focused pane is the wrong + question: pane selection moves on ⌃arrows, ⌃O and a tab click, and a press in a pane's *body* moves + none of them — so a drag in the pane beside the focused one left the copy looking elsewhere and + reporting `nothing selected`, which reads as a feature that does not work. It also retires the special + case for a frozen pane: one selection, one owner, whichever control that is. + - **A pane id and a window id are different namespaces that are both strings**, and this is where that + bites: `PaneOutputRects` is keyed by *pane*, while `SimulatePaneDrag` and `PaneSelection` take a + *window*. `PaneWindows()` maps between them, and exists because a test that passed a pane id to the + drag seam selected nothing — indistinguishable from the feature being broken. - **`NewPaneControl` is the one place a pane's control is made**, and it exists because enabling selection on `PaneContentFor` alone left the **main** window — the pane most people are looking at — unable to select anything. That control is built in the constructor, before a workspace exists; every diff --git a/src/SharpMUTerm.Tui/SharpMUTermApp.cs b/src/SharpMUTerm.Tui/SharpMUTermApp.cs index 9db621b..b2ab11f 100644 --- a/src/SharpMUTerm.Tui/SharpMUTermApp.cs +++ b/src/SharpMUTerm.Tui/SharpMUTermApp.cs @@ -2703,6 +2703,16 @@ private void FeedRange(MarkupControl control, List buffer, int from, i markup.Add(Compose(buffer[start + i])); } + // Every row this control holds is being replaced, so a selection anchored to the old ones has + // nothing left to describe. Here rather than at the call sites because this is the one function + // that replaces a pane control's content — RepaintPane is not the only caller, and clearing there + // left freeze and thaw re-feeding under a live selection. It matters most on the way *back*: + // freezing leaves the live control empty, so a stale anchor merely yields nothing, but after a + // thaw the rows exist again and it hands over real text nobody dragged across. + // + // MarkupControl.SetContent does not do this itself — only its append path does + // (OnContentAppended) — so it cannot be left to the framework. + control.ClearSelection(); control.SetContent(markup); } @@ -2777,10 +2787,6 @@ private void RepaintPane(string windowId) return; } - // Every row this pane holds is about to be replaced, so a selection anchored to the old ones has - // nothing left to describe. - ClearPaneSelection(windowId); - if (_freezePoints.TryGetValue(windowId, out var point)) { var split = Math.Clamp(point, 0, buffer.Count); @@ -6843,7 +6849,7 @@ internal bool DispatchCommand(string id) ToggleSecondBar(); return true; case "term:copy": - return CopyFocusedSelection(); + return CopySelection(); case "term:messages": if (!ComposerIsInTheWay("the client messages")) { @@ -7773,13 +7779,15 @@ private void EnableSelection(MarkupControl control) } /// - /// Puts the focused window's selected text on the clipboard (⌃C, and ⌃P ▸ Copy the selection). + /// Puts the selected text on the clipboard (⌃C, and ⌃P ▸ Copy the selection). /// - /// The focused window's, resolved the way everything else in this client resolves a window — - /// a selection lives in one pane at a time and the framework already arbitrates that, so this asks the - /// pane the keyboard is aimed at rather than hunting for whichever control happens to hold one. - /// A frozen pane is asked too: a pane someone has deliberately stopped is the one they are most likely - /// to be copying out of. + /// It asks the window's SelectionManager, not the focused pane. That manager owns the one + /// active selection and clears the previous owner when a new one starts, so it is the thing that knows + /// — and the focused pane is emphatically not. Pane selection moves on ⌃arrows, ⌃O and a tab click, and + /// a press in a pane's body moves none of them, so a drag in the pane beside the focused one + /// left the copy looking in the wrong place and reporting nothing selected — a feature that appeared + /// simply not to work. It also retires the special case for a frozen pane: one selection, one owner, + /// whichever control that turns out to be. /// /// /// Both empty cases speak. Nothing selected is the state ⌃C is pressed in by accident; no writer at all @@ -7787,16 +7795,9 @@ private void EnableSelection(MarkupControl control) /// gesture that failed. /// /// - private bool CopyFocusedSelection() + private bool CopySelection() { - var windowId = ActiveWindowId(); - var text = PaneSelection(windowId); - if (text.Length == 0 - && _frozenPanes.TryGetValue(windowId, out var frozen) - && frozen.HasSelection) - { - text = frozen.GetSelectedText(); - } + var text = _window.SelectionManager.GetSelectedText() ?? string.Empty; if (text.Length == 0) { @@ -7815,30 +7816,6 @@ private bool CopyFocusedSelection() return true; } - /// - /// Drops any live selection in a pane. Called wherever the buffer under one moves: a chrome row going - /// in or coming out (the freeze bar, the away bar, the NEW divider) and the whole-buffer re-feed - /// behind the timestamp column. - /// - /// The selection is anchored to display rows, and this client mutates buffers mid-stream — so a - /// selection left alone across an insert describes rows that have shifted under it, and the highlight - /// on screen then marks text nobody dragged over. Dropping is the honest answer: a gesture whose - /// subject has moved is a gesture that is over. - /// - /// - private void ClearPaneSelection(string windowId) - { - if (_panes.TryGetValue(windowId, out var pane)) - { - pane.ClearSelection(); - } - - if (_frozenPanes.TryGetValue(windowId, out var frozen)) - { - frozen.ClearSelection(); - } - } - /// /// A window's output as the pane actually shows it: its markup control inside a scroll viewport. /// @@ -9198,7 +9175,7 @@ private bool RouteToInput(ConsoleKeyInfo key) if (e.KeyInfo.Modifiers == ConsoleModifiers.Control && e.KeyInfo.Key == ConsoleKey.C) { e.Handled = true; - CopyFocusedSelection(); + CopySelection(); return null; } @@ -9793,6 +9770,28 @@ internal PaneDragSurface PaneSnapshot() /// the claim that the reported rows exclude the chrome. /// /// + /// + /// The window in front of each realised pane, keyed by pane id. It exists because those two + /// ids are different namespaces that are both strings, and a caller holding one of them cannot use it + /// where the other is wanted: is keyed by pane, while + /// and take a window. A test that passed a + /// pane id to the drag seam simply selected nothing, which reads as a broken feature rather than a + /// mistyped argument. + /// + internal IReadOnlyDictionary PaneWindows() + { + var windows = new Dictionary(StringComparer.Ordinal); + foreach (var (paneId, _, _) in RealisedPanes()) + { + if (_workspace.Layout.FindPane(paneId)?.ActiveTab is { } windowId) + { + windows[paneId] = windowId; + } + } + + return windows; + } + internal IReadOnlyDictionary PaneOutputRects() { var rects = new Dictionary(StringComparer.Ordinal); diff --git a/tests/SharpMUTerm.Tui.Tests/PaneSelectionTests.cs b/tests/SharpMUTerm.Tui.Tests/PaneSelectionTests.cs index 645a890..1e41ec9 100644 --- a/tests/SharpMUTerm.Tui.Tests/PaneSelectionTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/PaneSelectionTests.cs @@ -99,6 +99,58 @@ public async Task AnAppWithNoClipboardWriterCopiesNothingAndSaysSo() await Assert.That(app.StatusMarkup).Contains("no clipboard"); } + /// + /// A drag in a pane the keyboard is not aimed at still copies. Pane selection is moved by + /// ⌃arrows, ⌃O and a tab click — never by a press in a pane's body — so a copy resolved through + /// ActiveWindowId looked in the wrong pane and reported nothing selected, which is the shape of + /// a feature that does not work. The framework's SelectionManager already arbitrates one + /// selection per window; asking it is asking the thing that knows. + /// + [Test] + public async Task ADragInAPaneThatDoesNotHoldTheFocusIsStillWhatGetsCopied() + { + var (app, copied) = WithClipboard(); + app.RenderSnapshot("split"); + + // By window, not by pane: the two are separate id namespaces and the drag seam takes a window. + var elsewhere = app.PaneWindows().Values.First(id => id != app.ActiveWindowId()); + + app.SimulatePaneDrag(elsewhere, 0, 0, 20, 1); + app.SimulateKey(CtrlC); + + await Assert.That(app.ActiveWindowId()).IsNotEqualTo(elsewhere); + await Assert.That(copied).HasSingleItem(); + await Assert.That(copied[0]).IsEqualTo(app.PaneSelection(elsewhere)); + } + + /// + /// Freezing rebuilds the pane into a pinned half and a live half and re-feeds both, so a selection + /// anchored to the rows before the split describes a grid that no longer exists. It is a second + /// re-feed seam beside RepaintPane, which is why the clearing lives in FeedRange — the + /// one function that actually replaces a control's content — rather than at the call sites. + /// + /// + /// Freeze is a second re-feed seam beside RepaintPane — it rebuilds the pane into a pinned half + /// and a live half and feeds both — and unfreezing pours the whole buffer back. That round trip is the + /// one with teeth: freezing alone leaves the live control empty, so a stale anchor yields + /// nothing and the client refuses for the wrong reason, while after an unfreeze the rows exist again + /// and a stale anchor hands over real text nobody dragged across. + /// + [Test] + public async Task FreezingAndThawingDropsTheSelectionRatherThanReAnchoringIt() + { + var (app, copied) = WithClipboard(); + app.SimulatePaneDrag(SharpMUTermApp.MainWindowId, 0, 0, 20, 1); + await Assert.That(app.PaneSelection(SharpMUTermApp.MainWindowId)).IsNotEmpty(); + + app.DispatchCommand("term:freeze"); + app.DispatchCommand("term:unfreeze"); + app.SimulateKey(CtrlC); + + await Assert.That(copied).IsEmpty(); + await Assert.That(app.StatusMarkup).Contains("nothing selected"); + } + /// The ⌃P entry and the chord are one action, and the entry is how the chord is found at all. [Test] public async Task TheCommandSurfaceCopiesTheSameText()