From a0965b832fb9fa979ce694e3358ae80d9303d84e Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Mon, 14 Feb 2022 11:22:09 -0800 Subject: [PATCH 1/5] Run DebugConfigurationProviders for the result type of another DebugConfigurationProvider This enables chaining `DebugConfigurationProvider`s with two or more `type` changes (one `type` change is already supported) and calling `DebugConfigurationProvider`'s `resolveConfigurationsByProviders` callback when providers are chained. Fixes #142135 I'm not sure where to add tests, so I would appreciate any pointers. --- .../browser/debugConfigurationManager.ts | 38 ++++++++++++------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/src/vs/workbench/contrib/debug/browser/debugConfigurationManager.ts b/src/vs/workbench/contrib/debug/browser/debugConfigurationManager.ts index e3eaf0b143802..278c2e0fae9ce 100644 --- a/src/vs/workbench/contrib/debug/browser/debugConfigurationManager.ts +++ b/src/vs/workbench/contrib/debug/browser/debugConfigurationManager.ts @@ -119,22 +119,34 @@ export class ConfigurationManager implements IConfigurationManager { } async resolveConfigurationByProviders(folderUri: uri | undefined, type: string | undefined, config: IConfig, token: CancellationToken): Promise { + // activate debuggers early for the provided type in case any '*' typed debug configuration providers are activated + // based on the provided type. await this.activateDebuggers('onDebugResolve', type); - // pipe the config through the promises sequentially. Append at the end the '*' types - const providers = this.configProviders.filter(p => p.type === type && p.resolveDebugConfiguration) - .concat(this.configProviders.filter(p => p.type === '*' && p.resolveDebugConfiguration)); + const anyTypeProviders = this.configProviders.filter(p => p.type === '*' && p.resolveDebugConfiguration); + + let resolveDebugConfigurationForType = async (type: string | undefined, config: IConfig | null | undefined) => { + await this.activateDebuggers('onDebugResolve', type); + // pipe the config through the promises sequentially. Append at the end the '*' types + const providers = this.configProviders.filter(p => p.type === type && p.resolveDebugConfiguration) + .concat(anyTypeProviders); + + let result: IConfig | null | undefined = config; + await sequence(providers.map(provider => async () => { + // If any provider returned undefined or null make sure to respect that and do not pass the result to more resolver + if (result) { + result = await provider.resolveDebugConfiguration!(folderUri, result, token); + } + })); + return result; + }; - let result: IConfig | null | undefined = config; - await sequence(providers.map(provider => async () => { - // If any provider returned undefined or null make sure to respect that and do not pass the result to more resolver - if (result) { - result = await provider.resolveDebugConfiguration!(folderUri, result, token); - } - })); + let result = await resolveDebugConfigurationForType(type, config); + let seenTypes = new Set().add(type); + + while (result && !seenTypes.has(result.type)) { + seenTypes.add(result.type); - // The resolver can change the type, ensure activation happens, #135090 - if (result?.type && result.type !== config.type) { - await this.activateDebuggers('onDebugResolve', result.type); + result = await resolveDebugConfigurationForType(result.type, result); } return result; From 7dfc87a897b5d4bb8169393b01003048ffd23299 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Fri, 29 Jul 2022 15:41:01 -0700 Subject: [PATCH 2/5] Fix build break --- .../contrib/debug/browser/debugConfigurationManager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/debug/browser/debugConfigurationManager.ts b/src/vs/workbench/contrib/debug/browser/debugConfigurationManager.ts index 0b4e4869b4723..3ba19b01323d4 100644 --- a/src/vs/workbench/contrib/debug/browser/debugConfigurationManager.ts +++ b/src/vs/workbench/contrib/debug/browser/debugConfigurationManager.ts @@ -124,7 +124,7 @@ export class ConfigurationManager implements IConfigurationManager { await this.adapterManager.activateDebuggers('onDebugResolve', type); const anyTypeProviders = this.configProviders.filter(p => p.type === '*' && p.resolveDebugConfiguration); - let resolveDebugConfigurationForType = async (type: string | undefined, config: IConfig | null | undefined) => { + const resolveDebugConfigurationForType = async (type: string | undefined, config: IConfig | null | undefined) => { await this.adapterManager.activateDebuggers('onDebugResolve', type); // pipe the config through the promises sequentially. Append at the end the '*' types const providers = this.configProviders.filter(p => p.type === type && p.resolveDebugConfiguration) From 4135166f897db418635337a2e6c602c2ee6e2e8c Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Fri, 29 Jul 2022 16:40:07 -0700 Subject: [PATCH 3/5] Try adding some basic tests for the debug.ConfigurationManager --- .../browser/debugConfigurationManager.ts | 13 +-- .../workbench/contrib/debug/common/debug.ts | 5 + .../browser/debugConfigurationManager.test.ts | 107 ++++++++++++++++++ 3 files changed, 118 insertions(+), 7 deletions(-) create mode 100644 src/vs/workbench/contrib/debug/test/browser/debugConfigurationManager.test.ts diff --git a/src/vs/workbench/contrib/debug/browser/debugConfigurationManager.ts b/src/vs/workbench/contrib/debug/browser/debugConfigurationManager.ts index 3ba19b01323d4..0cc8ef90f2c26 100644 --- a/src/vs/workbench/contrib/debug/browser/debugConfigurationManager.ts +++ b/src/vs/workbench/contrib/debug/browser/debugConfigurationManager.ts @@ -18,7 +18,7 @@ import { IConfigurationService, ConfigurationTarget } from 'vs/platform/configur import { IFileService } from 'vs/platform/files/common/files'; import { IWorkspaceContextService, IWorkspaceFolder, WorkbenchState, IWorkspaceFoldersChangeEvent } from 'vs/platform/workspace/common/workspace'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; -import { IDebugConfigurationProvider, ICompound, IConfig, IGlobalConfig, IConfigurationManager, ILaunch, CONTEXT_DEBUG_CONFIGURATION_TYPE, IConfigPresentation, DebugConfigurationProviderTriggerKind } from 'vs/workbench/contrib/debug/common/debug'; +import { IDebugConfigurationProvider, ICompound, IConfig, IGlobalConfig, IConfigurationManager, ILaunch, CONTEXT_DEBUG_CONFIGURATION_TYPE, IConfigPresentation, DebugConfigurationProviderTriggerKind, IAdapterManager } from 'vs/workbench/contrib/debug/common/debug'; import { IEditorService, ACTIVE_GROUP } from 'vs/workbench/services/editor/common/editorService'; import { launchSchemaId } from 'vs/workbench/services/configuration/common/configuration'; import { IPreferencesService } from 'vs/workbench/services/preferences/common/preferences'; @@ -35,7 +35,6 @@ import { IHistoryService } from 'vs/workbench/services/history/common/history'; import { flatten, distinct } from 'vs/base/common/arrays'; import { getVisibleAndSorted } from 'vs/workbench/contrib/debug/common/debugUtils'; import { IUriIdentityService } from 'vs/platform/uriIdentity/common/uriIdentity'; -import { AdapterManager } from 'vs/workbench/contrib/debug/browser/debugAdapterManager'; import { debugConfigure } from 'vs/workbench/contrib/debug/browser/debugIcons'; import { ThemeIcon } from 'vs/platform/theme/common/themeService'; @@ -62,7 +61,7 @@ export class ConfigurationManager implements IConfigurationManager { private debugConfigurationTypeContext: IContextKey; constructor( - private readonly adapterManager: AdapterManager, + private readonly adapterManager: IAdapterManager, @IWorkspaceContextService private readonly contextService: IWorkspaceContextService, @IConfigurationService private readonly configurationService: IConfigurationService, @IQuickInputService private readonly quickInputService: IQuickInputService, @@ -481,7 +480,7 @@ abstract class AbstractLaunch { constructor( protected configurationManager: ConfigurationManager, - private readonly adapterManager: AdapterManager + private readonly adapterManager: IAdapterManager ) { } getCompound(name: string): ICompound | undefined { @@ -554,7 +553,7 @@ class Launch extends AbstractLaunch implements ILaunch { constructor( configurationManager: ConfigurationManager, - adapterManager: AdapterManager, + adapterManager: IAdapterManager, public workspace: IWorkspaceFolder, @IFileService private readonly fileService: IFileService, @ITextFileService private readonly textFileService: ITextFileService, @@ -637,7 +636,7 @@ class Launch extends AbstractLaunch implements ILaunch { class WorkspaceLaunch extends AbstractLaunch implements ILaunch { constructor( configurationManager: ConfigurationManager, - adapterManager: AdapterManager, + adapterManager: IAdapterManager, @IEditorService private readonly editorService: IEditorService, @IConfigurationService private readonly configurationService: IConfigurationService, @IWorkspaceContextService private readonly contextService: IWorkspaceContextService @@ -689,7 +688,7 @@ class UserLaunch extends AbstractLaunch implements ILaunch { constructor( configurationManager: ConfigurationManager, - adapterManager: AdapterManager, + adapterManager: IAdapterManager, @IConfigurationService private readonly configurationService: IConfigurationService, @IPreferencesService private readonly preferencesService: IPreferencesService ) { diff --git a/src/vs/workbench/contrib/debug/common/debug.ts b/src/vs/workbench/contrib/debug/common/debug.ts index ca1c5e318d289..31a9c6f6de388 100644 --- a/src/vs/workbench/contrib/debug/common/debug.ts +++ b/src/vs/workbench/contrib/debug/common/debug.ts @@ -157,6 +157,7 @@ export interface IDebugger { createDebugAdapter(session: IDebugSession): Promise; runInTerminal(args: DebugProtocol.RunInTerminalRequestArguments, sessionId: string): Promise; getCustomTelemetryEndpoint(): ITelemetryEndpoint | undefined; + getInitialConfigurationContent(initialConfigs?: IConfig[]): Promise; } export interface IDebuggerMetadata { @@ -882,6 +883,10 @@ export interface IAdapterManager { substituteVariables(debugType: string, folder: IWorkspaceFolder | undefined, config: IConfig): Promise; runInTerminal(debugType: string, args: DebugProtocol.RunInTerminalRequestArguments, sessionId: string): Promise; + getEnabledDebugger(type: string): (IDebugger & IDebuggerMetadata) | undefined; + guessDebugger(gettingConfigurations: boolean): Promise<(IDebugger & IDebuggerMetadata) | undefined>; + + get onDidDebuggersExtPointRead(): Event; } export interface ILaunch { diff --git a/src/vs/workbench/contrib/debug/test/browser/debugConfigurationManager.test.ts b/src/vs/workbench/contrib/debug/test/browser/debugConfigurationManager.test.ts new file mode 100644 index 0000000000000..950b044b58bc5 --- /dev/null +++ b/src/vs/workbench/contrib/debug/test/browser/debugConfigurationManager.test.ts @@ -0,0 +1,107 @@ +/*--------------------------------------------------------------------------------------------- + * 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 { CancellationToken } from 'vs/base/common/cancellation'; +import { DisposableStore } from 'vs/base/common/lifecycle'; +import { TestConfigurationService } from 'vs/platform/configuration/test/common/testConfigurationService'; +import { ContextKeyService } from 'vs/platform/contextkey/browser/contextKeyService'; +import { FileService } from 'vs/platform/files/common/fileService'; +import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock'; +import { NullLogService } from 'vs/platform/log/common/log'; +import { UriIdentityService } from 'vs/platform/uriIdentity/common/uriIdentityService'; +import { ConfigurationManager } from 'vs/workbench/contrib/debug/browser/debugConfigurationManager'; +import { DebugConfigurationProviderTriggerKind, IAdapterManager, IConfig, IDebugAdapterExecutable, IDebugSession } from 'vs/workbench/contrib/debug/common/debug'; +import { TestHistoryService, TestQuickInputService } from 'vs/workbench/test/browser/workbenchTestServices'; +import { TestContextService, TestExtensionService, TestStorageService } from 'vs/workbench/test/common/workbenchTestServices'; + +suite('debugConfigurationManager', () => { + const configurationProviderType = 'custom-type'; + let _debugConfigurationManager: ConfigurationManager; + const disposables = new DisposableStore(); + + const adapterManager = { + getDebugAdapterDescriptor(session: IDebugSession, config: IConfig): Promise { + return Promise.resolve(undefined); + } + }; + + const configurationService = new TestConfigurationService(); + setup(() => { + const fileService = disposables.add(new FileService(new NullLogService())); + _debugConfigurationManager = new ConfigurationManager( + adapterManager, + new TestContextService(), + configurationService, + new TestQuickInputService(), + new TestInstantiationService(), + new TestStorageService(), + new TestExtensionService(), + new TestHistoryService(), + new UriIdentityService(fileService), + new ContextKeyService(configurationService)); + }); + + test('resolves configuration based on type', async () => { + disposables.add(_debugConfigurationManager.registerDebugConfigurationProvider({ + type: configurationProviderType, + resolveDebugConfiguration: (folderUri, config, token) => { + assert.strictEqual(config.type, configurationProviderType); + return Promise.resolve({ + ...config, + configurationResolved: true + }); + }, + triggerKind: DebugConfigurationProviderTriggerKind.Initial + })); + + const initialConfig: IConfig = { + type: configurationProviderType, + request: 'launch', + name: 'configName', + }; + + const resultConfig = await _debugConfigurationManager.resolveConfigurationByProviders(undefined, configurationProviderType, initialConfig, CancellationToken.None); + assert.strictEqual((resultConfig as any).configurationResolved, true, 'Configuration should be updated by test provider'); + }); + + test('resolves configuration from second provider if type changes', async () => { + const secondProviderType = 'second-provider'; + disposables.add(_debugConfigurationManager.registerDebugConfigurationProvider({ + type: configurationProviderType, + resolveDebugConfiguration: (folderUri, config, token) => { + assert.strictEqual(config.type, configurationProviderType); + return Promise.resolve({ + ...config, + type: secondProviderType + }); + }, + triggerKind: DebugConfigurationProviderTriggerKind.Initial + })); + disposables.add(_debugConfigurationManager.registerDebugConfigurationProvider({ + type: secondProviderType, + resolveDebugConfiguration: (folderUri, config, token) => { + assert.strictEqual(config.type, secondProviderType); + return Promise.resolve({ + ...config, + configurationResolved: true + }); + }, + triggerKind: DebugConfigurationProviderTriggerKind.Initial + })); + + const initialConfig: IConfig = { + type: configurationProviderType, + request: 'launch', + name: 'configName', + }; + + const resultConfig = await _debugConfigurationManager.resolveConfigurationByProviders(undefined, configurationProviderType, initialConfig, CancellationToken.None); + assert.strictEqual(resultConfig!.type, secondProviderType); + assert.strictEqual((resultConfig as any).configurationResolved, true, 'Configuration should be updated by test provider'); + }); + + teardown(() => disposables.clear()); +}); From 594ae6e009e8ad6bf73d4bc41c0685558a00d332 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Mon, 1 Aug 2022 10:04:03 -0700 Subject: [PATCH 4/5] Update services surface areas to make sure test passes. --- .../browser/debugConfigurationManager.test.ts | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/debug/test/browser/debugConfigurationManager.test.ts b/src/vs/workbench/contrib/debug/test/browser/debugConfigurationManager.test.ts index 950b044b58bc5..d9978194ea848 100644 --- a/src/vs/workbench/contrib/debug/test/browser/debugConfigurationManager.test.ts +++ b/src/vs/workbench/contrib/debug/test/browser/debugConfigurationManager.test.ts @@ -6,6 +6,7 @@ import * as assert from 'assert'; import { CancellationToken } from 'vs/base/common/cancellation'; import { DisposableStore } from 'vs/base/common/lifecycle'; +import { Event } from 'vs/base/common/event'; import { TestConfigurationService } from 'vs/platform/configuration/test/common/testConfigurationService'; import { ContextKeyService } from 'vs/platform/contextkey/browser/contextKeyService'; import { FileService } from 'vs/platform/files/common/fileService'; @@ -16,6 +17,9 @@ import { ConfigurationManager } from 'vs/workbench/contrib/debug/browser/debugCo import { DebugConfigurationProviderTriggerKind, IAdapterManager, IConfig, IDebugAdapterExecutable, IDebugSession } from 'vs/workbench/contrib/debug/common/debug'; import { TestHistoryService, TestQuickInputService } from 'vs/workbench/test/browser/workbenchTestServices'; import { TestContextService, TestExtensionService, TestStorageService } from 'vs/workbench/test/common/workbenchTestServices'; +import { ServiceCollection } from 'vs/platform/instantiation/common/serviceCollection'; +import { IPreferencesService } from 'vs/workbench/services/preferences/common/preferences'; +import { URI } from 'vs/base/common/uri'; suite('debugConfigurationManager', () => { const configurationProviderType = 'custom-type'; @@ -25,9 +29,21 @@ suite('debugConfigurationManager', () => { const adapterManager = { getDebugAdapterDescriptor(session: IDebugSession, config: IConfig): Promise { return Promise.resolve(undefined); + }, + + activateDebuggers(activationEvent: string, debugType?: string): Promise { + return Promise.resolve(); + }, + + get onDidDebuggersExtPointRead(): Event { + return Event.None; } }; + const preferencesService = { + userSettingsResource: URI.file('/tmp/settings.json') + }; + const configurationService = new TestConfigurationService(); setup(() => { const fileService = disposables.add(new FileService(new NullLogService())); @@ -36,7 +52,7 @@ suite('debugConfigurationManager', () => { new TestContextService(), configurationService, new TestQuickInputService(), - new TestInstantiationService(), + new TestInstantiationService(new ServiceCollection([IPreferencesService, preferencesService])), new TestStorageService(), new TestExtensionService(), new TestHistoryService(), From 1bcbb1142a77349a844231a1be6320b4ed219cab Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Thu, 5 Jan 2023 15:17:22 -0800 Subject: [PATCH 5/5] address pr comments --- .../browser/debugConfigurationManager.ts | 37 +++++++------------ 1 file changed, 14 insertions(+), 23 deletions(-) diff --git a/src/vs/workbench/contrib/debug/browser/debugConfigurationManager.ts b/src/vs/workbench/contrib/debug/browser/debugConfigurationManager.ts index a572abd27a989..8e59a8a9bf6a3 100644 --- a/src/vs/workbench/contrib/debug/browser/debugConfigurationManager.ts +++ b/src/vs/workbench/contrib/debug/browser/debugConfigurationManager.ts @@ -119,34 +119,25 @@ export class ConfigurationManager implements IConfigurationManager { } async resolveConfigurationByProviders(folderUri: uri | undefined, type: string | undefined, config: IConfig, token: CancellationToken): Promise { - // activate debuggers early for the provided type in case any '*' typed debug configuration providers are activated - // based on the provided type. - await this.adapterManager.activateDebuggers('onDebugResolve', type); - const anyTypeProviders = this.configProviders.filter(p => p.type === '*' && p.resolveDebugConfiguration); - const resolveDebugConfigurationForType = async (type: string | undefined, config: IConfig | null | undefined) => { - await this.adapterManager.activateDebuggers('onDebugResolve', type); - // pipe the config through the promises sequentially. Append at the end the '*' types - const providers = this.configProviders.filter(p => p.type === type && p.resolveDebugConfiguration) - .concat(anyTypeProviders); - - let result: IConfig | null | undefined = config; - await sequence(providers.map(provider => async () => { - // If any provider returned undefined or null make sure to respect that and do not pass the result to more resolver - if (result) { - result = await provider.resolveDebugConfiguration!(folderUri, result, token); - } - })); - return result; - }; + if (type !== '*') { + await this.adapterManager.activateDebuggers('onDebugResolve', type); + } - let result = await resolveDebugConfigurationForType(type, config); - const seenTypes = new Set().add(type); + for (const p of this.configProviders) { + if (p.type === type && p.resolveDebugConfiguration && config) { + config = await p.resolveDebugConfiguration(folderUri, config, token); + } + } - while (result && !seenTypes.has(result.type)) { - seenTypes.add(result.type); + return config; + }; + let result: IConfig | null | undefined = config; + for (let seen = new Set(); result && !seen.has(result.type);) { + seen.add(result?.type); result = await resolveDebugConfigurationForType(result.type, result); + result = await resolveDebugConfigurationForType('*', result); } return result;