From a02a90fdafcbb20c7c93276ec54a2421635f042b Mon Sep 17 00:00:00 2001 From: Daniel Martin <56157528+dmartinochoa@users.noreply.github.com> Date: Tue, 19 May 2026 15:52:11 +0200 Subject: [PATCH] ux: filter findings, non-preview open, quieter copy toasts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three small UX wins from the post-v0.2.0 review queue. Stacked on ux-polish-2 (#20). #5 — Filter the Findings tree - Pipeline-Check: Filter Findings command opens an InputBox; matches against rule ID, message body, and file path (case-insensitive substring). Re-invoke pre-fills the current filter so users can edit or clear (empty input clears). - New `$(filter)` button on the Findings view title bar. - FindingsTreeProvider gets getFilter/setFilter/applyFilter. The badge tracks the filtered count; `lastFindingUris` still tracks the unfiltered universe so a publish for a currently-hidden URI still wakes the tree up (otherwise a CLEAR of a filtered-out finding would never refresh). - 8 tests covering: default empty, rule-id match, case-insensitivity, message-body match, fsPath match, clear via empty string, whitespace trim, badge reflects filtered count. #4 — Open Finding (non-preview) - Pipeline-Check: Open Finding context-menu entry on tree leaves. Opens with `preview: false` — pins the file as a permanent tab so triaging multiple findings side-by-side doesn't create tab clutter. The single-click default still uses preview, keeping the common "scan through findings" flow lightweight. - LeafLike type widened to include `uri` and `diagnostic.range`. #7 — Quieter clipboard confirmations - Copy Rule ID and Copy LSP Install Command switch from showInformationMessage (modal toast) to setStatusBarMessage with a 2-second TTL (CONFIRM_TTL_MS). The copy succeeds silently 95% of the time anyway; the status bar confirmation reads without stealing focus. Wiring - package.json: three new command declarations; view/title button for filter (group navigation@1, between scan and changeGrouping); view/item/context entry for openNonPreview (group navigation@0, top of the menu); commandPalette gates the leaf-context commands to `false` so they don't show palette-wide without a node. - src/findingsView.ts: filter state + setFilter + getFilter + applyFilter (in findings()); sets the `pipelineCheck.filterActive` context key so future manifest contributions can paint a "filter active" affordance. - src/test/integration/activation.test.ts: command-registration expected list grows by filter + openNonPreview. Total: 137 tests pass (was 129 on #20; +8 filter behaviour). Lint, compile, smoke, integration-compile all green. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 18 ++++ package.json | 27 +++++- src/extension.ts | 57 +++++++++++- src/findingsView.test.ts | 116 ++++++++++++++++++++++++ src/findingsView.ts | 48 +++++++++- src/test/integration/activation.test.ts | 2 + 6 files changed, 260 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 02c17a5..cc0a06b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,19 @@ versions follow [SemVer](https://semver.org/). ### Added +- **`Pipeline-Check: Filter Findings` command.** Opens an InputBox; + matches against rule ID, message body, and file path + (case-insensitive substring). Re-invoking the command pre-fills + the current filter so users can edit or clear (empty input + clears). New `$(filter)` button on the Findings view title bar. + The badge updates to reflect the filtered count; the + `lastFindingUris` set still tracks the unfiltered universe so a + publish for a currently-hidden finding still wakes the tree up. +- **`Pipeline-Check: Open Finding` context-menu entry** on Findings + tree leaves. Opens the finding as a **permanent (non-preview)** + tab — useful when triaging multiple findings side-by-side. The + default click-to-reveal still uses preview-style so the common + "click through to scan" flow doesn't create tab clutter. - **Status bar background colour reflects severity.** A workspace with any CRITICAL finding tints the bar to `statusBarItem.errorBackground` (red in the default themes); a workspace with HIGH but no CRITICAL @@ -41,6 +54,11 @@ versions follow [SemVer](https://semver.org/). ### Changed +- **Quieter clipboard confirmations.** Copy Rule ID and Copy LSP + Install Command now write a 2-second status-bar message instead of + firing a modal information toast. The copy still succeeded + silently 95% of the time anyway; this confirms the action without + stealing focus. - **"Refresh Findings" now triggers a real scan** instead of just re-painting the tree from already-published diagnostics. Matches the user's mental model: clicking a refresh icon should fetch new diff --git a/package.json b/package.json index 93c74c6..35a0350 100644 --- a/package.json +++ b/package.json @@ -131,6 +131,17 @@ "title": "Open Rule Documentation", "category": "Pipeline-Check" }, + { + "command": "pipelineCheck.findings.openNonPreview", + "title": "Open Finding", + "category": "Pipeline-Check" + }, + { + "command": "pipelineCheck.findings.filter", + "title": "Filter Findings", + "category": "Pipeline-Check", + "icon": "$(filter)" + }, { "command": "pipelineCheck.goToNextFinding", "title": "Go to Next Finding", @@ -162,10 +173,15 @@ "group": "navigation@0" }, { - "command": "pipelineCheck.findings.changeGrouping", + "command": "pipelineCheck.findings.filter", "when": "view == pipelineCheck.findings", "group": "navigation@1" }, + { + "command": "pipelineCheck.findings.changeGrouping", + "when": "view == pipelineCheck.findings", + "group": "navigation@2" + }, { "command": "pipelineCheck.findings.refresh", "when": "view == pipelineCheck.findings", @@ -173,6 +189,11 @@ } ], "view/item/context": [ + { + "command": "pipelineCheck.findings.openNonPreview", + "when": "view == pipelineCheck.findings && viewItem == pipelineCheck.finding", + "group": "navigation@0" + }, { "command": "pipelineCheck.findings.openRuleDocs", "when": "view == pipelineCheck.findings && viewItem == pipelineCheck.finding", @@ -200,6 +221,10 @@ { "command": "pipelineCheck.findings.openRuleDocs", "when": "false" + }, + { + "command": "pipelineCheck.findings.openNonPreview", + "when": "false" } ] }, diff --git a/src/extension.ts b/src/extension.ts index 5991c95..bb4e657 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -85,6 +85,11 @@ const LANGUAGE_ID = "pipelineCheck"; const LANGUAGE_NAME = "Pipeline-Check"; const OUTPUT_CHANNEL = "Pipeline-Check"; +// `setStatusBarMessage` TTL for transient confirmations (clipboard +// writes, etc.). Two seconds is long enough to be readable and short +// enough that a stream of copies doesn't pile up. +const CONFIRM_TTL_MS = 2000; + // Structural shape of a Findings-tree leaf node, used by the // context-menu commands. The real LeafNode lives in findingsView.ts; // duplicating just the fields the commands read keeps extension.ts @@ -93,6 +98,8 @@ type LeafLike = { readonly finding?: { readonly ruleId?: string; readonly docsUrl?: string; + readonly uri?: vscode.Uri; + readonly diagnostic?: { readonly range?: vscode.Range }; }; }; @@ -210,8 +217,9 @@ async function startClient(): Promise { await vscode.env.clipboard.writeText( 'pip install "pipeline-check[lsp]"', ); - void vscode.window.showInformationMessage( + vscode.window.setStatusBarMessage( 'Copied: pip install "pipeline-check[lsp]"', + CONFIRM_TTL_MS, ); } else if (choice === "Open server log") { outputChannel.show(); @@ -318,7 +326,10 @@ export async function activate( return; } await vscode.env.clipboard.writeText(id); - void vscode.window.showInformationMessage(`Copied ${id} to clipboard.`); + // Status-bar message instead of a modal toast — the copy + // succeeded silently 95% of the time anyway; this is a + // ~2-second confirmation that doesn't steal focus. + vscode.window.setStatusBarMessage(`Copied ${id}`, CONFIRM_TTL_MS); }, ), vscode.commands.registerCommand( @@ -334,6 +345,45 @@ export async function activate( await vscode.env.openExternal(vscode.Uri.parse(url)); }, ), + // Open a finding without using the editor's preview-tab slot. + // Same target as the default click-to-reveal, but `preview: false` + // pins each opened file as a permanent tab — useful when the user + // is opening several findings side-by-side. Lives only in the + // leaf context menu; the single-click path stays preview-style so + // the common "click through findings to triage" flow doesn't + // create tab clutter. + vscode.commands.registerCommand( + "pipelineCheck.findings.openNonPreview", + async (node: LeafLike | undefined) => { + const uri = node?.finding?.uri; + const range = node?.finding?.diagnostic?.range; + if (!uri) return; + await vscode.commands.executeCommand("vscode.open", uri, { + selection: range, + preserveFocus: false, + preview: false, + }); + }, + ), + // Filter the Findings tree by a substring. Matches against rule + // ID, message body, and fsPath case-insensitively. Re-invoking + // the command pre-fills the current filter so users can edit or + // clear it (empty string clears). + vscode.commands.registerCommand( + "pipelineCheck.findings.filter", + async () => { + const current = findingsProvider.getFilter(); + const next = await vscode.window.showInputBox({ + title: "Filter Pipeline-Check findings", + prompt: + "Match rule ID, message text, or file path. Empty to clear.", + value: current, + placeHolder: "e.g. GHA-001 or release.yml", + }); + if (next === undefined) return; // user cancelled + findingsProvider.setFilter(next); + }, + ), // Copy-install-command also lives in the welcome-state and is // promoted to a top-level command so users can re-find it after // dismissing the first-run notification. @@ -343,8 +393,9 @@ export async function activate( await vscode.env.clipboard.writeText( 'pip install "pipeline-check[lsp]"', ); - void vscode.window.showInformationMessage( + vscode.window.setStatusBarMessage( 'Copied: pip install "pipeline-check[lsp]"', + CONFIRM_TTL_MS, ); }, ), diff --git a/src/findingsView.test.ts b/src/findingsView.test.ts index 6fd555d..a194076 100644 --- a/src/findingsView.test.ts +++ b/src/findingsView.test.ts @@ -488,3 +488,119 @@ describe("FindingsTreeProvider — findings cache invalidation", () => { expect(roots[0].kind === "group" && roots[0].label).toBe("CRITICAL"); }); }); + +describe("FindingsTreeProvider — filter", () => { + // The filter narrows the visible tree to findings whose rule ID, + // message, or fsPath contains the filter string (case-insensitive). + // The badge tracks the filtered count; `lastFindingUris` keeps the + // full set so the batch-touches-us check still wakes us up for + // publishes that would currently be filtered out (otherwise a + // CLEAR of a filtered-out URI would never refresh). + + function fakeTreeView(): { badge: unknown } & object { + return { badge: undefined }; + } + + it("defaults to no filter (getFilter returns empty string)", () => { + const p = new FindingsTreeProvider(ctx); + expect(p.getFilter()).toBe(""); + }); + + it("setFilter narrows the tree to matching rule IDs", () => { + setStubDiagnostics([ + { file: "a.yml", rule: "GHA-001", severity: "HIGH" }, + { file: "b.yml", rule: "GHA-015", severity: "HIGH" }, + { file: "c.yml", rule: "GLI-002", severity: "HIGH" }, + ]); + const p = new FindingsTreeProvider(ctx); + p.setGroupMode("severity"); + expect(p.getChildren()[0]).toMatchObject({ kind: "group" }); + expect((p.getChildren()[0] as unknown as { children: unknown[] }).children).toHaveLength( + 3, + ); + + p.setFilter("GHA"); + const after = p.getChildren()[0] as unknown as { children: unknown[] }; + expect(after.children).toHaveLength(2); + }); + + it("filter is case-insensitive", () => { + setStubDiagnostics([ + { file: "a.yml", rule: "GHA-001", severity: "HIGH" }, + ]); + const p = new FindingsTreeProvider(ctx); + p.setGroupMode("severity"); + p.setFilter("gha"); + const roots = p.getChildren(); + expect(roots).toHaveLength(1); + }); + + it("filter matches the message body, not just the rule ID", () => { + setStubDiagnostics([ + { file: "a.yml", rule: "GHA-001", severity: "HIGH" }, + { file: "b.yml", rule: "GHA-002", severity: "HIGH" }, + ]); + // Both findings have message "GHA-001 title" / "GHA-002 title" + // because setStubDiagnostics builds the message from the rule. + // Filtering on "title" should keep both. + const p = new FindingsTreeProvider(ctx); + p.setGroupMode("severity"); + p.setFilter("title"); + const roots = p.getChildren(); + expect((roots[0] as unknown as { children: unknown[] }).children).toHaveLength(2); + }); + + it("filter matches the fsPath", () => { + setStubDiagnostics([ + { file: "workflows/ci.yml", rule: "GHA-001", severity: "HIGH" }, + { file: "config/dockerfile", rule: "DOCK-001", severity: "HIGH" }, + ]); + const p = new FindingsTreeProvider(ctx); + p.setGroupMode("severity"); + p.setFilter("workflows"); + expect((p.getChildren()[0] as unknown as { children: unknown[] }).children).toHaveLength( + 1, + ); + }); + + it("empty filter clears the narrowing", () => { + setStubDiagnostics([ + { file: "a.yml", rule: "GHA-001", severity: "HIGH" }, + { file: "b.yml", rule: "GLI-002", severity: "HIGH" }, + ]); + const p = new FindingsTreeProvider(ctx); + p.setGroupMode("severity"); + p.setFilter("GHA"); + expect((p.getChildren()[0] as unknown as { children: unknown[] }).children).toHaveLength( + 1, + ); + p.setFilter(""); + expect((p.getChildren()[0] as unknown as { children: unknown[] }).children).toHaveLength( + 2, + ); + }); + + it("setFilter trims whitespace before comparing for change", () => { + setStubDiagnostics([ + { file: "a.yml", rule: "GHA-001", severity: "HIGH" }, + ]); + const p = new FindingsTreeProvider(ctx); + p.setFilter(" GHA "); + expect(p.getFilter()).toBe("GHA"); + }); + + it("badge reflects the filtered count, not the workspace total", () => { + setStubDiagnostics([ + { file: "a.yml", rule: "GHA-001", severity: "HIGH" }, + { file: "b.yml", rule: "GLI-002", severity: "HIGH" }, + { file: "c.yml", rule: "GHA-003", severity: "HIGH" }, + ]); + const p = new FindingsTreeProvider(ctx); + const view = fakeTreeView(); + p.setTreeView(view as unknown as Parameters[0]); + expect((view.badge as { value: number }).value).toBe(3); + + p.setFilter("GHA"); + expect((view.badge as { value: number }).value).toBe(2); + }); +}); diff --git a/src/findingsView.ts b/src/findingsView.ts index a87318c..2ce65e7 100644 --- a/src/findingsView.ts +++ b/src/findingsView.ts @@ -117,6 +117,10 @@ export class FindingsTreeProvider implements vscode.TreeDataProvider { // onDidChangeDiagnostics batch with no remaining pipeline-check // diagnostic, we still need to refresh so the stale leaf vanishes. private lastFindingUris = new Set(); + // Case-insensitive substring; matches against ruleId, message, and + // fsPath. Empty string disables filtering. Stored verbatim (with + // original case) for echo in the InputBox; matched lowercased. + private filter = ""; constructor(context: vscode.ExtensionContext) { // VS Code does not expose a per-source filter on the diagnostic- @@ -152,6 +156,25 @@ export class FindingsTreeProvider implements vscode.TreeDataProvider { return this.groupMode; } + getFilter(): string { + return this.filter; + } + + setFilter(value: string): void { + const trimmed = value.trim(); + if (trimmed === this.filter) return; + this.filter = trimmed; + // Context key lets the manifest's `when` clauses paint a + // "filter active" affordance — e.g. swap the title-bar filter + // icon for a filled variant when the filter has content. + void vscode.commands.executeCommand( + "setContext", + "pipelineCheck.filterActive", + trimmed.length > 0, + ); + this.refresh(); + } + setGroupMode(mode: GroupMode): void { if (this.groupMode === mode) { return; @@ -177,17 +200,34 @@ export class FindingsTreeProvider implements vscode.TreeDataProvider { * findings. Walks the workspace diagnostic store once per refresh * instead of once per consumer, and rebuilds the "URIs we had * findings for" set so the next batch-skip check has fresh data. + * + * When a filter string is active, the returned list is restricted + * to findings whose rule ID, message, or fsPath contains the + * filter (case-insensitive). `lastFindingUris` still tracks the + * *full* set so the batch-touches-us check stays correct — a + * publish for a URI whose finding is currently filtered out + * should still wake us up. */ private findings(): Finding[] { if (this.cachedFindings === null) { - this.cachedFindings = collectFindings(); - this.lastFindingUris = new Set( - this.cachedFindings.map((f) => f.uri.toString()), - ); + const all = collectFindings(); + this.lastFindingUris = new Set(all.map((f) => f.uri.toString())); + this.cachedFindings = this.applyFilter(all); } return this.cachedFindings; } + private applyFilter(findings: readonly Finding[]): Finding[] { + if (!this.filter) return [...findings]; + const needle = this.filter.toLowerCase(); + return findings.filter((f) => { + if (f.ruleId.toLowerCase().includes(needle)) return true; + if (f.diagnostic.message.toLowerCase().includes(needle)) return true; + if (f.uri.fsPath.toLowerCase().includes(needle)) return true; + return false; + }); + } + /** * Returns true if any of the changed URIs in this batch either * carries a pipeline-check diagnostic right now (publish or update) diff --git a/src/test/integration/activation.test.ts b/src/test/integration/activation.test.ts index 9db8645..7670adc 100644 --- a/src/test/integration/activation.test.ts +++ b/src/test/integration/activation.test.ts @@ -37,8 +37,10 @@ suite("Pipeline-Check — activation", () => { "pipelineCheck.scanWorkspace", "pipelineCheck.findings.refresh", "pipelineCheck.findings.changeGrouping", + "pipelineCheck.findings.filter", "pipelineCheck.findings.copyRuleId", "pipelineCheck.findings.openRuleDocs", + "pipelineCheck.findings.openNonPreview", "pipelineCheck.goToNextFinding", "pipelineCheck.goToPreviousFinding", ];