diff --git a/.github/workflows/build-native.yml b/.github/workflows/build-native.yml index 5b49f0f8..452a4172 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.4.0 + uses: SimonCropp/PlatformHeads/.github/workflows/build-native.yml@0.5.0 permissions: contents: write pull-requests: write diff --git a/claude.md b/claude.md index 9ac17405..c9adfc5a 100644 --- a/claude.md +++ b/claude.md @@ -752,8 +752,11 @@ the comment there about not caching "nothing staged" asks for. second being what `ViewerClient.FoundUnowned` is for. - `Program.Main` is a synchronous `[STAThread]` method that blocks on `Inner`. An attribute on an `async Task Main` lands on a method the runtime does not start, and the thread came up MTA. -- `SessionEndWindow` is a hidden top level window that hears `WM_ENDSESSION`, which neither the - notify icon nor a message filter does. An owning tray stages its queue from inside the message +- `SessionEndWindow`, PlatformHeads.WindowsForms', is a hidden top level window that hears + `WM_ENDSESSION`, which neither the notify icon nor a message filter does. So are + `MenuOnOpening`, which builds the menu's tracked items on every open beside its fixed ones and + disposes them at the next, and `IssueLink` and `LinkOpener` under `IssueLauncher` and + `LinkLauncher`: BuildMonitor's tray shares them, and a fix to any of them goes there. An owning tray stages its queue from inside the message (`OwnedInlineHost.SessionEnding`) and refuses patches from then on, because a logoff never comes back through `Application.Run()`. - A move that arrives for a tracked pair with no tool keeps the tool it was tracked with diff --git a/native/CMakeLists.txt b/native/CMakeLists.txt index d98a4b0c..46ad5659 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.4.0 + GIT_TAG 0.5.0 GIT_SHALLOW ON) FetchContent_MakeAvailable(platformheads) include(${platformheads_SOURCE_DIR}/cmake/PlatformHeads.cmake) diff --git a/native/swift/Package.swift b/native/swift/Package.swift index ab7fad6d..f3c56918 100644 --- a/native/swift/Package.swift +++ b/native/swift/Package.swift @@ -12,7 +12,7 @@ let package = Package( dependencies: [ // What every AppKit head does underneath its drawing, shared with BuildMonitor. The sixth // pin of PlatformHeads, moved with the other five. - .package(url: "https://github.com/SimonCropp/PlatformHeads", exact: "0.4.0") + .package(url: "https://github.com/SimonCropp/PlatformHeads", exact: "0.5.0") ], targets: [ // Exists only to import the ABI. Swift does not guarantee struct layout, so the structs diff --git a/src/DiffEngineTray.Tests/IssueLauncherTests.cs b/src/DiffEngineTray.Tests/IssueLauncherTests.cs index 9268b899..6f399c94 100644 --- a/src/DiffEngineTray.Tests/IssueLauncherTests.cs +++ b/src/DiffEngineTray.Tests/IssueLauncherTests.cs @@ -30,15 +30,16 @@ public async Task An_ampersand_in_the_title_does_not_truncate_it() } /// - /// The body is encoded by its callers and handed over already escaped, so encoding the title - /// must not have changed what reaches GitHub as the body. + /// Handed over as plain text and encoded once, after the version and OS. /// [Test] public async Task Keeps_the_body_it_is_given() { - var url = IssueLauncher.BuildUrl("TheTitle", WebUtility.UrlEncode("\n * Action: TheAction")); + var url = IssueLauncher.BuildUrl("TheTitle", "\n * Action: R&D #1"); - await Assert.That(Query(url)["body"]).Contains("* Action: TheAction"); + var body = Query(url)["body"]; + await Assert.That(body).Contains("* DiffEngineTray Version:"); + await Assert.That(body).EndsWith("* Action: R&D #1"); } static Dictionary Query(string url) => diff --git a/src/DiffEngineTray.Tests/PiperTest.cs b/src/DiffEngineTray.Tests/PiperTest.cs index 5b85ce60..26bfcd41 100644 --- a/src/DiffEngineTray.Tests/PiperTest.cs +++ b/src/DiffEngineTray.Tests/PiperTest.cs @@ -315,10 +315,7 @@ public async Task AClientThatResetsBeforeItIsAcceptedIsNotReportedAsAnError() .CreateLogger(); // ExceptionHandler follows the log with a modal "open an issue?" box, which here would wait // for a click forever. It asks once per message, so this one is marked as already asked. - var asked = (ConcurrentBag) typeof(IssueLauncher) - .GetField("recorded", BindingFlags.NonPublic | BindingFlags.Static)! - .GetValue(null)!; - asked.Add("Failed to receive payload"); + IssueLauncher.AlreadyRaised("Failed to receive payload"); using var cancel = new CancelSource(); var held = new HeldContext(); diff --git a/src/DiffEngineTray.Tests/SessionEndWindowTest.cs b/src/DiffEngineTray.Tests/SessionEndWindowTest.cs index ee226aa4..2c6babc4 100644 --- a/src/DiffEngineTray.Tests/SessionEndWindowTest.cs +++ b/src/DiffEngineTray.Tests/SessionEndWindowTest.cs @@ -1,6 +1,7 @@ /// /// The two messages a logoff or a shutdown sends every top level window, sent to the one the tray -/// keeps for hearing them. +/// keeps for hearing them, wired as Program wires it. Whether the window hears a session ending, +/// and not one that carried on, is PlatformHeads' and tested there. /// /// Sent from the thread that made the window, which calls its procedure directly, so no message /// loop is needed and nothing here awaits between making the window and destroying it. @@ -8,39 +9,6 @@ /// public class SessionEndWindowTest { - [Test] - public async Task TheSessionEndingIsHeard() - { - var heard = 0; - IntPtr query; - using (var window = new SessionEndWindow(() => heard++)) - { - query = SendMessage(window.Handle, queryEndSession, IntPtr.Zero, logoff); - SendMessage(window.Handle, endSession, new(1), logoff); - } - - // Agreed to. Refusing would hold up the logoff, and there is nothing here worth that - await Assert.That(query).IsEqualTo(new(1)); - await Assert.That(heard).IsEqualTo(1); - } - - /// - /// Another application refused the query, so everything that agreed is told the session goes - /// on. A tray that staged its queue for that would have it both on disk and in memory. - /// - [Test] - public async Task ASessionThatDidNotEndAfterAllIsNotHeard() - { - var heard = 0; - using (var window = new SessionEndWindow(() => heard++)) - { - SendMessage(window.Handle, queryEndSession, IntPtr.Zero, logoff); - SendMessage(window.Handle, endSession, IntPtr.Zero, logoff); - } - - await Assert.That(heard).IsEqualTo(0); - } - /// /// Wired the way Program wires it, over a queue this process holds. The session ending never /// reaches the disposal that stages a queue on a clean exit: the message loop does not return diff --git a/src/DiffEngineTray/DiffEngineTray.csproj b/src/DiffEngineTray/DiffEngineTray.csproj index 718107f8..74e93709 100644 --- a/src/DiffEngineTray/DiffEngineTray.csproj +++ b/src/DiffEngineTray/DiffEngineTray.csproj @@ -26,8 +26,12 @@ + + + diff --git a/src/DiffEngineTray/IssueLauncher.cs b/src/DiffEngineTray/IssueLauncher.cs index 0fa389e5..ada13f02 100644 --- a/src/DiffEngineTray/IssueLauncher.cs +++ b/src/DiffEngineTray/IssueLauncher.cs @@ -1,29 +1,34 @@ static class IssueLauncher { - static ConcurrentBag recorded = []; - static string defaultBody; + static readonly IssueLink link = new("https://github.com/VerifyTests/DiffEngine"); - static IssueLauncher() => - defaultBody = WebUtility.UrlEncode( - $""" - * DiffEngineTray Version: {VersionReader.VersionString} - * OS: {Environment.OSVersion.VersionString} - """); + static readonly string defaultBody = + $""" + * DiffEngineTray Version: {VersionReader.VersionString} + * OS: {Environment.OSVersion.VersionString} + """; public static void Launch() => LinkLauncher.LaunchUrl(BuildUrl("TODO")); /// - /// The title is encoded like the body is. It is built from a message carrying a file path, and - /// a '#' in one started a fragment - dropping the body, and everything of the title after it - - /// while an '&' started a parameter GitHub does not have, truncating the title there. + /// Marks as already offered, so a test that makes it happen is not + /// left waiting on the modal box nobody is there to answer. + /// + internal static void AlreadyRaised(string message) => + link.FirstTime(message); + + /// + /// The title and body are encoded by , so a path's '#' or '&' in + /// the title neither drops the body nor cuts the title short. is + /// plain text. /// internal static string BuildUrl(string title, string extraBody = "") => - $"https://github.com/VerifyTests/DiffEngine/issues/new?title={WebUtility.UrlEncode(title)}&body={defaultBody}{extraBody}"; + link.Url(title, defaultBody + extraBody); public static void LaunchForException(string message, Exception exception) { - if (CheckRecorded(message)) + if (!link.FirstTime(message)) { return; } @@ -42,7 +47,7 @@ Open an issue on GitHub? return; } - var extraBody = WebUtility.UrlEncode( + var extraBody = $""" * Action: {message} @@ -50,13 +55,13 @@ Open an issue on GitHub? ``` {exception} ``` - """); + """; LinkLauncher.LaunchUrl(BuildUrl(message, extraBody)); } public static void LaunchForException(string message) { - if (CheckRecorded(message)) + if (!link.FirstTime(message)) { return; } @@ -73,11 +78,11 @@ Open an issue on GitHub? return; } - var extraBody = WebUtility.UrlEncode( + var extraBody = $""" * Action: {message} - """); + """; LinkLauncher.LaunchUrl(BuildUrl(message, extraBody)); } @@ -97,15 +102,4 @@ static bool AskIfOpenIssue(string text) MessageBoxIcon.Error); return result == DialogResult.No; } - - static bool CheckRecorded(string message) - { - if (recorded.Contains(message)) - { - return true; - } - - recorded.Add(message); - return false; - } } \ No newline at end of file diff --git a/src/DiffEngineTray/LinkLauncher.cs b/src/DiffEngineTray/LinkLauncher.cs index 2695d1eb..aae422fe 100644 --- a/src/DiffEngineTray/LinkLauncher.cs +++ b/src/DiffEngineTray/LinkLauncher.cs @@ -1,25 +1,15 @@ static class LinkLauncher { /// - /// Logged when it cannot be opened, and nothing more: no browser registered, or one that will - /// not start, is nothing the tray can do anything about. Thrown, it went onto the UI thread - /// from a click in the options form, and out of the handler that reports an error, in place - /// of the error being reported. + /// Logged when it cannot be opened, and nothing more: see . Thrown, it + /// went onto the UI thread from a click in the options form, and out of the handler that + /// reports an error, in place of the error being reported. /// public static void LaunchUrl(string url) { - var startInfo = new ProcessStartInfo + if (!LinkOpener.TryOpen(url, out var error)) { - UseShellExecute = true, - FileName = url - }; - try - { - using var process = Process.Start(startInfo); - } - catch (Exception exception) - { - Log.Error(exception, "Failed to open {Url}", url); + Log.Error(error, "Failed to open {Url}", url); } } } diff --git a/src/DiffEngineTray/MenuBuilder.cs b/src/DiffEngineTray/MenuBuilder.cs index bcb06ba5..d8d85896 100644 --- a/src/DiffEngineTray/MenuBuilder.cs +++ b/src/DiffEngineTray/MenuBuilder.cs @@ -7,35 +7,6 @@ public static ContextMenuStrip Build(Action exit, Action launchOptions, Tracker DefaultDropDownDirection = ToolStripDropDownDirection.AboveLeft }; var items = menu.Items; - // The items that survive a close, held as themselves. They used to be told apart by their - // text, so a solution named after one of them - Options, say - gave that group a header - // that was never removed, and another one on every open after that - var fixedItems = new HashSet(); - // Removed while closing and disposed at the next open, rather than disposed while the - // close they were removed by is still running - var removed = new List(); - menu.Closed += delegate - { - foreach (var item in TrackingMenuItems(items, fixedItems)) - { - items.Remove(item); - removed.Add(item); - } - }; - menu.Opening += delegate - { - foreach (var item in removed) - { - item.Dispose(); - } - - removed.Clear(); - - foreach (var item in BuildTrackingMenuItems(tracker)) - { - items.Add(item); - } - }; menu.Font = new(menu.Font.FontFamily, 10); items.Add(new MenuButton("Exit", exit, Images.Exit)); items.Add(new MenuButton("Options", launchOptions, Images.Options)); @@ -43,20 +14,21 @@ public static ContextMenuStrip Build(Action exit, Action launchOptions, Tracker items.Add(new MenuButton("Open logs", Logging.OpenDirectory, Images.Folder)); items.Add(new MenuButton("Purge verified files", FilePurger.Launch, Images.Folder)); items.Add(new MenuButton("Raise issue", IssueLauncher.Launch, Images.Link)); - fixedItems.UnionWith(items.Cast()); + // After the fixed items, which it keeps as themselves: told apart by their text, a + // solution named after one of them - Options, say - gave that group a header that was + // never removed. The tracked ones are built again on every open, and disposed at the next. + MenuOnOpening.Rebuild( + menu, + _ => + { + foreach (var item in BuildTrackingMenuItems(tracker)) + { + _.Add(item); + } + }); return menu; } - /// - /// Everything the last open added, which is everything but the items put - /// there. Materialised, because the caller is removing them from the collection it reads. - /// - static List TrackingMenuItems(ToolStripItemCollection items, HashSet fixedItems) => - items - .Cast() - .Where(_ => !fixedItems.Contains(_)) - .ToList(); - internal static IEnumerable BuildTrackingMenuItems(Tracker tracker) { // Read everything first and decide from the counts. TrackingAny is backed by the scan diff --git a/src/DiffEngineTray/Program.cs b/src/DiffEngineTray/Program.cs index 5efc8c61..30088ae3 100644 --- a/src/DiffEngineTray/Program.cs +++ b/src/DiffEngineTray/Program.cs @@ -108,7 +108,9 @@ void Warn(string message) => // A logoff or a shutdown does not come back through Application.Run(), so nothing the // unwind below performs happens for one - using var sessionEnd = new SessionEndWindow(SessionEnding(owned, TrayVersionFile.Delete)); + using var sessionEnd = new SessionEndWindow( + SessionEnding(owned, TrayVersionFile.Delete), + _ => Log.Error(_, "Failed while the session was ending")); // Not a using. Anything throwing between here and the await below would dispose a task // that is still running, and Task.Dispose throws for one that has not completed - which diff --git a/src/DiffEngineTray/SessionEndWindow.cs b/src/DiffEngineTray/SessionEndWindow.cs deleted file mode 100644 index e3500067..00000000 --- a/src/DiffEngineTray/SessionEndWindow.cs +++ /dev/null @@ -1,63 +0,0 @@ -/// -/// Hears that the Windows session is ending, which is how a tray started at login usually stops. -/// -/// Nothing else here would. A logoff or a shutdown sends every top level window -/// WM_QUERYENDSESSION and then WM_ENDSESSION, and Windows may end the process as soon as those are -/// answered. A Form turns them into a close, which is how the viewer hears of it. The tray has no -/// Form, only the notify icon's window, which agrees to the query and does nothing more: so -/// Application.Run() never returns, nothing after it in Program runs, and the queue an -/// owning tray holds in memory went with the process where a clean exit stages it. -/// -/// -/// A window of its own, because the two messages are sent rather than posted, which a message -/// filter never sees. Top level, because a message only window is not sent them. Never shown. -/// -/// -/// On WM_ENDSESSION rather than on the query before it. Another application can still refuse the -/// query, and a session that then carries on would have a queue both staged on disk and held here. -/// -/// -sealed class SessionEndWindow : - NativeWindow, - IDisposable -{ - const int endSession = 0x0016; - - readonly Action ending; - - /// - /// Run inside the message, on the thread this is created on, so whatever it does is done by - /// the time the message is answered: there may be no time after that. - /// - public SessionEndWindow(Action ending) - { - this.ending = ending; - CreateHandle(new()); - } - - protected override void WndProc(ref Message message) - { - // WParam is zero when the session is not ending after all, which everything that agreed - // to the query is told once something else has refused it - if (message.Msg == endSession && - message.WParam != IntPtr.Zero) - { - try - { - ending(); - } - catch (Exception exception) - { - // Logged and nothing more. Thrown out of here it reaches WinForms' exception - // dialog, which during a logoff is a window holding the session open with nobody - // left to answer it - Log.Error(exception, "Failed while the session was ending"); - } - } - - base.WndProc(ref message); - } - - public void Dispose() => - DestroyHandle(); -} diff --git a/src/Directory.Packages.props b/src/Directory.Packages.props index 9eeed207..3308efac 100644 --- a/src/Directory.Packages.props +++ b/src/Directory.Packages.props @@ -19,9 +19,9 @@ --> - - - + + + diff --git a/src/Packaging.Tests/Package.DiffEngineTray.verified.txt b/src/Packaging.Tests/Package.DiffEngineTray.verified.txt index 6118a801..ecac32b3 100644 --- a/src/Packaging.Tests/Package.DiffEngineTray.verified.txt +++ b/src/Packaging.Tests/Package.DiffEngineTray.verified.txt @@ -11,6 +11,8 @@ tools/net10.0/any/DiffEngineTray.dll tools/net10.0/any/DiffEngineTray.runtimeconfig.json tools/net10.0/any/DotnetToolSettings.xml tools/net10.0/any/EmptyFiles.dll +tools/net10.0/any/PlatformHeads.Core.dll +tools/net10.0/any/PlatformHeads.WindowsForms.dll tools/net10.0/any/Serilog.Sinks.File.dll tools/net10.0/any/Serilog.dll tools/net10.0/any/viewer/documents/AngleSharp.dll