CmdPal: A couple more run commands bugs (#42174)

Caching bugs are hard.

This fixes like, three different run commands bugs:
* typing `c:\windows\p`, then backspacing to `c:\windows` would populate
the cache for `c:\` with the files in `c:\` that matched `windows*`.
* Now when the dir chenges, we correctly fill the cache with everything
in that dir, then filter it.
* that also caused a similar edge case for `c:\windows\` -> `c:\windows`
(the first should show results under c:\windows\` the second should only
show things in `c:\` matching `windows`
* As of my last PR, we support commandlines with spaces. We however
forgot to handle _paths_ with spaces. We'll now correctly show path
results for something like `c:\program files\`

src/modules/cmdpal/Tests/Microsoft.CmdPal.Ext.Shell.UnitTests/QueryTests.cs

@@ -3,11 +3,13 @@
 // See the LICENSE file in the project root for more information.
 
 using System.Collections.Generic;
+using System.IO;
 using System.Linq;
 using System.Threading.Tasks;
 using Microsoft.CmdPal.Core.Common.Services;
 using Microsoft.CmdPal.Ext.Shell.Pages;
 using Microsoft.CmdPal.Ext.UnitTestBase;
+using Microsoft.CommandPalette.Extensions;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 using Moq;
 
@@ -153,4 +155,131 @@ public class QueryTests : CommandPaletteUnitTestBase
         // Should find at least the ping command from history
         Assert.IsTrue(commandList.Length > 1);
     }
+
+    [TestMethod]
+    public async Task TestCacheBackToSameDirectory()
+    {
+        // Setup
+        var settings = Settings.CreateDefaultSettings();
+        var mockHistoryService = CreateMockHistoryService();
+
+        var page = new ShellListPage(settings, mockHistoryService.Object, telemetryService: null);
+
+        // Load up everything in c:\, for the sake of comparing:
+        var filesInC = Directory.EnumerateFileSystemEntries("C:\\");
+
+        await UpdatePageAndWaitForItems(page, () => { page.SearchText = "c:\\"; });
+
+        var commandList = page.GetItems();
+
+        // Should find only items for what's in c:\
+        Assert.IsTrue(commandList.Length == filesInC.Count());
+
+        await UpdatePageAndWaitForItems(page, () => { page.SearchText = "c:\\Win"; });
+        await UpdatePageAndWaitForItems(page, () => { page.SearchText = "c:\\Windows"; });
+        await UpdatePageAndWaitForItems(page, () => { page.SearchText = "c:\\"; });
+
+        commandList = page.GetItems();
+
+        // Should still find everything
+        Assert.IsTrue(commandList.Length == filesInC.Count());
+
+        await TypeStringIntoPage(page, "c:\\Windows\\Pro");
+        await BackspaceSearchText(page, "c:\\Windows\\Pro", 3); // 3 characters for c:\
+
+        commandList = page.GetItems();
+
+        // Should still find everything
+        Assert.IsTrue(commandList.Length == filesInC.Count());
+    }
+
+    private async Task TypeStringIntoPage(IDynamicListPage page, string searchText)
+    {
+        // type the string one character at a time
+        for (var i = 0; i < searchText.Length; i++)
+        {
+            var substr = searchText[..i];
+            await UpdatePageAndWaitForItems(page, () => { page.SearchText = substr; });
+        }
+    }
+
+    private async Task BackspaceSearchText(IDynamicListPage page, string originalSearchText, int finalStringLength)
+    {
+        var originalLength = originalSearchText.Length;
+        for (var i = originalLength; i >= finalStringLength; i--)
+        {
+            var substr = originalSearchText[..i];
+            await UpdatePageAndWaitForItems(page, () => { page.SearchText = substr; });
+        }
+    }
+
+    [TestMethod]
+    public async Task TestCacheSameDirectorySlashy()
+    {
+        // Setup
+        var settings = Settings.CreateDefaultSettings();
+        var mockHistoryService = CreateMockHistoryService();
+
+        var page = new ShellListPage(settings, mockHistoryService.Object, telemetryService: null);
+
+        // Load up everything in c:\, for the sake of comparing:
+        var filesInC = Directory.EnumerateFileSystemEntries("C:\\");
+        var filesInWindows = Directory.EnumerateFileSystemEntries("C:\\Windows");
+        await UpdatePageAndWaitForItems(page, () => { page.SearchText = "c:\\"; });
+
+        var commandList = page.GetItems();
+        Assert.IsTrue(commandList.Length == filesInC.Count());
+
+        // First navigate to c:\Windows. This should match everything that matches "windows" inside of C:\
+        await UpdatePageAndWaitForItems(page, () => { page.SearchText = "c:\\Windows"; });
+        var cWindowsCommandsPre = page.GetItems();
+
+        // Then go into c:\windows\. This will only have the results in c:\windows\
+        await UpdatePageAndWaitForItems(page, () => { page.SearchText = "c:\\Windows\\"; });
+        var windowsCommands = page.GetItems();
+        Assert.IsTrue(windowsCommands.Length != cWindowsCommandsPre.Length);
+
+        // now go back to c:\windows. This should match the results from the last time we entered this string
+        await UpdatePageAndWaitForItems(page, () => { page.SearchText = "c:\\Windows"; });
+        var cWindowsCommandsPost = page.GetItems();
+        Assert.IsTrue(cWindowsCommandsPre.Length == cWindowsCommandsPost.Length);
+    }
+
+    [TestMethod]
+    public async Task TestPathWithSpaces()
+    {
+        // Setup
+        var settings = Settings.CreateDefaultSettings();
+        var mockHistoryService = CreateMockHistoryService();
+
+        var page = new ShellListPage(settings, mockHistoryService.Object, telemetryService: null);
+
+        // Load up everything in c:\, for the sake of comparing:
+        var filesInC = Directory.EnumerateFileSystemEntries("C:\\");
+        var filesInProgramFiles = Directory.EnumerateFileSystemEntries("C:\\Program Files");
+        await UpdatePageAndWaitForItems(page, () => { page.SearchText = "c:\\Program Files\\"; });
+
+        var commandList = page.GetItems();
+        Assert.IsTrue(commandList.Length == filesInProgramFiles.Count());
+    }
+
+    [TestMethod]
+    public async Task TestNoWrapSuggestionsWithSpaces()
+    {
+        // Setup
+        var settings = Settings.CreateDefaultSettings();
+        var mockHistoryService = CreateMockHistoryService();
+
+        var page = new ShellListPage(settings, mockHistoryService.Object, telemetryService: null);
+
+        await UpdatePageAndWaitForItems(page, () => { page.SearchText = "c:\\Program Files\\"; });
+
+        var commandList = page.GetItems();
+
+        foreach (var item in commandList)
+        {
+            Assert.IsTrue(!string.IsNullOrEmpty(item.TextToSuggest));
+            Assert.IsFalse(item.TextToSuggest.StartsWith('"'));
+        }
+    }
 }

src/modules/cmdpal/Tests/Microsoft.CmdPal.Ext.UnitTestsBase/CommandPaletteUnitTestBase.cs

@@ -7,6 +7,7 @@ using System.Linq;
 using System.Threading.Tasks;
 using Microsoft.CommandPalette.Extensions;
 using Microsoft.CommandPalette.Extensions.Toolkit;
+using Windows.Foundation;
 
 namespace Microsoft.CmdPal.Ext.UnitTestBase;
 
@@ -32,9 +33,14 @@ public class CommandPaletteUnitTestBase
         // and wait for the event to be raised.
         var tcs = new TaskCompletionSource<object>();
 
-        page.ItemsChanged += (sender, args) => tcs.SetResult(null);
+        TypedEventHandler<object, IItemsChangedEventArgs> handleItemsChanged = (object s, IItemsChangedEventArgs e) =>
+        {
+            tcs.TrySetResult(e);
+        };
 
+        page.ItemsChanged += handleItemsChanged;
         modification();
+
         await tcs.Task;
     }
 }

src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Shell/Helpers/CommandLineNormalizer.cs

@@ -51,7 +51,7 @@ public static class CommandLineNormalizer
     ///
     /// The resulting strings are used for comparisons in profile matching.
     /// </remarks>
-    public static string NormalizeCommandLine(string commandLine)
+    public static string NormalizeCommandLine(string commandLine, bool allowDirectory)
     {
         if (string.IsNullOrEmpty(commandLine))
         {
@@ -79,7 +79,7 @@ public static class CommandLineNormalizer
         // The given commandLine should start with an executable name or path.
         // This loop tries to resolve relative paths, as well as executable names in %PATH%
         // into absolute paths and normalizes them.
-        var executablePath = ResolveExecutablePath(argv, ref startOfArguments);
+        var executablePath = ResolveExecutablePath(argv, allowDirectory, ref startOfArguments);
 
         // We've (hopefully) finished resolving the path to the executable.
         // We're now going to append all remaining arguments to the resulting string.
@@ -163,7 +163,7 @@ public static class CommandLineNormalizer
     /// Resolves the executable path from the command line arguments.
     /// Handles cases where the path contains spaces and was split during parsing.
     /// </summary>
-    private static string ResolveExecutablePath(string[] argv, ref int startOfArguments)
+    private static string ResolveExecutablePath(string[] argv, bool allowDirectory, ref int startOfArguments)
     {
         if (argv.Length == 0)
         {
@@ -183,7 +183,7 @@ public static class CommandLineNormalizer
             }
 
             var candidatePath = pathBuilder.ToString();
-            var resolvedPath = TryResolveExecutable(candidatePath);
+            var resolvedPath = TryResolveExecutable(candidatePath, allowDirectory);
 
             if (!string.IsNullOrEmpty(resolvedPath))
             {
@@ -200,7 +200,7 @@ public static class CommandLineNormalizer
     /// <summary>
     /// Attempts to resolve an executable path using SearchPathW.
     /// </summary>
-    private static string TryResolveExecutable(string executableName)
+    private static string TryResolveExecutable(string executableName, bool allowDirectory)
     {
         var buffer = new char[MAX_PATH];
 
@@ -234,11 +234,15 @@ public static class CommandLineNormalizer
 
             var resolvedPath = new string(buffer, 0, (int)result);
 
-            // Verify the resolved path exists and is not a directory
+            // Verify the resolved path exists...
             var attributes = PInvoke.GetFileAttributes(resolvedPath);
 
+            // ... and if we don't want to allow directories, reject paths that are dirs
+            var rejectDirectory = !allowDirectory &&
+                 (attributes & (uint)FILE_FLAGS_AND_ATTRIBUTES.FILE_ATTRIBUTE_DIRECTORY) != 0;
+
             return attributes == INVALID_FILE_ATTRIBUTES ||
-                   (attributes & (uint)FILE_FLAGS_AND_ATTRIBUTES.FILE_ATTRIBUTE_DIRECTORY) != 0 ?
+                   rejectDirectory ?
                     string.Empty :
                     resolvedPath;
         }

src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Shell/Helpers/ShellListPageHelpers.cs

@@ -18,7 +18,6 @@ public class ShellListPageHelpers
 
     internal static bool FileExistInPath(string filename, out string fullPath, CancellationToken? token = null)
     {
-        // TODO! remove this method and just use ShellHelpers.FileExistInPath directly
         return ShellHelpers.FileExistInPath(filename, out fullPath, token ?? CancellationToken.None);
     }
 
@@ -109,7 +108,7 @@ public class ShellListPageHelpers
     /// </summary>
     public static void NormalizeCommandLineAndArgs(string input, out string executable, out string arguments)
     {
-        var normalized = CommandLineNormalizer.NormalizeCommandLine(input);
+        var normalized = CommandLineNormalizer.NormalizeCommandLine(input, allowDirectory: true);
         var segments = normalized.Split('\0', StringSplitOptions.RemoveEmptyEntries);
         executable = string.Empty;
         arguments = string.Empty;

src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Shell/Pages/PathListItem.cs

@@ -50,26 +50,7 @@ internal sealed partial class PathListItem : ListItem
         Title = fileName; // Just the name of the file is the Title
         Subtitle = path; // What the user typed is the subtitle
 
-        // NOTE ME:
-        // If there are spaces on originalDir, trim them off, BEFORE combining originalDir and fileName.
-        // THEN add quotes at the end
-
-        // Trim off leading & trailing quote, if there is one
-        var trimmed = originalDir.Trim('"');
-        var originalPath = Path.Combine(trimmed, fileName);
-        var suggestion = originalPath;
-        var hasSpace = originalPath.Contains(' ');
-        if (hasSpace)
-        {
-            // wrap it in quotes
-            suggestion = string.Concat("\"", suggestion, "\"");
-        }
-        else
-        {
-            suggestion = path;
-        }
-
-        TextToSuggest = suggestion;
+        TextToSuggest = path;
 
         MoreCommands = [
             new CommandContextItem(new OpenWithCommand(path)),

src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Shell/Pages/ShellListPage.cs

@@ -434,66 +434,67 @@ internal sealed partial class ShellListPage : DynamicListPage, IDisposable
                 return;
             }
 
-            if (directoryPath == _currentSubdir)
+            // If the directory we're in changed, then first rebuild the cache
+            // of all the items in the directory, _then_ filter them below.
+            if (directoryPath != _currentSubdir)
             {
-                // Filter the items we already had
-                var fuzzyString = searchPattern.TrimEnd('*');
-                var newMatchedPathItems = new List<ListItem>();
+                // Get all the files in the directory.
+                // Run this on a background thread to avoid blocking
+                var files = await Task.Run(() => Directory.GetFileSystemEntries(directoryPath), cancellationToken);
 
-                foreach (var kv in _currentPathItems)
+                // Check for cancellation after file enumeration
+                if (cancellationToken.IsCancellationRequested)
                 {
-                    var score = string.IsNullOrEmpty(fuzzyString) ?
-                        1 :
-                        FuzzyStringMatcher.ScoreFuzzy(fuzzyString, kv.Key);
-                    if (score > 0)
-                    {
-                        newMatchedPathItems.Add(kv.Value);
-                    }
+                    return;
                 }
 
-                ListHelpers.InPlaceUpdateList(_pathItems, newMatchedPathItems);
-                return;
+                var searchPathTrailer = trimmed.Remove(0, Math.Min(directoryPath.Length, trimmed.Length));
+                var originalBeginning = originalPath.EndsWith(searchPathTrailer, StringComparison.CurrentCultureIgnoreCase) ?
+                                            originalPath.Remove(originalPath.Length - searchPathTrailer.Length) :
+                                            originalPath;
+
+                if (isDriveRoot)
+                {
+                    originalBeginning = string.Concat(originalBeginning, '\\');
+                }
+
+                // Create a list of commands for each file
+                var newPathItems = files
+                    .Select(f => PathToListItem(f, originalBeginning))
+                    .ToDictionary(item => item.Title, item => item);
+
+                // Final cancellation check before updating results
+                if (cancellationToken.IsCancellationRequested)
+                {
+                    return;
+                }
+
+                // Add the commands to the list
+                _pathItems.Clear();
+                _currentSubdir = directoryPath;
+                _currentPathItems.Clear();
+                foreach ((var k, IListItem v) in newPathItems)
+                {
+                    _currentPathItems[k] = (ListItem)v;
+                }
             }
 
-            // Get all the files in the directory that start with the search text
-            // Run this on a background thread to avoid blocking
-            var files = await Task.Run(() => Directory.GetFileSystemEntries(directoryPath, searchPattern), cancellationToken);
+            // Filter the items from this directory
+            var fuzzyString = searchPattern.TrimEnd('*');
+            var newMatchedPathItems = new List<ListItem>();
 
-            // Check for cancellation after file enumeration
-            if (cancellationToken.IsCancellationRequested)
+            foreach (var kv in _currentPathItems)
             {
-                return;
+                var score = string.IsNullOrEmpty(fuzzyString) ?
+                    1 :
+                    FuzzyStringMatcher.ScoreFuzzy(fuzzyString, kv.Key);
+                if (score > 0)
+                {
+                    newMatchedPathItems.Add(kv.Value);
+                }
             }
 
-            var searchPathTrailer = trimmed.Remove(0, Math.Min(directoryPath.Length, trimmed.Length));
-            var originalBeginning = originalPath.EndsWith(searchPathTrailer, StringComparison.CurrentCultureIgnoreCase) ?
-                                        originalPath.Remove(originalPath.Length - searchPathTrailer.Length) :
-                                        originalPath;
-
-            if (isDriveRoot)
-            {
-                originalBeginning = string.Concat(originalBeginning, '\\');
-            }
-
-            // Create a list of commands for each file
-            var newPathItems = files
-                .Select(f => PathToListItem(f, originalBeginning))
-                .ToDictionary(item => item.Title, item => item);
-
-            // Final cancellation check before updating results
-            if (cancellationToken.IsCancellationRequested)
-            {
-                return;
-            }
-
-            // Add the commands to the list
-            _pathItems = newPathItems.Values.ToList();
-            _currentSubdir = directoryPath;
-            _currentPathItems.Clear();
-            foreach ((var k, IListItem v) in newPathItems)
-            {
-                _currentPathItems[k] = (ListItem)v;
-            }
+            ListHelpers.InPlaceUpdateList(_pathItems, newMatchedPathItems);
         }
         else
         {