Implement hover menu support for ruff-server; Issue #10595 #11096
Conversation
CodSpeed Performance ReportMerging #11096 will not alter performanceComparing Summary
|
|
There was a problem hiding this comment.
Thank you for getting this done! You have the honor of being the first outside contributor for ruff server 😄
There are a few things I'd like to improve before we merge this:
- Error resiliency - there's a lot of
.unwrap()s in here that could potentially panic in a real-world scenario. When I was testing this PR, I noticed that hovering over an incompletenoqacomment would cause the server to panic (for example:# noqa: RUF). I've made some suggestions that use early returns in the place of potentially panicking functions like.unwrap(). - Code complexity - sections of this implementation are actually more complicated than they need to be. I've left suggestions on how we can simplify them.
Once again, I appreciate you taking the time to implement this! This is going to be a nifty feature.
There was a problem hiding this comment.
Excellent! This seems to be working great on VS Code. Good catch with the comparison operators! I have one small nit, but otherwise you are free to merge 👍
|
My bad, I actually have to run the merge. Thanks for bearing with me 🤦♀️ |
Summary
Add support for hover menu to ruff_server, as requested in 10595.
Majority of new code is in hover.rs.
I reused the regex from ruff-lsp's implementation. Also reused the format_rule_text function from ruff/src/commands/rule.rs
Added capability registration in server.rs, and added the handler to api.rs.
Test Plan
Tested in NVIM v0.10.0-dev-2582+g2a8cef6bd, configured with lspconfig using the default options (other than cmd pointing to my test build, with options "server" and "--preview"). OS: Ubuntu 24.04, kernel 6.8.0-22.