Skip to content

Add new additionalPowerShellLocations option#5484

Open
chawyehsu wants to merge 3 commits into
PowerShell:mainfrom
chawyehsu:push-sntxrqrrpznt
Open

Add new additionalPowerShellLocations option#5484
chawyehsu wants to merge 3 commits into
PowerShell:mainfrom
chawyehsu:push-sntxrqrrpznt

Conversation

@chawyehsu
Copy link
Copy Markdown

@chawyehsu chawyehsu commented Apr 30, 2026

PR Summary

Implement and close #5233

This PR attempts to add a better (cross-platform friendly) way to configure additional PowerShell paths.

Details

This implementaion is a little different from what I've proposed last year in #5233, the primary difference is instead of a default?: boolean field, a weight?: number is used to rank entries and select the best matching entry. Since we cannot restrict the default: true per platform to only one in the VS Code extension configuration, using weighted sorting allows us to achieve a similar purpose. Furthermore, the more specific platform field involving arch is used to specify the target host instead of os.

Backward Compatibility? Yes

While the old powerShellAdditionalExePaths and powerShellDefaultVersion stay unchanged and can still be used to define additional PowerShell paths, the new additionalPowerShellLocations is added to be mutually exclusive, which means when additionalPowerShellLocations is defined(non-empty), old options are ignored. This keeps settings non-conflicting: users pick one path (old or new), not both. Intuitive, simple and easy to understand.

Old options could be marked deprecated after a transition period.

PR Checklist

Note: Tick the boxes below that apply to this pull request by putting an x between the square brackets.
Please mark anything not applicable to this PR NA.

  • PR has a meaningful title
  • Summarized changes
  • PR has tests
  • This PR is ready to merge and is not work in progress
    • If the PR is work in progress, please add the prefix WIP: to the beginning of the title and remove the prefix when the PR is ready

Signed-off-by: Chawye Hsu <su+git@chawyehsu.com>
@chawyehsu chawyehsu marked this pull request as ready for review April 30, 2026 12:03
Copilot AI review requested due to automatic review settings April 30, 2026 12:03
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a new cross-platform-friendly configuration option (powershell.additionalPowerShellLocations) to let users define additional PowerShell executables with per-platform (OS+arch) filtering and weighted ranking, integrating it into PowerShell discovery and the Session Menu flow.

Changes:

  • Added SupportedPlatform, IAdditionalPowerShellLocation, and getSupportedPlatform() plus a new enumeration path for additional locations in PowerShellExeFinder.
  • Updated SessionManager to prefer additionalPowerShellLocations over legacy settings and to adjust entry weight when switching via Session Menu.
  • Added unit tests for supported-platform mapping and additional-location filtering/sorting; added the new setting schema to package.json.

Reviewed changes

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

File Description
src/platform.ts Introduces the new platform/arch mapping + additional-location enumeration and integrates it into PowerShell discovery.
src/session.ts Reads the new setting, uses it to derive the requested PowerShell, and updates weight on Session Menu selection.
test/core/platform.test.ts Adds coverage for platform mapping and additional-location filtering/priority behavior.
package.json Adds powershell.additionalPowerShellLocations configuration schema and documentation.

Comment thread src/platform.ts
Comment on lines +218 to +222
const configuredPowerShells =
this.additionalPowerShellLocations.length > 0
? this.enumerateAdditionalPowerShellLocations()
: this.enumerateAdditionalPowerShellInstallations();
for await (const additionalPwsh of configuredPowerShells) {
Comment thread src/session.ts Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@chawyehsu chawyehsu changed the title Add new additionalPowerShellLocations option WIP: Add new additionalPowerShellLocations option Apr 30, 2026
Signed-off-by: Chawye Hsu <su+git@chawyehsu.com>
@chawyehsu chawyehsu changed the title WIP: Add new additionalPowerShellLocations option Add new additionalPowerShellLocations option Apr 30, 2026
@chawyehsu
Copy link
Copy Markdown
Author

By any chance can I get some reviews? @JustinGrote @andyleejordan

Copy link
Copy Markdown
Member

@andyleejordan andyleejordan left a comment

Choose a reason for hiding this comment

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

Just stopping by with a decision question: would there be a good way to handle the "default" cases of just windows/macos/linux (without architecture)? My thought is, and this is really an open question: are most people going to want or need to specify the architecture, especially as x64 is kinda on its way out (I mean look at macOS, you'd be hardpressed to find anything but Apple Silicon now). I'm thinking perhaps handle those as defaults, with architecture specific versions as options too.

On the note of these settings getting complicated, do we have a simpler way to handle "weight" that doesn't require the user to specify? I would think the ordered list of options would be implicit weights. Perhaps what this should be is three settings: Windows paths, macOS paths, Linux paths. Infer the priority from the order of paths in the list. Have an optional field on the subproperty (the required field being the path) that is the architecture. Eh?

@JustinGrote could you weigh in on this? Really just spitballing how to make this as clean of a user facing settings interface as possible.

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.

Need a better (cross-platform friendly) way to configure additional PowerShell paths

3 participants