Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expand the list of file types for external tools #3406

Merged
merged 3 commits into from
Jul 12, 2024

Conversation

andreww-msft
Copy link
Contributor

Summary of the pull request

Added explicit support for .bat, .cmd, .ps1, .msc files, and implicit support for any file type (only known file types were tested).

Updated the ExternalTool.Invoke method to return a Process instead of a bool. This will be useful later when we provide features to work with the launched process. As part of this, switched from explorer.exe to ApplicationActivationManager for launching packaged apps by AUMID - now invoking all tool types can return a Process (except for protocol activations).

Made the tool path textbox editable - so the user can simply type in a path instead of opening the file dialog, if they wish.

Additional comments

  1. Most .msc files require elevation. We explicitly don't attempt to solve this within PI - the user already has the option to launch PI elevated, and there's a separate workstream on clarifying the elevation story.
  2. Running unsigned .ps1 files may well require changes to execution policty, and again we don't attempt to solve this within PI. The user chooses which .ps1 files to register, and can therefore set policy accordingly.
  3. For now, for .bat/.cmd files, we don't redirect stdout or stderr. This is deferred until we have formulated the plan for handling this data within PI.

Validation steps performed

Tested registering/invoking:

  • valid .bat, .cmd, .ps1 files
  • .msc files that require elevation (services, procmon)
  • .msc file that doesn't require elevation (resmon)
  • invalid .bat, .cmd, .ps1, .msc files
  • invalid executable paths
  • unregistered protocols

PR checklist

Copy link
Contributor

@jaholme jaholme left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@andreww-msft andreww-msft merged commit 8e7fd89 into main Jul 12, 2024
4 checks passed
@andreww-msft andreww-msft deleted the user/andreww/moreExternalTools branch July 12, 2024 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[External Tools] When PI browses for external tools, include other extensions beyond exe and bat
4 participants