Adds document_symbol support for pyrefly#78
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the LSP client initialization handshake to advertise textDocument/documentSymbol support, improving compatibility with servers like pyrefly that may gate symbol responses on declared client capabilities.
Changes:
- Add
text_document.document_symbolclient capabilities during LSPinitialize. - Enable hierarchical document symbol support (
hierarchical_document_symbol_support: true).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| text_document: Some(lsp_types::TextDocumentClientCapabilities { | ||
| document_symbol: Some(lsp_types::DocumentSymbolClientCapabilities { | ||
| dynamic_registration: Some(false), | ||
| hierarchical_document_symbol_support: Some(true), | ||
| ..Default::default() | ||
| }), |
There was a problem hiding this comment.
This change adds document_symbol to the client capabilities, but there’s no test asserting that the initialize request includes this capability. Since this module already has unit tests, consider adding a small unit test (or factoring out capability construction) to prevent regressions where document_symbol support is accidentally dropped again.
There was a problem hiding this comment.
Thanks for the fix, @jackd ! Pyrefly compatibility is a good addition. Two small things before merge: a CHANGELOG.md entry under [Unreleased] (project convention), and a unit test verifying hierarchical_document_symbol_support is set in InitializeParams — keeps this from silently regressing.
| }), | ||
| text_document: Some(lsp_types::TextDocumentClientCapabilities { | ||
| document_symbol: Some(lsp_types::DocumentSymbolClientCapabilities { | ||
| dynamic_registration: Some(false), |
There was a problem hiding this comment.
Minor consistency note: other capability blocks in this file declare symbol_kind.value_set explicitly. Not blocking, but worth aligning when convenient.
Adds
document_symbolsupport for pyreflyI originally wrote this as an issue, but since I had the fix I thought I might as well offer it. I want to be completely up front and admit I'm not a rust person, this was agent-authored, and I'm not going to pretend to understand it. I hope that doesn't offend anybody, and I similarly won't be offended if this PR is promptly rejected on that basis.
I could have added tests, but given I wouldn't understand them either I don't see the value of including that to that project.
What I can say is the document_symbol didn't work with
pyreflybefore the change, and it did afterwards.Type of Change
Checklist
Additional Notes
Config that works with it: