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 Notebook support #642

Merged
merged 4 commits into from
Jul 6, 2022
Merged

Conversation

KamasamaK
Copy link
Contributor

Fixes #640

Signed-off-by: Matthew Brown <[email protected]>
@jonahgraham jonahgraham added this to the v0.15.0 milestone Jun 27, 2022
Copy link
Contributor

@pisv pisv left a comment

Choose a reason for hiding this comment

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

I have checked it against the spec. Great work, as always! Minor suggestions attached.

Signed-off-by: Matthew Brown <[email protected]>
@KamasamaK KamasamaK force-pushed the notebook-support branch 2 times, most recently from 6168c24 to dbd84ed Compare June 29, 2022 14:23
Signed-off-by: Matthew Brown <[email protected]>
Copy link
Contributor

@pisv pisv left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@Cyrik
Copy link

Cyrik commented Jul 4, 2022

So I tried using the PR and I noticed that org.eclipse.lsp4j.services.LanguageServer has no getNotebookDocumentService method. Is there some other way to plugin my own NotebookDocumentService implementation?

@KamasamaK
Copy link
Contributor Author

@Cyrik Sorry. This is my first time adding a new service, so I missed some things.

@Cyrik
Copy link

Cyrik commented Jul 5, 2022

No problem at all, thank you for the fast update! I didn't notice the missing bit either until I wanted to pass in my service 😄
I'll try it out tomorrow.

@Cyrik
Copy link

Cyrik commented Jul 5, 2022

I just tried to use the new version but there seems to be no snapshot of it. Can I somehow trigger the build myself?

@pisv
Copy link
Contributor

pisv commented Jul 5, 2022

It looks like this build includes the latest changes, does not it?

@Cyrik
Copy link

Cyrik commented Jul 5, 2022

ah yes, thank you. I didn't find that link and was trying to use the nightly or build it myself 😅
The one you posted worked and after a bit of jvm version switching, I can also build my own. I've got capabilities to work from Clojure, going to try the actual sync messages tomorrow. Thank you.

@Cyrik
Copy link

Cyrik commented Jul 6, 2022

I've tested all the notebookDocument/* calls and they work for me! Capabilities also produce the correct results. So I'd be happy to have this merged and base our next LSP version on it.
Thank you for integrating this quickly!

@KamasamaK KamasamaK merged commit 6e20087 into eclipse-lsp4j:main Jul 6, 2022
@KamasamaK KamasamaK deleted the notebook-support branch July 6, 2022 15:20
@eclipse-lsp4j eclipse-lsp4j deleted a comment from pisv Jul 9, 2022
@eclipse-lsp4j eclipse-lsp4j deleted a comment from pisv Jul 9, 2022
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.

Notebook support
4 participants