Fix document symbol search for editors without hierarchical support#3555
Fix document symbol search for editors without hierarchical support#3555mmatz-101 wants to merge 1 commit into
Conversation
Fixes facebook#2945 Editors like Helix do not declare `hierarchicalDocumentSymbolSupport` in their client capabilities. Previously pyrefly treated this as a signal to return nothing, so document symbol search was completely broken in Helix. Check the client capability at request time and return a flat `SymbolInformation` list when hierarchical support is absent, falling back to the existing nested `DocumentSymbol` response for clients like VS Code that support it. The flat list is produced by recursively unwrapping the symbol tree, preserving dotted container names (e.g. `Outer.Inner`) so clients can still display symbol hierarchy contextually.
|
Good direction overall this correctly restores symbol visibility for non-hierarchical clients like Helix. I think the key thing to validate is that flattening is purely a presentation-layer transformation and does not leak back into any semantic symbol resolution logic. If that separation holds, this is a clean compatibility fix. Might be worth adding a regression test for mixed-capability clients (hierarchical true vs false) to ensure identical symbol sets, just different shapes. |
|
Is this flattened representation part of the LSP & supported in other editors that don't support hierarchical document symbols, or is this something specific to Helix? I'm not sure what impact (if any) there is on other editors I'll let @kinto0 make the call here |
kinto0
left a comment
kinto0
left a comment
There was a problem hiding this comment.
looks great! thanks for working on it. I'd like a few more tests like lsp_interaction tests to make sure the behavior works end-to-end. and is there a way to make every document_symbols test test both flat + nested symbols? then we won't need to add any new tests
| return Err(EmptyResponseReason::LanguageServicesDisabled); | ||
| } | ||
| let Some(true) = self | ||
| let handle = self.make_handle_if_enabled(uri, Some(DocumentSymbolRequest::METHOD))?; |
There was a problem hiding this comment.
nit: we should avoid making the handle if they don't have any support for any document symbols
| result: &mut Vec<lsp_types::SymbolInformation>, | ||
| ) { | ||
| for sym in symbols { | ||
| let children = sym.children.unwrap_or_default(); |
There was a problem hiding this comment.
can you double check there's no better way to flatten DocumentSymbols through the lsp_types library?
4 participants
Summary
Editors like Helix do not declare
hierarchicalDocumentSymbolSupportin their client capabilities. Previously pyrefly treated this as a
signal to return nothing, so document symbol search was completely
broken in Helix.
Check the client capability at request time and return a flat
SymbolInformationlist when hierarchical support is absent, fallingback to the existing nested
DocumentSymbolresponse for clients likeVS Code that support it. The flat list is produced by recursively
unwrapping the symbol tree, preserving dotted container names
(e.g.
Outer.Inner) so clients can still display symbol hierarchycontextually.
Fixes #2945
Test Plan
Added unit tests for
flatten_to_symbol_informationcovering:Run with: cargo test test_flatten