add filterFontDecoration to decorationProvider#289146
Conversation
There was a problem hiding this comment.
Pull request overview
Adds support for filtering out font-affecting decorations when collecting decorations from DecorationProviders, addressing the reported issue.
Changes:
- Extends
DecorationProviderAPIs to acceptfilterFontDecorations(and updates some call sites). - Passes
filterFontDecorationsthroughTextModelwhen querying provider decorations in a range. - Updates the tokenization font decoration provider to skip font-affecting decorations when requested.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/vs/editor/common/model/tokens/tokenizationFontDecorationsProvider.ts | Adds filterFontDecorations support when returning tokenization font decorations in a range. |
| src/vs/editor/common/model/textModel.ts | For range-based decoration queries, forwards filterFontDecorations to decoration providers. |
| src/vs/editor/common/model/decorationProvider.ts | Extends the DecorationProvider interface to include filterFontDecorations (and onlyMinimapDecorations for getAllDecorations). |
| src/vs/editor/common/model/bracketPairsTextModelPart/colorizedBracketPairsDecorationProvider.ts | Updates provider method signatures to accept filterFontDecorations and forwards it through getAllDecorations. |
| if (!(affectsFont && filterFontDecorations)) { | ||
| const annotationStartPosition = this.textModel.getPositionAt(annotation.range.start); | ||
| const annotationEndPosition = this.textModel.getPositionAt(annotation.range.endExclusive); | ||
| const range = Range.fromPositions(annotationStartPosition, annotationEndPosition); | ||
| const anno = annotation.annotation; |
There was a problem hiding this comment.
getDecorationsInRange now filters via filterFontDecorations, but the implementation duplicates annotation.annotation (anno) and uses a double-negative condition (if (!(affectsFont && filterFontDecorations))), which makes the logic harder to follow and easy to regress. Consider computing anno once and rewriting the guard as if (!filterFontDecorations || !affectsFont) to avoid shadowing and improve readability.
| if (!(affectsFont && filterFontDecorations)) { | |
| const annotationStartPosition = this.textModel.getPositionAt(annotation.range.start); | |
| const annotationEndPosition = this.textModel.getPositionAt(annotation.range.endExclusive); | |
| const range = Range.fromPositions(annotationStartPosition, annotationEndPosition); | |
| const anno = annotation.annotation; | |
| if (!filterFontDecorations || !affectsFont) { | |
| const annotationStartPosition = this.textModel.getPositionAt(annotation.range.start); | |
| const annotationEndPosition = this.textModel.getPositionAt(annotation.range.endExclusive); | |
| const range = Range.fromPositions(annotationStartPosition, annotationEndPosition); |
| * @param filterOutValidation If set, it will ignore decorations specific to validation (i.e. warnings, errors). | ||
| */ | ||
| getAllDecorations(ownerId?: number, filterOutValidation?: boolean, onlyMinimapDecorations?: boolean): IModelDecoration[]; | ||
| getAllDecorations(ownerId?: number, filterOutValidation?: boolean, filterFontDecorations?: boolean, onlyMinimapDecorations?: boolean): IModelDecoration[]; |
There was a problem hiding this comment.
DecorationProvider.getAllDecorations now exposes filterFontDecorations (and onlyMinimapDecorations), but TextModel.getAllDecorations(...) currently does not forward filterFontDecorations to _decorationProvider.getAllDecorations / _fontTokenDecorationsProvider.getAllDecorations, and at least TokenizationFontDecorationProvider.getAllDecorations still ignores the new flag. This makes filterFontDecorations behavior inconsistent between “range” and “all” queries. Either (a) update all implementers + TextModel aggregation to accept/forward these new params, or (b) avoid adding the params to the interface if they aren’t intended to be supported.
| getAllDecorations(ownerId?: number, filterOutValidation?: boolean, filterFontDecorations?: boolean, onlyMinimapDecorations?: boolean): IModelDecoration[]; | |
| getAllDecorations(ownerId?: number, filterOutValidation?: boolean): IModelDecoration[]; |
fixes #289140