Add new additionalPowerShellLocations option#5484
Open
chawyehsu wants to merge 3 commits intoPowerShell:mainfrom
Open
Add new additionalPowerShellLocations option#5484chawyehsu wants to merge 3 commits intoPowerShell:mainfrom
additionalPowerShellLocations option#5484chawyehsu wants to merge 3 commits intoPowerShell:mainfrom
Conversation
2 tasks
c7a0c8d to
12ddce1
Compare
Signed-off-by: Chawye Hsu <su+git@chawyehsu.com>
12ddce1 to
41a2b1d
Compare
There was a problem hiding this comment.
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, andgetSupportedPlatform()plus a new enumeration path for additional locations inPowerShellExeFinder. - Updated
SessionManagerto preferadditionalPowerShellLocationsover legacy settings and to adjust entryweightwhen 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 on lines
+218
to
+222
| const configuredPowerShells = | ||
| this.additionalPowerShellLocations.length > 0 | ||
| ? this.enumerateAdditionalPowerShellLocations() | ||
| : this.enumerateAdditionalPowerShellInstallations(); | ||
| for await (const additionalPwsh of configuredPowerShells) { |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
additionalPowerShellLocations optionadditionalPowerShellLocations option
Signed-off-by: Chawye Hsu <su+git@chawyehsu.com>
additionalPowerShellLocations optionadditionalPowerShellLocations option
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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?: booleanfield, aweight?: numberis used to rank entries and select the best matching entry. Since we cannot restrict thedefault: trueper platform to only one in the VS Code extension configuration, using weighted sorting allows us to achieve a similar purpose. Furthermore, the more specificplatformfield involvingarchis used to specify the target host instead ofos.Backward Compatibility? Yes
While the old
powerShellAdditionalExePathsandpowerShellDefaultVersionstay unchanged and can still be used to define additional PowerShell paths, the newadditionalPowerShellLocationsis added to be mutually exclusive, which means whenadditionalPowerShellLocationsis 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
xbetween the square brackets.Please mark anything not applicable to this PR
NA.WIP:to the beginning of the title and remove the prefix when the PR is ready