From 899e952fb9aff34699e4f22470394c711a81eb86 Mon Sep 17 00:00:00 2001 From: Nathan Totten Date: Tue, 16 Mar 2021 10:43:17 -0400 Subject: [PATCH] removed notifications --- CHANGELOG.md | 4 + package.json | 2 +- src/ModuleResolver.ts | 107 +++++++++++++++++--------- src/NotificationService.ts | 81 ------------------- src/PrettierEditService.ts | 12 +-- src/extension.ts | 8 +- src/message.ts | 2 - src/test/suite/ModuleResolver.test.ts | 9 +-- src/test/suite/index.ts | 6 +- src/test/suite/notifications.test.ts | 64 --------------- 10 files changed, 83 insertions(+), 212 deletions(-) delete mode 100644 src/NotificationService.ts delete mode 100644 src/test/suite/notifications.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index d541a72cf..00b060c78 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ All notable changes to the "prettier-vscode" extension will be documented in thi +## [6.3.0] + +- Removed notifications, all messages logged with status icon update. + ## [6.2.1] - Fixed regressions where VS Code settings `settings.json` could not be formatted diff --git a/package.json b/package.json index df22ab8b7..28d1744fe 100644 --- a/package.json +++ b/package.json @@ -2,7 +2,7 @@ "name": "prettier-vscode", "displayName": "Prettier - Code formatter", "description": "Code formatter using prettier", - "version": "6.2.1", + "version": "6.3.0", "publisher": "esbenp", "author": "Prettier <@prettiercode>", "galleryBanner": { diff --git a/src/ModuleResolver.ts b/src/ModuleResolver.ts index 9612b9a63..5ea7e0330 100644 --- a/src/ModuleResolver.ts +++ b/src/ModuleResolver.ts @@ -5,19 +5,22 @@ import * as path from "path"; import * as prettier from "prettier"; import * as resolve from "resolve"; import * as semver from "semver"; -import { commands, Disposable, Uri } from "vscode"; +import { + commands, + Disposable, + MessageItem, + Uri, + window, + workspace, +} from "vscode"; import { resolveGlobalNodePath, resolveGlobalYarnPath } from "./Files"; import { LoggingService } from "./LoggingService"; import { FAILED_TO_LOAD_MODULE_MESSAGE, INVALID_PRETTIER_PATH_MESSAGE, - OUTDATED_PRETTIER_INSTALLED, + OUTDATED_PRETTIER_VERSION_MESSAGE, USING_BUNDLED_PRETTIER, } from "./message"; -import { - ConfirmationSelection, - NotificationService, -} from "./NotificationService"; import { getFromGlobalState, getFromWorkspaceState, @@ -34,6 +37,16 @@ declare const __non_webpack_require__: typeof require; const alwaysAllowedExecutionStateKey = "PRETTIER_MODULE_ALWAYS_ALLOWED"; const moduleExecutionStateKey = "moduleExecutionState"; +export enum ConfirmationSelection { + deny = 1, + allow = 2, + alwaysAllow = 3, +} + +export interface ConfirmMessageItem extends MessageItem { + value: ConfirmationSelection; +} + interface PrettierExecutionState { libs: { [key: string]: boolean }; } @@ -73,14 +86,46 @@ function globalPathGet(packageManager: PackageManagers): string | undefined { return undefined; } +async function askForModuleApproval( + modulePath: string, + isGlobal: boolean +): Promise { + const libraryUri = Uri.file(modulePath); + const folder = workspace.getWorkspaceFolder(libraryUri); + let message: string; + if (folder !== undefined) { + const relativePath = workspace.asRelativePath(libraryUri); + message = `The Prettier extension will use '${relativePath}' for validation, which is installed locally in folder '${folder.name}'. Do you allow the execution of this Prettier version including all plugins and configuration files it will load on your behalf?\n\nPress 'Allow Everywhere' to remember the choice for all workspaces.`; + } else { + message = isGlobal + ? `The Prettier extension will use a globally installed Prettier library for validation. Do you allow the execution of this Prettier version including all plugins and configuration files it will load on your behalf?\n\nPress 'Always Allow' to remember the choice for all workspaces.` + : `The Prettier extension will use a locally installed Prettier library for validation. Do you allow the execution of this Prettier version including all plugins and configuration files it will load on your behalf?\n\nPress 'Always Allow' to remember the choice for all workspaces.`; + } + + const messageItems: ConfirmMessageItem[] = [ + { title: "Allow Everywhere", value: ConfirmationSelection.alwaysAllow }, + { title: "Allow", value: ConfirmationSelection.allow }, + { title: "Deny", value: ConfirmationSelection.deny }, + ]; + const item = await window.showInformationMessage( + message, + { modal: true }, + ...messageItems + ); + + // Dialog got canceled. + if (item === undefined) { + return ConfirmationSelection.deny; + } else { + return item.value; + } +} + export class ModuleResolver implements Disposable { private path2Module = new Map(); private deniedModules = new Set(); - constructor( - private loggingService: LoggingService, - private notificationService: NotificationService - ) {} + constructor(private loggingService: LoggingService) {} /** * Returns an instance of the prettier module. @@ -117,13 +162,12 @@ export class ModuleResolver implements Disposable { } } - this.loggingService.logError(`Failed to local Prettier module.`, error); - - this.notificationService.showErrorMessage(FAILED_TO_LOAD_MODULE_MESSAGE, [ + this.loggingService.logInfo( `Attempted to load Prettier module from ${ modulePath || moduleDirectory || "package.json" - }`, - ]); + }` + ); + this.loggingService.logError(FAILED_TO_LOAD_MODULE_MESSAGE, error); // Return here because there is a local module, but we can't resolve it. // Must do NPM install for prettier to work. @@ -170,19 +214,12 @@ export class ModuleResolver implements Disposable { return undefined; } } catch (error) { - this.loggingService.logError( - `Failed to load Prettier module.`, - error - ); - - this.notificationService.showErrorMessage( - FAILED_TO_LOAD_MODULE_MESSAGE, - [ - `Attempted to load Prettier module from ${ - modulePath || "package.json" - }`, - ] + this.loggingService.logInfo( + `Attempted to load Prettier module from ${ + modulePath || "package.json" + }` ); + this.loggingService.logError(FAILED_TO_LOAD_MODULE_MESSAGE, error); // Returning here because module didn't load. return undefined; @@ -207,17 +244,14 @@ export class ModuleResolver implements Disposable { if (!isPrettierInstance && prettierPath) { this.loggingService.logError(INVALID_PRETTIER_PATH_MESSAGE); - this.notificationService.showErrorMessage( - INVALID_PRETTIER_PATH_MESSAGE - ); return undefined; } if (!isValidVersion) { - // We only prompt when formatting a file. If we did it on load there - // could be lots of these notifications which would be annoying. - this.notificationService.warnOutdatedPrettierVersion(modulePath); - this.loggingService.logError(OUTDATED_PRETTIER_INSTALLED); + this.loggingService.logInfo( + `Attempted to load Prettier module from ${modulePath}` + ); + this.loggingService.logError(OUTDATED_PRETTIER_VERSION_MESSAGE); return undefined; } } @@ -269,10 +303,7 @@ export class ModuleResolver implements Disposable { let isTrustedModule = moduleState.libs[modulePath]; if (!isTrustedModule) { - const approvalResult = await this.notificationService.askForModuleApproval( - modulePath, - isGlobal - ); + const approvalResult = await askForModuleApproval(modulePath, isGlobal); if (approvalResult === ConfirmationSelection.alwaysAllow) { isTrustedModule = true; diff --git a/src/NotificationService.ts b/src/NotificationService.ts deleted file mode 100644 index 32aa0e9e2..000000000 --- a/src/NotificationService.ts +++ /dev/null @@ -1,81 +0,0 @@ -import { MessageItem, Uri, window, workspace } from "vscode"; -import { LoggingService } from "./LoggingService"; -import { - OUTDATED_PRETTIER_VERSION_MESSAGE, - VIEW_LOGS_ACTION_TEXT, -} from "./message"; - -export enum ConfirmationSelection { - deny = 1, - allow = 2, - alwaysAllow = 3, -} - -export interface ConfirmMessageItem extends MessageItem { - value: ConfirmationSelection; -} - -export class NotificationService { - constructor(private loggingService: LoggingService) {} - - public warnOutdatedPrettierVersion(prettierPath?: string) { - window.showErrorMessage( - OUTDATED_PRETTIER_VERSION_MESSAGE.replace( - "{{path}}", - prettierPath || "unknown" - ) - ); - } - - public async askForModuleApproval( - modulePath: string, - isGlobal: boolean - ): Promise { - const libraryUri = Uri.file(modulePath); - const folder = workspace.getWorkspaceFolder(libraryUri); - let message: string; - if (folder !== undefined) { - const relativePath = workspace.asRelativePath(libraryUri); - message = `The Prettier extension will use '${relativePath}' for validation, which is installed locally in folder '${folder.name}'. Do you allow the execution of this Prettier version including all plugins and configuration files it will load on your behalf?\n\nPress 'Allow Everywhere' to remember the choice for all workspaces.`; - } else { - message = isGlobal - ? `The Prettier extension will use a globally installed Prettier library for validation. Do you allow the execution of this Prettier version including all plugins and configuration files it will load on your behalf?\n\nPress 'Always Allow' to remember the choice for all workspaces.` - : `The Prettier extension will use a locally installed Prettier library for validation. Do you allow the execution of this Prettier version including all plugins and configuration files it will load on your behalf?\n\nPress 'Always Allow' to remember the choice for all workspaces.`; - } - - const messageItems: ConfirmMessageItem[] = [ - { title: "Allow Everywhere", value: ConfirmationSelection.alwaysAllow }, - { title: "Allow", value: ConfirmationSelection.allow }, - { title: "Deny", value: ConfirmationSelection.deny }, - ]; - const item = await window.showInformationMessage( - message, - { modal: true }, - ...messageItems - ); - - // Dialog got canceled. - if (item === undefined) { - return ConfirmationSelection.deny; - } else { - return item.value; - } - } - - public async showErrorMessage(message: string, extraLines?: string[]) { - let result: string | undefined; - if (extraLines) { - const lines = [message]; - lines.push(...extraLines); - result = await window.showErrorMessage( - lines.join(" "), - VIEW_LOGS_ACTION_TEXT - ); - } else { - result = await window.showErrorMessage(message, VIEW_LOGS_ACTION_TEXT); - } - if (result && result === VIEW_LOGS_ACTION_TEXT) { - this.loggingService.show(); - } - } -} diff --git a/src/PrettierEditService.ts b/src/PrettierEditService.ts index a60e06e6a..dcbd537b1 100644 --- a/src/PrettierEditService.ts +++ b/src/PrettierEditService.ts @@ -18,13 +18,8 @@ import { getParserFromLanguageId, } from "./languageFilters"; import { LoggingService } from "./LoggingService"; -import { - INVALID_PRETTIER_CONFIG, - RESTART_TO_ENABLE, - UNABLE_TO_LOAD_PRETTIER, -} from "./message"; +import { INVALID_PRETTIER_CONFIG, RESTART_TO_ENABLE } from "./message"; import { ModuleResolver } from "./ModuleResolver"; -import { NotificationService } from "./NotificationService"; import { PrettierEditProvider } from "./PrettierEditProvider"; import { FormatterStatus, StatusBar } from "./StatusBar"; import { @@ -65,7 +60,6 @@ export default class PrettierEditService implements Disposable { constructor( private moduleResolver: ModuleResolver, private loggingService: LoggingService, - private notificationService: NotificationService, private statusBar: StatusBar ) {} @@ -182,7 +176,6 @@ export default class PrettierEditService implements Disposable { this.loggingService.logError( "The Prettier extension is blocked from execution in this project." ); - //this.notificationService.showErrorMessage(INVALID_PRETTIER_CONFIG); this.statusBar.update(FormatterStatus.Disabled); this.registeredWorkspaces.add(workspaceFolder.uri.fsPath); return; @@ -390,7 +383,7 @@ export default class PrettierEditService implements Disposable { "Invalid prettier configuration file detected.", error ); - this.notificationService.showErrorMessage(INVALID_PRETTIER_CONFIG); + this.loggingService.logError(INVALID_PRETTIER_CONFIG); this.statusBar.update(FormatterStatus.Error); return; } @@ -430,7 +423,6 @@ export default class PrettierEditService implements Disposable { this.loggingService.logError( "Prettier could not be loaded. See previous logs for more information." ); - this.notificationService.showErrorMessage(UNABLE_TO_LOAD_PRETTIER); this.statusBar.update(FormatterStatus.Error); return; } diff --git a/src/extension.ts b/src/extension.ts index 2a818abca..980776de3 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -2,7 +2,6 @@ import { commands, ExtensionContext, workspace } from "vscode"; import { createConfigFile } from "./commands"; import { LoggingService } from "./LoggingService"; import { ModuleResolver } from "./ModuleResolver"; -import { NotificationService } from "./NotificationService"; import PrettierEditService from "./PrettierEditService"; import { StatusBar } from "./StatusBar"; import { TemplateService } from "./TemplateService"; @@ -42,19 +41,14 @@ export function activate(context: ExtensionContext) { setWorkspaceState(context.workspaceState); const templateService = new TemplateService(loggingService); - const notificationService = new NotificationService(loggingService); - const moduleResolver = new ModuleResolver( - loggingService, - notificationService - ); + const moduleResolver = new ModuleResolver(loggingService); const statusBar = new StatusBar(); const editService = new PrettierEditService( moduleResolver, loggingService, - notificationService, statusBar ); editService.registerGlobal(); diff --git a/src/message.ts b/src/message.ts index 5dc84d078..82644e0ab 100644 --- a/src/message.ts +++ b/src/message.ts @@ -12,7 +12,5 @@ export const UNABLE_TO_LOAD_PRETTIER = export const RESTART_TO_ENABLE = "To enable or disable prettier after changing the `enable` setting, you must restart VS Code."; export const USING_BUNDLED_PRETTIER = "Using bundled version of prettier."; -export const OUTDATED_PRETTIER_INSTALLED = - "Outdated version of prettier installed. You must upgrade in order to use the Prettier extension."; export const EXTENSION_DISABLED = "Extension is disabled. No formatters will be registered. To enable, change the `prettier.enable` to `true` and restart VS Code."; diff --git a/src/test/suite/ModuleResolver.test.ts b/src/test/suite/ModuleResolver.test.ts index db0e3ac0b..827fa2804 100644 --- a/src/test/suite/ModuleResolver.test.ts +++ b/src/test/suite/ModuleResolver.test.ts @@ -6,9 +6,8 @@ import * as sinon from "sinon"; import { getWorkspaceFolderUri } from "./format.test"; import { ModuleResolver } from "../../ModuleResolver"; import { LoggingService } from "../../LoggingService"; -import { NotificationService } from "../../NotificationService"; import { - OUTDATED_PRETTIER_INSTALLED, + OUTDATED_PRETTIER_VERSION_MESSAGE, USING_BUNDLED_PRETTIER, } from "../../message"; @@ -21,9 +20,7 @@ suite("Test ModuleResolver", function () { const loggingService = new LoggingService(); logErrorSpy = sinon.spy(loggingService, "logError"); logDebugSpy = sinon.spy(loggingService, "logDebug"); - const notificationService = new NotificationService(loggingService); - - moduleResolver = new ModuleResolver(loggingService, notificationService); + moduleResolver = new ModuleResolver(loggingService); }); suite("getPrettierInstance", () => { @@ -50,7 +47,7 @@ suite("Test ModuleResolver", function () { ); assert.strictEqual(prettierInstance, undefined); - assert(logErrorSpy.calledWith(OUTDATED_PRETTIER_INSTALLED)); + assert(logErrorSpy.calledWith(OUTDATED_PRETTIER_VERSION_MESSAGE)); }); test("it returns prettier version from package.json", async () => { diff --git a/src/test/suite/index.ts b/src/test/suite/index.ts index 231a1387c..07eddca83 100644 --- a/src/test/suite/index.ts +++ b/src/test/suite/index.ts @@ -1,12 +1,12 @@ import * as glob from "glob"; import * as Mocha from "mocha"; import * as path from "path"; +import { MessageItem, MessageOptions, window } from "vscode"; +import * as sinon from "sinon"; import { ConfirmationSelection, ConfirmMessageItem, -} from "../../NotificationService"; -import { MessageItem, MessageOptions, window } from "vscode"; -import * as sinon from "sinon"; +} from "../../ModuleResolver"; const showInformationMessage: sinon.SinonStub< [string, MessageOptions, ...MessageItem[]], diff --git a/src/test/suite/notifications.test.ts b/src/test/suite/notifications.test.ts deleted file mode 100644 index b3034e7d5..000000000 --- a/src/test/suite/notifications.test.ts +++ /dev/null @@ -1,64 +0,0 @@ -import * as path from "path"; -import * as assert from "assert"; -import * as sinon from "sinon"; -import { MessageItem, MessageOptions, window, workspace } from "vscode"; -import { - OUTDATED_PRETTIER_VERSION_MESSAGE, - INVALID_PRETTIER_PATH_MESSAGE, -} from "../../message"; -import { format } from "./format.test"; - -function isWindows(): boolean { - return process.platform === "win32"; -} - -suite("Test notifications", function () { - let showWarningMessage: sinon.SinonStub< - [string, MessageOptions, ...MessageItem[]], - Thenable - >; - let showErrorMessage: sinon.SinonStub< - [string, MessageOptions, ...MessageItem[]], - Thenable - >; - - this.timeout(10000); - this.beforeEach(() => { - showWarningMessage = sinon.stub(window, "showWarningMessage"); - showErrorMessage = sinon.stub(window, "showErrorMessage"); - }); - this.afterEach(() => { - showWarningMessage.restore(); - showErrorMessage.restore(); - }); - - test("shows error for outdated prettier instance", async () => { - await format("outdated", "ugly.js"); - assert(showErrorMessage.calledWith(OUTDATED_PRETTIER_VERSION_MESSAGE)); - }); - - test("shows error for invalid prettier instance", async () => { - const settings = workspace.getConfiguration("prettier"); - const originalPrettierPath = settings.get("prettierPath"); - if (isWindows()) { - // On windows the file in the bin won't be a JS file, so test against - // an arbitrary non-prettier file instead - await settings.update("prettierPath", path.join(".", "index.js")); - } else { - await settings.update( - "prettierPath", - path.join(".", "node_modules", ".bin", "prettier") - ); - } - await format("explicit-dep", "index.js"); - - assert(showErrorMessage.calledWith(INVALID_PRETTIER_PATH_MESSAGE)); - - await settings.update("prettierPath", originalPrettierPath); - }); - - test("does not show error with valid project", async () => { - await format("plugins", "index.php"); - assert(showWarningMessage.notCalled); - }); -});