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

Add type hierarchy support #779

Merged
merged 3 commits into from
Sep 16, 2021
Merged

Conversation

Eskibear
Copy link
Member

@Eskibear Eskibear commented Jun 22, 2021

This is a prototype to support type hierarchy proposed in microsoft/language-server-protocol#1231

Because there is no implementation of type hierarchy providers and related APIs in VS Code, I have to implement a simple one here, immitating provider registration in VS Code core.

Changes are mainly two parts:

  • Type hierararchy feature, similar with other features e.g. call hierarchy
  • Implemetation of type hierarchy providers, which should eventually be moved to (re-implemented in) vscode core. already available in vscode.proposed.d.ts since v1.59

Status of this PR:

  • basic implementation
  • add integration test
  • verify E2E test for Java. (type hierarchy providers in vscode 1.59, peek view in vscode 1.60)
  • verify if it works with multiple LSs at the same time.

I'm still working on it. The draft PR is created for early review of the feasibility. Any critical problem or obvious mistake about the whole workflow, please let me know.

@Eskibear
Copy link
Member Author

Eskibear commented Sep 1, 2021

Type hierarchy provider is already in vscode, I'm going to refine this PR based on that.

One question is, do I have to move TypeHierarchy into proposed namespace (same as Diagnostics) or just keep it as is?

@dbaeumer
Copy link
Member

dbaeumer commented Sep 1, 2021

@Eskibear it has to go to the proposed namespace fist although it is available in VS Code.

@dbaeumer
Copy link
Member

@Eskibear in which state is this PR. It is marked as a draft and it does have duplicate files (e.g. server typeHierarchy.ts and proposed.typeHierarchy.ts)

Are you planning on finishing the PR. I think we can merge it this month and create a new LSP release to contain it.

@Eskibear
Copy link
Member Author

It's almost done. I just want to do some E2E verification before I mark it ready to review.

BTW is there any convenient way to create a private npm package for testing purpose? Due to the dependencies (client -> protocol -> types), previously to verify it E2E, I have to bump versions and create npm packs one-by-one, install them locally... and change the relative path to version number accordingly.

Signed-off-by: Yan Zhang <[email protected]>
@Eskibear
Copy link
Member Author

I just tested with a mock LS for plaintext. Both "Show Type Hierarchy" and "Peek Type Hierarchy" work fine.

The only problem is, when I use languageClient.registerProposedFeatures(); to enable typehierarchy support, there's an error from proposed.diagnostic , tracked in #811

@Eskibear Eskibear marked this pull request as ready for review September 14, 2021 14:42
@Eskibear
Copy link
Member Author

E2E smoke test against Java LS:

type-hierarchy-e2e.mp4

@dbaeumer
Copy link
Member

Cool. Thanks for providing this.

Copy link
Member

@dbaeumer dbaeumer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rest looks good to me

protocol/src/common/protocol.ts Outdated Show resolved Hide resolved
@dbaeumer dbaeumer merged commit d5b8fbe into microsoft:main Sep 16, 2021
@Eskibear Eskibear deleted the type-hierarchy branch September 16, 2021 09:35
@Eskibear Eskibear restored the type-hierarchy branch September 16, 2021 09:35
@Eskibear Eskibear deleted the type-hierarchy branch September 16, 2021 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants