Take the tray pieces from PlatformHeads

.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

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

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)

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

src/DiffEngineTray.Tests/IssueLauncherTests.cs

@@ -30,15 +30,16 @@ public async Task An_ampersand_in_the_title_does_not_truncate_it()
     }
 
     /// <summary>
-    /// 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.
     /// </summary>
     [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<string, string> Query(string url) =>

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<string>) 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();

src/DiffEngineTray.Tests/SessionEndWindowTest.cs

@@ -1,46 +1,14 @@
 /// <summary>
 /// 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.
 /// <para>
 /// 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.
 /// </para>
 /// </summary>
 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);
-    }
-
-    /// <summary>
-    /// 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.
-    /// </summary>
-    [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);
-    }
-
     /// <summary>
     /// 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

src/DiffEngineTray/DiffEngineTray.csproj

@@ -26,8 +26,12 @@
     <None Remove="**\*.ico" />
     <None Remove="**\*.png" />
     <Using Include="DiffEngine" />
+    <Using Include="PlatformHeads" />
     <EmbeddedResource Include="Images\*.ico" />
     <EmbeddedResource Include="Images\*.png" />
+    <!-- The tray pieces BuildMonitor's tray shares: SessionEndWindow, MenuOnOpening, IssueLink
+         and LinkOpener. -->
+    <PackageReference Include="PlatformHeads.WindowsForms" />
     <PackageReference Include="Serilog" />
     <PackageReference Include="Serilog.Sinks.File" />
     <PackageReference Include="ProjectDefaults" PrivateAssets="all" />

src/DiffEngineTray/IssueLauncher.cs

@@ -1,29 +1,34 @@
 static class IssueLauncher
 {
-    static ConcurrentBag<string> 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"));
 
     /// <summary>
-    /// 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 '&amp;' started a parameter GitHub does not have, truncating the title there.
+    /// Marks <paramref name="message"/> as already offered, so a test that makes it happen is not
+    /// left waiting on the modal box nobody is there to answer.
+    /// </summary>
+    internal static void AlreadyRaised(string message) =>
+        link.FirstTime(message);
+
+    /// <summary>
+    /// The title and body are encoded by <see cref="IssueLink"/>, so a path's '#' or '&amp;' in
+    /// the title neither drops the body nor cuts the title short. <paramref name="extraBody"/> is
+    /// plain text.
     /// </summary>
     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,21 +47,21 @@ Open an issue on GitHub?
             return;
         }
 
-        var extraBody = WebUtility.UrlEncode(
+        var extraBody =
             $"""
 
               * Action: {message}
               * Exception:
              ```
              {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;
-    }
 }

src/DiffEngineTray/LinkLauncher.cs

@@ -1,25 +1,15 @@
 static class LinkLauncher
 {
     /// <summary>
-    /// 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 <see cref="LinkOpener"/>. 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.
     /// </summary>
     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);
         }
     }
 }

src/DiffEngineTray/MenuBuilder.cs

@@ -7,56 +7,28 @@ 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<ToolStripItem>();
-        // 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<ToolStripItem>();
-        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));
         items.Add(new MenuButton("Debug view", async () => await DebugFormLauncher.Launch(tracker), Images.Options));
         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<ToolStripItem>());
+        // 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;
     }
 
-    /// <summary>
-    /// Everything the last open added, which is everything but the items <see cref="Build" /> put
-    /// there. Materialised, because the caller is removing them from the collection it reads.
-    /// </summary>
-    static List<ToolStripItem> TrackingMenuItems(ToolStripItemCollection items, HashSet<ToolStripItem> fixedItems) =>
-        items
-            .Cast<ToolStripItem>()
-            .Where(_ => !fixedItems.Contains(_))
-            .ToList();
-
     internal static IEnumerable<ToolStripItem> BuildTrackingMenuItems(Tracker tracker)
     {
         // Read everything first and decide from the counts. TrackingAny is backed by the scan

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