feat(desktop): add multi-line support for URL links in terminal#467
feat(desktop): add multi-line support for URL links in terminal#467
Conversation
Extract multi-line spanning logic into a shared MultiLineLinkProvider base class and create a new UrlLinkProvider that supports URLs spanning wrapped terminal lines, replacing WebLinksAddon. Changes: - Create MultiLineLinkProvider abstract base class with shared multi-line logic - Refactor FilePathLinkProvider to extend the base class - Add UrlLinkProvider for URL detection with multi-line support - Replace WebLinksAddon with UrlLinkProvider in helpers.ts - Add comprehensive tests for UrlLinkProvider 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Warning Rate limit exceeded@Kitenite has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 15 minutes and 8 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (5)
WalkthroughThis PR refactors terminal link handling in the desktop application by introducing a provider-based architecture. It replaces inline WebLinksAddon URL logic with a dedicated UrlLinkProvider, introduces a new FilePathLinkProvider for file path detection, and creates a MultiLineLinkProvider abstract base class to consolidate shared multi-line link detection and activation logic. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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: 1
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/UrlLinkProvider.ts (1)
7-8: Consider potential edge cases with the URL pattern.The URL pattern handles balanced parentheses well, but may not correctly match nested parentheses like
https://example.com/wiki/A_(B_(C)). The current pattern\([^\s<>[\]()'"]*\)only matches content without parentheses inside.This is a minor edge case—the current implementation handles most real-world URLs including Wikipedia links correctly.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/FilePathLinkProvider.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/MultiLineLinkProvider.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/UrlLinkProvider.test.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/UrlLinkProvider.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid using any type in TypeScript - maintain type safety unless absolutely necessary
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/UrlLinkProvider.test.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/UrlLinkProvider.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/MultiLineLinkProvider.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/FilePathLinkProvider.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Run Biome for formatting, linting, import organization, and safe fixes at the root level using bun run lint:fix
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/UrlLinkProvider.test.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/UrlLinkProvider.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/MultiLineLinkProvider.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/FilePathLinkProvider.ts
**/{components,features}/**/*.{ts,tsx,test.ts,test.tsx,stories.tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Co-locate component dependencies (utils, hooks, constants, config, tests, stories) next to the file using them
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/UrlLinkProvider.test.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/UrlLinkProvider.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/MultiLineLinkProvider.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/FilePathLinkProvider.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Place tests co-located with components using .test.ts or .test.tsx naming convention
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/UrlLinkProvider.test.ts
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules in renderer process or shared code - use only in main process (src/main/)
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/UrlLinkProvider.test.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/UrlLinkProvider.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/MultiLineLinkProvider.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/FilePathLinkProvider.ts
apps/desktop/src/{main,renderer,preload}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use type-safe IPC communication - define channel types in apps/desktop/src/shared/ipc-channels.ts before implementing handlers
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/UrlLinkProvider.test.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/UrlLinkProvider.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/MultiLineLinkProvider.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/FilePathLinkProvider.ts
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/UrlLinkProvider.test.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/UrlLinkProvider.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/MultiLineLinkProvider.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/FilePathLinkProvider.ts
🧠 Learnings (1)
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to apps/desktop/src/{main,renderer,preload}/**/*.{ts,tsx} : Use type-safe IPC communication - define channel types in apps/desktop/src/shared/ipc-channels.ts before implementing handlers
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts
🧬 Code graph analysis (4)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/UrlLinkProvider.test.ts (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/UrlLinkProvider.ts (1)
UrlLinkProvider(4-36)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/UrlLinkProvider.ts (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/MultiLineLinkProvider.ts (1)
LinkMatch(6-17)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/UrlLinkProvider.ts (1)
UrlLinkProvider(4-36)apps/desktop/src/renderer/lib/trpc-client.ts (1)
trpcClient(9-11)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/FilePathLinkProvider.ts (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/MultiLineLinkProvider.ts (1)
LinkMatch(6-17)
🪛 ast-grep (0.40.3)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/UrlLinkProvider.ts
[warning] 18-18: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(this.URL_PATTERN.source, "g")
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/FilePathLinkProvider.ts
[warning] 22-22: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(this.FILE_PATH_PATTERN.source, "g")
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (14)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts (1)
152-157: LGTM! Clean integration of UrlLinkProvider.The new URL link provider follows the same registration pattern as
FilePathLinkProvider, uses tRPC for IPC communication (as per coding guidelines), and includes proper error handling consistent with the existing file path link provider.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/UrlLinkProvider.test.ts (4)
6-14: LGTM! Well-structured mock helpers.The mock creation functions properly simulate terminal buffer lines with wrapped line indicators, which is essential for testing the multi-line link detection functionality.
48-143: Good coverage of URL detection scenarios.The tests comprehensively cover various URL formats including protocols, query parameters, fragments, ports, and parentheses (Wikipedia-style URLs). This validates the URL_PATTERN regex handles common real-world URLs correctly.
145-235: Thorough multi-line wrapping tests.The tests validate the core feature of this PR—detecting URLs that span wrapped terminal lines. Testing from multiple perspectives (forward-looking, backward-looking, and middle-line scanning) ensures robust cross-line detection.
270-322: Activation behavior properly tested.Tests correctly verify that URL activation requires either
metaKey(Cmd on Mac) orctrlKey(Windows/Linux), and that theonOpencallback receives the correct URL.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/UrlLinkProvider.ts (2)
17-20: Static analysis false positive—pattern is constructed from a constant.The ast-grep warning about ReDoS is a false positive. The regex is constructed from
this.URL_PATTERN.source, which is a constant private field defined at class initialization, not from variable/user input. The pattern is static and safe.
27-35: LGTM! Activation handling follows platform conventions.The modifier key check (
metaKeyfor Mac,ctrlKeyfor Windows/Linux) correctly implements the expected CMD+click / Ctrl+click behavior for opening links, consistent with terminal conventions.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/MultiLineLinkProvider.ts (3)
6-17: LGTM! Well-designed LinkMatch interface.The interface provides all necessary context for subclass filtering: the matched text, indices, combined text for context analysis, and regex groups. This enables flexible filtering in derived classes.
34-61: Clean abstract class design.The abstract hooks (
getPattern,shouldSkipMatch,handleActivation) provide a clear contract for subclasses while keeping the multi-line detection logic centralized in the base class. Good separation of concerns.
62-146: Solid multi-line link detection implementation.The
provideLinksmethod correctly:
- Combines adjacent wrapped lines into a single text buffer
- Tracks offsets to map match positions back to line coordinates
- Filters matches to only those overlapping the current line
- Creates ILink objects with proper activation callbacks
The approach of looking at prev + current + next lines ensures URLs/paths split across wrap boundaries are detected regardless of which line the user clicks.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/FilePathLinkProvider.ts (4)
5-19: LGTM! Clean refactoring to extend MultiLineLinkProvider.The class now properly extends the base class, delegating multi-line detection logic while retaining its file path-specific pattern and filtering. Constructor passes the terminal to super correctly.
26-61: Comprehensive match filtering.The
shouldSkipMatchmethod correctly filters out:
- URLs (http/https/ftp protocols)
- Version strings (v1.2.3 format)
- npm package references (detected via @Version context)
- Pure numbers (like line:column without a path)
This prevents false positives when file path patterns might match non-path text.
21-24: Static analysis false positive—same as UrlLinkProvider.The regex is constructed from the constant
FILE_PATH_PATTERN.sourcefield, not from variable input. This is not a ReDoS vulnerability.
63-77: LGTM! Activation logic preserved.The
handleActivationmethod correctly parses the path usingline-column-pathand invokes theonOpencallback with the parsed file, line, and column information. Modifier key check is consistent with URL provider.
![charliecreates[bot]](https://avatars.githubusercontent.com/in/1140780?s=60&v=4){kind=small}
There was a problem hiding this comment.
The biggest correctness gap is that MultiLineLinkProvider only supports links spanning at most three physical lines (prev/current/next), so very long wrapped URLs/paths can be truncated or mis-ranged. The URL matching also contradicts its comment about “excluding trailing punctuation” and will likely capture ./,/etc. at end-of-sentence, producing broken opens. There’s also some avoidable confusion around naming RegExpMatchArray as groups, and provider registration order could be made more robust against overlaps.
Additional notes (2)
- Maintainability |
...spaceView/ContentView/TabsContent/Terminal/MultiLineLinkProvider.ts:113-142
MultiLineLinkProviderpassesmatchtohandleActivationas thegroupsparameter, butmatchAll()yields aRegExpMatchArraywhose.indexand.inputare present and may be relied upon by subclasses. InLinkMatchyou’re also calling the arraygroups, which is confusing because JS regex has a separate.groupsobject for named capture groups.
This is a maintainability footgun: future subclasses will misread groups as “capture groups only” and may accidentally depend on array indices or .groups semantics inconsistently.
- Maintainability |
...omponents/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts:149-158
createTerminalInstancenow registers the URL provider after the file-path provider. With xterm link providers, ordering matters when links overlap; you may end up preferring a file-path match inside a URL-like string (or vice versa) depending on how xterm resolves conflicts.
Given you explicitly skip URLs inside FilePathLinkProvider.shouldSkipMatch, it’s probably fine, but it would be safer to register the URL provider first to avoid any subtle overlap or future regressions if the file-path regex becomes more permissive.
Summary of changes
What changed
✅ New multi-line link infrastructure
- Added an abstract
MultiLineLinkProvider(apps/desktop/.../Terminal/MultiLineLinkProvider.ts) that:- Detects wrapped lines via
IBufferLine.isWrapped - Builds a
combinedTextfrom[prev] + current + [next] - Filters matches to those overlapping the current line
- Computes an
ILink["range"]spanning up to 3 lines
- Detects wrapped lines via
✅ URL links now support wrapped lines
- Introduced
UrlLinkProvider(.../Terminal/UrlLinkProvider.ts) using a URL regex andMultiLineLinkProviderfor multi-line ranges. - Replaced
@xterm/addon-web-linksusage inhelpers.tswithxterm.registerLinkProvider(new UrlLinkProvider(...))to unify handling.
✅ File path provider refactor
- Refactored
FilePathLinkProviderto extendMultiLineLinkProvider, removing its custom multi-line/range logic. - Updated matching to use a global regex pattern and a
shouldSkipMatch()filter.
✅ Test coverage
- Added
UrlLinkProvider.test.tswith cases for single-line URLs, two-line wrap, three-line wrap, activation gating, and non-wrapped behavior.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/url-link-provider.test.ts (1)
5-43: Consider extracting shared test helpers.The
createMockLine,createMockTerminal, andgetLinkshelper functions are duplicated between this file andfile-path-link-provider.test.ts. Consider extracting these to a shared test utility file (e.g.,test-helpers.tsortest-utils.ts) to reduce duplication and improve maintainability.🔎 Suggested refactor to extract shared helpers
Create a new file
test-helpers.tsin the same directory:import { mock } from "bun:test"; import type { IBufferLine, ILink, ILinkProvider, Terminal } from "@xterm/xterm"; export function createMockLine(text: string, isWrapped = false): IBufferLine { return { translateToString: () => text, isWrapped, length: text.length, getCell: mock(() => null), getCells: mock(() => []), } as unknown as IBufferLine; } export function createMockTerminal( lines: Array<{ text: string; isWrapped?: boolean }>, ): Terminal { const mockLines = lines.map((l) => createMockLine(l.text, l.isWrapped ?? false), ); return { buffer: { active: { getLine: (index: number) => mockLines[index] ?? null, }, }, element: { style: { cursor: "" }, }, } as unknown as Terminal; } export function getLinks( provider: ILinkProvider, lineNumber: number, ): Promise<ILink[]> { return new Promise((resolve) => { provider.provideLinks(lineNumber, (links) => { resolve(links ?? []); }); }); }Then import in both test files:
-function createMockLine(text: string, isWrapped = false): IBufferLine { - return { - translateToString: () => text, - isWrapped, - length: text.length, - getCell: mock(() => null), - getCells: mock(() => []), - } as unknown as IBufferLine; -} - -function createMockTerminal( - lines: Array<{ text: string; isWrapped?: boolean }>, -): Terminal { - const mockLines = lines.map((l) => - createMockLine(l.text, l.isWrapped ?? false), - ); - - return { - buffer: { - active: { - getLine: (index: number) => mockLines[index] ?? null, - }, - }, - element: { - style: { cursor: "" }, - }, - } as unknown as Terminal; -} - -function getLinks( - provider: UrlLinkProvider, - lineNumber: number, -): Promise<ILink[]> { - return new Promise((resolve) => { - provider.provideLinks(lineNumber, (links) => { - resolve(links ?? []); - }); - }); -} +import { createMockLine, createMockTerminal, getLinks } from "./test-helpers";apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/url-link-provider.ts (1)
18-20: Consider caching the compiled RegExp.The
getPattern()method recreates a newRegExpinstance on every call. SinceURL_PATTERNis a constant, consider caching the compiled regex to avoid repeated compilation overhead.🔎 Suggested optimization to cache the compiled regex
export class UrlLinkProvider extends MultiLineLinkProvider { private readonly URL_PATTERN = /\bhttps?:\/\/(?:[^\s<>[\]()'"]+|\([^\s<>[\]()'"]*\))+/g; + private readonly compiledPattern: RegExp; constructor( terminal: Terminal, private readonly onOpen: (event: MouseEvent, uri: string) => void, ) { super(terminal); + this.compiledPattern = new RegExp(this.URL_PATTERN.source, "g"); } protected getPattern(): RegExp { - return new RegExp(this.URL_PATTERN.source, "g"); + return this.compiledPattern; }Note: Since the regex has the
gflag, you'll need to be careful about state. Consider either:
- Creating a fresh instance each time (current approach - stateless but has overhead)
- Resetting
lastIndexafter each use- Returning the source and flags separately for the base class to handle
The current approach is actually safer for stateless operation, so this optimization may not be necessary.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/file-path-link-provider.test.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/file-path-link-provider.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/multi-line-link-provider.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/url-link-provider.test.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/url-link-provider.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid using any type in TypeScript - maintain type safety unless absolutely necessary
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/url-link-provider.test.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/url-link-provider.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/file-path-link-provider.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/file-path-link-provider.test.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/multi-line-link-provider.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Run Biome for formatting, linting, import organization, and safe fixes at the root level using bun run lint:fix
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/url-link-provider.test.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/url-link-provider.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/file-path-link-provider.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/file-path-link-provider.test.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/multi-line-link-provider.ts
**/{components,features}/**/*.{ts,tsx,test.ts,test.tsx,stories.tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Co-locate component dependencies (utils, hooks, constants, config, tests, stories) next to the file using them
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/url-link-provider.test.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/url-link-provider.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/file-path-link-provider.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/file-path-link-provider.test.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/multi-line-link-provider.ts
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules in renderer process or shared code - use only in main process (src/main/)
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/url-link-provider.test.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/url-link-provider.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/file-path-link-provider.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/file-path-link-provider.test.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/multi-line-link-provider.ts
apps/desktop/src/{main,renderer,preload}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use type-safe IPC communication - define channel types in apps/desktop/src/shared/ipc-channels.ts before implementing handlers
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/url-link-provider.test.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/url-link-provider.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/file-path-link-provider.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/file-path-link-provider.test.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/multi-line-link-provider.ts
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/url-link-provider.test.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/url-link-provider.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/file-path-link-provider.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/file-path-link-provider.test.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/multi-line-link-provider.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Place tests co-located with components using .test.ts or .test.tsx naming convention
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/url-link-provider.test.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/file-path-link-provider.test.ts
🧠 Learnings (4)
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to apps/desktop/src/{main,renderer,preload}/**/*.{ts,tsx} : Use type-safe IPC communication - define channel types in apps/desktop/src/shared/ipc-channels.ts before implementing handlers
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/index.ts
📚 Learning: 2025-12-21T04:39:28.543Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-21T04:39:28.543Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : Use alias as defined in `tsconfig.json` when possible
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/index.ts
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to **/*.test.{ts,tsx} : Place tests co-located with components using .test.ts or .test.tsx naming convention
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/url-link-provider.test.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/file-path-link-provider.test.ts
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to **/{components,features}/**/*.{ts,tsx,test.ts,test.tsx,stories.tsx} : Co-locate component dependencies (utils, hooks, constants, config, tests, stories) next to the file using them
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/file-path-link-provider.test.ts
🧬 Code graph analysis (3)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/url-link-provider.test.ts (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/index.ts (1)
UrlLinkProvider(6-6)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/url-link-provider.ts (1)
UrlLinkProvider(7-34)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/url-link-provider.ts (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/multi-line-link-provider.ts (1)
LinkMatch(3-9)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/file-path-link-provider.ts (3)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/index.ts (3)
FilePathLinkProvider(1-1)MultiLineLinkProvider(4-4)LinkMatch(3-3)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/multi-line-link-provider.ts (1)
LinkMatch(3-9)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/FilePathLinkProvider.ts (1)
FilePathLinkProvider(4-190)
🪛 ast-grep (0.40.3)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/url-link-provider.ts
[warning] 18-18: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(this.URL_PATTERN.source, "g")
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/file-path-link-provider.ts
[warning] 24-24: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(this.FILE_PATH_PATTERN.source, "g")
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (13)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/file-path-link-provider.test.ts (1)
1-405: LGTM! Comprehensive test coverage.The test suite thoroughly validates file path detection including:
- Basic path formats (absolute, relative, home directory)
- Multi-line wrapping scenarios (forward, backward, three-line)
- Filtering false positives (URLs, versions, npm packages)
- Activation handling (metaKey/ctrlKey requirements)
- Edge cases (empty lines, non-existent lines)
The test helpers are well-structured and the import path change aligns with the new module structure.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/url-link-provider.test.ts (1)
45-357: LGTM! Excellent test coverage for URL link detection.The test suite comprehensively validates:
- Various URL formats (http/https, query params, fragments, ports, parentheses)
- Multi-line wrapping scenarios across 2-3 lines
- Activation requiring modifier keys (metaKey/ctrlKey)
- Proper edge case handling (empty lines, non-URLs, file paths)
The test on lines 347-355 correctly ensures file paths aren't mistakenly matched as URLs, which is important for preventing conflicts with FilePathLinkProvider.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/index.ts (1)
1-6: LGTM! Clean barrel export pattern.The centralized exports follow TypeScript best practices, providing a clean public API for the link-providers module. This makes imports simpler and more maintainable for consumers.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/url-link-provider.ts (2)
22-24: LGTM! Appropriate no-op implementation.The
shouldSkipMatchalways returningfalseis correct for URL links, as there are no false positive patterns that need filtering (unlike file paths that need to exclude URLs, versions, etc.).
26-33: LGTM! Correct activation handling.The activation logic properly:
- Requires metaKey (Cmd) or ctrlKey for activation
- Prevents default browser behavior
- Forwards the event and text to the onOpen callback
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/file-path-link-provider.ts (3)
28-59: LGTM! Comprehensive false positive filtering.The
shouldSkipMatchimplementation correctly filters out:
- URLs with various protocols (http://, https://, ftp://)
- URL-like patterns prefixed with
:(e.g.,://or:http)- Version strings (v1.2.3)
- npm package references (detected via
@1.2.3pattern in context)- Pure numeric patterns (123:456)
The logic is well-structured and should effectively prevent file path detection from conflicting with URL patterns and other non-file-path content.
61-75: LGTM! Proper activation handling with line/column parsing.The activation logic:
- Correctly requires modifier keys (metaKey or ctrlKey)
- Prevents default browser behavior
- Uses the
line-column-pathlibrary to parse file paths with optional line and column numbers- Safely checks for parsed file before invoking callback
- Forwards all parsed information to the
onOpencallback
9-10: No action required—nested quantifier risk is negligible given the implementation context.The file path pattern does contain a nested quantifier structure, but the risk is minimal because: (1) the regex operates line-by-line through
MultiLineLinkProvider, (2) terminal lines are naturally bounded, (3) character classes constrain matching scope, and (4) additional validation filters matches. This follows standard terminal link detection patterns used in other terminal emulators. The static analysis flag is a false positive in this context.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/multi-line-link-provider.ts (5)
3-9: LGTM! Well-designed LinkMatch interface.The
LinkMatchinterface provides a clean abstraction for passing regex match context to implementations, including:
- The matched text and its position
- The combined multi-line text for context
- Full regex match groups for advanced parsing
This design enables flexible filtering and activation logic in concrete providers.
11-20: LGTM! Clean template method pattern.The abstract class design follows the template method pattern effectively:
getPattern()allows each provider to define its matching patternshouldSkipMatch()enables provider-specific filtering logichandleActivation()provides the full match context including groups for parsingThis abstraction successfully eliminates code duplication between URL and file path providers.
33-49: LGTM! Correct multi-line text combination.The logic correctly:
- Prepends the previous line text only if the current line is wrapped
- Appends the next line text only if it's marked as wrapped
- Calculates the offset to map matches back to line coordinates
This approach ensures links spanning wrapped terminal lines are detected while avoiding false positives from unrelated adjacent lines.
54-95: LGTM! Robust match processing with proper filtering.The match processing loop correctly:
- Iterates all pattern matches in the combined text
- Filters matches that don't overlap the current line (lines 62-64)
- Creates LinkMatch context for filtering decisions (lines 66-72)
- Respects subclass filtering via
shouldSkipMatch(lines 74-76)- Calculates proper multi-line ranges (lines 78-86)
- Passes full match groups to activation handler (line 92)
The activation signature on line 92 correctly passes
match(the RegExpMatchArray) as the third parameter, aligning with the abstract method signature.
100-144: LGTM! Correct multi-line range calculation.The
calculateLinkRangemethod properly handles the complex coordinate mapping when links span multiple wrapped lines:
Start position calculation (lines 121-127):
- If match starts in previous line: uses previous line number with match position
- Otherwise: uses current line number with offset-adjusted position
End position calculation (lines 129-138):
- If match ends in next line: uses next line number with offset from current line end
- If match ends in previous line: uses previous line number with match end position
- Otherwise: uses current line number with offset-adjusted position
Coordinate system (1-based):
- Correctly adds 1 to convert from 0-based string indices to 1-based terminal coordinates
The logic correctly handles all edge cases for single-line, two-line, and three-line wrapped matches.
- Fix URL trailing punctuation: strip trailing .,;:!? from matched URLs - Add transformMatch hook in base class for match post-processing - Rename 'groups' to 'regexMatch' for clarity (avoid confusion with JS named groups) - Register URL provider first for safer overlap handling - Add JSDoc comment documenting 3-line limitation - Add 4 new tests for trailing punctuation stripping 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Replace nested quantifiers with simple pattern to avoid catastrophic backtracking. Parentheses balancing moved to O(n) imperative code. Before: /(?:[^\s<>[\]()'"]+|\([^\s<>[\]()'"]*\))+/ (ReDoS vulnerable) After: /[^\s<>[\]'"]+/ + trimUnbalancedParens() - Add trimUnbalancedParens() for O(n) paren handling - Add 3 tests for parentheses edge cases - Wikipedia-style URLs still work correctly 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add 3 tests with pathological inputs that would hang old pattern - Tests verify completion under 100ms - Fix trimUnbalancedParens to also trim trailing unmatched ( 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 Preview resources have been processed for cleanup |
1 participant
Summary
MultiLineLinkProviderbase classUrlLinkProviderthat supports URLs spanning wrapped terminal linesWebLinksAddonwith the new custom provider for consistent multi-line handlingFilePathLinkProviderto extend the base class, reducing code duplicationChanges
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.