From 80d59de7719b6c86059a375698179e7f018970eb Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Wed, 14 Oct 2026 21:00:36 +1100 Subject: [PATCH] Take the kernel from PlatformHeads.Core SessionHost, ScreenCache, the window contract, WindowPlacement and NativeResolver were written here and copied into BuildMonitor, and the copies drifted: each held fixes the other lacked. PlatformHeads 0.2.0 has one copy of them, made from both, which BuildMonitor took in SimonCropp/BuildMonitor#53, and this takes it: - SessionHost.cs, ScreenCache.cs, WindowPlacement.cs and NativeResolver.cs go. Directory.Build.props names SessionHost, ScreenCache and OpenWindow for the viewer's state, screen and window, and DiffEngineViewer.Benchmarks.csproj again, since it imports nothing above it. - IViewerWindow derives from IHeadWindow. Deview.Resolve registers the library by name. - SessionHost now throws when a mutation mutates again, which silently undid the outer one. Nothing here does: the whole suite passes. And one fix that comes with it. Copying handed the text to the window from inside the mutation, and the WinForms head swallowed the refusal of a clipboard another process held, so the copy was lost while the status line said it had been made. A copy now waits in SessionState.Clipboard, and the loop offers it through the kernel's ClipboardPump for a few frames, then says it could not copy. SetClipboard answers whether it took the text; the native heads always do. The packages carry PlatformHeads.Core.dll beside each head, which is the one change to the entries Packaging.Tests has on record. All four pins move to 0.2.0 together; nothing in the build package, the CMake module or the workflow changed. --- .github/workflows/build-native.yml | 2 +- claude.md | 17 ++- native/CMakeLists.txt | 2 +- .../TrayViewerSyncTest.cs | 2 +- .../DiffEngineViewer.Benchmarks.csproj | 7 + .../FrameBenchmarks.cs | 4 +- .../GroupAcceptBenchmarks.cs | 5 +- src/DiffEngineViewer.Benchmarks/NativeHead.cs | 2 +- .../SelectionFrameBenchmarks.cs | 4 +- src/DiffEngineViewer.Linux/Program.cs | 2 +- src/DiffEngineViewer.Mac/Program.cs | 2 +- .../AcceptAllProgressTests.cs | 5 +- .../AttachedViewerTests.cs | 5 +- .../ClipboardCopyTests.cs | 136 ++++++++++++++++++ .../DerivedAcceptTests.cs | 5 +- .../DiscardBatchTests.cs | 5 +- .../DrawingViewTests.cs | 5 +- src/DiffEngineViewer.Tests/EntryMenuTests.cs | 5 +- .../GroupAcceptBatchTests.cs | 5 +- .../MapProjectionTests.cs | 5 +- .../MinimalViewTests.cs | 5 +- .../ModuleInitializer.cs | 2 +- src/DiffEngineViewer.Tests/NativeTests.cs | 4 +- src/DiffEngineViewer.Tests/PaneMenuTests.cs | 10 +- .../ScreenCacheTests.cs | 27 ++-- src/DiffEngineViewer.Tests/SelectionTests.cs | 31 ++-- .../ViewerProgramTests.cs | 19 ++- src/DiffEngineViewer.Tests/ZoomTests.cs | 5 +- .../FormsHeadTests.cs | 5 +- .../FormsViewerWindow.cs | 9 +- src/DiffEngineViewer/ClipboardCopy.cs | 47 ++++++ src/DiffEngineViewer/DiffEngineViewer.csproj | 1 + src/DiffEngineViewer/IViewerWindow.cs | 65 +-------- src/DiffEngineViewer/Native/Deview.cs | 31 ++-- src/DiffEngineViewer/Native/NativeResolver.cs | 136 ------------------ .../Native/NativeViewerWindow.cs | 9 +- src/DiffEngineViewer/PendingCopy.cs | 13 ++ src/DiffEngineViewer/ScreenCache.cs | 36 ----- src/DiffEngineViewer/SessionHost.cs | 34 ----- src/DiffEngineViewer/SessionState.cs | 8 ++ src/DiffEngineViewer/ViewerProgram.cs | 20 ++- src/DiffEngineViewer/WindowPlacement.cs | 60 -------- src/Directory.Build.props | 14 ++ src/Directory.Packages.props | 3 +- .../Package.DiffEngine.verified.txt | 8 +- .../Package.DiffEngineTray.verified.txt | 4 +- ...ackage.DiffEngineViewer.Linux.verified.txt | 1 + .../Package.DiffEngineViewer.Mac.verified.txt | 1 + ...kage.DiffEngineViewer.Windows.verified.txt | 1 + 49 files changed, 381 insertions(+), 453 deletions(-) create mode 100644 src/DiffEngineViewer.Tests/ClipboardCopyTests.cs create mode 100644 src/DiffEngineViewer/ClipboardCopy.cs delete mode 100644 src/DiffEngineViewer/Native/NativeResolver.cs create mode 100644 src/DiffEngineViewer/PendingCopy.cs delete mode 100644 src/DiffEngineViewer/ScreenCache.cs delete mode 100644 src/DiffEngineViewer/SessionHost.cs delete mode 100644 src/DiffEngineViewer/WindowPlacement.cs diff --git a/.github/workflows/build-native.yml b/.github/workflows/build-native.yml index 120027cc5..b1f3dab9a 100644 --- a/.github/workflows/build-native.yml +++ b/.github/workflows/build-native.yml @@ -35,7 +35,7 @@ concurrency: jobs: native: - uses: SimonCropp/PlatformHeads/.github/workflows/build-native.yml@0.1.0 + uses: SimonCropp/PlatformHeads/.github/workflows/build-native.yml@0.2.0 permissions: contents: write pull-requests: write diff --git a/claude.md b/claude.md index cd49eaa69..3d3b43534 100644 --- a/claude.md +++ b/claude.md @@ -217,6 +217,13 @@ the comment there about not caching "nothing staged" asks for. structure, which is what makes the text snapshots meaningful and keeps three renderers honest. - `ViewerProgram.Run(args, OpenWindow)` owns the loop for all heads. A head is a `Main` that chooses a renderer; nothing else about the app is per platform. +- `SessionHost`, `ScreenCache`, `IViewerWindow`'s base `IHeadWindow`, `OpenWindow`, + `ClipboardPump` (driven by `ClipboardCopy`), `WindowPlacement` and `NativeResolver` are + PlatformHeads.Core's, the kernel BuildMonitor shares, in the `PlatformHeads` namespace; + `Directory.Build.props` names the generic ones for the viewer's state and screen, and + `DiffEngineViewer.Benchmarks.csproj` again, since it imports nothing above it. A fix to any of + them goes there. `SessionHost.Mutate` throws when a mutation mutates again, which silently undid + the outer one. - The loop presents sixty times a second and builds a screen only when the state is another one. `ScreenCache` keeps the last `SessionState` and its `Screen`: a state is immutable and only ever replaced, so the same reference is the same screen, and building one a frame was the whole @@ -502,7 +509,10 @@ the comment there about not caching "nothing staged" asks for. which the three pixel heads draw and `AsciiRenderer` cannot - a character grid has no way to invert part of a line without changing its width - so what the model universally states about a selection goes in the **status line**, where every renderer draws it, and the highlight is the - enrichment on top. Copying is `IViewerWindow.SetClipboard` rather than a `ViewerActions` member, + enrichment on top. A copy waits in `SessionState.Clipboard` and the loop offers it to the + window through `ClipboardPump` for a few frames, since a Windows clipboard another process holds + refuses a set outright, and handed over once from inside the mutation it was lost while the + status said it had been copied. Copying is `IViewerWindow.SetClipboard` rather than a `ViewerActions` member, because a clipboard belongs to a toolkit the way a window does, and it is answered before the owner link: the text is already in this process. - Columns are cells of `CellGrid`, decided in the model rather than by any head's fonts: a wide @@ -605,9 +615,10 @@ the comment there about not caching "nothing staged" asks for. **Native shim (`native/`), used by the Mac and Linux heads only:** - The build and packaging of the heads is PlatformHeads' (`D:\Code\PlatformHeads`, github.com/SimonCropp/PlatformHeads), which BuildMonitor shares: it was worked out here and - copied there, and fixes made here did not reach the copy. Three pins name one tag of it and move + copied there, and fixes made here did not reach the copy. Four pins name one tag of it and move together: the `uses:` of `.github/workflows/build-native.yml`, the `GIT_TAG` in - `native/CMakeLists.txt`, and `PlatformHeads` in `Directory.Packages.props`. The workflow is its + `native/CMakeLists.txt`, and `PlatformHeads` and `PlatformHeads.Core` in + `Directory.Packages.props`. The workflow is its reusable `build-native`. Its CMake module pins raylib and Dear ImGui, carries the raylib 6.0 option workarounds and links the library, and `native/CMakeLists.txt` names only the picture formats, the source file and the two libraries the viewer links beyond raylib. Its package's diff --git a/native/CMakeLists.txt b/native/CMakeLists.txt index 4966f2334..b1847e7a3 100644 --- a/native/CMakeLists.txt +++ b/native/CMakeLists.txt @@ -15,7 +15,7 @@ include(FetchContent) FetchContent_Declare( platformheads GIT_REPOSITORY https://github.com/SimonCropp/PlatformHeads.git - GIT_TAG 0.1.0 + GIT_TAG 0.2.0 GIT_SHALLOW ON) FetchContent_MakeAvailable(platformheads) include(${platformheads_SOURCE_DIR}/cmake/PlatformHeads.cmake) diff --git a/src/DiffEngineTray.Tests/TrayViewerSyncTest.cs b/src/DiffEngineTray.Tests/TrayViewerSyncTest.cs index dd4395896..be312324b 100644 --- a/src/DiffEngineTray.Tests/TrayViewerSyncTest.cs +++ b/src/DiffEngineTray.Tests/TrayViewerSyncTest.cs @@ -14,7 +14,7 @@ using CommandKind = viewer::CommandKind; using OwnerLink = viewer::OwnerLink; using QueueProjection = viewer::QueueProjection; -using SessionHost = viewer::SessionHost; +using SessionHost = PlatformHeads.SessionHost; using SessionMessageHandler = viewer::MessageHandler; using SessionState = viewer::SessionState; using ViewerActions = viewer::ViewerActions; diff --git a/src/DiffEngineViewer.Benchmarks/DiffEngineViewer.Benchmarks.csproj b/src/DiffEngineViewer.Benchmarks/DiffEngineViewer.Benchmarks.csproj index b085457d6..a7ca07113 100644 --- a/src/DiffEngineViewer.Benchmarks/DiffEngineViewer.Benchmarks.csproj +++ b/src/DiffEngineViewer.Benchmarks/DiffEngineViewer.Benchmarks.csproj @@ -24,6 +24,13 @@ them ambiguous. --> + + + + + diff --git a/src/DiffEngineViewer.Benchmarks/FrameBenchmarks.cs b/src/DiffEngineViewer.Benchmarks/FrameBenchmarks.cs index 0031b4d77..8052122e6 100644 --- a/src/DiffEngineViewer.Benchmarks/FrameBenchmarks.cs +++ b/src/DiffEngineViewer.Benchmarks/FrameBenchmarks.cs @@ -34,7 +34,7 @@ public void Setup() state = built; } - readonly ScreenCache screens = new(); + readonly ScreenCache screens = new(ScreenBuilder.Build); // One screen from one state: what every frame paid, and what a change to the state that leaves // the queue the list it was still does. A scroll and a drag are that, every frame they last. @@ -52,5 +52,5 @@ public object BuildScreenOfAChangedQueue() => // whenever the state is the one the last frame had. [Benchmark] public object FrameWithNothingChanged() => - screens.For(state); + screens.Get(state); } diff --git a/src/DiffEngineViewer.Benchmarks/GroupAcceptBenchmarks.cs b/src/DiffEngineViewer.Benchmarks/GroupAcceptBenchmarks.cs index b766b6763..bb4b48622 100644 --- a/src/DiffEngineViewer.Benchmarks/GroupAcceptBenchmarks.cs +++ b/src/DiffEngineViewer.Benchmarks/GroupAcceptBenchmarks.cs @@ -129,9 +129,8 @@ public void SetHidden(bool hidden) { } - public void SetClipboard(string text) - { - } + public bool SetClipboard(string text) => + true; public void Focus() { diff --git a/src/DiffEngineViewer.Benchmarks/NativeHead.cs b/src/DiffEngineViewer.Benchmarks/NativeHead.cs index ec017e064..326252735 100644 --- a/src/DiffEngineViewer.Benchmarks/NativeHead.cs +++ b/src/DiffEngineViewer.Benchmarks/NativeHead.cs @@ -185,7 +185,7 @@ static bool TryFindShim([NotNullWhen(true)] out string? path) return true; } - if (NativeResolver.TryFind(out path)) + if (NativeResolver.TryFind(Deview.Library, out path)) { shimPath = path; return true; diff --git a/src/DiffEngineViewer.Benchmarks/SelectionFrameBenchmarks.cs b/src/DiffEngineViewer.Benchmarks/SelectionFrameBenchmarks.cs index b9c900d5f..e4460230b 100644 --- a/src/DiffEngineViewer.Benchmarks/SelectionFrameBenchmarks.cs +++ b/src/DiffEngineViewer.Benchmarks/SelectionFrameBenchmarks.cs @@ -30,7 +30,7 @@ public void Setup() } } - readonly ScreenCache screens = new(); + readonly ScreenCache screens = new(ScreenBuilder.Build); // What a frame of a drag still pays, since each one moves the selection's end. [Benchmark(Baseline = true)] @@ -40,7 +40,7 @@ public object BuildScreen() => // And what every frame after ctrl+a pays, for as long as the selection stands. [Benchmark] public object FrameWithNothingChanged() => - screens.For(selected); + screens.Get(selected); static string Text(int lines, int changeEvery) { diff --git a/src/DiffEngineViewer.Linux/Program.cs b/src/DiffEngineViewer.Linux/Program.cs index f322f3914..028a57ba2 100644 --- a/src/DiffEngineViewer.Linux/Program.cs +++ b/src/DiffEngineViewer.Linux/Program.cs @@ -4,7 +4,7 @@ static int Main(string[] args) { // Registered before anything can trigger a load. Only the heads that P/Invoke need this, // so it lives with the renderer choice rather than in ViewerProgram. - NativeResolver.Register(); + Deview.Resolve(); return ViewerProgram.Run(args, NativeViewerWindow.Open); } } diff --git a/src/DiffEngineViewer.Mac/Program.cs b/src/DiffEngineViewer.Mac/Program.cs index f322f3914..028a57ba2 100644 --- a/src/DiffEngineViewer.Mac/Program.cs +++ b/src/DiffEngineViewer.Mac/Program.cs @@ -4,7 +4,7 @@ static int Main(string[] args) { // Registered before anything can trigger a load. Only the heads that P/Invoke need this, // so it lives with the renderer choice rather than in ViewerProgram. - NativeResolver.Register(); + Deview.Resolve(); return ViewerProgram.Run(args, NativeViewerWindow.Open); } } diff --git a/src/DiffEngineViewer.Tests/AcceptAllProgressTests.cs b/src/DiffEngineViewer.Tests/AcceptAllProgressTests.cs index 8c593093e..c9b9feb8d 100644 --- a/src/DiffEngineViewer.Tests/AcceptAllProgressTests.cs +++ b/src/DiffEngineViewer.Tests/AcceptAllProgressTests.cs @@ -391,9 +391,8 @@ public void Focus() { } - public void SetClipboard(string text) - { - } + public bool SetClipboard(string text) => + true; public bool Capture(Screen screen, int width, int height, string pngPath) => false; diff --git a/src/DiffEngineViewer.Tests/AttachedViewerTests.cs b/src/DiffEngineViewer.Tests/AttachedViewerTests.cs index fa04d6fd4..253642b48 100644 --- a/src/DiffEngineViewer.Tests/AttachedViewerTests.cs +++ b/src/DiffEngineViewer.Tests/AttachedViewerTests.cs @@ -697,9 +697,8 @@ public void Focus() { } - public void SetClipboard(string text) - { - } + public bool SetClipboard(string text) => + true; public bool Capture(Screen screen, int width, int height, string pngPath) => false; diff --git a/src/DiffEngineViewer.Tests/ClipboardCopyTests.cs b/src/DiffEngineViewer.Tests/ClipboardCopyTests.cs new file mode 100644 index 000000000..7b847bc88 --- /dev/null +++ b/src/DiffEngineViewer.Tests/ClipboardCopyTests.cs @@ -0,0 +1,136 @@ +/// +/// A copy waits in the state until the window takes it, and the status says so when it never does. +/// The window was handed the text inside the copy's mutation, and a clipboard another process held +/// lost it while the status said it had been copied. +/// +public class ClipboardCopyTests +{ + [Test] + public async Task ACopyTheWindowTakesIsLetGo() + { + var window = new Desktop(); + var host = Copied(out var copy); + + ClipboardCopy.Offer(new(), host, window, copy); + + await Assert.That(window.Taken).IsEquivalentTo(["the text"]); + await Assert.That(host.State.Clipboard).IsNull(); + await Assert.That(host.State.Message).IsEqualTo("Copied 1 line from the selection."); + } + + [Test] + public async Task ABusyClipboardKeepsTheCopyForTheNextFrame() + { + var window = new Desktop { Busy = true }; + var host = Copied(out var copy); + + ClipboardCopy.Offer(new(), host, window, copy); + + await Assert.That(host.State.Clipboard).IsSameReferenceAs(copy); + await Assert.That(host.State.Message).IsEqualTo("Copied 1 line from the selection."); + } + + [Test] + public async Task AClipboardThatStaysBusyIsGivenUpOnAndSaidSo() + { + var window = new Desktop { Busy = true }; + var host = Copied(out var copy); + var pump = new ClipboardPump(); + for (var attempt = 0; attempt < ClipboardPump.Attempts; attempt++) + { + ClipboardCopy.Offer(pump, host, window, copy); + } + + await Assert.That(host.State.Clipboard).IsNull(); + await Assert.That(host.State.Message).IsEqualTo("Could not copy the selection: another program is holding the clipboard."); + } + + /// + /// What came of one copy is not what came of a copy made since, which is still to be offered. + /// + [Test] + public async Task AnOutcomeLeavesALaterCopyAlone() + { + var window = new Desktop(); + var host = Copied(out var first); + var second = host.Mutate(_ => _ with { Clipboard = new("other text", "the selection") }).Clipboard!; + + ClipboardCopy.Offer(new(), host, window, first); + + await Assert.That(host.State.Clipboard).IsSameReferenceAs(second); + } + + /// + /// The loop offers what is waiting before it presents, so a copy reaches the window on the + /// frame after the key that made it. + /// + [Test] + public async Task TheLoopHandsTheCopyToTheWindow() + { + var window = new Desktop { Frames = 2 }; + var host = Copied(out _); + + ViewerProgram.Loop(host, window, null, null, null, new(), null, new(), new(ScreenBuilder.Build)); + + await Assert.That(window.Taken).IsEquivalentTo(["the text"]); + await Assert.That(host.State.Clipboard).IsNull(); + } + + static SessionHost Copied(out PendingCopy copy) + { + var host = new SessionHost(Fixtures.File()); + copy = host.Mutate(_ => _ with + { + Clipboard = new("the text", "the selection"), + Message = "Copied 1 line from the selection." + }).Clipboard!; + return host; + } + + /// + /// A window whose clipboard can be held by someone else, and which closes after a number of + /// frames. + /// + sealed class Desktop : IViewerWindow + { + int presented; + + public bool Busy { get; init; } + + public int Frames { get; init; } = 1; + + public List Taken { get; } = []; + + public bool Present(Screen screen) => + ++presented < Frames; + + public ViewerInput Poll() => + new(CommandKind.None, -1, -1, 0, false, Fixtures.Columns, Fixtures.Rows); + + public void SetHidden(bool hidden) + { + } + + public void Focus() + { + } + + public bool SetClipboard(string text) + { + if (Busy) + { + return false; + } + + Taken.Add(text); + return true; + } + + public bool Capture(Screen screen, int width, int height, string pngPath) => + false; + + public void Dispose() + { + } + } +} diff --git a/src/DiffEngineViewer.Tests/DerivedAcceptTests.cs b/src/DiffEngineViewer.Tests/DerivedAcceptTests.cs index 01327516c..860047880 100644 --- a/src/DiffEngineViewer.Tests/DerivedAcceptTests.cs +++ b/src/DiffEngineViewer.Tests/DerivedAcceptTests.cs @@ -460,9 +460,8 @@ public void Focus() { } - public void SetClipboard(string text) - { - } + public bool SetClipboard(string text) => + true; public bool Capture(Screen screen, int width, int height, string pngPath) => false; diff --git a/src/DiffEngineViewer.Tests/DiscardBatchTests.cs b/src/DiffEngineViewer.Tests/DiscardBatchTests.cs index b56a6d805..cfe854f33 100644 --- a/src/DiffEngineViewer.Tests/DiscardBatchTests.cs +++ b/src/DiffEngineViewer.Tests/DiscardBatchTests.cs @@ -236,9 +236,8 @@ public void Focus() { } - public void SetClipboard(string text) - { - } + public bool SetClipboard(string text) => + true; public bool Capture(Screen screen, int width, int height, string pngPath) => false; diff --git a/src/DiffEngineViewer.Tests/DrawingViewTests.cs b/src/DiffEngineViewer.Tests/DrawingViewTests.cs index a76e1635a..d9c43895f 100644 --- a/src/DiffEngineViewer.Tests/DrawingViewTests.cs +++ b/src/DiffEngineViewer.Tests/DrawingViewTests.cs @@ -205,9 +205,8 @@ public void Focus() { } - public void SetClipboard(string text) - { - } + public bool SetClipboard(string text) => + true; public bool Capture(Screen screen, int width, int height, string pngPath) => false; diff --git a/src/DiffEngineViewer.Tests/EntryMenuTests.cs b/src/DiffEngineViewer.Tests/EntryMenuTests.cs index 58d3a0775..047bad672 100644 --- a/src/DiffEngineViewer.Tests/EntryMenuTests.cs +++ b/src/DiffEngineViewer.Tests/EntryMenuTests.cs @@ -90,9 +90,8 @@ public void Focus() { } - public void SetClipboard(string text) - { - } + public bool SetClipboard(string text) => + true; public bool Capture(Screen screen, int width, int height, string pngPath) => false; diff --git a/src/DiffEngineViewer.Tests/GroupAcceptBatchTests.cs b/src/DiffEngineViewer.Tests/GroupAcceptBatchTests.cs index c774059cf..a7fe967f2 100644 --- a/src/DiffEngineViewer.Tests/GroupAcceptBatchTests.cs +++ b/src/DiffEngineViewer.Tests/GroupAcceptBatchTests.cs @@ -186,9 +186,8 @@ public void Focus() { } - public void SetClipboard(string text) - { - } + public bool SetClipboard(string text) => + true; public bool Capture(Screen screen, int width, int height, string pngPath) => false; diff --git a/src/DiffEngineViewer.Tests/MapProjectionTests.cs b/src/DiffEngineViewer.Tests/MapProjectionTests.cs index 6bd7d6a8d..568be7727 100644 --- a/src/DiffEngineViewer.Tests/MapProjectionTests.cs +++ b/src/DiffEngineViewer.Tests/MapProjectionTests.cs @@ -416,9 +416,8 @@ public void Focus() { } - public void SetClipboard(string text) - { - } + public bool SetClipboard(string text) => + true; public bool Capture(Screen screen, int width, int height, string pngPath) => false; diff --git a/src/DiffEngineViewer.Tests/MinimalViewTests.cs b/src/DiffEngineViewer.Tests/MinimalViewTests.cs index d4585cd87..9bb6d9e98 100644 --- a/src/DiffEngineViewer.Tests/MinimalViewTests.cs +++ b/src/DiffEngineViewer.Tests/MinimalViewTests.cs @@ -245,9 +245,8 @@ public void Focus() { } - public void SetClipboard(string text) - { - } + public bool SetClipboard(string text) => + true; public bool Capture(Screen screen, int width, int height, string pngPath) => false; diff --git a/src/DiffEngineViewer.Tests/ModuleInitializer.cs b/src/DiffEngineViewer.Tests/ModuleInitializer.cs index 9bfb1e348..d558b6aea 100644 --- a/src/DiffEngineViewer.Tests/ModuleInitializer.cs +++ b/src/DiffEngineViewer.Tests/ModuleInitializer.cs @@ -13,7 +13,7 @@ public static void Initialize() // Program.Main does this for the app. A test host never runs Main, so without it the // default probing rules would have to find the native under runtimes/{rid}/native, which // they only do for natives that arrived through a NuGet package. - NativeResolver.Register(); + Deview.Resolve(); // Before anything can touch DiffTools, which resolves and caches on first use. Only points // the resolver at the local build; nothing here launches anything. ManualViewer.Register(); diff --git a/src/DiffEngineViewer.Tests/NativeTests.cs b/src/DiffEngineViewer.Tests/NativeTests.cs index 2cfeaa240..c67f59246 100644 --- a/src/DiffEngineViewer.Tests/NativeTests.cs +++ b/src/DiffEngineViewer.Tests/NativeTests.cs @@ -20,7 +20,7 @@ public class NativeTests public async Task LoadsAndReportsItsAbiVersion() { if (!HasNativeRenderer || - !NativeResolver.TryFind(out var path)) + !NativeResolver.TryFind(Deview.Library, out var path)) { // No binary is shipped for this RID, for example linux-musl. Resolution falls through // to a globally installed tool, so there is nothing to check here. @@ -42,7 +42,7 @@ public async Task ShipsABinaryForThisPlatform() return; } - await Assert.That(NativeResolver.TryFind(out _)).IsTrue(); + await Assert.That(NativeResolver.TryFind(Deview.Library, out _)).IsTrue(); } /// diff --git a/src/DiffEngineViewer.Tests/PaneMenuTests.cs b/src/DiffEngineViewer.Tests/PaneMenuTests.cs index 4871ceed0..5a2234ef6 100644 --- a/src/DiffEngineViewer.Tests/PaneMenuTests.cs +++ b/src/DiffEngineViewer.Tests/PaneMenuTests.cs @@ -50,7 +50,7 @@ public async Task CopySelectionCopiesWhatWasDraggedAcross() state = Click(state, "Copy selection", window); - await Assert.That(window.Copied).IsEqualTo("brown dog"); + await Assert.That(state.Clipboard?.Text).IsEqualTo("brown dog"); await Assert.That(state.Menu).IsNull(); } @@ -66,7 +66,7 @@ public async Task CopyAllCopiesThePaneItWasAskedIn() state = Click(state, "Copy all", window); - await Assert.That(window.Copied).IsEqualTo("right one\nright two"); + await Assert.That(state.Clipboard?.Text).IsEqualTo("right one\nright two"); await Assert.That(state.Menu).IsNull(); } @@ -189,8 +189,6 @@ static ViewerInput Input() => sealed class Recorder : IViewerWindow { - public string? Copied { get; private set; } - public bool Present(Screen screen) => true; @@ -205,8 +203,8 @@ public void Focus() { } - public void SetClipboard(string text) => - Copied = text; + public bool SetClipboard(string text) => + true; public bool Capture(Screen screen, int width, int height, string pngPath) => false; diff --git a/src/DiffEngineViewer.Tests/ScreenCacheTests.cs b/src/DiffEngineViewer.Tests/ScreenCacheTests.cs index 550d7640f..5aa4b4a41 100644 --- a/src/DiffEngineViewer.Tests/ScreenCacheTests.cs +++ b/src/DiffEngineViewer.Tests/ScreenCacheTests.cs @@ -7,23 +7,23 @@ public class ScreenCacheTests [Test] public async Task The_same_state_is_the_same_screen() { - var cache = new ScreenCache(); + var cache = new ScreenCache(ScreenBuilder.Build); var state = Scrollable(); - var first = cache.For(state); + var first = cache.Get(state); - await Assert.That(ReferenceEquals(cache.For(state), first)).IsTrue(); - await Assert.That(ReferenceEquals(cache.For(state), first)).IsTrue(); + await Assert.That(ReferenceEquals(cache.Get(state), first)).IsTrue(); + await Assert.That(ReferenceEquals(cache.Get(state), first)).IsTrue(); } [Test] public async Task Another_state_is_another_screen() { - var cache = new ScreenCache(); + var cache = new ScreenCache(ScreenBuilder.Build); var state = Scrollable(); - var first = cache.For(state); + var first = cache.Get(state); - var scrolled = cache.For(ViewerSession.Apply(state, CommandKind.ScrollDown)); + var scrolled = cache.Get(ViewerSession.Apply(state, CommandKind.ScrollDown)); await Assert.That(ReferenceEquals(scrolled, first)).IsFalse(); await Assert.That(scrolled.Left.ScrollTop).IsEqualTo(first.Left.ScrollTop + 1); @@ -36,12 +36,12 @@ public async Task Another_state_is_another_screen() [Test] public async Task A_state_come_back_to_is_built_again_and_says_the_same() { - var cache = new ScreenCache(); + var cache = new ScreenCache(ScreenBuilder.Build); var state = Scrollable(); - var first = cache.For(state); - cache.For(ViewerSession.Apply(state, CommandKind.ScrollDown)); + var first = cache.Get(state); + cache.Get(ViewerSession.Apply(state, CommandKind.ScrollDown)); - var again = cache.For(state); + var again = cache.Get(state); await Assert.That(AsciiRenderer.Render(again)).IsEqualTo(AsciiRenderer.Render(first)); } @@ -120,9 +120,8 @@ public void Focus() { } - public void SetClipboard(string text) - { - } + public bool SetClipboard(string text) => + true; public bool Capture(Screen screen, int width, int height, string pngPath) => false; diff --git a/src/DiffEngineViewer.Tests/SelectionTests.cs b/src/DiffEngineViewer.Tests/SelectionTests.cs index 3cae4d9a6..c7e926e6a 100644 --- a/src/DiffEngineViewer.Tests/SelectionTests.cs +++ b/src/DiffEngineViewer.Tests/SelectionTests.cs @@ -168,7 +168,7 @@ public async Task CopyPutsTheSelectionOnTheClipboard() var copied = ViewerProgram.Apply(state, Input(CommandKind.Copy), link: null, window); - await Assert.That(window.Copied).IsEqualTo("dog\njumps over\nthe "); + await Assert.That(copied.Clipboard?.Text).IsEqualTo("dog\njumps over\nthe "); await Assert.That(copied.Message).IsEqualTo("Copied 3 lines from the selection."); } @@ -183,10 +183,10 @@ public async Task ADragInsideWideCharactersTakesThemWhole() // 中 is cells 0 and 1, 文 cells 2 and 3, a cell 4. From inside 中 to inside 文 var state = Drag(Fixtures.File("中文a", "中文a"), PaneSide.Left, 0, 1, 0, 3); - ViewerProgram.Apply(state, Input(CommandKind.Copy), link: null, window); + var copied = ViewerProgram.Apply(state, Input(CommandKind.Copy), link: null, window); var highlighted = ScreenBuilder.Build(state).Left.Rows[0].Selection; - await Assert.That(window.Copied).IsEqualTo("文"); + await Assert.That(copied.Clipboard?.Text).IsEqualTo("文"); await Assert.That((highlighted.Start, highlighted.Length)).IsEqualTo((2, 2)); } @@ -209,7 +209,7 @@ public async Task CopyWithNothingSelectedSaysSoAndWritesNothing() var copied = ViewerProgram.Apply(Files(), Input(CommandKind.Copy), link: null, window); - await Assert.That(window.Copied).IsNull(); + await Assert.That(copied.Clipboard).IsNull(); await Assert.That(copied.Message) .IsEqualTo("Nothing is selected. Drag across a pane, or press ctrl+a."); } @@ -222,7 +222,7 @@ public async Task CopyASideTakesAllOfItWithoutTheFiller() var copied = ViewerProgram.Apply(state, Input(CommandKind.CopyRight), link: null, window); - await Assert.That(window.Copied).IsEqualTo("one\nfour"); + await Assert.That(copied.Clipboard?.Text).IsEqualTo("one\nfour"); await Assert.That(copied.Message).IsEqualTo("Copied 2 lines from Sample.verified.txt."); } @@ -245,7 +245,7 @@ public async Task CopyIsLocalEvenWhenDisplayingSomeoneElsesQueue() var copied = ViewerProgram.Apply(state, Input(CommandKind.Copy), link, window); - await Assert.That(window.Copied).IsEqualTo("the"); + await Assert.That(copied.Clipboard?.Text).IsEqualTo("the"); await Assert.That(copied.Message).IsEqualTo("Copied 1 line from the selection."); } @@ -259,9 +259,9 @@ public async Task SelectAllThenCopyTakesTheWholeSide() var window = new Recorder(); var all = Key(Files(), CommandKind.SelectAll); - ViewerProgram.Apply(all, Input(CommandKind.Copy), link: null, window); + var copied = ViewerProgram.Apply(all, Input(CommandKind.Copy), link: null, window); - await Assert.That(window.Copied).IsEqualTo(Fixtures.Received); + await Assert.That(copied.Clipboard?.Text).IsEqualTo(Fixtures.Received); } static SessionState Files(string left = Fixtures.Received, string right = Fixtures.Expected) => @@ -343,13 +343,11 @@ static void Append(StringBuilder builder, Pane pane) } /// - /// A window that draws nothing and remembers what was copied, which is the only thing about a - /// head this needs. + /// A window that draws nothing. A copy waits in the state for the loop to offer it, so what was + /// copied is read from there. /// sealed class Recorder : IViewerWindow { - public string? Copied { get; private set; } - public bool Present(Screen screen) => true; @@ -364,8 +362,8 @@ public void Focus() { } - public void SetClipboard(string text) => - Copied = text; + public bool SetClipboard(string text) => + true; public bool Capture(Screen screen, int width, int height, string pngPath) => false; @@ -538,8 +536,7 @@ static List Lines(Random random, string[] pieces, int count) /// static string? Copy(SessionState state) { - var window = new Recorder(); - ViewerProgram.Apply(state, Input(CommandKind.Copy), link: null, window); - return window.Copied; + var copied = ViewerProgram.Apply(state, Input(CommandKind.Copy), link: null, new Recorder()); + return copied.Clipboard?.Text; } } diff --git a/src/DiffEngineViewer.Tests/ViewerProgramTests.cs b/src/DiffEngineViewer.Tests/ViewerProgramTests.cs index 04c46575b..ad723e945 100644 --- a/src/DiffEngineViewer.Tests/ViewerProgramTests.cs +++ b/src/DiffEngineViewer.Tests/ViewerProgramTests.cs @@ -192,7 +192,7 @@ public async Task AWindowNobodyCanSeeSlowsWhatIsKeptGoingBesideIt() [true, true, false], () => slowed.Add($"{link.Hidden} {watch.Hidden} {reader.Hidden}")); - ViewerProgram.Loop(host, window, link, reader, watch, new(), null, new(), new()); + ViewerProgram.Loop(host, window, link, reader, watch, new(), null, new(), new(ScreenBuilder.Build)); // As each frame was presented: before anything was said, after each of the two polls // that said nobody could see the window, and after the one that said somebody could @@ -214,7 +214,7 @@ public async Task AWindowHiddenFromHereStaysSlowedWhateverItsHeadSays() ConcurrentQueue commands = new(); commands.Enqueue(WindowCommand.Hide); - ViewerProgram.Loop(host, window, null, null, watch, commands, null, new(), new()); + ViewerProgram.Loop(host, window, null, null, watch, commands, null, new(), new(ScreenBuilder.Build)); await Assert.That(string.Join(", ", slowed)).IsEqualTo("True, True, True"); } @@ -245,9 +245,8 @@ public void Focus() { } - public void SetClipboard(string text) - { - } + public bool SetClipboard(string text) => + true; public bool Capture(Screen screen, int width, int height, string pngPath) => false; @@ -279,9 +278,8 @@ public void Focus() { } - public void SetClipboard(string text) - { - } + public bool SetClipboard(string text) => + true; public bool Capture(Screen screen, int width, int height, string pngPath) => false; @@ -319,9 +317,8 @@ public void Focus() { } - public void SetClipboard(string text) - { - } + public bool SetClipboard(string text) => + true; public bool Capture(Screen screen, int width, int height, string pngPath) => false; diff --git a/src/DiffEngineViewer.Tests/ZoomTests.cs b/src/DiffEngineViewer.Tests/ZoomTests.cs index 11444e281..68ef3ca06 100644 --- a/src/DiffEngineViewer.Tests/ZoomTests.cs +++ b/src/DiffEngineViewer.Tests/ZoomTests.cs @@ -271,9 +271,8 @@ public void Focus() { } - public void SetClipboard(string text) - { - } + public bool SetClipboard(string text) => + true; public bool Capture(Screen screen, int width, int height, string pngPath) => false; diff --git a/src/DiffEngineViewer.Windows.Tests/FormsHeadTests.cs b/src/DiffEngineViewer.Windows.Tests/FormsHeadTests.cs index 88f4b5fb0..4a1b3062a 100644 --- a/src/DiffEngineViewer.Windows.Tests/FormsHeadTests.cs +++ b/src/DiffEngineViewer.Windows.Tests/FormsHeadTests.cs @@ -1665,9 +1665,8 @@ public void SetHidden(bool hidden) { } - public void SetClipboard(string text) - { - } + public bool SetClipboard(string text) => + true; public void Focus() { diff --git a/src/DiffEngineViewer.Windows/FormsViewerWindow.cs b/src/DiffEngineViewer.Windows/FormsViewerWindow.cs index 75e46d118..cdcc663b1 100644 --- a/src/DiffEngineViewer.Windows/FormsViewerWindow.cs +++ b/src/DiffEngineViewer.Windows/FormsViewerWindow.cs @@ -152,18 +152,21 @@ public void Focus() form.Placement; /// - /// Best effort, the way revealing a file is. Another process can hold the clipboard open, and - /// failing to copy is not worth taking the reviewer's window down over. + /// False when another process holds the clipboard open, which WinForms has already retried + /// for about a second. Answered rather than thrown, and not retried here: the loop's + /// offers it again on the next frames, and says so if it never goes. /// - public void SetClipboard(string text) + public bool SetClipboard(string text) { try { Clipboard.SetText(text); + return true; } catch (ExternalException exception) { Console.Error.WriteLine($"Could not write to the clipboard: {exception.Message}"); + return false; } } diff --git a/src/DiffEngineViewer/ClipboardCopy.cs b/src/DiffEngineViewer/ClipboardCopy.cs new file mode 100644 index 000000000..e82364120 --- /dev/null +++ b/src/DiffEngineViewer/ClipboardCopy.cs @@ -0,0 +1,47 @@ +/// +/// Hands the text waiting in to the window through the +/// kernel's , and lets the state let go of it only once the window has +/// taken it or the pump has given up, which the status line then says. +/// +static class ClipboardCopy +{ + public static void Offer(ClipboardPump pump, SessionHost host, IViewerWindow window, PendingCopy copy) + { + switch (pump.Offer(copy, copy.Text, window.SetClipboard)) + { + case ClipboardOffer.Taken: + host.Mutate(_ => Release(_, copy)); + return; + case ClipboardOffer.GaveUp: + host.Mutate(_ => Failed(_, copy)); + return; + } + } + + /// + /// Only the copy that was offered: one made since is still to be offered. + /// + static SessionState Release(SessionState state, PendingCopy copy) + { + if (!ReferenceEquals(state.Clipboard, copy)) + { + return state; + } + + return state with { Clipboard = null }; + } + + static SessionState Failed(SessionState state, PendingCopy copy) + { + if (!ReferenceEquals(state.Clipboard, copy)) + { + return state; + } + + return state with + { + Clipboard = null, + Message = $"Could not copy {copy.What}: another program is holding the clipboard." + }; + } +} diff --git a/src/DiffEngineViewer/DiffEngineViewer.csproj b/src/DiffEngineViewer/DiffEngineViewer.csproj index 7dd6f7a19..090a315e0 100644 --- a/src/DiffEngineViewer/DiffEngineViewer.csproj +++ b/src/DiffEngineViewer/DiffEngineViewer.csproj @@ -36,6 +36,7 @@ + diff --git a/src/DiffEngineViewer/IViewerWindow.cs b/src/DiffEngineViewer/IViewerWindow.cs index a2e1498de..60fb1eb60 100644 --- a/src/DiffEngineViewer/IViewerWindow.cs +++ b/src/DiffEngineViewer/IViewerWindow.cs @@ -1,64 +1,5 @@ /// -/// One platform's renderer, and the only thing in the app that knows how pixels get drawn. -/// Everything above it is a pure function of , which is what lets -/// and its text snapshots stand in for every implementation. -/// -/// The frame contract is deliberately coarse: one carrying a whole screen -/// and one per iteration, rather than a stream of draw calls. That is what -/// makes a retained mode toolkit and an immediate mode renderer equally implementable behind it. -/// +/// One platform's renderer, as the kernel's has it. +/// The viewer asks nothing of a head beyond that. /// -interface IViewerWindow : IDisposable -{ - /// - /// Draws one frame. False once the window has closed, which is how the loop learns to stop. - /// - bool Present(Screen screen); - - /// - /// Everything the user did since the last call. Drains as it reads, so each event arrives once. - /// - ViewerInput Poll(); - - void SetHidden(bool hidden); - - /// - /// Puts text on the system clipboard. - /// - /// Here rather than in with the rest of the IO, because a - /// clipboard belongs to a toolkit the way a window does: WinForms, AppKit and GLFW each own - /// one, and the alternative was shelling out to clip, pbcopy and whichever of - /// xclip or wl-copy happened to be installed. - /// - /// - void SetClipboard(string text); - - void Focus(); - - /// - /// Where the window is and whether it is maximised, to open the next one the same way. Null - /// from a head that cannot say, which is also what a window that never opened says. - /// - /// Still answered once the window has gone, with how it was last seen: a session that is - /// ending closes the window before the loop gets to ask. - /// - /// - WindowPlacement? Placement => null; - - /// - /// Renders one frame offscreen to a PNG. Only the pixel snapshots use this. - /// - bool Capture(Screen screen, int width, int height, string pngPath); -} - -/// -/// Opens the window for one platform. Null with an rather than an -/// exception, because a machine with no renderer for its RID and no desktop session to draw into -/// are both ordinary, and both want the same message rather than a stack trace. -/// -/// -/// How the last window was left, to open this one the same way, or null to open at -/// by wherever the platform puts a new window. -/// A head ignores one that no longer fits a screen it has. -/// -delegate IViewerWindow? OpenWindow(string title, int width, int height, bool hidden, WindowPlacement? placement, out string? error); +interface IViewerWindow : IHeadWindow; diff --git a/src/DiffEngineViewer/Native/Deview.cs b/src/DiffEngineViewer/Native/Deview.cs index 877b39827..032457208 100644 --- a/src/DiffEngineViewer/Native/Deview.cs +++ b/src/DiffEngineViewer/Native/Deview.cs @@ -5,7 +5,14 @@ /// static unsafe partial class Deview { - const string library = "diffengine_viewer"; + public const string Library = "diffengine_viewer"; + + /// + /// Has the kernel's resolver find under runtimes/{rid}/native for these + /// imports. Once a process, however many threads ask. + /// + public static void Resolve() => + NativeResolver.Register(typeof(Deview).Assembly, Library); /// /// Must match DEVIEW_VERSION in native/include/deview.h. Bumped whenever the structs change, @@ -13,10 +20,10 @@ static unsafe partial class Deview /// public const int ExpectedVersion = 12; - [LibraryImport(library, EntryPoint = "deview_version")] + [LibraryImport(Library, EntryPoint = "deview_version")] public static partial int Version(); - [LibraryImport(library, EntryPoint = "deview_init", StringMarshalling = StringMarshalling.Utf8)] + [LibraryImport(Library, EntryPoint = "deview_init", StringMarshalling = StringMarshalling.Utf8)] public static partial int Init( int width, int height, @@ -26,30 +33,30 @@ public static partial int Init( float fontSize, int hidden); - [LibraryImport(library, EntryPoint = "deview_present")] + [LibraryImport(Library, EntryPoint = "deview_present")] public static partial int Present(DeviewScreen* screen); - [LibraryImport(library, EntryPoint = "deview_poll_input")] + [LibraryImport(Library, EntryPoint = "deview_poll_input")] public static partial void PollInput(DeviewInput* input); - [LibraryImport(library, EntryPoint = "deview_capture", StringMarshalling = StringMarshalling.Utf8)] + [LibraryImport(Library, EntryPoint = "deview_capture", StringMarshalling = StringMarshalling.Utf8)] public static partial int Capture(DeviewScreen* screen, int width, int height, string pngPath); - [LibraryImport(library, EntryPoint = "deview_set_hidden")] + [LibraryImport(Library, EntryPoint = "deview_set_hidden")] public static partial void SetHidden(int hidden); - [LibraryImport(library, EntryPoint = "deview_set_clipboard", StringMarshalling = StringMarshalling.Utf8)] + [LibraryImport(Library, EntryPoint = "deview_set_clipboard", StringMarshalling = StringMarshalling.Utf8)] public static partial void SetClipboard(string text); - [LibraryImport(library, EntryPoint = "deview_focus")] + [LibraryImport(Library, EntryPoint = "deview_focus")] public static partial void Focus(); - [LibraryImport(library, EntryPoint = "deview_set_placement")] + [LibraryImport(Library, EntryPoint = "deview_set_placement")] public static partial void SetPlacement(DeviewPlacement* placement); - [LibraryImport(library, EntryPoint = "deview_get_placement")] + [LibraryImport(Library, EntryPoint = "deview_get_placement")] public static partial int GetPlacement(DeviewPlacement* placement); - [LibraryImport(library, EntryPoint = "deview_shutdown")] + [LibraryImport(Library, EntryPoint = "deview_shutdown")] public static partial void Shutdown(); } diff --git a/src/DiffEngineViewer/Native/NativeResolver.cs b/src/DiffEngineViewer/Native/NativeResolver.cs deleted file mode 100644 index fbb747d3f..000000000 --- a/src/DiffEngineViewer/Native/NativeResolver.cs +++ /dev/null @@ -1,136 +0,0 @@ -/// -/// Finds the shim under runtimes/{rid}/native. -/// -/// The default probing rules only consult that layout for natives that arrived through a NuGet -/// package and are recorded in deps.json. This app ships them as plain content, both in the repo -/// and inside the RID agnostic dotnet tool package, so it resolves them itself. -/// -/// -static class NativeResolver -{ - const string name = "diffengine_viewer"; - static bool registered; - - public static void Register() - { - if (registered) - { - return; - } - - registered = true; - NativeLibrary.SetDllImportResolver(typeof(NativeResolver).Assembly, Resolve); - } - - static nint Resolve(string library, Assembly assembly, DllImportSearchPath? searchPath) - { - if (library != name) - { - return nint.Zero; - } - - foreach (var candidate in Candidates()) - { - if (File.Exists(candidate) && - NativeLibrary.TryLoad(candidate, out var handle)) - { - return handle; - } - } - - // Zero hands back to the default resolver, so a native sitting beside the executable, or - // on the system path, still works. - return nint.Zero; - } - - /// - /// The shipped binary for the current RID, or false when none is shipped for it, such as on - /// linux-musl. - /// - public static bool TryFind([NotNullWhen(true)] out string? path) - { - foreach (var candidate in Candidates()) - { - if (File.Exists(candidate)) - { - path = candidate; - return true; - } - } - - path = null; - return false; - } - - static IEnumerable Candidates() - { - var root = AppContext.BaseDirectory; - var file = FileName(); - foreach (var rid in Rids()) - { - yield return Path.Combine(root, "runtimes", rid, "native", file); - } - - yield return Path.Combine(root, file); - } - - static IEnumerable Rids() => - Rids(RuntimeInformation.RuntimeIdentifier); - - internal static IEnumerable Rids(string runtimeIdentifier) - { - // The exact RID first. On Alpine that is linux-musl-x64, which we do not ship, so the - // probe simply misses rather than loading a glibc binary and hard crashing. - yield return runtimeIdentifier; - - // And nothing else, or the synthesised RID below undoes that: linux-{arch} is the glibc - // build. BundledViewerDirectory.Rids stops here for the same reason. A failed load is - // caught and the queue staged, but one that succeeded would leave a crash to come later, - // past the point where anything stages. - if (runtimeIdentifier.Contains("-musl-", StringComparison.Ordinal)) - { - yield break; - } - - var architecture = RuntimeInformation.OSArchitecture switch - { - Architecture.X64 => "x64", - Architecture.Arm64 => "arm64", - Architecture.X86 => "x86", - _ => null - }; - - if (architecture is null) - { - yield break; - } - - if (OperatingSystem.IsWindows()) - { - yield return $"win-{architecture}"; - } - else if (OperatingSystem.IsMacOS()) - { - yield return $"osx-{architecture}"; - } - else if (OperatingSystem.IsLinux()) - { - yield return $"linux-{architecture}"; - } - } - - static string FileName() - { - if (OperatingSystem.IsWindows()) - { - return $"{name}.dll"; - } - - if (OperatingSystem.IsMacOS()) - { - return $"lib{name}.dylib"; - } - - return $"lib{name}.so"; - } -} diff --git a/src/DiffEngineViewer/Native/NativeViewerWindow.cs b/src/DiffEngineViewer/Native/NativeViewerWindow.cs index d61108b5e..b4bb841f1 100644 --- a/src/DiffEngineViewer/Native/NativeViewerWindow.cs +++ b/src/DiffEngineViewer/Native/NativeViewerWindow.cs @@ -147,8 +147,15 @@ public unsafe ViewerInput Poll() public void SetHidden(bool hidden) => Deview.SetHidden(hidden ? 1 : 0); - public void SetClipboard(string text) => + /// + /// True, since the shim takes the text and says nothing of how it went: GLFW and AppKit both + /// own the clipboard in this process, and neither is refused it the way Windows can be. + /// + public bool SetClipboard(string text) + { Deview.SetClipboard(text); + return true; + } public void Focus() => Deview.Focus(); diff --git a/src/DiffEngineViewer/PendingCopy.cs b/src/DiffEngineViewer/PendingCopy.cs new file mode 100644 index 000000000..1e819db4a --- /dev/null +++ b/src/DiffEngineViewer/PendingCopy.cs @@ -0,0 +1,13 @@ +/// +/// Text waiting for the clipboard (). Compared by reference: +/// a second copy of the same text is another copy, with attempts of its own. +/// +/// What was copied, as the status line names it, for saying it was not. +sealed record PendingCopy(string Text, string What) +{ + public bool Equals(PendingCopy? other) => + ReferenceEquals(this, other); + + public override int GetHashCode() => + RuntimeHelpers.GetHashCode(this); +} diff --git a/src/DiffEngineViewer/ScreenCache.cs b/src/DiffEngineViewer/ScreenCache.cs deleted file mode 100644 index 643bd9899..000000000 --- a/src/DiffEngineViewer/ScreenCache.cs +++ /dev/null @@ -1,36 +0,0 @@ -/// -/// The screen for the session's state, built once per state rather than once per frame. -/// -/// The loop presents sixty times a second, and a state changes when something happens, which for a -/// window somebody is reading is now and then. is immutable and only -/// ever replaced, so the same reference is the same screen. Building it again every frame was the -/// whole queue's labels, groups and tooltips, and a walk over every selected row, to arrive at the -/// screen already in hand: 2,000 pending entries were two megabytes of garbage a frame, from a -/// window nobody was touching. -/// -/// -/// Handing back the same is also how a head learns that nothing changed -/// without comparing two of them or encoding one: the WinForms head and -/// both stop at the reference. -/// -/// -/// One thread's. The loop and a head's modal loop both run on the thread that owns the window. -/// -/// -sealed class ScreenCache -{ - SessionState? state; - Screen? screen; - - public Screen For(SessionState current) - { - if (screen is null || - !ReferenceEquals(state, current)) - { - screen = ScreenBuilder.Build(current); - state = current; - } - - return screen; - } -} diff --git a/src/DiffEngineViewer/SessionHost.cs b/src/DiffEngineViewer/SessionHost.cs deleted file mode 100644 index f27f0922b..000000000 --- a/src/DiffEngineViewer/SessionHost.cs +++ /dev/null @@ -1,34 +0,0 @@ -/// -/// Guards the one mutable thing in the process. The render loop reads it every frame while the -/// socket listener writes to it from its own thread, so both go through here. -/// -/// Reads take no lock. is immutable, so a reader only needs the -/// reference, and reading a reference is already atomic; volatile is there to stop the read being -/// hoisted out of the render loop. -/// -/// -/// That is not an optimisation. Accepting runs InlineApplier inside , which -/// takes a cross process mutex and can wait ten seconds for it. A reader that queued behind the -/// writer would stall the render loop for that whole time, and a window that stops pumping for -/// five seconds is one Windows paints over with "Not Responding". -/// -/// -sealed class SessionHost(SessionState initial) -{ - readonly Lock gate = new(); - volatile SessionState state = initial; - - public SessionState State => state; - - /// - /// Still serialised, so two writers cannot compute from the same starting state and one lose. - /// - public SessionState Mutate(Func change) - { - lock (gate) - { - state = change(state); - return state; - } - } -} diff --git a/src/DiffEngineViewer/SessionState.cs b/src/DiffEngineViewer/SessionState.cs index e81cbd634..76a8d7366 100644 --- a/src/DiffEngineViewer/SessionState.cs +++ b/src/DiffEngineViewer/SessionState.cs @@ -72,6 +72,14 @@ record SessionState( /// public TextSelection? Selection { get; init; } + /// + /// Text copied and not yet taken by the clipboard, which the loop offers the window a frame at + /// a time through . Here rather than handed to the window as the + /// copy is made, because on Windows a clipboard another process holds refuses a set outright, + /// and the copy was then lost while the status line said it had been made. + /// + public PendingCopy? Clipboard { get; init; } + /// /// Whether the panes show only the changes, each with lines /// either side, rather than every line. A view setting like : it holds diff --git a/src/DiffEngineViewer/ViewerProgram.cs b/src/DiffEngineViewer/ViewerProgram.cs index fd69fdc0f..7bd6fc4b9 100644 --- a/src/DiffEngineViewer/ViewerProgram.cs +++ b/src/DiffEngineViewer/ViewerProgram.cs @@ -307,7 +307,7 @@ internal static int Run( } // One for the loop and for a head's modal loop, which draw the same window on one thread. - var screens = new ScreenCache(); + var screens = new ScreenCache(ScreenBuilder.Build); if (window is ILoopHooks hooks) { hooks.Frame = () => ModalFrame(host, window, link, preferences, screens); @@ -401,7 +401,7 @@ static Screen ModalFrame(SessionHost host, IViewerWindow window, OwnerLink? link preferences.Remember(state); } - return screens.For(state); + return screens.Get(state); } /// @@ -475,6 +475,7 @@ internal static void Loop( // screen, since only hiding it was known here. var hidden = false; var unseen = false; + var clipboard = new ClipboardPump(); void Slow() { @@ -526,8 +527,13 @@ void Slow() // The screen of the state last presented, for as long as that is still the state: see // ScreenCache. A frame in which nothing happened builds nothing. + if (host.State.Clipboard is { } copy) + { + ClipboardCopy.Offer(clipboard, host, window, copy); + } + var state = host.State; - if (!window.Present(screens.For(state))) + if (!window.Present(screens.Get(state))) { return; } @@ -776,7 +782,7 @@ static SessionState Dispatch(SessionState state, Command command, OwnerLink? lin // owner's answer would be the text this process already has. if (command.Kind is CommandKind.Copy or CommandKind.CopyLeft or CommandKind.CopyRight) { - return Copy(state, command.Kind, window); + return Copy(state, command.Kind); } // Nothing that changes the queue while an accept-all is working through it, whichever @@ -886,7 +892,7 @@ static SessionState Dispatch(SessionState state, Command command, OwnerLink? lin /// selection says so rather than silently putting nothing on the clipboard, since the two are /// indistinguishable afterwards. /// - static SessionState Copy(SessionState state, CommandKind kind, IViewerWindow window) + static SessionState Copy(SessionState state, CommandKind kind) { if (state.Current is not { } current) { @@ -925,10 +931,12 @@ static SessionState Copy(SessionState state, CommandKind kind, IViewerWindow win }; } - window.SetClipboard(text); + // Offered to the window by the loop rather than here, inside the mutation: see + // SessionState.Clipboard. var lines = text.Count(_ => _ == '\n') + 1; return state with { + Clipboard = new(text, what), Message = $"Copied {lines} line{(lines == 1 ? "" : "s")} from {what}.", Menu = null }; diff --git a/src/DiffEngineViewer/WindowPlacement.cs b/src/DiffEngineViewer/WindowPlacement.cs deleted file mode 100644 index 6f951970b..000000000 --- a/src/DiffEngineViewer/WindowPlacement.cs +++ /dev/null @@ -1,60 +0,0 @@ -using System.Globalization; - -/// -/// Where the window was when it was last on screen, and whether it filled the screen, so the next -/// one opens the same way. -/// -/// In whatever units the head that reported it counts in, and its own origin: device pixels from -/// the top left on Windows and Linux, points from the bottom left on macOS. Nothing but that head -/// ever reads the numbers back, so they need to mean the same thing only to it. -/// -/// -/// The window's left edge when it is not maximised. -/// The window's edge on the axis its head counts from, when it is not maximised. -/// The window's width when it is not maximised. -/// The window's height when it is not maximised. -/// -/// Whether it filled the screen. The bounds are then the ones it goes back to on being restored, -/// which is what makes maximise, close, open, restore land where it started. -/// -readonly record struct WindowPlacement(int X, int Y, int Width, int Height, bool Maximized) -{ - public override string ToString() => - string.Create(CultureInfo.InvariantCulture, $"{X},{Y},{Width},{Height},{(Maximized ? 1 : 0)}"); - - /// - /// Null for anything that is not five numbers, or names a window with no size: the file is one - /// a person can edit, and a window opened from a line that makes no sense opens as a new one - /// does. - /// - public static WindowPlacement? Parse(string? value) - { - if (value is null) - { - return null; - } - - var parts = value.Split(','); - if (parts.Length != 5) - { - return null; - } - - var numbers = new int[5]; - for (var index = 0; index < parts.Length; index++) - { - if (!int.TryParse(parts[index], NumberStyles.Integer, CultureInfo.InvariantCulture, out numbers[index])) - { - return null; - } - } - - if (numbers[2] <= 0 || - numbers[3] <= 0) - { - return null; - } - - return new(numbers[0], numbers[1], numbers[2], numbers[3], numbers[4] != 0); - } -} diff --git a/src/Directory.Build.props b/src/Directory.Build.props index ff770b179..d758e67b6 100644 --- a/src/Directory.Build.props +++ b/src/Directory.Build.props @@ -39,4 +39,18 @@ + + + + + + + + + + diff --git a/src/Directory.Packages.props b/src/Directory.Packages.props index cd79a4f52..f8871d365 100644 --- a/src/Directory.Packages.props +++ b/src/Directory.Packages.props @@ -19,7 +19,8 @@ --> - + + diff --git a/src/Packaging.Tests/Package.DiffEngine.verified.txt b/src/Packaging.Tests/Package.DiffEngine.verified.txt index c35e515f2..f65807232 100644 --- a/src/Packaging.Tests/Package.DiffEngine.verified.txt +++ b/src/Packaging.Tests/Package.DiffEngine.verified.txt @@ -26,32 +26,38 @@ tools/viewer/linux-arm64/DiffEngineViewer.Core.dll tools/viewer/linux-arm64/DiffEngineViewer.deps.json tools/viewer/linux-arm64/DiffEngineViewer.dll tools/viewer/linux-arm64/DiffEngineViewer.runtimeconfig.json +tools/viewer/linux-arm64/PlatformHeads.Core.dll tools/viewer/linux-arm64/runtimes/linux-arm64/native/libdiffengine_viewer.so tools/viewer/linux-x64/DiffEngineViewer tools/viewer/linux-x64/DiffEngineViewer.Core.dll tools/viewer/linux-x64/DiffEngineViewer.deps.json tools/viewer/linux-x64/DiffEngineViewer.dll tools/viewer/linux-x64/DiffEngineViewer.runtimeconfig.json +tools/viewer/linux-x64/PlatformHeads.Core.dll tools/viewer/linux-x64/runtimes/linux-x64/native/libdiffengine_viewer.so tools/viewer/osx-arm64/DiffEngineViewer tools/viewer/osx-arm64/DiffEngineViewer.Core.dll tools/viewer/osx-arm64/DiffEngineViewer.deps.json tools/viewer/osx-arm64/DiffEngineViewer.dll tools/viewer/osx-arm64/DiffEngineViewer.runtimeconfig.json +tools/viewer/osx-arm64/PlatformHeads.Core.dll tools/viewer/osx-arm64/runtimes/osx-arm64/native/libdiffengine_viewer.dylib tools/viewer/osx-x64/DiffEngineViewer tools/viewer/osx-x64/DiffEngineViewer.Core.dll tools/viewer/osx-x64/DiffEngineViewer.deps.json tools/viewer/osx-x64/DiffEngineViewer.dll tools/viewer/osx-x64/DiffEngineViewer.runtimeconfig.json +tools/viewer/osx-x64/PlatformHeads.Core.dll tools/viewer/osx-x64/runtimes/osx-x64/native/libdiffengine_viewer.dylib tools/viewer/win-arm64/DiffEngineViewer.Core.dll tools/viewer/win-arm64/DiffEngineViewer.deps.json tools/viewer/win-arm64/DiffEngineViewer.dll tools/viewer/win-arm64/DiffEngineViewer.exe tools/viewer/win-arm64/DiffEngineViewer.runtimeconfig.json +tools/viewer/win-arm64/PlatformHeads.Core.dll tools/viewer/win-x64/DiffEngineViewer.Core.dll tools/viewer/win-x64/DiffEngineViewer.deps.json tools/viewer/win-x64/DiffEngineViewer.dll tools/viewer/win-x64/DiffEngineViewer.exe -tools/viewer/win-x64/DiffEngineViewer.runtimeconfig.json \ No newline at end of file +tools/viewer/win-x64/DiffEngineViewer.runtimeconfig.json +tools/viewer/win-x64/PlatformHeads.Core.dll \ No newline at end of file diff --git a/src/Packaging.Tests/Package.DiffEngineTray.verified.txt b/src/Packaging.Tests/Package.DiffEngineTray.verified.txt index f41875663..d30936a3c 100644 --- a/src/Packaging.Tests/Package.DiffEngineTray.verified.txt +++ b/src/Packaging.Tests/Package.DiffEngineTray.verified.txt @@ -45,8 +45,10 @@ tools/net10.0/any/viewer/win-arm64/DiffEngineViewer.deps.json tools/net10.0/any/viewer/win-arm64/DiffEngineViewer.dll tools/net10.0/any/viewer/win-arm64/DiffEngineViewer.exe tools/net10.0/any/viewer/win-arm64/DiffEngineViewer.runtimeconfig.json +tools/net10.0/any/viewer/win-arm64/PlatformHeads.Core.dll tools/net10.0/any/viewer/win-x64/DiffEngineViewer.Core.dll tools/net10.0/any/viewer/win-x64/DiffEngineViewer.deps.json tools/net10.0/any/viewer/win-x64/DiffEngineViewer.dll tools/net10.0/any/viewer/win-x64/DiffEngineViewer.exe -tools/net10.0/any/viewer/win-x64/DiffEngineViewer.runtimeconfig.json \ No newline at end of file +tools/net10.0/any/viewer/win-x64/DiffEngineViewer.runtimeconfig.json +tools/net10.0/any/viewer/win-x64/PlatformHeads.Core.dll \ No newline at end of file diff --git a/src/Packaging.Tests/Package.DiffEngineViewer.Linux.verified.txt b/src/Packaging.Tests/Package.DiffEngineViewer.Linux.verified.txt index 17741b617..42ba0d923 100644 --- a/src/Packaging.Tests/Package.DiffEngineViewer.Linux.verified.txt +++ b/src/Packaging.Tests/Package.DiffEngineViewer.Linux.verified.txt @@ -10,6 +10,7 @@ tools/net10.0/any/DiffEngineViewer.deps.json tools/net10.0/any/DiffEngineViewer.dll tools/net10.0/any/DiffEngineViewer.runtimeconfig.json tools/net10.0/any/DotnetToolSettings.xml +tools/net10.0/any/PlatformHeads.Core.dll tools/net10.0/any/documents/AngleSharp.dll tools/net10.0/any/documents/DiffEngineViewer.Documents.deps.json tools/net10.0/any/documents/DiffEngineViewer.Documents.dll diff --git a/src/Packaging.Tests/Package.DiffEngineViewer.Mac.verified.txt b/src/Packaging.Tests/Package.DiffEngineViewer.Mac.verified.txt index a426a331b..9cc331921 100644 --- a/src/Packaging.Tests/Package.DiffEngineViewer.Mac.verified.txt +++ b/src/Packaging.Tests/Package.DiffEngineViewer.Mac.verified.txt @@ -10,6 +10,7 @@ tools/net10.0/any/DiffEngineViewer.deps.json tools/net10.0/any/DiffEngineViewer.dll tools/net10.0/any/DiffEngineViewer.runtimeconfig.json tools/net10.0/any/DotnetToolSettings.xml +tools/net10.0/any/PlatformHeads.Core.dll tools/net10.0/any/documents/AngleSharp.dll tools/net10.0/any/documents/DiffEngineViewer.Documents.deps.json tools/net10.0/any/documents/DiffEngineViewer.Documents.dll diff --git a/src/Packaging.Tests/Package.DiffEngineViewer.Windows.verified.txt b/src/Packaging.Tests/Package.DiffEngineViewer.Windows.verified.txt index 27c19735c..3ee219926 100644 --- a/src/Packaging.Tests/Package.DiffEngineViewer.Windows.verified.txt +++ b/src/Packaging.Tests/Package.DiffEngineViewer.Windows.verified.txt @@ -10,6 +10,7 @@ tools/net10.0/any/DiffEngineViewer.deps.json tools/net10.0/any/DiffEngineViewer.dll tools/net10.0/any/DiffEngineViewer.runtimeconfig.json tools/net10.0/any/DotnetToolSettings.xml +tools/net10.0/any/PlatformHeads.Core.dll tools/net10.0/any/documents/AngleSharp.dll tools/net10.0/any/documents/DiffEngineViewer.Documents.deps.json tools/net10.0/any/documents/DiffEngineViewer.Documents.dll