From cbf5a3094fbb8f8ce7e519b25b6f2d439a642f11 Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Sat, 22 Aug 2026 12:43:48 +1000 Subject: [PATCH] Resolve the same tool synchronously and asynchronously Launch asked TryFindByExtension and LaunchAsync asks TryFindForInputFilePath. The two differ: only the second can see a text file convention, since the first is handed an extension and a convention matches on the whole path. So a file matched by a convention rather than by its extension opened a diff tool through LaunchAsync and reported NoDiffToolFound through Launch. Kill used the extension lookup too, so it then logged that it could not find a tool for a pair LaunchAsync had opened - leaving the tool on screen for a test that had started passing, which is the one thing Kill exists to prevent. Both now go through TryFindForInputFilePath. Kill's log message changes with it, since "Extension not found" was already describing a lookup it no longer does. The test pins the gap between the two lookups rather than driving DiffRunner. A convention matched file can only resolve to the first text tool installed on the machine - a fake one cannot be registered for it - so a behavioural test would launch the developer's real diff tool. The change itself is which of the two calls these make, and is verified by reading. --- .../TextFileConventionResolutionTests.cs | 48 +++++++++++++++++++ src/DiffEngine/DiffRunner.cs | 8 ++-- src/DiffEngine/DiffRunner_Kill.cs | 9 ++-- 3 files changed, 58 insertions(+), 7 deletions(-) create mode 100644 src/DiffEngine.Tests/TextFileConventionResolutionTests.cs diff --git a/src/DiffEngine.Tests/TextFileConventionResolutionTests.cs b/src/DiffEngine.Tests/TextFileConventionResolutionTests.cs new file mode 100644 index 00000000..2cc7f5b9 --- /dev/null +++ b/src/DiffEngine.Tests/TextFileConventionResolutionTests.cs @@ -0,0 +1,48 @@ +/// +/// A text file convention is invisible to the extension lookup, which is why Launch and Kill had +/// to stop using it. +/// +/// Launch asked TryFindByExtension, which can only consult IsTextExtension, while LaunchAsync asks +/// TryFindForInputFilePath, which honours a text file convention. So a file matched by a +/// convention rather than by its extension - a name with no extension, a dotfile - opened a diff +/// tool asynchronously and reported NoDiffToolFound synchronously, and Kill then logged that it +/// could not find one for a pair LaunchAsync had opened, leaving the tool on screen. +/// +/// +/// This pins the gap between the two lookups. It does not drive DiffRunner.Launch, because +/// resolving a convention matched file can only ever land on the first text tool installed on the +/// machine - there is no way to register a fake one for it - and a test that launches the +/// developer's real diff tool is not worth having. The change in Launch and Kill is a change of +/// which of these two calls they make, and is verified by reading. +/// +/// +[NotInParallel] +public class TextFileConventionResolutionTests +{ + [Test] + public async Task AConventionIsInvisibleToTheExtensionLookup() + { + var name = $"conventionprobe{Guid.NewGuid():N}"; + FileExtensions.AddTextFileConvention(path => Path.GetFileNameWithoutExtension(path).StartsWith(name, StringComparison.Ordinal)); + + var path = Path.Combine(Path.GetTempPath(), $"{name}.unknownextension"); + + // What LaunchAsync resolves with + var byPath = DiffTools.TryFindForInputFilePath(path, out var forAsync); + // What Launch and Kill used to resolve with + var byExtension = DiffTools.TryFindByExtension(Path.GetExtension(path), out _); + + if (!byPath) + { + // No text tool resolved on this machine, so there is nothing for either to find and + // the two cannot disagree + return; + } + + await Assert.That(forAsync).IsNotNull(); + // The gap itself: the convention is invisible to the extension lookup + await Assert.That(byExtension).IsFalse(); + + await Assert.That(forAsync!.SupportsText).IsTrue(); + } +} diff --git a/src/DiffEngine/DiffRunner.cs b/src/DiffEngine/DiffRunner.cs index c21fabac..2aaab7a4 100644 --- a/src/DiffEngine/DiffRunner.cs +++ b/src/DiffEngine/DiffRunner.cs @@ -42,10 +42,10 @@ public static LaunchResult Launch(string tempFile, string targetFile, Encoding? return InnerLaunch( ([NotNullWhen(true)] out tool) => - { - var extension = Path.GetExtension(tempFile); - return DiffTools.TryFindByExtension(extension, out tool); - }, + // The same resolution LaunchAsync uses. Asking by extension alone cannot see a + // text file convention, so a file matched by one launched asynchronously and + // reported NoDiffToolFound synchronously + DiffTools.TryFindForInputFilePath(tempFile, out tool), tempFile, targetFile, encoding); diff --git a/src/DiffEngine/DiffRunner_Kill.cs b/src/DiffEngine/DiffRunner_Kill.cs index 1e32eb32..d347262a 100644 --- a/src/DiffEngine/DiffRunner_Kill.cs +++ b/src/DiffEngine/DiffRunner_Kill.cs @@ -12,10 +12,13 @@ public static void Kill(string tempFile, string targetFile) return; } - var extension = Path.GetExtension(tempFile); - if (!DiffTools.TryFindByExtension(extension, out var diffTool)) + // TryFindForInputFilePath rather than by extension, so this resolves the same tool the + // launch did. By extension alone a file matched by a text file convention resolved to + // nothing here, and Kill logged "Extension not found" for a pair LaunchAsync had opened - + // leaving the tool on screen for a test that now passes + if (!DiffTools.TryFindForInputFilePath(tempFile, out var diffTool)) { - Logging.Write($"Extension not found. {extension}"); + Logging.Write($"No diff tool for. {tempFile}"); return; }