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

Support using LSP4J with WebSockets #203

Closed
juliandolby opened this issue Jul 5, 2018 · 21 comments · Fixed by #314
Closed

Support using LSP4J with WebSockets #203

juliandolby opened this issue Jul 5, 2018 · 21 comments · Fixed by #314
Milestone

Comments

@juliandolby
Copy link

I needed to support LSP access from Monaco, the browser-based editor from Microsoft. As far as I could tell, WebSockets are the portable way to do that, so I have created a helper class to implement LSP endpoints on WebSockets. It uses javax WebSocket annotation support, and I have tested it using Tomcat. Would there be interest in me contributing this code?

@jonahgraham
Copy link
Contributor

I am interested. If it is easier, perhaps before making a PR you can point us at where your current implementation is?

@cdietrich
Copy link
Contributor

relates to #189

@juliandolby
Copy link
Author

My code is here: https:/wala/IDE/blob/master/com.ibm.wala.cast.lsp.tomcat/source/com/ibm/wala/cast/lsp/tomcat/WalaWebSocketServer.java

The code was written in the context of my specific project using LSP, but I tried to generalize the types to make clear it should work for any LanguageServer

@jonahgraham
Copy link
Contributor

To me this looks interesting, for both language server and (my primary interest) debug server. Having full support for this will probably require some API work, but that should make it easier to use for other endpoints too, like @cdietrich mentions for #189.

@juliandolby
Copy link
Author

Cool. I am happy to share this code and you can hack it any way you like. I added Eclipse license headers to make that clear.

@jonahgraham
Copy link
Contributor

In the original issue you mentioned contributing the code. Do you plan on doing that? I see you have since added the license header in the previously shared link. I would like to work with you to factor the code out of wala/IDE into LSP4J, but I don't think I will have time to be editing wala/IDE anytime soon :-( First step would be contributing your code as a PR with a test or two?

@juliandolby
Copy link
Author

I do indeed plan on contributing it. I'll be happy to make it into a pull request somewhere. Can you suggest a place to add it?

@juliandolby
Copy link
Author

I have now refactored the code a bit to extract the LSP WebSocket server to be independent of my specific server.

@jonahgraham
Copy link
Contributor

How about putting it somewhere near Launcher for the generic parts, and the specialization for the language server around LSPLauncher

You can, or I will if you can't, add the specialization for debug protocol in the same style next to DSPLauncher / DebugLauncher

@juliandolby
Copy link
Author

One possible issue: this code needs WebSocket annotations from javax.websocket, and I think those might not be part of J2SE. Hence, if we add this code to the same source tree, it might then need J2EE to compile. If that is the case, we may want to make a separate source tree for this code?

@jonahgraham
Copy link
Contributor

Hmm. That is a good point. I am going to have to solicit input on this from other committers/project lead. @spoenemann Do you have any input on this?

@juliandolby
Copy link
Author

Maybe it is not too bad... I added my server class to the org.eclipse.lsp4j project, and I only needed to add a compile dependence on the WebSockets api definition. In particular, I added the following line to its build.gradle file, under dependencies:

compile "javax.websocket:javax.websocket-api:1.1"

@jonahgraham
Copy link
Contributor

What does that do to the MANIFEST.MF? Add an Import-package/Require-bundle? Either one seems problematic if that means there is a whole bunch of dependencies pulled in that wouldn't normally be needed.

@juliandolby
Copy link
Author

I guess it would do that, but I only built with gradle. The manifest is generated by buildship? To avoid that, I guess we'd need a different project for the WebSocket support.

@juliandolby
Copy link
Author

I have now created a new project in the lsp4j repository that includes only the websocket support. that is the only par of the build that needs the javax.websockets api

@jonahgraham
Copy link
Contributor

Thanks. I am now waiting for @spoenemann or someone else to comment before proceeding.

@spoenemann
Copy link
Contributor

Putting this into a separate project sounds good.

You could also have a look at the websocket adapter we did for integrating Xtext with Sprotty:

https:/theia-ide/sprotty/tree/master/server/xtext-diagram/src/main/java/io/typefox/sprotty/server/xtext/websocket

@juliandolby
Copy link
Author

juliandolby commented Jul 24, 2018

My pull request is here: #211

@kguelzau
Copy link

👍 is there a schedule for the v0.8.x release? :-)

@spoenemann
Copy link
Contributor

Probably August.

@jonahgraham
Copy link
Contributor

There is a proposal in #471 upgrade websocket implementation to the Jakarta version. I am making this comment here so those who wrote and were interested in this initial version have a chance to comment before the decision is finalized.

cc: @juliandolby @kguelzau @spoenemann

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 a pull request may close this issue.

5 participants