Skip to content

Commit 7a42c84

Browse files
andyleejordanandyleejordan
committed
Improve warning when additional PowerShell isn't found
Since we added logic which searches for possible intended permutations of the given additional PowerShell path, we needed to make the warning show only if none of the permutations were found. This was accomplished by suppressing it in the first iterator and then yielding it again after the permutations were exhausted with the warning unsuppressed.
1 parent 0b61253 commit 7a42c84

File tree

2 files changed

+87
-8
lines changed

2 files changed

+87
-8
lines changed

‎src/platform.ts‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -239,17 +239,17 @@ export class PowerShellExeFinder {
239239
}
240240

241241
exePath = untildify(exePath);
242-
243-
// Always search for what the user gave us first
244-
yield new PossiblePowerShellExe(exePath, versionName);
245-
246-
// Also search for `pwsh[.exe]` and `powershell[.exe]` if missing
247242
const args: [string, undefined, boolean, boolean]
248243
// Must be a tuple type and is suppressing the warning
249244
= [versionName, undefined, true, true];
250245

251-
// Handle Windows where '.exe' and 'powershell' are things
246+
// Always search for what the user gave us first, but with the warning
247+
// suppressed so we can display it after all possibilities are exhausted
248+
yield new PossiblePowerShellExe(exePath, ...args);
249+
250+
// Also search for `pwsh[.exe]` and `powershell[.exe]` if missing
252251
if (this.platformDetails.operatingSystem === OperatingSystem.Windows) {
252+
// Handle Windows where '.exe' and 'powershell' are things
253253
if (!exePath.endsWith("pwsh.exe") && !exePath.endsWith("powershell.exe")) {
254254
if (exePath.endsWith("pwsh") || exePath.endsWith("powershell")) {
255255
// Add extension if that was missing
@@ -260,9 +260,13 @@ export class PowerShellExeFinder {
260260
yield new PossiblePowerShellExe(path.join(exePath, "pwsh.exe"), ...args);
261261
yield new PossiblePowerShellExe(path.join(exePath, "powershell.exe"), ...args);
262262
}
263-
} else if (!exePath.endsWith("pwsh")) { // Always just 'pwsh' on non-Windows
263+
} else if (!exePath.endsWith("pwsh")) {
264+
// Always just 'pwsh' on non-Windows
264265
yield new PossiblePowerShellExe(path.join(exePath, "pwsh"), ...args);
265266
}
267+
268+
// If we're still being iterated over, no permutation of the given path existed so yield an object with the warning unsuppressed
269+
yield new PossiblePowerShellExe(exePath, versionName, false, undefined, false);
266270
}
267271
}
268272
}

‎test/core/platform.test.ts‎

Lines changed: 76 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -468,7 +468,17 @@ if (process.platform === "win32") {
468468
isProcess64Bit: true,
469469
},
470470
environmentVars: {},
471+
// Note that for each given path, we expect:
472+
// 1. The path as-is.
473+
// 2. Any expected permutations of the path (for example, with a tilde or folder expanded, and/or '.exe' added).
474+
// 3. The path as-is again (in order for a warning to be displayed at the correct time).
475+
// An improvement here would be to check the suppressWarning field, but it's not currently exposed.
471476
expectedPowerShellSequence: [
477+
{
478+
exePath: "C:\\Users\\test\\pwsh\\pwsh.exe",
479+
displayName: "pwsh",
480+
supportsProperArguments: true
481+
},
472482
{
473483
exePath: "C:\\Users\\test\\pwsh\\pwsh.exe",
474484
displayName: "pwsh",
@@ -479,6 +489,11 @@ if (process.platform === "win32") {
479489
displayName: "pwsh-tilde",
480490
supportsProperArguments: true
481491
},
492+
{
493+
exePath: path.join(os.homedir(), "pwsh", "pwsh.exe"),
494+
displayName: "pwsh-tilde",
495+
supportsProperArguments: true
496+
},
482497
{
483498
exePath: "C:\\Users\\test\\pwsh\\pwsh",
484499
displayName: "pwsh-no-exe",
@@ -499,6 +514,11 @@ if (process.platform === "win32") {
499514
displayName: "pwsh-no-exe",
500515
supportsProperArguments: true
501516
},
517+
{
518+
exePath: "C:\\Users\\test\\pwsh\\pwsh",
519+
displayName: "pwsh-no-exe",
520+
supportsProperArguments: true
521+
},
502522
{
503523
exePath: "C:\\Users\\test\\pwsh\\",
504524
displayName: "pwsh-folder",
@@ -514,6 +534,11 @@ if (process.platform === "win32") {
514534
displayName: "pwsh-folder",
515535
supportsProperArguments: true
516536
},
537+
{
538+
exePath: "C:\\Users\\test\\pwsh\\",
539+
displayName: "pwsh-folder",
540+
supportsProperArguments: true
541+
},
517542
{
518543
exePath: "C:\\Users\\test\\pwsh",
519544
displayName: "pwsh-folder-no-slash",
@@ -534,6 +559,16 @@ if (process.platform === "win32") {
534559
displayName: "pwsh-folder-no-slash",
535560
supportsProperArguments: true
536561
},
562+
{
563+
exePath: "C:\\Users\\test\\pwsh",
564+
displayName: "pwsh-folder-no-slash",
565+
supportsProperArguments: true
566+
},
567+
{
568+
exePath: "C:\\Users\\test\\pwsh\\pwsh.exe",
569+
displayName: "pwsh-single-quotes",
570+
supportsProperArguments: true
571+
},
537572
{
538573
exePath: "C:\\Users\\test\\pwsh\\pwsh.exe",
539574
displayName: "pwsh-single-quotes",
@@ -544,6 +579,11 @@ if (process.platform === "win32") {
544579
displayName: "pwsh-double-quotes",
545580
supportsProperArguments: true
546581
},
582+
{
583+
exePath: "C:\\Users\\test\\pwsh\\pwsh.exe",
584+
displayName: "pwsh-double-quotes",
585+
supportsProperArguments: true
586+
},
547587
],
548588
filesystem: {},
549589
}
@@ -760,19 +800,34 @@ if (process.platform === "win32") {
760800

761801
successAdditionalTestCases = [
762802
{ // Also sufficient for macOS as the behavior is the same
763-
name: "Linux (Additional PowerShell Executables)",
803+
name: "Linux/macOS (Additional PowerShell Executables)",
764804
platformDetails: {
765805
operatingSystem: platform.OperatingSystem.Linux,
766806
isOS64Bit: true,
767807
isProcess64Bit: true,
768808
},
769809
environmentVars: {},
810+
// Note that for each given path, we expect:
811+
// 1. The path as-is.
812+
// 2. Any expected permutations of the path (for example, with a tilde or folder expanded).
813+
// 3. The path as-is again (in order for a warning to be displayed at the correct time).
814+
// An improvement here would be to check the suppressWarning field, but it's not currently exposed.
770815
expectedPowerShellSequence: [
771816
{
772817
exePath: "/home/bin/pwsh",
773818
displayName: "pwsh",
774819
supportsProperArguments: true
775820
},
821+
{
822+
exePath: "/home/bin/pwsh",
823+
displayName: "pwsh",
824+
supportsProperArguments: true
825+
},
826+
{
827+
exePath: path.join(os.homedir(), "bin", "pwsh"),
828+
displayName: "pwsh-tilde",
829+
supportsProperArguments: true
830+
},
776831
{
777832
exePath: path.join(os.homedir(), "bin", "pwsh"),
778833
displayName: "pwsh-tilde",
@@ -788,6 +843,11 @@ if (process.platform === "win32") {
788843
displayName: "pwsh-folder",
789844
supportsProperArguments: true
790845
},
846+
{
847+
exePath: "/home/bin/",
848+
displayName: "pwsh-folder",
849+
supportsProperArguments: true
850+
},
791851
{
792852
exePath: "/home/bin",
793853
displayName: "pwsh-folder-no-slash",
@@ -798,6 +858,16 @@ if (process.platform === "win32") {
798858
displayName: "pwsh-folder-no-slash",
799859
supportsProperArguments: true
800860
},
861+
{
862+
exePath: "/home/bin",
863+
displayName: "pwsh-folder-no-slash",
864+
supportsProperArguments: true
865+
},
866+
{
867+
exePath: "/home/bin/pwsh",
868+
displayName: "pwsh-single-quotes",
869+
supportsProperArguments: true
870+
},
801871
{
802872
exePath: "/home/bin/pwsh",
803873
displayName: "pwsh-single-quotes",
@@ -808,6 +878,11 @@ if (process.platform === "win32") {
808878
displayName: "pwsh-double-quotes",
809879
supportsProperArguments: true
810880
},
881+
{
882+
exePath: "/home/bin/pwsh",
883+
displayName: "pwsh-double-quotes",
884+
supportsProperArguments: true
885+
},
811886
],
812887
filesystem: {},
813888
}

0 commit comments

Comments
 (0)