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.

Sign up for GitHub

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

Improve warning when additional PowerShell isn't found #5099

Merged
merged 1 commit into from
Nov 21, 2024
Merged

Improve warning when additional PowerShell isn't found #5099

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions src/platform.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,17 +239,17 @@ export class PowerShellExeFinder {
}

exePath = untildify(exePath);

// Always search for what the user gave us first
yield new PossiblePowerShellExe(exePath, versionName);

// Also search for `pwsh[.exe]` and `powershell[.exe]` if missing
const args: [string, undefined, boolean, boolean]
// Must be a tuple type and is suppressing the warning
= [versionName, undefined, true, true];

// Handle Windows where '.exe' and 'powershell' are things
// Always search for what the user gave us first, but with the warning
// suppressed so we can display it after all possibilities are exhausted
yield new PossiblePowerShellExe(exePath, ...args);

// Also search for `pwsh[.exe]` and `powershell[.exe]` if missing
if (this.platformDetails.operatingSystem === OperatingSystem.Windows) {
// Handle Windows where '.exe' and 'powershell' are things
if (!exePath.endsWith("pwsh.exe") && !exePath.endsWith("powershell.exe")) {
if (exePath.endsWith("pwsh") || exePath.endsWith("powershell")) {
// Add extension if that was missing
Expand All @@ -260,9 +260,13 @@ export class PowerShellExeFinder {
yield new PossiblePowerShellExe(path.join(exePath, "pwsh.exe"), ...args);
yield new PossiblePowerShellExe(path.join(exePath, "powershell.exe"), ...args);
}
} else if (!exePath.endsWith("pwsh")) { // Always just 'pwsh' on non-Windows
} else if (!exePath.endsWith("pwsh")) {
// Always just 'pwsh' on non-Windows
yield new PossiblePowerShellExe(path.join(exePath, "pwsh"), ...args);
}

// If we're still being iterated over, no permutation of the given path existed so yield an object with the warning unsuppressed
yield new PossiblePowerShellExe(exePath, versionName, false, undefined, false);
}
}
}
Expand Down
77 changes: 76 additions & 1 deletion test/core/platform.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,17 @@ if (process.platform === "win32") {
isProcess64Bit: true,
},
environmentVars: {},
// Note that for each given path, we expect:
// 1. The path as-is.
// 2. Any expected permutations of the path (for example, with a tilde or folder expanded, and/or '.exe' added).
// 3. The path as-is again (in order for a warning to be displayed at the correct time).
// An improvement here would be to check the suppressWarning field, but it's not currently exposed.
expectedPowerShellSequence: [
{
exePath: "C:\\Users\\test\\pwsh\\pwsh.exe",
displayName: "pwsh",
supportsProperArguments: true
},
{
exePath: "C:\\Users\\test\\pwsh\\pwsh.exe",
displayName: "pwsh",
Expand All @@ -479,6 +489,11 @@ if (process.platform === "win32") {
displayName: "pwsh-tilde",
supportsProperArguments: true
},
{
exePath: path.join(os.homedir(), "pwsh", "pwsh.exe"),
displayName: "pwsh-tilde",
supportsProperArguments: true
},
{
exePath: "C:\\Users\\test\\pwsh\\pwsh",
displayName: "pwsh-no-exe",
Expand All @@ -499,6 +514,11 @@ if (process.platform === "win32") {
displayName: "pwsh-no-exe",
supportsProperArguments: true
},
{
exePath: "C:\\Users\\test\\pwsh\\pwsh",
displayName: "pwsh-no-exe",
supportsProperArguments: true
},
{
exePath: "C:\\Users\\test\\pwsh\\",
displayName: "pwsh-folder",
Expand All @@ -514,6 +534,11 @@ if (process.platform === "win32") {
displayName: "pwsh-folder",
supportsProperArguments: true
},
{
exePath: "C:\\Users\\test\\pwsh\\",
displayName: "pwsh-folder",
supportsProperArguments: true
},
{
exePath: "C:\\Users\\test\\pwsh",
displayName: "pwsh-folder-no-slash",
Expand All @@ -534,6 +559,16 @@ if (process.platform === "win32") {
displayName: "pwsh-folder-no-slash",
supportsProperArguments: true
},
{
exePath: "C:\\Users\\test\\pwsh",
displayName: "pwsh-folder-no-slash",
supportsProperArguments: true
},
{
exePath: "C:\\Users\\test\\pwsh\\pwsh.exe",
displayName: "pwsh-single-quotes",
supportsProperArguments: true
},
{
exePath: "C:\\Users\\test\\pwsh\\pwsh.exe",
displayName: "pwsh-single-quotes",
Expand All @@ -544,6 +579,11 @@ if (process.platform === "win32") {
displayName: "pwsh-double-quotes",
supportsProperArguments: true
},
{
exePath: "C:\\Users\\test\\pwsh\\pwsh.exe",
displayName: "pwsh-double-quotes",
supportsProperArguments: true
},
],
filesystem: {},
}
Expand Down Expand Up @@ -760,19 +800,34 @@ if (process.platform === "win32") {

successAdditionalTestCases = [
{ // Also sufficient for macOS as the behavior is the same
name: "Linux (Additional PowerShell Executables)",
name: "Linux/macOS (Additional PowerShell Executables)",
platformDetails: {
operatingSystem: platform.OperatingSystem.Linux,
isOS64Bit: true,
isProcess64Bit: true,
},
environmentVars: {},
// Note that for each given path, we expect:
// 1. The path as-is.
// 2. Any expected permutations of the path (for example, with a tilde or folder expanded).
// 3. The path as-is again (in order for a warning to be displayed at the correct time).
// An improvement here would be to check the suppressWarning field, but it's not currently exposed.
expectedPowerShellSequence: [
{
exePath: "/home/bin/pwsh",
displayName: "pwsh",
supportsProperArguments: true
},
{
exePath: "/home/bin/pwsh",
displayName: "pwsh",
supportsProperArguments: true
},
{
exePath: path.join(os.homedir(), "bin", "pwsh"),
displayName: "pwsh-tilde",
supportsProperArguments: true
},
{
exePath: path.join(os.homedir(), "bin", "pwsh"),
displayName: "pwsh-tilde",
Expand All @@ -788,6 +843,11 @@ if (process.platform === "win32") {
displayName: "pwsh-folder",
supportsProperArguments: true
},
{
exePath: "/home/bin/",
displayName: "pwsh-folder",
supportsProperArguments: true
},
{
exePath: "/home/bin",
displayName: "pwsh-folder-no-slash",
Expand All @@ -798,6 +858,16 @@ if (process.platform === "win32") {
displayName: "pwsh-folder-no-slash",
supportsProperArguments: true
},
{
exePath: "/home/bin",
displayName: "pwsh-folder-no-slash",
supportsProperArguments: true
},
{
exePath: "/home/bin/pwsh",
displayName: "pwsh-single-quotes",
supportsProperArguments: true
},
{
exePath: "/home/bin/pwsh",
displayName: "pwsh-single-quotes",
Expand All @@ -808,6 +878,11 @@ if (process.platform === "win32") {
displayName: "pwsh-double-quotes",
supportsProperArguments: true
},
{
exePath: "/home/bin/pwsh",
displayName: "pwsh-double-quotes",
supportsProperArguments: true
},
],
filesystem: {},
}
Expand Down