Fix document symbols order #5687
Conversation
|
@magurotuna did you file a bug with sublime's LSP? |
| @@ -273,19 +273,24 @@ pub(crate) fn handle_document_symbol( | |||
| parents.push((doc_symbol, symbol.parent)); | |||
| } | |||
| let mut document_symbols = Vec::new(); | |||
| // Constructs `document_symbols` from `parents`, in order from the end. | |||
| while let Some((node, parent)) = parents.pop() { | |||
| match parent { | |||
| None => document_symbols.push(node), | |||
There was a problem hiding this comment.
| None => document_symbols.push(node), | |
| None => { | |
| if let Some(children) = &mut node.children { | |
| children.reverse() | |
| } | |
| document_symbols.push(node) | |
| }, |
I think should do the trick without recursion.
There was a problem hiding this comment.
Thanks for the suggestion.
I tried your code, but it cannot deal with nested symbols.
For example,
mod a {
mod b {
struct B1;
struct B2;
}
}We want this snippet to be output like:
[
{
"children": [
{
"children": [
{
"kind": 23,
"name": "B1"
},
{
"kind": 23,
"name": "B2"
}
],
"kind": 2,
"name": "b"
}
],
"kind": 2,
"name": "a"
}
]But actually we get:
[
{
"children": [
{
"children": [
{
"kind": 23,
"name": "B2"
},
{
"kind": 23,
"name": "B1"
}
],
"kind": 2,
"name": "b"
}
],
"kind": 2,
"name": "a"
}
]Recursion way works as we expect.
| @@ -682,3 +685,313 @@ pub fn foo(_input: TokenStream) -> TokenStream { | |||
| let value = res.get("contents").unwrap().get("value").unwrap().to_string(); | |||
| assert_eq!(value, r#""```rust\nfoo::Bar\n```\n\n```rust\nfn bar()\n```""#) | |||
| } | |||
|
|
|||
| #[test] | |||
| fn test_document_symbol_with_hierarchy() { | |||
There was a problem hiding this comment.
Let's remove this test -- heavy tests are very slow, and don't really test much, as the end result is determined by the client anyway.
There was a problem hiding this comment.
the end result is determined by the client anyway.
The motivator for this PR is that Sublime displays the results in the order received from the server and doesn't do any sorting on the client side.
There was a problem hiding this comment.
Yeah, that's exactly my point: the property we want to check is that "client sensibly interprets the results", and we can't do that without the client.
To put it another way, we could have had the similar test before, with reverse order, and it would not have helped us to find out that the feature is broken in sublime.
There was a problem hiding this comment.
Okay. I have removed the tests. :)
No, I had not made a issue to it. I just made one: sublimelsp/LSP#1253 |
|
bors r+ |
Resolves #5655
And adds tests for
handle_document_symbol, both withhierarchical_symbolsenabled and with it disabled.Previously document symbols were displayed in reverse order in sublime text with its LSP plugin, but this patch fixes it like this: