fix: fixed not being able to start multiple containers for client

[ajwittmond]

fix for #48

The issue was that every time a new connection is requested to the lsp-server a new server process and thus a new container should be created. However, since the containers are identified by name, a suffix must be appended to the name each time a container is created. This was slightly complicated to handle due to the fact that the container name was needed to convert uri's to paths, however the hooks used to do this did not receive the workspace and as a result it was impossible to tell which container the uri's belong to.

The solution to this was to associate each container with the root directory of the workspace that they were created for. Thus to do a URI to path conversion all that is necessary is to look up which root directory the path belongs to and then look up the container that serves that root directory. To do this, the hook for registering the lsp-clients was modified so that when a new connection is connected, the suffix is incremented and the name of the new container is added to a hashmap with the root directory as a key. Then the uri to path hooks were modified to search through the keys of this map to find which one matches the start of the path the URI converts to, and lookup the correct container name to do the full conversion.

In order to get the workspace when a new connection is requested, the full version of the hook was needed instead of just the options exposed by lsp-stdio-connection. As a result, the function returned by lsp-stdio-connection was wrapped by a function that handles registering the container with the workspace's root directory.

[factyy]

@ajwittmond , thanks for the PR! Could you please elaborate more on the changes? It will help a lot (I see you replaced a call to default constructing lsp-stdio-connection with a manual construct) to understand them.

[ajwittmond]

I update the original comment, I hope it helps to clarify the changes. If it is still not clear I am happy to answer more questions
!

[factyy]

@ajwittmond , thanks for the clarification, it really helped.

I managed to get a quick look and wanted to ask what happens if there are two running containers with the same path mappings (or at least similar e.g. like a c++ project inside a folder of a Python project)? I may be wrong but I don't get the idea of relying solely on path mappings when determining container names.

As for translation functions we use partial to effectively bind arguments to translation functions.

BTW I may be missing something or may be wrong, so let's have a discussion :)

[ajwittmond]

You are correct that the problem of nested projects is an issue. I didn't think of that. However, is there any case where the workspace a file is under does not have nearest root about it?. To be clear if I have
dir1
--| dir2
------| file1

and there is a workspace with root dir2. Is it possible that file1 should belong to another workspace? If not, I can ammend the search through the roots to find the root dir that is the longest prefix to the path. As far as the use of partial goes, I'm not sure where I should be using it, but I'm of course open to anything that might simplify this code

[factyy]

Oh, one more thing: we are talking about launching multiple instances of the same client or instances of different clients? By the way there is a feature under way that will allow to specify multiple clients with individual path mappings for a project. Probably it will solve your issue as well. It is being developed by @sfavazza

[ajwittmond]

The issue is with multiple instances of the same client and ultimately the bug was not directly caused by the path mappings, it was caused by the container names not being properly incremented with each instance of the client. I just had to make the biggest changes to allow the path mapping to still work while incrementing the container names.

[factyy]

One more thing: could you please elaborate more on needing multiple instances of the same client, I really don't understand the idea behind it. Any examples will help a lot :)

[ajwittmond]

I was under the impression that every call to lsp--client-new-connection should create a new process for the given language server. I have to admit, from the documentation and code I've read, some things about the language server protocol are still unclear to me. Is it expected that one server process manage multiple workspaces? One container per workspace seemed to make sense to me.

[sfavazza]

The .lsp-docker.yml defines (currently) a single language server for the project it is defined in. As matter of fact it is not possible to map folders external to the project the configuration is defined in.

Is it expected that one server process manage multiple workspaces? One container per workspace seemed to make sense to me.

One container per workspace is the correct statement. Basically you should have a .lsp-docker.yml file per-project (workspace). A single container running the configured language-server is capable to serve any compatible file within the project it is defined in.

The docker containers already feature unique names, even if you invoke lsp-docker-register multiple times in the same workspace (an incrementing value is appended to the name, see #70).

[ajwittmond]

The issue was that it did not create a new name for each workspace, only for each client. lsp-docker-register is only called when a client is created. lsp--client-new-connection is what is called when a new workspace is created.