Skip to content

WIP: Add support for side-by-side PS Core preview on Linux/macOS #1366

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

Merged
merged 6 commits into from
Jul 2, 2018
Merged

WIP: Add support for side-by-side PS Core preview on Linux/macOS #1366

Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b64259c
Add support for side-by-side PS Core preview on Linux/macOS
rkeithhill Jun 14, 2018
65d5a56
Adjust name of default PS Core on Linux/macOS
rkeithhill Jun 14, 2018
2ef97e4
Make a bit DRY-er. Thx @tylerl0706
rkeithhill Jun 14, 2018
fc22305
Remove support for pre-ship PS Core Linux/macOS binary name - powershell
rkeithhill Jun 19, 2018
d85f943
Add support for PowerShell Core installed in a Snap
rkeithhill Jun 28, 2018
7545dfd
Simplify impl of getting available PS on Linux/macOS
rkeithhill Jul 1, 2018
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
49 changes: 39 additions & 10 deletions src/platform.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,18 @@
*--------------------------------------------------------*/

import fs = require("fs");
import os = require("os");
import path = require("path");
import process = require("process");
import vscode = require("vscode");
import Settings = require("./settings");

const linuxLegacyExePath = "/usr/bin/powershell";
const linuxExePath = "/usr/bin/pwsh";
const linuxPreviewExePath = "/usr/bin/pwsh-preview";

const macOSLegacyExePath = "/usr/local/bin/powershell";
const macOSExePath = "/usr/local/bin/pwsh";
const macOSPreviewExePath = "/usr/local/bin/pwsh-preview";

export enum OperatingSystem {
Unknown,
Windows,
Expand Down Expand Up @@ -68,14 +74,18 @@ export function getDefaultPowerShellPath(
: SysnativePowerShellPath;
}
} else if (platformDetails.operatingSystem === OperatingSystem.MacOS) {
powerShellExePath = "/usr/local/bin/powershell";
if (fs.existsSync("/usr/local/bin/pwsh")) {
powerShellExePath = "/usr/local/bin/pwsh";
powerShellExePath = macOSLegacyExePath;
if (fs.existsSync(macOSExePath)) {
powerShellExePath = macOSExePath;
} else if (fs.existsSync(macOSPreviewExePath)) {
powerShellExePath = macOSPreviewExePath;
}
} else if (platformDetails.operatingSystem === OperatingSystem.Linux) {
powerShellExePath = "/usr/bin/powershell";
if (fs.existsSync("/usr/bin/pwsh")) {
powerShellExePath = "/usr/bin/pwsh";
powerShellExePath = linuxLegacyExePath;
if (fs.existsSync(linuxExePath)) {
powerShellExePath = linuxExePath;
} else if (fs.existsSync(linuxPreviewExePath)) {
powerShellExePath = linuxPreviewExePath;
}
}

Expand Down Expand Up @@ -162,10 +172,29 @@ export function getAvailablePowerShellExes(
}
} else {
// Handle Linux and macOS case
const defaultExePath = this.getDefaultPowerShellPath(platformDetails);
paths.push({
versionName: "PowerShell Core",
exePath: this.getDefaultPowerShellPath(platformDetails),
versionName: "PowerShell Core" + (/-preview/.test(defaultExePath) ? " Preview" : ""),
exePath: defaultExePath,
});

// If defaultExePath is pwsh, check to see if pwsh-preview is installed and if so, make it available.
// If the defaultExePath is already pwsh-preview, then pwsh is not installed - nothing to do.
if (platformDetails.operatingSystem === OperatingSystem.MacOS) {
if ((defaultExePath === macOSExePath) && fs.existsSync(macOSPreviewExePath)) {
paths.push({
versionName: "PowerShell Core Preview",
exePath: macOSPreviewExePath,
});
}
} else if (platformDetails.operatingSystem === OperatingSystem.Linux) {
if ((defaultExePath === linuxExePath) && fs.existsSync(linuxPreviewExePath)) {
paths.push({
versionName: "PowerShell Core Preview",
exePath: linuxPreviewExePath,
});
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I refactored this a bit to define the paths conditionally and then do the logic. I think it's a little DRY-er. Totally, not a blocker but what do you think?

let osExePath, osPreviewExePath;
if (platformDetails.operatingSystem === OperatingSystem.MacOS) {
    osExePath = macOSExePath;
    osPreviewExePath = macOSPreviewExePath;
} else if (platformDetails.operatingSystem === OperatingSystem.Linux) {
    osExePath = linuxExePath;
    osPreviewExePath = linuxPreviewExePath;
}

if ((defaultExePath === osExePath) && fs.existsSync(osPreviewExePath)) {
    paths.push({
        versionName: "PowerShell Core Preview",
        exePath: linuxPreviewExePath,
    });
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Can you make this change on the PR branch and merge? I think I've allowed it to be edited. If not, I should be able to get to this tonight.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated. Please test on macOS and Linux.

}
}

// When unit testing, we don't have session settings to test so skip reading this setting
Expand Down