feat: add visual highlighting for unused values in YAML files #37
Conversation
- Add unusedValuesDecorator utility to highlight unused values with wavy underlines - Listen for LSP diagnostics with DiagnosticTagUnnecessary from helm-ls - Support both light and dark themes with appropriate colors - Update decorations in real-time as users type in values.yaml files Integrates with helm-ls unused values linting feature for better UX.
WalkthroughThe changes integrate a new unused values decorator utility into the VSCode extension that highlights unused YAML values with theme-aware visual decorations. The decorator synchronizes decorations with diagnostics from helm-ls and keeps them up-to-date during editor and text changes. Changes
Sequence DiagramsequenceDiagram
participant VSCode as VSCode Extension
participant Decorator as Unused Values Decorator
participant Diagnostics as Diagnostics (helm-ls)
participant Theme as Theme Manager
participant Editor as Active Editor
VSCode->>Decorator: activate(context) on extension load
Decorator->>Theme: Initialize decoration type (theme-aware)
Decorator->>Diagnostics: Listen to diagnostic changes
Decorator->>Editor: Listen to editor switches & text edits
Note over Decorator: Setup complete
Diagnostics-->>Decorator: Emit diagnostics update
Decorator->>Editor: Filter helm-ls "Unnecessary" diagnostics
Decorator->>Editor: Update decorations (wavy underline + ruler)
Theme-->>Decorator: Emit theme change event
Decorator->>Decorator: Refresh decoration type
Decorator->>Editor: Reapply decorations to all open editors
Editor-->>Decorator: Text document edited or editor changed
Decorator->>Editor: Update decorations for active editor
VSCode->>Decorator: deactivate() on extension unload
Decorator->>Decorator: Dispose all decorations & event listeners
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/extension.ts(2 hunks)src/util/unusedValuesDecorator.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/util/unusedValuesDecorator.ts (1)
src/extension.ts (2)
activate(26-85)deactivate(87-94)
src/extension.ts (1)
src/util/unusedValuesDecorator.ts (1)
deactivate(90-94)
🔇 Additional comments (6)
src/extension.ts (1)
89-89: LGTM!The deactivation order is correct: cleaning up the decorator before stopping the language client ensures decorations are removed before the diagnostic source is terminated.
src/util/unusedValuesDecorator.ts (5)
5-17: LGTM!The theme-aware decoration options are appropriate for indicating unused values. The wavy underline pattern and overview ruler integration align well with VSCode conventions for "unnecessary" code indicators.
19-61: LGTM!The event listener setup comprehensively covers all scenarios where decorations need updating: diagnostic changes, text edits, editor switches, and theme changes. All listeners are properly registered to
context.subscriptionsfor automatic cleanup.
67-71: LGTM!The helper correctly iterates through all visible editors to refresh decorations, which is appropriate for theme change scenarios.
90-94: LGTM!The deactivation properly disposes of the decoration type, which automatically clears all decorations. Event listeners registered to
context.subscriptionsin the activate function will be automatically disposed by VSCode.
76-81: Based on the web search results about VS Code's DiagnosticTag.Unnecessary, I can now analyze the review comment's validity:Key Finding: DiagnosticTag.Unnecessary is purely a rendering hint for presentation purposes, not a semantic filter for identifying unused values. This means:
- The concern in the review is technically valid - Multiple sources could set the Unnecessary tag on their diagnostics
- The empty source filter is indeed risky - Any diagnostic source sending diagnostics with an empty source string + Unnecessary tag would be matched, not just helm-ls
- The protection is incomplete - DiagnosticTag.Unnecessary doesn't prevent other tools from sending similar diagnostic patterns
However, I cannot verify:
- Whether the
isValuesFile()helper function actually exists in the codebase- Whether this is a practical concern in the real world
- Whether the suggested fix is implementable
Given that the repository is inaccessible and the core concern about the empty source filter's inadequacy is technically sound based on VS Code API documentation, but the specific suggested fix cannot be validated:
Empty source filter accepts diagnostics from any tool with Unnecessary tag, not just helm-ls.
The temporary workaround accepting diagnostics with empty source (line 79) is broader than intended. DiagnosticTag.Unnecessary is only a rendering hint and doesn't filter by semantic meaning—any diagnostic source setting this tag with an empty source string would match, creating false positives beyond helm-ls unused values.
Verify that
isValuesFile()exists and is accessible in scope before applying the suggested constraint. If unavailable, consider alternative validation (e.g., checking diagnostic message pattern or adding a more specific source identifier).
| client.start(); | ||
|
|
||
| unusedValuesDecorator.activate(context); |
There was a problem hiding this comment.
Await client.start() before activating the decorator to prevent race conditions.
The decorator is activated immediately after client.start() without awaiting the Promise. Since the decorator relies on diagnostics from helm-ls, activating it before the language server is fully initialized could lead to missing or incomplete decorations.
Apply this diff to await the client initialization:
- client.start();
-
- unusedValuesDecorator.activate(context);
+ await client.start();
+ unusedValuesDecorator.activate(context);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| client.start(); | |
| unusedValuesDecorator.activate(context); | |
| await client.start(); | |
| unusedValuesDecorator.activate(context); |
🤖 Prompt for AI Agents
In src/extension.ts around lines 82 to 84, the code calls client.start() and
immediately activates unusedValuesDecorator which can race with language-server
initialization; change the call to await client.start() (ensuring the enclosing
function is async) so the Promise resolves before calling
unusedValuesDecorator.activate(context); optionally add a try/catch around await
client.start() to log or handle startup failures before activating the
decorator.
| function isValuesFile(fileName: string): boolean { | ||
| return fileName.includes('values.yaml') || fileName.includes('values.yml'); | ||
| } |
There was a problem hiding this comment.
Align the file pattern check with the document selector in extension.ts.
The isValuesFile check uses a simple substring match that is inconsistent with the document selector pattern **/values*.yaml defined in extension.ts (line 68). This could cause decorations to appear in files like "myvalues.yaml" that aren't monitored by the language server, or miss files with uppercase extensions.
Apply this diff to align with the document selector pattern:
function isValuesFile(fileName: string): boolean {
- return fileName.includes('values.yaml') || fileName.includes('values.yml');
+ const lowerFileName = fileName.toLowerCase();
+ return /values.*\.ya?ml$/.test(lowerFileName);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function isValuesFile(fileName: string): boolean { | |
| return fileName.includes('values.yaml') || fileName.includes('values.yml'); | |
| } | |
| function isValuesFile(fileName: string): boolean { | |
| const lowerFileName = fileName.toLowerCase(); | |
| return /values.*\.ya?ml$/.test(lowerFileName); | |
| } |
🤖 Prompt for AI Agents
In src/util/unusedValuesDecorator.ts around lines 63-65, the current
isValuesFile uses a simple substring check that doesn't match the document
selector **/values*.yaml and is case-sensitive; replace it so it normalizes to
the filename basename and matches the pattern used by extension.ts, e.g. test
the basename against a case-insensitive pattern for values*.ya?ml (match
/^values.*\.ya?ml$/i) or use the same glob/minimatch pattern, ensuring only
files whose basename begins with "values" and ends with .yml or .yaml
(case-insensitive) return true.
|
Hi and thanks for the suggestion. So I would rather not include a hard coded color scheme for this in the extension. |
|
Hi @qvalentin, I tested it with your PR, and it works: 
I'll close this PR. Thank you. |
2 participants
Add Visual Highlighting for Unused Values in YAML Files
This PR adds visual highlighting for unused values in Helm values files, providing immediate feedback to users about
which values are not referenced in their templates.
Features
• Visual Indicators: Unused values are highlighted with wavy underlines that adapt to light/dark themes
• Real-time Updates: Decorations update immediately as users type, not just when diagnostics change
• LSP Integration: Leverages helm-ls diagnostics with DiagnosticTagUnnecessary for accurate detection
• Hover Information: Shows helpful messages when hovering over unused values
• Theme Support: Uses appropriate colors for both light and dark VSCode themes
Implementation
• Added unusedValuesDecorator.ts utility in src/util/
• Integrated with existing LSP client in main extension
• Listens for both diagnostic changes and text document changes for responsive UX
• Filters diagnostics by source (helm-ls unused values) and diagnostic tags
Compatibility
Includes temporary compatibility with current helm-ls versions that send empty source strings. This will be cleaned
up once helm-ls commit 780fcd0 is merged.
Screenshots
Related Issues
Addresses the need for better visual feedback on unused Helm values, improving developer experience when working
with large values files.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.