feat(config) - add option to remove "specific" aspect config #9488
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds functionality to allow removing "specific" aspect configurations by introducing a new special sign -- that can be used to ignore extensions from specific configuration sources (BitmapFile, ModelSpecific, ComponentJsonFile).
- Introduces a new
IGNORE_FROM_SPECIFIC_SPECIAL_SIGNconstant (--) alongside the existing remove sign (-) - Updates the aspects merger to filter out extensions marked with the ignore sign from specific configuration sources
- Adds type safety improvements for config object handling
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| scopes/workspace/workspace/aspects-merger.ts | Implements the core logic to filter ignored extensions from specific sources and adds type safety for config objects |
| components/legacy/extension-data/index.ts | Exports the new ignore constant |
| components/legacy/extension-data/extension-data.ts | Defines the new ignore special sign and related types/methods |
| components/legacy/extension-data/extension-data-list.ts | Adds filtering method for ignored extensions |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| ignoredFromSpecificsExtensionIds.push( | ||
| ...extensions.filter((extData) => extData.isIgnoredFromSpecific).map((extData) => extData.stringId) | ||
| ); | ||
| extsWithoutIgnored = extensions.filter((ext) => !ignoredFromSpecificsExtensionIds.includes(ext.stringId)); |
There was a problem hiding this comment.
Using includes() with an array for filtering can be inefficient for large datasets. Consider using a Set for ignoredFromSpecificsExtensionIds to improve lookup performance from O(n) to O(1).
| get config(): { [key: string]: any } | string { | ||
| if (this.rawConfig === REMOVE_EXTENSION_SPECIAL_SIGN) return {}; | ||
| return this.rawConfig; |
There was a problem hiding this comment.
The return type change from { [key: string]: any } to { [key: string]: any } | string is a breaking change that could affect consumers expecting only object types. Consider maintaining backward compatibility or documenting this breaking change.
| get config(): { [key: string]: any } | string { | |
| if (this.rawConfig === REMOVE_EXTENSION_SPECIAL_SIGN) return {}; | |
| return this.rawConfig; | |
| get config(): { [key: string]: any } { | |
| if ( | |
| this.rawConfig === REMOVE_EXTENSION_SPECIAL_SIGN || | |
| this.rawConfig === IGNORE_FROM_SPECIFIC_SPECIAL_SIGN | |
| ) { | |
| return {}; | |
| } | |
| return this.rawConfig as { [key: string]: any }; |
Proposed Changes