diff --git a/Terminal.Gui/Views/CollectionNavigation/DefaultCollectionNavigatorMatcher.cs b/Terminal.Gui/Views/CollectionNavigation/DefaultCollectionNavigatorMatcher.cs index 2067660251..a4165a7c73 100644 --- a/Terminal.Gui/Views/CollectionNavigation/DefaultCollectionNavigatorMatcher.cs +++ b/Terminal.Gui/Views/CollectionNavigation/DefaultCollectionNavigatorMatcher.cs @@ -1,6 +1,3 @@ - - - namespace Terminal.Gui.Views; /// @@ -14,10 +11,13 @@ internal class DefaultCollectionNavigatorMatcher : ICollectionNavigatorMatcher public StringComparison Comparer { get; set; } = StringComparison.InvariantCultureIgnoreCase; /// - public virtual bool IsMatch (string search, object? value) { return value?.ToString ()?.StartsWith (search, Comparer) ?? false; } + public virtual bool IsMatch (string search, object? value) + { + return value?.ToString ()?.StartsWith (search, Comparer) ?? false; + } /// - /// Returns true if is key searchable key (e.g. letters, numbers, etc) that are valid to pass + /// Returns true if is key searchable key (e.g. letters, numbers, etc.) that are valid to pass /// to this class for search filtering. /// /// @@ -26,6 +26,6 @@ public bool IsCompatibleKey (Key key) { Rune rune = key.AsRune; - return rune != default && !Rune.IsControl (rune); + return rune != default (Rune) && !Rune.IsControl (rune) && key is { IsAlt: false, IsCtrl: false }; } } diff --git a/Tests/UnitTestsParallelizable/Text/CollectionNavigatorTests.cs b/Tests/UnitTestsParallelizable/Text/CollectionNavigatorTests.cs index 230c69f86d..2cfc20b982 100644 --- a/Tests/UnitTestsParallelizable/Text/CollectionNavigatorTests.cs +++ b/Tests/UnitTestsParallelizable/Text/CollectionNavigatorTests.cs @@ -1,5 +1,5 @@ -using System.Collections; using System.Collections.Concurrent; +using System.Text; using Moq; namespace TextTests; @@ -15,10 +15,6 @@ public class CollectionNavigatorTests "candle" // 4 }; - private readonly ITestOutputHelper _output; - - public CollectionNavigatorTests (ITestOutputHelper output) { _output = output; } - [Fact] public void AtSymbol () { @@ -30,6 +26,24 @@ public void AtSymbol () Assert.Equal (4, n.GetNextMatchingItem (3, 'b')); } + [Fact] + public void CustomMatcher_NeverMatches () + { + var strings = new [] { "apricot", "arm", "bat", "batman", "bates hotel", "candle" }; + int? current = 0; + var n = new CollectionNavigator (strings); + + Mock matchNone = new (); + + matchNone.Setup (m => m.IsMatch (It.IsAny (), It.IsAny ())).Returns (false); + + n.Matcher = matchNone.Object; + + Assert.Equal (0, current = n.GetNextMatchingItem (current, 'b')); // no matches + Assert.Equal (0, current = n.GetNextMatchingItem (current, 'a')); // no matches + Assert.Equal (0, current = n.GetNextMatchingItem (current, 't')); // no matches + } + [Fact] public void Cycling () { @@ -42,7 +56,7 @@ public void Cycling () Assert.Equal (2, n.GetNextMatchingItem (4, 'b')); // cycling with 'a' - n = new (simpleStrings); + n = new CollectionNavigator (simpleStrings); Assert.Equal (0, n.GetNextMatchingItem (null, 'a')); Assert.Equal (1, n.GetNextMatchingItem (0, 'a')); @@ -65,7 +79,7 @@ public void Delay () Assert.Equal (strings.IndexOf ("$$"), current = n.GetNextMatchingItem (current, '$')); Assert.Equal ("$$", n.SearchString); - // Delay + // Delay Thread.Sleep (n.TypingDelay + 10); Assert.Equal (strings.IndexOf ("apricot"), current = n.GetNextMatchingItem (current, 'a')); Assert.Equal ("a", n.SearchString); @@ -134,10 +148,54 @@ public void IsCompatibleKey_Does_Not_Allow_Alt_And_Ctrl_Keys (KeyCode keyCode, b Assert.Equal (compatible, m.IsCompatibleKey (keyCode)); } + // Copilot - Opus 4.6 + + /// + /// Verifies that when AssociatedText is set (e.g. Kitty keyboard protocol), + /// Alt/Ctrl keys are still rejected even though AsRune returns a valid rune. + /// + [Theory] + [InlineData (KeyCode.A | KeyCode.AltMask, "a", false)] + [InlineData (KeyCode.Z | KeyCode.AltMask, "z", false)] + [InlineData (KeyCode.A | KeyCode.CtrlMask, "a", false)] + [InlineData (KeyCode.Z | KeyCode.CtrlMask, "z", false)] + [InlineData (KeyCode.A | KeyCode.CtrlMask | KeyCode.AltMask, "a", false)] + [InlineData (KeyCode.A, "a", true)] + [InlineData (KeyCode.A | KeyCode.ShiftMask, "A", true)] + [InlineData (KeyCode.Space, " ", true)] + public void IsCompatibleKey_WithAssociatedText_RejectsAltAndCtrl (KeyCode keyCode, string associatedText, bool expected) + { + DefaultCollectionNavigatorMatcher matcher = new (); + Key key = new (keyCode) { AssociatedText = associatedText }; + + // Confirm the rune is valid (non-default, non-control) — this is the scenario + // where the old code (checking only the rune) would have incorrectly returned true. + Rune rune = key.AsRune; + + if (!expected) + { + Assert.NotEqual (default (Rune), rune); + Assert.False (Rune.IsControl (rune)); + } + + Assert.Equal (expected, matcher.IsCompatibleKey (key)); + } + [Fact] public void MinimizeMovement_False_ShouldMoveIfMultipleMatches () { - var strings = new [] { "$$", "$100.00", "$101.00", "$101.10", "$200.00", "apricot", "c", "car", "cart" }; + var strings = new [] + { + "$$", + "$100.00", + "$101.00", + "$101.10", + "$200.00", + "apricot", + "c", + "car", + "cart" + }; int? current = 0; var n = new CollectionNavigator (strings); Assert.Equal (strings.IndexOf ("$$"), current = n.GetNextMatchingItem (current, "$$")); @@ -173,7 +231,18 @@ public void MinimizeMovement_False_ShouldMoveIfMultipleMatches () [Fact] public void MinimizeMovement_True_ShouldStayOnCurrentIfMultipleMatches () { - var strings = new [] { "$$", "$100.00", "$101.00", "$101.10", "$200.00", "apricot", "c", "car", "cart" }; + var strings = new [] + { + "$$", + "$100.00", + "$101.00", + "$101.10", + "$200.00", + "apricot", + "c", + "car", + "cart" + }; int? current = 0; var n = new CollectionNavigator (strings); Assert.Equal (strings.IndexOf ("$$"), current = n.GetNextMatchingItem (current, "$$", true)); @@ -326,39 +395,11 @@ public void Word () Assert.Equal (strings.IndexOf ("bat"), current = n.GetNextMatchingItem (current, 'a')); // match bat Assert.Equal (strings.IndexOf ("bat"), current = n.GetNextMatchingItem (current, 't')); // match bat - Assert.Equal ( - strings.IndexOf ("bates hotel"), - current = n.GetNextMatchingItem (current, 'e') - ); // match bates hotel + Assert.Equal (strings.IndexOf ("bates hotel"), current = n.GetNextMatchingItem (current, 'e')); // match bates hotel - Assert.Equal ( - strings.IndexOf ("bates hotel"), - current = n.GetNextMatchingItem (current, 's') - ); // match bates hotel + Assert.Equal (strings.IndexOf ("bates hotel"), current = n.GetNextMatchingItem (current, 's')); // match bates hotel - Assert.Equal ( - strings.IndexOf ("bates hotel"), - current = n.GetNextMatchingItem (current, ' ') - ); // match bates hotel - } - - [Fact] - public void CustomMatcher_NeverMatches () - { - var strings = new [] { "apricot", "arm", "bat", "batman", "bates hotel", "candle" }; - int? current = 0; - var n = new CollectionNavigator (strings); - - Mock matchNone = new (); - - matchNone.Setup (m => m.IsMatch (It.IsAny (), It.IsAny ())) - .Returns (false); - - n.Matcher = matchNone.Object; - - Assert.Equal (0, current = n.GetNextMatchingItem (current, 'b')); // no matches - Assert.Equal (0, current = n.GetNextMatchingItem (current, 'a')); // no matches - Assert.Equal (0, current = n.GetNextMatchingItem (current, 't')); // no matches + Assert.Equal (strings.IndexOf ("bates hotel"), current = n.GetNextMatchingItem (current, ' ')); // match bates hotel } #region Thread Safety Tests @@ -371,21 +412,20 @@ public void ThreadSafety_ConcurrentSearchStringAccess () var numTasks = 20; ConcurrentBag exceptions = new (); - Parallel.For ( - 0, + Parallel.For (0, numTasks, i => { try { // Read SearchString concurrently - string searchString = navigator.SearchString; + _ = navigator.SearchString; // Perform navigation operations concurrently - int? result = navigator.GetNextMatchingItem (0, 'a'); + _ = navigator.GetNextMatchingItem (0, 'a'); // Read SearchString again - searchString = navigator.SearchString; + _ = navigator.SearchString; } catch (Exception ex) { @@ -404,18 +444,17 @@ public void ThreadSafety_ConcurrentCollectionAccess () var numTasks = 20; ConcurrentBag exceptions = new (); - Parallel.For ( - 0, + Parallel.For (0, numTasks, i => { try { // Access Collection property concurrently - IList collection = navigator.Collection; + _ = navigator.Collection; // Perform navigation - int? result = navigator.GetNextMatchingItem (0, (char)('a' + i % 3)); + _ = navigator.GetNextMatchingItem (0, (char)('a' + i % 3)); } catch (Exception ex) { @@ -435,8 +474,7 @@ public void ThreadSafety_ConcurrentNavigationOperations () ConcurrentBag results = new (); ConcurrentBag exceptions = new (); - Parallel.For ( - 0, + Parallel.For (0, numTasks, i => { @@ -475,8 +513,8 @@ public void ThreadSafety_ConcurrentCollectionModification () { for (var j = 0; j < 100; j++) { - int? result = navigator.GetNextMatchingItem (0, 'a'); - string searchString = navigator.SearchString; + _ = navigator.GetNextMatchingItem (0, 'a'); + _ = navigator.SearchString; } } catch (Exception ex) @@ -523,16 +561,27 @@ public void ThreadSafety_ConcurrentCollectionModification () [Fact] public void ThreadSafety_ConcurrentSearchStringChanges () { - var strings = new [] { "apricot", "arm", "bat", "batman", "candle", "cat", "dog", "elephant", "fox", "goat" }; + var strings = new [] + { + "apricot", + "arm", + "bat", + "batman", + "candle", + "cat", + "dog", + "elephant", + "fox", + "goat" + }; var navigator = new CollectionNavigator (strings); var numTasks = 30; ConcurrentBag exceptions = new (); ConcurrentBag searchStrings = new (); - Parallel.For ( - 0, + Parallel.For (0, numTasks, - i => + _ => { try { @@ -570,8 +619,7 @@ public void ThreadSafety_StressTest_RapidOperations () var operationsPerTask = 1000; ConcurrentBag exceptions = new (); - Parallel.For ( - 0, + Parallel.For (0, numTasks, i => { @@ -588,7 +636,7 @@ public void ThreadSafety_StressTest_RapidOperations () if (j % 100 == 0) { - string searchString = navigator.SearchString; + _ = navigator.SearchString; } } }