Log when settings are changed by extension

Author: andyleejordan

src/features/UpdatePowerShell.ts

@@ -12,6 +12,7 @@ import * as util from "util";
 import { MessageItem, ProgressLocation, window } from "vscode";
 
 import { LanguageClient } from "vscode-languageclient/node";
+import { Logger } from "../logging";
 import { SessionManager } from "../session";
 import * as Settings from "../settings";
 import { isMacOS, isWindows } from "../utils";
@@ -21,7 +22,7 @@ const streamPipeline = util.promisify(stream.pipeline);
 
 const PowerShellGitHubReleasesUrl =
     "https://api.github.com/repos/PowerShell/PowerShell/releases/latest";
-const PowerShellGitHubPrereleasesUrl =
+const PowerShellGitHubPreReleasesUrl =
     "https://api.github.com/repos/PowerShell/PowerShell/releases";
 
 export class GitHubReleaseInformation {
@@ -40,7 +41,7 @@ export class GitHubReleaseInformation {
 
         // Fetch the latest PowerShell releases from GitHub.
         const response = await fetch(
-            preview ? PowerShellGitHubPrereleasesUrl : PowerShellGitHubReleasesUrl,
+            preview ? PowerShellGitHubPreReleasesUrl : PowerShellGitHubReleasesUrl,
             requestConfig);
 
         if (!response.ok) {
@@ -85,7 +86,8 @@ export async function InvokePowerShellUpdateCheck(
     languageServerClient: LanguageClient,
     localVersion: semver.SemVer,
     arch: string,
-    release: GitHubReleaseInformation) {
+    release: GitHubReleaseInformation,
+    logger: Logger) {
     const options: IUpdateMessageItem[] = [
         {
             id: 0,
@@ -103,6 +105,7 @@ export async function InvokePowerShellUpdateCheck(
 
     // If our local version is up-to-date, we can return early.
     if (semver.compare(localVersion, release.version) >= 0) {
+        logger.writeDiagnostic("PowerShell is up-to-date!");
         return;
     }
 
@@ -111,8 +114,7 @@ export async function InvokePowerShellUpdateCheck(
     }.`;
 
     if (process.platform === "linux") {
-        await window.showInformationMessage(
-            `${commonText} We recommend updating to the latest version.`);
+        void logger.writeAndShowInformation(`${commonText} We recommend updating to the latest version.`);
         return;
     }
 
@@ -122,7 +124,10 @@ export async function InvokePowerShellUpdateCheck(
         }`, ...options);
 
     // If the user cancels the notification.
-    if (!result) { return; }
+    if (!result) {
+        logger.writeDiagnostic("User canceled PowerShell update prompt.");
+        return;
+    }
 
     // Yes choice.
     switch (result.id) {
@@ -152,6 +157,7 @@ export async function InvokePowerShellUpdateCheck(
             });
 
             // Stop the session because Windows likes to hold on to files.
+            logger.writeDiagnostic("MSI downloaded, stopping session and closing terminals!");
             await sessionManager.stop();
 
             // Close all terminals with the name "pwsh" in the current VS Code session.
@@ -164,10 +170,12 @@ export async function InvokePowerShellUpdateCheck(
             }
 
             // Invoke the MSI via cmd.
+            logger.writeDiagnostic(`Running '${msiDownloadPath}' to update PowerShell...`);
             const msi = spawn("msiexec", ["/i", msiDownloadPath]);
 
             msi.on("close", () => {
                 // Now that the MSI is finished, restart the session.
+                logger.writeDiagnostic("MSI installation finished, restarting session.");
                 void sessionManager.start();
                 fs.unlinkSync(msiDownloadPath);
             });
@@ -177,6 +185,7 @@ export async function InvokePowerShellUpdateCheck(
                 ? "brew upgrade --cask powershell-preview"
                 : "brew upgrade --cask powershell";
 
+            logger.writeDiagnostic(`Running '${script}' to update PowerShell...`);
             await languageServerClient.sendRequest(EvaluateRequestType, {
                 expression: script,
             });
@@ -186,7 +195,7 @@ export async function InvokePowerShellUpdateCheck(
 
         // Never choice.
     case 2:
-        await Settings.change("promptToUpdatePowerShell", false, true);
+        await Settings.change("promptToUpdatePowerShell", false, true, logger);
         break;
     default:
         break;

src/main.ts

@@ -37,6 +37,7 @@ const PackageJSON: any = require("../package.json");
 // the application insights key (also known as instrumentation key) used for telemetry.
 const AI_KEY = "AIF-d9b70cd4-b9f9-4d70-929b-a071c400b217";
 
+let languageConfigurationDisposable: vscode.Disposable;
 let logger: Logger;
 let sessionManager: SessionManager;
 let languageClientConsumers: LanguageClientConsumer[] = [];
@@ -50,21 +51,27 @@ const documentSelector: DocumentSelector = [
 
 // NOTE: Now that this is async, we can probably improve a lot!
 export async function activate(context: vscode.ExtensionContext): Promise<IPowerShellExtensionClient> {
+    // Setup the logger. These settings cannot be passed to subsequent classes
+    // as the `cwd` hasn't been validated!
+    const earlySettings = Settings.load();
+    logger = new Logger(context.globalStorageUri);
+    logger.MinimumLogLevel = Logger.logLevelNameToValue(earlySettings.developer.editorServicesLogLevel);
+
     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.
     // TODO: Merge extensions and use preview channel in marketplace instead.
     if (PackageJSON.name.toLowerCase() === "powershell-preview"
         && vscode.extensions.getExtension("ms-vscode.powershell")) {
-        void vscode.window.showErrorMessage(
+        void logger.writeAndShowError(
             "'PowerShell' and 'PowerShell Preview' are both enabled. Please disable one for best performance.");
     }
 
     // Load and validate settings (will prompt for 'cwd' if necessary).
-    await Settings.validateCwdSetting();
+    await Settings.validateCwdSetting(logger);
     const settings = Settings.load();
 
-    vscode.languages.setLanguageConfiguration(
+    languageConfigurationDisposable = vscode.languages.setLanguageConfiguration(
         PowerShellLanguageId,
         {
             // TODO: Remove the useless escapes
@@ -125,10 +132,6 @@ export async function activate(context: vscode.ExtensionContext): Promise<IPower
             ],
         });
 
-    // Setup the logger.
-    logger = new Logger(context.globalStorageUri);
-    logger.MinimumLogLevel = Logger.logLevelNameToValue(settings.developer.editorServicesLogLevel);
-
     sessionManager = new SessionManager(
         context,
         settings,
@@ -202,4 +205,6 @@ export async function deactivate(): Promise<void> {
 
     // Dispose of telemetry reporter
     await telemetryReporter.dispose();
+
+    languageConfigurationDisposable.dispose();
 }

src/session.ts

@@ -212,7 +212,7 @@ export class SessionManager implements Middleware {
         await this.stop();
 
         // Re-load and validate the settings.
-        await Settings.validateCwdSetting();
+        await Settings.validateCwdSetting(this.logger);
         this.sessionSettings = Settings.load();
 
         await this.start(exeNameOverride);
@@ -348,8 +348,8 @@ export class SessionManager implements Middleware {
         const configurationTargetOfOldSetting = Settings.getEffectiveConfigurationTarget(deprecatedSetting);
         if (configurationTargetOfOldSetting !== undefined && configurationTargetOfNewSetting === undefined) {
             const value = configuration.get(deprecatedSetting, configurationTargetOfOldSetting);
-            await Settings.change(newSetting, value, configurationTargetOfOldSetting);
-            await Settings.change(deprecatedSetting, undefined, configurationTargetOfOldSetting);
+            await Settings.change(newSetting, value, configurationTargetOfOldSetting, this.logger);
+            await Settings.change(deprecatedSetting, undefined, configurationTargetOfOldSetting, this.logger);
         }
     }
 
@@ -373,7 +373,7 @@ export class SessionManager implements Middleware {
 
         this.suppressRestartPrompt = true;
         try {
-            await Settings.change("powerShellExePath", undefined, true);
+            await Settings.change("powerShellExePath", undefined, true, this.logger);
         } finally {
             this.suppressRestartPrompt = false;
         }
@@ -731,7 +731,8 @@ Type 'help' to get help.
                 this.languageClient!,
                 localVersion!,
                 this.versionDetails!.architecture,
-                release);
+                release,
+                this.logger);
         } catch (err) {
             // Best effort. This probably failed to fetch the data from GitHub.
             this.logger.writeWarning(err instanceof Error ? err.message : "unknown");
@@ -804,7 +805,7 @@ Type 'help' to get help.
     private async changePowerShellDefaultVersion(exePath: IPowerShellExeDetails) {
         this.suppressRestartPrompt = true;
         try {
-            await Settings.change("powerShellDefaultVersion", exePath.displayName, true);
+            await Settings.change("powerShellDefaultVersion", exePath.displayName, true, this.logger);
         } finally {
             this.suppressRestartPrompt = false;
         }

src/settings.ts

@@ -4,6 +4,7 @@
 import vscode = require("vscode");
 import utils = require("./utils");
 import os = require("os");
+import { Logger } from "./logging";
 
 enum CodeFormattingPreset {
     Custom,
@@ -288,11 +289,17 @@ export async function change(
     settingName: string,
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
     newValue: any,
-    configurationTarget?: vscode.ConfigurationTarget | boolean): Promise<void> {
+    configurationTarget: vscode.ConfigurationTarget | boolean | undefined,
+    logger: Logger | undefined): Promise<void> {
 
-    const configuration = vscode.workspace.getConfiguration(utils.PowerShellLanguageId);
-    // TODO: Consider wrapping with try/catch, but we can't log the error.
-    await configuration.update(settingName, newValue, configurationTarget);
+    logger?.writeDiagnostic(`Changing '${settingName}' at scope '${configurationTarget} to '${newValue}'`);
+
+    try {
+        const configuration = vscode.workspace.getConfiguration(utils.PowerShellLanguageId);
+        await configuration.update(settingName, newValue, configurationTarget);
+    } catch (err) {
+        logger?.writeError(`Failed to change setting: ${err}`);
+    }
 }
 
 function getWorkspaceSettingsWithDefaults<TSettings>(
@@ -311,7 +318,7 @@ function getWorkspaceSettingsWithDefaults<TSettings>(
 // We don't want to query the user more than once, so this is idempotent.
 let hasPrompted = false;
 
-export async function validateCwdSetting(): Promise<string> {
+export async function validateCwdSetting(logger: Logger): Promise<string> {
     let cwd = vscode.workspace.getConfiguration(utils.PowerShellLanguageId).get<string | undefined>("cwd");
 
     // Only use the cwd setting if it exists.
@@ -336,11 +343,7 @@ export async function validateCwdSetting(): Promise<string> {
         // Save the picked 'cwd' to the workspace settings.
         // We have to check again because the user may not have picked.
         if (cwd !== undefined && await utils.checkIfDirectoryExists(cwd)) {
-            try {
-                await change("cwd", cwd);
-            } catch {
-                // Could fail if workspace file is invalid.
-            }
+            await change("cwd", cwd, undefined, logger);
         }
     }
 

test/core/settings.test.ts

@@ -11,16 +11,16 @@ describe("Settings module", function () {
     });
 
     it("Updates correctly", async function () {
-        await Settings.change("helpCompletion", "LineComment", false);
+        await Settings.change("helpCompletion", "LineComment", false, undefined);
         assert.strictEqual(Settings.load().helpCompletion, "LineComment");
     });
 
     it("Gets the effective configuration target", async function () {
-        await Settings.change("helpCompletion", "LineComment", false);
+        await Settings.change("helpCompletion", "LineComment", false, undefined);
         let target = Settings.getEffectiveConfigurationTarget("helpCompletion");
         assert.strictEqual(target, vscode.ConfigurationTarget.Workspace);
 
-        await Settings.change("helpCompletion", undefined, false);
+        await Settings.change("helpCompletion", undefined, false, undefined);
         target = Settings.getEffectiveConfigurationTarget("helpCompletion");
         assert.strictEqual(target, undefined);
     });