feat: cmd+click to open file paths in terminal #2859
Conversation
Custom FilePathLinkProvider registered via xterm.js registerLinkProvider API. Detects file paths in terminal output and opens them in Wave's preview block on cmd+click (ctrl+click on Linux/Windows). Patterns matched: - Absolute paths: /path/to/file.ts - Home paths: ~/path/to/file - Relative paths: ./path/to/file - file:line:col: /path/file.ts:42:10 Builds on the hyperlink support from wavetermdev#1357.
WalkthroughA new terminal link provider is added to detect and enable file path references in terminal output. The FilePathLinkProvider class scans terminal lines using regex patterns to identify file paths, validates them, and generates clickable links with precise 1-based range coordinates. When activated with a modifier key (Cmd on macOS, Ctrl elsewhere), it resolves the path relative to the current working directory and opens a file preview. The provider is integrated into the terminal wrapper during construction. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
Warning Tools execution failed with the following error: Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error) Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
sawka
added
the
maintainer-interest
![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
🤖 Fix all issues with AI agents
In `@frontend/app/view/term/term-link-provider.ts`:
- Around line 20-21: KNOWN_EXTENSIONS only matches names with a leading dot so
files like "Makefile" or "Dockerfile" are being excluded; add a separate
set/array of knownFilenames (e.g., ["Makefile","Dockerfile","README","LICENSE"])
and update the filter that uses KNOWN_EXTENSIONS (in term-link-provider.ts) to
allow a path if it either matches KNOWN_EXTENSIONS OR its basename is in
knownFilenames; implement basename extraction using path.basename or similar and
use a Set for O(1) lookups to keep behavior consistent and performant.
In `@frontend/app/view/term/termwrap.ts`:
- Line 175: The call to this.terminal.registerLinkProvider(new
FilePathLinkProvider(this.terminal, this.blockId)) returns an IDisposable that
is not currently stored, causing a potential leak; modify the constructor to
capture that return value and push it onto this.toDispose (the same array used
for onContextLoss/onBell) so it will be disposed in dispose(), referencing
registerLinkProvider, FilePathLinkProvider, this.toDispose, and dispose() to
locate and update the code.
🧹 Nitpick comments (3)
frontend/app/view/term/term-link-provider.ts (3)
16-17: Regex allows very short absolute paths like/awhich may cause false positives.The absolute-path group
(\/[\w.+\-@/]*[\w.+\-@])matches any path starting with/followed by a single valid character. Outputs like/or references to/ain terminal text could trigger spurious links. Consider requiring at least two path segments (a subdirectory or filename with extension) for absolute paths as well, or at minimum a slightly longer minimum match. This is a minor UX concern rather than a correctness bug.
51-61: Minor duplication:getCwdandgetConnectionboth look up the same block atom.Both functions independently resolve the same
WOS.getWaveObjectAtomandglobalStore.getcall. Could be combined into a single helper returning{ cwd, connection }. Low priority.
130-148:text.indexOf(":")will mis-strip the path for Windows-style drive letters if ever encountered.Currently the regex doesn't match Windows paths (e.g.,
C:\...), so this is safe today. However, if the regex is ever extended,colonIdx > 0at position 1 would truncate a drive letter. Worth a brief comment noting the assumption, but not blocking.More practically — the
activatecallback silently does nothing when the modifier isn't held. This is consistent with howWebLinksAddonis used intermwrap.ts, so it's fine. But the link decorations (underline + pointer cursor) will show on hover even without the modifier, which may confuse users. xterm'sILinkProviderAPI doesn't support conditional decorations, so this is an inherent limitation.
| const KNOWN_EXTENSIONS = | ||
| /\.(ts|tsx|js|jsx|mjs|cjs|py|rb|go|rs|java|c|cpp|h|hpp|css|scss|less|html|json|yaml|yml|toml|md|txt|sh|bash|zsh|fish|lua|zig|swift|kt|scala|ex|exs|erl|hrl|vue|svelte|astro|sql|graphql|gql|proto|Makefile|Dockerfile|conf|cfg|ini|env|xml|csv|log)$/; |
There was a problem hiding this comment.
Makefile, Dockerfile, etc. will never match as "extensions".
The KNOWN_EXTENSIONS regex requires a leading dot (\.), so it will match .ts, .py, etc., but it will never match Makefile or Dockerfile since those are filenames, not extensions. A bare relative path like src/Makefile would be incorrectly filtered out.
Proposed fix: separate filename check
-const KNOWN_EXTENSIONS =
- /\.(ts|tsx|js|jsx|mjs|cjs|py|rb|go|rs|java|c|cpp|h|hpp|css|scss|less|html|json|yaml|yml|toml|md|txt|sh|bash|zsh|fish|lua|zig|swift|kt|scala|ex|exs|erl|hrl|vue|svelte|astro|sql|graphql|gql|proto|Makefile|Dockerfile|conf|cfg|ini|env|xml|csv|log)$/;
+const KNOWN_EXTENSIONS =
+ /\.(ts|tsx|js|jsx|mjs|cjs|py|rb|go|rs|java|c|cpp|h|hpp|css|scss|less|html|json|yaml|yml|toml|md|txt|sh|bash|zsh|fish|lua|zig|swift|kt|scala|ex|exs|erl|hrl|vue|svelte|astro|sql|graphql|gql|proto|conf|cfg|ini|env|xml|csv|log)$/;
+
+const KNOWN_FILENAMES = /\/(Makefile|Dockerfile|Rakefile|Gemfile|Justfile)$/;Then in the filter logic (around line 101):
- if (match[5] && !KNOWN_EXTENSIONS.test(match[5])) {
+ if (match[5] && !KNOWN_EXTENSIONS.test(match[5]) && !KNOWN_FILENAMES.test(match[5])) {🤖 Prompt for AI Agents
In `@frontend/app/view/term/term-link-provider.ts` around lines 20 - 21,
KNOWN_EXTENSIONS only matches names with a leading dot so files like "Makefile"
or "Dockerfile" are being excluded; add a separate set/array of knownFilenames
(e.g., ["Makefile","Dockerfile","README","LICENSE"]) and update the filter that
uses KNOWN_EXTENSIONS (in term-link-provider.ts) to allow a path if it either
matches KNOWN_EXTENSIONS OR its basename is in knownFilenames; implement
basename extraction using path.basename or similar and use a Set for O(1)
lookups to keep behavior consistent and performant.
| this.terminal.parser.registerOscHandler(16162, (data: string) => { | ||
| return handleOsc16162Command(data, this.blockId, this.loaded, this); | ||
| }); | ||
| this.terminal.registerLinkProvider(new FilePathLinkProvider(this.terminal, this.blockId)); |
There was a problem hiding this comment.
registerLinkProvider returns an IDisposable that is not tracked — potential resource leak.
terminal.registerLinkProvider(...) returns an IDisposable. Other registrations in this constructor (e.g., onContextLoss, onBell) are pushed to this.toDispose for cleanup in dispose(). This one should be too.
Proposed fix
- this.terminal.registerLinkProvider(new FilePathLinkProvider(this.terminal, this.blockId));
+ this.toDispose.push(
+ this.terminal.registerLinkProvider(new FilePathLinkProvider(this.terminal, this.blockId))
+ );📝 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.
| this.terminal.registerLinkProvider(new FilePathLinkProvider(this.terminal, this.blockId)); | |
| this.toDispose.push( | |
| this.terminal.registerLinkProvider(new FilePathLinkProvider(this.terminal, this.blockId)) | |
| ); |
🤖 Prompt for AI Agents
In `@frontend/app/view/term/termwrap.ts` at line 175, The call to
this.terminal.registerLinkProvider(new FilePathLinkProvider(this.terminal,
this.blockId)) returns an IDisposable that is not currently stored, causing a
potential leak; modify the constructor to capture that return value and push it
onto this.toDispose (the same array used for onContextLoss/onBell) so it will be
disposed in dispose(), referencing registerLinkProvider, FilePathLinkProvider,
this.toDispose, and dispose() to locate and update the code.
2 participants
Summary
Added cmd+click detection for file paths in the terminal. Click any file path and it opens in Wave's preview block. Way more useful than I expected — especially when Claude or build tools spit out file paths in error messages.
Implementation:
FilePathLinkProviderregistered via xterm.jsregisterLinkProviderAPI~/paths, andfile:line:colpatternspreview:block typetermwrap.ts, provider logic in new fileterm-link-provider.tsPatterns matched:
/absolute/path/to/file.ts./relative/path~/home/path/path/to/file.ts:42:10(file:line:col)Builds on the hyperlink support from #1357.
Test plan
~/path — should resolve and openfile:line:colpattern (e.g., from TypeScript errors) — should open