Fix: Copilot feedback

Author: BornToBeRoot

.github/dependabot.yml

@@ -25,3 +25,9 @@ updates:
   target-branch: main
   commit-message:
     prefix: "Dependency (dependabot/npm):"
+  groups:
+    # react and react-dom must have the exact same version (otherwise the website build fails)
+    react:
+      patterns:
+        - "react"
+        - "react-dom"

Source/NETworkManager.Controls/ComboBoxHistoryBehavior.cs

@@ -0,0 +1,69 @@
+using System;
+using System.Windows;
+using System.Windows.Controls;
+using System.Windows.Threading;
+
+namespace NETworkManager.Controls;
+
+/// <summary>
+/// Attached property that keeps the text of an editable <see cref="ComboBox"/> that shows a history (e.g. the hosts of
+/// the Traceroute) when its selected entry is removed from the history.
+///
+/// The history is changed in place (<c>ListHelper.Modify</c>): the oldest entries are removed and, if the number of
+/// stored entries is 0, all of them. If the removed entry is the selected item (e.g. the host that was just used, or the
+/// host of another tab that shares the history), WPF clears the text of the ComboBox. The text is restored after the
+/// ComboBox has updated it. If the entry is only deselected because another text was typed, it is still in the list and
+/// nothing is changed.
+/// </summary>
+public static class ComboBoxHistoryBehavior
+{
+    public static readonly DependencyProperty KeepTextWhenSelectedItemIsRemovedProperty =
+        DependencyProperty.RegisterAttached(
+            "KeepTextWhenSelectedItemIsRemoved",
+            typeof(bool),
+            typeof(ComboBoxHistoryBehavior),
+            new PropertyMetadata(false, OnKeepTextWhenSelectedItemIsRemovedChanged));
+
+    public static void SetKeepTextWhenSelectedItemIsRemoved(UIElement element, bool value) =>
+        element.SetValue(KeepTextWhenSelectedItemIsRemovedProperty, value);
+
+    public static bool GetKeepTextWhenSelectedItemIsRemoved(UIElement element) =>
+        (bool)element.GetValue(KeepTextWhenSelectedItemIsRemovedProperty);
+
+    private static void OnKeepTextWhenSelectedItemIsRemovedChanged(DependencyObject d,
+        DependencyPropertyChangedEventArgs e)
+    {
+        if (d is not ComboBox comboBox)
+            return;
+
+        if ((bool)e.NewValue)
+            comboBox.SelectionChanged += ComboBox_SelectionChanged;
+        else
+            comboBox.SelectionChanged -= ComboBox_SelectionChanged;
+    }
+
+    private static void ComboBox_SelectionChanged(object sender, SelectionChangedEventArgs e)
+    {
+        if (sender is not ComboBox { IsEditable: true } comboBox || comboBox.SelectedItem != null ||
+            e.RemovedItems.Count == 0)
+            return;
+
+        var removedItem = e.RemovedItems[0];
+
+        // Deselected because another text was typed: the entry is still in the list.
+        if (comboBox.Items.Contains(removedItem))
+            return;
+
+        var text = removedItem?.ToString();
+
+        if (string.IsNullOrEmpty(text))
+            return;
+
+        // After the ComboBox has cleared its text (and only if it did)
+        comboBox.Dispatcher.BeginInvoke(DispatcherPriority.DataBind, new Action(delegate
+        {
+            if (string.IsNullOrEmpty(comboBox.Text))
+                comboBox.Text = text;
+        }));
+    }
+}

Source/NETworkManager.Controls/ObservableRangeCollection.cs

@@ -2,6 +2,7 @@
 using System.Collections.ObjectModel;
 using System.Collections.Specialized;
 using System.ComponentModel;
+using System.Linq;
 
 namespace NETworkManager.Controls;
 
@@ -23,9 +24,12 @@ public void ReplaceAll(IEnumerable<T> items)
     {
         CheckReentrancy();
 
+        // Read the items first: if a lazy sequence throws, the collection is unchanged (and in sync with the views).
+        var newItems = items.ToList();
+
         Items.Clear();
 
-        foreach (var item in items)
+        foreach (var item in newItems)
             Items.Add(item);
 
         OnPropertyChanged(new PropertyChangedEventArgs(nameof(Count)));

Source/NETworkManager.Models/RunCommand/RunCommandTarget.cs

@@ -13,6 +13,17 @@ namespace NETworkManager.Models.RunCommand;
 /// </summary>
 public static class RunCommandTarget
 {
+    /// <summary>
+    ///     Checks if the ports of an entry of the port lookup (e.g. "22", "80-90/tcp") are valid: from 1 to 65535, a range
+    ///     from a lower to a higher port.
+    /// </summary>
+    private static bool IsValidPortLookupEntry(string entry)
+    {
+        var ports = entry.Split('/')[0].Split('-').Select(x => int.TryParse(x, out var port) ? port : 0).ToArray();
+
+        return ports.All(x => x is > 0 and < 65536) && (ports.Length == 1 || ports[0] < ports[1]);
+    }
+
     /// <summary>
     ///     Gets the kind of the target.
     /// </summary>
@@ -33,11 +44,10 @@ public static RunCommandTargetKind GetKind(string target)
         if (RegexHelper.MACAddressFirst3BytesWithSeparatorRegex().IsMatch(target))
             return RunCommandTargetKind.OUI;
 
-        // Only ports from 1 to 65535 (e.g. not "99999", which the port lookup searches as a service)
+        // Like the port lookup: ports from 1 to 65535 and ranges from a lower to a higher port (e.g. not "99999" or
+        // "90-80", which the port lookup searches as a service)
         if (RegexHelper.PortLookupEntriesRegex().IsMatch(target))
-            return Regex.Matches(target, "[0-9]+").All(x => int.TryParse(x.Value, out var port) && port is > 0 and < 65536)
-                ? RunCommandTargetKind.Ports
-                : RunCommandTargetKind.None;
+            return target.Split(';').All(IsValidPortLookupEntry) ? RunCommandTargetKind.Ports : RunCommandTargetKind.None;
 
         if (RegexHelper.ASNumberWithPrefixRegex().IsMatch(target))
             return RunCommandTargetKind.ASN;

Source/NETworkManager.Tests/RunCommand/RunCommandTargetTests.cs

@@ -72,6 +72,10 @@ public void HostsWithHostnames(string target)
     [InlineData("0")]
     [InlineData("99999")]
     [InlineData("22;99999")]
+    // A range from a lower to a higher port, like the port lookup
+    [InlineData("90-80")]
+    [InlineData("80-80")]
+    [InlineData("22;90-80/tcp")]
     // A shortened or partly typed IPv4 address
     [InlineData("10.1")]
     [InlineData("192.168.1")]

Source/NETworkManager.Tests/Utilities/ListHelperTests.cs

@@ -117,9 +117,20 @@ public void UsesTheComparerAndKeepsTheLatestSpelling()
         Assert.Equal(["Ping 10.0.0.1", "a"], list);
     }
 
+    // "Number of stored entries" can be set to 0 in the settings.
     [Fact]
-    public void ThrowsIfTheLengthIsNotPositive()
+    public void LengthZeroStoresNoEntries()
     {
-        Assert.Throws<ArgumentOutOfRangeException>(() => ListHelper.Modify(new ObservableCollection<string>(), "a", 0));
+        var list = new ObservableCollection<string> { "a", "b" };
+
+        ListHelper.Modify(list, "c", 0);
+
+        Assert.Empty(list);
+    }
+
+    [Fact]
+    public void ThrowsIfTheLengthIsNegative()
+    {
+        Assert.Throws<ArgumentOutOfRangeException>(() => ListHelper.Modify(new ObservableCollection<string>(), "a", -1));
     }
 }

Source/NETworkManager.Utilities/ListHelper.cs

@@ -18,16 +18,29 @@ public static class ListHelper
     /// </summary>
     /// <param name="list">List to modify (e.g. a history of the settings).</param>
     /// <param name="entry">Entry to add to the list.</param>
-    /// <param name="length">Maximum length of the list. Oldest entries will be removed if the list exceeds this length.</param>
+    /// <param name="length">
+    ///     Maximum length of the list. Oldest entries will be removed if the list exceeds this length. 0 stores no
+    ///     entries (e.g. "Number of stored entries" is set to 0), the list is emptied.
+    /// </param>
     /// <param name="comparer">Comparer for existing entries (e.g. case-insensitive). If null, the default comparer is used.</param>
     /// <typeparam name="T">Type of the list entries. Currently <see cref="string"/> or <see cref="int"/>.</typeparam>
     public static void Modify<T>(ObservableCollection<T> list, T entry, int length,
         IEqualityComparer<T> comparer = null)
     {
         comparer ??= EqualityComparer<T>.Default;
 
-        if (length <= 0)
-            throw new ArgumentOutOfRangeException(nameof(length), "Length must be greater than zero.");
+        if (length < 0)
+            throw new ArgumentOutOfRangeException(nameof(length), "Length must not be negative.");
+
+        // No history: all entries are removed, one by one like the oldest entries below. An editable ComboBox that shows
+        // the list keeps its text (ComboBoxHistoryBehavior in the HistoryComboBox style).
+        if (length == 0)
+        {
+            while (list.Count > 0)
+                list.RemoveAt(list.Count - 1);
+
+            return;
+        }
 
         if (EqualityComparer<T>.Default.Equals(entry, default) || entry is string text && string.IsNullOrEmpty(text))
             return;

Source/NETworkManager/Resources/Styles/ComboBoxStyles.xaml

@@ -1,6 +1,7 @@
 <ResourceDictionary xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
                     xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
-                    xmlns:mah="http://metro.mahapps.com/winfx/xaml/controls">
+                    xmlns:mah="http://metro.mahapps.com/winfx/xaml/controls"
+                    xmlns:controls="clr-namespace:NETworkManager.Controls;assembly=NETworkManager.Controls">
     <Style x:Key="DefaultComboBox" TargetType="{x:Type ComboBox}" BasedOn="{StaticResource MahApps.Styles.ComboBox}">
         <Setter Property="FontSize" Value="14" />
         <!--<Setter Property="BorderThickness" Value="0,0,0,1" />-->
@@ -17,6 +18,8 @@
     </Style>
     <Style x:Key="HistoryComboBox" TargetType="{x:Type ComboBox}" BasedOn="{StaticResource DefaultComboBox}">
         <Setter Property="IsEditable" Value="True" />
+        <!-- The history is changed in place, removing the selected entry would clear the text -->
+        <Setter Property="controls:ComboBoxHistoryBehavior.KeepTextWhenSelectedItemIsRemoved" Value="True" />
         <Setter Property="ContextMenu" Value="{StaticResource CutCopyPasteContextMenu}" />
     </Style>
 </ResourceDictionary>

Source/NETworkManager/RunCommandManager.cs

@@ -51,31 +51,27 @@ public static class RunCommandManager
     /// </summary>
     private static readonly Dictionary<RunCommandTargetKind, ApplicationName[]> TargetApplications = new()
     {
+        // The Port Scanner is not suggested, since it needs ports (or a port profile).
         [RunCommandTargetKind.Host] =
         [
             ApplicationName.PingMonitor, ApplicationName.Traceroute, ApplicationName.DNSLookup,
-            ApplicationName.PortScanner, ApplicationName.RemoteDesktop, ApplicationName.PuTTY,
-            ApplicationName.PowerShell, ApplicationName.TigerVNC, ApplicationName.IPGeolocation, ApplicationName.Whois,
-            ApplicationName.RDAP, ApplicationName.SNMP, ApplicationName.IPScanner
+            ApplicationName.RemoteDesktop, ApplicationName.PuTTY, ApplicationName.PowerShell, ApplicationName.TigerVNC,
+            ApplicationName.IPGeolocation, ApplicationName.Whois, ApplicationName.RDAP, ApplicationName.SNMP,
+            ApplicationName.IPScanner
         ],
         // Without a domain (e.g. in a company network): no tools for public hosts (Whois, RDAP, IP Geolocation)
         [RunCommandTargetKind.Hostname] =
         [
             ApplicationName.PingMonitor, ApplicationName.Traceroute, ApplicationName.DNSLookup,
-            ApplicationName.PortScanner, ApplicationName.RemoteDesktop, ApplicationName.PuTTY,
-            ApplicationName.PowerShell, ApplicationName.TigerVNC, ApplicationName.SNMP, ApplicationName.IPScanner
-        ],
-        [RunCommandTargetKind.Hosts] =
-        [
-            ApplicationName.PingMonitor, ApplicationName.DNSLookup, ApplicationName.PortScanner,
-            ApplicationName.IPScanner
+            ApplicationName.RemoteDesktop, ApplicationName.PuTTY, ApplicationName.PowerShell, ApplicationName.TigerVNC,
+            ApplicationName.SNMP, ApplicationName.IPScanner
         ],
-        [RunCommandTargetKind.IPRange] =
-            [ApplicationName.IPScanner, ApplicationName.PortScanner, ApplicationName.PingMonitor],
+        [RunCommandTargetKind.Hosts] = [ApplicationName.PingMonitor, ApplicationName.DNSLookup, ApplicationName.IPScanner],
+        [RunCommandTargetKind.IPRange] = [ApplicationName.IPScanner, ApplicationName.PingMonitor],
         [RunCommandTargetKind.Subnet] =
         [
-            ApplicationName.IPScanner, ApplicationName.SubnetCalculator, ApplicationName.PortScanner,
-            ApplicationName.PingMonitor, ApplicationName.RDAP
+            ApplicationName.IPScanner, ApplicationName.SubnetCalculator, ApplicationName.PingMonitor,
+            ApplicationName.RDAP
         ],
         [RunCommandTargetKind.MACAddress] = [ApplicationName.WakeOnLAN, ApplicationName.Lookup],
         [RunCommandTargetKind.OUI] = [ApplicationName.Lookup],