Skip to content

Add hierarchy lsp picker function#329

Merged
ray-x merged 2 commits intoray-x:masterfrom
ddaniel27:update-hierarchy-calls
Jul 28, 2025
Merged

Add hierarchy lsp picker function#329
ray-x merged 2 commits intoray-x:masterfrom
ddaniel27:update-hierarchy-calls

Conversation

@ddaniel27
Copy link
Copy Markdown
Contributor

@ddaniel27 ddaniel27 commented Jul 26, 2025

Hey again! Recently I updated the plugin version from a loooong time without updating it (since now I have some time to deal with this kind of errors lol) and found that when you have multiple LSP's running in a buffer (like a code agent running aside the main LSP as in the screenshot) you will have an error calling the hierarchy lsp function since the handler chooses the first option from the returned clients.

image

This PR's adds a new function called pick_hierarchy_lsp_client just to pick the first LSP that has the server capabilities to use the callHierarchy related functions and has a relation with the current buffer. Let me know if more information is needed or any change required!

Thanks again for this amazing plugin! :)

EDIT:
Why I updated the plugin and what I want actually work is the definition_preview handler, I added that too in this PR, basically the actual response structure for result is a structure like this
local result = client:request_sync(method, params, timeout_ms or 1000, bufnr)

{                                                                                                                                                                                                                   
  result = { {                                                                                                                                                                                                      
      range = {                                                                                                                                                                                                     
        ["end"] = {                                                                                                                                                                                                 
          character = 23,                                                                                                                                                                                           
          line = 12                                                                                                                                                                                                 
        },                                                                                                                                                                                                          
        start = {                                                                                                                                                                                                   
          character = 5,                                                                                                                                                                                            
          line = 12                                                                                                                                                                                                 
        }                                                                                                                                                                                                           
      },                                                                                                                                                                                                            
      uri = "file:///home/ddaniel27/folder/go-project/internal/telemetry/attributes.go"                                                                                                                         
    } }                                                                                                                                                                                                             
}  

So, I updated the for loop where data is being populated, let me know your thoughts!

Copy link
Copy Markdown
Owner

@ray-x ray-x left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi @ddaniel27
Thanks for the PR
Just a question regarding root check. Otherwise, the changes look good to me.

if value ~= nil and value.result ~= nil and not vim.tbl_isempty(value.result) then
table.insert(data, value.result[1])
if value ~= nil and not vim.tbl_isempty(value) then
table.insert(data, value[1])
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I did not notice it was broken.

for _, client in ipairs(vim.lsp.get_clients({ bufnr = bufnr, method = method })) do
if client.server_capabilities.callHierarchyProvider then
local root = client.config and client.config.root_dir
if not root or file:find(root, 1, true) == 1 then
Copy link
Copy Markdown
Owner

@ray-x ray-x Jul 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be?

if root and file:find(root, 1, true) == 1

Is there an example of lsp returns nil value for root_dir?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My thoughts here were to not enforce root to cover edge cases where a bad configuration or architectural decisions of some LSP's could lead to a nil root_dir since the protocol allows it. Personally, I've no encounter these cases since all the LSP's that I use always have a defined root_dir but I'll let you decide here, if you think it's better to enforce root I can change it, let me know.

Copy link
Copy Markdown
Owner

@ray-x ray-x left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@ray-x ray-x merged commit 17ed4fa into ray-x:master Jul 28, 2025
3 checks passed
@ddaniel27 ddaniel27 deleted the update-hierarchy-calls branch July 28, 2025 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants