feat(tree-view): show LS HTML tree view in Eclipse plugin [IDE-1837]#387
Open
bastiandoetsch wants to merge 30 commits into
Open
feat(tree-view): show LS HTML tree view in Eclipse plugin [IDE-1837]#387bastiandoetsch wants to merge 30 commits into
bastiandoetsch wants to merge 30 commits into
Conversation
…owser [IDE-1837] - Add TreeViewParams DTO mirroring SummaryPanelParams - Add SNYK_TREE_VIEW constant to LsConstants - Add @JsonNotification handler in SnykExtendedLanguageClient - Create TreeViewBrowserHandler (reuses ExecuteCommandBridge) - Add USE_HTML_TREE_VIEW preference gate (default false) - Host treeBrowser in SnykToolView.createPartControl behind preference - Implement updateTreeViewHtml delegating to treeBrowserHandler on SWT thread - Add TDD tests (T-S-001, T-I-001) verifying DTO and notification dispatch Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…[IDE-1837] - Guard snykTreeView() against null params/html at the LSP boundary - Skip dispatch when USE_HTML_TREE_VIEW preference is disabled (avoids invisible browser renders in the default-off configuration) - Add isDisposed() guard in TreeViewBrowserHandler.setBrowserText() - Remove leaking OSGi IPreferenceChangeListener (no UI toggle exists yet; preference is read once at createPartControl time) - Remove unused InstanceScope and Activator imports from SnykToolView - Remove totalIssues dead field from TreeViewParams (no caller) - Guard treeBrowserHandler null in updateTreeViewHtml - Expand tests: null params, null html, preference-disabled, and preference-enabled dispatch all verified (5 tests, 0 failures) Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
- Add EclipseThemeCssProvider: maps SWT colors to --vscode-* CSS vars - TreeViewBrowserHandler.setBrowserText injects theme style block - Tests: EclipseThemeCssProviderTest (toHex, buildStyleBlock), TreeViewBrowserHandlerTest (theme injection, null no-op) Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…-1837]
- Use pt unit for font-size (SWT FontData.getHeight() returns points, not px)
- Quote multi-word font family names to produce valid CSS font-family values
- Use rgba() for widget-shadow and list-hoverBackground to preserve
transparency intent (consistent with the LS template fallback values)
- Refactor doubled getSystemFont().getFontData()[0] calls into resolveFontData()
- Use replaceFirst("(?i)</head>") instead of replace() to avoid injecting
style block at multiple </head> occurrences
- Add toRgba, quoteFontFamily, font-size-unit, and rgba-fallback tests
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
- Remove toRgba regex that could corrupt alpha values >= 0.10/0.20; %.2f already produces correct output (0.12, 0.15) without post-processing - Remove Javadoc from ISnykToolView.updateTreeViewHtml (interface has no Javadoc on other abstract methods — consistent with project convention) - Add missing newline at EOF in LsConstants.java - Delete redundant TreeViewBridgeTest (tested ExecuteCommandBridge.buildClientScript which is already covered by ExecuteCommandBridgeTest; added no tree-view coverage) Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Contributor
Author
|
/describe |
Contributor
Author
|
/review |
|
PR Description updated to latest commit (04c8394) |
This comment has been minimized.
This comment has been minimized.
…-1837] - Add TreeViewBrowserHandler.selectNode(issueId): injects window.__selectTreeNode__ into the HTML tree browser - Wire selectNode into SnykToolView: called on IssueTreeNode selection and from selectTreeNode(Issue, String) programmatic navigation - Buffer pending HTML in SnykToolView when treeBrowserHandler not yet initialized; drain buffer in createPartControl after handler init - Tests: selectNode null-safety (null browser, null/empty id)
… [IDE-1837] String.format with %.2f uses system locale by default; on non-US locales (e.g. Germany) this produces a comma as decimal separator, generating invalid CSS rgba() values. Fix by passing Locale.ROOT explicitly.
- Declare treeBrowserHandler volatile to make safe-publication explicit for background-thread reads in updateTreeViewHtml and selectTreeNode - Remove double selectNode call: selectTreeNode(Issue, String) no longer calls treeBrowserHandler.selectNode directly; the selectionChanged listener is the sole coupling point from JFace selection to HTML tree - Fix pendingHtml drain: call setBrowserText directly in createPartControl (already on UI thread) instead of via asyncExec to avoid stale-overwrite when a concurrent updateTreeViewHtml arrives before the drain executes - Extract escapeJsSingleQuotedString: extends escaping to \n, \r, U+2028, U+2029 which break single-quoted JS string literals - Remove selectTreeNode(String) from ISnykToolView interface (no external callers; removed dead implementation from SnykToolView) - Add 4 tests for escapeJsSingleQuotedString covering all escape cases Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…837] - Replace volatile String pendingHtml with AtomicReference<String> to eliminate NullAssignment PMD violation (priority 3) in createPartControl; AtomicReference.getAndSet(null) is both PMD-clean and provides stronger atomic semantics than volatile + manual null assignment - Sync settings-fallback.html from snyk-ls main (sha512 drift caused static-resource-checks CI failure; unrelated to IDE-1837 changes) Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Replace catch (Exception e) in resolveFontData with direct call; the null/disposed guards above make the broad catch unnecessary. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…ion [IDE-1837]
Add EclipseThemeCssProvider.buildVariableMap() returning a full map of all
--vscode-* variables to their Eclipse SWT colour values. Rewrite
TreeViewBrowserHandler.injectThemeCss() to do a single-pass regex
replacement of var(--vscode-xxx, fallback) expressions with resolved values,
and to strip the ${ideStyle}/${ideScript} placeholders instead of injecting
a :root block. buildStyleBlock() is retained for backward compatibility and
now delegates to buildVariableMap().
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…[IDE-1837] Remove the DIAGNOSTIC_SOURCE_SNYK_CODE-only guard from SnykShowFixUriDetails.isValid() so that OSS, IaC, and Secrets products are all accepted when the action is showInDetailPanel. Add a guard in SnykExtendedLanguageClient.showDocument() to return ShowDocumentResult(false) for any snyk:// URI with an unrecognised action instead of delegating to super.showDocument() which crashes with "No file system defined for scheme: snyk". Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Contributor
Author
|
/describe |
Contributor
Author
|
/review |
|
PR Description updated to latest commit (ec8d7f1) |
This comment has been minimized.
This comment has been minimized.
…[IDE-1837] - Increase testSnykTreeViewDispatchesHtmlToToolView timeout 5s→15s to accommodate macOS CI thread scheduling latency (test was passing at 5.155s, just over the 5000ms window) - Fix showDocument() to return CompletableFuture.completedFuture(false) instead of null when URISyntaxException occurs or issue is not in cache; lsp4j calls .thenAccept() on the returned future which NPEs on null Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
When toolView is already set, call updateTreeViewHtml synchronously instead of via CompletableFuture.runAsync — openToolView() is a no-op in that path anyway. The async path is only needed when the view must be opened first. This removes the test's dependency on thread-pool scheduling latency (was causing 15s+ delays on Ubuntu/macOS CI runners). Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
- Normalize LS product codenames (oss/code/iac) to Eclipse cache keys (Snyk Open Source/Code/IaC) before cache lookup; the HTML tree sends short codenames via data-product attribute, but the cache is keyed on display names - Font size: use px unit (matching IntelliJ) instead of pt; SWT FontData.getHeight() returns points but CSS px is the correct unit for screen rendering at the browser's reference pixel - Font family: add system-ui, -apple-system, sans-serif fallback chain after the system font name, matching IntelliJ's ThemeBasedStylingGenerator Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…tion [IDE-1837] Replace custom --vscode-only regex with IntelliJ's ThemeBasedStylingGenerator CSS_PATTERN that handles both var(--xxx) usages and --xxx: declarations in a single pass. Declaration replacement bakes concrete values directly into the CSS property declarations, eliminating all --vscode-* variable references from the rendered HTML. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…837] - Font family/size now read from JFaceResources.DEFAULT_FONT (same theme source as BaseHtmlProvider) instead of SWT system font or hardcoded values; font changes with the Eclipse theme - Navigation: convert issue.filterableIssueType() via SCAN_PARAMS_TO_DISPLAYED before calling selectTreeNode; ContentRootNode.getProductNode() expects DISPLAYED_* format, not scan-params format, causing IllegalArgumentException Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
| if (data != null && data.length > 0) { | ||
| return quoteFontFamily(data[0].getName()) + ", system-ui, -apple-system, sans-serif"; | ||
| } | ||
| } catch (Exception e) { |
| if (data != null && data.length > 0) { | ||
| return data[0].getHeight() + "px"; | ||
| } | ||
| } catch (Exception e) { |
|
PR Description updated to latest commit (b62427c) |
This comment has been minimized.
This comment has been minimized.
Contributor
Author
|
/describe |
|
PR Description updated to latest commit (b62427c) |
Contributor
Author
|
/describe |
|
PR Description updated to latest commit (b62427c) |
Contributor
Author
|
/describe |
|
PR Description updated to latest commit (b62427c) |
Contributor
Author
|
/describe |
|
PR Description updated to latest commit (b62427c) |
Documents the $/snyk.treeView rendering flow, JS bridge contract (window.__ideExecuteCommand__ and window.__selectTreeNode__), CSS theme substitution via BaseHtmlProvider, pending-HTML buffer pattern, selection sync, and how to enable the feature.
Contributor
Author
|
/describe |
1 similar comment
Contributor
Author
|
/describe |
|
PR Description updated to latest commit (42302ce) |
1 similar comment
|
PR Description updated to latest commit (42302ce) |
This comment has been minimized.
This comment has been minimized.
Contributor
Author
|
/describe |
|
PR Description updated to latest commit (42302ce) |
Contributor
Author
|
/describe |
|
PR Description updated to latest commit (42302ce) |
- Fix literal U+2028/U+2029 bytes in TreeViewBrowserHandler.escapeJsSingleQuotedString with Java Unicode escapes ( / ) to prevent JS parse errors on strings containing Unicode line/paragraph separators - Add Locale.ROOT to BaseHtmlProvider.getRelativeFontSize String.format call so the decimal separator is always a dot, not a comma on German/French locales - Add null guard (if issue == null return) at top of SnykToolView.selectTreeNode to prevent NullPointerException from either branch - Add selectTreeNode guard in SnykToolView to delegate to treeBrowserHandler when USE_HTML_TREE_VIEW is enabled, avoiding null-deref on productNode - Add clarifying comments: pendingHtml last-write-wins semantics, SashForm weight-0 behavior with setVisible(false) as non-interactive guard - Add tests: locale-safe font-size in BaseHtmlProviderTest, U+2028/U+2029 escaping in TreeViewBrowserHandlerTest (using Java Unicode escapes in test inputs to avoid the same encoding-corruption risk)
Contributor
Author
|
/describe |
|
PR Description updated to latest commit (42302ce) |
Contributor
Author
|
/describe |
This comment has been minimized.
This comment has been minimized.
|
PR Description updated to latest commit (f66b9e4) |
…sk [IDE-1837] Replace weight=0 (undocumented behaviour per SWT Javadoc) with weight=3. setWeights() counts all children regardless of visibility, so all 3 weights must always be positive integers. layout() skips invisible children, so setVisible(false) alone collapses the inactive pane without needing weight=0. Addresses PR review: weight=0 could cause IllegalArgumentException or rendering glitches on strict GTK/Cocoa implementations.
Contributor
Author
|
/describe |
|
PR Description updated to latest commit (9ac53da) |
PR Reviewer Guide 🔍
|
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
User description
Description
Implements the Eclipse client side of the language server HTML tree view feature (IDE-1837). The language server already emits
$/snyk.treeViewnotifications containing a fully-rendered HTML tree; this PR wires that into an embedded SWTBrowserin the Snyk tool view.PR stack:
What this PR includes (Steps 2.1 + 2.2 + 2.3):
LSP wire-up (Step 2.1):
LsConstants.SNYK_TREE_VIEW— LSP notification method constantTreeViewParamsDTO — deserializes{ treeViewHtml }from LS@JsonNotificationhandler inSnykExtendedLanguageClient— guarded against null params, null HTML, and disabled preferenceTreeViewBrowserHandler— SWT Browser wrapper; installsExecuteCommandBridge(reused from settings page) so allsnyk.*commands from tree.js reach the LS viaworkspace/executeCommandPreferences.USE_HTML_TREE_VIEW— feature flag, default OFFTheme CSS (Step 2.2):
EclipseThemeCssProvider— maps 15 Eclipse SWT system colors to--vscode-*CSS variables; usesrgba()for transparency-sensitive properties, properly quoted font families,ptunits for font size,Locale.ROOTfor decimal formattingSelection sync + edge cases (Step 2.3):
TreeViewBrowserHandler.selectNode(issueId)— injectswindow.__selectTreeNode__into the HTML tree browser; full JS string escaping including\n,\r, U+2028, U+2029SnykToolViewwired to callselectNodewhen anIssueTreeNodeis selected in the legacy treependingHtmlbuffer (AtomicReference<String>) — stores HTML iftreeBrowserHandlernot yet initialized; drains increatePartControltreeBrowserHandlerdeclaredvolatilefor safe publication to background threadsChecklist
Screenshots / GIFs
UI is gated behind
USE_HTML_TREE_VIEW=truepreference (default OFF). Screenshots to follow when the feature is promoted to default.PR Type
Enhancement
Description
Introduce HTML tree view for Snyk issues.
Integrate with Language Server for rendering.
Apply Eclipse theme colors to HTML.
Synchronize selection with legacy tree.
Add feature flag and documentation.
Diagram Walkthrough
File Walkthrough
1 files
Ensure locale-independent string formatting for CSS8 files
Add HTML tree view feature flag preferenceAdd method for updating HTML tree viewImplement HTML tree view in Snyk toolNew handler for HTML tree browserAdd LSP constant for tree view notificationHandle tree view notification from LSNew DTO for tree view parametersUpdate fallback settings form for CLI config1 files
Improve Snyk URI validation logic4 files
Test locale formatting in HTML providerTest HTML tree view browser handler logicTest URI details product normalizationTest tree view notification dispatch1 files
Add documentation for HTML tree view