Conversation
- Add lsp_definition method to NeovimClientTrait interface - Implement Lua script for textDocument/definition LSP requests - Add lsp_definition MCP tool with universal document identifier support - Include comprehensive integration tests for definition lookup - Rename HoverParams to TextDocumentPositionParams for broader LSP usage
- Change return type from Vec<Location> to DefinitionResult enum - Add LocationLink struct for enhanced definition information - Support single location, location list, and location link formats - Update integration tests to handle all definition result types
Contributor
There was a problem hiding this comment.
Pull Request Overview
Adds LSP definition support to the MCP server enabling clients to retrieve symbol definitions through the Neovim LSP integration. This enhancement allows users to programmatically navigate to symbol definitions using the Language Server Protocol.
- Implements new
lsp_definitiontool in the MCP server with position-based symbol lookup - Adds comprehensive definition result handling supporting Location, Locations array, and LocationLinks formats
- Includes integration tests validating definition lookup functionality with Go code examples
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/server/tools.rs | Adds DefinitionParams struct and lsp_definition tool method for handling definition requests |
| src/neovim/lua/lsp_definition.lua | Implements Lua script for executing LSP textDocument/definition requests through Neovim |
| src/neovim/integration_tests.rs | Adds comprehensive test case validating definition lookup with Go language server |
| src/neovim/client.rs | Extends client interface with definition support and renames HoverParams to TextDocumentPositionParams for reusability |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
- Update tool count from 13 to 14 tools in documentation - Add lsp_definition to universal LSP tools list - Fix LSP definition result handling to return Option<DefinitionResult> - Remove unused buffer_id parameter from Lua definition script - Update integration test assertions for optional definition results - Fix markdown line length issues for linter compliance
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
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.
Add LSP definition support to MCP server with enhanced result handling for multiple response formats.