From d6d1ad3721b142ac4fc200b916f7223d728adecf Mon Sep 17 00:00:00 2001 From: Andy Jordan <2226434+andyleejordan@users.noreply.github.com> Date: Wed, 20 Nov 2024 14:01:11 -0800 Subject: [PATCH] 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. --- src/platform.ts | 18 +++++---- test/core/platform.test.ts | 77 +++++++++++++++++++++++++++++++++++++- 2 files changed, 87 insertions(+), 8 deletions(-) diff --git a/src/platform.ts b/src/platform.ts index 5e63dca6bd..706c9f56e2 100644 --- a/src/platform.ts +++ b/src/platform.ts @@ -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 @@ -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); } } } diff --git a/test/core/platform.test.ts b/test/core/platform.test.ts index 853b550685..299e125626 100644 --- a/test/core/platform.test.ts +++ b/test/core/platform.test.ts @@ -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", @@ -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", @@ -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", @@ -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", @@ -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", @@ -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: {}, } @@ -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", @@ -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", @@ -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", @@ -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: {}, }