[cmdpal] Support invoke command in Bookmarks extension

[moooyo]

Summary of the Pull Request

• Support invoke command
• Add BookmarkType enum to categorize bookmarks
• Use JsonSerializer to serialize all i18n string to avoid invalid character.
• Add IconHelper to create icon for bookmark by type.
• Forward compatibility. No issue if we upgrade from old bookmark ext.

PR Checklist

• Closes: CmdPal: Bookmarking a bat (or other executable) should still launch it #38700
• Communication: I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected
• Tests: Added/updated and all pass
• Localization: All end user facing strings can be localized
• Dev docs: Added/updated
• New binaries: Added on the required places
  • JSON for signing for new binaries
  • WXS for installer for new binaries and localization folder
  • YML for CI pipeline for new test projects
  • YML for signed pipeline
• Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

[moooyo]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[htcfreek]

I took a look at the code. See my comments.

I do not have the time to run and try it. And I do not have the permission to unblock your PR later.

[zadjii-msft]

I don't think I love the type dropdown here. I don't really want people to have to think about what the type of thing is. Plus, if we add built-in support for python 2 and 3, then why not other things? why not bash or node or ...? What if they don't have python installed? Or pwsh?

I think it's cleaner for us to require the actual executable in the bookmark. And then we can heuristically figure out:

• Is this a URL -> launch it with the URL opener (we probably could move the Open in browser stuff from the web search plugin to the toolkit)
• Is it a path to a folder? -> add the open folder commands (we probably could even lift some stuff out of the indexer into the toolkit for that)
• else, it's something to ShellExecute. (test.bat, or cmd.exe /k ipconfig, or pwsh foo.ps1, or py foo.py or path\to\some_exe.exe with args, or heck even "path\to\My Word Doc.docx" etc) and we should just start that.

I know that's not necessarily as clean as just explicitly bookmarking "foo.py as a Python bookmark", but I think it's almost better to just have to bookmark "python foo.py".

(plus, I bet we could try to use the ThumbnailHelper to get the thumbnail of the exe for the bookmark)

[htcfreek]

@zadjii-msft
Fair point. And this solves my short thought about not being able to select "Windows Terminal".

[zadjii-msft]

off-topic: it might be a hot take, but I don't think we should have the "open in Windows Terminal"-like options anymore. The Default Terminal setting is available back even on Windows 10 these days. We should just always respect that

(I believe I have similar notes in #38277)

[moooyo]

Okay I have feedback that I don't think fits in one place in particular

• I think we can probably make this a lot simpler, by getting rid of the File and Folder command types too. We can probably just OpenInShellHelper.OpenInShell them too.

• Trying to determine if it's a "command" or a "path" based on the presence of a literal space is probably not the right way to do that.

  • Consider:

    • cmd.exe (this is a "command" that doesn't have a space)
    • c:\users\Mike\OneDrive - Microsoft\foo.png (this is a file that has a space in the path)

  • I think we need to better be able to parse the string into (path, args...). I would think that System.CommandLine should be able to do that? But this is notoriously challenging

• We may want to do the lazy ThumbnailHelper icon loading for files/folders/{the exe of a command}, rather than just the static File/Folder/Command icons

  • But that could be just a follow-up

• I only don't immediately recommend merging the web commands with the rest, because that one has Special Favicon Handling

  • by all accounts, OpenInShellHelper.OpenInShell should Just Work for URLs too
  • heck, I bet we could still probably figure out the favicon handling without too much trouble

Emm, you are right.

Made the following changes:

1. Merged the file, folder and Web into OpenInShell. But keep the bookmark type in to avoid dup jug. Yeah, you are right. We can merge all of them into openInShell. Actually, favicon is not a blocker. I've moved this logic into IconHelper.
2. Fetch file and folder icon (ha actually I want to create a new PR for it before but we need a big change for this PR. So I decide to do it in this PR)
3. Remove unused string.
4. Updated the PR's descrption.

[moooyo]

1. Merged the file, folder

@zadjii-msft

The only bug is we can not open a "oneDrive" folder as I did in the PR's descrption. But the normal folder is ok.

[moooyo]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[yeelam-gordon]

I think it is difficult to "assume" what's URL, File/Folder, or Command.
If we add more thing later on, it just not scalability here on what does a Command parsing means.

Two thoughts:

1. Can we have "interaction" between extension? i.e. At the "Run Command", I can add say "Bookmark" it?
   Since people can try out the command at the "Run command", and then bookmark it.
2. It will be more streamline if we guess the type first, and then let people to "change" the bookmark type later on.

[yeelam-gordon]

Not suggest to get in for 0.91 except there is special reason

[yeelam-gordon]

I think it is difficult to "assume" what's URL, File/Folder, or Command. If we add more thing later on, it just not scalability here on what does a Command parsing means.

Two thoughts:

1. Can we have "interaction" between extension? i.e. At the "Run Command", I can add say "Bookmark" it?
   Since people can try out the command at the "Run command", and then bookmark it.
2. It will be more streamline if we guess the type first, and then let people to "change" the bookmark type later on.

Looks like from what Mike just added the issue thread #39261 , bookmark from other common extension like "All Apps", "Search Files (and Folder)", "Run Command", does make sense.
There is already alias and shortcut management for bookmark, just not easy to be discovered at the bookmark UX itself (i.e. need go to setting page now)

[moooyo]

I think it is difficult to "assume" what's URL, File/Folder, or Command. If we add more thing later on, it just not scalability here on what does a Command parsing means.
Two thoughts:

1. Can we have "interaction" between extension? i.e. At the "Run Command", I can add say "Bookmark" it?
   Since people can try out the command at the "Run command", and then bookmark it.
2. It will be more streamline if we guess the type first, and then let people to "change" the bookmark type later on.

Looks like from what Mike just added the issue thread #39261 , bookmark from other common extension like "All Apps", "Search Files (and Folder)", "Run Command", does make sense. There is already alias and shortcut management for bookmark, just not easy to be discovered at the bookmark UX itself (i.e. need go to setting page now)

Yeah, agree.

[moooyo]

Not suggest to get in for 0.91 except there is special reason

If possible, I want to get it in. But if not, it's ok to hold.

In fact, the original version also have many assumption. This PR at least I think it just add a new ability to invoke command.

So, If we want to consider how to get the correct bookmark type (through a new choices input or other way), we can do in the future.

[Copilot]

Pull Request Overview

This PR introduces support for invoking commands in the Bookmarks extension by refactoring URL invocation logic and adding a new BookmarkType enum to more clearly categorize bookmarks. Key changes include removing the old UrlCommand in favor of ShellCommand, integrated icon handling via IconHelper, and updating bookmark forms to use JsonSerializer and the new enum.

Reviewed Changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
UrlCommand.cs	Removed legacy URL command implementation.
Resources.resx	Added a new error message for command invocation failures.
BookmarkPlaceholderPage.cs	Updated constructor to use BookmarkType and the new IconHelper.
BookmarkPlaceholderForm.cs	Refactored invocation logic to call ShellCommand instead of direct URL launching.
AddBookmarkForm.cs	Updated bookmark type derivation and JSON serialization for adaptive cards.
BookmarkType.cs, BookmarkData.cs	Introduced and integrated a new BookmarkType enum.
OpenInShellHelper.cs, IconHelper.cs	Added helper methods to encapsulate URI parsing and icon creation.
ShellCommand.cs, OpenInTerminalCommand.cs	Implemented robust command invocation for web, file, folder, and command bookmark types.
BookmarksCommandProvider.cs	Refactored command item creation to use new helper methods.

Files not reviewed (1)

• src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Bookmark/Properties/Resources.Designer.cs: Language not supported

[moooyo]

@zadjii-msft

any suggestion? I think we can move on.