Skip to content

fix(lsp): codeActions not found if multiple clients are active #749

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 4, 2025

Conversation

carloskiki
Copy link
Contributor

@carloskiki carloskiki commented May 4, 2025

This fixes #744, which was thought to be caused by "copilot.vim", but actually happens anytime rust-analyzer does not have id: 1. We thought that this case was already handled (see: #744 (comment)), but it turns out that it wasn't.

According to the lua manual:

ipairs (t)

Returns three values (an iterator function, the table t, and 0) so that the construction

 for i,v in ipairs(t) do body end

will iterate over the key–value pairs (1,t[1]), (2,t[2]), ..., up to the first absent index. (emphasis mine).

What happens is that the result table returned by

vim.lsp.buf_request_all(0, 'textDocument/codeAction', params, function(results, ctx) 
-- handle `result` table.
end)

is sparse with respect to the LSPs that return a codeAction table. So consider rust-analyzer has id: 2, and the LSP with id: 1 does not return a codeAction table (presumably because the lsp is not made for rust files). Then ipairs will stop at index 1 and return nil, since it stops at the first absent index.

This PR fixes the bug by indexing only the codeActions provided by rust-analyzer.

@mrcjkb, when you tested out with null-ls, I believe you simply got lucky and rust-analyzer happened to be the lsp with id: 1.

Copy link
Contributor

github-actions bot commented May 4, 2025

Review Checklist

Does this PR follow the Contribution Guidelines? Following is a partial checklist:

Proper conventional commit scoping:

  • For example, fix(lsp): some lsp-related bugfix

  • Pull request title has the appropriate conventional commit prefix.

If applicable:

  • Tested
    • Tests have been added.
    • Tested manually (Steps to reproduce in PR description).
  • Updated documentation.

Copy link
Owner

@mrcjkb mrcjkb left a comment

Choose a reason for hiding this comment

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

Thanks for troubleshooting.

issue: We want to display all code actions, not just the rust-analyzer ones.
I believe a simpler fix would be to leave the code as is, but to use pairs instead of ipairs.

todo: Please change the commit message and PR title to comply with the conventional commits specification (see the bot comment).

@carloskiki carloskiki changed the title Fix codeActions when rust-analyzer does not have lsp id 1. fix: codeActions when rust-analyzer does not have lsp id 1. May 4, 2025
@mrcjkb mrcjkb changed the title fix: codeActions when rust-analyzer does not have lsp id 1. fix(lsp): codeActions not found if multiple clients are active May 4, 2025
Copy link
Owner

@mrcjkb mrcjkb left a comment

Choose a reason for hiding this comment

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

Thanks 🙏

@mrcjkb mrcjkb enabled auto-merge (squash) May 4, 2025 17:58
@mrcjkb mrcjkb merged commit c0183df into mrcjkb:master May 4, 2025
5 checks passed
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.

codeActions not found when multiple clients are attached
2 participants