diff --git a/src/DiffEngine.Tests/ToolOrderDuplicateTests.cs b/src/DiffEngine.Tests/ToolOrderDuplicateTests.cs
new file mode 100644
index 00000000..5410db3b
--- /dev/null
+++ b/src/DiffEngine.Tests/ToolOrderDuplicateTests.cs
@@ -0,0 +1,57 @@
+///
+/// A repeated name in DiffEngine_ToolOrder.
+///
+/// Sort looked each requested tool up in Definitions, which holds every tool whether it is
+/// installed or not - so the lookup only ever failed on a second occurrence, because the first had
+/// removed it from the list. That was then reported as "is not installed", which was untrue, and
+/// since DiffTools resolves the order in a static constructor it was also permanent: every later
+/// use of DiffTools in that process threw TypeInitializationException.
+///
+///
+[NotInParallel]
+public class ToolOrderDuplicateTests :
+ IDisposable
+{
+ [Test]
+ public async Task ARepeatedToolIsNotAnUninstalledOne()
+ {
+ // An installed one, chosen from what this machine actually has, since throwForNoTool is
+ // the flag under test and naming an absent tool would throw for the right reason
+ var installed = DiffTools.Resolved.FirstOrDefault(_ => _.Tool != null)?.Tool;
+ if (installed == null)
+ {
+ // No built in tool resolved here, so there is nothing to repeat
+ return;
+ }
+
+ DiffTools.UseOrder(true, installed.Value, installed.Value);
+
+ // Still ordered, rather than dropped along with the duplicate
+ await Assert.That(DiffTools.Resolved.Any(_ => _.Tool == installed)).IsTrue();
+ }
+
+ ///
+ /// And the flag still means what it says for a tool that genuinely is not here.
+ ///
+ [Test]
+ public async Task AnUninstalledToolStillThrows()
+ {
+ var installed = DiffTools.Resolved
+ .Where(_ => _.Tool != null)
+ .Select(_ => _.Tool!.Value)
+ .ToHashSet();
+
+ var absent = Enum.GetValues().FirstOrDefault(_ => !installed.Contains(_));
+ if (installed.Count == Enum.GetValues().Length)
+ {
+ // Every tool installed, which no real machine is
+ return;
+ }
+
+ await Assert.That(() => DiffTools.UseOrder(true, absent))
+ .Throws();
+ }
+
+ public void Dispose() =>
+ DiffTools.Reset();
+}
diff --git a/src/DiffEngine/DiffTools_Add.cs b/src/DiffEngine/DiffTools_Add.cs
index 9b9af4db..2ca26ac6 100644
--- a/src/DiffEngine/DiffTools_Add.cs
+++ b/src/DiffEngine/DiffTools_Add.cs
@@ -118,10 +118,10 @@ static void InitTools(bool throwForNoTool, IEnumerable order)
firstTextTool = null;
resolved.Clear();
- foreach (var definition in ToolsOrder.Sort(throwForNoTool, order).Reverse())
+ foreach (var (definition, requested) in ToolsOrder.Sort(order).Reverse())
{
var tool = definition.Tool;
- AddTool(
+ var added = AddTool(
tool.ToString(),
tool,
definition.AutoRefresh,
@@ -133,6 +133,16 @@ static void InitTools(bool throwForNoTool, IEnumerable order)
definition.UseShellExecute,
definition.CreateNoWindow,
definition.KillLockingProcess);
+
+ // Here rather than in Sort, because this is where being installed is decided: Sort
+ // works from Definitions, which holds every tool whether it is on the machine or not,
+ // so it could never answer this question
+ if (added == null &&
+ requested &&
+ throwForNoTool)
+ {
+ throw new($"`DiffEngine_ToolOrder` is configured to use '{tool}' but it is not installed.");
+ }
}
custom.Reverse();
diff --git a/src/DiffEngine/ToolsOrder.cs b/src/DiffEngine/ToolsOrder.cs
index 2bac6e42..58904c02 100644
--- a/src/DiffEngine/ToolsOrder.cs
+++ b/src/DiffEngine/ToolsOrder.cs
@@ -1,28 +1,33 @@
static class ToolsOrder
{
- public static IEnumerable Sort(bool throwForNoTool, IEnumerable order)
+ ///
+ /// The requested tools first, in the order asked for, then everything else. Each is flagged
+ /// with whether it was asked for, so the caller can tell a tool the user named and could not
+ /// have from one that simply is not installed on this machine and was never mentioned.
+ ///
+ public static IEnumerable<(Definition Definition, bool Requested)> Sort(IEnumerable order)
{
var allTools = Definitions.Tools.ToList();
- foreach (var diffTool in order)
+ // Distinct, because a repeated name is a typo rather than a request for two of something.
+ // Without it the second occurrence found nothing - the first had already removed it - and
+ // that was reported as "is not installed", which was both untrue and, from a static
+ // constructor, permanent: DiffEngine_ToolOrder=VisualStudio,VisualStudio turned every
+ // later use of DiffTools into a TypeInitializationException
+ foreach (var diffTool in order.Distinct())
{
var definition = allTools.SingleOrDefault(_ => _.Tool == diffTool);
if (definition == null)
{
- if (!throwForNoTool)
- {
- continue;
- }
-
- throw new($"`DiffEngine_ToolOrder` is configured to use '{diffTool}' but it is not installed.");
+ continue;
}
- yield return definition;
+ yield return (definition, true);
allTools.Remove(definition);
}
foreach (var definition in allTools)
{
- yield return definition;
+ yield return (definition, false);
}
}
-}
\ No newline at end of file
+}