diff --git a/src/DiffEngineTray.Tests/DiffToolLauncherFlagsTest.cs b/src/DiffEngineTray.Tests/DiffToolLauncherFlagsTest.cs
new file mode 100644
index 00000000..0aea8f40
--- /dev/null
+++ b/src/DiffEngineTray.Tests/DiffToolLauncherFlagsTest.cs
@@ -0,0 +1,77 @@
+///
+/// The start flags the tray relaunches a tracked move with, which have to be the tool's own rather
+/// than a fixed pair.
+///
+/// They were fixed at ShellExecute with no CreateNoWindow, so a console subsystem tool - the
+/// bundled viewer is one - came up from "Open diff tool" with a console attached, while
+/// DiffEngine's own launch of the very same tool did not. The window that console puts on screen
+/// is zero sized and never activates, so nobody saw it; what it left behind was a conhost process
+/// per relaunch and two launch paths that disagreed.
+///
+///
+public class DiffToolLauncherFlagsTest :
+ IDisposable
+{
+ [Test]
+ public async Task AToolsOwnFlagsAreUsed()
+ {
+ var registered = DiffTools.AddTool(
+ name: "FakeConsoleTool",
+ autoRefresh: false,
+ isMdi: false,
+ supportsText: true,
+ requiresTarget: false,
+ useShellExecute: false,
+ launchArguments: new(
+ Left: (temp, target) => $"\"{target}\" \"{temp}\"",
+ Right: (temp, target) => $"\"{temp}\" \"{target}\""),
+ exePath: exe,
+ binaryExtensions: [],
+ createNoWindow: true);
+
+ await Assert.That(registered).IsNotNull();
+ await Assert.That(DiffToolLauncher.FlagsFor(exe)).IsEqualTo((false, true));
+ }
+
+ ///
+ /// The path a move carries is the sending process's. For the bundled viewer that is inside
+ /// that project's package folder, which this process has never looked in, so the exact path
+ /// misses and the executable's name is what is left to go on.
+ ///
+ [Test]
+ public async Task TheSameToolAtAnotherPathIsStillThatTool()
+ {
+ DiffTools.AddTool(
+ name: "FakeConsoleTool",
+ autoRefresh: false,
+ isMdi: false,
+ supportsText: true,
+ requiresTarget: false,
+ useShellExecute: false,
+ launchArguments: new(
+ Left: (temp, target) => $"\"{target}\" \"{temp}\"",
+ Right: (temp, target) => $"\"{temp}\" \"{target}\""),
+ exePath: exe,
+ binaryExtensions: [],
+ createNoWindow: true);
+
+ var elsewhere = Path.Combine(@"c:\somewhere\else", Path.GetFileName(exe));
+
+ await Assert.That(DiffToolLauncher.FlagsFor(elsewhere)).IsEqualTo((false, true));
+ }
+
+ ///
+ /// Nothing resolves for a tool that is no longer installed, or an exe a payload named that
+ /// never was one. That keeps what this has always done, and ShellExecute is the safe end of
+ /// it: without it the launched tool inherits the launching process's handles.
+ ///
+ [Test]
+ public async Task AnUnknownExeKeepsTheOldPair() =>
+ await Assert.That(DiffToolLauncher.FlagsFor(@"c:\nothing\here.exe")).IsEqualTo((true, false));
+
+ readonly string exe = Environment.ProcessPath!;
+
+ public void Dispose() =>
+ // Registered into the static lookup, so the rest of the run has to get it back.
+ DiffTools.Reset();
+}
diff --git a/src/DiffEngineTray/DiffToolLauncher.cs b/src/DiffEngineTray/DiffToolLauncher.cs
index 9931dcb2..b5b3a45c 100644
--- a/src/DiffEngineTray/DiffToolLauncher.cs
+++ b/src/DiffEngineTray/DiffToolLauncher.cs
@@ -6,6 +6,47 @@ static class DiffToolLauncher
public static void Launch(TrackedMove move) =>
Launch(move.Exe!, move.Arguments!, move.CanKill, move.Process, _ => move.Process = _);
+ ///
+ /// The two start flags the tool itself declares, which is what DiffRunner.LaunchProcess
+ /// launches it with. Hard coded here before, so a console subsystem tool - the bundled viewer
+ /// is one - was started without CreateNoWindow and came up with a console attached, which
+ /// DiffEngine's own launch of the same tool does not do.
+ ///
+ /// Resolved by path rather than carried on the move, because the payload the tray receives has
+ /// no room for them: PiperServer's format is frozen, every stable DiffEngine embeds the client
+ /// that writes it, and a new field would be read as nothing by all of them.
+ ///
+ ///
+ /// By name when the path misses, which for the bundled viewer it does: the path a move carries
+ /// is the sending process's, and that one sits inside that project's package folder, somewhere
+ /// this process has never looked. The same tool found somewhere else is still that tool, and
+ /// its start flags belong to the executable rather than to where it was installed.
+ ///
+ ///
+ /// An exe that resolves to neither keeps what this always did. ShellExecute is the safe end of
+ /// that: a tool started without it inherits the launching process's handles, which is what
+ /// is about.
+ ///
+ ///
+ internal static (bool useShellExecute, bool createNoWindow) FlagsFor(string exe)
+ {
+ if (DiffTools.TryFindByPath(exe, out var tool))
+ {
+ return (tool.UseShellExecute, tool.CreateNoWindow);
+ }
+
+ var name = Path.GetFileName(exe);
+ foreach (var candidate in DiffTools.Resolved)
+ {
+ if (string.Equals(Path.GetFileName(candidate.ExePath), name, StringComparison.OrdinalIgnoreCase))
+ {
+ return (candidate.UseShellExecute, candidate.CreateNoWindow);
+ }
+ }
+
+ return (true, false);
+ }
+
static void Launch(string exe, string arguments, bool canKill, Process? process, Action assign)
{
if (process is { HasExited: false })
@@ -24,12 +65,11 @@ static void Launch(string exe, string arguments, bool canKill, Process? process,
process?.Dispose();
assign(null);
+ var (useShellExecute, createNoWindow) = FlagsFor(exe);
var startInfo = new ProcessStartInfo(exe, arguments)
{
- // Given the full exe path is known we dont need UseShellExecute https://stackoverflow.com/a/5255335
- // however UseShellExecute allows the test running to not block when the difftool is launched
- // https://github.com/VerifyTests/Verify/issues/1229
- UseShellExecute = true
+ UseShellExecute = useShellExecute,
+ CreateNoWindow = createNoWindow
};
try