CmdPal: A different approach to bookmarking scripts, exes (try 2) (#40758)

_⚠️ targets #40427_ 

This is a different approach to #39059 that I was thinking about like a
month ago. It builds on the work from the rejuv'd run page (#39955) to
process the bookmark as an exe/path/url automatically.

I need to cross-check this with #39059 - I haven't cached that back in
since I got back from leave. I remember thinking that I wanted to try
this approach, but wasn't sure if it was right. More than anything, I
want to get it off my local PC and out for discussion

* We don't need to manually store the type anymore. 
* breaking change: paths with a space do need to be wrapped in spaces

closes #38700

----

I accidentally destroyed #40430 with a fat-finger merge from #40427 into
it. This resurrects that PR

src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Bookmark/AddBookmarkForm.cs

@@ -2,8 +2,6 @@
 // The Microsoft Corporation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-using System;
-using System.IO;
 using System.Text.Json;
 using System.Text.Json.Nodes;
 using Microsoft.CmdPal.Ext.Bookmarks.Properties;
@@ -75,31 +73,9 @@ internal sealed partial class AddBookmarkForm : FormContent
         var formBookmark = formInput["bookmark"] ?? string.Empty;
         var hasPlaceholder = formBookmark.ToString().Contains('{') && formBookmark.ToString().Contains('}');
 
-        // Determine the type of the bookmark
-        string bookmarkType;
-
-        if (formBookmark.ToString().StartsWith("http://", StringComparison.OrdinalIgnoreCase) || formBookmark.ToString().StartsWith("https://", StringComparison.OrdinalIgnoreCase))
-        {
-            bookmarkType = "web";
-        }
-        else if (File.Exists(formBookmark.ToString()))
-        {
-            bookmarkType = "file";
-        }
-        else if (Directory.Exists(formBookmark.ToString()))
-        {
-            bookmarkType = "folder";
-        }
-        else
-        {
-            // Default to web if we can't determine the type
-            bookmarkType = "web";
-        }
-
         var updated = _bookmark ?? new BookmarkData();
         updated.Name = formName.ToString();
         updated.Bookmark = formBookmark.ToString();
-        updated.Type = bookmarkType;
 
         AddedCommand?.Invoke(this, updated);
         return CommandResult.GoHome();

src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Bookmark/BookmarkData.cs

@@ -2,7 +2,9 @@
 // The Microsoft Corporation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+using System;
 using System.Text.Json.Serialization;
+using Microsoft.CommandPalette.Extensions.Toolkit;
 
 namespace Microsoft.CmdPal.Ext.Bookmarks;
 
@@ -12,8 +14,38 @@ public class BookmarkData
 
     public string Bookmark { get; set; } = string.Empty;
 
-    public string Type { get; set; } = string.Empty;
-
+    // public string Type { get; set; } = string.Empty;
     [JsonIgnore]
     public bool IsPlaceholder => Bookmark.Contains('{') && Bookmark.Contains('}');
+
+    internal void GetExeAndArgs(out string exe, out string args)
+    {
+        ShellHelpers.ParseExecutableAndArgs(Bookmark, out exe, out args);
+    }
+
+    internal bool IsWebUrl()
+    {
+        GetExeAndArgs(out var exe, out var args);
+        if (string.IsNullOrEmpty(exe))
+        {
+            return false;
+        }
+
+        if (Uri.TryCreate(exe, UriKind.Absolute, out var uri))
+        {
+            if (uri.Scheme == Uri.UriSchemeFile)
+            {
+                return false;
+            }
+
+            // return true if the scheme is http or https, or if there's no scheme (e.g., "www.example.com") but there is a dot in the host
+            return
+                uri.Scheme == Uri.UriSchemeHttp ||
+                uri.Scheme == Uri.UriSchemeHttps ||
+                (string.IsNullOrEmpty(uri.Scheme) && uri.Host.Contains('.'));
+        }
+
+        // If we can't parse it as a URI, we assume it's not a web URL
+        return false;
+    }
 }

src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Bookmark/BookmarkPlaceholderForm.cs

@@ -2,17 +2,14 @@
 // The Microsoft Corporation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-using System;
 using System.Collections.Generic;
 using System.Globalization;
 using System.Linq;
 using System.Text;
 using System.Text.Json.Nodes;
 using System.Text.RegularExpressions;
-using ManagedCommon;
 using Microsoft.CmdPal.Ext.Bookmarks.Properties;
 using Microsoft.CommandPalette.Extensions.Toolkit;
-using Windows.System;
 
 namespace Microsoft.CmdPal.Ext.Bookmarks;
 
@@ -25,7 +22,7 @@ internal sealed partial class BookmarkPlaceholderForm : FormContent
     private readonly string _bookmark = string.Empty;
 
     // TODO pass in an array of placeholders
-    public BookmarkPlaceholderForm(string name, string url, string type)
+    public BookmarkPlaceholderForm(string name, string url)
     {
         _bookmark = url;
         var r = new Regex(Regex.Escape("{") + "(.*?)" + Regex.Escape("}"));
@@ -88,23 +85,8 @@ internal sealed partial class BookmarkPlaceholderForm : FormContent
             target = target.Replace(placeholderString, placeholderData);
         }
 
-        try
-        {
-            var uri = UrlCommand.GetUri(target);
-            if (uri != null)
-            {
-                _ = Launcher.LaunchUriAsync(uri);
-            }
-            else
-            {
-                // throw new UriFormatException("The provided URL is not valid.");
-            }
-        }
-        catch (Exception ex)
-        {
-            Logger.LogError(ex.Message);
-        }
+        var success = UrlCommand.LaunchCommand(target);
 
-        return CommandResult.GoHome();
+        return success ? CommandResult.Dismiss() : CommandResult.KeepOpen();
     }
 }

src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Bookmark/BookmarkPlaceholderPage.cs

@@ -2,6 +2,7 @@
 // The Microsoft Corporation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+using System;
 using Microsoft.CommandPalette.Extensions;
 using Microsoft.CommandPalette.Extensions.Toolkit;
 
@@ -9,19 +10,30 @@ namespace Microsoft.CmdPal.Ext.Bookmarks;
 
 internal sealed partial class BookmarkPlaceholderPage : ContentPage
 {
+    private readonly Lazy<IconInfo> _icon;
     private readonly FormContent _bookmarkPlaceholder;
 
     public override IContent[] GetContent() => [_bookmarkPlaceholder];
 
+    public override IconInfo Icon { get => _icon.Value; set => base.Icon = value; }
+
     public BookmarkPlaceholderPage(BookmarkData data)
-        : this(data.Name, data.Bookmark, data.Type)
+        : this(data.Name, data.Bookmark)
     {
     }
 
-    public BookmarkPlaceholderPage(string name, string url, string type)
+    public BookmarkPlaceholderPage(string name, string url)
     {
-        Name = name;
-        Icon = new IconInfo(UrlCommand.IconFromUrl(url, type));
-        _bookmarkPlaceholder = new BookmarkPlaceholderForm(name, url, type);
+        Name = Properties.Resources.bookmarks_command_name_open;
+
+        _bookmarkPlaceholder = new BookmarkPlaceholderForm(name, url);
+
+        _icon = new Lazy<IconInfo>(() =>
+        {
+            ShellHelpers.ParseExecutableAndArgs(url, out var exe, out var args);
+            var t = UrlCommand.GetIconForPath(exe);
+            t.Wait();
+            return t.Result;
+        });
     }
 }

src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Bookmark/BookmarksCommandProvider.cs

@@ -4,7 +4,6 @@
 
 using System;
 using System.Collections.Generic;
-using System.Diagnostics;
 using System.IO;
 using System.Linq;
 using ManagedCommon;
@@ -35,10 +34,7 @@ public partial class BookmarksCommandProvider : CommandProvider
     private void AddNewCommand_AddedCommand(object sender, BookmarkData args)
     {
         ExtensionHost.LogMessage($"Adding bookmark ({args.Name},{args.Bookmark})");
-        if (_bookmarks != null)
-        {
-            _bookmarks.Data.Add(args);
-        }
+        _bookmarks?.Data.Add(args);
 
         SaveAndUpdateCommands();
     }
@@ -116,7 +112,7 @@ public partial class BookmarksCommandProvider : CommandProvider
         // Add commands for folder types
         if (command is UrlCommand urlCommand)
         {
-            if (urlCommand.Type == "folder")
+            if (!bookmark.IsWebUrl())
             {
                 contextMenu.Add(
                     new CommandContextItem(new DirectoryPage(urlCommand.Url)));
@@ -124,10 +120,11 @@ public partial class BookmarksCommandProvider : CommandProvider
                 contextMenu.Add(
                     new CommandContextItem(new OpenInTerminalCommand(urlCommand.Url)));
             }
-
-            listItem.Subtitle = urlCommand.Url;
         }
 
+        listItem.Title = bookmark.Name;
+        listItem.Subtitle = bookmark.Bookmark;
+
         var edit = new AddBookmarkPage(bookmark) { Icon = Icons.EditIcon };
         edit.AddedCommand += Edit_AddedCommand;
         contextMenu.Add(new CommandContextItem(edit));

src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Bookmark/Properties/Resources.Designer.cs

@@ -78,6 +78,15 @@ namespace Microsoft.CmdPal.Ext.Bookmarks.Properties {
             }
         }
         
+        /// <summary>
+        ///   Looks up a localized string similar to Open.
+        /// </summary>
+        public static string bookmarks_command_name_open {
+            get {
+                return ResourceManager.GetString("bookmarks_command_name_open", resourceCulture);
+            }
+        }
+        
         /// <summary>
         ///   Looks up a localized string similar to Delete.
         /// </summary>

src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Bookmark/Properties/Resources.resx

@@ -148,6 +148,9 @@
   <data name="bookmarks_form_open" xml:space="preserve">
     <value>Open</value>
   </data>
+  <data name="bookmarks_command_name_open" xml:space="preserve">
+    <value>Open</value>
+  </data>
   <data name="bookmarks_form_name_required" xml:space="preserve">
     <value>Name is required</value>
   </data>

src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Bookmark/UrlCommand.cs

@@ -3,52 +3,89 @@
 // See the LICENSE file in the project root for more information.
 
 using System;
+using System.Threading;
+using System.Threading.Tasks;
 using ManagedCommon;
 using Microsoft.CommandPalette.Extensions.Toolkit;
+using Windows.Storage.Streams;
 using Windows.System;
 
 namespace Microsoft.CmdPal.Ext.Bookmarks;
 
 public partial class UrlCommand : InvokableCommand
 {
-    public string Type { get; }
+    private readonly Lazy<IconInfo> _icon;
 
     public string Url { get; }
 
+    public override IconInfo Icon { get => _icon.Value; set => base.Icon = value; }
+
     public UrlCommand(BookmarkData data)
-        : this(data.Name, data.Bookmark, data.Type)
+        : this(data.Name, data.Bookmark)
     {
     }
 
-    public UrlCommand(string name, string url, string type)
+    public UrlCommand(string name, string url)
     {
-        Name = name;
-        Type = type;
+        Name = Properties.Resources.bookmarks_command_name_open;
+
         Url = url;
-        Icon = new IconInfo(IconFromUrl(Url, type));
+
+        _icon = new Lazy<IconInfo>(() =>
+        {
+            ShellHelpers.ParseExecutableAndArgs(Url, out var exe, out var args);
+            var t = GetIconForPath(exe);
+            t.Wait();
+            return t.Result;
+        });
     }
 
     public override CommandResult Invoke()
     {
-        var target = Url;
-        try
+        var success = LaunchCommand(Url);
+
+        return success ? CommandResult.Dismiss() : CommandResult.KeepOpen();
+    }
+
+    internal static bool LaunchCommand(string target)
+    {
+        ShellHelpers.ParseExecutableAndArgs(target, out var exe, out var args);
+        return LaunchCommand(exe, args);
+    }
+
+    internal static bool LaunchCommand(string exe, string args)
+    {
+        if (string.IsNullOrEmpty(exe))
         {
-            var uri = GetUri(target);
+            var message = "No executable found in the command.";
+            Logger.LogError(message);
+
+            return false;
+        }
+
+        if (ShellHelpers.OpenInShell(exe, args))
+        {
+            return true;
+        }
+
+        // If we reach here, it means the command could not be executed
+        // If there aren't args, then try again as a https: uri
+        if (string.IsNullOrEmpty(args))
+        {
+            var uri = GetUri(exe);
             if (uri != null)
             {
                 _ = Launcher.LaunchUriAsync(uri);
             }
             else
             {
-                // throw new UriFormatException("The provided URL is not valid.");
+                Logger.LogError("The provided URL is not valid.");
             }
-        }
-        catch (Exception ex)
-        {
-            Logger.LogError(ex.Message);
+
+            return true;
         }
 
-        return CommandResult.Dismiss();
+        return false;
     }
 
     internal static Uri? GetUri(string url)
@@ -65,35 +102,90 @@ public partial class UrlCommand : InvokableCommand
         return uri;
     }
 
-    internal static string IconFromUrl(string url, string type)
+    public static async Task<IconInfo> GetIconForPath(string target)
     {
-        switch (type)
-        {
-            case "file":
-                return "📄";
-            case "folder":
-                return "📁";
-            case "web":
-            default:
-                // Get the base url up to the first placeholder
-                var placeholderIndex = url.IndexOf('{');
-                var baseString = placeholderIndex > 0 ? url.Substring(0, placeholderIndex) : url;
-                try
-                {
-                    var uri = GetUri(baseString);
-                    if (uri != null)
-                    {
-                        var hostname = uri.Host;
-                        var faviconUrl = $"{uri.Scheme}://{hostname}/favicon.ico";
-                        return faviconUrl;
-                    }
-                }
-                catch (UriFormatException ex)
-                {
-                    Logger.LogError(ex.Message);
-                }
+        IconInfo? icon = null;
 
-                return "🔗";
+        // First, try to get the icon from the thumbnail helper
+        // This works for local files and folders
+        icon = await MaybeGetIconForPath(target);
+        if (icon != null)
+        {
+            return icon;
         }
+
+        // Okay, that failed. Try to resolve the full path of the executable
+        var exeExists = false;
+        var fullExePath = string.Empty;
+        try
+        {
+            using var cts = new CancellationTokenSource(TimeSpan.FromMilliseconds(200));
+
+            // Use Task.Run with timeout - this will actually timeout even if the sync operations don't respond to cancellation
+            var pathResolutionTask = Task.Run(
+                () =>
+            {
+                // Don't check cancellation token here - let the Task timeout handle it
+                exeExists = ShellHelpers.FileExistInPath(target, out fullExePath);
+            },
+                CancellationToken.None);
+
+            // Wait for either completion or timeout
+            pathResolutionTask.Wait(cts.Token);
+        }
+        catch (OperationCanceledException)
+        {
+            // Debug.WriteLine("Operation was canceled.");
+        }
+
+        if (exeExists)
+        {
+            // If the executable exists, try to get the icon from the file
+            icon = await MaybeGetIconForPath(fullExePath);
+            if (icon != null)
+            {
+                return icon;
+            }
+        }
+
+        // Get the base url up to the first placeholder
+        var placeholderIndex = target.IndexOf('{');
+        var baseString = placeholderIndex > 0 ? target.Substring(0, placeholderIndex) : target;
+        try
+        {
+            var uri = GetUri(baseString);
+            if (uri != null)
+            {
+                var hostname = uri.Host;
+                var faviconUrl = $"{uri.Scheme}://{hostname}/favicon.ico";
+                icon = new IconInfo(faviconUrl);
+            }
+        }
+        catch (UriFormatException)
+        {
+        }
+
+        // If we still don't have an icon, use the target as the icon
+        icon = icon ?? new IconInfo(target);
+
+        return icon;
+    }
+
+    private static async Task<IconInfo?> MaybeGetIconForPath(string target)
+    {
+        try
+        {
+            var stream = await ThumbnailHelper.GetThumbnail(target);
+            if (stream != null)
+            {
+                var data = new IconData(RandomAccessStreamReference.CreateFromStream(stream));
+                return new IconInfo(data, data);
+            }
+        }
+        catch
+        {
+        }
+
+        return null;
     }
 }

src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Shell/FallbackExecuteItem.cs

@@ -89,7 +89,7 @@ internal sealed partial class FallbackExecuteItem : FallbackCommandItem, IDispos
             return;
         }
 
-        ShellListPage.ParseExecutableAndArgs(searchText, out var exe, out var args);
+        ShellHelpers.ParseExecutableAndArgs(searchText, out var exe, out var args);
 
         // Check for cancellation before file system operations
         cancellationToken.ThrowIfCancellationRequested();
@@ -191,7 +191,7 @@ internal sealed partial class FallbackExecuteItem : FallbackCommandItem, IDispos
             return false;
         }
 
-        ShellListPage.ParseExecutableAndArgs(searchText, out var exe, out var args);
+        ShellHelpers.ParseExecutableAndArgs(searchText, out var exe, out var args);
         var exeExists = ShellListPageHelpers.FileExistInPath(exe, out var fullExePath);
         var pathIsDir = Directory.Exists(exe);
 

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

@@ -54,47 +54,8 @@ public class ShellListPageHelpers
 
     internal static bool FileExistInPath(string filename, out string fullPath, CancellationToken? token = null)
     {
-        fullPath = string.Empty;
-
-        if (File.Exists(filename))
-        {
-            token?.ThrowIfCancellationRequested();
-            fullPath = Path.GetFullPath(filename);
-            return true;
-        }
-        else
-        {
-            var values = Environment.GetEnvironmentVariable("PATH");
-            if (values != null)
-            {
-                foreach (var path in values.Split(';'))
-                {
-                    var path1 = Path.Combine(path, filename);
-                    if (File.Exists(path1))
-                    {
-                        fullPath = Path.GetFullPath(path1);
-                        return true;
-                    }
-
-                    token?.ThrowIfCancellationRequested();
-
-                    var path2 = Path.Combine(path, filename + ".exe");
-                    if (File.Exists(path2))
-                    {
-                        fullPath = Path.GetFullPath(path2);
-                        return true;
-                    }
-
-                    token?.ThrowIfCancellationRequested();
-                }
-
-                return false;
-            }
-            else
-            {
-                return false;
-            }
-        }
+        // TODO! remove this method and just use ShellHelpers.FileExistInPath directly
+        return ShellHelpers.FileExistInPath(filename, out fullPath, token ?? CancellationToken.None);
     }
 
     internal static ListItem? ListItemForCommandString(string query, Action<string>? addToHistory)
@@ -109,7 +70,7 @@ public class ShellListPageHelpers
             return null;
         }
 
-        ShellListPage.ParseExecutableAndArgs(searchText, out var exe, out var args);
+        ShellHelpers.ParseExecutableAndArgs(searchText, out var exe, out var args);
 
         var exeExists = false;
         var pathIsDir = false;

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

@@ -152,7 +152,7 @@ internal sealed partial class ShellListPage : DynamicListPage, IDisposable
             return;
         }
 
-        ParseExecutableAndArgs(expanded, out var exe, out var args);
+        ShellHelpers.ParseExecutableAndArgs(expanded, out var exe, out var args);
 
         // Check for cancellation before file system operations
         cancellationToken.ThrowIfCancellationRequested();
@@ -439,46 +439,6 @@ internal sealed partial class ShellListPage : DynamicListPage, IDisposable
         }
     }
 
-    internal static void ParseExecutableAndArgs(string input, out string executable, out string arguments)
-    {
-        input = input.Trim();
-        executable = string.Empty;
-        arguments = string.Empty;
-
-        if (string.IsNullOrEmpty(input))
-        {
-            return;
-        }
-
-        if (input.StartsWith("\"", System.StringComparison.InvariantCultureIgnoreCase))
-        {
-            // Find the closing quote
-            var closingQuoteIndex = input.IndexOf('\"', 1);
-            if (closingQuoteIndex > 0)
-            {
-                executable = input.Substring(1, closingQuoteIndex - 1);
-                if (closingQuoteIndex + 1 < input.Length)
-                {
-                    arguments = input.Substring(closingQuoteIndex + 1).TrimStart();
-                }
-            }
-        }
-        else
-        {
-            // Executable ends at first space
-            var firstSpaceIndex = input.IndexOf(' ');
-            if (firstSpaceIndex > 0)
-            {
-                executable = input.Substring(0, firstSpaceIndex);
-                arguments = input[(firstSpaceIndex + 1)..].TrimStart();
-            }
-            else
-            {
-                executable = input;
-            }
-        }
-    }
-
     internal void CreateUriItems(string searchText)
     {
         if (!System.Uri.TryCreate(searchText, UriKind.Absolute, out var uri))

src/modules/cmdpal/extensionsdk/Microsoft.CommandPalette.Extensions.Toolkit/ShellHelpers.cs

@@ -59,4 +59,101 @@ public static class ShellHelpers
         Administrator,
         OtherUser,
     }
+
+    /// <summary>
+    /// Parses the input string to extract the executable and its arguments.
+    /// </summary>
+    public static void ParseExecutableAndArgs(string input, out string executable, out string arguments)
+    {
+        input = input.Trim();
+        executable = string.Empty;
+        arguments = string.Empty;
+
+        if (string.IsNullOrEmpty(input))
+        {
+            return;
+        }
+
+        if (input.StartsWith("\"", System.StringComparison.InvariantCultureIgnoreCase))
+        {
+            // Find the closing quote
+            var closingQuoteIndex = input.IndexOf('\"', 1);
+            if (closingQuoteIndex > 0)
+            {
+                executable = input.Substring(1, closingQuoteIndex - 1);
+                if (closingQuoteIndex + 1 < input.Length)
+                {
+                    arguments = input.Substring(closingQuoteIndex + 1).TrimStart();
+                }
+            }
+        }
+        else
+        {
+            // Executable ends at first space
+            var firstSpaceIndex = input.IndexOf(' ');
+            if (firstSpaceIndex > 0)
+            {
+                executable = input.Substring(0, firstSpaceIndex);
+                arguments = input[(firstSpaceIndex + 1)..].TrimStart();
+            }
+            else
+            {
+                executable = input;
+            }
+        }
+    }
+
+    /// <summary>
+    /// Checks if a file exists somewhere in the PATH.
+    /// If it exists, returns the full path to the file in the out parameter.
+    /// If it does not exist, returns false and the out parameter is set to an empty string.
+    /// <param name="filename">The name of the file to check.</param>
+    /// <param name="fullPath">The full path to the file if it exists; otherwise an empty string.</param>
+    /// <param name="token">An optional cancellation token to cancel the operation.</param>
+    /// <returns>True if the file exists in the PATH; otherwise false.</returns>
+    /// </summary>
+    public static bool FileExistInPath(string filename, out string fullPath, CancellationToken? token = null)
+    {
+        fullPath = string.Empty;
+
+        if (File.Exists(filename))
+        {
+            token?.ThrowIfCancellationRequested();
+            fullPath = Path.GetFullPath(filename);
+            return true;
+        }
+        else
+        {
+            var values = Environment.GetEnvironmentVariable("PATH");
+            if (values != null)
+            {
+                foreach (var path in values.Split(';'))
+                {
+                    var path1 = Path.Combine(path, filename);
+                    if (File.Exists(path1))
+                    {
+                        fullPath = Path.GetFullPath(path1);
+                        return true;
+                    }
+
+                    token?.ThrowIfCancellationRequested();
+
+                    var path2 = Path.Combine(path, filename + ".exe");
+                    if (File.Exists(path2))
+                    {
+                        fullPath = Path.GetFullPath(path2);
+                        return true;
+                    }
+
+                    token?.ThrowIfCancellationRequested();
+                }
+
+                return false;
+            }
+            else
+            {
+                return false;
+            }
+        }
+    }
 }