Naming and purpose of org.eclipse.cdt.lsp.editor.ui plug-in

[ghentschke]

Hi,
I'am not satisfied with the current plug-in name. I think org.eclipse.cdt.lsp.default / org.eclipse.cdt.lsp.default.server or something like this would be more appropriate since it should IMO reflect the purpose of the plug-in.

IMO it should contain all stuff for a default language server definition in CDT which is (currently) clangd. This server definition is enabled by a project property setting. Other IDE vendors may provide their own server definitions depending on their specific toolchain with enabledWhen based on a specific project nature or whatever. They can override the default server definition by setting a higher priority in the server definition extension point.

The org.eclipse.cdt.lsp should contain the logic used to configure LSP4E and manage which editor to use for a given C/C++ source file depending on the enable expression provided by the server provider. It also provides stuff which LSP4E cannot, because it needs a language specific implementation (e.g. content provider for the C data model). So this plugin contains everything which is independent of the provided server.

[ghentschke]

Currently I'am working on a PR to separate/refactor the code according to my purpose above.

[ruspl-afed]

let's try to shape the whole subsystem first, not just one bundle.
and default is not good since it matches Java keyword

[ruspl-afed]

This is how it looks from my side, 4 bundles seem to be a good compromise between necessity and perfectionism:
[general, clangd] * [headless, ui] =

• general.headless
  • branding bundle
  • tm4e grammars
  • facilities to register LS
  • facilities to override LS
  • no .ui dependencies
  • no clangd-related code
• general.ui
  • generic UI code
  • generic .ui contributions
  • no clangd-related code
• clangd.headless
  • registration of clangd
  • no .ui dependencies
• clangd.ui
  • UI for clangd location and options
  • UI for Toggle Source/Header functionality

Having org.eclipse.cdt.lsp as a prefix and (for example) the mapping for segments like

• headless = ``
• ui = editor
• general = ``
• clangd = clangd

we will get the following list of bundles:

• org.eclipse.cdt.lsp
• org.eclipse.cdt.lsp.editor
• org.eclipse.cdt.lsp.clangd
• org.eclipse.cdt.lsp.clangd.editor

WDYT @ghentschke @ddscharfe ?

[ghentschke]

We should not mix these two technical requirements here:

1. A generic implementation which is able to support different LS.
2. Separate headless from ui stuff.

Regarding 1.:
When I started, my intention was to have one "core" plugin which does the configuration of LSP4E. This should have been independent of a distinct server definition. Actually we are glued to clangd, because we implemented a clangd specific extension of the LSP. I'am okay with that. IMO the benefit of the clangd feature is greater than a generic solution. So a rename of org.eclipse.cdt.lsp into org.eclipse.cdt.lsp.clangd would make sense to me.

The second plugin should implement a default server for CDT. Including its configuration possibilities. It's clangd with a default configuration.

If someone has a good proposal to make the org.eclipse.cdt.lsp plugin independent of clangd stuff by integrating it in the second plugin, I would appreciate it. Then the first plugin can stay org.eclipse.cdt.lsp and the second can be org.eclipse.cdt.lsp.clangd

Regarding 2.:
I agree with Jonah when he says:

The "core" / "headless" part of this solution is clangd running out of process

We could split it up here as @ruspl-afed suggested, but what are the benefits? And do the benefits predominate? I think there are not much since the whole lsp thing here is mostly about editor selection and configuration and therefor much ui dependent. A separation makes the tests more complicated. IMO test cases should cover functionality not implementation and this is more difficult to test when it's spread across several bundles.

Conclusion:
If we cannot remove the clangd dependency from org.eclipse.cdt.lsp we could put everything into one plugin/bundle called org.eclipse.cdt.lsp.clangd

[ghentschke]

@ruspl-afed What are the technical benefits of splitting ui and headless/core in this specific case. IMO it makes only sense when I can use a distinct functionality without ui. That's not the case here.

[ruspl-afed]

Personally I can see more use cases how to use data received with LSP, such a data should not necessary go "right to the editor and only to the editor". But I'm fine to forget about it for a while, since some people could be too fascinated by architecture and design of LSP4E and VS Code to discuss "headless vs ui" constructively.

Great, we've already spent a lot of energy on this and we need to move on.

I'm very thankful for all the positive comments here and suggest to agree on the following bundle structure proposed by @ghentschke :

• org.eclipse.cdt.lsp
  • no clangd-related code
• org.eclipse.cdt.lsp.clangd
  • clangd-related code

[ddscharfe]

+1, I still prefer having four plugins, but let's agree on org.eclipse.cdt.lsp and org.eclipse.cdt.lsp.clangd.

[ruspl-afed]

Sure, me too, but we can do it later @ddscharfe
For now we can provide a package structure within each bundle that is ready for split.