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

Bridge the ide/api.lsp services for ide/lsp.client #7811

Merged
merged 1 commit into from
Oct 16, 2024

Conversation

lahodaj
Copy link
Contributor

@lahodaj lahodaj commented Sep 29, 2024

Discussed before, if a module provides implements ide/api.lsp services, it might be good to use them internally in the IDE. This is a bit more polished prototype doing that. It is done by having an internal LS server bridging the services to lsp.client. Note that the data are not serialized, they are just converted from the ide/api.lsp structures to the LS structures, and then to the IDE structures.

CC @jtulach

@lahodaj lahodaj added the LSP [ci] enable Language Server Protocol tests label Sep 29, 2024
@jtulach
Copy link
Contributor

jtulach commented Oct 2, 2024

@lahodaj thank you. I can confirm your patch does work. It displays errors/hints and shows code completion. All of that with just a three line change in Enso layer.xml:

NetBeans+PR7811+Enso

However navigator is broken. Without your PR the structure is visible (thanks to #7483), but with your PR it disappears...

@matthiasblaesing
Copy link
Contributor

matthiasblaesing commented Oct 2, 2024

@jtulach the missing navigator should be fixed by #7757, the feature was broken by #7483. So test this together with current master.

@lahodaj
Copy link
Contributor Author

lahodaj commented Oct 2, 2024

I've added bridging for documentSymbols, which should fix the navigator (and should also give you breadcrumbs for free, I think). I think this issue with the navigator demonstrates the existing approach is quite fragile.

I've also moved the annotation to api.lsp, as requested elsewhere.

@jtulach jtulach self-requested a review October 8, 2024 17:13
@jtulach
Copy link
Contributor

jtulach commented Oct 8, 2024

Enso works with the current state of the PR:

  • errors
  • code completion
  • navigator

I don't see breadcrumbs, but that OK. Please get this into a release!

@lahodaj lahodaj marked this pull request as ready for review October 9, 2024 13:56
@lahodaj
Copy link
Contributor Author

lahodaj commented Oct 15, 2024

Unless there are objections, I'll squash and merge in the next day or so.

@lahodaj lahodaj merged commit 793cb83 into apache:master Oct 16, 2024
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LSP [ci] enable Language Server Protocol tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants