Run DebugConfigurationProviders for the result type of another DebugConfigurationProvider #143054
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…onfigurationProvider 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 microsoft#142135 I'm not sure where to add tests, so I would appreciate any pointers.
weinand
added
the
debug
|
I've added some tests for this code. @weinand anything else you'd like me to do for this PR? |
connor4312
reviewed
Dec 9, 2022
| @@ -119,22 +118,34 @@ export class ConfigurationManager implements IConfigurationManager { | |||
| } | |||
|
|
|||
| async resolveConfigurationByProviders(folderUri: uri | undefined, type: string | undefined, config: IConfig, token: CancellationToken): Promise<IConfig | null | undefined> { | |||
| // 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); | |||
There was a problem hiding this comment.
This seems unnecessary since the first resolveDebugConfigurationForType will activate the type
There was a problem hiding this comment.
Is this just so you do the filter once below instead of inside resolveDebugConfigurationForType? I would probably keep it as it was before. Most debug calls won't hit this more than once, and even for the ones that do an await costs much more than filtering the relatively small list of configProvider
|
cc @roblourens. This looks generally okay to me. |
roblourens
approved these changes
Dec 31, 2022
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This enables chaining
DebugConfigurationProviders with two or moretypechanges (onetypechange is already supported) and callingDebugConfigurationProvider'sresolveConfigurationsByProviderscallback when providers are chained.This can be tested manually be defining two different configuration providers and having one return a configuration with the 'type' of the next one and ensuring that the second callback is called.
This PR fixes #142135
Fixes #110889