emacs-lsp / lsp-docker

Scripts and configurations to leverage lsp-mode in docker environment
GNU General Public License v3.0
241 stars 34 forks source link

feature request (?) multiple language server for project #79

Closed sfavazza closed 3 months ago

sfavazza commented 10 months ago

Assuming a multi-language project. Is it possible to define multiple language-servers within a .lsp-docker.yml file?

The same goes for the debug servers, is it possible to define multiple?

factyy commented 10 months ago

Oh, right now it is not possible. BTW I planned to add it later but had no time sadly

sfavazza commented 10 months ago

Right now I don't need it, but it might come handy eventually. I might come up with a proposal, so that we can discuss it. As I am willing to contribute.

Because of the actual yml file structure, the new feature shall be back compatible.

factyy commented 10 months ago

Sounds pretty awesome! :)

sfavazza commented 10 months ago

The proposed feature shall be back compatible with the current implementation (using the keyword 'server). The following is the YML configuration file that lsp-docker should interpret (the same goes for the lsp-docker-persistent-default-config variable customizable via dir-locals.el):

lsp:
  servers:
    <local-unique-server-id-1>:
      ...                       # keys as in the classic single server case, e.g. type, subtype, etc...

    ...                         # other single server instances

    <local-unique-server-id-n>:
      ...
  mappings:
    - <path-on-host>:<path-on-ls>
    ...

I guess that's it, let me know what you think and whether I hadn't considered something.

factyy commented 10 months ago

Looks nice! The only issue I can think of is whether it is ok that all mappings will be the same across all servers

sfavazza commented 10 months ago

I am always in favor of major flexibility, but not at the cost of code duplication.

I propose to support these mutually exclusive conditions:

To keep the implementation simple I would consider invalid any hybrid mapping (i.e. a mix of global and per-server mappings).

factyy commented 10 months ago

Yeah, I agree.

I propose to focus mostly on global mappings (keep in mind that there may also be a debugger section). And only then implement per-server mappings as an additional feature.

sfavazza commented 10 months ago

keep in mind that there may also be a debugger section

About that, if you consider that dap-mode natively supports launch.json configuration files, I don't see the advantage in implementating a debug section.

In my daily workflow I manually start a docker container featuring a debug server and simply connect to it via its IP adress (reported in the launch.json file).

Or do you already have an idea of what this future feature should provide?

And only then implement per-server mappings as an additional feature.

Can you be more specific, I am not sure I fully understand. How should the additional feature look like?

factyy commented 10 months ago

Yeah, debug section is basically a duplicate of launch.json. The main idea was to have a full config (both for lsp- and dap- modes) in a single file.

As for the per-server mappings I simply meant adding support for mappings for each server individually. Probably as a subkey for <local-unique-server-id-n> for each server from your example.

sfavazza commented 10 months ago

Hello!

I am working on the feature, but it requires some heavy changes. To ease up the process I will split the change in smaller ones. But the idea would be to merge them in a dedicated feature-branch. So that it won't affect other users adopting the current version.

Could you create a branch that I can use as a target for the intermediate MR requests?

factyy commented 10 months ago

Feel free to create something like feature/multiple-servers-support and then work with it. When finished create a PR from this branch, I'm probably not needed for it :)

sfavazza commented 10 months ago

What I meant was you create this feature/multiple-servers-support branch in your repo, so that I can open MR against it (I can't create branches on your repo AFAIK). This allows for small "incomplete" changes to not disrupt the main branch.

If you don't like this approach, I can make a full scale feature implementation and good luck with a monolithic review 😂

factyy commented 10 months ago

Ah, you want to work using my repo, that's ok. Created a branch for you :)

sfavazza commented 10 months ago

Do I have access to it? I cannot find it: image

factyy commented 10 months ago

Oh, a bit strange TBH. I'll take a look at this (but I never opened my private forks for external contributors, I may be missing something). What about a direct link?

sfavazza commented 10 months ago

Ok, I think I misunderstood the situation here. @factyy you have the right to merge into emacs-lsp/lsp-docker (judging from last MR you authorized), but can you also create branches on this repo?

I checked again your link and it is pointing to a fork of yours, but this was not my intention. I wish to have a MR target branch on emacs-lsp/lsp-docker not the factyy/lsp-docker. Is this possible? Do you know who is responsible for this repository?

factyy commented 10 months ago

Mostly @yyoncho

I don't feel really comfortable doing anything except merging simple PRs without his explicit approval :)

sfavazza commented 10 months ago

In that case I will invite you as collaborator on mine, we work there and then I will open the big MR here when the feature is in a decent state.

factyy commented 3 months ago

Since @sfavazza's implementation is already merged I'm closing this issue