Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [lsp4e-dev] [CAUTION] Re: Requesting feedback to a change that adds support for nested sub-projects to LSP4E

Hey!

I think there is a basic difference between the way this is dealt with in VSCode vs. LSP4E:

- vs code requires you to write a client-side extension, which takes care of starting the ls
- LSP4E is different, since it takes care of starting the ls and therefore is less flexible

I think what Dirk refers to is the ability of the client-side extension to decide whether to start a ls per workspace-folder, per project, per file, per whatever. And this is sort-of independent of the protocol itself. In contrast, LSP4E takes care of mapping the Eclipse workspace/project structure to language servers, since there is no client-side extension per language server that is able to do that.

I also looked at the patch and I am worried a bit. From what I’ve seen it always tries to find the parent project and uses that as the initial root project for the language server. But what happens for language servers that are not multi-root enabled and need to work on individual projects? They would only get the parent project and would have no idea about the individual projects. Is that correct? If so, I don’t think we can/should do that.

Supporting multiple roots in the language server is an elegant way to deal with this. Then language servers can do those checks (which root is a parent of which other root) themselves and do whatever they think is best.

The question remains what LSP4E should do if language servers do not support multiple roots, but the workspace has a nested project structure. A fixed implementation doesn’t seem to work for the various cases. I can imagine the server to use the outer-most project or the inner-most project as a root. We could also allow language server extensions to define a strategy for this that decides when a new language server should be started and when not - and use the current LSP4E implementation as the default. This would be the equivalent of the vscode client-side extension flexibility.

Just a few thoughts,
Martin









> Am 08.05.2018 um 16:36 schrieb Pohl, Christoph <christoph.pohl@xxxxxxx>:
> 
> Hi all,
>  
> Dbaeumers response does not sound like the protocol will change in this respect:
>  
> “Actually the specification intentionally doesn't clarify this since it might depend on the server implementation. For example the TSServer handles all workspace root folders and nested projects in one server. The MS C++ server can only handle on project pre server and the client starts a server per project and dispatches the requests to the corresponding server. For VS Code we even maintain two example projects demos both use cases here: https://github.com/Microsoft/vscode-extension-samples”
>  
> 
> --
> Christoph
> 
>  
> From: lsp4e-dev-bounces@xxxxxxxxxxx [mailto:lsp4e-dev-bounces@xxxxxxxxxxx] On Behalf Of Ofterdinger, Markus
> Sent: Montag, 7. Mai 2018 09:57
> To: lsp4e developer discussions <lsp4e-dev@xxxxxxxxxxx>
> Subject: [CAUTION] Re: [lsp4e-dev] Requesting feedback to a change that adds support for nested sub-projects to LSP4E
>  
> Hi Mikael,
>  
> I created an issue to ask for clarification: https://github.com/Microsoft/language-server-protocol/issues/475
>  
> Thanks,
> Markus
>  
> From: <lsp4e-dev-bounces@xxxxxxxxxxx> on behalf of Mickael Istria <mistria@xxxxxxxxxx>
> Reply-To: lsp4e developer discussions <lsp4e-dev@xxxxxxxxxxx>
> Date: Friday, 4. May 2018 at 09:46
> To: lsp4e developer discussions <lsp4e-dev@xxxxxxxxxxx>
> Subject: Re: [lsp4e-dev] Requesting feedback to a change that adds support for nested sub-projects to LSP4E
>  
> Hi,
> 
> Thanks a lot for this interesting proposal and opening it for discussion.
> As mentioned on the bug, I think you should immediately consider enabling multi-root support in your LS to workaround this issue (additionally to the tremendous perf benefit brought by 1 LS per workspace instead of 1 LS per project). That said...
>  
> I think it's also worth starting the discussion on the LSP tracker itself about whether the protocol should emit some documentation, constraints or recommendations for such case: is a language-server expected to serve for all children file of the root folder? Can you please bring this to the LSP spec too? LSP4E can take a pragmatic choice independently of the spec: if we see all LS that are used over LSP4E these days do work for any child under a project, let's not make the spec a blocker and let's proceed with your proposal.
> 
> I'll have to check with Eclipse aCute (Omnisharp-Roslyn language server) and Eclipse Corrosion (RustLS) but I'm quite busy today and I'm on PTO for a week after that. Lucas is also on PTO for 2 or 3 weeks IIRC, so we won't be able to provide feedback about those projects before ~10 days.
> If after extensive thoughts and testing (ideally against aCute and Corrosion too) some other committers are fine merging this change in my absence, I'm all fine with it. I don't want to be a blocker here.
> 
> Cheers,
> _______________________________________________
> lsp4e-dev mailing list
> lsp4e-dev@xxxxxxxxxxx
> To change your delivery options, retrieve your password, or unsubscribe from this list, visit
> https://dev.eclipse.org/mailman/listinfo/lsp4e-dev



Back to the top