Skip to content
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

Filtering in the Outline View #254

Closed
gerdleon opened this issue Oct 6, 2022 · 19 comments
Closed

Filtering in the Outline View #254

gerdleon opened this issue Oct 6, 2022 · 19 comments
Labels
enhancement New feature or request

Comments

@gerdleon
Copy link
Contributor

gerdleon commented Oct 6, 2022

Similar to other Outline Views it would be useful to be able to filter the elements shown in the outline. For a minimal viable feature it is likely the SymbolKind that is used for the filtering.

For a user it would be nice to be able to do such filtering but I am unsure about it in the context of LSP since it could differ based on the content-type. Meaning in one content-type hiding fields will be a reasonable filter but for another it would filter out nearly everything.

Do you have an opinion? Are the others seeing a need for such filtering?

@mickaelistria
Copy link
Contributor

Can you please give an example of 1 actual user-story where filtering would be helpful (eg in Wild Web Developer, or Corrosion, or ShellWax...)?

@gerdleon
Copy link
Contributor Author

I do not know enough about Corrosion, etc.

I think it is natural to offer this - which is also why VS Code has this implemented in their Outline/Breadcrumbs (per content-type https://www.youtube.com/watch?v=eyJS7rgRSYw).
Not all elements returned from the LS make always sense to be shown
A similar functionality is implemented in Eclipse JDT. There it is possible to filter the elements shown.

@mickaelistria
Copy link
Contributor

Not all elements returned from the LS make always sense to be shown

Then it seems to be something to discuss with the Language Server.

@gerdleon
Copy link
Contributor Author

I looked at a few tickets opened for WildWebDeveloper. There is no such concrete ticket but I see one usability ticket that goes in a similar direction:
eclipse-wildwebdeveloper/wildwebdeveloper#404 --> here the request is to group constants to allow folding (which looks difficult to implement, since the group would need its own SymbolKind).
My interpretation is that the user wants to be able to see this information but it is not always useful to show it in the outline.

@gerdleon
Copy link
Contributor Author

Would you be open to a contribution of this feature?

@mickaelistria
Copy link
Contributor

From what I read of the description, it really doesn't seem like a client side/Wild Web Developer issue. When I read "Not all elements returned from the LS make always sense to be shown", then it's a clear sign that the language server should be changed or configured to hide what doesn't make sense. The purpose of LSP spec is to interact with the UI, the grain of action is the UI, so having things that do not make sense in the UI being returned by the LS seems like a LS issue.
LSP4E is not a place to host workarounds for bugs in language servers.

@gerdleon
Copy link
Contributor Author

gerdleon commented Nov 2, 2022

I do not think it is correct to say that it is a workaround for a bug in a language server. The language server will return as many elements as make sense - that is: if there is a constant, method, function, enum, enumKind, etc. it should be in the response even if there are many of them.

How the result is visualized is up to the client.

Taking Java as an example, there I would expect that all methods, all fields, all constants, etc. are returned from the LS.
But if a class contains 100 constants, it might be the fault of the developer of the class but it is not the fault of the LS.

Given there is limited space on a screen, it makes sense to filter out certain elements in the Outline View based on the users needs. I am not aware of specific support in LSP to configure the result returned from the LS.

Please remember that established products already implemented this functionality:

  • VS Code has filtering based on SymbolKind
  • Eclipse JDT has very extensive filtering (hide static, private, etc. which I think is not possible with LSP). It even provides pattern matching on the names

So I argue that this is not providing a workaround but implementing an expected feature of the LSP client.

@mickaelistria
Copy link
Contributor

VS Code has filtering based on SymbolKind

Is it available for all languages using LSP? (XML, Java...)? Is it real filtering on client side, or passing some LS configuration so the LS skips from SymbolKinds?
It should be fine just mimicking what VSCode does by default, as it's the reference implementation. Having a reference implementation is a good way to take a decision ;)

@gerdleon
Copy link
Contributor Author

gerdleon commented Nov 3, 2022

I checked VS Code 1.71.2 by tracing the communication between LS and VS Code. There is no configuration request sent to the LS that restricts the SymbolKinds so it all happens in the client.
And yes, it is available for all languages using LSP.

The setting which SymbolKinds are shown is not per content-type in VS Code.

@mickaelistria
Copy link
Contributor

OK. Let's try to mimic that in LSP4E.

@sebthom sebthom added the enhancement New feature or request label Jan 14, 2023
@travkin79
Copy link
Contributor

Has there been any progress on this? Maybe I could give it a try if nobody started implementing that feature.

@mickaelistria
Copy link
Contributor

I'm not aware of anyone actually started to work on it, so contributions here remain fully welcome!

travkin79 added a commit to travkin79/lsp4e that referenced this issue Aug 12, 2024
travkin79 added a commit to travkin79/lsp4e that referenced this issue Aug 12, 2024
travkin79 added a commit to travkin79/lsp4e that referenced this issue Aug 12, 2024
@travkin79
Copy link
Contributor

I've implemented a first prototype that filters the elements in the content provider. The outline view's menu entries look as follows. I think, that satisfies most requirements, doesn't it?

image

travkin79 added a commit to travkin79/lsp4e that referenced this issue Aug 13, 2024
travkin79 added a commit to travkin79/lsp4e that referenced this issue Aug 13, 2024
@travkin79
Copy link
Contributor

Hello @mickaelistria,
I've created a PR #1049. Could you take a look at it and check if you can merge it? Thank you very much.

travkin79 added a commit to travkin79/lsp4e that referenced this issue Aug 13, 2024
travkin79 added a commit to travkin79/lsp4e that referenced this issue Aug 13, 2024
travkin79 added a commit to travkin79/lsp4e that referenced this issue Aug 13, 2024
travkin79 added a commit to travkin79/lsp4e that referenced this issue Aug 13, 2024
travkin79 added a commit to travkin79/lsp4e that referenced this issue Aug 13, 2024
@rubenporras
Copy link
Contributor

Hi @gerdleon,

I think that after the recently merged PR, LSP4E now has the same functionality as VS Code, according to your comment in #254 (comment), thus I am closing the issue.

Regards

@gerdleon
Copy link
Contributor Author

@travkin79 I have seen some strange behavior. Sometimes the "Donut", marked red in the screenshot, disappears.
image
And here it is not appearing
image

It seems that it happens most often upon first opening of the outline view but others in my team have seen it happen in seemingly random places.

@travkin79
Copy link
Contributor

Hi @gerdleon,
Thank you for reporting that issue. Could you please create a new issue? I'll check that and I hope to fix that as soon as I can.

@travkin79
Copy link
Contributor

Hi @gerdleon,
It's fixed now. See PR #1152

@gerdleon
Copy link
Contributor Author

@travkin79 thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants