diff --git a/src/modules/cmdpal/Tests/Microsoft.CmdPal.Ext.Shell.UnitTests/QueryTests.cs b/src/modules/cmdpal/Tests/Microsoft.CmdPal.Ext.Shell.UnitTests/QueryTests.cs index b6247143cb..8618c815ba 100644 --- a/src/modules/cmdpal/Tests/Microsoft.CmdPal.Ext.Shell.UnitTests/QueryTests.cs +++ b/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('"')); + } + } } diff --git a/src/modules/cmdpal/Tests/Microsoft.CmdPal.Ext.UnitTestsBase/CommandPaletteUnitTestBase.cs b/src/modules/cmdpal/Tests/Microsoft.CmdPal.Ext.UnitTestsBase/CommandPaletteUnitTestBase.cs index e509b90550..e00f198ab6 100644 --- a/src/modules/cmdpal/Tests/Microsoft.CmdPal.Ext.UnitTestsBase/CommandPaletteUnitTestBase.cs +++ b/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(); - page.ItemsChanged += (sender, args) => tcs.SetResult(null); + TypedEventHandler handleItemsChanged = (object s, IItemsChangedEventArgs e) => + { + tcs.TrySetResult(e); + }; + page.ItemsChanged += handleItemsChanged; modification(); + await tcs.Task; } } diff --git a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Shell/Helpers/CommandLineNormalizer.cs b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Shell/Helpers/CommandLineNormalizer.cs index 0ce15174b0..6a9fe10562 100644 --- a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Shell/Helpers/CommandLineNormalizer.cs +++ b/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. /// - 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. /// - 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 /// /// Attempts to resolve an executable path using SearchPathW. /// - 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; } diff --git a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Shell/Helpers/ShellListPageHelpers.cs b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Shell/Helpers/ShellListPageHelpers.cs index 3522e32346..15330a2751 100644 --- a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Shell/Helpers/ShellListPageHelpers.cs +++ b/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 /// 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; diff --git a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Shell/Pages/PathListItem.cs b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Shell/Pages/PathListItem.cs index b5463d3b73..b937ec7796 100644 --- a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Shell/Pages/PathListItem.cs +++ b/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)), diff --git a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Shell/Pages/ShellListPage.cs b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Shell/Pages/ShellListPage.cs index 2735e77000..82c6c2ddbc 100644 --- a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Shell/Pages/ShellListPage.cs +++ b/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(); + // 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(); - // 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 {