Add Language Servers View

[sebthom]

This PR addresses #518 and introduces a new "Language Server Processes" view, looking like this:

[sebthom]

@ahmedneilhussain since you are doing a lot of refactoring lately maybe you have some ideas regarding the open issues?

[ahmedneilhussain]

I'm looking at the LS lifecycle code at the moment in an attempt to tidy it up and make the started/stopped/error states a little bit more explicit and atomic. I'll see what can be done.

[ahmedneilhussain]

Hi @sebthom I've been busy wading through the LS lifecycle code (in LanguageServerWrapper - there's some more in LanguageServiceAccessor). See #520 for the gory details.

I think it would be relatively straightforward to have LSW instances call a method on LanguageServiceAccessor to notify it of lifecycle events. The latter could then support subscribers (probably just your view, in practice!) to have these events forwarded on from all wrappers in the standard Observable pattern.

Start and stop are obviously explicitly called methods that are easy to hook into. I think we could also add a 'whenComplete' hook onto launcherFuture since that completes when the stream to the server is closed, which is a way of detecting crashes.

I think we could also expose some of the server info on ServerConnection in my PR.

[sebthom]

The fields on ServerConnection look very useful!

[angelozerr]

Just for your informartion, I have ported LSP4E classes (LanguageServerWrapper, etc) in IJ and I provide a LSP consoles which show both the started language server and the console. IMHO I think it is better to have processes and console in the same view.

Here a screenshot:

I have managed too lifecycle (starting, stopping, started, stopped) of language server to update UI according the sate:

[sebthom]

@angelozerr that looks really nice... unfortunately you implemented it for the "wrong" IDE :-)

I don't have the resources to match this in Eclipse atm. I still believe that the language server view offered by this PR is a worthwhile addition and can be extended later if required.

[angelozerr]

@angelozerr that looks really nice... unfortunately you implemented it for the "wrong" IDE :-)

Indeed I'm an Eclipse user, but I love discovering anything like vscode and IJ. I wanted just to share with you my feature that I think it could be very nice on LSP4E.

I don't have the resources to match this in Eclipse atm. I still believe that the language server view offered by this PR is a worthwhile addition and can be extended later if required.

yes sure!

[rubenporras]

@sebthom , I have not followed this PR, but it would be great if we could merge before the new release is done. Is it in an state where could merge it?

[mickaelistria]

I'd like to release LSP4E today or tomorrow, and I'm not comfortable with the idea of merging it big chunks of lowly-tested code into the codebase just before a release.

I also have concerns with the code that is relatively intrusive (introducing interfaces that core APIs must implement rather than using adapters) while the view itself would rather be totally standalone, and maybe even worth being part of a seperate bundle.
I will try to give a more precise review after the release.

Additionally to those technical concerns, I have a "strategical" one: if we give possibility to workaround possible issues with LS by giving control over the process, doesn't it bring a risk that people just rely more and more on workarounds rather than actually fixing the causes?...

[mickaelistria]

Could something like LanguageServers.anyMatching().collectAll((w, t) -> w) work?

[sebthom]

I don't see how. Each LanguageServers instance is for a specific document or project and obtained via LanguageServers#forDocument or LanguageServers#forProject. I need a list of all running LS no matter for which project or document.

[mickaelistria]

Additionally to the inlined comments, here are some suggestions:

• Use ProcessHandle that does everything well
• Use some adapterFactories to adapt the StreamProvider and LanguageServerWrapper to ProcessHandle; and consumers just call Adapters.adapt(wrapper, ProcessHandle.class). This prevents API from extra interface hierarchy
• I don't think such view is useful for the average user. I would rather see it part of a new dedicated bundle (eg org.eclipse.lsp4e.sdk) that is dedicated to LS management, monitoring, debugging... in the IDE.

[mickaelistria]

Thanks!

[rubenporras]

Thanks a lot, I am looking forward for this feature.