Skip to content

Commit

Permalink
Fix additional PowerShell warning (take two)
Browse files Browse the repository at this point in the history
Since Show Session Menu always fully enumerates the iterator we needed
to move the existence check into the generator. Fortunately the 'exists'
method was already idempotent. I'd like this to be cleaner, but at least
the tests now make sense (and required a stub fix).
  • Loading branch information
andyleejordan committed Nov 22, 2024
1 parent 7a42c84 commit 3c71fcf
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 159 deletions.
34 changes: 27 additions & 7 deletions src/platform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ export class PowerShellExeFinder {

// Also show any additionally configured PowerShells
// These may be duplicates of the default installations, but given a different name.
for (const additionalPwsh of this.enumerateAdditionalPowerShellInstallations()) {
for await (const additionalPwsh of this.enumerateAdditionalPowerShellInstallations()) {
if (await additionalPwsh.exists()) {
yield additionalPwsh;
} else if (!additionalPwsh.suppressWarning) {
Expand Down Expand Up @@ -230,7 +230,7 @@ export class PowerShellExeFinder {
* Iterates through the configured additional PowerShell executable locations,
* without checking for their existence.
*/
private *enumerateAdditionalPowerShellInstallations(): Iterable<IPossiblePowerShellExe> {
private async *enumerateAdditionalPowerShellInstallations(): AsyncIterable<IPossiblePowerShellExe> {
for (const versionName in this.additionalPowerShellExes) {
if (Object.prototype.hasOwnProperty.call(this.additionalPowerShellExes, versionName)) {
let exePath: string | undefined = utils.stripQuotePair(this.additionalPowerShellExes[versionName]);
Expand All @@ -245,24 +245,44 @@ export class PowerShellExeFinder {

// 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);
let pwsh = new PossiblePowerShellExe(exePath, ...args);
if (await pwsh.exists()) {
yield pwsh;
continue;
}

// 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
yield new PossiblePowerShellExe(exePath + ".exe", ...args);
pwsh = new PossiblePowerShellExe(exePath + ".exe", ...args);
if (await pwsh.exists()) {
yield pwsh;
continue;
}
}
// Also add full exe names (this isn't an else just in case
// the folder was named "pwsh" or "powershell")
yield new PossiblePowerShellExe(path.join(exePath, "pwsh.exe"), ...args);
yield new PossiblePowerShellExe(path.join(exePath, "powershell.exe"), ...args);
pwsh = new PossiblePowerShellExe(path.join(exePath, "pwsh.exe"), ...args);
if (await pwsh.exists()) {
yield pwsh;
continue;
}
pwsh = new PossiblePowerShellExe(path.join(exePath, "powershell.exe"), ...args);
if (await pwsh.exists()) {
yield pwsh;
continue;
}
}
} else if (!exePath.endsWith("pwsh")) {
// Always just 'pwsh' on non-Windows
yield new PossiblePowerShellExe(path.join(exePath, "pwsh"), ...args);
pwsh = new PossiblePowerShellExe(path.join(exePath, "pwsh"), ...args);
if (await pwsh.exists()) {
yield pwsh;
continue;
}
}

// If we're still being iterated over, no permutation of the given path existed so yield an object with the warning unsuppressed
Expand Down
Loading

0 comments on commit 3c71fcf

Please sign in to comment.