-
Notifications
You must be signed in to change notification settings - Fork 36k
extension unification ignore settings validation #274376
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a mechanism to bypass validation for certain configuration settings during extension unification. It creates an allowlist of settings (stored in IGNORE_VALIDATION_SETTINGS) that can skip validation checks that would normally reject unknown or duplicate settings.
- Adds
IGNORE_VALIDATION_SETTINGSset to bypass validation for specific settings from product configuration - Updates configuration editing and extension point validation to check this allowlist before rejecting settings
- Temporarily allows settings defined in
product.defaultChatAgent.completionsEnablementSettingto bypass validation
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/vs/workbench/services/configuration/common/configuration.ts | Defines the IGNORE_VALIDATION_SETTINGS set populated from product configuration, with a temporary comment indicating removal after extension unification |
| src/vs/workbench/services/configuration/common/configurationEditing.ts | Adds validation bypass check for settings in the ignore list when validating configuration keys |
| src/vs/workbench/api/common/configurationExtensionPoint.ts | Skips validation warnings and duplicate property checks for settings in the ignore list during extension point registration |
src/vs/workbench/services/configuration/common/configuration.ts
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <[email protected]>
|
PR is not ready, but open to different ideas if someone has a better approach |
|
I considered a different approach which would allow two extensions to contribute the same configuration in a proper manner, however, that adds a ton of complexity and adds risks of causing a regression. As this would only lead to a small improvement I prefer to keep this less nice but simpler approach |
| * - the operation is to delete the key | ||
| */ | ||
| if (!operation.workspaceStandAloneConfigurationKey) { | ||
| if (!operation.workspaceStandAloneConfigurationKey && !IGNORE_VALIDATION_SETTINGS.has(operation.key)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this one in #274797
This is the only way I could come up with which would support both extensions contributing the same configuration and being able to write to them when either one or both are enabled.
https://github.com/microsoft/vscode-internalbacklog/issues/6073