From 60c7478046f6eb46bd52f82e5399b650c0f8946a Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Mon, 6 Jan 2020 09:03:42 +0100 Subject: [PATCH 01/36] first cut --- .../userDataSync/common/localizationSync.ts | 160 ++++++++++++++++++ 1 file changed, 160 insertions(+) create mode 100644 src/vs/platform/userDataSync/common/localizationSync.ts diff --git a/src/vs/platform/userDataSync/common/localizationSync.ts b/src/vs/platform/userDataSync/common/localizationSync.ts new file mode 100644 index 00000000000..724eac4dee5 --- /dev/null +++ b/src/vs/platform/userDataSync/common/localizationSync.ts @@ -0,0 +1,160 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { Disposable } from 'vs/base/common/lifecycle'; +import { IUserData, UserDataSyncStoreError, UserDataSyncStoreErrorCode, ISynchroniser, SyncStatus, IUserDataSyncStoreService, ISyncExtension, IUserDataSyncLogService } from 'vs/platform/userDataSync/common/userDataSync'; +import { VSBuffer } from 'vs/base/common/buffer'; +import { Emitter, Event } from 'vs/base/common/event'; +import { IEnvironmentService } from 'vs/platform/environment/common/environment'; +import { URI } from 'vs/base/common/uri'; +import { joinPath } from 'vs/base/common/resources'; +import { IExtensionManagementService, IExtensionGalleryService } from 'vs/platform/extensionManagement/common/extensionManagement'; +import { ExtensionType, IExtensionIdentifier } from 'vs/platform/extensions/common/extensions'; +import { areSameExtensions } from 'vs/platform/extensionManagement/common/extensionManagementUtil'; +import { IFileService } from 'vs/platform/files/common/files'; +import { Queue } from 'vs/base/common/async'; +import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; +import { localize } from 'vs/nls'; +import { merge } from 'vs/platform/userDataSync/common/extensionsMerge'; + +export class LocalizationSynchroniser extends Disposable implements ISynchroniser { + + private static EXTERNAL_USER_DATA_LOCALIZATION_KEY: string = 'localization'; + + private _status: SyncStatus = SyncStatus.Idle; + get status(): SyncStatus { return this._status; } + private _onDidChangStatus: Emitter = this._register(new Emitter()); + readonly onDidChangeStatus: Event = this._onDidChangStatus.event; + + private _onDidChangeLocal: Emitter = this._register(new Emitter()); + readonly onDidChangeLocal: Event = this._onDidChangeLocal.event; + + private readonly lastSyncExtensionsResource: URI; + + constructor( + @IEnvironmentService environmentService: IEnvironmentService, + @IFileService private readonly fileService: IFileService, + @IUserDataSyncStoreService private readonly userDataSyncStoreService: IUserDataSyncStoreService, + @IUserDataSyncLogService private readonly logService: IUserDataSyncLogService, + @IConfigurationService private readonly configurationService: IConfigurationService, + ) { + super(); + this.lastSyncExtensionsResource = joinPath(environmentService.userRoamingDataHome, '.lastSyncLocalization'); + this._register( + Event.debounce( + Event.any( + Event.filter(this.extensionManagementService.onDidInstallExtension, (e => !!e.gallery)), + Event.filter(this.extensionManagementService.onDidUninstallExtension, (e => !e.error))), + () => undefined, 500)(() => this._onDidChangeLocal.fire())); + } + + private setStatus(status: SyncStatus): void { + if (this._status !== status) { + this._status = status; + this._onDidChangStatus.fire(status); + } + } + + async sync(): Promise { + if (!this.configurationService.getValue('sync.enableLocale')) { + this.logService.trace('Extensions: Skipping synchronizing locale as it is disabled.'); + return false; + } + if (this.status !== SyncStatus.Idle) { + this.logService.trace('Extensions: Skipping synchronizing locale as it is running already.'); + return false; + } + + this.logService.trace('Extensions: Started synchronizing locale...'); + this.setStatus(SyncStatus.Syncing); + + try { + await this.doSync(); + } catch (e) { + this.setStatus(SyncStatus.Idle); + if (e instanceof UserDataSyncStoreError && e.code === UserDataSyncStoreErrorCode.Rejected) { + // Rejected as there is a new remote version. Syncing again, + this.logService.info('Extensions: Failed to synchronise extensions as there is a new remote version available. Synchronizing again...'); + return this.sync(); + } + throw e; + } + + this.logService.trace('Extensions: Finised synchronizing extensions.'); + this.setStatus(SyncStatus.Idle); + return true; + } + + stop(): void { } + + private async doSync(): Promise { + const lastSyncData = await this.getLastSyncUserData(); + const lastSyncExtensions: ISyncExtension[] | null = lastSyncData ? JSON.parse(lastSyncData.content!) : null; + let skippedExtensions: ISyncExtension[] = lastSyncData ? lastSyncData.skippedExtensions || [] : []; + + let remoteData = await this.userDataSyncStoreService.read(ExtensionsSynchroniser.EXTERNAL_USER_DATA_LOCALIZATION_KEY, lastSyncData); + const remoteExtensions: ISyncExtension[] = remoteData.content ? JSON.parse(remoteData.content) : null; + + const localExtensions = await this.getLocalExtensions(); + + if (remoteExtensions) { + this.logService.trace('Extensions: Merging remote extensions with local extensions...'); + } else { + this.logService.info('Extensions: Remote extensions does not exist. Synchronizing extensions for the first time.'); + } + + const ignoredExtensions = this.configurationService.getValue('sync.ignoredExtensions') || []; + const { added, removed, updated, remote } = merge(localExtensions, remoteExtensions, lastSyncExtensions, skippedExtensions, ignoredExtensions); + + if (!added.length && !removed.length && !updated.length && !remote) { + this.logService.trace('Extensions: No changes found during synchronizing extensions.'); + } + + if (added.length || removed.length || updated.length) { + this.logService.info('Extensions: Updating local extensions...'); + skippedExtensions = await this.updateLocalExtensions(added, removed, updated, skippedExtensions); + } + + if (remote) { + // update remote + this.logService.info('Extensions: Updating remote extensions...'); + remoteData = await this.writeToRemote(remote, remoteData.ref); + } + + if (remoteData.content + && (!lastSyncData || lastSyncData.ref !== remoteData.ref) + ) { + // update last sync + this.logService.info('Extensions: Updating last synchronised extensions...'); + await this.updateLastSyncValue({ ...remoteData, skippedExtensions }); + } + } + + private async getLocalExtensions(): Promise { + const installedExtensions = await this.extensionManagementService.getInstalled(ExtensionType.User); + return installedExtensions + .map(({ identifier }) => ({ identifier, enabled: true })); + } + + private async getLastSyncUserData(): Promise { + try { + const content = await this.fileService.readFile(this.lastSyncExtensionsResource); + return JSON.parse(content.value.toString()); + } catch (error) { + return null; + } + } + + private async updateLastSyncValue(lastSyncUserData: ILastSyncUserData): Promise { + await this.fileService.writeFile(this.lastSyncExtensionsResource, VSBuffer.fromString(JSON.stringify(lastSyncUserData))); + } + + private async writeToRemote(extensions: ISyncExtension[], ref: string | null): Promise { + const content = JSON.stringify(extensions); + ref = await this.userDataSyncStoreService.write(ExtensionsSynchroniser.EXTERNAL_USER_DATA_LOCALIZATION_KEY, content, ref); + return { content, ref }; + } + +} From c3b939bfeaa1b84ff7868090c2561ec23125a355 Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Mon, 6 Jan 2020 16:03:40 +0100 Subject: [PATCH 02/36] Sync locale using global state --- .../userDataSync/common/globalStateMerge.ts | 86 ++++++++++ .../userDataSync/common/globalStateSync.ts | 157 +++++++++++++++++ .../userDataSync/common/localizationSync.ts | 160 ------------------ .../userDataSync/common/userDataSync.ts | 5 + 4 files changed, 248 insertions(+), 160 deletions(-) create mode 100644 src/vs/platform/userDataSync/common/globalStateMerge.ts create mode 100644 src/vs/platform/userDataSync/common/globalStateSync.ts delete mode 100644 src/vs/platform/userDataSync/common/localizationSync.ts diff --git a/src/vs/platform/userDataSync/common/globalStateMerge.ts b/src/vs/platform/userDataSync/common/globalStateMerge.ts new file mode 100644 index 00000000000..f4520a76a38 --- /dev/null +++ b/src/vs/platform/userDataSync/common/globalStateMerge.ts @@ -0,0 +1,86 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as objects from 'vs/base/common/objects'; +import { IGlobalState } from 'vs/platform/userDataSync/common/userDataSync'; +import { IStringDictionary } from 'vs/base/common/collections'; +import { values } from 'vs/base/common/map'; + +export function merge(localGloablState: IGlobalState, remoteGlobalState: IGlobalState | null, lastSyncGlobalState: IGlobalState | null): { local?: IGlobalState, remote?: IGlobalState } { + if (!remoteGlobalState) { + return { remote: localGloablState }; + } + + const { local: localArgv, remote: remoteArgv } = doMerge(localGloablState.argv, remoteGlobalState.argv, lastSyncGlobalState ? lastSyncGlobalState.argv : null); + const { local: localStorage, remote: remoteStorage } = doMerge(localGloablState.storage, remoteGlobalState.storage, lastSyncGlobalState ? lastSyncGlobalState.storage : null); + const local: IGlobalState | undefined = localArgv || localStorage ? { argv: localArgv || localGloablState.argv, storage: localStorage || localGloablState.storage } : undefined; + const remote: IGlobalState | undefined = remoteArgv || remoteStorage ? { argv: remoteArgv || remoteGlobalState.argv, storage: remoteStorage || remoteGlobalState.storage } : undefined; + + return { local, remote }; +} + +function doMerge(local: IStringDictionary, remote: IStringDictionary, base: IStringDictionary | null): { local?: IStringDictionary, remote?: IStringDictionary } { + const localToRemote = compare(local, remote); + if (localToRemote.added.size === 0 && localToRemote.removed.size === 0 && localToRemote.updated.size === 0) { + // No changes found between local and remote. + return {}; + } + + const baseToRemote = base ? compare(base, remote) : { added: Object.keys(remote).reduce((r, k) => { r.add(k); return r; }, new Set()), removed: new Set(), updated: new Set() }; + if (baseToRemote.added.size === 0 && baseToRemote.removed.size === 0 && baseToRemote.updated.size === 0) { + // No changes found between base and remote. + return { remote: local }; + } + + const baseToLocal = base ? compare(base, local) : { added: Object.keys(local).reduce((r, k) => { r.add(k); return r; }, new Set()), removed: new Set(), updated: new Set() }; + if (baseToLocal.added.size === 0 && baseToLocal.removed.size === 0 && baseToLocal.updated.size === 0) { + // No changes found between base and local. + return { local: remote }; + } + + const merged = objects.deepClone(local); + + // Added in remote + for (const key of values(baseToRemote.added)) { + merged[key] = remote[key]; + } + + // Updated in Remote + for (const key of values(baseToRemote.updated)) { + merged[key] = remote[key]; + } + + // Removed in remote & local + for (const key of values(baseToRemote.removed)) { + // Got removed in local + if (baseToLocal.removed.has(key)) { + delete merged[key]; + } + } + + return { local: merged, remote: merged }; +} + +function compare(from: IStringDictionary, to: IStringDictionary): { added: Set, removed: Set, updated: Set } { + const fromKeys = Object.keys(from); + const toKeys = Object.keys(to); + const added = toKeys.filter(key => fromKeys.indexOf(key) === -1).reduce((r, key) => { r.add(key); return r; }, new Set()); + const removed = fromKeys.filter(key => toKeys.indexOf(key) === -1).reduce((r, key) => { r.add(key); return r; }, new Set()); + const updated: Set = new Set(); + + for (const key of fromKeys) { + if (removed.has(key)) { + continue; + } + const value1 = from[key]; + const value2 = to[key]; + if (!objects.equals(value1, value2)) { + updated.add(key); + } + } + + return { added, removed, updated }; +} + diff --git a/src/vs/platform/userDataSync/common/globalStateSync.ts b/src/vs/platform/userDataSync/common/globalStateSync.ts new file mode 100644 index 00000000000..d5e218a964b --- /dev/null +++ b/src/vs/platform/userDataSync/common/globalStateSync.ts @@ -0,0 +1,157 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { Disposable } from 'vs/base/common/lifecycle'; +import { IUserData, UserDataSyncStoreError, UserDataSyncStoreErrorCode, ISynchroniser, SyncStatus, IUserDataSyncStoreService, IUserDataSyncLogService, IGlobalState } from 'vs/platform/userDataSync/common/userDataSync'; +import { VSBuffer } from 'vs/base/common/buffer'; +import { Emitter, Event } from 'vs/base/common/event'; +import { IEnvironmentService } from 'vs/platform/environment/common/environment'; +import { URI } from 'vs/base/common/uri'; +import { joinPath, dirname } from 'vs/base/common/resources'; +import { IFileService } from 'vs/platform/files/common/files'; +import { IStringDictionary } from 'vs/base/common/collections'; +import { edit } from 'vs/platform/userDataSync/common/content'; +import { merge } from 'vs/platform/userDataSync/common/globalStateMerge'; + +const argvProperties: string[] = ['locale']; + +export class GlobalStateSynchroniser extends Disposable implements ISynchroniser { + + private static EXTERNAL_USER_DATA_GLOBAL_STATE_KEY: string = 'globalState'; + + private _status: SyncStatus = SyncStatus.Idle; + get status(): SyncStatus { return this._status; } + private _onDidChangStatus: Emitter = this._register(new Emitter()); + readonly onDidChangeStatus: Event = this._onDidChangStatus.event; + + private _onDidChangeLocal: Emitter = this._register(new Emitter()); + readonly onDidChangeLocal: Event = this._onDidChangeLocal.event; + + private readonly lastSyncGlobalStateResource: URI; + + constructor( + @IFileService private readonly fileService: IFileService, + @IUserDataSyncStoreService private readonly userDataSyncStoreService: IUserDataSyncStoreService, + @IUserDataSyncLogService private readonly logService: IUserDataSyncLogService, + @IEnvironmentService private readonly environmentService: IEnvironmentService, + ) { + super(); + this.lastSyncGlobalStateResource = joinPath(environmentService.userRoamingDataHome, '.lastSyncGlobalState'); + this._register(this.fileService.watch(dirname(this.environmentService.argvResource))); + this._register(Event.filter(this.fileService.onFileChanges, e => e.contains(this.environmentService.argvResource))(() => this._onDidChangeLocal.fire())); + } + + private setStatus(status: SyncStatus): void { + if (this._status !== status) { + this._status = status; + this._onDidChangStatus.fire(status); + } + } + + async sync(): Promise { + if (this.status !== SyncStatus.Idle) { + this.logService.trace('Global State: Skipping synchronizing global state as it is running already.'); + return false; + } + + this.logService.trace('Global State: Started synchronizing global state...'); + this.setStatus(SyncStatus.Syncing); + + try { + await this.doSync(); + this.logService.trace('Global State: Finised synchronizing global state.'); + this.setStatus(SyncStatus.Idle); + return true; + } catch (e) { + this.setStatus(SyncStatus.Idle); + if (e instanceof UserDataSyncStoreError && e.code === UserDataSyncStoreErrorCode.Rejected) { + // Rejected as there is a new remote version. Syncing again, + this.logService.info('Global State: Failed to synchronise global state as there is a new remote version available. Synchronizing again...'); + return this.sync(); + } + throw e; + } + } + + stop(): void { } + + private async doSync(): Promise { + const lastSyncData = await this.getLastSyncUserData(); + const lastSyncGlobalState = lastSyncData && lastSyncData.content ? JSON.parse(lastSyncData.content) : null; + + let remoteData = await this.userDataSyncStoreService.read(GlobalStateSynchroniser.EXTERNAL_USER_DATA_GLOBAL_STATE_KEY, lastSyncData); + const remoteGlobalState: IGlobalState = remoteData.content ? JSON.parse(remoteData.content) : null; + + const localGloablState = await this.getLocalGlobalState(); + + const { local, remote } = merge(localGloablState, remoteGlobalState, lastSyncGlobalState); + + if (local) { + // update local + this.logService.info('Global State: Updating local global state...'); + await this.writeLocalGlobalState(local); + } + + if (remote) { + // update remote + this.logService.info('Global State: Updating remote global state...'); + remoteData = await this.writeToRemote(remote, remoteData.ref); + } + + if (remoteData.content + && (!lastSyncData || lastSyncData.ref !== remoteData.ref) + ) { + // update last sync + this.logService.info('Global State: Updating last synchronised global state...'); + await this.updateLastSyncValue(remoteData); + } + } + + private async getLocalGlobalState(): Promise { + const argv: IStringDictionary = {}; + const storage: IStringDictionary = {}; + try { + const content = await this.fileService.readFile(this.environmentService.argvResource); + const argvValue: IStringDictionary = JSON.parse(content.value.toString()); + for (const argvProperty of argvProperties) { + if (argvValue[argvProperty] !== undefined) { + argv[argvProperty] = argvValue[argvProperty]; + } + } + } catch (error) { } + return { argv, storage }; + } + + private async writeLocalGlobalState(globalState: IGlobalState): Promise { + const content = await this.fileService.readFile(this.environmentService.argvResource); + let argvContent = content.value.toString(); + for (const argvProperty of Object.keys(globalState.argv)) { + argvContent = edit(argvContent, [argvProperty], globalState.argv[argvProperty], {}); + } + if (argvContent !== content.value.toString()) { + await this.fileService.writeFile(this.environmentService.argvResource, VSBuffer.fromString(argvContent)); + } + } + + private async getLastSyncUserData(): Promise { + try { + const content = await this.fileService.readFile(this.lastSyncGlobalStateResource); + return JSON.parse(content.value.toString()); + } catch (error) { + return null; + } + } + + private async updateLastSyncValue(remoteUserData: IUserData): Promise { + await this.fileService.writeFile(this.lastSyncGlobalStateResource, VSBuffer.fromString(JSON.stringify(remoteUserData))); + } + + private async writeToRemote(globalState: IGlobalState, ref: string | null): Promise { + const content = JSON.stringify(globalState); + ref = await this.userDataSyncStoreService.write(GlobalStateSynchroniser.EXTERNAL_USER_DATA_GLOBAL_STATE_KEY, content, ref); + return { content, ref }; + } + +} diff --git a/src/vs/platform/userDataSync/common/localizationSync.ts b/src/vs/platform/userDataSync/common/localizationSync.ts deleted file mode 100644 index 724eac4dee5..00000000000 --- a/src/vs/platform/userDataSync/common/localizationSync.ts +++ /dev/null @@ -1,160 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See License.txt in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -import { Disposable } from 'vs/base/common/lifecycle'; -import { IUserData, UserDataSyncStoreError, UserDataSyncStoreErrorCode, ISynchroniser, SyncStatus, IUserDataSyncStoreService, ISyncExtension, IUserDataSyncLogService } from 'vs/platform/userDataSync/common/userDataSync'; -import { VSBuffer } from 'vs/base/common/buffer'; -import { Emitter, Event } from 'vs/base/common/event'; -import { IEnvironmentService } from 'vs/platform/environment/common/environment'; -import { URI } from 'vs/base/common/uri'; -import { joinPath } from 'vs/base/common/resources'; -import { IExtensionManagementService, IExtensionGalleryService } from 'vs/platform/extensionManagement/common/extensionManagement'; -import { ExtensionType, IExtensionIdentifier } from 'vs/platform/extensions/common/extensions'; -import { areSameExtensions } from 'vs/platform/extensionManagement/common/extensionManagementUtil'; -import { IFileService } from 'vs/platform/files/common/files'; -import { Queue } from 'vs/base/common/async'; -import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; -import { localize } from 'vs/nls'; -import { merge } from 'vs/platform/userDataSync/common/extensionsMerge'; - -export class LocalizationSynchroniser extends Disposable implements ISynchroniser { - - private static EXTERNAL_USER_DATA_LOCALIZATION_KEY: string = 'localization'; - - private _status: SyncStatus = SyncStatus.Idle; - get status(): SyncStatus { return this._status; } - private _onDidChangStatus: Emitter = this._register(new Emitter()); - readonly onDidChangeStatus: Event = this._onDidChangStatus.event; - - private _onDidChangeLocal: Emitter = this._register(new Emitter()); - readonly onDidChangeLocal: Event = this._onDidChangeLocal.event; - - private readonly lastSyncExtensionsResource: URI; - - constructor( - @IEnvironmentService environmentService: IEnvironmentService, - @IFileService private readonly fileService: IFileService, - @IUserDataSyncStoreService private readonly userDataSyncStoreService: IUserDataSyncStoreService, - @IUserDataSyncLogService private readonly logService: IUserDataSyncLogService, - @IConfigurationService private readonly configurationService: IConfigurationService, - ) { - super(); - this.lastSyncExtensionsResource = joinPath(environmentService.userRoamingDataHome, '.lastSyncLocalization'); - this._register( - Event.debounce( - Event.any( - Event.filter(this.extensionManagementService.onDidInstallExtension, (e => !!e.gallery)), - Event.filter(this.extensionManagementService.onDidUninstallExtension, (e => !e.error))), - () => undefined, 500)(() => this._onDidChangeLocal.fire())); - } - - private setStatus(status: SyncStatus): void { - if (this._status !== status) { - this._status = status; - this._onDidChangStatus.fire(status); - } - } - - async sync(): Promise { - if (!this.configurationService.getValue('sync.enableLocale')) { - this.logService.trace('Extensions: Skipping synchronizing locale as it is disabled.'); - return false; - } - if (this.status !== SyncStatus.Idle) { - this.logService.trace('Extensions: Skipping synchronizing locale as it is running already.'); - return false; - } - - this.logService.trace('Extensions: Started synchronizing locale...'); - this.setStatus(SyncStatus.Syncing); - - try { - await this.doSync(); - } catch (e) { - this.setStatus(SyncStatus.Idle); - if (e instanceof UserDataSyncStoreError && e.code === UserDataSyncStoreErrorCode.Rejected) { - // Rejected as there is a new remote version. Syncing again, - this.logService.info('Extensions: Failed to synchronise extensions as there is a new remote version available. Synchronizing again...'); - return this.sync(); - } - throw e; - } - - this.logService.trace('Extensions: Finised synchronizing extensions.'); - this.setStatus(SyncStatus.Idle); - return true; - } - - stop(): void { } - - private async doSync(): Promise { - const lastSyncData = await this.getLastSyncUserData(); - const lastSyncExtensions: ISyncExtension[] | null = lastSyncData ? JSON.parse(lastSyncData.content!) : null; - let skippedExtensions: ISyncExtension[] = lastSyncData ? lastSyncData.skippedExtensions || [] : []; - - let remoteData = await this.userDataSyncStoreService.read(ExtensionsSynchroniser.EXTERNAL_USER_DATA_LOCALIZATION_KEY, lastSyncData); - const remoteExtensions: ISyncExtension[] = remoteData.content ? JSON.parse(remoteData.content) : null; - - const localExtensions = await this.getLocalExtensions(); - - if (remoteExtensions) { - this.logService.trace('Extensions: Merging remote extensions with local extensions...'); - } else { - this.logService.info('Extensions: Remote extensions does not exist. Synchronizing extensions for the first time.'); - } - - const ignoredExtensions = this.configurationService.getValue('sync.ignoredExtensions') || []; - const { added, removed, updated, remote } = merge(localExtensions, remoteExtensions, lastSyncExtensions, skippedExtensions, ignoredExtensions); - - if (!added.length && !removed.length && !updated.length && !remote) { - this.logService.trace('Extensions: No changes found during synchronizing extensions.'); - } - - if (added.length || removed.length || updated.length) { - this.logService.info('Extensions: Updating local extensions...'); - skippedExtensions = await this.updateLocalExtensions(added, removed, updated, skippedExtensions); - } - - if (remote) { - // update remote - this.logService.info('Extensions: Updating remote extensions...'); - remoteData = await this.writeToRemote(remote, remoteData.ref); - } - - if (remoteData.content - && (!lastSyncData || lastSyncData.ref !== remoteData.ref) - ) { - // update last sync - this.logService.info('Extensions: Updating last synchronised extensions...'); - await this.updateLastSyncValue({ ...remoteData, skippedExtensions }); - } - } - - private async getLocalExtensions(): Promise { - const installedExtensions = await this.extensionManagementService.getInstalled(ExtensionType.User); - return installedExtensions - .map(({ identifier }) => ({ identifier, enabled: true })); - } - - private async getLastSyncUserData(): Promise { - try { - const content = await this.fileService.readFile(this.lastSyncExtensionsResource); - return JSON.parse(content.value.toString()); - } catch (error) { - return null; - } - } - - private async updateLastSyncValue(lastSyncUserData: ILastSyncUserData): Promise { - await this.fileService.writeFile(this.lastSyncExtensionsResource, VSBuffer.fromString(JSON.stringify(lastSyncUserData))); - } - - private async writeToRemote(extensions: ISyncExtension[], ref: string | null): Promise { - const content = JSON.stringify(extensions); - ref = await this.userDataSyncStoreService.write(ExtensionsSynchroniser.EXTERNAL_USER_DATA_LOCALIZATION_KEY, content, ref); - return { content, ref }; - } - -} diff --git a/src/vs/platform/userDataSync/common/userDataSync.ts b/src/vs/platform/userDataSync/common/userDataSync.ts index 21f92cacb8e..e660f31e2c6 100644 --- a/src/vs/platform/userDataSync/common/userDataSync.ts +++ b/src/vs/platform/userDataSync/common/userDataSync.ts @@ -145,6 +145,11 @@ export interface ISyncExtension { enabled: boolean; } +export interface IGlobalState { + argv: IStringDictionary; + storage: IStringDictionary; +} + export const enum SyncSource { Settings = 1, Keybindings, From e8da35fe13069eb74a2234f0d0a0432be84f9803 Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Mon, 6 Jan 2020 17:51:41 +0100 Subject: [PATCH 03/36] Configure syncing ui state --- .../userDataSync/common/globalStateSync.ts | 23 ++++++++++++------- .../userDataSync/common/userDataSync.ts | 18 ++++++++++----- .../common/userDataSyncService.ts | 5 +++- .../userDataSync/browser/userDataSync.ts | 3 +++ 4 files changed, 34 insertions(+), 15 deletions(-) diff --git a/src/vs/platform/userDataSync/common/globalStateSync.ts b/src/vs/platform/userDataSync/common/globalStateSync.ts index d5e218a964b..0e711eec298 100644 --- a/src/vs/platform/userDataSync/common/globalStateSync.ts +++ b/src/vs/platform/userDataSync/common/globalStateSync.ts @@ -14,6 +14,7 @@ import { IFileService } from 'vs/platform/files/common/files'; import { IStringDictionary } from 'vs/base/common/collections'; import { edit } from 'vs/platform/userDataSync/common/content'; import { merge } from 'vs/platform/userDataSync/common/globalStateMerge'; +import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; const argvProperties: string[] = ['locale']; @@ -36,6 +37,7 @@ export class GlobalStateSynchroniser extends Disposable implements ISynchroniser @IUserDataSyncStoreService private readonly userDataSyncStoreService: IUserDataSyncStoreService, @IUserDataSyncLogService private readonly logService: IUserDataSyncLogService, @IEnvironmentService private readonly environmentService: IEnvironmentService, + @IConfigurationService private readonly configurationService: IConfigurationService, ) { super(); this.lastSyncGlobalStateResource = joinPath(environmentService.userRoamingDataHome, '.lastSyncGlobalState'); @@ -51,24 +53,29 @@ export class GlobalStateSynchroniser extends Disposable implements ISynchroniser } async sync(): Promise { - if (this.status !== SyncStatus.Idle) { - this.logService.trace('Global State: Skipping synchronizing global state as it is running already.'); + if (!this.configurationService.getValue('sync.enableUIState')) { + this.logService.trace('UI State: Skipping synchronizing UI state as it is disabled.'); return false; } - this.logService.trace('Global State: Started synchronizing global state...'); + if (this.status !== SyncStatus.Idle) { + this.logService.trace('UI State: Skipping synchronizing ui state as it is running already.'); + return false; + } + + this.logService.trace('UI State: Started synchronizing ui state...'); this.setStatus(SyncStatus.Syncing); try { await this.doSync(); - this.logService.trace('Global State: Finised synchronizing global state.'); + this.logService.trace('UI State: Finised synchronizing ui state.'); this.setStatus(SyncStatus.Idle); return true; } catch (e) { this.setStatus(SyncStatus.Idle); if (e instanceof UserDataSyncStoreError && e.code === UserDataSyncStoreErrorCode.Rejected) { // Rejected as there is a new remote version. Syncing again, - this.logService.info('Global State: Failed to synchronise global state as there is a new remote version available. Synchronizing again...'); + this.logService.info('UI State: Failed to synchronise ui state as there is a new remote version available. Synchronizing again...'); return this.sync(); } throw e; @@ -90,13 +97,13 @@ export class GlobalStateSynchroniser extends Disposable implements ISynchroniser if (local) { // update local - this.logService.info('Global State: Updating local global state...'); + this.logService.info('UI State: Updating local ui state...'); await this.writeLocalGlobalState(local); } if (remote) { // update remote - this.logService.info('Global State: Updating remote global state...'); + this.logService.info('UI State: Updating remote ui state...'); remoteData = await this.writeToRemote(remote, remoteData.ref); } @@ -104,7 +111,7 @@ export class GlobalStateSynchroniser extends Disposable implements ISynchroniser && (!lastSyncData || lastSyncData.ref !== remoteData.ref) ) { // update last sync - this.logService.info('Global State: Updating last synchronised global state...'); + this.logService.info('UI State: Updating last synchronised ui state...'); await this.updateLastSyncValue(remoteData); } } diff --git a/src/vs/platform/userDataSync/common/userDataSync.ts b/src/vs/platform/userDataSync/common/userDataSync.ts index e660f31e2c6..e273fdde569 100644 --- a/src/vs/platform/userDataSync/common/userDataSync.ts +++ b/src/vs/platform/userDataSync/common/userDataSync.ts @@ -49,18 +49,24 @@ export function registerConfiguration(): IDisposable { default: true, scope: ConfigurationScope.APPLICATION, }, - 'sync.enableExtensions': { - type: 'boolean', - description: localize('sync.enableExtensions', "Enable synchronizing extensions."), - default: true, - scope: ConfigurationScope.APPLICATION, - }, 'sync.enableKeybindings': { type: 'boolean', description: localize('sync.enableKeybindings', "Enable synchronizing keybindings."), default: true, scope: ConfigurationScope.APPLICATION, }, + 'sync.enableUIState': { + type: 'boolean', + description: localize('sync.enableUIState', "Enable synchronizing UI state."), + default: true, + scope: ConfigurationScope.APPLICATION, + }, + 'sync.enableExtensions': { + type: 'boolean', + description: localize('sync.enableExtensions', "Enable synchronizing extensions."), + default: true, + scope: ConfigurationScope.APPLICATION, + }, 'sync.keybindingsPerPlatform': { type: 'boolean', description: localize('sync.keybindingsPerPlatform', "Synchronize keybindings per platform."), diff --git a/src/vs/platform/userDataSync/common/userDataSyncService.ts b/src/vs/platform/userDataSync/common/userDataSyncService.ts index b906b66aafa..7619d616ddb 100644 --- a/src/vs/platform/userDataSync/common/userDataSyncService.ts +++ b/src/vs/platform/userDataSync/common/userDataSyncService.ts @@ -12,6 +12,7 @@ import { ExtensionsSynchroniser } from 'vs/platform/userDataSync/common/extensio import { IExtensionIdentifier } from 'vs/platform/extensions/common/extensions'; import { IAuthTokenService, AuthTokenStatus } from 'vs/platform/auth/common/auth'; import { KeybindingsSynchroniser } from 'vs/platform/userDataSync/common/keybindingsSync'; +import { GlobalStateSynchroniser } from 'vs/platform/userDataSync/common/globalStateSync'; export class UserDataSyncService extends Disposable implements IUserDataSyncService { @@ -32,6 +33,7 @@ export class UserDataSyncService extends Disposable implements IUserDataSyncServ private readonly settingsSynchroniser: SettingsSynchroniser; private readonly keybindingsSynchroniser: KeybindingsSynchroniser; private readonly extensionsSynchroniser: ExtensionsSynchroniser; + private readonly globalStateSynchroniser: GlobalStateSynchroniser; constructor( @IUserDataSyncStoreService private readonly userDataSyncStoreService: IUserDataSyncStoreService, @@ -41,8 +43,9 @@ export class UserDataSyncService extends Disposable implements IUserDataSyncServ super(); this.settingsSynchroniser = this._register(this.instantiationService.createInstance(SettingsSynchroniser)); this.keybindingsSynchroniser = this._register(this.instantiationService.createInstance(KeybindingsSynchroniser)); + this.globalStateSynchroniser = this._register(this.instantiationService.createInstance(GlobalStateSynchroniser)); this.extensionsSynchroniser = this._register(this.instantiationService.createInstance(ExtensionsSynchroniser)); - this.synchronisers = [this.settingsSynchroniser, this.keybindingsSynchroniser, this.extensionsSynchroniser]; + this.synchronisers = [this.settingsSynchroniser, this.keybindingsSynchroniser, this.globalStateSynchroniser, this.extensionsSynchroniser]; this.updateStatus(); if (this.userDataSyncStoreService.userDataSyncStore) { diff --git a/src/vs/workbench/contrib/userDataSync/browser/userDataSync.ts b/src/vs/workbench/contrib/userDataSync/browser/userDataSync.ts index 8fd8c739cf4..69eac6711ac 100644 --- a/src/vs/workbench/contrib/userDataSync/browser/userDataSync.ts +++ b/src/vs/workbench/contrib/userDataSync/browser/userDataSync.ts @@ -212,6 +212,9 @@ export class UserDataSyncWorkbenchContribution extends Disposable implements IWo }, { id: 'sync.enableKeybindings', label: localize('user keybindings', "User Keybindings") + }, { + id: 'sync.enableUIState', + label: localize('ui state', "UI State") }, { id: 'sync.enableExtensions', label: localize('extensions', "Extensions") From 078b82d428b60ddee5537edb8b3ba99605731b90 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Tue, 7 Jan 2020 11:03:59 +0100 Subject: [PATCH 04/36] debt - rewrite layers checker to catch more cases --- .github/workflows/ci.yml | 12 +- .../darwin/continuous-build-darwin.yml | 4 +- .../linux/continuous-build-linux.yml | 4 +- build/azure-pipelines/product-compile.yml | 4 +- .../win32/continuous-build-win32.yml | 4 +- build/lib/globalsLinter.js | 176 ------------- build/lib/globalsLinter.ts | 209 --------------- build/lib/layersChecker.js | 205 +++++++++++++++ build/lib/layersChecker.ts | 241 ++++++++++++++++++ package.json | 2 +- src/vs/base/common/worker/simpleWorker.ts | 4 +- .../base/parts/quickopen/common/quickOpen.ts | 4 +- 12 files changed, 464 insertions(+), 405 deletions(-) delete mode 100644 build/lib/globalsLinter.js delete mode 100644 build/lib/globalsLinter.ts create mode 100644 build/lib/layersChecker.js create mode 100644 build/lib/layersChecker.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9fbfa2f16b4..5c9a7a74c52 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -39,8 +39,8 @@ jobs: name: Run Hygiene Checks - run: yarn monaco-compile-check name: Run Monaco Editor Checks - - run: yarn valid-globals-check - name: Run Valid Globals Checks + - run: yarn valid-layers-check + name: Run Valid Layers Checks - run: yarn compile name: Compile Sources - run: yarn download-builtin-extensions @@ -71,8 +71,8 @@ jobs: name: Run Hygiene Checks - run: yarn monaco-compile-check name: Run Monaco Editor Checks - - run: yarn valid-globals-check - name: Run Valid Globals Checks + - run: yarn valid-layers-check + name: Run Valid Layers Checks - run: yarn compile name: Compile Sources - run: yarn download-builtin-extensions @@ -100,8 +100,8 @@ jobs: name: Run Hygiene Checks - run: yarn monaco-compile-check name: Run Monaco Editor Checks - - run: yarn valid-globals-check - name: Run Valid Globals Checks + - run: yarn valid-layers-check + name: Run Valid Layers Checks - run: yarn compile name: Compile Sources - run: yarn download-builtin-extensions diff --git a/build/azure-pipelines/darwin/continuous-build-darwin.yml b/build/azure-pipelines/darwin/continuous-build-darwin.yml index 72265e035e0..d0aafb2a3e6 100644 --- a/build/azure-pipelines/darwin/continuous-build-darwin.yml +++ b/build/azure-pipelines/darwin/continuous-build-darwin.yml @@ -30,8 +30,8 @@ steps: yarn monaco-compile-check displayName: Run Monaco Editor Checks - script: | - yarn valid-globals-check - displayName: Run Valid Globals Checks + yarn valid-layers-check + displayName: Run Valid Layers Checks - script: | yarn compile displayName: Compile Sources diff --git a/build/azure-pipelines/linux/continuous-build-linux.yml b/build/azure-pipelines/linux/continuous-build-linux.yml index dac42d4ffad..24e05537d9b 100644 --- a/build/azure-pipelines/linux/continuous-build-linux.yml +++ b/build/azure-pipelines/linux/continuous-build-linux.yml @@ -38,8 +38,8 @@ steps: yarn monaco-compile-check displayName: Run Monaco Editor Checks - script: | - yarn valid-globals-check - displayName: Run Valid Globals Checks + yarn valid-layers-check + displayName: Run Valid Layers Checks - script: | yarn compile displayName: Compile Sources diff --git a/build/azure-pipelines/product-compile.yml b/build/azure-pipelines/product-compile.yml index 0055d48f0e4..db6524be03b 100644 --- a/build/azure-pipelines/product-compile.yml +++ b/build/azure-pipelines/product-compile.yml @@ -90,8 +90,8 @@ steps: set -e yarn gulp hygiene yarn monaco-compile-check - yarn valid-globals-check - displayName: Run hygiene, monaco compile & valid globals checks + yarn valid-layers-check + displayName: Run hygiene, monaco compile & valid layers checks condition: and(succeeded(), ne(variables['CacheExists-Compilation'], 'true'), eq(variables['VSCODE_STEP_ON_IT'], 'false')) - script: | diff --git a/build/azure-pipelines/win32/continuous-build-win32.yml b/build/azure-pipelines/win32/continuous-build-win32.yml index 17cb23d406f..8e55440c3a8 100644 --- a/build/azure-pipelines/win32/continuous-build-win32.yml +++ b/build/azure-pipelines/win32/continuous-build-win32.yml @@ -35,8 +35,8 @@ steps: yarn monaco-compile-check displayName: Run Monaco Editor Checks - script: | - yarn valid-globals-check - displayName: Run Valid Globals Checks + yarn valid-layers-check + displayName: Run Valid Layers Checks - powershell: | yarn compile displayName: Compile Sources diff --git a/build/lib/globalsLinter.js b/build/lib/globalsLinter.js deleted file mode 100644 index 1a71d0f4a20..00000000000 --- a/build/lib/globalsLinter.js +++ /dev/null @@ -1,176 +0,0 @@ -"use strict"; -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See License.txt in the project root for license information. - *--------------------------------------------------------------------------------------------*/ -Object.defineProperty(exports, "__esModule", { value: true }); -const ts = require("typescript"); -const fs_1 = require("fs"); -const path_1 = require("path"); -const minimatch_1 = require("minimatch"); -// -// ############################################################################################# -// -// A custom typescript linter for the specific task of detecting the use of certain globals in a -// layer that does not allow the use. For example: -// - using DOM globals in common/node/electron-main layer (e.g. HTMLElement) -// - using node.js globals in common/browser layer (e.g. process) -// -// Make changes to below RULES to lift certain files from these checks only if absolutely needed -// -// ############################################################################################# -// -const RULES = { - 'no-nodejs-globals': [ - { - 'target': '**/vs/**/test/{common,browser}/**', - 'allowed': [ - 'process', - 'Buffer', - '__filename', - '__dirname' - ] - }, - { - 'target': '**/vs/workbench/api/common/extHostExtensionService.ts', - 'allowed': [ - 'global' // -> safe access to 'global' - ] - }, - { - 'target': '**/vs/**/{common,browser}/**', - 'allowed': [ /* none */] - } - ], - 'no-dom-globals': [ - { - 'target': '**/vs/base/parts/quickopen/common/quickOpen.ts', - 'allowed': [ - 'HTMLElement' // quick open will be replaced with a different widget soon - ] - }, - { - 'target': '**/vs/**/test/{common,node,electron-main}/**', - 'allowed': [ - 'document', - 'HTMLElement', - 'createElement' - ] - }, - { - 'target': '**/vs/**/{common,node,electron-main}/**', - 'allowed': [ /* none */] - } - ] -}; -const TS_CONFIG_PATH = path_1.join(__dirname, '../../', 'src', 'tsconfig.json'); -const DOM_GLOBALS_DEFINITION = 'lib.dom.d.ts'; -const DISALLOWED_DOM_GLOBALS = [ - 'window', - 'document', - 'HTMLElement', - 'createElement' -]; -const NODE_GLOBALS_DEFINITION = '@types/node'; -const DISALLOWED_NODE_GLOBALS = [ - // https://nodejs.org/api/globals.html#globals_global_objects - 'NodeJS', - 'Buffer', - '__dirname', - '__filename', - 'clearImmediate', - 'exports', - 'global', - 'module', - 'process', - 'setImmediate' -]; -let hasErrors = false; -function checkFile(program, sourceFile, rule) { - checkNode(sourceFile); - function checkNode(node) { - if (node.kind !== ts.SyntaxKind.Identifier) { - return ts.forEachChild(node, checkNode); // recurse down - } - const text = node.getText(sourceFile); - if (!rule.disallowedGlobals.some(disallowedGlobal => disallowedGlobal === text)) { - return; // only if disallowed - } - if (rule.allowedGlobals.some(allowed => allowed === text)) { - return; // override - } - const checker = program.getTypeChecker(); - const symbol = checker.getSymbolAtLocation(node); - if (symbol) { - const declarations = symbol.declarations; - if (Array.isArray(declarations) && symbol.declarations.some(declaration => { - if (declaration) { - const parent = declaration.parent; - if (parent) { - const sourceFile = parent.getSourceFile(); - if (sourceFile) { - const fileName = sourceFile.fileName; - if (fileName && fileName.indexOf(rule.definition) >= 0) { - return true; - } - } - } - } - return false; - })) { - const { line, character } = sourceFile.getLineAndCharacterOfPosition(node.getStart()); - console.log(`build/lib/globalsLinter.ts: Cannot use global '${text}' in ${sourceFile.fileName} (${line + 1},${character + 1})`); - hasErrors = true; - } - } - } -} -function createProgram(tsconfigPath) { - const tsConfig = ts.readConfigFile(tsconfigPath, ts.sys.readFile); - const configHostParser = { fileExists: fs_1.existsSync, readDirectory: ts.sys.readDirectory, readFile: file => fs_1.readFileSync(file, 'utf8'), useCaseSensitiveFileNames: process.platform === 'linux' }; - const tsConfigParsed = ts.parseJsonConfigFileContent(tsConfig.config, configHostParser, path_1.resolve(path_1.dirname(tsconfigPath)), { noEmit: true }); - const compilerHost = ts.createCompilerHost(tsConfigParsed.options, true); - return ts.createProgram(tsConfigParsed.fileNames, tsConfigParsed.options, compilerHost); -} -// -// Create program and start checking -// -const program = createProgram(TS_CONFIG_PATH); -for (const sourceFile of program.getSourceFiles()) { - let noDomGlobalsLinter = undefined; - let noNodeJSGlobalsLinter = undefined; - for (const rules of RULES['no-dom-globals']) { - if (minimatch_1.match([sourceFile.fileName], rules.target).length > 0) { - noDomGlobalsLinter = { allowed: rules.allowed }; - break; - } - } - for (const rules of RULES['no-nodejs-globals']) { - if (minimatch_1.match([sourceFile.fileName], rules.target).length > 0) { - noNodeJSGlobalsLinter = { allowed: rules.allowed }; - break; - } - } - if (!noDomGlobalsLinter && !noNodeJSGlobalsLinter) { - continue; // no rule to run - } - // No DOM Globals - if (noDomGlobalsLinter) { - checkFile(program, sourceFile, { - definition: DOM_GLOBALS_DEFINITION, - disallowedGlobals: DISALLOWED_DOM_GLOBALS, - allowedGlobals: noDomGlobalsLinter.allowed - }); - } - // No node.js Globals - if (noNodeJSGlobalsLinter) { - checkFile(program, sourceFile, { - definition: NODE_GLOBALS_DEFINITION, - disallowedGlobals: DISALLOWED_NODE_GLOBALS, - allowedGlobals: noNodeJSGlobalsLinter.allowed - }); - } -} -if (hasErrors) { - process.exit(1); -} diff --git a/build/lib/globalsLinter.ts b/build/lib/globalsLinter.ts deleted file mode 100644 index 07e87f3c685..00000000000 --- a/build/lib/globalsLinter.ts +++ /dev/null @@ -1,209 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See License.txt in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -import * as ts from 'typescript'; -import { readFileSync, existsSync } from 'fs'; -import { resolve, dirname, join } from 'path'; -import { match } from 'minimatch'; - -// -// ############################################################################################# -// -// A custom typescript linter for the specific task of detecting the use of certain globals in a -// layer that does not allow the use. For example: -// - using DOM globals in common/node/electron-main layer (e.g. HTMLElement) -// - using node.js globals in common/browser layer (e.g. process) -// -// Make changes to below RULES to lift certain files from these checks only if absolutely needed -// -// ############################################################################################# -// - -const RULES = { - 'no-nodejs-globals': [ - { - 'target': '**/vs/**/test/{common,browser}/**', - 'allowed': [ // -> less strict for test files - 'process', - 'Buffer', - '__filename', - '__dirname' - ] - }, - { - 'target': '**/vs/workbench/api/common/extHostExtensionService.ts', - 'allowed': [ - 'global' // -> safe access to 'global' - ] - }, - { - 'target': '**/vs/**/{common,browser}/**', - 'allowed': [ /* none */] - } - ], - 'no-dom-globals': [ - { - 'target': '**/vs/base/parts/quickopen/common/quickOpen.ts', - 'allowed': [ - 'HTMLElement' // quick open will be replaced with a different widget soon - ] - }, - { - 'target': '**/vs/**/test/{common,node,electron-main}/**', - 'allowed': [ // -> less strict for test files - 'document', - 'HTMLElement', - 'createElement' - ] - }, - { - 'target': '**/vs/**/{common,node,electron-main}/**', - 'allowed': [ /* none */] - } - ] -}; - -const TS_CONFIG_PATH = join(__dirname, '../../', 'src', 'tsconfig.json'); - -const DOM_GLOBALS_DEFINITION = 'lib.dom.d.ts'; - -const DISALLOWED_DOM_GLOBALS = [ - 'window', - 'document', - 'HTMLElement', - 'createElement' -]; - -const NODE_GLOBALS_DEFINITION = '@types/node'; - -const DISALLOWED_NODE_GLOBALS = [ - // https://nodejs.org/api/globals.html#globals_global_objects - 'NodeJS', - 'Buffer', - '__dirname', - '__filename', - 'clearImmediate', - 'exports', - 'global', - 'module', - 'process', - 'setImmediate' -]; - -interface IRule { - definition: string; - disallowedGlobals: string[]; - allowedGlobals: string[]; -} - -let hasErrors = false; - -function checkFile(program: ts.Program, sourceFile: ts.SourceFile, rule: IRule) { - checkNode(sourceFile); - - function checkNode(node: ts.Node): void { - if (node.kind !== ts.SyntaxKind.Identifier) { - return ts.forEachChild(node, checkNode); // recurse down - } - - const text = node.getText(sourceFile); - - if (!rule.disallowedGlobals.some(disallowedGlobal => disallowedGlobal === text)) { - return; // only if disallowed - } - - if (rule.allowedGlobals.some(allowed => allowed === text)) { - return; // override - } - - const checker = program.getTypeChecker(); - const symbol = checker.getSymbolAtLocation(node); - if (symbol) { - const declarations = symbol.declarations; - if (Array.isArray(declarations) && symbol.declarations.some(declaration => { - if (declaration) { - const parent = declaration.parent; - if (parent) { - const sourceFile = parent.getSourceFile(); - if (sourceFile) { - const fileName = sourceFile.fileName; - if (fileName && fileName.indexOf(rule.definition) >= 0) { - return true; - } - } - } - } - - return false; - })) { - const { line, character } = sourceFile.getLineAndCharacterOfPosition(node.getStart()); - console.log(`build/lib/globalsLinter.ts: Cannot use global '${text}' in ${sourceFile.fileName} (${line + 1},${character + 1})`); - - hasErrors = true; - } - } - } -} - -function createProgram(tsconfigPath: string): ts.Program { - const tsConfig = ts.readConfigFile(tsconfigPath, ts.sys.readFile); - - const configHostParser: ts.ParseConfigHost = { fileExists: existsSync, readDirectory: ts.sys.readDirectory, readFile: file => readFileSync(file, 'utf8'), useCaseSensitiveFileNames: process.platform === 'linux' }; - const tsConfigParsed = ts.parseJsonConfigFileContent(tsConfig.config, configHostParser, resolve(dirname(tsconfigPath)), { noEmit: true }); - - const compilerHost = ts.createCompilerHost(tsConfigParsed.options, true); - - return ts.createProgram(tsConfigParsed.fileNames, tsConfigParsed.options, compilerHost); -} - -// -// Create program and start checking -// -const program = createProgram(TS_CONFIG_PATH); - -for (const sourceFile of program.getSourceFiles()) { - let noDomGlobalsLinter: { allowed: string[] } | undefined = undefined; - let noNodeJSGlobalsLinter: { allowed: string[] } | undefined = undefined; - - for (const rules of RULES['no-dom-globals']) { - if (match([sourceFile.fileName], rules.target).length > 0) { - noDomGlobalsLinter = { allowed: rules.allowed }; - break; - } - } - - for (const rules of RULES['no-nodejs-globals']) { - if (match([sourceFile.fileName], rules.target).length > 0) { - noNodeJSGlobalsLinter = { allowed: rules.allowed }; - break; - } - } - - if (!noDomGlobalsLinter && !noNodeJSGlobalsLinter) { - continue; // no rule to run - } - - // No DOM Globals - if (noDomGlobalsLinter) { - checkFile(program, sourceFile, { - definition: DOM_GLOBALS_DEFINITION, - disallowedGlobals: DISALLOWED_DOM_GLOBALS, - allowedGlobals: noDomGlobalsLinter.allowed - }); - } - - // No node.js Globals - if (noNodeJSGlobalsLinter) { - checkFile(program, sourceFile, { - definition: NODE_GLOBALS_DEFINITION, - disallowedGlobals: DISALLOWED_NODE_GLOBALS, - allowedGlobals: noNodeJSGlobalsLinter.allowed - }); - } -} - -if (hasErrors) { - process.exit(1); -} diff --git a/build/lib/layersChecker.js b/build/lib/layersChecker.js new file mode 100644 index 00000000000..be871ae5228 --- /dev/null +++ b/build/lib/layersChecker.js @@ -0,0 +1,205 @@ +"use strict"; +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ +Object.defineProperty(exports, "__esModule", { value: true }); +const ts = require("typescript"); +const fs_1 = require("fs"); +const path_1 = require("path"); +const minimatch_1 = require("minimatch"); +// +// ############################################################################################# +// +// A custom typescript checker for the specific task of detecting the use of certain types in a +// layer that does not allow such use. For example: +// - using DOM globals in common/node/electron-main layer (e.g. HTMLElement) +// - using node.js globals in common/browser layer (e.g. process) +// +// Make changes to below RULES to lift certain files from these checks only if absolutely needed +// +// ############################################################################################# +// +// Types we assume are present in all implementations of JS VMs (node.js, browsers) +// Feel free to add more core types as you see needed if present in node.js and browsers +const CORE_TYPES = [ + 'require', + 'atob', + 'btoa', + 'setTimeout', + 'clearTimeout', + 'setInterval', + 'clearInterval', + 'console', + 'log', + 'info', + 'warn', + 'error', + 'group', + 'groupEnd', + 'table', + 'Error', + 'String', + 'throws', + 'stack', + 'captureStackTrace', + 'stackTraceLimit', + 'TextDecoder', + 'TextEncoder', + 'encode', + 'decode', + 'self', + 'trimLeft', + 'trimRight' +]; +const RULES = [ + // Tests: skip + { + target: '**/vs/**/test/**', + skip: true // -> skip all test files + }, + // Common: vs/base/common/platform.ts + { + target: '**/vs/base/common/platform.ts', + allowedTypes: [ + ...CORE_TYPES, + // Safe access to postMessage() and friends + 'MessageEvent', + 'data' + ], + disallowedDefinitions: [ + 'lib.dom.d.ts', + '@types' // no node.js + ] + }, + // Common: vs/workbench/api/common/extHostExtensionService.ts + { + target: '**/vs/workbench/api/common/extHostExtensionService.ts', + allowedTypes: [ + ...CORE_TYPES, + // Safe access to global + 'global' + ], + disallowedDefinitions: [ + 'lib.dom.d.ts', + '@types' // no node.js + ] + }, + // Common + { + target: '**/vs/**/common/**', + allowedTypes: CORE_TYPES, + disallowedDefinitions: [ + 'lib.dom.d.ts', + '@types' // no node.js + ] + }, + // Browser + { + target: '**/vs/**/browser/**', + allowedTypes: CORE_TYPES, + disallowedDefinitions: [ + '@types' // no node.js + ] + }, + // node.js + { + target: '**/vs/**/node/**', + allowedTypes: [ + ...CORE_TYPES, + // --> types from node.d.ts that duplicate from lib.dom.d.ts + 'URL', + 'protocol', + 'hostname', + 'port', + 'pathname', + 'search', + 'username', + 'password' + ], + disallowedDefinitions: [ + 'lib.dom.d.ts' // no DOM + ] + }, + // Electron (renderer): skip + { + target: '**/vs/**/electron-browser/**', + skip: true // -> supports all types + }, + // Electron (main) + { + target: '**/vs/**/electron-main/**', + allowedTypes: [ + ...CORE_TYPES, + // --> types from electron.d.ts that duplicate from lib.dom.d.ts + 'Event', + 'Request' + ], + disallowedDefinitions: [ + 'lib.dom.d.ts' // no DOM + ] + } +]; +const TS_CONFIG_PATH = path_1.join(__dirname, '../../', 'src', 'tsconfig.json'); +let hasErrors = false; +function checkFile(program, sourceFile, rule) { + checkNode(sourceFile); + function checkNode(node) { + var _a, _b; + if (node.kind !== ts.SyntaxKind.Identifier) { + return ts.forEachChild(node, checkNode); // recurse down + } + const text = node.getText(sourceFile); + if ((_a = rule.allowedTypes) === null || _a === void 0 ? void 0 : _a.some(allowed => allowed === text)) { + return; // override + } + const checker = program.getTypeChecker(); + const symbol = checker.getSymbolAtLocation(node); + if (symbol) { + const declarations = symbol.declarations; + if (Array.isArray(declarations)) { + for (const declaration of declarations) { + if (declaration) { + const parent = declaration.parent; + if (parent) { + const parentSourceFile = parent.getSourceFile(); + if (parentSourceFile) { + const definitionFileName = parentSourceFile.fileName; + if ((_b = rule.disallowedDefinitions) === null || _b === void 0 ? void 0 : _b.some(disallowedDefinition => definitionFileName.indexOf(disallowedDefinition) >= 0)) { + const { line, character } = sourceFile.getLineAndCharacterOfPosition(node.getStart()); + console.log(`[build/lib/layersChecker.ts]: Reference to '${text}' from ${path_1.basename(definitionFileName)} violates layers (${sourceFile.fileName} (${line + 1},${character + 1})`); + hasErrors = true; + break; + } + } + } + } + } + } + } + } +} +function createProgram(tsconfigPath) { + const tsConfig = ts.readConfigFile(tsconfigPath, ts.sys.readFile); + const configHostParser = { fileExists: fs_1.existsSync, readDirectory: ts.sys.readDirectory, readFile: file => fs_1.readFileSync(file, 'utf8'), useCaseSensitiveFileNames: process.platform === 'linux' }; + const tsConfigParsed = ts.parseJsonConfigFileContent(tsConfig.config, configHostParser, path_1.resolve(path_1.dirname(tsconfigPath)), { noEmit: true }); + const compilerHost = ts.createCompilerHost(tsConfigParsed.options, true); + return ts.createProgram(tsConfigParsed.fileNames, tsConfigParsed.options, compilerHost); +} +// +// Create program and start checking +// +const program = createProgram(TS_CONFIG_PATH); +for (const sourceFile of program.getSourceFiles()) { + for (const rule of RULES) { + if (minimatch_1.match([sourceFile.fileName], rule.target).length > 0) { + if (!rule.skip) { + checkFile(program, sourceFile, rule); + } + break; + } + } +} +if (hasErrors) { + process.exit(1); +} diff --git a/build/lib/layersChecker.ts b/build/lib/layersChecker.ts new file mode 100644 index 00000000000..800fc3590a6 --- /dev/null +++ b/build/lib/layersChecker.ts @@ -0,0 +1,241 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as ts from 'typescript'; +import { readFileSync, existsSync } from 'fs'; +import { resolve, dirname, join, basename } from 'path'; +import { match } from 'minimatch'; + +// +// ############################################################################################# +// +// A custom typescript checker for the specific task of detecting the use of certain types in a +// layer that does not allow such use. For example: +// - using DOM globals in common/node/electron-main layer (e.g. HTMLElement) +// - using node.js globals in common/browser layer (e.g. process) +// +// Make changes to below RULES to lift certain files from these checks only if absolutely needed +// +// ############################################################################################# +// + +// Types we assume are present in all implementations of JS VMs (node.js, browsers) +// Feel free to add more core types as you see needed if present in node.js and browsers +const CORE_TYPES = [ + 'require', // from our AMD loader + 'atob', + 'btoa', + 'setTimeout', + 'clearTimeout', + 'setInterval', + 'clearInterval', + 'console', + 'log', + 'info', + 'warn', + 'error', + 'group', + 'groupEnd', + 'table', + 'Error', + 'String', + 'throws', + 'stack', + 'captureStackTrace', + 'stackTraceLimit', + 'TextDecoder', + 'TextEncoder', + 'encode', + 'decode', + 'self', + 'trimLeft', + 'trimRight' +]; + +const RULES = [ + + // Tests: skip + { + target: '**/vs/**/test/**', + skip: true // -> skip all test files + }, + + // Common: vs/base/common/platform.ts + { + target: '**/vs/base/common/platform.ts', + allowedTypes: [ + ...CORE_TYPES, + + // Safe access to postMessage() and friends + 'MessageEvent', + 'data' + ], + disallowedDefinitions: [ + 'lib.dom.d.ts', // no DOM + '@types' // no node.js + ] + }, + + // Common: vs/workbench/api/common/extHostExtensionService.ts + { + target: '**/vs/workbench/api/common/extHostExtensionService.ts', + allowedTypes: [ + ...CORE_TYPES, + + // Safe access to global + 'global' + ], + disallowedDefinitions: [ + 'lib.dom.d.ts', // no DOM + '@types' // no node.js + ] + }, + + // Common + { + target: '**/vs/**/common/**', + allowedTypes: CORE_TYPES, + disallowedDefinitions: [ + 'lib.dom.d.ts', // no DOM + '@types' // no node.js + ] + }, + + // Browser + { + target: '**/vs/**/browser/**', + allowedTypes: CORE_TYPES, + disallowedDefinitions: [ + '@types' // no node.js + ] + }, + + // node.js + { + target: '**/vs/**/node/**', + allowedTypes: [ + ...CORE_TYPES, + + // --> types from node.d.ts that duplicate from lib.dom.d.ts + 'URL', + 'protocol', + 'hostname', + 'port', + 'pathname', + 'search', + 'username', + 'password' + ], + disallowedDefinitions: [ + 'lib.dom.d.ts' // no DOM + ] + }, + + // Electron (renderer): skip + { + target: '**/vs/**/electron-browser/**', + skip: true // -> supports all types + }, + + // Electron (main) + { + target: '**/vs/**/electron-main/**', + allowedTypes: [ + ...CORE_TYPES, + + // --> types from electron.d.ts that duplicate from lib.dom.d.ts + 'Event', + 'Request' + ], + disallowedDefinitions: [ + 'lib.dom.d.ts' // no DOM + ] + } +]; + +const TS_CONFIG_PATH = join(__dirname, '../../', 'src', 'tsconfig.json'); + +interface IRule { + target: string; + skip?: boolean; + allowedTypes?: string[]; + disallowedDefinitions?: string[]; +} + +let hasErrors = false; + +function checkFile(program: ts.Program, sourceFile: ts.SourceFile, rule: IRule) { + checkNode(sourceFile); + + function checkNode(node: ts.Node): void { + if (node.kind !== ts.SyntaxKind.Identifier) { + return ts.forEachChild(node, checkNode); // recurse down + } + + const text = node.getText(sourceFile); + + if (rule.allowedTypes?.some(allowed => allowed === text)) { + return; // override + } + + const checker = program.getTypeChecker(); + const symbol = checker.getSymbolAtLocation(node); + if (symbol) { + const declarations = symbol.declarations; + if (Array.isArray(declarations)) { + for (const declaration of declarations) { + if (declaration) { + const parent = declaration.parent; + if (parent) { + const parentSourceFile = parent.getSourceFile(); + if (parentSourceFile) { + const definitionFileName = parentSourceFile.fileName; + if (rule.disallowedDefinitions?.some(disallowedDefinition => definitionFileName.indexOf(disallowedDefinition) >= 0)) { + const { line, character } = sourceFile.getLineAndCharacterOfPosition(node.getStart()); + console.log(`[build/lib/layersChecker.ts]: Reference to '${text}' from ${basename(definitionFileName)} violates layers (${sourceFile.fileName} (${line + 1},${character + 1})`); + + hasErrors = true; + break; + } + } + } + } + } + } + } + } +} + +function createProgram(tsconfigPath: string): ts.Program { + const tsConfig = ts.readConfigFile(tsconfigPath, ts.sys.readFile); + + const configHostParser: ts.ParseConfigHost = { fileExists: existsSync, readDirectory: ts.sys.readDirectory, readFile: file => readFileSync(file, 'utf8'), useCaseSensitiveFileNames: process.platform === 'linux' }; + const tsConfigParsed = ts.parseJsonConfigFileContent(tsConfig.config, configHostParser, resolve(dirname(tsconfigPath)), { noEmit: true }); + + const compilerHost = ts.createCompilerHost(tsConfigParsed.options, true); + + return ts.createProgram(tsConfigParsed.fileNames, tsConfigParsed.options, compilerHost); +} + +// +// Create program and start checking +// +const program = createProgram(TS_CONFIG_PATH); + +for (const sourceFile of program.getSourceFiles()) { + for (const rule of RULES) { + if (match([sourceFile.fileName], rule.target).length > 0) { + if (!rule.skip) { + checkFile(program, sourceFile, rule); + } + + break; + } + } +} + +if (hasErrors) { + process.exit(1); +} diff --git a/package.json b/package.json index 5a5d54ce626..aa95fce241d 100644 --- a/package.json +++ b/package.json @@ -25,7 +25,7 @@ "smoketest": "cd test/smoke && node test/index.js", "download-builtin-extensions": "node build/lib/builtInExtensions.js", "monaco-compile-check": "tsc -p src/tsconfig.monaco.json --noEmit", - "valid-globals-check": "node build/lib/globalsLinter.js", + "valid-layers-check": "node build/lib/layersChecker.js", "strict-function-types-watch": "tsc --watch -p src/tsconfig.json --noEmit --strictFunctionTypes", "update-distro": "node build/npm/update-distro.js", "web": "node scripts/code-web.js", diff --git a/src/vs/base/common/worker/simpleWorker.ts b/src/vs/base/common/worker/simpleWorker.ts index 6c9815d3aaa..479c2ceb32c 100644 --- a/src/vs/base/common/worker/simpleWorker.ts +++ b/src/vs/base/common/worker/simpleWorker.ts @@ -12,7 +12,7 @@ const INITIALIZE = '$initialize'; export interface IWorker extends IDisposable { getId(): number; - postMessage(message: any, transfer: Transferable[]): void; + postMessage(message: any, transfer: ArrayBuffer[]): void; } export interface IWorkerCallback { @@ -302,7 +302,7 @@ export class SimpleWorkerServer { private _requestHandler: IRequestHandler | null; private _protocol: SimpleWorkerProtocol; - constructor(postMessage: (msg: any, transfer?: Transferable[]) => void, requestHandlerFactory: IRequestHandlerFactory | null) { + constructor(postMessage: (msg: any, transfer?: ArrayBuffer[]) => void, requestHandlerFactory: IRequestHandlerFactory | null) { this._requestHandlerFactory = requestHandlerFactory; this._requestHandler = null; this._protocol = new SimpleWorkerProtocol({ diff --git a/src/vs/base/parts/quickopen/common/quickOpen.ts b/src/vs/base/parts/quickopen/common/quickOpen.ts index d6039c68802..582ddf56ee6 100644 --- a/src/vs/base/parts/quickopen/common/quickOpen.ts +++ b/src/vs/base/parts/quickopen/common/quickOpen.ts @@ -68,9 +68,7 @@ export interface IDataSource { export interface IRenderer { getHeight(entry: T): number; getTemplateId(entry: T): string; - // rationale: will be replaced by quickinput later - // tslint:disable-next-line: no-dom-globals - renderTemplate(templateId: string, container: HTMLElement, styles: any): any; + renderTemplate(templateId: string, container: any /* HTMLElement */, styles: any): any; renderElement(entry: T, templateId: string, templateData: any, styles: any): void; disposeTemplate(templateId: string, templateData: any): void; } From 9b6b25112e976709a9ff30947f63c670402e0926 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Tue, 7 Jan 2020 11:10:01 +0100 Subject: [PATCH 05/36] :lipstick: --- build/lib/layersChecker.js | 24 ++++++++++++++---------- build/lib/layersChecker.ts | 24 ++++++++++++++---------- 2 files changed, 28 insertions(+), 20 deletions(-) diff --git a/build/lib/layersChecker.js b/build/lib/layersChecker.js index be871ae5228..09342450b76 100644 --- a/build/lib/layersChecker.js +++ b/build/lib/layersChecker.js @@ -69,7 +69,7 @@ const RULES = [ ], disallowedDefinitions: [ 'lib.dom.d.ts', - '@types' // no node.js + '@types/node' // no node.js ] }, // Common: vs/workbench/api/common/extHostExtensionService.ts @@ -82,7 +82,7 @@ const RULES = [ ], disallowedDefinitions: [ 'lib.dom.d.ts', - '@types' // no node.js + '@types/node' // no node.js ] }, // Common @@ -91,7 +91,7 @@ const RULES = [ allowedTypes: CORE_TYPES, disallowedDefinitions: [ 'lib.dom.d.ts', - '@types' // no node.js + '@types/node' // no node.js ] }, // Browser @@ -99,7 +99,7 @@ const RULES = [ target: '**/vs/**/browser/**', allowedTypes: CORE_TYPES, disallowedDefinitions: [ - '@types' // no node.js + '@types/node' // no node.js ] }, // node.js @@ -145,7 +145,7 @@ let hasErrors = false; function checkFile(program, sourceFile, rule) { checkNode(sourceFile); function checkNode(node) { - var _a, _b; + var _a; if (node.kind !== ts.SyntaxKind.Identifier) { return ts.forEachChild(node, checkNode); // recurse down } @@ -165,11 +165,15 @@ function checkFile(program, sourceFile, rule) { const parentSourceFile = parent.getSourceFile(); if (parentSourceFile) { const definitionFileName = parentSourceFile.fileName; - if ((_b = rule.disallowedDefinitions) === null || _b === void 0 ? void 0 : _b.some(disallowedDefinition => definitionFileName.indexOf(disallowedDefinition) >= 0)) { - const { line, character } = sourceFile.getLineAndCharacterOfPosition(node.getStart()); - console.log(`[build/lib/layersChecker.ts]: Reference to '${text}' from ${path_1.basename(definitionFileName)} violates layers (${sourceFile.fileName} (${line + 1},${character + 1})`); - hasErrors = true; - break; + if (rule.disallowedDefinitions) { + for (const disallowedDefinition of rule.disallowedDefinitions) { + if (definitionFileName.indexOf(disallowedDefinition) >= 0) { + const { line, character } = sourceFile.getLineAndCharacterOfPosition(node.getStart()); + console.log(`[build/lib/layersChecker.ts]: Reference to '${text}' from '${disallowedDefinition}' violates layers (${sourceFile.fileName} (${line + 1},${character + 1})`); + hasErrors = true; + return; + } + } } } } diff --git a/build/lib/layersChecker.ts b/build/lib/layersChecker.ts index 800fc3590a6..fd7cc352325 100644 --- a/build/lib/layersChecker.ts +++ b/build/lib/layersChecker.ts @@ -5,7 +5,7 @@ import * as ts from 'typescript'; import { readFileSync, existsSync } from 'fs'; -import { resolve, dirname, join, basename } from 'path'; +import { resolve, dirname, join } from 'path'; import { match } from 'minimatch'; // @@ -74,7 +74,7 @@ const RULES = [ ], disallowedDefinitions: [ 'lib.dom.d.ts', // no DOM - '@types' // no node.js + '@types/node' // no node.js ] }, @@ -89,7 +89,7 @@ const RULES = [ ], disallowedDefinitions: [ 'lib.dom.d.ts', // no DOM - '@types' // no node.js + '@types/node' // no node.js ] }, @@ -99,7 +99,7 @@ const RULES = [ allowedTypes: CORE_TYPES, disallowedDefinitions: [ 'lib.dom.d.ts', // no DOM - '@types' // no node.js + '@types/node' // no node.js ] }, @@ -108,7 +108,7 @@ const RULES = [ target: '**/vs/**/browser/**', allowedTypes: CORE_TYPES, disallowedDefinitions: [ - '@types' // no node.js + '@types/node' // no node.js ] }, @@ -192,12 +192,16 @@ function checkFile(program: ts.Program, sourceFile: ts.SourceFile, rule: IRule) const parentSourceFile = parent.getSourceFile(); if (parentSourceFile) { const definitionFileName = parentSourceFile.fileName; - if (rule.disallowedDefinitions?.some(disallowedDefinition => definitionFileName.indexOf(disallowedDefinition) >= 0)) { - const { line, character } = sourceFile.getLineAndCharacterOfPosition(node.getStart()); - console.log(`[build/lib/layersChecker.ts]: Reference to '${text}' from ${basename(definitionFileName)} violates layers (${sourceFile.fileName} (${line + 1},${character + 1})`); + if (rule.disallowedDefinitions) { + for (const disallowedDefinition of rule.disallowedDefinitions) { + if (definitionFileName.indexOf(disallowedDefinition) >= 0) { + const { line, character } = sourceFile.getLineAndCharacterOfPosition(node.getStart()); + console.log(`[build/lib/layersChecker.ts]: Reference to '${text}' from '${disallowedDefinition}' violates layers (${sourceFile.fileName} (${line + 1},${character + 1})`); - hasErrors = true; - break; + hasErrors = true; + return; + } + } } } } From d59de94ba176a1bc0a296b60c90a979e2079b8c5 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Tue, 7 Jan 2020 11:19:01 +0100 Subject: [PATCH 06/36] :lipstick: --- build/lib/layersChecker.js | 2 +- build/lib/layersChecker.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/build/lib/layersChecker.js b/build/lib/layersChecker.js index 09342450b76..8f77ed81458 100644 --- a/build/lib/layersChecker.js +++ b/build/lib/layersChecker.js @@ -169,7 +169,7 @@ function checkFile(program, sourceFile, rule) { for (const disallowedDefinition of rule.disallowedDefinitions) { if (definitionFileName.indexOf(disallowedDefinition) >= 0) { const { line, character } = sourceFile.getLineAndCharacterOfPosition(node.getStart()); - console.log(`[build/lib/layersChecker.ts]: Reference to '${text}' from '${disallowedDefinition}' violates layers (${sourceFile.fileName} (${line + 1},${character + 1})`); + console.log(`[build/lib/layersChecker.ts]: Reference to '${text}' from '${disallowedDefinition}' violates layer '${rule.target}' (${sourceFile.fileName} (${line + 1},${character + 1})`); hasErrors = true; return; } diff --git a/build/lib/layersChecker.ts b/build/lib/layersChecker.ts index fd7cc352325..787ddf967ce 100644 --- a/build/lib/layersChecker.ts +++ b/build/lib/layersChecker.ts @@ -196,7 +196,7 @@ function checkFile(program: ts.Program, sourceFile: ts.SourceFile, rule: IRule) for (const disallowedDefinition of rule.disallowedDefinitions) { if (definitionFileName.indexOf(disallowedDefinition) >= 0) { const { line, character } = sourceFile.getLineAndCharacterOfPosition(node.getStart()); - console.log(`[build/lib/layersChecker.ts]: Reference to '${text}' from '${disallowedDefinition}' violates layers (${sourceFile.fileName} (${line + 1},${character + 1})`); + console.log(`[build/lib/layersChecker.ts]: Reference to '${text}' from '${disallowedDefinition}' violates layer '${rule.target}' (${sourceFile.fileName} (${line + 1},${character + 1})`); hasErrors = true; return; From 3ab2c1b5aa5babb3ab76d99871459fb1a6da3242 Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Tue, 7 Jan 2020 11:54:36 +0100 Subject: [PATCH 07/36] Fix #88174 --- src/vs/workbench/browser/parts/views/views.ts | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/vs/workbench/browser/parts/views/views.ts b/src/vs/workbench/browser/parts/views/views.ts index 2c5cf72cdfb..a3b9148647a 100644 --- a/src/vs/workbench/browser/parts/views/views.ts +++ b/src/vs/workbench/browser/parts/views/views.ts @@ -664,8 +664,17 @@ export class ViewsService extends Disposable implements IViewsService { } getViewDescriptors(container: ViewContainer): IViewDescriptorCollection | null { - const viewDescriptorCollectionItem = this.viewDescriptorCollections.get(container); - return viewDescriptorCollectionItem ? viewDescriptorCollectionItem.viewDescriptorCollection : null; + const registeredViewContainer = Registry.as(ViewExtensions.ViewContainersRegistry).get(container.id); + if (registeredViewContainer) { + let viewDescriptorCollectionItem = this.viewDescriptorCollections.get(registeredViewContainer); + if (!viewDescriptorCollectionItem) { + // Create and register the collection if does not exist + this.onDidRegisterViewContainer(registeredViewContainer); + viewDescriptorCollectionItem = this.viewDescriptorCollections.get(registeredViewContainer); + } + return viewDescriptorCollectionItem!.viewDescriptorCollection; + } + return null; } async openView(id: string, focus: boolean): Promise { From 19c07c0c0e8ba16804b6834df3029305f91c100c Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Tue, 7 Jan 2020 12:15:20 +0100 Subject: [PATCH 08/36] Fix #88132 --- src/vs/workbench/services/search/common/search.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/workbench/services/search/common/search.ts b/src/vs/workbench/services/search/common/search.ts index 5d64c576132..21e77c7f90f 100644 --- a/src/vs/workbench/services/search/common/search.ts +++ b/src/vs/workbench/services/search/common/search.ts @@ -18,7 +18,7 @@ import { Event } from 'vs/base/common/event'; import { relative } from 'vs/base/common/path'; export const VIEWLET_ID = 'workbench.view.search'; -export const PANEL_ID = 'workbench.view.search'; +export const PANEL_ID = 'workbench.panel.search'; export const VIEW_ID = 'workbench.view.search'; export const ISearchService = createDecorator('searchService'); From df419967a6d7333bb982df41b63b86a08aaf83e1 Mon Sep 17 00:00:00 2001 From: isidor Date: Tue, 7 Jan 2020 12:22:25 +0100 Subject: [PATCH 09/36] debug: unit tests --- .../contrib/debug/browser/baseDebugView.ts | 2 +- .../workbench/contrib/debug/common/debug.ts | 2 +- .../contrib/debug/common/debugModel.ts | 20 ++--- .../debug/test/browser/baseDebugView.test.ts | 12 ++- .../debug/test/browser/debugModel.test.ts | 86 +++++++++++++++++++ 5 files changed, 109 insertions(+), 13 deletions(-) diff --git a/src/vs/workbench/contrib/debug/browser/baseDebugView.ts b/src/vs/workbench/contrib/debug/browser/baseDebugView.ts index 9acef112583..c40e105ffef 100644 --- a/src/vs/workbench/contrib/debug/browser/baseDebugView.ts +++ b/src/vs/workbench/contrib/debug/browser/baseDebugView.ts @@ -42,7 +42,7 @@ export interface IVariableTemplateData { } export function renderViewTree(container: HTMLElement): HTMLElement { - const treeContainer = document.createElement('div'); + const treeContainer = $('.'); dom.addClass(treeContainer, 'debug-view-content'); container.appendChild(treeContainer); return treeContainer; diff --git a/src/vs/workbench/contrib/debug/common/debug.ts b/src/vs/workbench/contrib/debug/common/debug.ts index 1ee4a7e22ad..55b78e3a7d1 100644 --- a/src/vs/workbench/contrib/debug/common/debug.ts +++ b/src/vs/workbench/contrib/debug/common/debug.ts @@ -441,7 +441,7 @@ export interface IBreakpointsChangeEvent { added?: Array; removed?: Array; changed?: Array; - sessionOnly?: boolean; + sessionOnly: boolean; } // Debug configuration interfaces diff --git a/src/vs/workbench/contrib/debug/common/debugModel.ts b/src/vs/workbench/contrib/debug/common/debugModel.ts index 1fafbec02bf..809df51172a 100644 --- a/src/vs/workbench/contrib/debug/common/debugModel.ts +++ b/src/vs/workbench/contrib/debug/common/debugModel.ts @@ -1010,7 +1010,7 @@ export class DebugModel implements IDebugModel { this.sortAndDeDup(); if (fireEvent) { - this._onDidChangeBreakpoints.fire({ added: newBreakpoints }); + this._onDidChangeBreakpoints.fire({ added: newBreakpoints, sessionOnly: false }); } return newBreakpoints; @@ -1018,7 +1018,7 @@ export class DebugModel implements IDebugModel { removeBreakpoints(toRemove: IBreakpoint[]): void { this.breakpoints = this.breakpoints.filter(bp => !toRemove.some(toRemove => toRemove.getId() === bp.getId())); - this._onDidChangeBreakpoints.fire({ removed: toRemove }); + this._onDidChangeBreakpoints.fire({ removed: toRemove, sessionOnly: false }); } updateBreakpoints(data: Map): void { @@ -1031,7 +1031,7 @@ export class DebugModel implements IDebugModel { } }); this.sortAndDeDup(); - this._onDidChangeBreakpoints.fire({ changed: updated }); + this._onDidChangeBreakpoints.fire({ changed: updated, sessionOnly: false }); } setBreakpointSessionData(sessionId: string, capabilites: DebugProtocol.Capabilities, data: Map | undefined): void { @@ -1100,7 +1100,7 @@ export class DebugModel implements IDebugModel { this.breakpointsActivated = true; } - this._onDidChangeBreakpoints.fire({ changed: changed }); + this._onDidChangeBreakpoints.fire({ changed: changed, sessionOnly: false }); } } @@ -1129,13 +1129,13 @@ export class DebugModel implements IDebugModel { this.breakpointsActivated = true; } - this._onDidChangeBreakpoints.fire({ changed: changed }); + this._onDidChangeBreakpoints.fire({ changed: changed, sessionOnly: false }); } addFunctionBreakpoint(functionName: string, id?: string): IFunctionBreakpoint { const newFunctionBreakpoint = new FunctionBreakpoint(functionName, true, undefined, undefined, undefined, id); this.functionBreakpoints.push(newFunctionBreakpoint); - this._onDidChangeBreakpoints.fire({ added: [newFunctionBreakpoint] }); + this._onDidChangeBreakpoints.fire({ added: [newFunctionBreakpoint], sessionOnly: false }); return newFunctionBreakpoint; } @@ -1144,7 +1144,7 @@ export class DebugModel implements IDebugModel { const functionBreakpoint = this.functionBreakpoints.filter(fbp => fbp.getId() === id).pop(); if (functionBreakpoint) { functionBreakpoint.name = name; - this._onDidChangeBreakpoints.fire({ changed: [functionBreakpoint] }); + this._onDidChangeBreakpoints.fire({ changed: [functionBreakpoint], sessionOnly: false }); } } @@ -1157,13 +1157,13 @@ export class DebugModel implements IDebugModel { removed = this.functionBreakpoints; this.functionBreakpoints = []; } - this._onDidChangeBreakpoints.fire({ removed }); + this._onDidChangeBreakpoints.fire({ removed, sessionOnly: false }); } addDataBreakpoint(label: string, dataId: string, canPersist: boolean, accessTypes: DebugProtocol.DataBreakpointAccessType[] | undefined): void { const newDataBreakpoint = new DataBreakpoint(label, dataId, canPersist, true, undefined, undefined, undefined, accessTypes); this.dataBreakopints.push(newDataBreakpoint); - this._onDidChangeBreakpoints.fire({ added: [newDataBreakpoint] }); + this._onDidChangeBreakpoints.fire({ added: [newDataBreakpoint], sessionOnly: false }); } removeDataBreakpoints(id?: string): void { @@ -1175,7 +1175,7 @@ export class DebugModel implements IDebugModel { removed = this.dataBreakopints; this.dataBreakopints = []; } - this._onDidChangeBreakpoints.fire({ removed }); + this._onDidChangeBreakpoints.fire({ removed, sessionOnly: false }); } getWatchExpressions(): Expression[] { diff --git a/src/vs/workbench/contrib/debug/test/browser/baseDebugView.test.ts b/src/vs/workbench/contrib/debug/test/browser/baseDebugView.test.ts index 48e2c89f0bc..a913d17cb19 100644 --- a/src/vs/workbench/contrib/debug/test/browser/baseDebugView.test.ts +++ b/src/vs/workbench/contrib/debug/test/browser/baseDebugView.test.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import * as assert from 'assert'; -import { replaceWhitespace, renderExpressionValue, renderVariable } from 'vs/workbench/contrib/debug/browser/baseDebugView'; +import { replaceWhitespace, renderExpressionValue, renderVariable, renderViewTree } from 'vs/workbench/contrib/debug/browser/baseDebugView'; import * as dom from 'vs/base/browser/dom'; import { Expression, Variable, Scope, StackFrame, Thread } from 'vs/workbench/contrib/debug/common/debugModel'; import { MockSession } from 'vs/workbench/contrib/debug/test/common/mockDebug'; @@ -32,6 +32,16 @@ suite('Debug - Base Debug View', () => { assert.equal(replaceWhitespace('hey \r\t\n\t\t\n there'), 'hey \\r\\t\\n\\t\\t\\n there'); }); + test('render view tree', () => { + const container = $('.container'); + const treeContainer = renderViewTree(container); + + assert.equal(treeContainer.className, 'debug-view-content'); + assert.equal(container.childElementCount, 1); + assert.equal(container.firstChild, treeContainer); + assert.equal(treeContainer instanceof HTMLDivElement, true); + }); + test('render expression value', () => { let container = $('.container'); renderExpressionValue('render \n me', container, { showHover: true, preserveWhitespace: true }); diff --git a/src/vs/workbench/contrib/debug/test/browser/debugModel.test.ts b/src/vs/workbench/contrib/debug/test/browser/debugModel.test.ts index 977a5a0d861..57132235e76 100644 --- a/src/vs/workbench/contrib/debug/test/browser/debugModel.test.ts +++ b/src/vs/workbench/contrib/debug/test/browser/debugModel.test.ts @@ -16,6 +16,7 @@ import { IBreakpointUpdateData, IDebugSessionOptions } from 'vs/workbench/contri import { NullOpenerService } from 'vs/platform/opener/common/opener'; import { RawDebugSession } from 'vs/workbench/contrib/debug/browser/rawDebugSession'; import { timeout } from 'vs/base/common/async'; +import { dispose } from 'vs/base/common/lifecycle'; function createMockSession(model: DebugModel, name = 'mockSession', options?: IDebugSessionOptions): DebugSession { return new DebugSession({ resolved: { name, type: 'node', request: 'launch' }, unresolved: undefined }, undefined!, model, options, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!, NullOpenerService, undefined!); @@ -34,16 +35,40 @@ suite('Debug - Model', () => { test('breakpoints simple', () => { const modelUri = uri.file('/myfolder/myfile.js'); + let eventCount = 0; + + let toDispose = model.onDidChangeBreakpoints(e => { + eventCount++; + assert.equal(e?.added?.length, 2); + assert.equal(e?.sessionOnly, false); + assert.equal(e?.removed, undefined); + assert.equal(e?.changed, undefined); + + dispose(toDispose); + }); model.addBreakpoints(modelUri, [{ lineNumber: 5, enabled: true }, { lineNumber: 10, enabled: false }]); + assert.equal(eventCount, 1); assert.equal(model.areBreakpointsActivated(), true); assert.equal(model.getBreakpoints().length, 2); + toDispose = model.onDidChangeBreakpoints(e => { + eventCount++; + assert.equal(e?.added, undefined); + assert.equal(e?.sessionOnly, false); + assert.equal(e?.removed?.length, 2); + assert.equal(e?.changed, undefined); + + dispose(toDispose); + }); + model.removeBreakpoints(model.getBreakpoints()); + assert.equal(eventCount, 2); assert.equal(model.getBreakpoints().length, 0); }); test('breakpoints toggling', () => { const modelUri = uri.file('/myfolder/myfile.js'); + model.addBreakpoints(modelUri, [{ lineNumber: 5, enabled: true }, { lineNumber: 10, enabled: false }]); model.addBreakpoints(modelUri, [{ lineNumber: 12, enabled: true, condition: 'fake condition' }]); assert.equal(model.getBreakpoints().length, 3); @@ -66,16 +91,33 @@ suite('Debug - Model', () => { model.addBreakpoints(modelUri2, [{ lineNumber: 1, enabled: true }, { lineNumber: 2, enabled: true }, { lineNumber: 3, enabled: false }]); assert.equal(model.getBreakpoints().length, 5); + assert.equal(model.getBreakpoints({ uri: modelUri1 }).length, 2); + assert.equal(model.getBreakpoints({ uri: modelUri2 }).length, 3); + assert.equal(model.getBreakpoints({ lineNumber: 5 }).length, 1); + assert.equal(model.getBreakpoints({ column: 5 }).length, 0); + + const bp = model.getBreakpoints()[0]; const update = new Map(); update.set(bp.getId(), { lineNumber: 100 }); + let eventFired = false; + model.onDidChangeBreakpoints(e => { + eventFired = true; + assert.equal(e?.added, undefined); + assert.equal(e?.removed, undefined); + assert.equal(e?.changed?.length, 1); + }); model.updateBreakpoints(update); + assert.equal(eventFired, true); assert.equal(bp.lineNumber, 100); + assert.equal(model.getBreakpoints({ enabledOnly: true }).length, 3); model.enableOrDisableAllBreakpoints(false); model.getBreakpoints().forEach(bp => { assert.equal(bp.enabled, false); }); + assert.equal(model.getBreakpoints({ enabledOnly: true }).length, 0); + model.setEnablement(bp, true); assert.equal(bp.enabled, true); @@ -150,6 +192,50 @@ suite('Debug - Model', () => { assert.equal(breakpoints[0].supported, true); }); + test('exception breakpoints', () => { + let eventCount = 0; + model.onDidChangeBreakpoints(() => eventCount++); + model.setExceptionBreakpoints([{ filter: 'uncaught', label: 'UNCAUGHT', default: true }]); + assert.equal(eventCount, 1); + let exceptionBreakpoints = model.getExceptionBreakpoints(); + assert.equal(exceptionBreakpoints.length, 1); + assert.equal(exceptionBreakpoints[0].filter, 'uncaught'); + assert.equal(exceptionBreakpoints[0].enabled, true); + + model.setExceptionBreakpoints([{ filter: 'uncaught', label: 'UNCAUGHT' }, { filter: 'caught', label: 'CAUGHT' }]); + assert.equal(eventCount, 2); + exceptionBreakpoints = model.getExceptionBreakpoints(); + assert.equal(exceptionBreakpoints.length, 2); + assert.equal(exceptionBreakpoints[0].filter, 'uncaught'); + assert.equal(exceptionBreakpoints[0].enabled, true); + assert.equal(exceptionBreakpoints[1].filter, 'caught'); + assert.equal(exceptionBreakpoints[1].label, 'CAUGHT'); + assert.equal(exceptionBreakpoints[1].enabled, false); + }); + + test('data breakpoints', () => { + let eventCount = 0; + model.onDidChangeBreakpoints(() => eventCount++); + + model.addDataBreakpoint('label', 'id', true, ['read']); + model.addDataBreakpoint('second', 'secondId', false, ['readWrite']); + const dataBreakpoints = model.getDataBreakpoints(); + assert.equal(dataBreakpoints[0].canPersist, true); + assert.equal(dataBreakpoints[0].dataId, 'id'); + assert.equal(dataBreakpoints[1].canPersist, false); + assert.equal(dataBreakpoints[1].description, 'second'); + + assert.equal(eventCount, 2); + + model.removeDataBreakpoints(dataBreakpoints[0].getId()); + assert.equal(eventCount, 3); + assert.equal(model.getDataBreakpoints().length, 1); + + model.removeDataBreakpoints(); + assert.equal(model.getDataBreakpoints().length, 0); + assert.equal(eventCount, 4); + }); + // Threads test('threads simple', () => { From edbe0c717362bf5923c99384e45cea81219e8d2a Mon Sep 17 00:00:00 2001 From: isidor Date: Tue, 7 Jan 2020 12:41:03 +0100 Subject: [PATCH 10/36] debug unit tests: better check for breakpoint events --- .../contrib/debug/browser/breakpointsView.ts | 19 +++---- .../debug/test/browser/debugModel.test.ts | 55 +++++++++++-------- 2 files changed, 42 insertions(+), 32 deletions(-) diff --git a/src/vs/workbench/contrib/debug/browser/breakpointsView.ts b/src/vs/workbench/contrib/debug/browser/breakpointsView.ts index 4e5c9e5de43..f92246d9c5d 100644 --- a/src/vs/workbench/contrib/debug/browser/breakpointsView.ts +++ b/src/vs/workbench/contrib/debug/browser/breakpointsView.ts @@ -7,7 +7,7 @@ import * as nls from 'vs/nls'; import * as resources from 'vs/base/common/resources'; import * as dom from 'vs/base/browser/dom'; import { IAction, Action } from 'vs/base/common/actions'; -import { IDebugService, IBreakpoint, CONTEXT_BREAKPOINTS_FOCUSED, State, DEBUG_SCHEME, IFunctionBreakpoint, IExceptionBreakpoint, IEnablement, BREAKPOINT_EDITOR_CONTRIBUTION_ID, IBreakpointEditorContribution } from 'vs/workbench/contrib/debug/common/debug'; +import { IDebugService, IBreakpoint, CONTEXT_BREAKPOINTS_FOCUSED, State, DEBUG_SCHEME, IFunctionBreakpoint, IExceptionBreakpoint, IEnablement, BREAKPOINT_EDITOR_CONTRIBUTION_ID, IBreakpointEditorContribution, IDebugModel } from 'vs/workbench/contrib/debug/common/debug'; import { ExceptionBreakpoint, FunctionBreakpoint, Breakpoint, DataBreakpoint } from 'vs/workbench/contrib/debug/common/debugModel'; import { AddFunctionBreakpointAction, ToggleBreakpointsActivatedAction, RemoveAllBreakpointsAction, RemoveBreakpointAction, EnableAllBreakpointsAction, DisableAllBreakpointsAction, ReapplyBreakpointsAction } from 'vs/workbench/contrib/debug/browser/debugActions'; import { IContextMenuService, IContextViewService } from 'vs/platform/contextview/browser/contextView'; @@ -45,9 +45,14 @@ function createCheckbox(): HTMLInputElement { return checkbox; } +const MAX_VISIBLE_BREAKPOINTS = 9; +function getExpandedBodySize(model: IDebugModel): number { + const length = model.getBreakpoints().length + model.getExceptionBreakpoints().length + model.getFunctionBreakpoints().length + model.getDataBreakpoints().length; + return Math.min(MAX_VISIBLE_BREAKPOINTS, length) * 22; +} + export class BreakpointsView extends ViewPane { - private static readonly MAX_VISIBLE_FILES = 9; private list!: WorkbenchList; private needsRefresh = false; @@ -65,7 +70,7 @@ export class BreakpointsView extends ViewPane { ) { super({ ...(options as IViewPaneOptions), ariaHeaderLabel: nls.localize('breakpointsSection', "Breakpoints Section") }, keybindingService, contextMenuService, configurationService, contextKeyService); - this.minimumBodySize = this.maximumBodySize = this.getExpandedBodySize(); + this.minimumBodySize = this.maximumBodySize = getExpandedBodySize(this.debugService.getModel()); this._register(this.debugService.getModel().onDidChangeBreakpoints(() => this.onBreakpointsChange())); } @@ -215,7 +220,7 @@ export class BreakpointsView extends ViewPane { private onBreakpointsChange(): void { if (this.isBodyVisible()) { - this.minimumBodySize = this.getExpandedBodySize(); + this.minimumBodySize = getExpandedBodySize(this.debugService.getModel()); if (this.maximumBodySize < Number.POSITIVE_INFINITY) { this.maximumBodySize = this.minimumBodySize; } @@ -234,12 +239,6 @@ export class BreakpointsView extends ViewPane { return elements; } - - private getExpandedBodySize(): number { - const model = this.debugService.getModel(); - const length = model.getBreakpoints().length + model.getExceptionBreakpoints().length + model.getFunctionBreakpoints().length + model.getDataBreakpoints().length; - return Math.min(BreakpointsView.MAX_VISIBLE_FILES, length) * 22; - } } class BreakpointsDelegate implements IListVirtualDelegate { diff --git a/src/vs/workbench/contrib/debug/test/browser/debugModel.test.ts b/src/vs/workbench/contrib/debug/test/browser/debugModel.test.ts index 57132235e76..ad4dd271c78 100644 --- a/src/vs/workbench/contrib/debug/test/browser/debugModel.test.ts +++ b/src/vs/workbench/contrib/debug/test/browser/debugModel.test.ts @@ -6,13 +6,13 @@ import * as assert from 'assert'; import { URI as uri } from 'vs/base/common/uri'; import severity from 'vs/base/common/severity'; -import { DebugModel, Expression, StackFrame, Thread } from 'vs/workbench/contrib/debug/common/debugModel'; +import { DebugModel, Expression, StackFrame, Thread, Breakpoint } from 'vs/workbench/contrib/debug/common/debugModel'; import * as sinon from 'sinon'; import { MockRawSession, MockDebugAdapter } from 'vs/workbench/contrib/debug/test/common/mockDebug'; import { Source } from 'vs/workbench/contrib/debug/common/debugSource'; import { DebugSession } from 'vs/workbench/contrib/debug/browser/debugSession'; import { SimpleReplElement, RawObjectReplElement, ReplEvaluationInput, ReplModel, ReplEvaluationResult } from 'vs/workbench/contrib/debug/common/replModel'; -import { IBreakpointUpdateData, IDebugSessionOptions } from 'vs/workbench/contrib/debug/common/debug'; +import { IBreakpointUpdateData, IDebugSessionOptions, IBreakpointData } from 'vs/workbench/contrib/debug/common/debug'; import { NullOpenerService } from 'vs/platform/opener/common/opener'; import { RawDebugSession } from 'vs/workbench/contrib/debug/browser/rawDebugSession'; import { timeout } from 'vs/base/common/async'; @@ -22,6 +22,26 @@ function createMockSession(model: DebugModel, name = 'mockSession', options?: ID return new DebugSession({ resolved: { name, type: 'node', request: 'launch' }, unresolved: undefined }, undefined!, model, options, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!, NullOpenerService, undefined!); } +function addBreakpointsAndCheckEvents(model: DebugModel, uri: uri, data: IBreakpointData[]): void { + let eventCount = 0; + const toDispose = model.onDidChangeBreakpoints(e => { + assert.equal(e?.sessionOnly, false); + assert.equal(e?.changed, undefined); + assert.equal(e?.removed, undefined); + const added = e?.added; + assert.notEqual(added, undefined); + assert.equal(added!.length, data.length); + eventCount++; + dispose(toDispose); + for (let i = 0; i < data.length; i++) { + assert.equal(e!.added![i] instanceof Breakpoint, true); + assert.equal((e!.added![i] as Breakpoint).lineNumber, data[i].lineNumber); + } + }); + model.addBreakpoints(uri, data); + assert.equal(eventCount, 1); +} + suite('Debug - Model', () => { let model: DebugModel; let rawSession: MockRawSession; @@ -35,23 +55,13 @@ suite('Debug - Model', () => { test('breakpoints simple', () => { const modelUri = uri.file('/myfolder/myfile.js'); - let eventCount = 0; - let toDispose = model.onDidChangeBreakpoints(e => { - eventCount++; - assert.equal(e?.added?.length, 2); - assert.equal(e?.sessionOnly, false); - assert.equal(e?.removed, undefined); - assert.equal(e?.changed, undefined); - - dispose(toDispose); - }); - model.addBreakpoints(modelUri, [{ lineNumber: 5, enabled: true }, { lineNumber: 10, enabled: false }]); - assert.equal(eventCount, 1); + addBreakpointsAndCheckEvents(model, modelUri, [{ lineNumber: 5, enabled: true }, { lineNumber: 10, enabled: false }]); assert.equal(model.areBreakpointsActivated(), true); assert.equal(model.getBreakpoints().length, 2); - toDispose = model.onDidChangeBreakpoints(e => { + let eventCount = 0; + const toDispose = model.onDidChangeBreakpoints(e => { eventCount++; assert.equal(e?.added, undefined); assert.equal(e?.sessionOnly, false); @@ -62,15 +72,15 @@ suite('Debug - Model', () => { }); model.removeBreakpoints(model.getBreakpoints()); - assert.equal(eventCount, 2); + assert.equal(eventCount, 1); assert.equal(model.getBreakpoints().length, 0); }); test('breakpoints toggling', () => { const modelUri = uri.file('/myfolder/myfile.js'); - model.addBreakpoints(modelUri, [{ lineNumber: 5, enabled: true }, { lineNumber: 10, enabled: false }]); - model.addBreakpoints(modelUri, [{ lineNumber: 12, enabled: true, condition: 'fake condition' }]); + addBreakpointsAndCheckEvents(model, modelUri, [{ lineNumber: 5, enabled: true }, { lineNumber: 10, enabled: false }]); + addBreakpointsAndCheckEvents(model, modelUri, [{ lineNumber: 12, enabled: true, condition: 'fake condition' }]); assert.equal(model.getBreakpoints().length, 3); const bp = model.getBreakpoints().pop(); if (bp) { @@ -87,8 +97,8 @@ suite('Debug - Model', () => { test('breakpoints two files', () => { const modelUri1 = uri.file('/myfolder/my file first.js'); const modelUri2 = uri.file('/secondfolder/second/second file.js'); - model.addBreakpoints(modelUri1, [{ lineNumber: 5, enabled: true }, { lineNumber: 10, enabled: false }]); - model.addBreakpoints(modelUri2, [{ lineNumber: 1, enabled: true }, { lineNumber: 2, enabled: true }, { lineNumber: 3, enabled: false }]); + addBreakpointsAndCheckEvents(model, modelUri1, [{ lineNumber: 5, enabled: true }, { lineNumber: 10, enabled: false }]); + addBreakpointsAndCheckEvents(model, modelUri2, [{ lineNumber: 1, enabled: true }, { lineNumber: 2, enabled: true }, { lineNumber: 3, enabled: false }]); assert.equal(model.getBreakpoints().length, 5); assert.equal(model.getBreakpoints({ uri: modelUri1 }).length, 2); @@ -127,7 +137,7 @@ suite('Debug - Model', () => { test('breakpoints conditions', () => { const modelUri1 = uri.file('/myfolder/my file first.js'); - model.addBreakpoints(modelUri1, [{ lineNumber: 5, condition: 'i < 5', hitCondition: '17' }, { lineNumber: 10, condition: 'j < 3' }]); + addBreakpointsAndCheckEvents(model, modelUri1, [{ lineNumber: 5, condition: 'i < 5', hitCondition: '17' }, { lineNumber: 10, condition: 'j < 3' }]); const breakpoints = model.getBreakpoints(); assert.equal(breakpoints[0].condition, 'i < 5'); @@ -156,7 +166,8 @@ suite('Debug - Model', () => { test('breakpoints multiple sessions', () => { const modelUri = uri.file('/myfolder/myfile.js'); - const breakpoints = model.addBreakpoints(modelUri, [{ lineNumber: 5, enabled: true, condition: 'x > 5' }, { lineNumber: 10, enabled: false }]); + addBreakpointsAndCheckEvents(model, modelUri, [{ lineNumber: 5, enabled: true, condition: 'x > 5' }, { lineNumber: 10, enabled: false }]); + const breakpoints = model.getBreakpoints(); const session = createMockSession(model); const data = new Map(); From e5d2d68c90a93750f13bd48baa898f8678267ecf Mon Sep 17 00:00:00 2001 From: isidor Date: Tue, 7 Jan 2020 12:57:17 +0100 Subject: [PATCH 11/36] debug: fix tests --- .../workbench/contrib/debug/test/browser/debugModel.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/vs/workbench/contrib/debug/test/browser/debugModel.test.ts b/src/vs/workbench/contrib/debug/test/browser/debugModel.test.ts index ad4dd271c78..98fb2868af2 100644 --- a/src/vs/workbench/contrib/debug/test/browser/debugModel.test.ts +++ b/src/vs/workbench/contrib/debug/test/browser/debugModel.test.ts @@ -106,16 +106,16 @@ suite('Debug - Model', () => { assert.equal(model.getBreakpoints({ lineNumber: 5 }).length, 1); assert.equal(model.getBreakpoints({ column: 5 }).length, 0); - const bp = model.getBreakpoints()[0]; const update = new Map(); update.set(bp.getId(), { lineNumber: 100 }); let eventFired = false; - model.onDidChangeBreakpoints(e => { + const toDispose = model.onDidChangeBreakpoints(e => { eventFired = true; assert.equal(e?.added, undefined); assert.equal(e?.removed, undefined); assert.equal(e?.changed?.length, 1); + dispose(toDispose); }); model.updateBreakpoints(update); assert.equal(eventFired, true); From 4fa0c5481ae0c9df1083b4e4ee5f40d4db3f3fab Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Tue, 7 Jan 2020 13:48:34 +0100 Subject: [PATCH 12/36] parse using jsonc parser --- src/vs/platform/userDataSync/common/globalStateSync.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/vs/platform/userDataSync/common/globalStateSync.ts b/src/vs/platform/userDataSync/common/globalStateSync.ts index 0e711eec298..ff5ce0d5ae9 100644 --- a/src/vs/platform/userDataSync/common/globalStateSync.ts +++ b/src/vs/platform/userDataSync/common/globalStateSync.ts @@ -15,6 +15,7 @@ import { IStringDictionary } from 'vs/base/common/collections'; import { edit } from 'vs/platform/userDataSync/common/content'; import { merge } from 'vs/platform/userDataSync/common/globalStateMerge'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; +import { parse } from 'vs/base/common/json'; const argvProperties: string[] = ['locale']; @@ -121,7 +122,7 @@ export class GlobalStateSynchroniser extends Disposable implements ISynchroniser const storage: IStringDictionary = {}; try { const content = await this.fileService.readFile(this.environmentService.argvResource); - const argvValue: IStringDictionary = JSON.parse(content.value.toString()); + const argvValue: IStringDictionary = parse(content.value.toString()); for (const argvProperty of argvProperties) { if (argvValue[argvProperty] !== undefined) { argv[argvProperty] = argvValue[argvProperty]; From b3cc42c2b4c3bf7d30d4d5835db1b3a3659bb5ab Mon Sep 17 00:00:00 2001 From: isidor Date: Tue, 7 Jan 2020 14:46:53 +0100 Subject: [PATCH 13/36] debug: seperate breakpoints.test.ts into its own file --- .../browser/breakpointEditorContribution.ts | 14 +- .../contrib/debug/browser/breakpointsView.ts | 21 +- .../debug/test/browser/breakpoints.test.ts | 246 ++++++++++++++++++ .../debug/test/browser/debugModel.test.ts | 222 +--------------- 4 files changed, 265 insertions(+), 238 deletions(-) create mode 100644 src/vs/workbench/contrib/debug/test/browser/breakpoints.test.ts diff --git a/src/vs/workbench/contrib/debug/browser/breakpointEditorContribution.ts b/src/vs/workbench/contrib/debug/browser/breakpointEditorContribution.ts index 6bd2606ee28..d67cd961c1b 100644 --- a/src/vs/workbench/contrib/debug/browser/breakpointEditorContribution.ts +++ b/src/vs/workbench/contrib/debug/browser/breakpointEditorContribution.ts @@ -17,7 +17,7 @@ import { IInstantiationService } from 'vs/platform/instantiation/common/instanti import { IContextKeyService, IContextKey } from 'vs/platform/contextkey/common/contextkey'; import { IContextMenuService } from 'vs/platform/contextview/browser/contextView'; import { RemoveBreakpointAction } from 'vs/workbench/contrib/debug/browser/debugActions'; -import { IDebugService, IBreakpoint, CONTEXT_BREAKPOINT_WIDGET_VISIBLE, BreakpointWidgetContext, BREAKPOINT_EDITOR_CONTRIBUTION_ID, IBreakpointEditorContribution, IBreakpointUpdateData, IDebugConfiguration } from 'vs/workbench/contrib/debug/common/debug'; +import { IDebugService, IBreakpoint, CONTEXT_BREAKPOINT_WIDGET_VISIBLE, BreakpointWidgetContext, BREAKPOINT_EDITOR_CONTRIBUTION_ID, IBreakpointEditorContribution, IBreakpointUpdateData, IDebugConfiguration, State } from 'vs/workbench/contrib/debug/common/debug'; import { IMarginData } from 'vs/editor/browser/controller/mouseTarget'; import { ContextSubMenu } from 'vs/base/browser/contextmenu'; import { IDialogService } from 'vs/platform/dialogs/common/dialogs'; @@ -51,7 +51,7 @@ const breakpointHelperDecoration: IModelDecorationOptions = { stickiness: TrackedRangeStickiness.NeverGrowsWhenTypingAtEdges }; -function createBreakpointDecorations(model: ITextModel, breakpoints: ReadonlyArray, debugService: IDebugService, debugSettings: IDebugConfiguration): { range: Range; options: IModelDecorationOptions; }[] { +function createBreakpointDecorations(model: ITextModel, breakpoints: ReadonlyArray, state: State, breakpointsActivated: boolean, debugSettings: IDebugConfiguration): { range: Range; options: IModelDecorationOptions; }[] { const result: { range: Range; options: IModelDecorationOptions; }[] = []; breakpoints.forEach((breakpoint) => { if (breakpoint.lineNumber > model.getLineCount()) { @@ -64,7 +64,7 @@ function createBreakpointDecorations(model: ITextModel, breakpoints: ReadonlyArr ); result.push({ - options: getBreakpointDecorationOptions(model, breakpoint, debugService, debugSettings), + options: getBreakpointDecorationOptions(model, breakpoint, state, breakpointsActivated, debugSettings), range }); }); @@ -72,8 +72,8 @@ function createBreakpointDecorations(model: ITextModel, breakpoints: ReadonlyArr return result; } -function getBreakpointDecorationOptions(model: ITextModel, breakpoint: IBreakpoint, debugService: IDebugService, debugSettings: IDebugConfiguration): IModelDecorationOptions { - const { className, message } = getBreakpointMessageAndClassName(debugService, breakpoint); +function getBreakpointDecorationOptions(model: ITextModel, breakpoint: IBreakpoint, state: State, breakpointsActivated: boolean, debugSettings: IDebugConfiguration): IModelDecorationOptions { + const { className, message } = getBreakpointMessageAndClassName(state, breakpointsActivated, breakpoint); let glyphMarginHoverMessage: MarkdownString | undefined; if (message) { @@ -403,7 +403,7 @@ class BreakpointEditorContribution implements IBreakpointEditorContribution { const model = activeCodeEditor.getModel(); const breakpoints = this.debugService.getModel().getBreakpoints({ uri: model.uri }); const debugSettings = this.configurationService.getValue('debug'); - const desiredBreakpointDecorations = createBreakpointDecorations(model, breakpoints, this.debugService, debugSettings); + const desiredBreakpointDecorations = createBreakpointDecorations(model, breakpoints, this.debugService.state, this.debugService.getModel().areBreakpointsActivated(), debugSettings); try { this.ignoreDecorationsChangedEvent = true; @@ -446,7 +446,7 @@ class BreakpointEditorContribution implements IBreakpointEditorContribution { // Candidate decoration has a breakpoint attached when a breakpoint is already at that location and we did not yet set a decoration there // In practice this happens for the first breakpoint that was set on a line // We could have also rendered this first decoration as part of desiredBreakpointDecorations however at that moment we have no location information - const cssClass = candidate.breakpoint ? getBreakpointMessageAndClassName(this.debugService, candidate.breakpoint).className : 'codicon-debug-breakpoint-disabled'; + const cssClass = candidate.breakpoint ? getBreakpointMessageAndClassName(this.debugService.state, this.debugService.getModel().areBreakpointsActivated(), candidate.breakpoint).className : 'codicon-debug-breakpoint-disabled'; const contextMenuActions = () => this.getContextMenuActions(candidate.breakpoint ? [candidate.breakpoint] : [], activeCodeEditor.getModel().uri, candidate.range.startLineNumber, candidate.range.startColumn); const inlineWidget = new InlineBreakpointWidget(activeCodeEditor, decorationId, cssClass, candidate.breakpoint, this.debugService, this.contextMenuService, contextMenuActions); diff --git a/src/vs/workbench/contrib/debug/browser/breakpointsView.ts b/src/vs/workbench/contrib/debug/browser/breakpointsView.ts index f92246d9c5d..efb58d76746 100644 --- a/src/vs/workbench/contrib/debug/browser/breakpointsView.ts +++ b/src/vs/workbench/contrib/debug/browser/breakpointsView.ts @@ -46,7 +46,7 @@ function createCheckbox(): HTMLInputElement { } const MAX_VISIBLE_BREAKPOINTS = 9; -function getExpandedBodySize(model: IDebugModel): number { +export function getExpandedBodySize(model: IDebugModel): number { const length = model.getBreakpoints().length + model.getExceptionBreakpoints().length + model.getFunctionBreakpoints().length + model.getDataBreakpoints().length; return Math.min(MAX_VISIBLE_BREAKPOINTS, length) * 22; } @@ -350,7 +350,7 @@ class BreakpointsRenderer implements IListRenderer { + assert.equal(e?.sessionOnly, false); + assert.equal(e?.changed, undefined); + assert.equal(e?.removed, undefined); + const added = e?.added; + assert.notEqual(added, undefined); + assert.equal(added!.length, data.length); + eventCount++; + dispose(toDispose); + for (let i = 0; i < data.length; i++) { + assert.equal(e!.added![i] instanceof Breakpoint, true); + assert.equal((e!.added![i] as Breakpoint).lineNumber, data[i].lineNumber); + } + }); + model.addBreakpoints(uri, data); + assert.equal(eventCount, 1); +} + +suite('Debug - Breakpoints', () => { + let model: DebugModel; + + setup(() => { + model = new DebugModel([], [], [], [], [], { isDirty: (e: any) => false }); + }); + + // Breakpoints + + test('simple', () => { + const modelUri = uri.file('/myfolder/myfile.js'); + + addBreakpointsAndCheckEvents(model, modelUri, [{ lineNumber: 5, enabled: true }, { lineNumber: 10, enabled: false }]); + assert.equal(model.areBreakpointsActivated(), true); + assert.equal(model.getBreakpoints().length, 2); + + let eventCount = 0; + const toDispose = model.onDidChangeBreakpoints(e => { + eventCount++; + assert.equal(e?.added, undefined); + assert.equal(e?.sessionOnly, false); + assert.equal(e?.removed?.length, 2); + assert.equal(e?.changed, undefined); + + dispose(toDispose); + }); + + model.removeBreakpoints(model.getBreakpoints()); + assert.equal(eventCount, 1); + assert.equal(model.getBreakpoints().length, 0); + }); + + test('toggling', () => { + const modelUri = uri.file('/myfolder/myfile.js'); + + addBreakpointsAndCheckEvents(model, modelUri, [{ lineNumber: 5, enabled: true }, { lineNumber: 10, enabled: false }]); + addBreakpointsAndCheckEvents(model, modelUri, [{ lineNumber: 12, enabled: true, condition: 'fake condition' }]); + assert.equal(model.getBreakpoints().length, 3); + const bp = model.getBreakpoints().pop(); + if (bp) { + model.removeBreakpoints([bp]); + } + assert.equal(model.getBreakpoints().length, 2); + + model.setBreakpointsActivated(false); + assert.equal(model.areBreakpointsActivated(), false); + model.setBreakpointsActivated(true); + assert.equal(model.areBreakpointsActivated(), true); + }); + + test('two files', () => { + const modelUri1 = uri.file('/myfolder/my file first.js'); + const modelUri2 = uri.file('/secondfolder/second/second file.js'); + addBreakpointsAndCheckEvents(model, modelUri1, [{ lineNumber: 5, enabled: true }, { lineNumber: 10, enabled: false }]); + assert.equal(getExpandedBodySize(model), 44); + + addBreakpointsAndCheckEvents(model, modelUri2, [{ lineNumber: 1, enabled: true }, { lineNumber: 2, enabled: true }, { lineNumber: 3, enabled: false }]); + assert.equal(getExpandedBodySize(model), 110); + + assert.equal(model.getBreakpoints().length, 5); + assert.equal(model.getBreakpoints({ uri: modelUri1 }).length, 2); + assert.equal(model.getBreakpoints({ uri: modelUri2 }).length, 3); + assert.equal(model.getBreakpoints({ lineNumber: 5 }).length, 1); + assert.equal(model.getBreakpoints({ column: 5 }).length, 0); + + const bp = model.getBreakpoints()[0]; + const update = new Map(); + update.set(bp.getId(), { lineNumber: 100 }); + let eventFired = false; + const toDispose = model.onDidChangeBreakpoints(e => { + eventFired = true; + assert.equal(e?.added, undefined); + assert.equal(e?.removed, undefined); + assert.equal(e?.changed?.length, 1); + dispose(toDispose); + }); + model.updateBreakpoints(update); + assert.equal(eventFired, true); + assert.equal(bp.lineNumber, 100); + + assert.equal(model.getBreakpoints({ enabledOnly: true }).length, 3); + model.enableOrDisableAllBreakpoints(false); + model.getBreakpoints().forEach(bp => { + assert.equal(bp.enabled, false); + }); + assert.equal(model.getBreakpoints({ enabledOnly: true }).length, 0); + + model.setEnablement(bp, true); + assert.equal(bp.enabled, true); + + model.removeBreakpoints(model.getBreakpoints({ uri: modelUri1 })); + assert.equal(getExpandedBodySize(model), 66); + + assert.equal(model.getBreakpoints().length, 3); + }); + + test('conditions', () => { + const modelUri1 = uri.file('/myfolder/my file first.js'); + addBreakpointsAndCheckEvents(model, modelUri1, [{ lineNumber: 5, condition: 'i < 5', hitCondition: '17' }, { lineNumber: 10, condition: 'j < 3' }]); + const breakpoints = model.getBreakpoints(); + + assert.equal(breakpoints[0].condition, 'i < 5'); + assert.equal(breakpoints[0].hitCondition, '17'); + assert.equal(breakpoints[1].condition, 'j < 3'); + assert.equal(!!breakpoints[1].hitCondition, false); + + assert.equal(model.getBreakpoints().length, 2); + model.removeBreakpoints(model.getBreakpoints()); + assert.equal(model.getBreakpoints().length, 0); + }); + + test('function breakpoints', () => { + model.addFunctionBreakpoint('foo', '1'); + model.addFunctionBreakpoint('bar', '2'); + model.renameFunctionBreakpoint('1', 'fooUpdated'); + model.renameFunctionBreakpoint('2', 'barUpdated'); + + const functionBps = model.getFunctionBreakpoints(); + assert.equal(functionBps[0].name, 'fooUpdated'); + assert.equal(functionBps[1].name, 'barUpdated'); + + model.removeFunctionBreakpoints(); + assert.equal(model.getFunctionBreakpoints().length, 0); + }); + + test('multiple sessions', () => { + const modelUri = uri.file('/myfolder/myfile.js'); + addBreakpointsAndCheckEvents(model, modelUri, [{ lineNumber: 5, enabled: true, condition: 'x > 5' }, { lineNumber: 10, enabled: false }]); + const breakpoints = model.getBreakpoints(); + const session = createMockSession(model); + const data = new Map(); + + assert.equal(breakpoints[0].lineNumber, 5); + assert.equal(breakpoints[1].lineNumber, 10); + + data.set(breakpoints[0].getId(), { verified: false, line: 10 }); + data.set(breakpoints[1].getId(), { verified: true, line: 50 }); + model.setBreakpointSessionData(session.getId(), {}, data); + assert.equal(breakpoints[0].lineNumber, 5); + assert.equal(breakpoints[1].lineNumber, 50); + + const session2 = createMockSession(model); + const data2 = new Map(); + data2.set(breakpoints[0].getId(), { verified: true, line: 100 }); + data2.set(breakpoints[1].getId(), { verified: true, line: 500 }); + model.setBreakpointSessionData(session2.getId(), {}, data2); + + // Breakpoint is verified only once, show that line + assert.equal(breakpoints[0].lineNumber, 100); + // Breakpoint is verified two times, show the original line + assert.equal(breakpoints[1].lineNumber, 10); + + model.setBreakpointSessionData(session.getId(), {}, undefined); + // No more double session verification + assert.equal(breakpoints[0].lineNumber, 100); + assert.equal(breakpoints[1].lineNumber, 500); + + assert.equal(breakpoints[0].supported, false); + const data3 = new Map(); + data3.set(breakpoints[0].getId(), { verified: true, line: 500 }); + model.setBreakpointSessionData(session2.getId(), { supportsConditionalBreakpoints: true }, data2); + assert.equal(breakpoints[0].supported, true); + }); + + test('exception breakpoints', () => { + let eventCount = 0; + model.onDidChangeBreakpoints(() => eventCount++); + model.setExceptionBreakpoints([{ filter: 'uncaught', label: 'UNCAUGHT', default: true }]); + assert.equal(eventCount, 1); + let exceptionBreakpoints = model.getExceptionBreakpoints(); + assert.equal(exceptionBreakpoints.length, 1); + assert.equal(exceptionBreakpoints[0].filter, 'uncaught'); + assert.equal(exceptionBreakpoints[0].enabled, true); + + model.setExceptionBreakpoints([{ filter: 'uncaught', label: 'UNCAUGHT' }, { filter: 'caught', label: 'CAUGHT' }]); + assert.equal(eventCount, 2); + exceptionBreakpoints = model.getExceptionBreakpoints(); + assert.equal(exceptionBreakpoints.length, 2); + assert.equal(exceptionBreakpoints[0].filter, 'uncaught'); + assert.equal(exceptionBreakpoints[0].enabled, true); + assert.equal(exceptionBreakpoints[1].filter, 'caught'); + assert.equal(exceptionBreakpoints[1].label, 'CAUGHT'); + assert.equal(exceptionBreakpoints[1].enabled, false); + }); + + test('data breakpoints', () => { + let eventCount = 0; + model.onDidChangeBreakpoints(() => eventCount++); + + model.addDataBreakpoint('label', 'id', true, ['read']); + model.addDataBreakpoint('second', 'secondId', false, ['readWrite']); + const dataBreakpoints = model.getDataBreakpoints(); + assert.equal(dataBreakpoints[0].canPersist, true); + assert.equal(dataBreakpoints[0].dataId, 'id'); + assert.equal(dataBreakpoints[1].canPersist, false); + assert.equal(dataBreakpoints[1].description, 'second'); + + assert.equal(eventCount, 2); + + model.removeDataBreakpoints(dataBreakpoints[0].getId()); + assert.equal(eventCount, 3); + assert.equal(model.getDataBreakpoints().length, 1); + + model.removeDataBreakpoints(); + assert.equal(model.getDataBreakpoints().length, 0); + assert.equal(eventCount, 4); + }); +}); diff --git a/src/vs/workbench/contrib/debug/test/browser/debugModel.test.ts b/src/vs/workbench/contrib/debug/test/browser/debugModel.test.ts index 98fb2868af2..9b1f22d67c7 100644 --- a/src/vs/workbench/contrib/debug/test/browser/debugModel.test.ts +++ b/src/vs/workbench/contrib/debug/test/browser/debugModel.test.ts @@ -4,44 +4,22 @@ *--------------------------------------------------------------------------------------------*/ import * as assert from 'assert'; -import { URI as uri } from 'vs/base/common/uri'; import severity from 'vs/base/common/severity'; -import { DebugModel, Expression, StackFrame, Thread, Breakpoint } from 'vs/workbench/contrib/debug/common/debugModel'; +import { DebugModel, Expression, StackFrame, Thread } from 'vs/workbench/contrib/debug/common/debugModel'; import * as sinon from 'sinon'; import { MockRawSession, MockDebugAdapter } from 'vs/workbench/contrib/debug/test/common/mockDebug'; import { Source } from 'vs/workbench/contrib/debug/common/debugSource'; import { DebugSession } from 'vs/workbench/contrib/debug/browser/debugSession'; import { SimpleReplElement, RawObjectReplElement, ReplEvaluationInput, ReplModel, ReplEvaluationResult } from 'vs/workbench/contrib/debug/common/replModel'; -import { IBreakpointUpdateData, IDebugSessionOptions, IBreakpointData } from 'vs/workbench/contrib/debug/common/debug'; +import { IDebugSessionOptions } from 'vs/workbench/contrib/debug/common/debug'; import { NullOpenerService } from 'vs/platform/opener/common/opener'; import { RawDebugSession } from 'vs/workbench/contrib/debug/browser/rawDebugSession'; import { timeout } from 'vs/base/common/async'; -import { dispose } from 'vs/base/common/lifecycle'; function createMockSession(model: DebugModel, name = 'mockSession', options?: IDebugSessionOptions): DebugSession { return new DebugSession({ resolved: { name, type: 'node', request: 'launch' }, unresolved: undefined }, undefined!, model, options, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!, NullOpenerService, undefined!); } -function addBreakpointsAndCheckEvents(model: DebugModel, uri: uri, data: IBreakpointData[]): void { - let eventCount = 0; - const toDispose = model.onDidChangeBreakpoints(e => { - assert.equal(e?.sessionOnly, false); - assert.equal(e?.changed, undefined); - assert.equal(e?.removed, undefined); - const added = e?.added; - assert.notEqual(added, undefined); - assert.equal(added!.length, data.length); - eventCount++; - dispose(toDispose); - for (let i = 0; i < data.length; i++) { - assert.equal(e!.added![i] instanceof Breakpoint, true); - assert.equal((e!.added![i] as Breakpoint).lineNumber, data[i].lineNumber); - } - }); - model.addBreakpoints(uri, data); - assert.equal(eventCount, 1); -} - suite('Debug - Model', () => { let model: DebugModel; let rawSession: MockRawSession; @@ -51,202 +29,6 @@ suite('Debug - Model', () => { rawSession = new MockRawSession(); }); - // Breakpoints - - test('breakpoints simple', () => { - const modelUri = uri.file('/myfolder/myfile.js'); - - addBreakpointsAndCheckEvents(model, modelUri, [{ lineNumber: 5, enabled: true }, { lineNumber: 10, enabled: false }]); - assert.equal(model.areBreakpointsActivated(), true); - assert.equal(model.getBreakpoints().length, 2); - - let eventCount = 0; - const toDispose = model.onDidChangeBreakpoints(e => { - eventCount++; - assert.equal(e?.added, undefined); - assert.equal(e?.sessionOnly, false); - assert.equal(e?.removed?.length, 2); - assert.equal(e?.changed, undefined); - - dispose(toDispose); - }); - - model.removeBreakpoints(model.getBreakpoints()); - assert.equal(eventCount, 1); - assert.equal(model.getBreakpoints().length, 0); - }); - - test('breakpoints toggling', () => { - const modelUri = uri.file('/myfolder/myfile.js'); - - addBreakpointsAndCheckEvents(model, modelUri, [{ lineNumber: 5, enabled: true }, { lineNumber: 10, enabled: false }]); - addBreakpointsAndCheckEvents(model, modelUri, [{ lineNumber: 12, enabled: true, condition: 'fake condition' }]); - assert.equal(model.getBreakpoints().length, 3); - const bp = model.getBreakpoints().pop(); - if (bp) { - model.removeBreakpoints([bp]); - } - assert.equal(model.getBreakpoints().length, 2); - - model.setBreakpointsActivated(false); - assert.equal(model.areBreakpointsActivated(), false); - model.setBreakpointsActivated(true); - assert.equal(model.areBreakpointsActivated(), true); - }); - - test('breakpoints two files', () => { - const modelUri1 = uri.file('/myfolder/my file first.js'); - const modelUri2 = uri.file('/secondfolder/second/second file.js'); - addBreakpointsAndCheckEvents(model, modelUri1, [{ lineNumber: 5, enabled: true }, { lineNumber: 10, enabled: false }]); - addBreakpointsAndCheckEvents(model, modelUri2, [{ lineNumber: 1, enabled: true }, { lineNumber: 2, enabled: true }, { lineNumber: 3, enabled: false }]); - - assert.equal(model.getBreakpoints().length, 5); - assert.equal(model.getBreakpoints({ uri: modelUri1 }).length, 2); - assert.equal(model.getBreakpoints({ uri: modelUri2 }).length, 3); - assert.equal(model.getBreakpoints({ lineNumber: 5 }).length, 1); - assert.equal(model.getBreakpoints({ column: 5 }).length, 0); - - const bp = model.getBreakpoints()[0]; - const update = new Map(); - update.set(bp.getId(), { lineNumber: 100 }); - let eventFired = false; - const toDispose = model.onDidChangeBreakpoints(e => { - eventFired = true; - assert.equal(e?.added, undefined); - assert.equal(e?.removed, undefined); - assert.equal(e?.changed?.length, 1); - dispose(toDispose); - }); - model.updateBreakpoints(update); - assert.equal(eventFired, true); - assert.equal(bp.lineNumber, 100); - - assert.equal(model.getBreakpoints({ enabledOnly: true }).length, 3); - model.enableOrDisableAllBreakpoints(false); - model.getBreakpoints().forEach(bp => { - assert.equal(bp.enabled, false); - }); - assert.equal(model.getBreakpoints({ enabledOnly: true }).length, 0); - - model.setEnablement(bp, true); - assert.equal(bp.enabled, true); - - model.removeBreakpoints(model.getBreakpoints({ uri: modelUri1 })); - assert.equal(model.getBreakpoints().length, 3); - }); - - test('breakpoints conditions', () => { - const modelUri1 = uri.file('/myfolder/my file first.js'); - addBreakpointsAndCheckEvents(model, modelUri1, [{ lineNumber: 5, condition: 'i < 5', hitCondition: '17' }, { lineNumber: 10, condition: 'j < 3' }]); - const breakpoints = model.getBreakpoints(); - - assert.equal(breakpoints[0].condition, 'i < 5'); - assert.equal(breakpoints[0].hitCondition, '17'); - assert.equal(breakpoints[1].condition, 'j < 3'); - assert.equal(!!breakpoints[1].hitCondition, false); - - assert.equal(model.getBreakpoints().length, 2); - model.removeBreakpoints(model.getBreakpoints()); - assert.equal(model.getBreakpoints().length, 0); - }); - - test('function breakpoints', () => { - model.addFunctionBreakpoint('foo', '1'); - model.addFunctionBreakpoint('bar', '2'); - model.renameFunctionBreakpoint('1', 'fooUpdated'); - model.renameFunctionBreakpoint('2', 'barUpdated'); - - const functionBps = model.getFunctionBreakpoints(); - assert.equal(functionBps[0].name, 'fooUpdated'); - assert.equal(functionBps[1].name, 'barUpdated'); - - model.removeFunctionBreakpoints(); - assert.equal(model.getFunctionBreakpoints().length, 0); - }); - - test('breakpoints multiple sessions', () => { - const modelUri = uri.file('/myfolder/myfile.js'); - addBreakpointsAndCheckEvents(model, modelUri, [{ lineNumber: 5, enabled: true, condition: 'x > 5' }, { lineNumber: 10, enabled: false }]); - const breakpoints = model.getBreakpoints(); - const session = createMockSession(model); - const data = new Map(); - - assert.equal(breakpoints[0].lineNumber, 5); - assert.equal(breakpoints[1].lineNumber, 10); - - data.set(breakpoints[0].getId(), { verified: false, line: 10 }); - data.set(breakpoints[1].getId(), { verified: true, line: 50 }); - model.setBreakpointSessionData(session.getId(), {}, data); - assert.equal(breakpoints[0].lineNumber, 5); - assert.equal(breakpoints[1].lineNumber, 50); - - const session2 = createMockSession(model); - const data2 = new Map(); - data2.set(breakpoints[0].getId(), { verified: true, line: 100 }); - data2.set(breakpoints[1].getId(), { verified: true, line: 500 }); - model.setBreakpointSessionData(session2.getId(), {}, data2); - - // Breakpoint is verified only once, show that line - assert.equal(breakpoints[0].lineNumber, 100); - // Breakpoint is verified two times, show the original line - assert.equal(breakpoints[1].lineNumber, 10); - - model.setBreakpointSessionData(session.getId(), {}, undefined); - // No more double session verification - assert.equal(breakpoints[0].lineNumber, 100); - assert.equal(breakpoints[1].lineNumber, 500); - - assert.equal(breakpoints[0].supported, false); - const data3 = new Map(); - data3.set(breakpoints[0].getId(), { verified: true, line: 500 }); - model.setBreakpointSessionData(session2.getId(), { supportsConditionalBreakpoints: true }, data2); - assert.equal(breakpoints[0].supported, true); - }); - - test('exception breakpoints', () => { - let eventCount = 0; - model.onDidChangeBreakpoints(() => eventCount++); - model.setExceptionBreakpoints([{ filter: 'uncaught', label: 'UNCAUGHT', default: true }]); - assert.equal(eventCount, 1); - let exceptionBreakpoints = model.getExceptionBreakpoints(); - assert.equal(exceptionBreakpoints.length, 1); - assert.equal(exceptionBreakpoints[0].filter, 'uncaught'); - assert.equal(exceptionBreakpoints[0].enabled, true); - - model.setExceptionBreakpoints([{ filter: 'uncaught', label: 'UNCAUGHT' }, { filter: 'caught', label: 'CAUGHT' }]); - assert.equal(eventCount, 2); - exceptionBreakpoints = model.getExceptionBreakpoints(); - assert.equal(exceptionBreakpoints.length, 2); - assert.equal(exceptionBreakpoints[0].filter, 'uncaught'); - assert.equal(exceptionBreakpoints[0].enabled, true); - assert.equal(exceptionBreakpoints[1].filter, 'caught'); - assert.equal(exceptionBreakpoints[1].label, 'CAUGHT'); - assert.equal(exceptionBreakpoints[1].enabled, false); - }); - - test('data breakpoints', () => { - let eventCount = 0; - model.onDidChangeBreakpoints(() => eventCount++); - - model.addDataBreakpoint('label', 'id', true, ['read']); - model.addDataBreakpoint('second', 'secondId', false, ['readWrite']); - const dataBreakpoints = model.getDataBreakpoints(); - assert.equal(dataBreakpoints[0].canPersist, true); - assert.equal(dataBreakpoints[0].dataId, 'id'); - assert.equal(dataBreakpoints[1].canPersist, false); - assert.equal(dataBreakpoints[1].description, 'second'); - - assert.equal(eventCount, 2); - - model.removeDataBreakpoints(dataBreakpoints[0].getId()); - assert.equal(eventCount, 3); - assert.equal(model.getDataBreakpoints().length, 1); - - model.removeDataBreakpoints(); - assert.equal(model.getDataBreakpoints().length, 0); - assert.equal(eventCount, 4); - }); - // Threads test('threads simple', () => { From 9608852189774be3a85357a45b065ae02a8aaac4 Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Tue, 7 Jan 2020 15:02:40 +0100 Subject: [PATCH 14/36] #26707 Change resource to uri --- src/vs/vscode.proposed.d.ts | 2 +- .../workbench/api/common/extHostConfiguration.ts | 15 +++------------ 2 files changed, 4 insertions(+), 13 deletions(-) diff --git a/src/vs/vscode.proposed.d.ts b/src/vs/vscode.proposed.d.ts index 404896791d8..a2cbad3a365 100644 --- a/src/vs/vscode.proposed.d.ts +++ b/src/vs/vscode.proposed.d.ts @@ -1318,7 +1318,7 @@ declare module 'vscode' { //#region Language specific settings: https://github.com/microsoft/vscode/issues/26707 - export type ConfigurationScope = Uri | TextDocument | WorkspaceFolder | { resource: Uri, languageId: string }; + export type ConfigurationScope = Uri | TextDocument | WorkspaceFolder | { uri: Uri, languageId: string }; /** * An event describing the change in Configuration diff --git a/src/vs/workbench/api/common/extHostConfiguration.ts b/src/vs/workbench/api/common/extHostConfiguration.ts index 40bc2a63312..2319465e377 100644 --- a/src/vs/workbench/api/common/extHostConfiguration.ts +++ b/src/vs/workbench/api/common/extHostConfiguration.ts @@ -47,12 +47,6 @@ type ConfigurationInspect = { workspaceFolderLanguageValue?: T; }; -function isTextDocument(thing: any): thing is vscode.TextDocument { - return thing - && thing.uri instanceof URI - && (!thing.languageId || typeof thing.languageId === 'string'); -} - function isWorkspaceFolder(thing: any): thing is vscode.WorkspaceFolder { return thing && thing.uri instanceof URI @@ -64,9 +58,9 @@ function isUri(thing: any): thing is vscode.Uri { return thing instanceof URI; } -function isResourceLanguage(thing: any): thing is { resource: URI, languageId: string } { +function isResourceLanguage(thing: any): thing is { uri: URI, languageId: string } { return thing - && thing.resource instanceof URI + && thing.uri instanceof URI && (!thing.languageId || typeof thing.languageId === 'string'); } @@ -77,11 +71,8 @@ function scopeToOverrides(scope: vscode.ConfigurationScope | undefined | null): if (isWorkspaceFolder(scope)) { return { resource: scope.uri }; } - if (isTextDocument(scope)) { - return { resource: scope.uri, overrideIdentifier: scope.languageId }; - } if (isResourceLanguage(scope)) { - return scope; + return { resource: scope.uri, overrideIdentifier: scope.languageId }; } return undefined; } From a0b84fe4a9e29de6eb5787088f12656e14a2976e Mon Sep 17 00:00:00 2001 From: isidor Date: Tue, 7 Jan 2020 15:33:49 +0100 Subject: [PATCH 15/36] debug: getBreakpointMessageAndClassName tests --- .../contrib/debug/browser/breakpointsView.ts | 10 +-- .../debug/test/browser/breakpoints.test.ts | 70 ++++++++++++++++++- 2 files changed, 73 insertions(+), 7 deletions(-) diff --git a/src/vs/workbench/contrib/debug/browser/breakpointsView.ts b/src/vs/workbench/contrib/debug/browser/breakpointsView.ts index efb58d76746..3e402cfb207 100644 --- a/src/vs/workbench/contrib/debug/browser/breakpointsView.ts +++ b/src/vs/workbench/contrib/debug/browser/breakpointsView.ts @@ -7,7 +7,7 @@ import * as nls from 'vs/nls'; import * as resources from 'vs/base/common/resources'; import * as dom from 'vs/base/browser/dom'; import { IAction, Action } from 'vs/base/common/actions'; -import { IDebugService, IBreakpoint, CONTEXT_BREAKPOINTS_FOCUSED, State, DEBUG_SCHEME, IFunctionBreakpoint, IExceptionBreakpoint, IEnablement, BREAKPOINT_EDITOR_CONTRIBUTION_ID, IBreakpointEditorContribution, IDebugModel } from 'vs/workbench/contrib/debug/common/debug'; +import { IDebugService, IBreakpoint, CONTEXT_BREAKPOINTS_FOCUSED, State, DEBUG_SCHEME, IFunctionBreakpoint, IExceptionBreakpoint, IEnablement, BREAKPOINT_EDITOR_CONTRIBUTION_ID, IBreakpointEditorContribution, IDebugModel, IDataBreakpoint } from 'vs/workbench/contrib/debug/common/debug'; import { ExceptionBreakpoint, FunctionBreakpoint, Breakpoint, DataBreakpoint } from 'vs/workbench/contrib/debug/common/debugModel'; import { AddFunctionBreakpointAction, ToggleBreakpointsActivatedAction, RemoveAllBreakpointsAction, RemoveBreakpointAction, EnableAllBreakpointsAction, DisableAllBreakpointsAction, ReapplyBreakpointsAction } from 'vs/workbench/contrib/debug/browser/debugActions'; import { IContextMenuService, IContextViewService } from 'vs/platform/contextview/browser/contextView'; @@ -637,7 +637,7 @@ export function openBreakpointSource(breakpoint: IBreakpoint, sideBySide: boolea }, sideBySide ? SIDE_GROUP : ACTIVE_GROUP); } -export function getBreakpointMessageAndClassName(state: State, breakpointsActivated: boolean, breakpoint: IBreakpoint | FunctionBreakpoint | DataBreakpoint): { message?: string, className: string } { +export function getBreakpointMessageAndClassName(state: State, breakpointsActivated: boolean, breakpoint: IBreakpoint | IFunctionBreakpoint | IDataBreakpoint): { message?: string, className: string } { const debugActive = state === State.Running || state === State.Stopped; if (!breakpoint.enabled || !breakpointsActivated) { @@ -648,12 +648,12 @@ export function getBreakpointMessageAndClassName(state: State, breakpointsActiva } const appendMessage = (text: string): string => { - return !(breakpoint instanceof FunctionBreakpoint) && !(breakpoint instanceof DataBreakpoint) && breakpoint.message ? text.concat(', ' + breakpoint.message) : text; + return ('message' in breakpoint && breakpoint.message) ? text.concat(', ' + breakpoint.message) : text; }; if (debugActive && !breakpoint.verified) { return { className: breakpoint instanceof DataBreakpoint ? 'codicon-debug-breakpoint-data-unverified' : breakpoint instanceof FunctionBreakpoint ? 'codicon-debug-breakpoint-function-unverified' : breakpoint.logMessage ? 'codicon-debug-breakpoint-log-unverified' : 'codicon-debug-breakpoint-unverified', - message: breakpoint.message || (breakpoint.logMessage ? nls.localize('unverifiedLogpoint', "Unverified Logpoint") : nls.localize('unverifiedBreakopint', "Unverified Breakpoint")), + message: ('message' in breakpoint && breakpoint.message) ? breakpoint.message : (breakpoint.logMessage ? nls.localize('unverifiedLogpoint', "Unverified Logpoint") : nls.localize('unverifiedBreakopint', "Unverified Breakpoint")), }; } @@ -713,6 +713,6 @@ export function getBreakpointMessageAndClassName(state: State, breakpointsActiva return { className: 'codicon-debug-breakpoint', - message: breakpoint.message || nls.localize('breakpoint', "Breakpoint") + message: ('message' in breakpoint && breakpoint.message) ? breakpoint.message : nls.localize('breakpoint', "Breakpoint") }; } diff --git a/src/vs/workbench/contrib/debug/test/browser/breakpoints.test.ts b/src/vs/workbench/contrib/debug/test/browser/breakpoints.test.ts index 178766bdb9e..7aa6720b30c 100644 --- a/src/vs/workbench/contrib/debug/test/browser/breakpoints.test.ts +++ b/src/vs/workbench/contrib/debug/test/browser/breakpoints.test.ts @@ -8,9 +8,9 @@ import { URI as uri } from 'vs/base/common/uri'; import { DebugModel, Breakpoint } from 'vs/workbench/contrib/debug/common/debugModel'; import { DebugSession } from 'vs/workbench/contrib/debug/browser/debugSession'; import { NullOpenerService } from 'vs/platform/opener/common/opener'; -import { getExpandedBodySize } from 'vs/workbench/contrib/debug/browser/breakpointsView'; +import { getExpandedBodySize, getBreakpointMessageAndClassName } from 'vs/workbench/contrib/debug/browser/breakpointsView'; import { dispose } from 'vs/base/common/lifecycle'; -import { IBreakpointData, IDebugSessionOptions, IBreakpointUpdateData } from 'vs/workbench/contrib/debug/common/debug'; +import { IBreakpointData, IDebugSessionOptions, IBreakpointUpdateData, State } from 'vs/workbench/contrib/debug/common/debug'; function createMockSession(model: DebugModel, name = 'mockSession', options?: IDebugSessionOptions): DebugSession { return new DebugSession({ resolved: { name, type: 'node', request: 'launch' }, unresolved: undefined }, undefined!, model, options, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!, NullOpenerService, undefined!); @@ -243,4 +243,70 @@ suite('Debug - Breakpoints', () => { assert.equal(model.getDataBreakpoints().length, 0); assert.equal(eventCount, 4); }); + + test('message and class name', () => { + const modelUri = uri.file('/myfolder/my file first.js'); + addBreakpointsAndCheckEvents(model, modelUri, [ + { lineNumber: 5, enabled: true, condition: 'x > 5' }, + { lineNumber: 10, enabled: false }, + { lineNumber: 12, enabled: true, logMessage: 'hello' }, + { lineNumber: 15, enabled: true, hitCondition: '12' }, + { lineNumber: 500, enabled: true }, + ]); + const breakpoints = model.getBreakpoints(); + + let result = getBreakpointMessageAndClassName(State.Stopped, true, breakpoints[0]); + assert.equal(result.message, 'Expression: x > 5'); + assert.equal(result.className, 'codicon-debug-breakpoint-conditional'); + + result = getBreakpointMessageAndClassName(State.Stopped, true, breakpoints[1]); + assert.equal(result.message, 'Disabled Breakpoint'); + assert.equal(result.className, 'codicon-debug-breakpoint-disabled'); + + result = getBreakpointMessageAndClassName(State.Stopped, true, breakpoints[2]); + assert.equal(result.message, 'Log Message: hello'); + assert.equal(result.className, 'codicon-debug-breakpoint-log'); + + result = getBreakpointMessageAndClassName(State.Stopped, true, breakpoints[3]); + assert.equal(result.message, 'Hit Count: 12'); + assert.equal(result.className, 'codicon-debug-breakpoint-conditional'); + + result = getBreakpointMessageAndClassName(State.Stopped, true, breakpoints[4]); + assert.equal(result.message, 'Breakpoint'); + assert.equal(result.className, 'codicon-debug-breakpoint'); + + result = getBreakpointMessageAndClassName(State.Stopped, false, breakpoints[2]); + assert.equal(result.message, 'Disabled Logpoint'); + assert.equal(result.className, 'codicon-debug-breakpoint-log-disabled'); + + model.addDataBreakpoint('label', 'id', true, ['read']); + const dataBreakpoints = model.getDataBreakpoints(); + result = getBreakpointMessageAndClassName(State.Stopped, true, dataBreakpoints[0]); + assert.equal(result.message, 'Data Breakpoint'); + assert.equal(result.className, 'codicon-debug-breakpoint-data'); + + const functionBreakpoint = model.addFunctionBreakpoint('foo', '1'); + result = getBreakpointMessageAndClassName(State.Stopped, true, functionBreakpoint); + assert.equal(result.message, 'Function Breakpoint'); + assert.equal(result.className, 'codicon-debug-breakpoint-function'); + + const data = new Map(); + data.set(breakpoints[0].getId(), { verified: false, line: 10 }); + data.set(breakpoints[1].getId(), { verified: true, line: 50 }); + data.set(breakpoints[2].getId(), { verified: true, line: 50, message: 'world' }); + data.set(functionBreakpoint.getId(), { verified: true }); + model.setBreakpointSessionData('mocksessionid', { supportsFunctionBreakpoints: false, supportsDataBreakpoints: true, supportsLogPoints: true }, data); + + result = getBreakpointMessageAndClassName(State.Stopped, true, breakpoints[0]); + assert.equal(result.message, 'Unverified Breakpoint'); + assert.equal(result.className, 'codicon-debug-breakpoint-unverified'); + + result = getBreakpointMessageAndClassName(State.Stopped, true, functionBreakpoint); + assert.equal(result.message, 'Function breakpoints not supported by this debug type'); + assert.equal(result.className, 'codicon-debug-breakpoint-function-unverified'); + + result = getBreakpointMessageAndClassName(State.Stopped, true, breakpoints[2]); + assert.equal(result.message, 'Log Message: hello, world'); + assert.equal(result.className, 'codicon-debug-breakpoint-log'); + }); }); From 0a2380ac825d66f9b006059abb3c73110bfe0333 Mon Sep 17 00:00:00 2001 From: isidor Date: Tue, 7 Jan 2020 15:47:51 +0100 Subject: [PATCH 16/36] breakpoint widget: polish, use model.findPrevBracket --- .../contrib/debug/browser/breakpointWidget.ts | 70 ++++++++----------- 1 file changed, 31 insertions(+), 39 deletions(-) diff --git a/src/vs/workbench/contrib/debug/browser/breakpointWidget.ts b/src/vs/workbench/contrib/debug/browser/breakpointWidget.ts index f350be985e0..2100b5a8228 100644 --- a/src/vs/workbench/contrib/debug/browser/breakpointWidget.ts +++ b/src/vs/workbench/contrib/debug/browser/breakpointWidget.ts @@ -15,7 +15,7 @@ import { ZoneWidget } from 'vs/editor/contrib/zoneWidget/zoneWidget'; import { IContextViewService } from 'vs/platform/contextview/browser/contextView'; import { IDebugService, IBreakpoint, BreakpointWidgetContext as Context, CONTEXT_BREAKPOINT_WIDGET_VISIBLE, DEBUG_SCHEME, CONTEXT_IN_BREAKPOINT_WIDGET, IBreakpointUpdateData, IBreakpointEditorContribution, BREAKPOINT_EDITOR_CONTRIBUTION_ID } from 'vs/workbench/contrib/debug/common/debug'; import { attachSelectBoxStyler } from 'vs/platform/theme/common/styler'; -import { IThemeService } from 'vs/platform/theme/common/themeService'; +import { IThemeService, ITheme } from 'vs/platform/theme/common/themeService'; import { createDecorator, IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { IContextKeyService } from 'vs/platform/contextkey/common/contextkey'; import { ServicesAccessor, EditorCommand, registerEditorCommand } from 'vs/editor/browser/editorExtensions'; @@ -46,6 +46,34 @@ export interface IPrivateBreakpointWidgetService { } const DECORATION_KEY = 'breakpointwidgetdecoration'; +function isCurlyBracketOpen(input: IActiveCodeEditor): boolean { + const model = input.getModel(); + const prevBracket = model.findPrevBracket(input.getPosition()); + if (prevBracket && prevBracket.isOpen) { + return true; + } + + return false; +} + +function createDecorations(theme: ITheme, placeHolder: string): IDecorationOptions[] { + const transparentForeground = transparent(editorForeground, 0.4)(theme); + return [{ + range: { + startLineNumber: 0, + endLineNumber: 0, + startColumn: 0, + endColumn: 1 + }, + renderOptions: { + after: { + contentText: placeHolder, + color: transparentForeground ? transparentForeground.toString() : undefined + } + } + }]; +} + export class BreakpointWidget extends ZoneWidget implements IPrivateBreakpointWidgetService { _serviceBrand: undefined; @@ -197,7 +225,7 @@ export class BreakpointWidget extends ZoneWidget implements IPrivateBreakpointWi this.toDispose.push(model); const setDecorations = () => { const value = this.input.getModel().getValue(); - const decorations = !!value ? [] : this.createDecorations(); + const decorations = !!value ? [] : createDecorations(this.themeService.getTheme(), this.placeholder); this.input.setDecorations(DECORATION_KEY, decorations); }; this.input.getModel().onDidChangeContent(() => setDecorations()); @@ -207,7 +235,7 @@ export class BreakpointWidget extends ZoneWidget implements IPrivateBreakpointWi provideCompletionItems: (model: ITextModel, position: Position, _context: CompletionContext, token: CancellationToken): Promise => { let suggestionsPromise: Promise; const underlyingModel = this.editor.getModel(); - if (underlyingModel && (this.context === Context.CONDITION || this.context === Context.LOG_MESSAGE && this.isCurlyBracketOpen())) { + if (underlyingModel && (this.context === Context.CONDITION || (this.context === Context.LOG_MESSAGE && isCurlyBracketOpen(this.input)))) { suggestionsPromise = provideSuggestionItems(underlyingModel, new Position(this.lineNumber, 1), new CompletionOptions(undefined, new Set().add(CompletionItemKind.Snippet)), _context, token).then(suggestions => { let overwriteBefore = 0; @@ -260,42 +288,6 @@ export class BreakpointWidget extends ZoneWidget implements IPrivateBreakpointWi } } - private createDecorations(): IDecorationOptions[] { - const transparentForeground = transparent(editorForeground, 0.4)(this.themeService.getTheme()); - return [{ - range: { - startLineNumber: 0, - endLineNumber: 0, - startColumn: 0, - endColumn: 1 - }, - renderOptions: { - after: { - contentText: this.placeholder, - color: transparentForeground ? transparentForeground.toString() : undefined - } - } - }]; - } - - private isCurlyBracketOpen(): boolean { - const value = this.input.getModel().getValue(); - const position = this.input.getPosition(); - if (position) { - for (let i = position.column - 2; i >= 0; i--) { - if (value[i] === '{') { - return true; - } - - if (value[i] === '}') { - return false; - } - } - } - - return false; - } - close(success: boolean): void { if (success) { // if there is already a breakpoint on this location - remove it. From 8f95087a1d9f3d5bfe6c41108aabcdd89d781c67 Mon Sep 17 00:00:00 2001 From: Alex Ross Date: Tue, 7 Jan 2020 16:02:44 +0100 Subject: [PATCH 17/36] Add error for unforwardable port Fixes #88179 --- .../contrib/remote/browser/tunnelView.ts | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/vs/workbench/contrib/remote/browser/tunnelView.ts b/src/vs/workbench/contrib/remote/browser/tunnelView.ts index f2006794e8f..5af79a0bf68 100644 --- a/src/vs/workbench/contrib/remote/browser/tunnelView.ts +++ b/src/vs/workbench/contrib/remote/browser/tunnelView.ts @@ -37,6 +37,7 @@ import { IThemeService } from 'vs/platform/theme/common/themeService'; import { IKeyboardEvent } from 'vs/base/browser/keyboardEvent'; import { ViewPane, IViewPaneOptions } from 'vs/workbench/browser/parts/views/viewPaneContainer'; import { URI } from 'vs/base/common/uri'; +import { RemoteTunnel } from 'vs/platform/remote/common/tunnel'; export const forwardedPortsViewEnabled = new RawContextKey('forwardedPortsViewEnabled', false); @@ -608,17 +609,24 @@ namespace ForwardPortAction { return null; } + function error(notificationService: INotificationService, tunnel: RemoteTunnel | void, host: string, port: number) { + if (!tunnel) { + notificationService.error(nls.localize('remote.tunnel.forwardError', "Unable to forward {0}:{1}. The host may not be available.", host, port)); + } + } + export function inlineHandler(): ICommandHandler { return async (accessor, arg) => { const remoteExplorerService = accessor.get(IRemoteExplorerService); + const notificationService = accessor.get(INotificationService); if (arg instanceof TunnelItem) { - remoteExplorerService.forward({ host: arg.remoteHost, port: arg.remotePort }); + remoteExplorerService.forward({ host: arg.remoteHost, port: arg.remotePort }).then(tunnel => error(notificationService, tunnel, arg.remoteHost, arg.remotePort)); } else { remoteExplorerService.setEditable(undefined, { onFinish: (value, success) => { let parsed: { host: string, port: number } | undefined; if (success && (parsed = parseInput(value))) { - remoteExplorerService.forward({ host: parsed.host, port: parsed.port }); + remoteExplorerService.forward({ host: parsed.host, port: parsed.port }).then(tunnel => error(notificationService, tunnel, parsed!.host, parsed!.port)); } remoteExplorerService.setEditable(undefined, null); }, @@ -632,6 +640,7 @@ namespace ForwardPortAction { export function commandPaletteHandler(): ICommandHandler { return async (accessor, arg) => { const remoteExplorerService = accessor.get(IRemoteExplorerService); + const notificationService = accessor.get(INotificationService); const viewsService = accessor.get(IViewsService); const quickInputService = accessor.get(IQuickInputService); await viewsService.openView(TunnelPanel.ID, true); @@ -641,7 +650,7 @@ namespace ForwardPortAction { }); let parsed: { host: string, port: number } | undefined; if (value && (parsed = parseInput(value))) { - remoteExplorerService.forward({ host: parsed.host, port: parsed.port }); + remoteExplorerService.forward({ host: parsed.host, port: parsed.port }).then(tunnel => error(notificationService, tunnel, parsed!.host, parsed!.port)); } }; } From 487122b117c87f4fdcc2d84ba73c8d70161a5ad5 Mon Sep 17 00:00:00 2001 From: isidor Date: Tue, 7 Jan 2020 16:23:31 +0100 Subject: [PATCH 18/36] debug unit tests: decorations --- .../browser/breakpointEditorContribution.ts | 24 ++++++------ .../debug/test/browser/breakpoints.test.ts | 37 +++++++++++++++++++ 2 files changed, 48 insertions(+), 13 deletions(-) diff --git a/src/vs/workbench/contrib/debug/browser/breakpointEditorContribution.ts b/src/vs/workbench/contrib/debug/browser/breakpointEditorContribution.ts index d67cd961c1b..0052f255b5b 100644 --- a/src/vs/workbench/contrib/debug/browser/breakpointEditorContribution.ts +++ b/src/vs/workbench/contrib/debug/browser/breakpointEditorContribution.ts @@ -17,7 +17,7 @@ import { IInstantiationService } from 'vs/platform/instantiation/common/instanti import { IContextKeyService, IContextKey } from 'vs/platform/contextkey/common/contextkey'; import { IContextMenuService } from 'vs/platform/contextview/browser/contextView'; import { RemoveBreakpointAction } from 'vs/workbench/contrib/debug/browser/debugActions'; -import { IDebugService, IBreakpoint, CONTEXT_BREAKPOINT_WIDGET_VISIBLE, BreakpointWidgetContext, BREAKPOINT_EDITOR_CONTRIBUTION_ID, IBreakpointEditorContribution, IBreakpointUpdateData, IDebugConfiguration, State } from 'vs/workbench/contrib/debug/common/debug'; +import { IDebugService, IBreakpoint, CONTEXT_BREAKPOINT_WIDGET_VISIBLE, BreakpointWidgetContext, BREAKPOINT_EDITOR_CONTRIBUTION_ID, IBreakpointEditorContribution, IBreakpointUpdateData, IDebugConfiguration, State, IDebugSession } from 'vs/workbench/contrib/debug/common/debug'; import { IMarginData } from 'vs/editor/browser/controller/mouseTarget'; import { ContextSubMenu } from 'vs/base/browser/contextmenu'; import { IDialogService } from 'vs/platform/dialogs/common/dialogs'; @@ -51,7 +51,7 @@ const breakpointHelperDecoration: IModelDecorationOptions = { stickiness: TrackedRangeStickiness.NeverGrowsWhenTypingAtEdges }; -function createBreakpointDecorations(model: ITextModel, breakpoints: ReadonlyArray, state: State, breakpointsActivated: boolean, debugSettings: IDebugConfiguration): { range: Range; options: IModelDecorationOptions; }[] { +export function createBreakpointDecorations(model: ITextModel, breakpoints: ReadonlyArray, state: State, breakpointsActivated: boolean, showBreakpointsInOverviewRuler: boolean): { range: Range; options: IModelDecorationOptions; }[] { const result: { range: Range; options: IModelDecorationOptions; }[] = []; breakpoints.forEach((breakpoint) => { if (breakpoint.lineNumber > model.getLineCount()) { @@ -64,7 +64,7 @@ function createBreakpointDecorations(model: ITextModel, breakpoints: ReadonlyArr ); result.push({ - options: getBreakpointDecorationOptions(model, breakpoint, state, breakpointsActivated, debugSettings), + options: getBreakpointDecorationOptions(model, breakpoint, state, breakpointsActivated, showBreakpointsInOverviewRuler), range }); }); @@ -72,7 +72,7 @@ function createBreakpointDecorations(model: ITextModel, breakpoints: ReadonlyArr return result; } -function getBreakpointDecorationOptions(model: ITextModel, breakpoint: IBreakpoint, state: State, breakpointsActivated: boolean, debugSettings: IDebugConfiguration): IModelDecorationOptions { +function getBreakpointDecorationOptions(model: ITextModel, breakpoint: IBreakpoint, state: State, breakpointsActivated: boolean, showBreakpointsInOverviewRuler: boolean): IModelDecorationOptions { const { className, message } = getBreakpointMessageAndClassName(state, breakpointsActivated, breakpoint); let glyphMarginHoverMessage: MarkdownString | undefined; @@ -85,14 +85,12 @@ function getBreakpointDecorationOptions(model: ITextModel, breakpoint: IBreakpoi } } - let overviewRulerDecoration: IModelDecorationOverviewRulerOptions | null; - if (debugSettings.showBreakpointsInOverviewRuler) { + let overviewRulerDecoration: IModelDecorationOverviewRulerOptions | null = null; + if (showBreakpointsInOverviewRuler) { overviewRulerDecoration = { color: 'rgb(124, 40, 49)', position: OverviewRulerLane.Left }; - } else { - overviewRulerDecoration = null; } const renderInline = breakpoint.column && (breakpoint.column > model.getLineFirstNonWhitespaceColumn(breakpoint.lineNumber)); @@ -105,11 +103,10 @@ function getBreakpointDecorationOptions(model: ITextModel, breakpoint: IBreakpoi }; } -async function createCandidateDecorations(model: ITextModel, breakpointDecorations: IBreakpointDecoration[], debugService: IDebugService): Promise<{ range: Range; options: IModelDecorationOptions; breakpoint: IBreakpoint | undefined }[]> { +async function createCandidateDecorations(model: ITextModel, breakpointDecorations: IBreakpointDecoration[], session: IDebugSession): Promise<{ range: Range; options: IModelDecorationOptions; breakpoint: IBreakpoint | undefined }[]> { const lineNumbers = distinct(breakpointDecorations.map(bpd => bpd.range.startLineNumber)); const result: { range: Range; options: IModelDecorationOptions; breakpoint: IBreakpoint | undefined }[] = []; - const session = debugService.getViewModel().focusedSession; - if (session && session.capabilities.supportsBreakpointLocationsRequest) { + if (session.capabilities.supportsBreakpointLocationsRequest) { await Promise.all(lineNumbers.map(async lineNumber => { try { const positions = await session.breakpointsLocations(model.uri, lineNumber); @@ -403,7 +400,7 @@ class BreakpointEditorContribution implements IBreakpointEditorContribution { const model = activeCodeEditor.getModel(); const breakpoints = this.debugService.getModel().getBreakpoints({ uri: model.uri }); const debugSettings = this.configurationService.getValue('debug'); - const desiredBreakpointDecorations = createBreakpointDecorations(model, breakpoints, this.debugService.state, this.debugService.getModel().areBreakpointsActivated(), debugSettings); + const desiredBreakpointDecorations = createBreakpointDecorations(model, breakpoints, this.debugService.state, this.debugService.getModel().areBreakpointsActivated(), debugSettings.showBreakpointsInOverviewRuler); try { this.ignoreDecorationsChangedEvent = true; @@ -436,7 +433,8 @@ class BreakpointEditorContribution implements IBreakpointEditorContribution { } // Set breakpoint candidate decorations - const desiredCandidateDecorations = debugSettings.showInlineBreakpointCandidates ? await createCandidateDecorations(this.editor.getModel(), this.breakpointDecorations, this.debugService) : []; + const session = this.debugService.getViewModel().focusedSession; + const desiredCandidateDecorations = debugSettings.showInlineBreakpointCandidates && session ? await createCandidateDecorations(this.editor.getModel(), this.breakpointDecorations, session) : []; const candidateDecorationIds = this.editor.deltaDecorations(this.candidateDecorations.map(c => c.decorationId), desiredCandidateDecorations); this.candidateDecorations.forEach(candidate => { candidate.inlineWidget.dispose(); diff --git a/src/vs/workbench/contrib/debug/test/browser/breakpoints.test.ts b/src/vs/workbench/contrib/debug/test/browser/breakpoints.test.ts index 7aa6720b30c..31844187fbe 100644 --- a/src/vs/workbench/contrib/debug/test/browser/breakpoints.test.ts +++ b/src/vs/workbench/contrib/debug/test/browser/breakpoints.test.ts @@ -10,7 +10,13 @@ import { DebugSession } from 'vs/workbench/contrib/debug/browser/debugSession'; import { NullOpenerService } from 'vs/platform/opener/common/opener'; import { getExpandedBodySize, getBreakpointMessageAndClassName } from 'vs/workbench/contrib/debug/browser/breakpointsView'; import { dispose } from 'vs/base/common/lifecycle'; +import { Range } from 'vs/editor/common/core/range'; import { IBreakpointData, IDebugSessionOptions, IBreakpointUpdateData, State } from 'vs/workbench/contrib/debug/common/debug'; +import { TextModel } from 'vs/editor/common/model/textModel'; +import { LanguageIdentifier, LanguageId } from 'vs/editor/common/modes'; +import { createBreakpointDecorations } from 'vs/workbench/contrib/debug/browser/breakpointEditorContribution'; +import { OverviewRulerLane } from 'vs/editor/common/model'; +import { MarkdownString } from 'vs/base/common/htmlContent'; function createMockSession(model: DebugModel, name = 'mockSession', options?: IDebugSessionOptions): DebugSession { return new DebugSession({ resolved: { name, type: 'node', request: 'launch' }, unresolved: undefined }, undefined!, model, options, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!, NullOpenerService, undefined!); @@ -309,4 +315,35 @@ suite('Debug - Breakpoints', () => { assert.equal(result.message, 'Log Message: hello, world'); assert.equal(result.className, 'codicon-debug-breakpoint-log'); }); + + test('decorations', () => { + const modelUri = uri.file('/myfolder/my file first.js'); + const languageIdentifier = new LanguageIdentifier('testMode', LanguageId.PlainText); + const textModel = new TextModel( + ['this is line one', 'this is line two', ' this is line three it has whitespace at start', 'this is line four', 'this is line five'].join('\n'), + TextModel.DEFAULT_CREATION_OPTIONS, + languageIdentifier + ); + addBreakpointsAndCheckEvents(model, modelUri, [ + { lineNumber: 1, enabled: true, condition: 'x > 5' }, + { lineNumber: 2, column: 4, enabled: false }, + { lineNumber: 3, enabled: true, logMessage: 'hello' }, + { lineNumber: 500, enabled: true }, + ]); + const breakpoints = model.getBreakpoints(); + + let decorations = createBreakpointDecorations(textModel, breakpoints, State.Running, true, true); + assert.equal(decorations.length, 3); // last breakpoint filtered out since it has a large line number + assert.deepEqual(decorations[0].range, new Range(1, 1, 1, 2)); + assert.deepEqual(decorations[1].range, new Range(2, 4, 2, 5)); + assert.deepEqual(decorations[2].range, new Range(3, 5, 3, 6)); + assert.equal(decorations[0].options.beforeContentClassName, undefined); + assert.equal(decorations[1].options.beforeContentClassName, `debug-breakpoint-placeholder`); + assert.equal(decorations[0].options.overviewRuler?.position, OverviewRulerLane.Left); + const expected = new MarkdownString().appendCodeblock(languageIdentifier.language, 'Expression: x > 5'); + assert.deepEqual(decorations[0].options.glyphMarginHoverMessage, expected); + + decorations = createBreakpointDecorations(textModel, breakpoints, State.Running, true, false); + assert.equal(decorations[0].options.overviewRuler, null); + }); }); From a4fa4ba6b71777d7082ed4be4199dfeddc6f7892 Mon Sep 17 00:00:00 2001 From: isidor Date: Tue, 7 Jan 2020 16:53:09 +0100 Subject: [PATCH 19/36] debug: unit tests reorganisation --- .../{debugModel.test.ts => callStack.test.ts} | 179 +----------------- .../contrib/debug/test/browser/repl.test.ts | 154 +++++++++++++++ .../contrib/debug/test/browser/watch.test.ts | 51 +++++ 3 files changed, 209 insertions(+), 175 deletions(-) rename src/vs/workbench/contrib/debug/test/browser/{debugModel.test.ts => callStack.test.ts} (56%) create mode 100644 src/vs/workbench/contrib/debug/test/browser/repl.test.ts create mode 100644 src/vs/workbench/contrib/debug/test/browser/watch.test.ts diff --git a/src/vs/workbench/contrib/debug/test/browser/debugModel.test.ts b/src/vs/workbench/contrib/debug/test/browser/callStack.test.ts similarity index 56% rename from src/vs/workbench/contrib/debug/test/browser/debugModel.test.ts rename to src/vs/workbench/contrib/debug/test/browser/callStack.test.ts index 9b1f22d67c7..1ce82d828af 100644 --- a/src/vs/workbench/contrib/debug/test/browser/debugModel.test.ts +++ b/src/vs/workbench/contrib/debug/test/browser/callStack.test.ts @@ -4,23 +4,19 @@ *--------------------------------------------------------------------------------------------*/ import * as assert from 'assert'; -import severity from 'vs/base/common/severity'; -import { DebugModel, Expression, StackFrame, Thread } from 'vs/workbench/contrib/debug/common/debugModel'; +import { DebugModel, StackFrame, Thread } from 'vs/workbench/contrib/debug/common/debugModel'; import * as sinon from 'sinon'; -import { MockRawSession, MockDebugAdapter } from 'vs/workbench/contrib/debug/test/common/mockDebug'; +import { MockRawSession } from 'vs/workbench/contrib/debug/test/common/mockDebug'; import { Source } from 'vs/workbench/contrib/debug/common/debugSource'; import { DebugSession } from 'vs/workbench/contrib/debug/browser/debugSession'; -import { SimpleReplElement, RawObjectReplElement, ReplEvaluationInput, ReplModel, ReplEvaluationResult } from 'vs/workbench/contrib/debug/common/replModel'; import { IDebugSessionOptions } from 'vs/workbench/contrib/debug/common/debug'; import { NullOpenerService } from 'vs/platform/opener/common/opener'; -import { RawDebugSession } from 'vs/workbench/contrib/debug/browser/rawDebugSession'; -import { timeout } from 'vs/base/common/async'; -function createMockSession(model: DebugModel, name = 'mockSession', options?: IDebugSessionOptions): DebugSession { +export function createMockSession(model: DebugModel, name = 'mockSession', options?: IDebugSessionOptions): DebugSession { return new DebugSession({ resolved: { name, type: 'node', request: 'launch' }, unresolved: undefined }, undefined!, model, options, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!, NullOpenerService, undefined!); } -suite('Debug - Model', () => { +suite('Debug - CallStack', () => { let model: DebugModel; let rawSession: MockRawSession; @@ -217,63 +213,6 @@ suite('Debug - Model', () => { assert.equal(session.getAllThreads().length, 0); }); - // Expressions - - function assertWatchExpressions(watchExpressions: Expression[], expectedName: string) { - assert.equal(watchExpressions.length, 2); - watchExpressions.forEach(we => { - assert.equal(we.available, false); - assert.equal(we.reference, 0); - assert.equal(we.name, expectedName); - }); - } - - test('watch expressions', () => { - assert.equal(model.getWatchExpressions().length, 0); - model.addWatchExpression('console'); - model.addWatchExpression('console'); - let watchExpressions = model.getWatchExpressions(); - assertWatchExpressions(watchExpressions, 'console'); - - model.renameWatchExpression(watchExpressions[0].getId(), 'new_name'); - model.renameWatchExpression(watchExpressions[1].getId(), 'new_name'); - assertWatchExpressions(model.getWatchExpressions(), 'new_name'); - - assertWatchExpressions(model.getWatchExpressions(), 'new_name'); - - model.addWatchExpression('mockExpression'); - model.moveWatchExpression(model.getWatchExpressions()[2].getId(), 1); - watchExpressions = model.getWatchExpressions(); - assert.equal(watchExpressions[0].name, 'new_name'); - assert.equal(watchExpressions[1].name, 'mockExpression'); - assert.equal(watchExpressions[2].name, 'new_name'); - - model.removeWatchExpressions(); - assert.equal(model.getWatchExpressions().length, 0); - }); - - test('repl expressions', () => { - const session = createMockSession(model); - assert.equal(session.getReplElements().length, 0); - model.addSession(session); - - session['raw'] = rawSession; - const thread = new Thread(session, 'mockthread', 1); - const stackFrame = new StackFrame(thread, 1, undefined, 'app.js', 'normal', { startLineNumber: 1, startColumn: 1, endLineNumber: 1, endColumn: 10 }, 1); - const replModel = new ReplModel(); - replModel.addReplExpression(session, stackFrame, 'myVariable').then(); - replModel.addReplExpression(session, stackFrame, 'myVariable').then(); - replModel.addReplExpression(session, stackFrame, 'myVariable').then(); - - assert.equal(replModel.getReplElements().length, 3); - replModel.getReplElements().forEach(re => { - assert.equal((re).value, 'myVariable'); - }); - - replModel.removeReplExpressions(); - assert.equal(replModel.getReplElements().length, 0); - }); - test('stack frame get specific source name', () => { const session = createMockSession(model); model.addSession(session); @@ -341,114 +280,4 @@ suite('Debug - Model', () => { assert.equal(sessions[4].getId(), anotherChild.getId()); assert.equal(sessions[5].getId(), thirdSession.getId()); }); - - // Repl output - - test('repl output', () => { - const session = createMockSession(model); - const repl = new ReplModel(); - repl.appendToRepl(session, 'first line\n', severity.Error); - repl.appendToRepl(session, 'second line ', severity.Error); - repl.appendToRepl(session, 'third line ', severity.Error); - repl.appendToRepl(session, 'fourth line', severity.Error); - - let elements = repl.getReplElements(); - assert.equal(elements.length, 2); - assert.equal(elements[0].value, 'first line\n'); - assert.equal(elements[0].severity, severity.Error); - assert.equal(elements[1].value, 'second line third line fourth line'); - assert.equal(elements[1].severity, severity.Error); - - repl.appendToRepl(session, '1', severity.Warning); - elements = repl.getReplElements(); - assert.equal(elements.length, 3); - assert.equal(elements[2].value, '1'); - assert.equal(elements[2].severity, severity.Warning); - - const keyValueObject = { 'key1': 2, 'key2': 'value' }; - repl.appendToRepl(session, new RawObjectReplElement('fakeid', 'fake', keyValueObject), severity.Info); - const element = repl.getReplElements()[3]; - assert.equal(element.value, 'Object'); - assert.deepEqual(element.valueObj, keyValueObject); - - repl.removeReplExpressions(); - assert.equal(repl.getReplElements().length, 0); - - repl.appendToRepl(session, '1\n', severity.Info); - repl.appendToRepl(session, '2', severity.Info); - repl.appendToRepl(session, '3\n4', severity.Info); - repl.appendToRepl(session, '5\n', severity.Info); - repl.appendToRepl(session, '6', severity.Info); - elements = repl.getReplElements(); - assert.equal(elements.length, 3); - assert.equal(elements[0], '1\n'); - assert.equal(elements[1], '23\n45\n'); - assert.equal(elements[2], '6'); - }); - - test('repl merging', () => { - // 'mergeWithParent' should be ignored when there is no parent. - const parent = createMockSession(model, 'parent', { repl: 'mergeWithParent' }); - const child1 = createMockSession(model, 'child1', { parentSession: parent, repl: 'separate' }); - const child2 = createMockSession(model, 'child2', { parentSession: parent, repl: 'mergeWithParent' }); - const grandChild = createMockSession(model, 'grandChild', { parentSession: child2, repl: 'mergeWithParent' }); - const child3 = createMockSession(model, 'child3', { parentSession: parent }); - - let parentChanges = 0; - parent.onDidChangeReplElements(() => ++parentChanges); - - parent.appendToRepl('1\n', severity.Info); - assert.equal(parentChanges, 1); - assert.equal(parent.getReplElements().length, 1); - assert.equal(child1.getReplElements().length, 0); - assert.equal(child2.getReplElements().length, 1); - assert.equal(grandChild.getReplElements().length, 1); - assert.equal(child3.getReplElements().length, 0); - - grandChild.appendToRepl('1\n', severity.Info); - assert.equal(parentChanges, 2); - assert.equal(parent.getReplElements().length, 2); - assert.equal(child1.getReplElements().length, 0); - assert.equal(child2.getReplElements().length, 2); - assert.equal(grandChild.getReplElements().length, 2); - assert.equal(child3.getReplElements().length, 0); - - child3.appendToRepl('1\n', severity.Info); - assert.equal(parentChanges, 2); - assert.equal(parent.getReplElements().length, 2); - assert.equal(child1.getReplElements().length, 0); - assert.equal(child2.getReplElements().length, 2); - assert.equal(grandChild.getReplElements().length, 2); - assert.equal(child3.getReplElements().length, 1); - - child1.appendToRepl('1\n', severity.Info); - assert.equal(parentChanges, 2); - assert.equal(parent.getReplElements().length, 2); - assert.equal(child1.getReplElements().length, 1); - assert.equal(child2.getReplElements().length, 2); - assert.equal(grandChild.getReplElements().length, 2); - assert.equal(child3.getReplElements().length, 1); - }); - - test('repl ordering', async () => { - const session = createMockSession(model); - model.addSession(session); - - const adapter = new MockDebugAdapter(); - const raw = new RawDebugSession(adapter, undefined!, undefined!, undefined!, undefined!, undefined!); - session.initializeForTest(raw); - - await session.addReplExpression(undefined, 'before.1'); - assert.equal(session.getReplElements().length, 3); - assert.equal((session.getReplElements()[0]).value, 'before.1'); - assert.equal((session.getReplElements()[1]).value, 'before.1'); - assert.equal((session.getReplElements()[2]).value, '=before.1'); - - await session.addReplExpression(undefined, 'after.2'); - await timeout(0); - assert.equal(session.getReplElements().length, 6); - assert.equal((session.getReplElements()[3]).value, 'after.2'); - assert.equal((session.getReplElements()[4]).value, '=after.2'); - assert.equal((session.getReplElements()[5]).value, 'after.2'); - }); }); diff --git a/src/vs/workbench/contrib/debug/test/browser/repl.test.ts b/src/vs/workbench/contrib/debug/test/browser/repl.test.ts new file mode 100644 index 00000000000..c155eca7669 --- /dev/null +++ b/src/vs/workbench/contrib/debug/test/browser/repl.test.ts @@ -0,0 +1,154 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + + +import * as assert from 'assert'; +import severity from 'vs/base/common/severity'; +import { DebugModel, StackFrame, Thread } from 'vs/workbench/contrib/debug/common/debugModel'; +import { MockRawSession, MockDebugAdapter } from 'vs/workbench/contrib/debug/test/common/mockDebug'; +import { SimpleReplElement, RawObjectReplElement, ReplEvaluationInput, ReplModel, ReplEvaluationResult } from 'vs/workbench/contrib/debug/common/replModel'; +import { RawDebugSession } from 'vs/workbench/contrib/debug/browser/rawDebugSession'; +import { timeout } from 'vs/base/common/async'; +import { createMockSession } from 'vs/workbench/contrib/debug/test/browser/callStack.test'; + +suite('Debug - REPL', () => { + let model: DebugModel; + let rawSession: MockRawSession; + + setup(() => { + model = new DebugModel([], [], [], [], [], { isDirty: (e: any) => false }); + rawSession = new MockRawSession(); + }); + + test('repl output', () => { + const session = createMockSession(model); + const repl = new ReplModel(); + repl.appendToRepl(session, 'first line\n', severity.Error); + repl.appendToRepl(session, 'second line ', severity.Error); + repl.appendToRepl(session, 'third line ', severity.Error); + repl.appendToRepl(session, 'fourth line', severity.Error); + + let elements = repl.getReplElements(); + assert.equal(elements.length, 2); + assert.equal(elements[0].value, 'first line\n'); + assert.equal(elements[0].severity, severity.Error); + assert.equal(elements[1].value, 'second line third line fourth line'); + assert.equal(elements[1].severity, severity.Error); + + repl.appendToRepl(session, '1', severity.Warning); + elements = repl.getReplElements(); + assert.equal(elements.length, 3); + assert.equal(elements[2].value, '1'); + assert.equal(elements[2].severity, severity.Warning); + + const keyValueObject = { 'key1': 2, 'key2': 'value' }; + repl.appendToRepl(session, new RawObjectReplElement('fakeid', 'fake', keyValueObject), severity.Info); + const element = repl.getReplElements()[3]; + assert.equal(element.value, 'Object'); + assert.deepEqual(element.valueObj, keyValueObject); + + repl.removeReplExpressions(); + assert.equal(repl.getReplElements().length, 0); + + repl.appendToRepl(session, '1\n', severity.Info); + repl.appendToRepl(session, '2', severity.Info); + repl.appendToRepl(session, '3\n4', severity.Info); + repl.appendToRepl(session, '5\n', severity.Info); + repl.appendToRepl(session, '6', severity.Info); + elements = repl.getReplElements(); + assert.equal(elements.length, 3); + assert.equal(elements[0], '1\n'); + assert.equal(elements[1], '23\n45\n'); + assert.equal(elements[2], '6'); + }); + + test('repl merging', () => { + // 'mergeWithParent' should be ignored when there is no parent. + const parent = createMockSession(model, 'parent', { repl: 'mergeWithParent' }); + const child1 = createMockSession(model, 'child1', { parentSession: parent, repl: 'separate' }); + const child2 = createMockSession(model, 'child2', { parentSession: parent, repl: 'mergeWithParent' }); + const grandChild = createMockSession(model, 'grandChild', { parentSession: child2, repl: 'mergeWithParent' }); + const child3 = createMockSession(model, 'child3', { parentSession: parent }); + + let parentChanges = 0; + parent.onDidChangeReplElements(() => ++parentChanges); + + parent.appendToRepl('1\n', severity.Info); + assert.equal(parentChanges, 1); + assert.equal(parent.getReplElements().length, 1); + assert.equal(child1.getReplElements().length, 0); + assert.equal(child2.getReplElements().length, 1); + assert.equal(grandChild.getReplElements().length, 1); + assert.equal(child3.getReplElements().length, 0); + + grandChild.appendToRepl('1\n', severity.Info); + assert.equal(parentChanges, 2); + assert.equal(parent.getReplElements().length, 2); + assert.equal(child1.getReplElements().length, 0); + assert.equal(child2.getReplElements().length, 2); + assert.equal(grandChild.getReplElements().length, 2); + assert.equal(child3.getReplElements().length, 0); + + child3.appendToRepl('1\n', severity.Info); + assert.equal(parentChanges, 2); + assert.equal(parent.getReplElements().length, 2); + assert.equal(child1.getReplElements().length, 0); + assert.equal(child2.getReplElements().length, 2); + assert.equal(grandChild.getReplElements().length, 2); + assert.equal(child3.getReplElements().length, 1); + + child1.appendToRepl('1\n', severity.Info); + assert.equal(parentChanges, 2); + assert.equal(parent.getReplElements().length, 2); + assert.equal(child1.getReplElements().length, 1); + assert.equal(child2.getReplElements().length, 2); + assert.equal(grandChild.getReplElements().length, 2); + assert.equal(child3.getReplElements().length, 1); + }); + + test('repl expressions', () => { + const session = createMockSession(model); + assert.equal(session.getReplElements().length, 0); + model.addSession(session); + + session['raw'] = rawSession; + const thread = new Thread(session, 'mockthread', 1); + const stackFrame = new StackFrame(thread, 1, undefined, 'app.js', 'normal', { startLineNumber: 1, startColumn: 1, endLineNumber: 1, endColumn: 10 }, 1); + const replModel = new ReplModel(); + replModel.addReplExpression(session, stackFrame, 'myVariable').then(); + replModel.addReplExpression(session, stackFrame, 'myVariable').then(); + replModel.addReplExpression(session, stackFrame, 'myVariable').then(); + + assert.equal(replModel.getReplElements().length, 3); + replModel.getReplElements().forEach(re => { + assert.equal((re).value, 'myVariable'); + }); + + replModel.removeReplExpressions(); + assert.equal(replModel.getReplElements().length, 0); + }); + + test('repl ordering', async () => { + const session = createMockSession(model); + model.addSession(session); + + const adapter = new MockDebugAdapter(); + const raw = new RawDebugSession(adapter, undefined!, undefined!, undefined!, undefined!, undefined!); + session.initializeForTest(raw); + + await session.addReplExpression(undefined, 'before.1'); + assert.equal(session.getReplElements().length, 3); + assert.equal((session.getReplElements()[0]).value, 'before.1'); + assert.equal((session.getReplElements()[1]).value, 'before.1'); + assert.equal((session.getReplElements()[2]).value, '=before.1'); + + await session.addReplExpression(undefined, 'after.2'); + await timeout(0); + assert.equal(session.getReplElements().length, 6); + assert.equal((session.getReplElements()[3]).value, 'after.2'); + assert.equal((session.getReplElements()[4]).value, '=after.2'); + assert.equal((session.getReplElements()[5]).value, 'after.2'); + }); +}); diff --git a/src/vs/workbench/contrib/debug/test/browser/watch.test.ts b/src/vs/workbench/contrib/debug/test/browser/watch.test.ts new file mode 100644 index 00000000000..2dedc695698 --- /dev/null +++ b/src/vs/workbench/contrib/debug/test/browser/watch.test.ts @@ -0,0 +1,51 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as assert from 'assert'; +import { Expression, DebugModel } from 'vs/workbench/contrib/debug/common/debugModel'; + +// Expressions + +function assertWatchExpressions(watchExpressions: Expression[], expectedName: string) { + assert.equal(watchExpressions.length, 2); + watchExpressions.forEach(we => { + assert.equal(we.available, false); + assert.equal(we.reference, 0); + assert.equal(we.name, expectedName); + }); +} + +suite('Debug - Watch', () => { + + let model: DebugModel; + + setup(() => { + model = new DebugModel([], [], [], [], [], { isDirty: (e: any) => false }); + }); + + test('watch expressions', () => { + assert.equal(model.getWatchExpressions().length, 0); + model.addWatchExpression('console'); + model.addWatchExpression('console'); + let watchExpressions = model.getWatchExpressions(); + assertWatchExpressions(watchExpressions, 'console'); + + model.renameWatchExpression(watchExpressions[0].getId(), 'new_name'); + model.renameWatchExpression(watchExpressions[1].getId(), 'new_name'); + assertWatchExpressions(model.getWatchExpressions(), 'new_name'); + + assertWatchExpressions(model.getWatchExpressions(), 'new_name'); + + model.addWatchExpression('mockExpression'); + model.moveWatchExpression(model.getWatchExpressions()[2].getId(), 1); + watchExpressions = model.getWatchExpressions(); + assert.equal(watchExpressions[0].name, 'new_name'); + assert.equal(watchExpressions[1].name, 'mockExpression'); + assert.equal(watchExpressions[2].name, 'new_name'); + + model.removeWatchExpressions(); + assert.equal(model.getWatchExpressions().length, 0); + }); +}); From d9a3924d1cc7458e5c9dcb3536968480cb574b94 Mon Sep 17 00:00:00 2001 From: isidor Date: Tue, 7 Jan 2020 16:53:25 +0100 Subject: [PATCH 20/36] debug: extract createDecraotionsForStackFrame to a function --- .../browser/callStackEditorContribution.ts | 139 +++++++++--------- 1 file changed, 66 insertions(+), 73 deletions(-) diff --git a/src/vs/workbench/contrib/debug/browser/callStackEditorContribution.ts b/src/vs/workbench/contrib/debug/browser/callStackEditorContribution.ts index aa5ca3a1e92..f2298a0de37 100644 --- a/src/vs/workbench/contrib/debug/browser/callStackEditorContribution.ts +++ b/src/vs/workbench/contrib/debug/browser/callStackEditorContribution.ts @@ -17,6 +17,71 @@ import { registerEditorContribution } from 'vs/editor/browser/editorExtensions'; const stickiness = TrackedRangeStickiness.NeverGrowsWhenTypingAtEdges; +// we need a separate decoration for glyph margin, since we do not want it on each line of a multi line statement. +const TOP_STACK_FRAME_MARGIN: IModelDecorationOptions = { + glyphMarginClassName: 'codicon-debug-stackframe', + stickiness +}; +const FOCUSED_STACK_FRAME_MARGIN: IModelDecorationOptions = { + glyphMarginClassName: 'codicon-debug-stackframe-focused', + stickiness +}; +const TOP_STACK_FRAME_DECORATION: IModelDecorationOptions = { + isWholeLine: true, + className: 'debug-top-stack-frame-line', + stickiness +}; +const TOP_STACK_FRAME_INLINE_DECORATION: IModelDecorationOptions = { + beforeContentClassName: 'debug-top-stack-frame-column' +}; +const FOCUSED_STACK_FRAME_DECORATION: IModelDecorationOptions = { + isWholeLine: true, + className: 'debug-focused-stack-frame-line', + stickiness +}; + +function createDecorationsForStackFrame(stackFrame: IStackFrame, topStackFrameRange: Range | undefined): IModelDeltaDecoration[] { + // only show decorations for the currently focused thread. + const result: IModelDeltaDecoration[] = []; + const columnUntilEOLRange = new Range(stackFrame.range.startLineNumber, stackFrame.range.startColumn, stackFrame.range.startLineNumber, Constants.MAX_SAFE_SMALL_INTEGER); + const range = new Range(stackFrame.range.startLineNumber, stackFrame.range.startColumn, stackFrame.range.startLineNumber, stackFrame.range.startColumn + 1); + + // compute how to decorate the editor. Different decorations are used if this is a top stack frame, focused stack frame, + // an exception or a stack frame that did not change the line number (we only decorate the columns, not the whole line). + const callStack = stackFrame.thread.getCallStack(); + if (callStack && callStack.length && stackFrame === callStack[0]) { + result.push({ + options: TOP_STACK_FRAME_MARGIN, + range + }); + + result.push({ + options: TOP_STACK_FRAME_DECORATION, + range: columnUntilEOLRange + }); + + if (topStackFrameRange && topStackFrameRange.startLineNumber === stackFrame.range.startLineNumber && topStackFrameRange.startColumn !== stackFrame.range.startColumn) { + result.push({ + options: TOP_STACK_FRAME_INLINE_DECORATION, + range: columnUntilEOLRange + }); + } + topStackFrameRange = columnUntilEOLRange; + } else { + result.push({ + options: FOCUSED_STACK_FRAME_MARGIN, + range + }); + + result.push({ + options: FOCUSED_STACK_FRAME_DECORATION, + range: columnUntilEOLRange + }); + } + + return result; +} + class CallStackEditorContribution implements IEditorContribution { private toDispose: IDisposable[] = []; private decorationIds: string[] = []; @@ -52,7 +117,7 @@ class CallStackEditorContribution implements IEditorContribution { } if (candidateStackFrame && candidateStackFrame.source.uri.toString() === this.editor.getModel()?.uri.toString()) { - decorations.push(...this.createDecorationsForStackFrame(candidateStackFrame)); + decorations.push(...createDecorationsForStackFrame(candidateStackFrame, this.topStackFrameRange)); } } }); @@ -61,78 +126,6 @@ class CallStackEditorContribution implements IEditorContribution { return decorations; } - private createDecorationsForStackFrame(stackFrame: IStackFrame): IModelDeltaDecoration[] { - // only show decorations for the currently focused thread. - const result: IModelDeltaDecoration[] = []; - const columnUntilEOLRange = new Range(stackFrame.range.startLineNumber, stackFrame.range.startColumn, stackFrame.range.startLineNumber, Constants.MAX_SAFE_SMALL_INTEGER); - const range = new Range(stackFrame.range.startLineNumber, stackFrame.range.startColumn, stackFrame.range.startLineNumber, stackFrame.range.startColumn + 1); - - // compute how to decorate the editor. Different decorations are used if this is a top stack frame, focused stack frame, - // an exception or a stack frame that did not change the line number (we only decorate the columns, not the whole line). - const callStack = stackFrame.thread.getCallStack(); - if (callStack && callStack.length && stackFrame === callStack[0]) { - result.push({ - options: CallStackEditorContribution.TOP_STACK_FRAME_MARGIN, - range - }); - - result.push({ - options: CallStackEditorContribution.TOP_STACK_FRAME_DECORATION, - range: columnUntilEOLRange - }); - - if (this.topStackFrameRange && this.topStackFrameRange.startLineNumber === stackFrame.range.startLineNumber && this.topStackFrameRange.startColumn !== stackFrame.range.startColumn) { - result.push({ - options: CallStackEditorContribution.TOP_STACK_FRAME_INLINE_DECORATION, - range: columnUntilEOLRange - }); - } - this.topStackFrameRange = columnUntilEOLRange; - } else { - result.push({ - options: CallStackEditorContribution.FOCUSED_STACK_FRAME_MARGIN, - range - }); - - result.push({ - options: CallStackEditorContribution.FOCUSED_STACK_FRAME_DECORATION, - range: columnUntilEOLRange - }); - } - - return result; - } - - // editor decorations - - static readonly STICKINESS = TrackedRangeStickiness.NeverGrowsWhenTypingAtEdges; - // we need a separate decoration for glyph margin, since we do not want it on each line of a multi line statement. - private static TOP_STACK_FRAME_MARGIN: IModelDecorationOptions = { - glyphMarginClassName: 'codicon-debug-stackframe', - stickiness - }; - - private static FOCUSED_STACK_FRAME_MARGIN: IModelDecorationOptions = { - glyphMarginClassName: 'codicon-debug-stackframe-focused', - stickiness - }; - - private static TOP_STACK_FRAME_DECORATION: IModelDecorationOptions = { - isWholeLine: true, - className: 'debug-top-stack-frame-line', - stickiness - }; - - private static TOP_STACK_FRAME_INLINE_DECORATION: IModelDecorationOptions = { - beforeContentClassName: 'debug-top-stack-frame-column' - }; - - private static FOCUSED_STACK_FRAME_DECORATION: IModelDecorationOptions = { - isWholeLine: true, - className: 'debug-focused-stack-frame-line', - stickiness - }; - dispose(): void { this.editor.deltaDecorations(this.decorationIds, []); this.toDispose = dispose(this.toDispose); From 62a231717615fae87c425b400f97b8c667dd9421 Mon Sep 17 00:00:00 2001 From: Alex Ross Date: Tue, 7 Jan 2020 17:13:37 +0100 Subject: [PATCH 21/36] Option for hiding candidate ports --- .../remote/common/remoteAuthorityResolver.ts | 1 + src/vs/vscode.proposed.d.ts | 2 ++ .../api/common/extHostExtensionService.ts | 2 +- .../electron-browser/extensionService.ts | 2 +- .../remote/common/remoteExplorerService.ts | 20 +++++++++++++++---- 5 files changed, 21 insertions(+), 6 deletions(-) diff --git a/src/vs/platform/remote/common/remoteAuthorityResolver.ts b/src/vs/platform/remote/common/remoteAuthorityResolver.ts index 7cd49599314..2f1d4020484 100644 --- a/src/vs/platform/remote/common/remoteAuthorityResolver.ts +++ b/src/vs/platform/remote/common/remoteAuthorityResolver.ts @@ -19,6 +19,7 @@ export interface ResolvedOptions { export interface TunnelInformation { environmentTunnels?: { remoteAddress: { port: number, host: string }, localAddress: string }[]; + hideCandidatePorts?: boolean; } export interface ResolverResult { diff --git a/src/vs/vscode.proposed.d.ts b/src/vs/vscode.proposed.d.ts index a2cbad3a365..29a517933a7 100644 --- a/src/vs/vscode.proposed.d.ts +++ b/src/vs/vscode.proposed.d.ts @@ -59,6 +59,8 @@ declare module 'vscode' { * detected are read-only from the forwarded ports UI. */ environmentTunnels?: { remoteAddress: { port: number, host: string }, localAddress: string }[]; + + hideCandidatePorts?: boolean; } export type ResolverResult = ResolvedAuthority & ResolvedOptions & TunnelInformation; diff --git a/src/vs/workbench/api/common/extHostExtensionService.ts b/src/vs/workbench/api/common/extHostExtensionService.ts index 944546d61d9..b0cb8707782 100644 --- a/src/vs/workbench/api/common/extHostExtensionService.ts +++ b/src/vs/workbench/api/common/extHostExtensionService.ts @@ -662,7 +662,7 @@ export abstract class AbstractExtHostExtensionService implements ExtHostExtensio value: { authority, options, - tunnelInformation: { environmentTunnels: result.environmentTunnels } + tunnelInformation: { environmentTunnels: result.environmentTunnels, hideCandidatePorts: result.hideCandidatePorts } } }; } catch (err) { diff --git a/src/vs/workbench/services/extensions/electron-browser/extensionService.ts b/src/vs/workbench/services/extensions/electron-browser/extensionService.ts index 04b014914bb..9e95b1d4a6f 100644 --- a/src/vs/workbench/services/extensions/electron-browser/extensionService.ts +++ b/src/vs/workbench/services/extensions/electron-browser/extensionService.ts @@ -473,7 +473,7 @@ export class ExtensionService extends AbstractExtensionService implements IExten // set the resolved authority this._remoteAuthorityResolverService.setResolvedAuthority(resolvedAuthority.authority, resolvedAuthority.options); - this._remoteExplorerService.addEnvironmentTunnels(resolvedAuthority.tunnelInformation?.environmentTunnels); + this._remoteExplorerService.setTunnelInformation(resolvedAuthority.tunnelInformation); // monitor for breakage const connection = this._remoteAgentService.getConnection(); diff --git a/src/vs/workbench/services/remote/common/remoteExplorerService.ts b/src/vs/workbench/services/remote/common/remoteExplorerService.ts index 3ef12502500..8a94eeccd97 100644 --- a/src/vs/workbench/services/remote/common/remoteExplorerService.ts +++ b/src/vs/workbench/services/remote/common/remoteExplorerService.ts @@ -11,6 +11,7 @@ import { ITunnelService, RemoteTunnel } from 'vs/platform/remote/common/tunnel'; import { Disposable } from 'vs/base/common/lifecycle'; import { IEditableData } from 'vs/workbench/common/views'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; +import { TunnelInformation } from 'vs/platform/remote/common/remoteAuthorityResolver'; export const IRemoteExplorerService = createDecorator('remoteExplorerService'); export const REMOTE_EXPLORER_TYPE_KEY: string = 'remote.explorerType'; @@ -54,6 +55,7 @@ export function MakeAddress(host: string, port: number): string { export class TunnelModel extends Disposable { readonly forwarded: Map; readonly detected: Map; + private _candidatesEnabled: boolean = true; private _onForwardPort: Emitter = new Emitter(); public onForwardPort: Event = this._onForwardPort.event; private _onClosePort: Emitter<{ host: string, port: number }> = new Emitter(); @@ -181,6 +183,10 @@ export class TunnelModel extends Disposable { }); } + set candidateEnabled(enabled: boolean) { + this._candidatesEnabled = enabled; + } + registerCandidateFinder(finder: () => Promise<{ host: string, port: number, detail: string }[]>): void { this._candidateFinder = finder; } @@ -190,6 +196,10 @@ export class TunnelModel extends Disposable { } private async updateCandidates(): Promise { + if (!this._candidatesEnabled) { + this._candidates = []; + return; + } if (this._candidateFinder) { this._candidates = await this._candidateFinder(); } @@ -211,7 +221,7 @@ export interface IRemoteExplorerService { getEditableData(tunnelItem: ITunnelItem | undefined): IEditableData | undefined; forward(remote: { host: string, port: number }, localPort?: number, name?: string): Promise; close(remote: { host: string, port: number }): Promise; - addEnvironmentTunnels(tunnels: { remoteAddress: { port: number, host: string }, localAddress: string }[] | undefined): void; + setTunnelInformation(tunnelInformation: TunnelInformation | undefined): void; registerCandidateFinder(finder: () => Promise<{ host: string, port: number, detail: string }[]>): void; refresh(): Promise; } @@ -258,10 +268,12 @@ class RemoteExplorerService implements IRemoteExplorerService { return this.tunnelModel.close(remote.host, remote.port); } - addEnvironmentTunnels(tunnels: { remoteAddress: { port: number, host: string }, localAddress: string }[] | undefined): void { - if (tunnels) { - this.tunnelModel.addEnvironmentTunnels(tunnels); + setTunnelInformation(tunnelInformation: TunnelInformation | undefined): void { + if (tunnelInformation && tunnelInformation.environmentTunnels) { + this.tunnelModel.addEnvironmentTunnels(tunnelInformation.environmentTunnels); } + + this.tunnelModel.candidateEnabled = tunnelInformation ? (tunnelInformation.hideCandidatePorts !== true) : true; } setEditable(tunnelItem: ITunnelItem | undefined, data: IEditableData | null): void { From 458c8d93c4502d24c9f437b3b19090e2bc32fd95 Mon Sep 17 00:00:00 2001 From: isidor Date: Tue, 7 Jan 2020 17:15:43 +0100 Subject: [PATCH 22/36] call stack decorations test --- .../browser/callStackEditorContribution.ts | 4 +- .../debug/test/browser/callStack.test.ts | 85 ++++++++++++++----- 2 files changed, 65 insertions(+), 24 deletions(-) diff --git a/src/vs/workbench/contrib/debug/browser/callStackEditorContribution.ts b/src/vs/workbench/contrib/debug/browser/callStackEditorContribution.ts index f2298a0de37..ad6a80d55db 100644 --- a/src/vs/workbench/contrib/debug/browser/callStackEditorContribution.ts +++ b/src/vs/workbench/contrib/debug/browser/callStackEditorContribution.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { Constants } from 'vs/base/common/uint'; -import { Range } from 'vs/editor/common/core/range'; +import { Range, IRange } from 'vs/editor/common/core/range'; import { TrackedRangeStickiness, IModelDeltaDecoration, IModelDecorationOptions } from 'vs/editor/common/model'; import { IDebugService, IStackFrame } from 'vs/workbench/contrib/debug/common/debug'; import { registerThemingParticipant } from 'vs/platform/theme/common/themeService'; @@ -40,7 +40,7 @@ const FOCUSED_STACK_FRAME_DECORATION: IModelDecorationOptions = { stickiness }; -function createDecorationsForStackFrame(stackFrame: IStackFrame, topStackFrameRange: Range | undefined): IModelDeltaDecoration[] { +export function createDecorationsForStackFrame(stackFrame: IStackFrame, topStackFrameRange: IRange | undefined): IModelDeltaDecoration[] { // only show decorations for the currently focused thread. const result: IModelDeltaDecoration[] = []; const columnUntilEOLRange = new Range(stackFrame.range.startLineNumber, stackFrame.range.startColumn, stackFrame.range.startLineNumber, Constants.MAX_SAFE_SMALL_INTEGER); diff --git a/src/vs/workbench/contrib/debug/test/browser/callStack.test.ts b/src/vs/workbench/contrib/debug/test/browser/callStack.test.ts index 1ce82d828af..5ac24860494 100644 --- a/src/vs/workbench/contrib/debug/test/browser/callStack.test.ts +++ b/src/vs/workbench/contrib/debug/test/browser/callStack.test.ts @@ -9,14 +9,43 @@ import * as sinon from 'sinon'; import { MockRawSession } from 'vs/workbench/contrib/debug/test/common/mockDebug'; import { Source } from 'vs/workbench/contrib/debug/common/debugSource'; import { DebugSession } from 'vs/workbench/contrib/debug/browser/debugSession'; +import { Range } from 'vs/editor/common/core/range'; import { IDebugSessionOptions } from 'vs/workbench/contrib/debug/common/debug'; import { NullOpenerService } from 'vs/platform/opener/common/opener'; +import { createDecorationsForStackFrame } from 'vs/workbench/contrib/debug/browser/callStackEditorContribution'; +import { Constants } from 'vs/base/common/uint'; export function createMockSession(model: DebugModel, name = 'mockSession', options?: IDebugSessionOptions): DebugSession { return new DebugSession({ resolved: { name, type: 'node', request: 'launch' }, unresolved: undefined }, undefined!, model, options, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!, NullOpenerService, undefined!); } -suite('Debug - CallStack', () => { +function createTwoStackFrames(session: DebugSession): { firstStackFrame: StackFrame, secondStackFrame: StackFrame } { + let firstStackFrame: StackFrame; + let secondStackFrame: StackFrame; + const thread = new class extends Thread { + public getCallStack(): StackFrame[] { + return [firstStackFrame, secondStackFrame]; + } + }(session, 'mockthread', 1); + + const firstSource = new Source({ + name: 'internalModule.js', + path: 'a/b/c/d/internalModule.js', + sourceReference: 10, + }, 'aDebugSessionId'); + const secondSource = new Source({ + name: 'internalModule.js', + path: 'z/x/c/d/internalModule.js', + sourceReference: 11, + }, 'aDebugSessionId'); + + firstStackFrame = new StackFrame(thread, 1, firstSource, 'app.js', 'normal', { startLineNumber: 1, startColumn: 1, endLineNumber: 1, endColumn: 10 }, 1); + secondStackFrame = new StackFrame(thread, 1, secondSource, 'app.js', 'normal', { startLineNumber: 1, startColumn: 1, endLineNumber: 1, endColumn: 10 }, 1); + + return { firstStackFrame, secondStackFrame }; +} + +suite.only('Debug - CallStack', () => { let model: DebugModel; let rawSession: MockRawSession; @@ -216,27 +245,7 @@ suite('Debug - CallStack', () => { test('stack frame get specific source name', () => { const session = createMockSession(model); model.addSession(session); - - let firstStackFrame: StackFrame; - let secondStackFrame: StackFrame; - const thread = new class extends Thread { - public getCallStack(): StackFrame[] { - return [firstStackFrame, secondStackFrame]; - } - }(session, 'mockthread', 1); - - const firstSource = new Source({ - name: 'internalModule.js', - path: 'a/b/c/d/internalModule.js', - sourceReference: 10, - }, 'aDebugSessionId'); - const secondSource = new Source({ - name: 'internalModule.js', - path: 'z/x/c/d/internalModule.js', - sourceReference: 11, - }, 'aDebugSessionId'); - firstStackFrame = new StackFrame(thread, 1, firstSource, 'app.js', 'normal', { startLineNumber: 1, startColumn: 1, endLineNumber: 1, endColumn: 10 }, 1); - secondStackFrame = new StackFrame(thread, 1, secondSource, 'app.js', 'normal', { startLineNumber: 1, startColumn: 1, endLineNumber: 1, endColumn: 10 }, 1); + const { firstStackFrame, secondStackFrame } = createTwoStackFrames(session); assert.equal(firstStackFrame.getSpecificSourceName(), '.../b/c/d/internalModule.js'); assert.equal(secondStackFrame.getSpecificSourceName(), '.../x/c/d/internalModule.js'); @@ -280,4 +289,36 @@ suite('Debug - CallStack', () => { assert.equal(sessions[4].getId(), anotherChild.getId()); assert.equal(sessions[5].getId(), thirdSession.getId()); }); + + test('decorations', () => { + const session = createMockSession(model); + model.addSession(session); + const { firstStackFrame, secondStackFrame } = createTwoStackFrames(session); + let decorations = createDecorationsForStackFrame(firstStackFrame, firstStackFrame.range); + assert.equal(decorations.length, 2); + assert.deepEqual(decorations[0].range, new Range(1, 2, 1, 1)); + assert.equal(decorations[0].options.glyphMarginClassName, 'codicon-debug-stackframe'); + assert.deepEqual(decorations[1].range, new Range(1, Constants.MAX_SAFE_SMALL_INTEGER, 1, 1)); + assert.equal(decorations[1].options.className, 'debug-top-stack-frame-line'); + assert.equal(decorations[1].options.isWholeLine, true); + + decorations = createDecorationsForStackFrame(secondStackFrame, firstStackFrame.range); + assert.equal(decorations.length, 2); + assert.deepEqual(decorations[0].range, new Range(1, 2, 1, 1)); + assert.equal(decorations[0].options.glyphMarginClassName, 'codicon-debug-stackframe-focused'); + assert.deepEqual(decorations[1].range, new Range(1, Constants.MAX_SAFE_SMALL_INTEGER, 1, 1)); + assert.equal(decorations[1].options.className, 'debug-focused-stack-frame-line'); + assert.equal(decorations[1].options.isWholeLine, true); + + decorations = createDecorationsForStackFrame(firstStackFrame, new Range(1, 5, 1, 6)); + assert.equal(decorations.length, 3); + assert.deepEqual(decorations[0].range, new Range(1, 2, 1, 1)); + assert.equal(decorations[0].options.glyphMarginClassName, 'codicon-debug-stackframe'); + assert.deepEqual(decorations[1].range, new Range(1, Constants.MAX_SAFE_SMALL_INTEGER, 1, 1)); + assert.equal(decorations[1].options.className, 'debug-top-stack-frame-line'); + assert.equal(decorations[1].options.isWholeLine, true); + // Inline decoration gets rendered in this case + assert.equal(decorations[2].options.beforeContentClassName, 'debug-top-stack-frame-column'); + assert.deepEqual(decorations[2].range, new Range(1, Constants.MAX_SAFE_SMALL_INTEGER, 1, 1)); + }); }); From 2851ffbcd2853d0fef1936d2a3ebc159d1ac2a2c Mon Sep 17 00:00:00 2001 From: isidor Date: Tue, 7 Jan 2020 17:16:54 +0100 Subject: [PATCH 23/36] remove .only --- src/vs/workbench/contrib/debug/test/browser/callStack.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/debug/test/browser/callStack.test.ts b/src/vs/workbench/contrib/debug/test/browser/callStack.test.ts index 5ac24860494..a6811ed52c1 100644 --- a/src/vs/workbench/contrib/debug/test/browser/callStack.test.ts +++ b/src/vs/workbench/contrib/debug/test/browser/callStack.test.ts @@ -45,7 +45,7 @@ function createTwoStackFrames(session: DebugSession): { firstStackFrame: StackFr return { firstStackFrame, secondStackFrame }; } -suite.only('Debug - CallStack', () => { +suite('Debug - CallStack', () => { let model: DebugModel; let rawSession: MockRawSession; From f548111c5c3435622c5f4fe42a121ed8ba28d543 Mon Sep 17 00:00:00 2001 From: isidor Date: Tue, 7 Jan 2020 17:25:26 +0100 Subject: [PATCH 24/36] callstack test contexts --- .../contrib/debug/browser/callStackView.ts | 41 ++++++++++--------- .../debug/test/browser/callStack.test.ts | 28 +++++++++++++ 2 files changed, 49 insertions(+), 20 deletions(-) diff --git a/src/vs/workbench/contrib/debug/browser/callStackView.ts b/src/vs/workbench/contrib/debug/browser/callStackView.ts index 4ae48c7e38c..18d812625a4 100644 --- a/src/vs/workbench/contrib/debug/browser/callStackView.ts +++ b/src/vs/workbench/contrib/debug/browser/callStackView.ts @@ -40,7 +40,7 @@ const $ = dom.$; type CallStackItem = IStackFrame | IThread | IDebugSession | string | ThreadAndSessionIds | IStackFrame[]; -function getContext(element: CallStackItem | null): any { +export function getContext(element: CallStackItem | null): any { return element instanceof StackFrame ? { sessionId: element.thread.session.getId(), threadId: element.thread.getId(), @@ -53,6 +53,25 @@ function getContext(element: CallStackItem | null): any { } : undefined; } +// Extensions depend on this context, should not be changed even though it is not fully deterministic +export function getContextForContributedActions(element: CallStackItem | null): string | number { + if (element instanceof StackFrame) { + if (element.source.inMemory) { + return element.source.raw.path || element.source.reference || ''; + } + + return element.source.uri.toString(); + } + if (element instanceof Thread) { + return element.threadId; + } + if (isDebugSession(element)) { + return element.getId(); + } + + return ''; +} + export class CallStackView extends ViewPane { private pauseMessage!: HTMLSpanElement; private pauseMessageLabel!: HTMLSpanElement; @@ -336,7 +355,7 @@ export class CallStackView extends ViewPane { } const actions: IAction[] = []; - const actionsDisposable = createAndFillInContextMenuActions(this.contributedContextMenu, { arg: this.getContextForContributedActions(element), shouldForwardArgs: true }, actions, this.contextMenuService); + const actionsDisposable = createAndFillInContextMenuActions(this.contributedContextMenu, { arg: getContextForContributedActions(element), shouldForwardArgs: true }, actions, this.contextMenuService); this.contextMenuService.showContextMenu({ getAnchor: () => e.anchor, @@ -345,24 +364,6 @@ export class CallStackView extends ViewPane { onHide: () => dispose(actionsDisposable) }); } - - private getContextForContributedActions(element: CallStackItem | null): string | number { - if (element instanceof StackFrame) { - if (element.source.inMemory) { - return element.source.raw.path || element.source.reference || ''; - } - - return element.source.uri.toString(); - } - if (element instanceof Thread) { - return element.threadId; - } - if (isDebugSession(element)) { - return element.getId(); - } - - return ''; - } } interface IThreadTemplateData { diff --git a/src/vs/workbench/contrib/debug/test/browser/callStack.test.ts b/src/vs/workbench/contrib/debug/test/browser/callStack.test.ts index a6811ed52c1..8442cd4086c 100644 --- a/src/vs/workbench/contrib/debug/test/browser/callStack.test.ts +++ b/src/vs/workbench/contrib/debug/test/browser/callStack.test.ts @@ -14,6 +14,7 @@ import { IDebugSessionOptions } from 'vs/workbench/contrib/debug/common/debug'; import { NullOpenerService } from 'vs/platform/opener/common/opener'; import { createDecorationsForStackFrame } from 'vs/workbench/contrib/debug/browser/callStackEditorContribution'; import { Constants } from 'vs/base/common/uint'; +import { getContext, getContextForContributedActions } from 'vs/workbench/contrib/debug/browser/callStackView'; export function createMockSession(model: DebugModel, name = 'mockSession', options?: IDebugSessionOptions): DebugSession { return new DebugSession({ resolved: { name, type: 'node', request: 'launch' }, unresolved: undefined }, undefined!, model, options, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!, NullOpenerService, undefined!); @@ -321,4 +322,31 @@ suite('Debug - CallStack', () => { assert.equal(decorations[2].options.beforeContentClassName, 'debug-top-stack-frame-column'); assert.deepEqual(decorations[2].range, new Range(1, Constants.MAX_SAFE_SMALL_INTEGER, 1, 1)); }); + + test('contexts', () => { + const session = createMockSession(model); + model.addSession(session); + const { firstStackFrame, secondStackFrame } = createTwoStackFrames(session); + let context = getContext(firstStackFrame); + assert.equal(context.sessionId, firstStackFrame.thread.session.getId()); + assert.equal(context.threadId, firstStackFrame.thread.getId()); + assert.equal(context.frameId, firstStackFrame.getId()); + + context = getContext(secondStackFrame.thread); + assert.equal(context.sessionId, secondStackFrame.thread.session.getId()); + assert.equal(context.threadId, secondStackFrame.thread.getId()); + assert.equal(context.frameId, undefined); + + context = getContext(session); + assert.equal(context.sessionId, session.getId()); + assert.equal(context.threadId, undefined); + assert.equal(context.frameId, undefined); + + let contributedContext = getContextForContributedActions(firstStackFrame); + assert.equal(contributedContext, firstStackFrame.source.raw.path); + contributedContext = getContextForContributedActions(firstStackFrame.thread); + assert.equal(contributedContext, firstStackFrame.thread.threadId); + contributedContext = getContextForContributedActions(session); + assert.equal(contributedContext, session.getId()); + }); }); From eaf427d740f7d3aea7c385d113c320956c8a85dc Mon Sep 17 00:00:00 2001 From: Andre Weinand Date: Tue, 7 Jan 2020 18:12:41 +0100 Subject: [PATCH 25/36] properly use the automationShell setting; fixes #87700 --- .../workbench/api/node/extHostDebugService.ts | 23 ++++++++++++--- .../workbench/contrib/debug/node/terminals.ts | 28 +++++-------------- 2 files changed, 26 insertions(+), 25 deletions(-) diff --git a/src/vs/workbench/api/node/extHostDebugService.ts b/src/vs/workbench/api/node/extHostDebugService.ts index ac05c037e00..f73486a4763 100644 --- a/src/vs/workbench/api/node/extHostDebugService.ts +++ b/src/vs/workbench/api/node/extHostDebugService.ts @@ -5,6 +5,7 @@ import * as nls from 'vs/nls'; import * as vscode from 'vscode'; +import * as env from 'vs/base/common/platform'; import { DebugAdapterExecutable } from 'vs/workbench/api/common/extHostTypes'; import { ExecutableDebugAdapter, SocketDebugAdapter } from 'vs/workbench/contrib/debug/node/debugAdapter'; import { AbstractDebugAdapter } from 'vs/workbench/contrib/debug/common/abstractDebugAdapter'; @@ -23,7 +24,6 @@ import { SignService } from 'vs/platform/sign/node/signService'; import { hasChildProcesses, prepareCommand, runInExternalTerminal } from 'vs/workbench/contrib/debug/node/terminals'; import { IDisposable } from 'vs/base/common/lifecycle'; import { AbstractVariableResolverService } from 'vs/workbench/services/configurationResolver/common/variableResolver'; -import { IProcessEnvironment } from 'vs/base/common/platform'; export class ExtHostDebugService extends ExtHostDebugServiceBase { @@ -87,7 +87,22 @@ export class ExtHostDebugService extends ExtHostDebugServiceBase { } const configProvider = await this._configurationService.getConfigProvider(); - const shell = this._terminalService.getDefaultShell(true, configProvider); + const terminalConfig = configProvider.getConfiguration('terminal'); + + let shell; + const automationShellConfig = terminalConfig.integrated.automationShell; + if (automationShellConfig) { + if (env.isWindows) { + shell = automationShellConfig.windows; + } else if (env.isLinux) { + shell = automationShellConfig.linux; + } else if (env.isMacintosh) { + shell = automationShellConfig.osx; + } + } + if (!shell) { + shell = this._terminalService.getDefaultShell(true, configProvider); + } if (needNewTerminal || !this._integratedTerminalInstance) { @@ -108,7 +123,7 @@ export class ExtHostDebugService extends ExtHostDebugServiceBase { terminal.show(); const shellProcessId = await this._integratedTerminalInstance.processId; - const command = prepareCommand(args, shell, configProvider); + const command = prepareCommand(args, shell); terminal.sendText(command, true); return shellProcessId; @@ -121,7 +136,7 @@ export class ExtHostDebugService extends ExtHostDebugServiceBase { } protected createVariableResolver(folders: vscode.WorkspaceFolder[], editorService: ExtHostDocumentsAndEditors, configurationService: ExtHostConfigProvider): AbstractVariableResolverService { - return new ExtHostVariableResolverService(folders, editorService, configurationService, process.env as IProcessEnvironment); + return new ExtHostVariableResolverService(folders, editorService, configurationService, process.env as env.IProcessEnvironment); } } diff --git a/src/vs/workbench/contrib/debug/node/terminals.ts b/src/vs/workbench/contrib/debug/node/terminals.ts index e4e0abd2280..a14e52c7a18 100644 --- a/src/vs/workbench/contrib/debug/node/terminals.ts +++ b/src/vs/workbench/contrib/debug/node/terminals.ts @@ -5,7 +5,6 @@ import * as cp from 'child_process'; import * as env from 'vs/base/common/platform'; -import { getSystemShell } from 'vs/workbench/contrib/terminal/node/terminal'; import { WindowsExternalTerminalService, MacExternalTerminalService, LinuxExternalTerminalService } from 'vs/workbench/contrib/externalTerminal/node/externalTerminalService'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; import { IExternalTerminalService } from 'vs/workbench/contrib/externalTerminal/common/externalTerminal'; @@ -76,35 +75,22 @@ export function hasChildProcesses(processId: number | undefined): Promise= 0 || shell.indexOf('pwsh') >= 0) { shellType = ShellType.powershell; } else if (shell.indexOf('cmd.exe') >= 0) { shellType = ShellType.cmd; } else if (shell.indexOf('bash') >= 0) { shellType = ShellType.bash; + } else if (env.isWindows) { + shellType = ShellType.cmd; // pick a good default for Windows + } else { + shellType = ShellType.bash; // pick a good default for anything else } let quote: (s: string) => string; From 3ecaff4093415eb7524119991f6349572fc4946f Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Tue, 7 Jan 2020 09:42:56 -0800 Subject: [PATCH 26/36] xterm@4.4.0-beta.15 Diff: https://github.com/xtermjs/xterm.js/compare/2a9e16b...fbeb45d - Adding FluffOS to xtermjs user list xtermjs/xterm.js#2663 - Flag lines as wrapped after CUP in windows mode xtermjs/xterm.js#2667 Part of #85839 --- package.json | 2 +- remote/package.json | 2 +- remote/web/package.json | 2 +- remote/web/yarn.lock | 8 ++++---- remote/yarn.lock | 8 ++++---- yarn.lock | 8 ++++---- 6 files changed, 15 insertions(+), 15 deletions(-) diff --git a/package.json b/package.json index aa95fce241d..a0713956468 100644 --- a/package.json +++ b/package.json @@ -55,7 +55,7 @@ "vscode-ripgrep": "^1.5.7", "vscode-sqlite3": "4.0.9", "vscode-textmate": "4.4.0", - "xterm": "4.4.0-beta.13", + "xterm": "4.4.0-beta.15", "xterm-addon-search": "0.4.0-beta4", "xterm-addon-web-links": "0.2.1", "xterm-addon-webgl": "0.5.0-beta.7", diff --git a/remote/package.json b/remote/package.json index 493a2ebda1a..95ea409067f 100644 --- a/remote/package.json +++ b/remote/package.json @@ -20,7 +20,7 @@ "vscode-proxy-agent": "^0.5.2", "vscode-ripgrep": "^1.5.7", "vscode-textmate": "4.4.0", - "xterm": "4.4.0-beta.13", + "xterm": "4.4.0-beta.15", "xterm-addon-search": "0.4.0-beta4", "xterm-addon-web-links": "0.2.1", "xterm-addon-webgl": "0.5.0-beta.7", diff --git a/remote/web/package.json b/remote/web/package.json index dad7548703f..5bf2c6c7d9c 100644 --- a/remote/web/package.json +++ b/remote/web/package.json @@ -5,7 +5,7 @@ "onigasm-umd": "2.2.5", "semver-umd": "^5.5.5", "vscode-textmate": "4.4.0", - "xterm": "4.4.0-beta.13", + "xterm": "4.4.0-beta.15", "xterm-addon-search": "0.4.0-beta4", "xterm-addon-web-links": "0.2.1", "xterm-addon-webgl": "0.5.0-beta.7" diff --git a/remote/web/yarn.lock b/remote/web/yarn.lock index b7969abc564..12afd3c3d84 100644 --- a/remote/web/yarn.lock +++ b/remote/web/yarn.lock @@ -46,7 +46,7 @@ xterm-addon-webgl@0.5.0-beta.7: resolved "https://registry.yarnpkg.com/xterm-addon-webgl/-/xterm-addon-webgl-0.5.0-beta.7.tgz#b7b95a362e942ad6f86fa286d7b7bd8ee3e7cf67" integrity sha512-v6aCvhm1C6mvaurGwUYQfyhb2cAUyuVnzf3Ob/hy5ebtyzUj4wW0N9NbqDEJk67UeMi1lV2xZqrO5gNeTpVqFA== -xterm@4.4.0-beta.13: - version "4.4.0-beta.13" - resolved "https://registry.yarnpkg.com/xterm/-/xterm-4.4.0-beta.13.tgz#f7c5fa0d2b098ce0dd8b7c96d3d5fcaee22b86ed" - integrity sha512-ZoDOVO3w84CXekBveGw1H2lcvM4HkJG5suXesE/3S+N4DnBhBcK/vw4kdooALGoorJV2GtgA1XEA6+m4N5Sgnw== +xterm@4.4.0-beta.15: + version "4.4.0-beta.15" + resolved "https://registry.yarnpkg.com/xterm/-/xterm-4.4.0-beta.15.tgz#5897bf79d29d1a2496ccd54665aded28c341b1cc" + integrity sha512-Dvz1CMCYKeoxPF7uIDznbRgUA2Mct49Bq93K2nnrDU0pDMM3Sf1t9fkEyz59wxSx5XEHVdLS80jywsz4sjXBjQ== diff --git a/remote/yarn.lock b/remote/yarn.lock index 9137652df57..387226ccd2a 100644 --- a/remote/yarn.lock +++ b/remote/yarn.lock @@ -433,10 +433,10 @@ xterm-addon-webgl@0.5.0-beta.7: resolved "https://registry.yarnpkg.com/xterm-addon-webgl/-/xterm-addon-webgl-0.5.0-beta.7.tgz#b7b95a362e942ad6f86fa286d7b7bd8ee3e7cf67" integrity sha512-v6aCvhm1C6mvaurGwUYQfyhb2cAUyuVnzf3Ob/hy5ebtyzUj4wW0N9NbqDEJk67UeMi1lV2xZqrO5gNeTpVqFA== -xterm@4.4.0-beta.13: - version "4.4.0-beta.13" - resolved "https://registry.yarnpkg.com/xterm/-/xterm-4.4.0-beta.13.tgz#f7c5fa0d2b098ce0dd8b7c96d3d5fcaee22b86ed" - integrity sha512-ZoDOVO3w84CXekBveGw1H2lcvM4HkJG5suXesE/3S+N4DnBhBcK/vw4kdooALGoorJV2GtgA1XEA6+m4N5Sgnw== +xterm@4.4.0-beta.15: + version "4.4.0-beta.15" + resolved "https://registry.yarnpkg.com/xterm/-/xterm-4.4.0-beta.15.tgz#5897bf79d29d1a2496ccd54665aded28c341b1cc" + integrity sha512-Dvz1CMCYKeoxPF7uIDznbRgUA2Mct49Bq93K2nnrDU0pDMM3Sf1t9fkEyz59wxSx5XEHVdLS80jywsz4sjXBjQ== yauzl@^2.9.2: version "2.10.0" diff --git a/yarn.lock b/yarn.lock index aa4c370f2b0..6ef2a9f7c77 100644 --- a/yarn.lock +++ b/yarn.lock @@ -9978,10 +9978,10 @@ xterm-addon-webgl@0.5.0-beta.7: resolved "https://registry.yarnpkg.com/xterm-addon-webgl/-/xterm-addon-webgl-0.5.0-beta.7.tgz#b7b95a362e942ad6f86fa286d7b7bd8ee3e7cf67" integrity sha512-v6aCvhm1C6mvaurGwUYQfyhb2cAUyuVnzf3Ob/hy5ebtyzUj4wW0N9NbqDEJk67UeMi1lV2xZqrO5gNeTpVqFA== -xterm@4.4.0-beta.13: - version "4.4.0-beta.13" - resolved "https://registry.yarnpkg.com/xterm/-/xterm-4.4.0-beta.13.tgz#f7c5fa0d2b098ce0dd8b7c96d3d5fcaee22b86ed" - integrity sha512-ZoDOVO3w84CXekBveGw1H2lcvM4HkJG5suXesE/3S+N4DnBhBcK/vw4kdooALGoorJV2GtgA1XEA6+m4N5Sgnw== +xterm@4.4.0-beta.15: + version "4.4.0-beta.15" + resolved "https://registry.yarnpkg.com/xterm/-/xterm-4.4.0-beta.15.tgz#5897bf79d29d1a2496ccd54665aded28c341b1cc" + integrity sha512-Dvz1CMCYKeoxPF7uIDznbRgUA2Mct49Bq93K2nnrDU0pDMM3Sf1t9fkEyz59wxSx5XEHVdLS80jywsz4sjXBjQ== y18n@^3.2.1: version "3.2.1" From 4dfa4583cf0e404198e827484b2db585a61bb184 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Tue, 7 Jan 2020 09:49:48 -0800 Subject: [PATCH 27/36] Update distro --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index a0713956468..06789d3e072 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "code-oss-dev", "version": "1.42.0", - "distro": "57c315cb98e064a152bcc6a599ed459d67d933e2", + "distro": "730174fd92c39e946f34d96f5a48c23a8ddb33b2", "author": { "name": "Microsoft Corporation" }, From fd673718ca0bf055d8dad999aab354a6014b273a Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Tue, 7 Jan 2020 11:28:58 -0800 Subject: [PATCH 28/36] Only treat diff editors as matching if they have the same forceOpenAsBinary setting (#88183) Fixes #88166 Custom editors used in diff views for binary files were broken. The root cause we were creating a `DiffEditorInput` that set `forceOpenAsBinary === true`, however this input was discarded as it was judged to be the same as the existing editor input which had `forceOpenAsBinary === undefined` The fix here is to make the `matches` function take `forceOpenAsBinary` into account --- src/vs/workbench/common/editor/diffEditorInput.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/vs/workbench/common/editor/diffEditorInput.ts b/src/vs/workbench/common/editor/diffEditorInput.ts index 3b30153f526..12e57ac49e0 100644 --- a/src/vs/workbench/common/editor/diffEditorInput.ts +++ b/src/vs/workbench/common/editor/diffEditorInput.ts @@ -28,6 +28,13 @@ export class DiffEditorInput extends SideBySideEditorInput { super(name, description, original, modified); } + matches(otherInput: unknown): boolean { + if (!super.matches(otherInput)) { + return false; + } + return otherInput instanceof DiffEditorInput && otherInput.forceOpenAsBinary === this.forceOpenAsBinary; + } + getTypeId(): string { return DiffEditorInput.ID; } From 08d431dc17c6edc6f1a7cf28418107f8a7745d3a Mon Sep 17 00:00:00 2001 From: Sana Ajani Date: Tue, 7 Jan 2020 15:09:02 -0800 Subject: [PATCH 29/36] distro --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 06789d3e072..989bc744f63 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "code-oss-dev", "version": "1.42.0", - "distro": "730174fd92c39e946f34d96f5a48c23a8ddb33b2", + "distro": "aafa05cffde354e7f58dc9fd2dcfc9aa3e81ea77", "author": { "name": "Microsoft Corporation" }, From 74c8922d31330baa15263a3f6f2684caa538445e Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Tue, 7 Jan 2020 11:37:12 -0800 Subject: [PATCH 30/36] Make names for code action config contributions more explicit --- .../codeActions/common/codeActions.contribution.ts | 8 ++++---- .../workbench/contrib/codeActions/common/configuration.ts | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/vs/workbench/contrib/codeActions/common/codeActions.contribution.ts b/src/vs/workbench/contrib/codeActions/common/codeActions.contribution.ts index ddf83c61dc7..c23b8d27d41 100644 --- a/src/vs/workbench/contrib/codeActions/common/codeActions.contribution.ts +++ b/src/vs/workbench/contrib/codeActions/common/codeActions.contribution.ts @@ -8,7 +8,7 @@ import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding'; import { LifecyclePhase } from 'vs/platform/lifecycle/common/lifecycle'; import { Registry } from 'vs/platform/registry/common/platform'; import { Extensions as WorkbenchExtensions, IWorkbenchContributionsRegistry } from 'vs/workbench/common/contributions'; -import { CodeActionWorkbenchContribution, editorConfiguration } from 'vs/workbench/contrib/codeActions/common/configuration'; +import { CodeActionWorkbenchConfigurationContribution, editorConfiguration } from 'vs/workbench/contrib/codeActions/common/configuration'; import { CodeActionsExtensionPoint, codeActionsExtensionPointDescriptor } from 'vs/workbench/contrib/codeActions/common/extensionPoint'; import { ExtensionsRegistry } from 'vs/workbench/services/extensions/common/extensionsRegistry'; @@ -17,13 +17,13 @@ const codeActionsExtensionPoint = ExtensionsRegistry.registerExtensionPoint(Extensions.Configuration) .registerConfiguration(editorConfiguration); -class WorkbenchContribution { +class WorkbenchConfigurationContribution { constructor( @IKeybindingService keybindingsService: IKeybindingService, ) { - new CodeActionWorkbenchContribution(codeActionsExtensionPoint, keybindingsService); + new CodeActionWorkbenchConfigurationContribution(codeActionsExtensionPoint, keybindingsService); } } Registry.as(WorkbenchExtensions.Workbench) - .registerWorkbenchContribution(WorkbenchContribution, LifecyclePhase.Eventually); + .registerWorkbenchContribution(WorkbenchConfigurationContribution, LifecyclePhase.Eventually); diff --git a/src/vs/workbench/contrib/codeActions/common/configuration.ts b/src/vs/workbench/contrib/codeActions/common/configuration.ts index bdbcc19eb6a..19b9ac85ddf 100644 --- a/src/vs/workbench/contrib/codeActions/common/configuration.ts +++ b/src/vs/workbench/contrib/codeActions/common/configuration.ts @@ -50,7 +50,7 @@ export const editorConfiguration = Object.freeze({ } }); -export class CodeActionWorkbenchContribution extends Disposable implements IWorkbenchContribution { +export class CodeActionWorkbenchConfigurationContribution extends Disposable implements IWorkbenchContribution { private _contributedCodeActions: CodeActionsExtensionPoint[] = []; From f0336455eda145921acef2175dec60ffc003a133 Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Tue, 7 Jan 2020 15:40:20 -0800 Subject: [PATCH 31/36] Adding documentation.refactor proposed contribution point For #86788 --- .../typescript-language-features/package.json | 10 ++ .../package.nls.json | 3 +- .../src/commands/index.ts | 4 +- .../commands/learnMoreAboutRefactorings.ts | 15 +++ .../contrib/codeAction/codeActionMenu.ts | 26 +++++- src/vs/editor/contrib/codeAction/types.ts | 1 + .../common/codeActions.contribution.ts | 14 ++- ...guration.ts => codeActionsContribution.ts} | 6 +- ...nPoint.ts => codeActionsExtensionPoint.ts} | 0 .../common/documentationContribution.ts | 91 +++++++++++++++++++ .../common/documentationExtensionPoint.ts | 64 +++++++++++++ 11 files changed, 220 insertions(+), 14 deletions(-) create mode 100644 extensions/typescript-language-features/src/commands/learnMoreAboutRefactorings.ts rename src/vs/workbench/contrib/codeActions/common/{configuration.ts => codeActionsContribution.ts} (96%) rename src/vs/workbench/contrib/codeActions/common/{extensionPoint.ts => codeActionsExtensionPoint.ts} (100%) create mode 100644 src/vs/workbench/contrib/codeActions/common/documentationContribution.ts create mode 100644 src/vs/workbench/contrib/codeActions/common/documentationExtensionPoint.ts diff --git a/extensions/typescript-language-features/package.json b/extensions/typescript-language-features/package.json index 7ea2b6f5af6..577a4fd1686 100644 --- a/extensions/typescript-language-features/package.json +++ b/extensions/typescript-language-features/package.json @@ -45,10 +45,20 @@ "onCommand:typescript.openTsServerLog", "onCommand:workbench.action.tasks.runTask", "onCommand:_typescript.configurePlugin", + "onCommand:_typescript.learnMoreAboutRefactorings", "onLanguage:jsonc" ], "main": "./out/extension", "contributes": { + "documentation": { + "refactoring": [ + { + "title": "%documentation.refactoring.title%", + "when": "typescript.isManagedFile", + "command": "_typescript.learnMoreAboutRefactorings" + } + ] + }, "jsonValidation": [ { "fileMatch": "package.json", diff --git a/extensions/typescript-language-features/package.nls.json b/extensions/typescript-language-features/package.nls.json index 1bd543cb55c..adad367759b 100644 --- a/extensions/typescript-language-features/package.nls.json +++ b/extensions/typescript-language-features/package.nls.json @@ -97,5 +97,6 @@ "codeActions.refactor.rewrite.parameters.toDestructured.title": "Convert parameters to destructured object", "codeActions.refactor.rewrite.property.generateAccessors.title": "Generate accessors", "codeActions.refactor.rewrite.property.generateAccessors.description": "Generate 'get' and 'set' accessors", - "codeActions.source.organizeImports.title": "Organize imports" + "codeActions.source.organizeImports.title": "Organize imports", + "documentation.refactoring.title": "Learn more about JS/TS refactorings" } diff --git a/extensions/typescript-language-features/src/commands/index.ts b/extensions/typescript-language-features/src/commands/index.ts index f22b3e448de..8e4cfddcccd 100644 --- a/extensions/typescript-language-features/src/commands/index.ts +++ b/extensions/typescript-language-features/src/commands/index.ts @@ -13,6 +13,7 @@ import { OpenTsServerLogCommand } from './openTsServerLog'; import { ReloadJavaScriptProjectsCommand, ReloadTypeScriptProjectsCommand } from './reloadProject'; import { RestartTsServerCommand } from './restartTsServer'; import { SelectTypeScriptVersionCommand } from './selectTypeScriptVersion'; +import { LearnMoreAboutRefactoringsCommand } from './learnMoreAboutRefactorings'; export function registerCommands( commandManager: CommandManager, @@ -27,4 +28,5 @@ export function registerCommands( commandManager.register(new TypeScriptGoToProjectConfigCommand(lazyClientHost)); commandManager.register(new JavaScriptGoToProjectConfigCommand(lazyClientHost)); commandManager.register(new ConfigurePluginCommand(pluginManager)); -} \ No newline at end of file + commandManager.register(new LearnMoreAboutRefactoringsCommand()); +} diff --git a/extensions/typescript-language-features/src/commands/learnMoreAboutRefactorings.ts b/extensions/typescript-language-features/src/commands/learnMoreAboutRefactorings.ts new file mode 100644 index 00000000000..3c1673bb63c --- /dev/null +++ b/extensions/typescript-language-features/src/commands/learnMoreAboutRefactorings.ts @@ -0,0 +1,15 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as vscode from 'vscode'; +import { Command } from '../utils/commandManager'; + +export class LearnMoreAboutRefactoringsCommand implements Command { + public readonly id = '_typescript.learnMoreAboutRefactorings'; + + public execute() { + vscode.env.openExternal(vscode.Uri.parse('https://go.microsoft.com/fwlink/?linkid=2114477')); + } +} diff --git a/src/vs/editor/contrib/codeAction/codeActionMenu.ts b/src/vs/editor/contrib/codeAction/codeActionMenu.ts index c2cdc9c4f0f..7be72d3d2e3 100644 --- a/src/vs/editor/contrib/codeAction/codeActionMenu.ts +++ b/src/vs/editor/contrib/codeAction/codeActionMenu.ts @@ -4,8 +4,9 @@ *--------------------------------------------------------------------------------------------*/ import { getDomNodePagePosition } from 'vs/base/browser/dom'; +import { Separator } from 'vs/base/browser/ui/actionbar/actionbar'; import { IAnchor } from 'vs/base/browser/ui/contextview/contextview'; -import { Action } from 'vs/base/common/actions'; +import { Action, IAction } from 'vs/base/common/actions'; import { canceled } from 'vs/base/common/errors'; import { ResolvedKeybinding } from 'vs/base/common/keyCodes'; import { Lazy } from 'vs/base/common/lazy'; @@ -14,7 +15,7 @@ import { ICodeEditor } from 'vs/editor/browser/editorBrowser'; import { IPosition, Position } from 'vs/editor/common/core/position'; import { ScrollType } from 'vs/editor/common/editorCommon'; import { CodeAction } from 'vs/editor/common/modes'; -import { CodeActionSet, refactorCommandId, sourceActionCommandId, codeActionCommandId, organizeImportsCommandId, fixAllCommandId } from 'vs/editor/contrib/codeAction/codeAction'; +import { codeActionCommandId, CodeActionSet, fixAllCommandId, organizeImportsCommandId, refactorCommandId, sourceActionCommandId } from 'vs/editor/contrib/codeAction/codeAction'; import { CodeActionAutoApply, CodeActionCommandArgs, CodeActionKind } from 'vs/editor/contrib/codeAction/types'; import { IContextMenuService } from 'vs/platform/contextview/browser/contextView'; import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding'; @@ -83,8 +84,7 @@ export class CodeActionMenu extends Disposable { this._visible = true; this._showingActions.value = codeActions; - const menuActions = actionsToShow.map(action => - new CodeActionAction(action, () => this._delegate.onSelectCodeAction(action))); + const menuActions = this.getMenuActions(actionsToShow); const anchor = Position.isIPosition(at) ? this._toCoords(at) : at || { x: 0, y: 0 }; const resolver = this._keybindingResolver.getResolver(); @@ -101,6 +101,24 @@ export class CodeActionMenu extends Disposable { }); } + private getMenuActions(actionsToShow: readonly CodeAction[]): IAction[] { + const allActions = actionsToShow + .map(action => new CodeActionAction(action, () => this._delegate.onSelectCodeAction(action))); + + // Treat documentation actions as special + const result: IAction[] = allActions + .filter(action => !action.action.kind || !CodeActionKind.RefactorDocumentation.contains(new CodeActionKind(action.action.kind))); + + const documentationActions = allActions + .filter(action => action.action.kind && CodeActionKind.RefactorDocumentation.contains(new CodeActionKind(action.action.kind))); + + if (documentationActions.length) { + result.push(new Separator(), ...documentationActions); + } + + return result; + } + private _toCoords(position: IPosition): { x: number, y: number } { if (!this._editor.hasModel()) { return { x: 0, y: 0 }; diff --git a/src/vs/editor/contrib/codeAction/types.ts b/src/vs/editor/contrib/codeAction/types.ts index 650be89e98c..bce9f612329 100644 --- a/src/vs/editor/contrib/codeAction/types.ts +++ b/src/vs/editor/contrib/codeAction/types.ts @@ -14,6 +14,7 @@ export class CodeActionKind { public static readonly Empty = new CodeActionKind(''); public static readonly QuickFix = new CodeActionKind('quickfix'); public static readonly Refactor = new CodeActionKind('refactor'); + public static readonly RefactorDocumentation = new CodeActionKind('refactor.documentation'); public static readonly Source = new CodeActionKind('source'); public static readonly SourceOrganizeImports = CodeActionKind.Source.append('organizeImports'); public static readonly SourceFixAll = CodeActionKind.Source.append('fixAll'); diff --git a/src/vs/workbench/contrib/codeActions/common/codeActions.contribution.ts b/src/vs/workbench/contrib/codeActions/common/codeActions.contribution.ts index c23b8d27d41..c7969c9d1c0 100644 --- a/src/vs/workbench/contrib/codeActions/common/codeActions.contribution.ts +++ b/src/vs/workbench/contrib/codeActions/common/codeActions.contribution.ts @@ -4,24 +4,28 @@ *--------------------------------------------------------------------------------------------*/ import { Extensions, IConfigurationRegistry } from 'vs/platform/configuration/common/configurationRegistry'; -import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding'; +import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { LifecyclePhase } from 'vs/platform/lifecycle/common/lifecycle'; import { Registry } from 'vs/platform/registry/common/platform'; import { Extensions as WorkbenchExtensions, IWorkbenchContributionsRegistry } from 'vs/workbench/common/contributions'; -import { CodeActionWorkbenchConfigurationContribution, editorConfiguration } from 'vs/workbench/contrib/codeActions/common/configuration'; -import { CodeActionsExtensionPoint, codeActionsExtensionPointDescriptor } from 'vs/workbench/contrib/codeActions/common/extensionPoint'; +import { CodeActionsContribution, editorConfiguration } from 'vs/workbench/contrib/codeActions/common/codeActionsContribution'; +import { CodeActionsExtensionPoint, codeActionsExtensionPointDescriptor } from 'vs/workbench/contrib/codeActions/common/codeActionsExtensionPoint'; +import { CodeActionDocumentationContribution } from 'vs/workbench/contrib/codeActions/common/documentationContribution'; +import { DocumentationExtensionPoint, documentationExtensionPointDescriptor } from 'vs/workbench/contrib/codeActions/common/documentationExtensionPoint'; import { ExtensionsRegistry } from 'vs/workbench/services/extensions/common/extensionsRegistry'; const codeActionsExtensionPoint = ExtensionsRegistry.registerExtensionPoint(codeActionsExtensionPointDescriptor); +const documentationExtensionPoint = ExtensionsRegistry.registerExtensionPoint(documentationExtensionPointDescriptor); Registry.as(Extensions.Configuration) .registerConfiguration(editorConfiguration); class WorkbenchConfigurationContribution { constructor( - @IKeybindingService keybindingsService: IKeybindingService, + @IInstantiationService instantiationService: IInstantiationService, ) { - new CodeActionWorkbenchConfigurationContribution(codeActionsExtensionPoint, keybindingsService); + instantiationService.createInstance(CodeActionsContribution, codeActionsExtensionPoint); + instantiationService.createInstance(CodeActionDocumentationContribution, documentationExtensionPoint); } } diff --git a/src/vs/workbench/contrib/codeActions/common/configuration.ts b/src/vs/workbench/contrib/codeActions/common/codeActionsContribution.ts similarity index 96% rename from src/vs/workbench/contrib/codeActions/common/configuration.ts rename to src/vs/workbench/contrib/codeActions/common/codeActionsContribution.ts index 19b9ac85ddf..53153e88dca 100644 --- a/src/vs/workbench/contrib/codeActions/common/configuration.ts +++ b/src/vs/workbench/contrib/codeActions/common/codeActionsContribution.ts @@ -15,7 +15,7 @@ import { Extensions, IConfigurationNode, IConfigurationRegistry, ConfigurationSc import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding'; import { Registry } from 'vs/platform/registry/common/platform'; import { IWorkbenchContribution } from 'vs/workbench/common/contributions'; -import { CodeActionsExtensionPoint, ContributedCodeAction } from 'vs/workbench/contrib/codeActions/common/extensionPoint'; +import { CodeActionsExtensionPoint, ContributedCodeAction } from 'vs/workbench/contrib/codeActions/common/codeActionsExtensionPoint'; import { IExtensionPoint } from 'vs/workbench/services/extensions/common/extensionsRegistry'; import { editorConfigurationBaseNode } from 'vs/editor/common/config/commonEditorConfig'; @@ -50,7 +50,7 @@ export const editorConfiguration = Object.freeze({ } }); -export class CodeActionWorkbenchConfigurationContribution extends Disposable implements IWorkbenchContribution { +export class CodeActionsContribution extends Disposable implements IWorkbenchContribution { private _contributedCodeActions: CodeActionsExtensionPoint[] = []; @@ -58,7 +58,7 @@ export class CodeActionWorkbenchConfigurationContribution extends Disposable imp constructor( codeActionsExtensionPoint: IExtensionPoint, - keybindingService: IKeybindingService, + @IKeybindingService keybindingService: IKeybindingService, ) { super(); diff --git a/src/vs/workbench/contrib/codeActions/common/extensionPoint.ts b/src/vs/workbench/contrib/codeActions/common/codeActionsExtensionPoint.ts similarity index 100% rename from src/vs/workbench/contrib/codeActions/common/extensionPoint.ts rename to src/vs/workbench/contrib/codeActions/common/codeActionsExtensionPoint.ts diff --git a/src/vs/workbench/contrib/codeActions/common/documentationContribution.ts b/src/vs/workbench/contrib/codeActions/common/documentationContribution.ts new file mode 100644 index 00000000000..cc26586e2b4 --- /dev/null +++ b/src/vs/workbench/contrib/codeActions/common/documentationContribution.ts @@ -0,0 +1,91 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { CancellationToken } from 'vs/base/common/cancellation'; +import { Disposable } from 'vs/base/common/lifecycle'; +import { Range } from 'vs/editor/common/core/range'; +import { Selection } from 'vs/editor/common/core/selection'; +import { ITextModel } from 'vs/editor/common/model'; +import { CodeAction, CodeActionContext, CodeActionList, CodeActionProvider, CodeActionProviderRegistry } from 'vs/editor/common/modes'; +import { CodeActionKind } from 'vs/editor/contrib/codeAction/types'; +import { ContextKeyExpr, IContextKeyService } from 'vs/platform/contextkey/common/contextkey'; +import { IWorkbenchContribution } from 'vs/workbench/common/contributions'; +import { IExtensionPoint } from 'vs/workbench/services/extensions/common/extensionsRegistry'; +import { DocumentationExtensionPoint } from './documentationExtensionPoint'; + + +export class CodeActionDocumentationContribution extends Disposable implements IWorkbenchContribution, CodeActionProvider { + + private contributions: { + title: string; + when: ContextKeyExpr; + command: string; + }[] = []; + + constructor( + extensionPoint: IExtensionPoint, + @IContextKeyService private readonly contextKeyService: IContextKeyService, + ) { + super(); + + CodeActionProviderRegistry.register('*', this); + + extensionPoint.setHandler(points => { + this.contributions = []; + for (const documentation of points) { + if (!documentation.value.refactoring) { + continue; + } + + for (const contribution of documentation.value.refactoring) { + const precondition = ContextKeyExpr.deserialize(contribution.when); + if (!precondition) { + continue; + } + + this.contributions.push({ + title: contribution.title, + when: precondition, + command: contribution.command + }); + + } + } + }); + } + + async provideCodeActions(_model: ITextModel, _range: Range | Selection, context: CodeActionContext, _token: CancellationToken): Promise { + if (!context.only || !CodeActionKind.Refactor.contains(new CodeActionKind(context.only))) { + return { + actions: [], + dispose: () => { } + }; + } + + const actions: CodeAction[] = []; + + for (const contribution of this.contributions) { + if (!this.contextKeyService.contextMatchesRules(contribution.when)) { + continue; + } + + actions.push({ + title: contribution.title, + kind: CodeActionKind.RefactorDocumentation.value, + command: { + id: contribution.command, + title: contribution.title + } + }); + } + + return { + actions, + dispose: () => { } + }; + } + + public readonly providedCodeActionKinds = [CodeActionKind.RefactorDocumentation.value] as const; +} diff --git a/src/vs/workbench/contrib/codeActions/common/documentationExtensionPoint.ts b/src/vs/workbench/contrib/codeActions/common/documentationExtensionPoint.ts new file mode 100644 index 00000000000..bb848f8d64d --- /dev/null +++ b/src/vs/workbench/contrib/codeActions/common/documentationExtensionPoint.ts @@ -0,0 +1,64 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as nls from 'vs/nls'; +import { IConfigurationPropertySchema } from 'vs/platform/configuration/common/configurationRegistry'; +import { languagesExtPoint } from 'vs/workbench/services/mode/common/workbenchModeService'; + +export enum DocumentationExtensionPointFields { + when = 'when', + title = 'title', + command = 'command', +} + +export interface RefactoringDocumentationExtensionPoint { + readonly [DocumentationExtensionPointFields.title]: string; + readonly [DocumentationExtensionPointFields.when]: string; + readonly [DocumentationExtensionPointFields.command]: string; +} + +export interface DocumentationExtensionPoint { + readonly refactoring?: readonly RefactoringDocumentationExtensionPoint[]; +} + +const documentationExtensionPointSchema = Object.freeze({ + type: 'object', + description: nls.localize('contributes.documentation', "Contributed documentation."), + properties: { + 'refactoring': { + type: 'array', + description: nls.localize('contributes.documentation.refactorings', "Contributed documentation for refactorings."), + items: { + type: 'object', + description: nls.localize('contributes.documentation.refactoring', "Contributed documentation for refactoring."), + required: [ + DocumentationExtensionPointFields.title, + DocumentationExtensionPointFields.when, + DocumentationExtensionPointFields.command + ], + properties: { + [DocumentationExtensionPointFields.title]: { + type: 'string', + description: nls.localize('contributes.documentation.refactoring.title', "Label for the documentation used in the UI."), + }, + [DocumentationExtensionPointFields.when]: { + type: 'string', + description: nls.localize('contributes.documentation.refactoring.when', "When clause."), + }, + [DocumentationExtensionPointFields.command]: { + type: 'string', + description: nls.localize('contributes.documentation.refactoring.command', "Command executed."), + }, + }, + } + } + } +}); + +export const documentationExtensionPointDescriptor = { + extensionPoint: 'documentation', + deps: [languagesExtPoint], + jsonSchema: documentationExtensionPointSchema +}; From 80ccf6fd9d317a5f812f1c98c6ddd2a853ac6b32 Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Tue, 7 Jan 2020 16:10:08 -0800 Subject: [PATCH 32/36] Make sure we always explicitly reset pendingGetErr #88209 --- .../src/features/bufferSyncSupport.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/extensions/typescript-language-features/src/features/bufferSyncSupport.ts b/extensions/typescript-language-features/src/features/bufferSyncSupport.ts index 43775c78ca0..6a86599bcbe 100644 --- a/extensions/typescript-language-features/src/features/bufferSyncSupport.ts +++ b/extensions/typescript-language-features/src/features/bufferSyncSupport.ts @@ -547,6 +547,8 @@ export default class BufferSyncSupport extends Disposable { orderedFileSet.set(resource, undefined); } } + + this.pendingGetErr = undefined; } // Add all open TS buffers to the geterr request. They might be visible From 349c36e92118194b751b461227c338638a7d9e72 Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Tue, 7 Jan 2020 17:40:41 -0800 Subject: [PATCH 33/36] Unhide some common launch configs --- .vscode/launch.json | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index ccbf4a70ae3..558cc9f0f3b 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -141,10 +141,7 @@ "type": "chrome", "request": "attach", "name": "Attach to VS Code", - "port": 9222, - "presentation": { - "hidden": true - } + "port": 9222 }, { "type": "chrome", @@ -170,10 +167,7 @@ "--inspect=5875", "--no-cached-data" ], - "webRoot": "${workspaceFolder}", - "presentation": { - "hidden": true - } + "webRoot": "${workspaceFolder}" }, { "type": "node", From cba31b819ea99836daf51cee56299348e9c6d20b Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Tue, 7 Jan 2020 17:47:48 -0800 Subject: [PATCH 34/36] Register MockDebugService and add mocks that are used by the callstack and bp editor contributions, to fix tests --- .../contrib/debug/test/common/mockDebug.ts | 124 ++++++++++++++++-- .../workbench/test/workbenchTestServices.ts | 3 + 2 files changed, 116 insertions(+), 11 deletions(-) diff --git a/src/vs/workbench/contrib/debug/test/common/mockDebug.ts b/src/vs/workbench/contrib/debug/test/common/mockDebug.ts index 14d4a539a74..24625cbbd51 100644 --- a/src/vs/workbench/contrib/debug/test/common/mockDebug.ts +++ b/src/vs/workbench/contrib/debug/test/common/mockDebug.ts @@ -3,19 +3,29 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { URI as uri } from 'vs/base/common/uri'; -import { Event } from 'vs/base/common/event'; -import { IWorkspaceFolder } from 'vs/platform/workspace/common/workspace'; -import { Position, IPosition } from 'vs/editor/common/core/position'; -import { ILaunch, IDebugService, State, IDebugSession, IConfigurationManager, IStackFrame, IBreakpointData, IBreakpointUpdateData, IConfig, IDebugModel, IViewModel, IBreakpoint, LoadedSourceEvent, IThread, IRawModelUpdate, IFunctionBreakpoint, IExceptionBreakpoint, IDebugger, IExceptionInfo, AdapterEndEvent, IReplElement, IExpression, IReplElementSource, IDataBreakpoint, IDebugSessionOptions } from 'vs/workbench/contrib/debug/common/debug'; -import { Source } from 'vs/workbench/contrib/debug/common/debugSource'; +import { Emitter, Event } from 'vs/base/common/event'; import Severity from 'vs/base/common/severity'; +import { URI as uri } from 'vs/base/common/uri'; +import { IPosition, Position } from 'vs/editor/common/core/position'; +import { IWorkspaceFolder } from 'vs/platform/workspace/common/workspace'; import { AbstractDebugAdapter } from 'vs/workbench/contrib/debug/common/abstractDebugAdapter'; +import { AdapterEndEvent, IBreakpoint, IBreakpointData, IBreakpointsChangeEvent, IBreakpointUpdateData, IConfig, IConfigurationManager, IDataBreakpoint, IDebugger, IDebugModel, IDebugService, IDebugSession, IDebugSessionOptions, IEvaluate, IExceptionBreakpoint, IExceptionInfo, IExpression, IFunctionBreakpoint, ILaunch, IRawModelUpdate, IReplElement, IReplElementSource, IStackFrame, IThread, IViewModel, LoadedSourceEvent, State } from 'vs/workbench/contrib/debug/common/debug'; +import { Source } from 'vs/workbench/contrib/debug/common/debugSource'; + +const noopEvent = new Emitter().event; export class MockDebugService implements IDebugService { public _serviceBrand: undefined; + private readonly _model: IDebugModel; + private readonly _viewModel: IViewModel; + + constructor() { + this._model = new MockDebugModel(); + this._viewModel = new MockDebugViewModel(); + } + public get state(): State { throw new Error('not implemented'); } @@ -32,9 +42,7 @@ export class MockDebugService implements IDebugService { throw new Error('not implemented'); } - public get onDidChangeState(): Event { - throw new Error('not implemented'); - } + public onDidChangeState: Event = noopEvent; public getConfigurationManager(): IConfigurationManager { throw new Error('not implemented'); @@ -116,11 +124,11 @@ export class MockDebugService implements IDebugService { } public getModel(): IDebugModel { - throw new Error('not implemented'); + return this._model; } public getViewModel(): IViewModel { - throw new Error('not implemented'); + return this._viewModel; } public logToRepl(session: IDebugSession, value: string): void { } @@ -528,3 +536,97 @@ export class MockDebugAdapter extends AbstractDebugAdapter { } } } + +class MockDebugModel implements IDebugModel { + onDidChangeBreakpoints: Event = noopEvent; + + get onDidChangeCallStack(): Event { + throw new Error('not implemented'); + } + + get onDidChangeWatchExpressions(): Event { + throw new Error('not implemented'); + } + + getSession(sessionId: string | undefined, includeInactive?: boolean | undefined): IDebugSession | undefined { + throw new Error('not implemented.'); + } + + getSessions(includeInactive?: boolean | undefined): IDebugSession[] { + return []; + } + + getBreakpoints(filter?: { uri?: uri | undefined; lineNumber?: number | undefined; column?: number | undefined; enabledOnly?: boolean | undefined; } | undefined): readonly IBreakpoint[] { + return []; + } + + areBreakpointsActivated(): boolean { + throw new Error('not implemented.'); + } + + getFunctionBreakpoints(): readonly IFunctionBreakpoint[] { + throw new Error('not implemented.'); + } + + getDataBreakpoints(): readonly IDataBreakpoint[] { + throw new Error('not implemented.'); + } + + getExceptionBreakpoints(): readonly IExceptionBreakpoint[] { + throw new Error('not implemented.'); + } + + getWatchExpressions(): readonly (IExpression & IEvaluate)[] { + throw new Error('not implemented.'); + } + + getId(): string { + throw new Error('not implemented.'); + } +} + +class MockDebugViewModel implements IViewModel { + get focusedSession(): IDebugSession | undefined { + throw new Error('not implemented'); + } + + get focusedThread(): IThread | undefined { + throw new Error('not implemented'); + } + + focusedStackFrame: IStackFrame | undefined = undefined; + + get onDidFocusSession(): Event { + throw new Error('not implemented'); + } + + onDidFocusStackFrame: Event<{ stackFrame: IStackFrame | undefined; explicit: boolean; }> = noopEvent; + + get onDidSelectExpression(): Event { + throw new Error('not implemented'); + } + + getSelectedExpression(): IExpression | undefined { + throw new Error('Method not implemented.'); + } + + getSelectedFunctionBreakpoint(): IFunctionBreakpoint | undefined { + throw new Error('Method not implemented.'); + } + + setSelectedExpression(expression: IExpression | undefined): void { + throw new Error('Method not implemented.'); + } + + setSelectedFunctionBreakpoint(functionBreakpoint: IFunctionBreakpoint | undefined): void { + throw new Error('Method not implemented.'); + } + + isMultiSessionView(): boolean { + throw new Error('Method not implemented.'); + } + + getId(): string { + throw new Error('Method not implemented.'); + } +} diff --git a/src/vs/workbench/test/workbenchTestServices.ts b/src/vs/workbench/test/workbenchTestServices.ts index 868e183d45c..f611fe3763d 100644 --- a/src/vs/workbench/test/workbenchTestServices.ts +++ b/src/vs/workbench/test/workbenchTestServices.ts @@ -95,6 +95,8 @@ import { find } from 'vs/base/common/arrays'; import { WorkingCopyService, IWorkingCopyService } from 'vs/workbench/services/workingCopy/common/workingCopyService'; import { IFilesConfigurationService, FilesConfigurationService } from 'vs/workbench/services/filesConfiguration/common/filesConfigurationService'; import { IAccessibilityService, AccessibilitySupport } from 'vs/platform/accessibility/common/accessibility'; +import { IDebugService } from 'vs/workbench/contrib/debug/common/debug'; +import { MockDebugService } from 'vs/workbench/contrib/debug/test/common/mockDebug'; export function createFileInput(instantiationService: IInstantiationService, resource: URI): FileEditorInput { return instantiationService.createInstance(FileEditorInput, resource, undefined, undefined); @@ -324,6 +326,7 @@ export function workbenchInstantiationService(): ITestInstantiationService { instantiationService.stub(ICodeEditorService, new TestCodeEditorService()); instantiationService.stub(IViewletService, new TestViewletService()); instantiationService.stub(IWorkingCopyService, new TestWorkingCopyService()); + instantiationService.stub(IDebugService, new MockDebugService()); return instantiationService; } From f8233c9f32123d0712d9d8a118f847732626fc05 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Wed, 8 Jan 2020 07:22:53 +0100 Subject: [PATCH 35/36] editors - introduce IEditorInput.isSaving() So far, file editors have reported isDirty() === false when auto save was enabled e.g. to avoid showing dirty indicators. With the advent of custom editors, this needs to be cleaned up. The new method isSaving() serves as a hint that an editor can be dirty but in the process of being saved. As such, we can decide to hide the dirty indicator in that case. But in other cases we may want to know if the editor is dirty or not, even if auto saved. --- .../browser/parts/editor/editorActions.ts | 6 ++-- .../browser/parts/editor/editorGroupView.ts | 20 +++++------ .../browser/parts/editor/editorPicker.ts | 2 +- .../browser/parts/editor/editorsObserver.ts | 4 +-- .../parts/editor/noTabsTitleControl.ts | 9 +++-- .../browser/parts/editor/tabsTitleControl.ts | 4 +-- .../parts/quickopen/quickOpenController.ts | 23 +++++++----- .../browser/parts/titlebar/titlebarPart.ts | 2 +- src/vs/workbench/common/editor.ts | 36 +++++++++++++------ .../common/editor/untitledTextEditorModel.ts | 6 ++-- .../customEditor/browser/customEditorInput.ts | 14 ++++++++ .../files/browser/views/openEditorsView.ts | 6 ++-- .../files/common/dirtyFilesIndicator.ts | 2 +- .../files/common/editors/fileEditorInput.ts | 22 +++++++++--- .../workbench/contrib/files/common/files.ts | 4 +++ src/vs/workbench/electron-browser/window.ts | 2 +- .../editor/test/browser/editorService.test.ts | 16 ++++----- .../textfile/common/textFileEditorModel.ts | 4 +-- .../workingCopy/common/workingCopyService.ts | 7 ++-- 19 files changed, 125 insertions(+), 64 deletions(-) diff --git a/src/vs/workbench/browser/parts/editor/editorActions.ts b/src/vs/workbench/browser/parts/editor/editorActions.ts index 28cfc1b0ace..83632b3ef2b 100644 --- a/src/vs/workbench/browser/parts/editor/editorActions.ts +++ b/src/vs/workbench/browser/parts/editor/editorActions.ts @@ -632,7 +632,7 @@ export abstract class BaseCloseAllAction extends Action { // can review if the files should be changed or not. await Promise.all(this.groupsToClose.map(async groupToClose => { for (const editor of groupToClose.getEditors(EditorsOrder.MOST_RECENTLY_ACTIVE)) { - if (editor.isDirty()) { + if (editor.isDirty() && !editor.isSaving() /* ignore editors that are being saved */) { return groupToClose.openEditor(editor); } } @@ -644,8 +644,8 @@ export abstract class BaseCloseAllAction extends Action { const dirtyEditorsToConfirmByResource = new ResourceMap(); for (const editor of this.editorService.editors) { - if (!editor.isDirty()) { - continue; // only interested in dirty editors + if (!editor.isDirty() || editor.isSaving()) { + continue; // only interested in dirty editors (unless in the process of saving) } const resource = editor.getResource(); diff --git a/src/vs/workbench/browser/parts/editor/editorGroupView.ts b/src/vs/workbench/browser/parts/editor/editorGroupView.ts index e6fb5560aba..fd6d2e95511 100644 --- a/src/vs/workbench/browser/parts/editor/editorGroupView.ts +++ b/src/vs/workbench/browser/parts/editor/editorGroupView.ts @@ -226,8 +226,8 @@ export class EditorGroupView extends Themable implements IEditorGroupView { const activeEditor = this._group.activeEditor; if (activeEditor) { - groupActiveEditorDirtyContextKey.set(activeEditor.isDirty()); - activeEditorListener.value = activeEditor.onDidChangeDirty(() => groupActiveEditorDirtyContextKey.set(activeEditor.isDirty())); + groupActiveEditorDirtyContextKey.set(activeEditor.isDirty() && !activeEditor.isSaving()); + activeEditorListener.value = activeEditor.onDidChangeDirty(() => groupActiveEditorDirtyContextKey.set(activeEditor.isDirty() && !activeEditor.isSaving())); } else { groupActiveEditorDirtyContextKey.set(false); } @@ -1271,8 +1271,8 @@ export class EditorGroupView extends Themable implements IEditorGroupView { } private async doHandleDirtyClosing(editor: EditorInput): Promise { - if (!editor.isDirty()) { - return false; // editor must be dirty + if (!editor.isDirty() || editor.isSaving()) { + return false; // editor must be dirty and not saving } if (editor instanceof SideBySideEditorInput && this.isOpened(editor.master)) { @@ -1309,11 +1309,11 @@ export class EditorGroupView extends Themable implements IEditorGroupView { const editorResource = toResource(editor, { supportSideBySide: SideBySideEditor.MASTER }); const res = await this.fileDialogService.showSaveConfirm(editorResource ? [editorResource] : [editor.getName()]); - // It could be that the editor saved meanwhile, so we check again - // to see if anything needs to happen before closing for good. + // It could be that the editor saved meanwhile or is saving, so we check + // again to see if anything needs to happen before closing for good. // This can happen for example if autoSave: onFocusChange is configured // so that the save happens when the dialog opens. - if (!editor.isDirty()) { + if (!editor.isDirty() || editor.isSaving()) { return res === ConfirmResult.CANCEL ? true : false; } @@ -1376,9 +1376,9 @@ export class EditorGroupView extends Themable implements IEditorGroupView { let editorsToClose = this._group.getEditors(hasDirection ? EditorsOrder.SEQUENTIAL : EditorsOrder.MOST_RECENTLY_ACTIVE); // in MRU order only if direction is not specified - // Filter: saved only + // Filter: saved or saving only if (filter.savedOnly) { - editorsToClose = editorsToClose.filter(e => !e.isDirty()); + editorsToClose = editorsToClose.filter(e => !e.isDirty() || e.isSaving()); } // Filter: direction (left / right) @@ -1471,7 +1471,7 @@ export class EditorGroupView extends Themable implements IEditorGroupView { let activeReplacement: EditorReplacement | undefined; const inactiveReplacements: EditorReplacement[] = []; editors.forEach(({ editor, replacement, options }) => { - if (editor.isDirty()) { + if (editor.isDirty() && !editor.isSaving()) { return; // we do not handle dirty in this method, so ignore all dirty } diff --git a/src/vs/workbench/browser/parts/editor/editorPicker.ts b/src/vs/workbench/browser/parts/editor/editorPicker.ts index efc8a15b98d..213367495bf 100644 --- a/src/vs/workbench/browser/parts/editor/editorPicker.ts +++ b/src/vs/workbench/browser/parts/editor/editorPicker.ts @@ -42,7 +42,7 @@ export class EditorPickerEntry extends QuickOpenEntryGroup { } getIcon(): string { - return this.editor.isDirty() ? 'codicon codicon-circle-filled' : ''; + return this.editor.isDirty() && !this.editor.isSaving() ? 'codicon codicon-circle-filled' : ''; } get group(): IEditorGroup { diff --git a/src/vs/workbench/browser/parts/editor/editorsObserver.ts b/src/vs/workbench/browser/parts/editor/editorsObserver.ts index 0545c9710d1..b370de392c8 100644 --- a/src/vs/workbench/browser/parts/editor/editorsObserver.ts +++ b/src/vs/workbench/browser/parts/editor/editorsObserver.ts @@ -260,8 +260,8 @@ export class EditorsObserver extends Disposable { // Extract least recently used editors that can be closed const leastRecentlyClosableEditors = mostRecentEditors.reverse().filter(({ editor, groupId }) => { - if (editor.isDirty()) { - return false; // not dirty editors + if (editor.isDirty() && !editor.isSaving()) { + return false; // not dirty editors (unless in the process of saving) } if (exclude && editor === exclude.editor && groupId === exclude.groupId) { diff --git a/src/vs/workbench/browser/parts/editor/noTabsTitleControl.ts b/src/vs/workbench/browser/parts/editor/noTabsTitleControl.ts index dd709b9e85c..211fdec8ce1 100644 --- a/src/vs/workbench/browser/parts/editor/noTabsTitleControl.ts +++ b/src/vs/workbench/browser/parts/editor/noTabsTitleControl.ts @@ -166,9 +166,14 @@ export class NoTabsTitleControl extends TitleControl { updateEditorDirty(editor: IEditorInput): void { this.ifEditorIsActive(editor, () => { const titleContainer = assertIsDefined(this.titleContainer); - if (editor.isDirty()) { + + // Signal dirty (unless saving) + if (editor.isDirty() && !editor.isSaving()) { addClass(titleContainer, 'dirty'); - } else { + } + + // Otherwise, clear dirty + else { removeClass(titleContainer, 'dirty'); } }); diff --git a/src/vs/workbench/browser/parts/editor/tabsTitleControl.ts b/src/vs/workbench/browser/parts/editor/tabsTitleControl.ts index f459d1ed60d..6011bbc2f45 100644 --- a/src/vs/workbench/browser/parts/editor/tabsTitleControl.ts +++ b/src/vs/workbench/browser/parts/editor/tabsTitleControl.ts @@ -1001,8 +1001,8 @@ export class TabsTitleControl extends TitleControl { private doRedrawEditorDirty(isGroupActive: boolean, isTabActive: boolean, editor: IEditorInput, tabContainer: HTMLElement): boolean { let hasModifiedBorderColor = false; - // Tab: dirty - if (editor.isDirty()) { + // Tab: dirty (unless saving) + if (editor.isDirty() && !editor.isSaving()) { addClass(tabContainer, 'dirty'); // Highlight modified tabs with a border if configured diff --git a/src/vs/workbench/browser/parts/quickopen/quickOpenController.ts b/src/vs/workbench/browser/parts/quickopen/quickOpenController.ts index 91a2da5a52b..265dc48427a 100644 --- a/src/vs/workbench/browser/parts/quickopen/quickOpenController.ts +++ b/src/vs/workbench/browser/parts/quickopen/quickOpenController.ts @@ -726,7 +726,7 @@ export class EditorHistoryEntry extends EditorQuickOpenEntry { private resource: URI | undefined; private label: string; private description?: string; - private dirty: boolean; + private icon: string; constructor( input: IEditorInput | IResourceInput, @@ -747,22 +747,29 @@ export class EditorHistoryEntry extends EditorQuickOpenEntry { this.resource = resourceForEditorHistory(input, fileService); this.label = input.getName(); this.description = input.getDescription(); - this.dirty = input.isDirty(); + this.icon = this.getDirtyIndicatorForEditor(input); } else { const resourceInput = input as IResourceInput; this.resource = resourceInput.resource; this.label = resources.basenameOrAuthority(resourceInput.resource); this.description = labelService.getUriLabel(resources.dirname(this.resource), { relative: true }); - this.dirty = this.resource && this.textFileService.isDirty(this.resource); - - if (this.dirty && this.filesConfigurationService.getAutoSaveMode() === AutoSaveMode.AFTER_SHORT_DELAY) { - this.dirty = false; // no dirty decoration if auto save is on with a short timeout - } + this.icon = this.getDirtyIndicatorForEditor(resourceInput); } } + private getDirtyIndicatorForEditor(input: EditorInput | IResourceInput): string { + let signalDirty = false; + if (input instanceof EditorInput) { + signalDirty = input.isDirty() && !input.isSaving(); + } else { + signalDirty = this.textFileService.isDirty(input.resource) && this.filesConfigurationService.getAutoSaveMode() !== AutoSaveMode.AFTER_SHORT_DELAY; + } + + return signalDirty ? 'codicon codicon-circle-filled' : ''; + } + getIcon(): string { - return this.dirty ? 'codicon codicon-circle-filled' : ''; + return this.icon; } getLabel(): string { diff --git a/src/vs/workbench/browser/parts/titlebar/titlebarPart.ts b/src/vs/workbench/browser/parts/titlebar/titlebarPart.ts index 670a91764ac..d00ec667dee 100644 --- a/src/vs/workbench/browser/parts/titlebar/titlebarPart.ts +++ b/src/vs/workbench/browser/parts/titlebar/titlebarPart.ts @@ -316,7 +316,7 @@ export class TitlebarPart extends Part implements ITitleService { const rootPath = root ? this.labelService.getUriLabel(root) : ''; const folderName = folder ? folder.name : ''; const folderPath = folder ? this.labelService.getUriLabel(folder.uri) : ''; - const dirty = editor?.isDirty() ? TitlebarPart.TITLE_DIRTY : ''; + const dirty = editor?.isDirty() && !editor.isSaving() ? TitlebarPart.TITLE_DIRTY : ''; const appName = this.productService.nameLong; const remoteName = this.labelService.getHostLabel(REMOTE_HOST_SCHEME, this.environmentService.configuration.remoteAuthority); const separator = TitlebarPart.TITLE_SEPARATOR; diff --git a/src/vs/workbench/common/editor.ts b/src/vs/workbench/common/editor.ts index 7f8dc048690..92b1874ffcf 100644 --- a/src/vs/workbench/common/editor.ts +++ b/src/vs/workbench/common/editor.ts @@ -403,6 +403,14 @@ export interface IEditorInput extends IDisposable { */ isDirty(): boolean; + /** + * Returns if this input is currently being saved or soon to be + * saved. Based on this assumption the editor may for example + * decide to not signal the dirty state to the user assuming that + * the save is scheduled to happen anyway. + */ + isSaving(): boolean; + /** * Saves the editor. The provided groupId helps * implementors to e.g. preserve view state of the editor @@ -508,16 +516,20 @@ export abstract class EditorInput extends Disposable implements IEditorInput { return false; } - save(groupId: GroupIdentifier, options?: ISaveOptions): Promise { - return Promise.resolve(true); + isSaving(): boolean { + return false; } - saveAs(groupId: GroupIdentifier, options?: ISaveOptions): Promise { - return Promise.resolve(true); + async save(groupId: GroupIdentifier, options?: ISaveOptions): Promise { + return true; } - revert(options?: IRevertOptions): Promise { - return Promise.resolve(true); + async saveAs(groupId: GroupIdentifier, options?: ISaveOptions): Promise { + return true; + } + + async revert(options?: IRevertOptions): Promise { + return true; } /** @@ -701,6 +713,10 @@ export class SideBySideEditorInput extends EditorInput { return this.master.isDirty(); } + isSaving(): boolean { + return this.master.isSaving(); + } + save(groupId: GroupIdentifier, options?: ISaveOptions): Promise { return this.master.save(groupId, options); } @@ -741,8 +757,8 @@ export class SideBySideEditorInput extends EditorInput { this._register(this.master.onDidChangeLabel(() => this._onDidChangeLabel.fire())); } - resolve(): Promise { - return Promise.resolve(null); + async resolve(): Promise { + return null; } getTypeId(): string { @@ -791,8 +807,8 @@ export class EditorModel extends Disposable implements IEditorModel { /** * Causes this model to load returning a promise when loading is completed. */ - load(): Promise { - return Promise.resolve(this); + async load(): Promise { + return this; } /** diff --git a/src/vs/workbench/common/editor/untitledTextEditorModel.ts b/src/vs/workbench/common/editor/untitledTextEditorModel.ts index b52f38c056b..a34992acbc9 100644 --- a/src/vs/workbench/common/editor/untitledTextEditorModel.ts +++ b/src/vs/workbench/common/editor/untitledTextEditorModel.ts @@ -16,7 +16,7 @@ import { ITextResourceConfigurationService } from 'vs/editor/common/services/tex import { ITextBufferFactory } from 'vs/editor/common/model'; import { createTextBufferFactory } from 'vs/editor/common/model/textModel'; import { IResolvedTextEditorModel } from 'vs/editor/common/services/resolverService'; -import { IWorkingCopyService, IWorkingCopy } from 'vs/workbench/services/workingCopy/common/workingCopyService'; +import { IWorkingCopyService, IWorkingCopy, WorkingCopyCapabilities } from 'vs/workbench/services/workingCopy/common/workingCopyService'; import { ITextFileService } from 'vs/workbench/services/textfile/common/textfiles'; export class UntitledTextEditorModel extends BaseTextEditorModel implements IEncodingSupport, IWorkingCopy { @@ -32,12 +32,12 @@ export class UntitledTextEditorModel extends BaseTextEditorModel implements IEnc private readonly _onDidChangeEncoding: Emitter = this._register(new Emitter()); readonly onDidChangeEncoding: Event = this._onDidChangeEncoding.event; - readonly capabilities = 0; + readonly capabilities = WorkingCopyCapabilities.Untitled; private dirty = false; private versionId = 0; private readonly contentChangeEventScheduler = this._register(new RunOnceScheduler(() => this._onDidChangeContent.fire(), UntitledTextEditorModel.DEFAULT_CONTENT_CHANGE_BUFFER_DELAY)); - private configuredEncoding?: string; + private configuredEncoding: string | undefined; constructor( private readonly preferredMode: string | undefined, diff --git a/src/vs/workbench/contrib/customEditor/browser/customEditorInput.ts b/src/vs/workbench/contrib/customEditor/browser/customEditorInput.ts index 483d0490e38..1ff775a4dc3 100644 --- a/src/vs/workbench/contrib/customEditor/browser/customEditorInput.ts +++ b/src/vs/workbench/contrib/customEditor/browser/customEditorInput.ts @@ -21,6 +21,7 @@ import { FileEditorInput } from 'vs/workbench/contrib/files/common/editors/fileE import { WebviewEditorOverlay } from 'vs/workbench/contrib/webview/browser/webview'; import { IWebviewWorkbenchService, LazilyResolvedWebviewEditorInput } from 'vs/workbench/contrib/webview/browser/webviewWorkbenchService'; import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; +import { IFilesConfigurationService, AutoSaveMode } from 'vs/workbench/services/filesConfiguration/common/filesConfigurationService'; export class CustomFileEditorInput extends LazilyResolvedWebviewEditorInput { @@ -41,6 +42,7 @@ export class CustomFileEditorInput extends LazilyResolvedWebviewEditorInput { @ICustomEditorService private readonly customEditorService: ICustomEditorService, @IEditorService private readonly editorService: IEditorService, @IFileDialogService private readonly fileDialogService: IFileDialogService, + @IFilesConfigurationService private readonly filesConfigurationService: IFilesConfigurationService ) { super(id, viewType, '', webview, webviewWorkbenchService, lifecycleService); this._editorResource = resource; @@ -109,6 +111,18 @@ export class CustomFileEditorInput extends LazilyResolvedWebviewEditorInput { return this._model ? this._model.isDirty() : false; } + public isSaving(): boolean { + if (!this.isDirty()) { + return false; // the editor needs to be dirty for being saved + } + + if (this.filesConfigurationService.getAutoSaveMode() === AutoSaveMode.AFTER_SHORT_DELAY) { + return true; // a short auto save is configured, treat this as being saved + } + + return false; + } + public save(groupId: GroupIdentifier, options?: ISaveOptions): Promise { return this._model ? this._model.save(options) : Promise.resolve(false); } diff --git a/src/vs/workbench/contrib/files/browser/views/openEditorsView.ts b/src/vs/workbench/contrib/files/browser/views/openEditorsView.ts index 2760b5c5659..f5166ae8ee8 100644 --- a/src/vs/workbench/contrib/files/browser/views/openEditorsView.ts +++ b/src/vs/workbench/contrib/files/browser/views/openEditorsView.ts @@ -249,7 +249,7 @@ export class OpenEditorsView extends ViewPane { const element = e.elements.length ? e.elements[0] : undefined; if (element instanceof OpenEditor) { const resource = element.getResource(); - this.dirtyEditorFocusedContext.set(element.editor.isDirty()); + this.dirtyEditorFocusedContext.set(element.editor.isDirty() && !element.editor.isSaving()); this.readonlyEditorFocusedContext.set(element.editor.isReadonly()); this.resourceContext.set(withUndefinedAsNull(resource)); } else if (!!element) { @@ -419,7 +419,7 @@ export class OpenEditorsView extends ViewPane { private updateDirtyIndicator(workingCopy?: IWorkingCopy): void { if (workingCopy) { const gotDirty = workingCopy.isDirty(); - if (gotDirty && !!(workingCopy.capabilities & WorkingCopyCapabilities.AutoSave) && this.filesConfigurationService.getAutoSaveMode() === AutoSaveMode.AFTER_SHORT_DELAY) { + if (gotDirty && !(workingCopy.capabilities & WorkingCopyCapabilities.Untitled) && this.filesConfigurationService.getAutoSaveMode() === AutoSaveMode.AFTER_SHORT_DELAY) { return; // do not indicate dirty of working copies that are auto saved after short delay } } @@ -586,7 +586,7 @@ class OpenEditorRenderer implements IListRenderer { diff --git a/src/vs/workbench/contrib/files/common/files.ts b/src/vs/workbench/contrib/files/common/files.ts index c2bdc69cf7b..066cbd51ec5 100644 --- a/src/vs/workbench/contrib/files/common/files.ts +++ b/src/vs/workbench/contrib/files/common/files.ts @@ -246,6 +246,10 @@ export class OpenEditor implements IEditorIdentifier { return this.editor.isDirty(); } + isSaving(): boolean { + return this.editor.isSaving(); + } + getResource(): URI | undefined { return toResource(this.editor, { supportSideBySide: SideBySideEditor.MASTER }); } diff --git a/src/vs/workbench/electron-browser/window.ts b/src/vs/workbench/electron-browser/window.ts index 02735f4a472..84d3ec53cbf 100644 --- a/src/vs/workbench/electron-browser/window.ts +++ b/src/vs/workbench/electron-browser/window.ts @@ -270,7 +270,7 @@ export class ElectronWindow extends Disposable { if (isMacintosh) { this._register(this.workingCopyService.onDidChangeDirty(workingCopy => { const gotDirty = workingCopy.isDirty(); - if (gotDirty && !!(workingCopy.capabilities & WorkingCopyCapabilities.AutoSave) && this.filesConfigurationService.getAutoSaveMode() === AutoSaveMode.AFTER_SHORT_DELAY) { + if (gotDirty && !(workingCopy.capabilities & WorkingCopyCapabilities.Untitled) && this.filesConfigurationService.getAutoSaveMode() === AutoSaveMode.AFTER_SHORT_DELAY) { return; // do not indicate dirty of working copies that are auto saved after short delay } diff --git a/src/vs/workbench/services/editor/test/browser/editorService.test.ts b/src/vs/workbench/services/editor/test/browser/editorService.test.ts index aa2a004dff2..fe74fba0aea 100644 --- a/src/vs/workbench/services/editor/test/browser/editorService.test.ts +++ b/src/vs/workbench/services/editor/test/browser/editorService.test.ts @@ -73,19 +73,19 @@ class TestEditorInput extends EditorInput implements IFileEditorInput { setFailToOpen(): void { this.fails = true; } - save(groupId: GroupIdentifier, options?: ISaveOptions): Promise { + async save(groupId: GroupIdentifier, options?: ISaveOptions): Promise { this.gotSaved = true; - return Promise.resolve(true); + return true; } - saveAs(groupId: GroupIdentifier, options?: ISaveOptions): Promise { + async saveAs(groupId: GroupIdentifier, options?: ISaveOptions): Promise { this.gotSavedAs = true; - return Promise.resolve(true); + return true; } - revert(options?: IRevertOptions): Promise { + async revert(options?: IRevertOptions): Promise { this.gotReverted = true; this.gotSaved = false; this.gotSavedAs = false; - return Promise.resolve(true); + return true; } isDirty(): boolean { return this.dirty; @@ -383,12 +383,12 @@ suite('EditorService', () => { const ed = instantiationService.createInstance(MyEditor, 'my.editor'); const inp = instantiationService.createInstance(ResourceEditorInput, 'name', 'description', URI.parse('my://resource-delegate'), undefined); - const delegate = instantiationService.createInstance(DelegatingEditorService, (delegate, group, input) => { + const delegate = instantiationService.createInstance(DelegatingEditorService, async (delegate, group, input) => { assert.strictEqual(input, inp); done(); - return Promise.resolve(ed); + return ed; }); delegate.openEditor(inp); diff --git a/src/vs/workbench/services/textfile/common/textFileEditorModel.ts b/src/vs/workbench/services/textfile/common/textFileEditorModel.ts index 13c8397c189..539c595282f 100644 --- a/src/vs/workbench/services/textfile/common/textFileEditorModel.ts +++ b/src/vs/workbench/services/textfile/common/textFileEditorModel.ts @@ -29,7 +29,7 @@ import { ILogService } from 'vs/platform/log/common/log'; import { isEqual, isEqualOrParent, extname, basename, joinPath } from 'vs/base/common/resources'; import { onUnexpectedError } from 'vs/base/common/errors'; import { Schemas } from 'vs/base/common/network'; -import { IWorkingCopyService, WorkingCopyCapabilities } from 'vs/workbench/services/workingCopy/common/workingCopyService'; +import { IWorkingCopyService } from 'vs/workbench/services/workingCopy/common/workingCopyService'; import { IFilesConfigurationService, IAutoSaveConfiguration } from 'vs/workbench/services/filesConfiguration/common/filesConfigurationService'; export interface IBackupMetaData { @@ -82,7 +82,7 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil private readonly _onDidChangeDirty = this._register(new Emitter()); readonly onDidChangeDirty = this._onDidChangeDirty.event; - readonly capabilities = WorkingCopyCapabilities.AutoSave; + readonly capabilities = 0; private contentEncoding: string | undefined; // encoding as reported from disk diff --git a/src/vs/workbench/services/workingCopy/common/workingCopyService.ts b/src/vs/workbench/services/workingCopy/common/workingCopyService.ts index ae74659bc3f..55d754bb392 100644 --- a/src/vs/workbench/services/workingCopy/common/workingCopyService.ts +++ b/src/vs/workbench/services/workingCopy/common/workingCopyService.ts @@ -13,10 +13,11 @@ import { TernarySearchTree } from 'vs/base/common/map'; export const enum WorkingCopyCapabilities { /** - * Signals that the working copy participates - * in auto saving as configured by the user. + * Signals that the working copy requires + * additional input when saving, e.g. an + * associated path to save to. */ - AutoSave = 1 << 1 + Untitled = 1 << 1 } export interface IWorkingCopy { From 1f3efeb026ecbcf1f13c494ad38a393509a4507c Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Wed, 8 Jan 2020 09:09:17 +0100 Subject: [PATCH 36/36] fix #88232 --- .../gotoSymbol/peek/referencesController.ts | 31 ++++++++++++------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/src/vs/editor/contrib/gotoSymbol/peek/referencesController.ts b/src/vs/editor/contrib/gotoSymbol/peek/referencesController.ts index a140ff4501a..969f5e09d5d 100644 --- a/src/vs/editor/contrib/gotoSymbol/peek/referencesController.ts +++ b/src/vs/editor/contrib/gotoSymbol/peek/referencesController.ts @@ -329,24 +329,31 @@ KeybindingsRegistry.registerCommandAndKeybindingRule({ } }); -KeybindingsRegistry.registerCommandAndKeybindingRule({ +// commands that aren't needed anymore because there is now ContextKeyExpr.OR +CommandsRegistry.registerCommandAlias('goToNextReferenceFromEmbeddedEditor', 'goToNextReference'); +CommandsRegistry.registerCommandAlias('goToPreviousReferenceFromEmbeddedEditor', 'goToPreviousReference'); + +// close +CommandsRegistry.registerCommandAlias('closeReferenceSearchEditor', 'closeReferenceSearch'); +CommandsRegistry.registerCommand( + 'closeReferenceSearch', + accessor => withController(accessor, controller => controller.closeWidget()) +); +KeybindingsRegistry.registerKeybindingRule({ id: 'closeReferenceSearch', weight: KeybindingWeight.EditorContrib - 101, primary: KeyCode.Escape, secondary: [KeyMod.Shift | KeyCode.Escape], - when: ContextKeyExpr.or( - ContextKeyExpr.and(ctxReferenceSearchVisible, ContextKeyExpr.not('config.editor.stablePeek')), - ContextKeyExpr.and(PeekContext.inPeekEditor, ContextKeyExpr.not('config.editor.stablePeek')) - ), - handler(accessor: ServicesAccessor) { - withController(accessor, controller => controller.closeWidget()); - } + when: ContextKeyExpr.and(PeekContext.inPeekEditor, ContextKeyExpr.not('config.editor.stablePeek')) +}); +KeybindingsRegistry.registerKeybindingRule({ + id: 'closeReferenceSearch', + weight: KeybindingWeight.WorkbenchContrib + 50, + primary: KeyCode.Escape, + secondary: [KeyMod.Shift | KeyCode.Escape], + when: ContextKeyExpr.and(ctxReferenceSearchVisible, ContextKeyExpr.not('config.editor.stablePeek')) }); -// commands that aren't needed anymore because there is now ContextKeyExpr.OR -CommandsRegistry.registerCommandAlias('goToNextReferenceFromEmbeddedEditor', 'goToNextReference'); -CommandsRegistry.registerCommandAlias('goToPreviousReferenceFromEmbeddedEditor', 'goToPreviousReference'); -CommandsRegistry.registerCommandAlias('closeReferenceSearchEditor', 'closeReferenceSearch'); KeybindingsRegistry.registerCommandAndKeybindingRule({ id: 'openReferenceToSide',