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

GH-508: Support for Call Hierarchy #925

Closed
wants to merge 4 commits into from

Conversation

kittaakos
Copy link

@kittaakos kittaakos commented Jan 28, 2019

Issue: #508.

This PR contains

  • the migration of the o.e.j.i.coreext.callhierarchy package to the the JDT LS.
    • Copied o.e.j.i.corext.callhierarchy source from JDT UI to LS. ccf7771
    • Switched from JavaPlugin to JavaLanguageServerPlugin. c7ea1da
    • Got rid of the Eclipse workbench dependencies. 33b0a71
  • the handler for the language server.

Akos Kitta added 3 commits January 24, 2019 16:02
…JDT UI to LS.

Signed-off-by: Akos Kitta <kittaakos@typefox.io>
…rPlugin`.

Also, reused the `StringMatcher` from the CA instead of `jdt.ui.util`.

Signed-off-by: Akos Kitta <kittaakos@typefox.io>
Signed-off-by: Akos Kitta <kittaakos@typefox.io>
Closes eclipse-jdtls#508

Signed-off-by: Akos Kitta <kittaakos@typefox.io>
@kittaakos kittaakos changed the title [WIP] GH-508: Support for Call Hierarchy GH-508: Support for Call Hierarchy Jan 29, 2019
@kittaakos
Copy link
Author

kittaakos commented Jan 29, 2019

This PR is ready for review. I start working on the client implementation in Theia, and I will update this PR with an example so reviewers can try the new call hierarchy LS feature in action.

@kittaakos
Copy link
Author

The next LSP4J release is 21 February according to our plans. It will include the call- and type-hierarchy capabilities.

@fbricon
Copy link
Contributor

fbricon commented Feb 4, 2019

Can you check if the changes in jdt core are sufficient to cover your needs? #508 (comment)

@kittaakos
Copy link
Author

Can you check if the changes in jdt core are sufficient to cover your needs?

I have looked into the PR, and all seems reasonable. The DEFAULT_IGNORE_FILTERS is initialized with an empty String in the CallHierarchyCore, and I am not aware of any other required modifications. Thank you for the refactoring! 👍

Do I have to wait till the jdt.core changes get merged?

@fbricon
Copy link
Contributor

fbricon commented Apr 11, 2019

So it seems the Call Hierarchy API has changed in the past couple weeks microsoft/vscode-languageserver-node#420 (comment). We'll need lsp4j to adjust to the new API and then propagate the impacts to this PR

@fbricon
Copy link
Contributor

fbricon commented Apr 11, 2019

@rgrunber @jjohnstn when can we expect the changes in jdt core to be available?

@rgrunber
Copy link
Contributor

I'll have a look at the change next week. and hopefully we can get it pushed in. After that it should be available within a day since you track 4.12-I-builds in your target platform.

@fbricon
Copy link
Contributor

fbricon commented May 2, 2019

@kittaakos FYI, the JDT changes have been merged upstream

@kittaakos
Copy link
Author

@kittaakos FYI, the JDT changes have been merged upstream

Thank you for the update, @fbricon!

I'll try to adjust the PR in the forthcoming days.

@yaohaizh
Copy link
Contributor

@kittaakos Any update on this PR? Would like to try this feature if possible.

@testforstephen
Copy link
Contributor

Looks like this PR is pending for a while, really look forward to have a play with this feature.

@kittaakos is there any update about this PR? If you feel it's ok, i would like to provide help to resolve the remaining issue.

@kittaakos
Copy link
Author

If you feel it's ok, i would like to provide help to resolve the remaining issue.

Feel free grabbing and reusing whatever you want. I won't have time to work on this in the near future. Please keep in mind this: eclipse-lsp4j/lsp4j#330.

Without the LSP4J changes, you have to build and host your own target platform.

@testforstephen
Copy link
Contributor

testforstephen commented Jul 18, 2019

Have tried the feature locally, here is the status:

The VS Code client implementation is proposed api, and it lags behind the language server proposal, i have to change lsp4j back for local integration test, it works very well. Thank @kittaakos for your great work.

In the next step, I need wait for VS Code client being stable and fully ready in the official version, before finalizing the server side implementation.

@kittaakos
Copy link
Author

change lsp4j back

Please note, I have created a PR in LSP4J, so it is in-sync with the proposed protocol: eclipse-lsp4j/lsp4j#336

@MaskRay
Copy link

MaskRay commented Jul 21, 2019

I think it will be good not to call it hierarchy, because nothing prevents the result from being a flattened list. See microsoft/vscode-languageserver-node#420 (comment)

@testforstephen
Copy link
Contributor

Merged as part of #1306

Thanks @kittaakos

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.

6 participants