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

Move Type Hierarchy implementation to LSP #2376

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

CsCherrYY
Copy link
Contributor

@CsCherrYY CsCherrYY commented Mar 24, 2022

Requires eclipse-jdtls/eclipse.jdt.ls#2033

The change is able to review/merge, but we will get several BREAKING changes, so feel free to discuss we can now continue the process or wait for further changes.

Pros:

  • supertype/subtype hierarchy view is supported in vscode, we can get dismiss/history features for free.
  • All communications about TypeHierarchy are based on LSP, as for future updates, we can git it for free.

Cons:

  • The type hierarchy in LSP only supports supertype hierarchy and subtype hierarchy, no class hierarchy. We should still hold custom implementations about the class hierarchy view.

  • Since "Show Type Hierarchy" is contributed from language client/vscode itself, we have no way to control its behavior, so the default view of "Show Type Hierarchy" would be the subtype hierarchy view. We added a new entry "Show Class hierarchy" to open the class hierarchy view by default.
    image

  • As for those types have no children, the star will always show there. This is also the limitation of current call hierarchy view.
    image

@rgrunber
Copy link
Member

Can this along with the change on the language server be revived now that we've updated to the 3.17 spec ?

@CsCherrYY
Copy link
Contributor Author

Can this along with the change on the language server be revived now that we've updated to the 3.17 spec ?

I'm looking at this. The UX change (some UI limitation) of this PR is mentioned in #2376 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants