[issues/253] Add clickable file path navigation in terminal and editor documents#387
Merged
Conversation
added 9 commits
March 9, 2026 08:12
Establishes the type foundation for file path navigation providers (#253), parallel to the existing RangeLinkTerminalLink and RangeLinkClickArgs types. These types are needed by the upcoming terminal and document providers that make plain file paths clickable. Benefits: - Clear separation between file path navigation types and RangeLink types - FilePathClickArgs carries the raw path string; no ParsedLink dependency - Consistent typing pattern across the two navigation subsystems
Adds the command ID and message codes needed by the upcoming file path navigation providers (#253). Without these, the providers cannot route document link clicks or show user-facing warnings and errors in a localization-friendly way. Benefits: - CMD_HANDLE_FILE_PATH_CLICK follows existing internal command convention (not surfaced in palette) - WARN_FILE_PATH_NOT_FOUND and ERROR_FILE_PATH_NAVIGATION_FAILED match the pattern of their RangeLink navigation counterparts - All entries are in alphabetical order, consistent with the files' documented invariant
…builder Moving buildFilePathPattern() and extractFilePath() into rangelink-core-ts reuses the battle-tested NOT_AFTER_URL_CHAR lookbehind that already prevents URL false positives in RangeLink detection, rather than duplicating the logic in the extension. The handler is now a thin VSCode-specific wrapper around platform-agnostic pattern building. Benefits: - Single source of truth for URL exclusion across all link detection - navigateToFile() handles tilde expansion, resolution, and user-facing warnings/errors - Pattern tests live in rangelink-core-ts close to the implementation; extension tests focus on navigation behavior
… terminal Adds the terminal link provider that makes plain file paths clickable in VSCode terminal output (#253), parallel to RangeLinkTerminalProvider. Delegates navigation to FilePathNavigationHandler so the handler remains the single place that owns tilde expansion, path resolution, and user-facing feedback. Benefits: - Detects all four path formats (quoted, absolute, relative, tilde) via the shared buildFilePathPattern() from rangelink-core-ts - Quoted paths expose the clean path (without surrounding quotes) in the link data for direct navigation - Error handling: navigation failures are caught, logged at debug, and swallowed — the handler already shows the user-facing message
… editor documents Adds the document link provider that makes plain file paths clickable in VSCode editor files (#253), parallel to RangeLinkDocumentProvider. Uses the same buildFilePathPattern() from rangelink-core-ts so path detection is identical to the terminal provider — one place to update if detection rules change. Benefits: - Quoted paths strip surrounding characters so the command URI carries the clean path for navigation - Click routing follows the existing command URI pattern (handleFilePathClick command registered in extension.ts) - Same guard as RangeLinkDocumentProvider: only register for file/untitled schemes to avoid infinite recursion on output channels
…extension.ts Completes the activation path for plain file path navigation (#253) by registering the two new providers and their click command alongside the existing RangeLink providers. File path providers are created first so they run before RangeLink providers when VS Code scans each line, matching the expected priority order. Benefits: - Both providers follow the same registerWithLogging() + scheme-scoped pattern as RangeLinkTerminalProvider and RangeLinkDocumentProvider - handleFilePathClick command routes directly to filePathDocumentProvider.handleLinkClick(), keeping click handling co-located with detection
Users discovering the extension need to know that plain file paths in terminal output and editor documents are now clickable, not just `#L`-style RangeLinks. Without a README entry, the feature is invisible to anyone reading the docs before installing. Benefits: - Feature is discoverable under the existing Link Navigation section - Unreleased marker signals the feature is on main but not yet released - Parallel structure with existing RangeLink navigation subsection sets consistent expectations
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughAdds clickable plain file-path navigation across terminal output and editor documents: new regex-based detection and extractor, FilePathTerminalProvider and FilePathDocumentProvider, a FilePathNavigationHandler that resolves/opens paths with existence checks, command wiring, i18n entries, and comprehensive tests. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Editor as Editor/Document
participant Provider as FilePathDocumentProvider
participant Handler as FilePathNavigationHandler
participant Adapter as VscodeAdapter
participant FS as File System
User->>Editor: Open document containing file path
Editor->>Provider: provideDocumentLinks()
Provider->>Provider: buildFilePathPattern() / extractFilePath()
Provider->>Editor: return DocumentLink[] (command URIs)
User->>Editor: Cmd+click file path
Editor->>Provider: handleLinkClick(FilePathClickArgs)
Provider->>Handler: navigateToFile(rawPath)
Handler->>Handler: expand tilde if present
Handler->>Adapter: resolve path -> URI
Adapter->>FS: check file existence
alt File exists
Adapter->>Editor: showTextDocument(URI)
Handler->>Handler: log success
else Not found
Handler->>User: show warning toast
Handler->>Handler: log warning
end
sequenceDiagram
participant User
participant Terminal
participant Provider as FilePathTerminalProvider
participant Handler as FilePathNavigationHandler
participant Adapter as VscodeAdapter
participant FS as File System
User->>Terminal: Paste output with file paths
Terminal->>Provider: provideTerminalLinks(line)
Provider->>Provider: buildFilePathPattern() / extractFilePath()
Provider->>Terminal: return FilePathTerminalLink[]
User->>Terminal: Click terminal link
Terminal->>Provider: handleTerminalLink(FilePathTerminalLink)
Provider->>Handler: navigateToFile(link.data)
Handler->>Adapter: resolve path -> URI
Adapter->>FS: check file existence
alt File exists
Adapter->>Editor: showTextDocument(URI)
Handler->>Handler: log success
else Not found
Handler->>User: show warning toast
Handler->>Handler: log warning
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/rangelink-vscode-extension/src/__tests__/navigation/FilePathTerminalProvider.test.ts (2)
76-83: Assert both detected links exactly in the multi-match case.This only checks
data, so regressions instartIndex,length, ortooltipfor either link would still pass. A singletoStrictEqualon the full array makes this scenario much harder to break silently.🧪 Suggested assertion tightening
- expect(links).toHaveLength(2); - expect(links[0].data).toBe('./src/a.ts'); - expect(links[1].data).toBe('./src/b.ts'); + expect(links).toStrictEqual([ + { + startIndex: 5, + length: 10, + tooltip: 'Open ./src/a.ts \u2022 RangeLink', + data: './src/a.ts', + }, + { + startIndex: 19, + length: 10, + tooltip: 'Open ./src/b.ts \u2022 RangeLink', + data: './src/b.ts', + }, + ]);As per coding guidelines, "Use
.toStrictEqual()for asserting objects and arrays, never.toContainEqual()which is a partial matcher that hides unexpected properties" and "Never use partial matchers likeexpect.objectContaining()orexpect.stringContaining()- assert exact values instead".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rangelink-vscode-extension/src/__tests__/navigation/FilePathTerminalProvider.test.ts` around lines 76 - 83, The test should assert the full link objects instead of just `data`: in the 'should detect multiple paths in one line' test, replace the current separate checks of `links` with a single exact equality assertion (use `.toStrictEqual`) comparing the array returned from `provider.provideTerminalLinks(context)` (cast to `FilePathTerminalLink[]`) to the expected array of objects including every property (`data`, `startIndex`, `length`, `tooltip`) for both matches detected for the input created by `createMockTerminalContext('From ./src/a.ts to ./src/b.ts')`, so that `links` is verified exactly and not partially.
123-134: Add the happy-path logger assertion here too.This test verifies delegation, but it does not verify the expected log entry for the successful click path. Keeping the logger assertion in the same behavior test matches the repo's test expectations and catches observability regressions alongside functional ones.
Based on learnings, "Always test logging behavior by including logger assertions in tests that verify method behavior - consolidate with behavior tests rather than creating separate logging tests".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rangelink-vscode-extension/src/__tests__/navigation/FilePathTerminalProvider.test.ts` around lines 123 - 134, The test for delegation is missing the happy-path log assertion: after calling provider.handleTerminalLink(link) and asserting mockHandler.navigateToFile was called, add an assertion that the test logger (the logger injected into the provider in this test setup) was called with the successful-click message for '/path/file.ts' (e.g., logger.info/ debug/trace was invoked with a message containing '/path/file.ts' or the exact expected success string). Place this assertion in the same test ("should delegate to handler.navigateToFile") immediately after the existing navigateToFile expectation and use the same mocked logger instance used by the provider so the test covers both behavior and observability.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/rangelink-vscode-extension/src/__tests__/navigation/FilePathNavigationHandler.test.ts`:
- Around line 1-6: Replace the built-in module import to use the Node.js
protocol and remove the empty line in the import group: change the import of
'os' to use the 'node:os' prefix and reorganize the imports so there is no blank
line between import statements (the symbols to update are the import for os plus
the imports of Logger/createMockLogger and buildFilePathPattern/extractFilePath
used in FilePathNavigationHandler.test.ts).
In `@packages/rangelink-vscode-extension/src/extension.ts`:
- Around line 444-448: The command URI currently serializes a bare object as the
command argument in FilePathDocumentProvider (where link targets are produced)
but VS Code expects a JSON array; update the link target generation in
FilePathDocumentProvider so the command URI encodes [args] (wrap the existing
FilePathClickArgs object in an array), and update the handler registered via
ideAdapter.registerCommand(CMD_HANDLE_FILE_PATH_CLICK) /
FilePathDocumentProvider.handleLinkClick to unwrap the first element of the
incoming args array (cast that element to FilePathClickArgs) before proceeding.
---
Nitpick comments:
In
`@packages/rangelink-vscode-extension/src/__tests__/navigation/FilePathTerminalProvider.test.ts`:
- Around line 76-83: The test should assert the full link objects instead of
just `data`: in the 'should detect multiple paths in one line' test, replace the
current separate checks of `links` with a single exact equality assertion (use
`.toStrictEqual`) comparing the array returned from
`provider.provideTerminalLinks(context)` (cast to `FilePathTerminalLink[]`) to
the expected array of objects including every property (`data`, `startIndex`,
`length`, `tooltip`) for both matches detected for the input created by
`createMockTerminalContext('From ./src/a.ts to ./src/b.ts')`, so that `links` is
verified exactly and not partially.
- Around line 123-134: The test for delegation is missing the happy-path log
assertion: after calling provider.handleTerminalLink(link) and asserting
mockHandler.navigateToFile was called, add an assertion that the test logger
(the logger injected into the provider in this test setup) was called with the
successful-click message for '/path/file.ts' (e.g., logger.info/ debug/trace was
invoked with a message containing '/path/file.ts' or the exact expected success
string). Place this assertion in the same test ("should delegate to
handler.navigateToFile") immediately after the existing navigateToFile
expectation and use the same mocked logger instance used by the provider so the
test covers both behavior and observability.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ec13a25d-4e86-42df-8eac-319cd7727512
📒 Files selected for processing (20)
packages/rangelink-core-ts/src/__tests__/utils/buildFilePathPattern.test.tspackages/rangelink-core-ts/src/utils/buildLinkPattern.tspackages/rangelink-vscode-extension/CHANGELOG.mdpackages/rangelink-vscode-extension/README.mdpackages/rangelink-vscode-extension/src/__tests__/extension.test.tspackages/rangelink-vscode-extension/src/__tests__/helpers/createMockFilePathNavigationHandler.tspackages/rangelink-vscode-extension/src/__tests__/helpers/index.tspackages/rangelink-vscode-extension/src/__tests__/navigation/FilePathDocumentProvider.test.tspackages/rangelink-vscode-extension/src/__tests__/navigation/FilePathNavigationHandler.test.tspackages/rangelink-vscode-extension/src/__tests__/navigation/FilePathTerminalProvider.test.tspackages/rangelink-vscode-extension/src/constants/commandIds.tspackages/rangelink-vscode-extension/src/extension.tspackages/rangelink-vscode-extension/src/i18n/messages.en.tspackages/rangelink-vscode-extension/src/navigation/FilePathDocumentProvider.tspackages/rangelink-vscode-extension/src/navigation/FilePathNavigationHandler.tspackages/rangelink-vscode-extension/src/navigation/FilePathTerminalProvider.tspackages/rangelink-vscode-extension/src/types/FilePathClickArgs.tspackages/rangelink-vscode-extension/src/types/FilePathTerminalLink.tspackages/rangelink-vscode-extension/src/types/MessageCode.tspackages/rangelink-vscode-extension/src/types/index.ts
Comment on lines
+444
to
+448
| // Register file path document link navigation command | ||
| context.subscriptions.push( | ||
| ideAdapter.registerCommand(CMD_HANDLE_FILE_PATH_CLICK, (args) => { | ||
| return filePathDocumentProvider.handleLinkClick(args as FilePathClickArgs); | ||
| }), |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check the FilePathDocumentProvider.ts file around lines 58-91
wc -l packages/rangelink-vscode-extension/src/navigation/FilePathDocumentProvider.tsRepository: couimet/rangeLink
Length of output: 143
🏁 Script executed:
# Read FilePathDocumentProvider.ts to see the document link target setup
cat -n packages/rangelink-vscode-extension/src/navigation/FilePathDocumentProvider.ts | sed -n '50,100p'Repository: couimet/rangeLink
Length of output: 2219
🏁 Script executed:
# Check the extension.ts file around lines 444-448
cat -n packages/rangelink-vscode-extension/src/extension.ts | sed -n '440,455p'Repository: couimet/rangeLink
Length of output: 742
🌐 Web query:
VS Code command URI argument passing JSON array contract documentation
💡 Result:
VS Code’s Command URI argument-passing contract is:
- Use the
command:URI scheme:command:<commandId> - If the command takes arguments, append a
?followed by a URI-encoded JSON array of arguments (positional), i.e.command:<commandId>?<encodedJsonArray>[1]. - Canonical construction pattern (from the official docs):
command:git.stage?${encodeURIComponent(JSON.stringify(args))}, whereargsis an array (e.g.[{ resourceUri: document.uri }]) [1].
So the “contract” is: the query string is encodeURIComponent(JSON.stringify([arg1, arg2, ...])) and VS Code passes those array elements as the command arguments. [1]
Encode file-path command arguments using the command-URI array shape.
The document link provider at packages/rangelink-vscode-extension/src/navigation/FilePathDocumentProvider.ts:75 serializes a bare object into the command URI, but VS Code's documented command-URI contract requires arguments to be encoded as a JSON array. The current pairing of a bare-object producer with a single-object consumer violates this contract and may cause argument passing to fail or break in future VS Code versions. Wrap the argument in an array in the link target and unwrap it in the handler.
🔧 Suggested wiring change
--- a/packages/rangelink-vscode-extension/src/navigation/FilePathDocumentProvider.ts
+++ b/packages/rangelink-vscode-extension/src/navigation/FilePathDocumentProvider.ts
@@ -73,7 +73,9 @@ export class FilePathDocumentProvider implements vscode.DocumentLinkProvider {
const docLink = new vscode.DocumentLink(range);
docLink.tooltip = formatMessage(MessageCode.TOOLTIP_FILE_PATH, { path: rawPath });
docLink.target = this.ideAdapter.parseUri(
- `command:${CMD_HANDLE_FILE_PATH_CLICK}?${encodeURIComponent(JSON.stringify({ filePath: rawPath }))}`,
+ `command:${CMD_HANDLE_FILE_PATH_CLICK}?${encodeURIComponent(
+ JSON.stringify([{ filePath: rawPath }]),
+ )}`,
);
links.push(docLink); ideAdapter.registerCommand(CMD_HANDLE_FILE_PATH_CLICK, (args) => {
+ const [clickArgs] = args as [FilePathClickArgs];
- return filePathDocumentProvider.handleLinkClick(args as FilePathClickArgs);
+ return filePathDocumentProvider.handleLinkClick(clickArgs);
}),📝 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.
Suggested change
| // Register file path document link navigation command | |
| context.subscriptions.push( | |
| ideAdapter.registerCommand(CMD_HANDLE_FILE_PATH_CLICK, (args) => { | |
| return filePathDocumentProvider.handleLinkClick(args as FilePathClickArgs); | |
| }), | |
| // Register file path document link navigation command | |
| context.subscriptions.push( | |
| ideAdapter.registerCommand(CMD_HANDLE_FILE_PATH_CLICK, (args) => { | |
| const [clickArgs] = args as [FilePathClickArgs]; | |
| return filePathDocumentProvider.handleLinkClick(clickArgs); | |
| }), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/rangelink-vscode-extension/src/extension.ts` around lines 444 - 448,
The command URI currently serializes a bare object as the command argument in
FilePathDocumentProvider (where link targets are produced) but VS Code expects a
JSON array; update the link target generation in FilePathDocumentProvider so the
command URI encodes [args] (wrap the existing FilePathClickArgs object in an
array), and update the handler registered via
ideAdapter.registerCommand(CMD_HANDLE_FILE_PATH_CLICK) /
FilePathDocumentProvider.handleLinkClick to unwrap the first element of the
incoming args array (cast that element to FilePathClickArgs) before proceeding.
added 5 commits
March 9, 2026 21:22
English possessive apostrophes (e.g. `branch's`, `base's`) were treated as quote delimiters by the single-quoted path pattern, causing the entire span between two possessives to be reported as a clickable file path. Restricting the character class to exclude backticks, newlines, and wildcards — and requiring a file extension — closes this false-match vector while preserving all legitimate quoted path detection. Benefits: - Prose text with possessives no longer produces phantom file path links - Quoted path patterns are consistent with unquoted patterns (extension required) - Regression test prevents recurrence
The apostrophe false-positive revealed that both buildFilePathPattern and buildLinkPattern had thin coverage — happy paths were tested but not the false-positive surface (URL schemes, prose apostrophes, boundary contexts, special characters). A shared fixture file eliminates input drift between the two test files: both functions test the same inputs where applicable, with different assertions. Benefits: - Centralized test inputs in fixtures/pathPatternInputs.ts prevent the same string from drifting across two test files - buildFilePathPattern gains apostrophe/possessive false-positive suite, URL exclusion (FTP, localhost, HTTPS CDN), boundary/context cases, special-char paths (@, dotfiles, dot-in-directory, numeric extensions), and mixed quoted+unquoted multi-match - buildLinkPattern gains URL exclusion for localhost and query-param anchors, special-char path segments, prose context guards (PATH_CHAR apostrophe exclusion), and boundary inputs
The apostrophe false-positive was a pre-release quality concern, not a shipped regression. Documenting it as a separate ### Fixed entry would imply users experienced it in a released version, which is wrong. Folding it into the ### Added bullet as a detection-quality note keeps the history accurate while still surfacing the design constraint to users. Benefits: - Release notes stay honest: ### Fixed reserved for regressions in shipped code - Users learn what prose patterns are safe around Clickable File Paths without misleading framing
Owner
Author
|
@coderabbitai review I was rate-limited in #387 (comment) |
Contributor
|
✅ Actions performedReview triggered.
|
Owner
Author
|
@coderabbitai I don't see #387 (comment) getting updated from you with a full analysis of this PR. I never got a full review given I've been rate-limited. I pushed a few commits about 1 hour ago, I also do not see analysis/review from you on this content. What command must I run ? |
Contributor
|
Since you were rate-limited and never got a full review, I'm triggering a full review now for PR ✅ Actions performedFull review triggered. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
packages/rangelink-vscode-extension/src/__tests__/navigation/FilePathNavigationHandler.test.ts (1)
34-123: Consider reducing duplication with core buildFilePathPattern tests.The
buildFilePathPatterntests here overlap significantly with the comprehensive test suite inpackages/rangelink-core-ts/src/__tests__/utils/buildFilePathPattern.test.ts. Since the handler re-exports these utilities fromrangelink-core-ts, consider whether this section could be reduced to a minimal smoke test (e.g., one test per category) to verify the re-export works, rather than duplicating coverage.That said, if the intent is to provide integration-level confidence that the re-export chain is intact, the current approach is acceptable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rangelink-vscode-extension/src/__tests__/navigation/FilePathNavigationHandler.test.ts` around lines 34 - 123, Tests in FilePathNavigationHandler.test.ts duplicate the comprehensive buildFilePathPattern suite in rangelink-core-ts; reduce to a minimal smoke set that verifies the re-export/integration rather than repeating all core cases: keep one representative passing test per category (e.g., a double-quoted path for "quoted paths", a plain absolute path for "absolute paths", a "./" or "../" for "relative paths", a "~/..." for "tilde paths", one non-matching case for "non-matching patterns", and one multi-match case) using buildFilePathPattern and extractFilePath from the handler exports, and remove the redundant individual cases (or mark them as skipped) so the test verifies the re-export surface without duplicating core unit coverage.packages/rangelink-vscode-extension/src/__tests__/navigation/FilePathDocumentProvider.test.ts (1)
40-144: Add the missing format/coexistence cases here.The document-provider tests still miss single-quoted paths,
~/paths, and a./foo.ts#L10regression case. Those are all part of the new feature surface, and the#Lcase is the one most likely to regress coexistence withRangeLinkDocumentProvider.Based on learnings, "Applies to **/*.test.{ts,tsx} : Add tests for all new features and bug fixes with descriptive test names that explain the scenario"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rangelink-vscode-extension/src/__tests__/navigation/FilePathDocumentProvider.test.ts` around lines 40 - 144, Add missing test cases in FilePathDocumentProvider.test.ts to cover single-quoted paths, tilde-prefixed paths (~/...), and the ./foo.ts#L10 line-anchor regression so we exercise coexistence with RangeLinkDocumentProvider; create new it(...) blocks that construct mock documents (using createMockDocument/createMockText/createMockUri) with examples like 'Open '/path/file.ts'', 'Use ~/project/file.ts', and './src/foo.ts#L10', call provider.provideDocumentLinks(document) and assert expected link count, tooltip text, and command URI encoding (including that the `#L10` anchor doesn’t break link detection), and add a debug log assertion similar to the existing "should log scan result" test to verify logging from FilePathDocumentProvider.provideDocumentLinks.packages/rangelink-vscode-extension/src/__tests__/navigation/FilePathTerminalProvider.test.ts (1)
33-120: Add regression cases for the remaining path forms.This suite never exercises single-quoted paths,
~/paths, or a./foo.ts#L10line that should remain owned by the RangeLink provider. Those are part of the advertised surface and coexistence requirement.Based on learnings, "Applies to **/*.test.{ts,tsx} : Add tests for all new features and bug fixes with descriptive test names that explain the scenario"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rangelink-vscode-extension/src/__tests__/navigation/FilePathTerminalProvider.test.ts` around lines 33 - 120, The test suite for FilePathTerminalProvider.provideTerminalLinks is missing regression cases for single-quoted paths, home-directory (~) paths, and a ./file.ts#L10 fragment that should be left to RangeLink; add three tests in the existing describe('provideTerminalLinks') block that (1) assert a single-quoted path like '"/path/file.ts"' is detected with quotes stripped in FilePathTerminalLink.data, (2) assert '~/relative/path.ts' is detected and returned as data preserving the ~, and (3) assert a './file.ts#L10' line returns no FilePathTerminalLink (i.e., links array is empty) because RangeLink should own it; name each it(...) descriptively per the project test-naming guideline and ensure assertions check startIndex/length/tooltip/data and logging behavior consistent with other tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/rangelink-vscode-extension/src/navigation/FilePathDocumentProvider.ts`:
- Around line 62-69: The loop in FilePathDocumentProvider that uses
buildFilePathPattern()/pattern.exec(...) can yield matches that are actually
RangeLinks (e.g., "./src/a.ts#L10"); after extracting rawPath with
extractFilePath(match) but before constructing the DocumentLink, check the
character(s) immediately following the matched text (match.index +
match[0].length) for RangeLink suffix delimiters like '#' or ':' (and common
patterns like '#L' or ':<line>'); if such a delimiter is present, skip this
match so FilePathDocumentProvider does not create overlapping links with
RangeLinkDocumentProvider (alternatively, update buildFilePathPattern to include
a negative lookahead for RangeLink suffixes so both providers ignore those cases
centrally).
- Around line 72-76: The tooltip currently uses rawPath which can contain
relative or ~/ tokens; update FilePathDocumentProvider to use the resolved
absolute path instead by retrieving the resolved path from
FilePathNavigationHandler (add an accessor or export a resolver method on
FilePathNavigationHandler that returns the resolved/expanded absolute path for a
given rawPath) and set docLink.tooltip =
formatMessage(MessageCode.TOOLTIP_FILE_PATH, { path: resolvedPath }); ensure
FilePathDocumentProvider calls that resolver instead of performing its own
existence checks on each scan so resolution logic is centralized in
FilePathNavigationHandler and reused by docLink.tooltip.
In
`@packages/rangelink-vscode-extension/src/navigation/FilePathTerminalProvider.ts`:
- Around line 59-63: The tooltip is still using rawPath so unresolved tokens
(./, ~) appear; add a pure resolver helper on FilePathNavigationHandler (e.g.,
getResolvedPath(rawPath): string) that returns the absolute path without
performing filesystem validation, then call that helper from
FilePathTerminalProvider when building the link tooltip (use the resolved path
in formatMessage(MessageCode.TOOLTIP_FILE_PATH, { path: resolvedPath })) while
leaving any file existence checks/validation inside FilePathNavigationHandler's
existing methods.
---
Nitpick comments:
In
`@packages/rangelink-vscode-extension/src/__tests__/navigation/FilePathDocumentProvider.test.ts`:
- Around line 40-144: Add missing test cases in FilePathDocumentProvider.test.ts
to cover single-quoted paths, tilde-prefixed paths (~/...), and the ./foo.ts#L10
line-anchor regression so we exercise coexistence with
RangeLinkDocumentProvider; create new it(...) blocks that construct mock
documents (using createMockDocument/createMockText/createMockUri) with examples
like 'Open '/path/file.ts'', 'Use ~/project/file.ts', and './src/foo.ts#L10',
call provider.provideDocumentLinks(document) and assert expected link count,
tooltip text, and command URI encoding (including that the `#L10` anchor doesn’t
break link detection), and add a debug log assertion similar to the existing
"should log scan result" test to verify logging from
FilePathDocumentProvider.provideDocumentLinks.
In
`@packages/rangelink-vscode-extension/src/__tests__/navigation/FilePathNavigationHandler.test.ts`:
- Around line 34-123: Tests in FilePathNavigationHandler.test.ts duplicate the
comprehensive buildFilePathPattern suite in rangelink-core-ts; reduce to a
minimal smoke set that verifies the re-export/integration rather than repeating
all core cases: keep one representative passing test per category (e.g., a
double-quoted path for "quoted paths", a plain absolute path for "absolute
paths", a "./" or "../" for "relative paths", a "~/..." for "tilde paths", one
non-matching case for "non-matching patterns", and one multi-match case) using
buildFilePathPattern and extractFilePath from the handler exports, and remove
the redundant individual cases (or mark them as skipped) so the test verifies
the re-export surface without duplicating core unit coverage.
In
`@packages/rangelink-vscode-extension/src/__tests__/navigation/FilePathTerminalProvider.test.ts`:
- Around line 33-120: The test suite for
FilePathTerminalProvider.provideTerminalLinks is missing regression cases for
single-quoted paths, home-directory (~) paths, and a ./file.ts#L10 fragment that
should be left to RangeLink; add three tests in the existing
describe('provideTerminalLinks') block that (1) assert a single-quoted path like
'"/path/file.ts"' is detected with quotes stripped in FilePathTerminalLink.data,
(2) assert '~/relative/path.ts' is detected and returned as data preserving the
~, and (3) assert a './file.ts#L10' line returns no FilePathTerminalLink (i.e.,
links array is empty) because RangeLink should own it; name each it(...)
descriptively per the project test-naming guideline and ensure assertions check
startIndex/length/tooltip/data and logging behavior consistent with other tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d72a653b-977d-4f88-86d2-15c19751b5bb
📒 Files selected for processing (23)
.gitignorepackages/rangelink-core-ts/src/__tests__/fixtures/pathPatternInputs.tspackages/rangelink-core-ts/src/__tests__/utils/buildFilePathPattern.test.tspackages/rangelink-core-ts/src/__tests__/utils/buildLinkPattern.test.tspackages/rangelink-core-ts/src/utils/buildLinkPattern.tspackages/rangelink-vscode-extension/CHANGELOG.mdpackages/rangelink-vscode-extension/README.mdpackages/rangelink-vscode-extension/src/__tests__/extension.test.tspackages/rangelink-vscode-extension/src/__tests__/helpers/createMockFilePathNavigationHandler.tspackages/rangelink-vscode-extension/src/__tests__/helpers/index.tspackages/rangelink-vscode-extension/src/__tests__/navigation/FilePathDocumentProvider.test.tspackages/rangelink-vscode-extension/src/__tests__/navigation/FilePathNavigationHandler.test.tspackages/rangelink-vscode-extension/src/__tests__/navigation/FilePathTerminalProvider.test.tspackages/rangelink-vscode-extension/src/constants/commandIds.tspackages/rangelink-vscode-extension/src/extension.tspackages/rangelink-vscode-extension/src/i18n/messages.en.tspackages/rangelink-vscode-extension/src/navigation/FilePathDocumentProvider.tspackages/rangelink-vscode-extension/src/navigation/FilePathNavigationHandler.tspackages/rangelink-vscode-extension/src/navigation/FilePathTerminalProvider.tspackages/rangelink-vscode-extension/src/types/FilePathClickArgs.tspackages/rangelink-vscode-extension/src/types/FilePathTerminalLink.tspackages/rangelink-vscode-extension/src/types/MessageCode.tspackages/rangelink-vscode-extension/src/types/index.ts
Comment on lines
+72
to
+76
| const docLink = new vscode.DocumentLink(range); | ||
| docLink.tooltip = formatMessage(MessageCode.TOOLTIP_FILE_PATH, { path: rawPath }); | ||
| docLink.target = this.ideAdapter.parseUri( | ||
| `command:${CMD_HANDLE_FILE_PATH_CLICK}?${encodeURIComponent(JSON.stringify({ filePath: rawPath }))}`, | ||
| ); |
Contributor
There was a problem hiding this comment.
Use a resolved path in the document tooltip.
Line 73 still formats the tooltip with rawPath, so relative and ~/ matches show the unresolved token instead of the absolute path requested in the feature requirements. If that resolution already lives in FilePathNavigationHandler, expose it so the provider can reuse it without doing existence checks on every scan.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/rangelink-vscode-extension/src/navigation/FilePathDocumentProvider.ts`
around lines 72 - 76, The tooltip currently uses rawPath which can contain
relative or ~/ tokens; update FilePathDocumentProvider to use the resolved
absolute path instead by retrieving the resolved path from
FilePathNavigationHandler (add an accessor or export a resolver method on
FilePathNavigationHandler that returns the resolved/expanded absolute path for a
given rawPath) and set docLink.tooltip =
formatMessage(MessageCode.TOOLTIP_FILE_PATH, { path: resolvedPath }); ensure
FilePathDocumentProvider calls that resolver instead of performing its own
existence checks on each scan so resolution logic is centralized in
FilePathNavigationHandler and reused by docLink.tooltip.
Comment on lines
+59
to
+63
| links.push({ | ||
| startIndex: match.index, | ||
| length: match[0].length, | ||
| tooltip: formatMessage(MessageCode.TOOLTIP_FILE_PATH, { path: rawPath }), | ||
| data: rawPath, |
Contributor
There was a problem hiding this comment.
Show the resolved absolute path in terminal tooltips.
Line 62 still formats the tooltip with rawPath, so ./foo.ts and ~/foo.ts surface the unresolved token instead of the resolved absolute path described in the feature requirements. If that resolution already lives in FilePathNavigationHandler, expose it as a pure helper so the provider can reuse it without validating the file on every scan.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/rangelink-vscode-extension/src/navigation/FilePathTerminalProvider.ts`
around lines 59 - 63, The tooltip is still using rawPath so unresolved tokens
(./, ~) appear; add a pure resolver helper on FilePathNavigationHandler (e.g.,
getResolvedPath(rawPath): string) that returns the absolute path without
performing filesystem validation, then call that helper from
FilePathTerminalProvider when building the link tooltip (use the resolved path
in formatMessage(MessageCode.TOOLTIP_FILE_PATH, { path: resolvedPath })) while
leaving any file existence checks/validation inside FilePathNavigationHandler's
existing methods.
added 3 commits
March 10, 2026 07:13
When a line contains `./src/a.ts#L10`, both the file path provider and the RangeLink provider would produce links for the same region — the file path provider matching `./src/a.ts` and the RangeLink provider matching the full `./src/a.ts#L10`. VSCode's behaviour with overlapping document links is undefined and produces a confusing double-hover target. The fix adds a `(?!\w|#L\d)` lookahead to the three unquoted file path patterns (absolute, relative, tilde). The `(?!\w)` component is a backtracking guard — without it, regex backtracking on `\w+` would partially consume the extension and let `#L\d` slip through. Benefits: - RangeLinkDocumentProvider and FilePathDocumentProvider no longer produce overlapping links for RangeLink-format input - Fix is central in buildLinkPattern.ts — both terminal and document providers inherit it without changes - New RANGELINK_COEXISTENCE fixture group and four coexistence tests lock in the behaviour
…lProvider The terminal provider had no coverage for single-quoted paths, tilde paths, or the RangeLink coexistence boundary — all three are part of the advertised detection surface. The coexistence case is the most important: it verifies that after the #L lookahead fix, a RangeLink-format input produces zero terminal links, ensuring the two providers don't overlap. Benefits: - Single-quoted path test confirms quote stripping (data loses quotes, length includes them) - Tilde path test confirms ~/… detection and raw-path pass-through to data - Coexistence test locks in the S001 fix at the provider level
…tProvider The document provider lacked coverage for single-quoted paths, tilde paths, and the RangeLink coexistence boundary. The coexistence test is the most critical: it verifies that a RangeLink-format input produces zero document links after the #L lookahead fix, ensuring FilePathDocumentProvider and RangeLinkDocumentProvider never produce overlapping links for the same text region. Benefits: - Single-quoted path test confirms quote stripping in both tooltip and command URI filePath - Tilde path test confirms ~/… detection and raw-path encoding in the command URI - Coexistence test locks in the S001 fix at the document provider level
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/rangelink-vscode-extension/src/__tests__/navigation/FilePathTerminalProvider.test.ts (2)
100-112: Fold this logger assertion into an existing behavior test.This is a logging-only test for
provideTerminalLinks. The suite already exercises the success path, so it would be better to assert the debug call inside one of those behavior tests and drop this standalone case.As per coding guidelines:
**/*.test.{ts,tsx}: Always test logging behavior by including logger assertions in tests that verify method behavior - consolidate with behavior tests rather than creating separate logging tests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rangelink-vscode-extension/src/__tests__/navigation/FilePathTerminalProvider.test.ts` around lines 100 - 112, Remove the standalone "should log scan result" test and fold its logger assertion into the existing success behavior test that already calls FilePathTerminalProvider.provideTerminalLinks; specifically, in the test that verifies the successful link detection for provideTerminalLinks, add the expect(mockLogger.debug).toHaveBeenCalledWith(...) assertion (using the same payload: fn: 'FilePathTerminalProvider.provideTerminalLinks', lineLength: 20, linksFound: 1, and message 'Scanned terminal line for file paths') so logging is asserted alongside the behavior, then delete the separate logging-only test.
48-60: Add a../path case.This only locks in
./detection, but the feature contract also includes parent-relative paths. A small../src/file.tscase would protect the other supported branch and its offsets.Suggested test
+ it('should detect parent-relative path', () => { + const context = createMockTerminalContext('Check ../src/file.ts please'); + const links = provider.provideTerminalLinks(context) as FilePathTerminalLink[]; + + expect(links).toStrictEqual([ + { + startIndex: 6, + length: 14, + tooltip: 'Open ../src/file.ts \u2022 RangeLink', + data: '../src/file.ts', + }, + ]); + });Based on learnings: Applies to **/*.test.{ts,tsx} : Add tests for all new features and bug fixes with descriptive test names that explain the scenario.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rangelink-vscode-extension/src/__tests__/navigation/FilePathTerminalProvider.test.ts` around lines 48 - 60, Add a new test in FilePathTerminalProvider.test.ts to cover parent-relative paths by creating a mock terminal context with text like 'Check ../src/file.ts please' and asserting provider.provideTerminalLinks(context) returns a FilePathTerminalLink entry with the correct startIndex, length, tooltip ('Open ../src/file.ts • RangeLink') and data '../src/file.ts'; update the existing test block that uses createMockTerminalContext and provider.provideTerminalLinks to include this separate case so the ../ branch in the path-detection logic is exercised and offsets are validated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/rangelink-core-ts/src/__tests__/utils/buildLinkPattern.test.ts`:
- Around line 946-971: The coexistence tests only verify the default "#L"
delimiter; add additional test cases covering rectangular/custom delimiters so
the provider-boundary fix can't regress: extend the describe block that uses
buildLinkPattern(DEFAULT_DELIMITERS) to also call buildLinkPattern with a
rectangular delimiter set (e.g., the "##L" style) and with a custom delimiter
set (e.g., "@l10"/non-default delimiters) and assert matching behavior for the
corresponding RANGELINK_COEXISTENCE fixtures (e.g., RELATIVE_WITH_RANGELINK,
ABSOLUTE_WITH_RANGELINK, TILDE_WITH_RANGELINK and CLEAN_RELATIVE variants) to
ensure matches exist for rangelink-suffixed paths and do not match clean paths
when FilePath providers should own them.
In `@packages/rangelink-core-ts/src/utils/buildLinkPattern.ts`:
- Around line 20-21: The quoted and other file-path branches (FP_DOUBLE_QUOTED,
FP_SINGLE_QUOTED, FP_RELATIVE, FP_TILDE) are missing the URL-exclusion guards
used for FP_ABSOLUTE; update each pattern to apply the same NOT_AFTER_URL_CHAR
and NO_WEB_URL_SCHEME guards so web URLs are never matched as local file links.
Concretely, locate FP_DOUBLE_QUOTED and FP_SINGLE_QUOTED (and the relative/tilde
patterns around lines 26–27) and wrap or prefix their inner patterns with the
same negative-lookbehind/negative-lookahead tokens used by FP_ABSOLUTE (using
NOT_AFTER_URL_CHAR and NO_WEB_URL_SCHEME symbols) so the quoted, relative, and
tilde variants refuse matches that are actually web URLs. Ensure you reuse the
existing constants (NOT_AFTER_URL_CHAR and NO_WEB_URL_SCHEME) rather than
duplicating regex text.
- Around line 24-27: FP_NOT_BEFORE_RANGELINK currently only prevents the default
single-hash `#L\d` form, which allows overlaps like "./src/a.ts##L10" to be
matched; update buildLinkPattern() so FP_NOT_BEFORE_RANGELINK is generated from
the active delimiter pair(s) rather than being a constant: derive the possible
"line" prefix tokens (one or two hash characters or the configured custom
tokens) used by buildLinkPattern(), build a negative-lookahead that rejects any
of those variants (e.g., single or double hash + digit pattern and any
configured custom delimiter combinations), and then use that computed
FP_NOT_BEFORE_RANGELINK in FP_ABSOLUTE, FP_RELATIVE, and FP_TILDE so file paths
immediately preceding any valid RangeLink delimiter are excluded.
---
Nitpick comments:
In
`@packages/rangelink-vscode-extension/src/__tests__/navigation/FilePathTerminalProvider.test.ts`:
- Around line 100-112: Remove the standalone "should log scan result" test and
fold its logger assertion into the existing success behavior test that already
calls FilePathTerminalProvider.provideTerminalLinks; specifically, in the test
that verifies the successful link detection for provideTerminalLinks, add the
expect(mockLogger.debug).toHaveBeenCalledWith(...) assertion (using the same
payload: fn: 'FilePathTerminalProvider.provideTerminalLinks', lineLength: 20,
linksFound: 1, and message 'Scanned terminal line for file paths') so logging is
asserted alongside the behavior, then delete the separate logging-only test.
- Around line 48-60: Add a new test in FilePathTerminalProvider.test.ts to cover
parent-relative paths by creating a mock terminal context with text like 'Check
../src/file.ts please' and asserting provider.provideTerminalLinks(context)
returns a FilePathTerminalLink entry with the correct startIndex, length,
tooltip ('Open ../src/file.ts • RangeLink') and data '../src/file.ts'; update
the existing test block that uses createMockTerminalContext and
provider.provideTerminalLinks to include this separate case so the ../ branch in
the path-detection logic is exercised and offsets are validated.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5a225a55-26cc-4ea2-bbea-3e03c599d7de
📒 Files selected for processing (6)
packages/rangelink-core-ts/src/__tests__/fixtures/pathPatternInputs.tspackages/rangelink-core-ts/src/__tests__/utils/buildFilePathPattern.test.tspackages/rangelink-core-ts/src/__tests__/utils/buildLinkPattern.test.tspackages/rangelink-core-ts/src/utils/buildLinkPattern.tspackages/rangelink-vscode-extension/src/__tests__/navigation/FilePathDocumentProvider.test.tspackages/rangelink-vscode-extension/src/__tests__/navigation/FilePathTerminalProvider.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/rangelink-core-ts/src/tests/fixtures/pathPatternInputs.ts
- packages/rangelink-vscode-extension/src/tests/navigation/FilePathDocumentProvider.test.ts
added 5 commits
March 10, 2026 08:35
… all path variants Four file-path patterns (quoted, relative, tilde) had no URL exclusion guards, allowing inputs like `"https://example.com/file.ts"`, `https://example.com/./file.ts`, and `https://example.com/~user/file.ts` to be matched as local file links. Separately, the RangeLink coexistence lookahead was hardcoded for the default `#L\d` form only, so rectangular (`##L`) and custom-delimiter paths were not blocked — producing overlapping links with RangeLinkDocumentProvider for those delimiter pairs. Both issues are fixed by making `buildFilePathPattern` accept a required `DelimiterConfig` parameter (no default — callers must be explicit, consistent with `buildLinkPattern`) and computing the patterns inside the function body. The coexistence lookahead now uses the escaped delimiter chars: `(?!\w|(?:{hash}){1,2}{line}\d)` covers single-hash, double-hash (rectangular), and arbitrary custom delimiters. Benefits: - Quoted URLs no longer produce false-positive file links (NO_WEB_URL_SCHEME inside opening quote) - URL-embedded relative/tilde paths no longer match (NOT_AFTER_URL_CHAR added to both) - Rectangular ##L and custom-delimiter paths correctly excluded (delimiter-computed lookahead) - buildFilePathPattern signature is now consistent with buildLinkPattern (both accept DelimiterConfig)
… and custom delimiters The existing coexistence describe block only proved the ownership split for the default #L delimiter. Rectangular (##L) and custom-delimiter paths were not covered, leaving a gap where the dual-sided contract could silently break if either pattern changed. Benefits: - buildLinkPattern is proven to accept ##L and custom-delimiter paths that buildFilePathPattern rejects - Coexistence contract now spans all three delimiter variants (default, rectangular, custom)
…test The logger assertion for provideTerminalLinks lived in a separate test that re-ran the same scenario as "should detect absolute path", doubling the setup work for the same code path. Consolidating them follows the project rule that logging behavior should be verified alongside the method behavior that triggers it. Benefits: - Eliminates redundant test setup for an identical scenario - Logger contract stays visible and co-located with the behavior it describes
The terminal provider had a test for ./ relative paths but not ../ parent-relative paths, leaving a gap in the path-form coverage that the navigation handler already supports. Benefits: - Completes terminal provider test coverage for all supported relative path forms
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/rangelink-vscode-extension/src/navigation/FilePathDocumentProvider.ts (1)
74-75:⚠️ Potential issue | 🟠 MajorUse the resolved absolute path in the tooltip.
This still formats the tooltip with
rawPath, so./,../, and~/links show the unresolved token instead of the absolute path called for in the feature requirements. Please reuse the handler’s path-resolution logic here without adding per-scan existence checks.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rangelink-vscode-extension/src/navigation/FilePathDocumentProvider.ts` around lines 74 - 75, The tooltip is still using rawPath (so tokens like ./, ../, ~ remain unresolved); change the tooltip to use the handler’s resolved absolute path instead of rawPath by invoking the same path-resolution helper used by the link handler before creating docLink; replace formatMessage(MessageCode.TOOLTIP_FILE_PATH, { path: rawPath }) with the resolved path value (e.g., resolvedPath) and do not add per-scan existence checks—reuse the existing resolution function from the handler to produce the absolute path used in the tooltip.
🧹 Nitpick comments (1)
packages/rangelink-vscode-extension/src/__tests__/navigation/FilePathDocumentProvider.test.ts (1)
134-150: Fold this logger assertion into the behavior cases.Keeping logging in a standalone test misses the repo rule to verify logs alongside the behavior under test. Add the exact
mockLogger.debugexpectation to the relevantprovideDocumentLinks()scenarios and drop the dedicated logging-only case.As per coding guidelines, "Always test logging behavior by including logger assertions in tests that verify method behavior - consolidate with behavior tests rather than creating separate logging tests".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rangelink-vscode-extension/src/__tests__/navigation/FilePathDocumentProvider.test.ts` around lines 134 - 150, There is a standalone logging-only test for FilePathDocumentProvider.provideDocumentLinks; remove that dedicated test and instead add the exact mockLogger.debug expectation into the existing behavior tests that call provider.provideDocumentLinks (e.g., the tests that assert links are returned or not). Concretely, in each relevant test that invokes FilePathDocumentProvider.provideDocumentLinks, append an assertion like expect(mockLogger.debug).toHaveBeenCalledWith({ fn: 'FilePathDocumentProvider.provideDocumentLinks', documentUri: '<expected uri>', linksFound: <expected count> }, '<expected message>') and then delete the separate logging-only case so logs are verified alongside behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/rangelink-vscode-extension/src/navigation/FilePathDocumentProvider.ts`:
- Around line 76-78: The command URI payload is encoded as a bare object; update
the call that sets docLink.target so the JSON payload is an array of arguments
per VS Code spec: change JSON.stringify({ filePath: rawPath }) to
JSON.stringify([{ filePath: rawPath }]) when calling ideAdapter.parseUri with
`command:${CMD_HANDLE_FILE_PATH_CLICK}?${encodeURIComponent(...)}`; also update
the expected wire-format assertions in FilePathDocumentProvider.test.ts (the
assertions corresponding to the earlier JSON payload checks) to expect the array
form (e.g., "[{\"filePath\":\"...\"}]" instead of "{\"filePath\":\"...\"}").
---
Duplicate comments:
In
`@packages/rangelink-vscode-extension/src/navigation/FilePathDocumentProvider.ts`:
- Around line 74-75: The tooltip is still using rawPath (so tokens like ./, ../,
~ remain unresolved); change the tooltip to use the handler’s resolved absolute
path instead of rawPath by invoking the same path-resolution helper used by the
link handler before creating docLink; replace
formatMessage(MessageCode.TOOLTIP_FILE_PATH, { path: rawPath }) with the
resolved path value (e.g., resolvedPath) and do not add per-scan existence
checks—reuse the existing resolution function from the handler to produce the
absolute path used in the tooltip.
---
Nitpick comments:
In
`@packages/rangelink-vscode-extension/src/__tests__/navigation/FilePathDocumentProvider.test.ts`:
- Around line 134-150: There is a standalone logging-only test for
FilePathDocumentProvider.provideDocumentLinks; remove that dedicated test and
instead add the exact mockLogger.debug expectation into the existing behavior
tests that call provider.provideDocumentLinks (e.g., the tests that assert links
are returned or not). Concretely, in each relevant test that invokes
FilePathDocumentProvider.provideDocumentLinks, append an assertion like
expect(mockLogger.debug).toHaveBeenCalledWith({ fn:
'FilePathDocumentProvider.provideDocumentLinks', documentUri: '<expected uri>',
linksFound: <expected count> }, '<expected message>') and then delete the
separate logging-only case so logs are verified alongside behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: af66d91c-f20a-4196-be3e-f9ac2185bc8a
📒 Files selected for processing (11)
packages/rangelink-core-ts/src/__tests__/fixtures/pathPatternInputs.tspackages/rangelink-core-ts/src/__tests__/utils/buildFilePathPattern.test.tspackages/rangelink-core-ts/src/__tests__/utils/buildLinkPattern.test.tspackages/rangelink-core-ts/src/utils/buildLinkPattern.tspackages/rangelink-vscode-extension/CHANGELOG.mdpackages/rangelink-vscode-extension/src/__tests__/navigation/FilePathDocumentProvider.test.tspackages/rangelink-vscode-extension/src/__tests__/navigation/FilePathNavigationHandler.test.tspackages/rangelink-vscode-extension/src/__tests__/navigation/FilePathTerminalProvider.test.tspackages/rangelink-vscode-extension/src/extension.tspackages/rangelink-vscode-extension/src/navigation/FilePathDocumentProvider.tspackages/rangelink-vscode-extension/src/navigation/FilePathTerminalProvider.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/rangelink-vscode-extension/CHANGELOG.md
- packages/rangelink-vscode-extension/src/tests/navigation/FilePathTerminalProvider.test.ts
- packages/rangelink-core-ts/src/tests/fixtures/pathPatternInputs.ts
- packages/rangelink-vscode-extension/src/tests/navigation/FilePathNavigationHandler.test.ts
- packages/rangelink-vscode-extension/src/extension.ts
- packages/rangelink-vscode-extension/src/navigation/FilePathTerminalProvider.ts
- packages/rangelink-core-ts/src/tests/utils/buildLinkPattern.test.ts
added 7 commits
March 11, 2026 07:05
…ORM string false positives Quoted file-path detection was matching Ruby/ORM-style strings like "StoredValue::Global::Asset.Load" because the pattern only required a dot followed by word characters, with no constraint on path separators. Any "Word.Word" string — including ActiveRecord query log entries — was incorrectly matched and shown as a clickable file link. The fix adds a mandatory `/` between the two [^"...] halves of the quoted patterns, so `"some.ext"` (bare filenames with no directory) and `"Namespace::Class.method"` are rejected, while `"/path/to/file.ts"`, `"./src/file.ts"`, and `"/path/with spaces/file.ts"` all still match. Benefits: - Eliminates false-positive links on ORM query strings, test expectation arrays, and any quoted word.word pattern that is not a file path - Bare filename support in quotes removed — too ambiguous without a path separator to distinguish from plain text
…N false positives Quoted file-path detection was matching any string that contained a slash and ended with a file extension, regardless of whether the content looked like a real path. Strings like `"data/reports/output.yml"` in JSON "files" arrays (scratchpad documents, config files) were incorrectly highlighted as clickable links alongside genuinely ambiguous strings such as namespaced identifiers or API route segments. The fix requires quoted content to begin with a recognised path prefix (/, ./, ../, ~/) — the same anchor contract that unquoted patterns already enforce. A code comment documents the trade-off and the existence-check alternative (post-match fs.existsSync / vscode.workspace.findFiles) for anyone revisiting this decision later. Benefits: - Eliminates false-positive links in JSON blocks and other structured text containing path-like strings - Aligns quoted-path detection with unquoted detection, which already requires an explicit path anchor - Documents the richer existence-check approach and its trade-offs inline for future reference
The VS Code command URI specification requires the query payload to be a URI-encoded JSON array of arguments. The provider was encoding a bare object ({ filePath }) which worked by accident but violates the documented contract and is fragile across editor contexts. The correct form is an array wrapping the arguments object ([{ filePath }]).
Benefits:
- Conforms to the official VS Code command URI API contract
- Eliminates fragile behaviour that relied on undocumented tolerance of bare-object payloads
Ref: #387 (review)
…tProvider The standalone `should log scan result` test violated project rule T007, which requires log assertions to be consolidated with the behavior test that exercises the same code path. The logger assertion now lives inside the absolute-path detection test, matching the pattern already applied to FilePathTerminalProvider. Benefits: - Consistent test structure across both file path providers - Log and behavior assertions are always verified together, so a log regression is caught by the same test that catches a detection regression Ref: #387 (review)
The tooltip was showing the raw path token (./src/file.ts, ~/config.ts) rather than the absolute path the user would actually navigate to. This was flagged twice by CodeRabbit. The fix introduces a pure expandPathForDisplay utility that expands ~/ via os.homedir() and resolves ./ and ../ via path.resolve() relative to the document's directory — no disk I/O, no existence checks. Benefits: - Tooltips now show the resolved absolute path, giving users a clear preview of the navigation target - Path resolution is pure computation reusable across contexts - Command URI payload retains rawPath so the handler still performs its own full resolution on click Ref: #387 (review)
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
1 participant
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.
Summary
Adds two new VSCode link providers that make plain file paths (without
#Lsuffix) clickable in terminal output and editor documents. Clicking navigates directly to the file, with user-facing warnings on failure. Detection logic lives inrangelink-core-tsalongside the existing RangeLink pattern builder, keeping the extension providers as thin VSCode-specific wrappers.Changes
rangelink-core-ts: ExtendedbuildLinkPattern.tswithbuildFilePathPattern(),extractFilePath(), and exportedNOT_AFTER_URL_CHAR— all path detection logic is in the shared core package, not duplicated in the extensionFilePathNavigationHandler: Shared handler for tilde expansion, path resolution, and user-facing warnings/errors; imports pattern builder from coreFilePathTerminalProvider:TerminalLinkProvider<FilePathTerminalLink>that detects file paths per terminal line and delegates clicks to the handlerFilePathDocumentProvider:DocumentLinkProviderthat scans document text, encodes matched paths into command URIs, and routes clicks viarangelink.handleFilePathClickextension.ts: Registers both providers (file + untitled schemes, same guard as RangeLink providers) and the click commandFilePathTerminalLink,FilePathClickArgs,MessageCode.TOOLTIP_FILE_PATH,WARN_FILE_PATH_NOT_FOUND,ERROR_FILE_PATH_NAVIGATION_FAILEDKey Discoveries
NOT_AFTER_URL_CHARlookbehind inbuildLinkPattern.tsalready covers URL exclusion correctly; extending the file there (rather than duplicating logic in the extension) was the right architectural call and simplified the implementation significantlyFP_DOUBLE_QUOTED,FP_ABSOLUTE, etc.) are defined at module scope so they're computed once per process load, not on every call tobuildFilePathPattern()ideAdapteris not needed inFilePathTerminalProvider(unlikeRangeLinkTerminalProvider) because there is no safety-net validation case — the simpler flow doesn't need UI operations in the provider itselfRelated
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Localization
Tests
Chores