From 08fd2b78bb376bd1f1e6cc1bce7e06eaaf790998 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Wed, 6 Jul 2022 13:56:42 -0700 Subject: [PATCH 1/5] Add `utils.checkIfDirectoryExists()` --- src/utils.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/utils.ts b/src/utils.ts index e4c89b2bbe..e2f4567217 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -109,6 +109,16 @@ export function checkIfFileExists(filePath: string): boolean { } } +export function checkIfDirectoryExists(directoryPath: string): boolean { + try { + // tslint:disable-next-line:no-bitwise + fs.accessSync(directoryPath, fs.constants.R_OK | fs.constants.O_DIRECTORY); + return true; + } catch (e) { + return false; + } +} + export function getTimestampString() { const time = new Date(); return `[${time.getHours()}:${time.getMinutes()}:${time.getSeconds()}]`; From 4473bd3b72dcba5e342b282e84fe241173c98e92 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Thu, 7 Jul 2022 09:11:14 -0700 Subject: [PATCH 2/5] Add multi-root choice experience to `powershell.cwd` --- src/main.ts | 17 ++++++++++------- src/settings.ts | 38 ++++++++++++++++++++++++++++++++------ 2 files changed, 42 insertions(+), 13 deletions(-) diff --git a/src/main.ts b/src/main.ts index 65ec2ddc7a..acd3b64d7a 100644 --- a/src/main.ts +++ b/src/main.ts @@ -52,19 +52,25 @@ const documentSelector: DocumentSelector = [ { language: "powershell", scheme: "untitled" }, ]; -export function activate(context: vscode.ExtensionContext): IPowerShellExtensionClient { - // create telemetry reporter on extension activation +// NOTE: Now that this is async, we can probably improve a lot! +export async function activate(context: vscode.ExtensionContext): Promise { telemetryReporter = new TelemetryReporter(PackageJSON.name, PackageJSON.version, AI_KEY); - // If both extensions are enabled, this will cause unexpected behavior since both register the same commands + // If both extensions are enabled, this will cause unexpected behavior since both register the same commands. + // TODO: Merge extensions and use preview channel in marketplace instead. if (PackageJSON.name.toLowerCase() === "powershell-preview" && vscode.extensions.getExtension("ms-vscode.powershell")) { vscode.window.showWarningMessage( "'PowerShell' and 'PowerShell Preview' are both enabled. Please disable one for best performance."); } + // This displays a popup and a changelog after an update. checkForUpdatedVersion(context, PackageJSON.version); + // Load and validate settings (will prompt for 'cwd' if necessary). + await Settings.validateCwdSetting(); + const extensionSettings = Settings.load(); + vscode.languages.setLanguageConfiguration( PowerShellLanguageId, { @@ -118,11 +124,8 @@ export function activate(context: vscode.ExtensionContext): IPowerShellExtension ], }); - // Create the logger + // Setup the logger. logger = new Logger(); - - // Set the log level - const extensionSettings = Settings.load(); logger.MinimumLogLevel = LogLevel[extensionSettings.developer.editorServicesLogLevel]; sessionManager = diff --git a/src/settings.ts b/src/settings.ts index c22eb32b57..325322fbb9 100644 --- a/src/settings.ts +++ b/src/settings.ts @@ -134,10 +134,10 @@ export interface INotebooksSettings { saveMarkdownCellsAs?: CommentType; } +// TODO: This could probably be async, and call `validateCwdSetting()` directly. export function load(): ISettings { const configuration: vscode.WorkspaceConfiguration = - vscode.workspace.getConfiguration( - utils.PowerShellLanguageId); + vscode.workspace.getConfiguration(utils.PowerShellLanguageId); const defaultBugReportingSettings: IBugReportingSettings = { project: "https://github.com/PowerShell/vscode-powershell", @@ -265,17 +265,17 @@ export function load(): ISettings { // is the reason terminals on macOS typically run login shells by default which set up // the environment. See http://unix.stackexchange.com/a/119675/115410" configuration.get("startAsLoginShell", defaultStartAsLoginShellSettings), - cwd: // TODO: Should we resolve this path and/or default to a workspace folder? - configuration.get("cwd", null), + cwd: // NOTE: This must be validated at startup via `validateCwdSetting()`. There's probably a better way to do this. + configuration.get("cwd", undefined), }; } // Get the ConfigurationTarget (read: scope) of where the *effective* setting value comes from export async function getEffectiveConfigurationTarget(settingName: string): Promise { const configuration = vscode.workspace.getConfiguration(utils.PowerShellLanguageId); - const detail = configuration.inspect(settingName); let configurationTarget = null; + if (typeof detail.workspaceFolderValue !== "undefined") { configurationTarget = vscode.ConfigurationTarget.WorkspaceFolder; } @@ -294,7 +294,6 @@ export async function change( configurationTarget?: vscode.ConfigurationTarget | boolean): Promise { const configuration = vscode.workspace.getConfiguration(utils.PowerShellLanguageId); - await configuration.update(settingName, newValue, configurationTarget); } @@ -312,3 +311,30 @@ function getWorkspaceSettingsWithDefaults( } return defaultSettings; } + +export async function validateCwdSetting(): Promise { + let cwd: string = vscode.workspace.getConfiguration(utils.PowerShellLanguageId).get("cwd", null); + + // Only use the cwd setting if it exists. + if (utils.checkIfDirectoryExists(cwd)) { + return cwd; + } else { + // Otherwise use a workspace folder, prompting if necessary. + if (vscode.workspace.workspaceFolders?.length > 1) { + const options: vscode.WorkspaceFolderPickOptions = { + placeHolder: "Select a folder to use as the PowerShell extension's working directory.", + } + cwd = (await vscode.window.showWorkspaceFolderPick(options))?.uri.fsPath; + // Save the picked 'cwd' to the workspace settings. + await change("cwd", cwd); + } else { + cwd = vscode.workspace.workspaceFolders?.[0].uri.fsPath; + } + // If there were no workspace folders, or somehow they don't exist, use + // the home directory. + if (cwd === undefined || !utils.checkIfDirectoryExists(cwd)) { + return process.cwd(); + } + return cwd; + } +} From a128c36456bba607cb71f7275294b0d5e8dbd81d Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Thu, 7 Jul 2022 09:11:46 -0700 Subject: [PATCH 3/5] Handle changes to `powershell.cwd` in restart prompt detection --- src/features/UpdatePowerShell.ts | 4 +-- src/main.ts | 2 +- src/session.ts | 46 ++++++++++++++++++-------------- src/settings.ts | 3 ++- 4 files changed, 31 insertions(+), 24 deletions(-) diff --git a/src/features/UpdatePowerShell.ts b/src/features/UpdatePowerShell.ts index e8ff0680de..33322893d6 100644 --- a/src/features/UpdatePowerShell.ts +++ b/src/features/UpdatePowerShell.ts @@ -164,9 +164,9 @@ export async function InvokePowerShellUpdateCheck( // Invoke the MSI via cmd. const msi = spawn("msiexec", ["/i", msiDownloadPath]); - msi.on("close", (code) => { + msi.on("close", async () => { // Now that the MSI is finished, start the Integrated Console session. - sessionManager.start(); + await sessionManager.start(); fs.unlinkSync(msiDownloadPath); }); diff --git a/src/main.ts b/src/main.ts index acd3b64d7a..ad0665e4a9 100644 --- a/src/main.ts +++ b/src/main.ts @@ -172,7 +172,7 @@ export async function activate(context: vscode.ExtensionContext): Promise { + "Yes", "No"); + if (response === "Yes") { - this.restartSession(); + await this.restartSession(); } - }); } } @@ -433,7 +436,7 @@ export class SessionManager implements Middleware { this.registeredCommands = [ vscode.commands.registerCommand("PowerShell.RestartSession", () => { this.restartSession(); }), vscode.commands.registerCommand(this.ShowSessionMenuCommandName, () => { this.showSessionMenu(); }), - vscode.workspace.onDidChangeConfiguration(() => this.onConfigurationUpdated()), + vscode.workspace.onDidChangeConfiguration(async () => { await this.onConfigurationUpdated(); }), vscode.commands.registerCommand( "PowerShell.ShowSessionConsole", (isExecute?: boolean) => { this.showSessionConsole(isExecute); }), ]; @@ -457,10 +460,10 @@ export class SessionManager implements Middleware { this.sessionSettings); this.languageServerProcess.onExited( - () => { + async () => { if (this.sessionStatus === SessionStatus.Running) { this.setSessionStatus("Session Exited", SessionStatus.Failed); - this.promptForRestart(); + await this.promptForRestart(); } }); @@ -503,11 +506,14 @@ export class SessionManager implements Middleware { }); } - private promptForRestart() { - vscode.window.showErrorMessage( + private async promptForRestart() { + const response: string = await vscode.window.showErrorMessage( "The PowerShell Integrated Console (PSIC) has stopped, would you like to restart it? (IntelliSense will not work unless the PSIC is active and unblocked.)", - "Yes", "No") - .then((answer) => { if (answer === "Yes") { this.restartSession(); }}); + "Yes", "No"); + + if (response === "Yes") { + await this.restartSession(); + } } private startLanguageClient(sessionDetails: utils.IEditorServicesSessionDetails) { @@ -756,7 +762,7 @@ export class SessionManager implements Middleware { // rather than pull from the settings. The issue we prevent here is when a // workspace setting is defined which gets priority over user settings which // is what the change above sets. - this.restartSession(exePath.displayName); + await this.restartSession(exePath.displayName); } private showSessionConsole(isExecute?: boolean) { @@ -817,10 +823,10 @@ export class SessionManager implements Middleware { new SessionMenuItem( "Restart Current Session", - () => { + async () => { // We pass in the display name so we guarantee that the session // will be the same PowerShell. - this.restartSession(this.PowerShellExeDetails.displayName); + await this.restartSession(this.PowerShellExeDetails.displayName); }), new SessionMenuItem( diff --git a/src/settings.ts b/src/settings.ts index 325322fbb9..f3edb1d38a 100644 --- a/src/settings.ts +++ b/src/settings.ts @@ -5,6 +5,7 @@ import vscode = require("vscode"); import utils = require("./utils"); +import os = require("os"); enum CodeFormattingPreset { Custom, @@ -333,7 +334,7 @@ export async function validateCwdSetting(): Promise { // If there were no workspace folders, or somehow they don't exist, use // the home directory. if (cwd === undefined || !utils.checkIfDirectoryExists(cwd)) { - return process.cwd(); + return os.homedir(); } return cwd; } From 4f2fe1eb286b9eb690a5c1a338613801428d9db3 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Mon, 11 Jul 2022 16:39:09 -0700 Subject: [PATCH 4/5] Use `context.storageUri` for logs and support `None` level --- package.json | 5 +++-- src/logging.ts | 39 +++++++++++++++++++++------------------ src/main.ts | 2 +- src/process.ts | 2 +- src/session.ts | 2 +- 5 files changed, 27 insertions(+), 23 deletions(-) diff --git a/package.json b/package.json index 1346e30e48..9a9659626d 100644 --- a/package.json +++ b/package.json @@ -798,10 +798,11 @@ "Verbose", "Normal", "Warning", - "Error" + "Error", + "None" ], "default": "Normal", - "description": "Sets the logging verbosity level for the PowerShell Editor Services host executable. Valid values are 'Diagnostic', 'Verbose', 'Normal', 'Warning', and 'Error'" + "description": "Sets the logging verbosity level for the PowerShell Editor Services host executable. Valid values are 'Diagnostic', 'Verbose', 'Normal', 'Warning', 'Error', and 'None'" }, "powershell.developer.editorServicesWaitForDebugger": { "type": "boolean", diff --git a/src/logging.ts b/src/logging.ts index 5faf23a3d2..b0438a3b48 100644 --- a/src/logging.ts +++ b/src/logging.ts @@ -13,6 +13,7 @@ export enum LogLevel { Normal, Warning, Error, + None, } /** Interface for logging operations. New features should use this interface for the "type" of logger. @@ -29,19 +30,24 @@ export interface ILogger { export class Logger implements ILogger { - public logBasePath: string; - public logSessionPath: string; + public logBasePath: vscode.Uri; + public logSessionPath: vscode.Uri; public MinimumLogLevel: LogLevel = LogLevel.Normal; private commands: vscode.Disposable[]; private logChannel: vscode.OutputChannel; - private logFilePath: string; + private logFilePath: vscode.Uri; - constructor() { + constructor(logBasePath: vscode.Uri) { this.logChannel = vscode.window.createOutputChannel("PowerShell Extension Logs"); - this.logBasePath = path.resolve(__dirname, "../logs"); - utils.ensurePathExists(this.logBasePath); + if (logBasePath === undefined) { + // No workspace, we have to use another folder. + this.logBasePath = vscode.Uri.file(path.resolve(__dirname, "../logs")); + utils.ensurePathExists(this.logBasePath.fsPath); + } else { + this.logBasePath = vscode.Uri.joinPath(logBasePath, "logs"); + } this.commands = [ vscode.commands.registerCommand( @@ -59,8 +65,8 @@ export class Logger implements ILogger { this.logChannel.dispose(); } - public getLogFilePath(baseName: string): string { - return path.resolve(this.logSessionPath, `${baseName}.log`); + public getLogFilePath(baseName: string): vscode.Uri { + return vscode.Uri.joinPath(this.logSessionPath, `${baseName}.log`); } public writeAtLevel(logLevel: LogLevel, message: string, ...additionalMessages: string[]) { @@ -136,17 +142,16 @@ export class Logger implements ILogger { } } - public startNewLog(minimumLogLevel: string = "Normal") { + public async startNewLog(minimumLogLevel: string = "Normal") { this.MinimumLogLevel = this.logLevelNameToValue(minimumLogLevel.trim()); this.logSessionPath = - path.resolve( + vscode.Uri.joinPath( this.logBasePath, `${Math.floor(Date.now() / 1000)}-${vscode.env.sessionId}`); this.logFilePath = this.getLogFilePath("vscode-powershell"); - - utils.ensurePathExists(this.logSessionPath); + await vscode.workspace.fs.createDirectory(this.logSessionPath); } private logLevelNameToValue(logLevelName: string): LogLevel { @@ -156,6 +161,7 @@ export class Logger implements ILogger { case "normal": return LogLevel.Normal; case "warning": return LogLevel.Warning; case "error": return LogLevel.Error; + case "none": return LogLevel.None; default: return LogLevel.Normal; } } @@ -168,10 +174,7 @@ export class Logger implements ILogger { if (this.logSessionPath) { // Open the folder in VS Code since there isn't an easy way to // open the folder in the platform's file browser - vscode.commands.executeCommand( - "vscode.openFolder", - vscode.Uri.file(this.logSessionPath), - true); + vscode.commands.executeCommand("vscode.openFolder", this.logSessionPath, true); } } @@ -181,9 +184,9 @@ export class Logger implements ILogger { `${now.toLocaleDateString()} ${now.toLocaleTimeString()} [${LogLevel[level].toUpperCase()}] - ${message}`; this.logChannel.appendLine(timestampedMessage); - if (this.logFilePath) { + if (this.logFilePath && this.MinimumLogLevel !== LogLevel.None) { fs.appendFile( - this.logFilePath, + this.logFilePath.fsPath, timestampedMessage + os.EOL, (err) => { if (err) { diff --git a/src/main.ts b/src/main.ts index ad0665e4a9..2854cd58cc 100644 --- a/src/main.ts +++ b/src/main.ts @@ -125,7 +125,7 @@ export async function activate(context: vscode.ExtensionContext): Promise Date: Tue, 12 Jul 2022 15:43:19 -0700 Subject: [PATCH 5/5] Add `getStorageUri` to API (to fix tests) --- src/features/ExternalApi.ts | 5 +++++ src/main.ts | 1 + test/core/paths.test.ts | 9 ++++++-- test/features/ExternalApi.test.ts | 34 +++++++++++++++---------------- test/utils.ts | 16 +++++++-------- 5 files changed, 37 insertions(+), 28 deletions(-) diff --git a/src/features/ExternalApi.ts b/src/features/ExternalApi.ts index a715454c7d..9ae950bace 100644 --- a/src/features/ExternalApi.ts +++ b/src/features/ExternalApi.ts @@ -19,6 +19,7 @@ export interface IPowerShellExtensionClient { unregisterExternalExtension(uuid: string): boolean; getPowerShellVersionDetails(uuid: string): Promise; waitUntilStarted(uuid: string): Promise; + getStorageUri(): vscode.Uri; } /* @@ -166,6 +167,10 @@ export class ExternalApiFeature extends LanguageClientConsumer implements IPower return this.sessionManager.waitUntilStarted(); } + public getStorageUri(): vscode.Uri { + return this.extensionContext.storageUri; + } + public dispose() { // Nothing to dispose. } diff --git a/src/main.ts b/src/main.ts index 2854cd58cc..8a48463662 100644 --- a/src/main.ts +++ b/src/main.ts @@ -180,6 +180,7 @@ export async function activate(context: vscode.ExtensionContext): Promise externalApi.unregisterExternalExtension(uuid), getPowerShellVersionDetails: uuid => externalApi.getPowerShellVersionDetails(uuid), waitUntilStarted: uuid => externalApi.waitUntilStarted(uuid), + getStorageUri: () => externalApi.getStorageUri(), }; } diff --git a/test/core/paths.test.ts b/test/core/paths.test.ts index 99f31a3639..8559db5f79 100644 --- a/test/core/paths.test.ts +++ b/test/core/paths.test.ts @@ -5,10 +5,15 @@ import * as assert from "assert"; import * as fs from "fs"; import * as path from "path"; import * as vscode from "vscode"; +import { IPowerShellExtensionClient } from "../../src/features/ExternalApi"; import utils = require("../utils"); describe("Path assumptions", function () { - before(utils.ensureEditorServicesIsConnected); + let storageUri: vscode.Uri; + before(async () => { + const extension: IPowerShellExtensionClient = await utils.ensureEditorServicesIsConnected(); + storageUri = extension.getStorageUri(); + }); // TODO: This is skipped because it interferes with other tests. Either // need to find a way to close the opened folder via a Code API, or find @@ -22,6 +27,6 @@ describe("Path assumptions", function () { }); it("Creates the log folder at the correct path", function () { - assert(fs.existsSync(path.resolve(utils.rootPath, "logs"))); + assert(fs.existsSync(vscode.Uri.joinPath(storageUri, "logs").fsPath)); }); }); diff --git a/test/features/ExternalApi.test.ts b/test/features/ExternalApi.test.ts index 0bc44c2811..4c946096d9 100644 --- a/test/features/ExternalApi.test.ts +++ b/test/features/ExternalApi.test.ts @@ -7,51 +7,50 @@ import { IExternalPowerShellDetails, IPowerShellExtensionClient } from "../../sr describe("ExternalApi feature", function () { describe("External extension registration", function () { - let powerShellExtensionClient: IPowerShellExtensionClient; + let extension: IPowerShellExtensionClient; before(async function () { - const powershellExtension = await utils.ensureExtensionIsActivated(); - powerShellExtensionClient = powershellExtension!.exports as IPowerShellExtensionClient; + extension = await utils.ensureExtensionIsActivated(); }); it("Registers and unregisters an extension", function () { - const sessionId: string = powerShellExtensionClient.registerExternalExtension(utils.extensionId); + const sessionId: string = extension.registerExternalExtension(utils.extensionId); assert.notStrictEqual(sessionId, ""); assert.notStrictEqual(sessionId, null); assert.strictEqual( - powerShellExtensionClient.unregisterExternalExtension(sessionId), + extension.unregisterExternalExtension(sessionId), true); }); it("Registers and unregisters an extension with a version", function () { - const sessionId: string = powerShellExtensionClient.registerExternalExtension(utils.extensionId, "v2"); + const sessionId: string = extension.registerExternalExtension(utils.extensionId, "v2"); assert.notStrictEqual(sessionId, ""); assert.notStrictEqual(sessionId, null); assert.strictEqual( - powerShellExtensionClient.unregisterExternalExtension(sessionId), + extension.unregisterExternalExtension(sessionId), true); }); it("Rejects if not registered", async function () { assert.rejects( - async () => await powerShellExtensionClient.getPowerShellVersionDetails("")) + async () => await extension.getPowerShellVersionDetails("")) }); it("Throws if attempting to register an extension more than once", async function () { - const sessionId: string = powerShellExtensionClient.registerExternalExtension(utils.extensionId); + const sessionId: string = extension.registerExternalExtension(utils.extensionId); try { assert.throws( - () => powerShellExtensionClient.registerExternalExtension(utils.extensionId), + () => extension.registerExternalExtension(utils.extensionId), { message: `The extension '${utils.extensionId}' is already registered.` }); } finally { - powerShellExtensionClient.unregisterExternalExtension(sessionId); + extension.unregisterExternalExtension(sessionId); } }); it("Throws when unregistering an extension that isn't registered", async function () { assert.throws( - () => powerShellExtensionClient.unregisterExternalExtension("not-real"), + () => extension.unregisterExternalExtension("not-real"), { message: `No extension registered with session UUID: not-real` }); @@ -60,18 +59,17 @@ describe("ExternalApi feature", function () { describe("PowerShell version details", () => { let sessionId: string; - let powerShellExtensionClient: IPowerShellExtensionClient; + let extension: IPowerShellExtensionClient; before(async function () { - const powershellExtension = await utils.ensureExtensionIsActivated(); - powerShellExtensionClient = powershellExtension!.exports as IPowerShellExtensionClient; - sessionId = powerShellExtensionClient.registerExternalExtension(utils.extensionId); + extension = await utils.ensureExtensionIsActivated(); + sessionId = extension.registerExternalExtension(utils.extensionId); }); - after(function () { powerShellExtensionClient.unregisterExternalExtension(sessionId); }); + after(function () { extension.unregisterExternalExtension(sessionId); }); it("Gets non-empty version details from the PowerShell Editor Services", async function () { - const versionDetails: IExternalPowerShellDetails = await powerShellExtensionClient.getPowerShellVersionDetails(sessionId); + const versionDetails: IExternalPowerShellDetails = await extension.getPowerShellVersionDetails(sessionId); assert.notStrictEqual(versionDetails.architecture, ""); assert.notStrictEqual(versionDetails.architecture, null); diff --git a/test/utils.ts b/test/utils.ts index c9710b5b71..b47c113e9c 100644 --- a/test/utils.ts +++ b/test/utils.ts @@ -14,16 +14,16 @@ export const rootPath = path.resolve(__dirname, "../../") const packageJSON: any = require(path.resolve(rootPath, "package.json")); export const extensionId = `${packageJSON.publisher}.${packageJSON.name}`; -export async function ensureExtensionIsActivated(): Promise> { +export async function ensureExtensionIsActivated(): Promise { const extension = vscode.extensions.getExtension(extensionId); if (!extension.isActive) { await extension.activate(); } - return extension; + return extension!.exports as IPowerShellExtensionClient; } -export async function ensureEditorServicesIsConnected(): Promise { - const powershellExtension = await ensureExtensionIsActivated(); - const client = powershellExtension!.exports as IPowerShellExtensionClient; - const sessionId = client.registerExternalExtension(extensionId); - await client.waitUntilStarted(sessionId); - client.unregisterExternalExtension(sessionId); +export async function ensureEditorServicesIsConnected(): Promise { + const extension = await ensureExtensionIsActivated(); + const sessionId = extension.registerExternalExtension(extensionId); + await extension.waitUntilStarted(sessionId); + extension.unregisterExternalExtension(sessionId); + return extension; }