Handle multiple matches for package: URIs in TerminalLinkProvider and DocumentLinks better #5695
Conversation
|
/gemini review |
There was a problem hiding this comment.
Pull Request Overview
This PR improves handling of package: URIs in the Dart extension's terminal link provider and document links by addressing scenarios where multiple packages might match the same URI. The changes enhance the user experience by providing options when multiple matches exist and prioritizing project-specific package maps for document links.
- Enhanced terminal link handling to support multiple package URI matches with peek functionality
- Improved document link resolution to prioritize current project's package map
- Added TODO comments for future test coverage improvements
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/extension/terminal/package_uri_link_provider.ts | Enhanced package URI resolution to handle multiple matches and improved document link provider logic |
| src/test/dart/providers/terminal_link_providers.test.ts | Added TODO comments outlining needed test coverage for multiple package URI scenarios |
There was a problem hiding this comment.
Code Review
This pull request improves the handling of package: URIs by resolving them to all possible file locations when multiple projects in the workspace have the same dependency. When a terminal link has multiple targets, it now shows a peek definition view to allow the user to choose. For document links, it prioritizes the package map of the current project. The changes are well-implemented. I've left a couple of minor suggestions to remove some debug code and improve code clarity.
| const link = locations[0]; | ||
| void vs.commands.executeCommand("_dart.jumpToLineColInUri", link.uri, link.range.start.line, link.range.start.character); |
There was a problem hiding this comment.
The constant link here shadows the function parameter with the same name. To avoid confusion and improve readability, consider renaming it to something like location.
const location = locations[0];
void vs.commands.executeCommand("_dart.jumpToLineColInUri", location.uri, location.range.start.line, location.range.start.character);There was a problem hiding this comment.
Codex Review: Here are some suggestions.
Reply with @codex fix comments to fix any unresolved comments.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
Fixes #5692
Fixes #5694