Lsp: 0.9.3 reintroduced issue #79

[olegbl]

My project has multiple folders defined, with some being direct ancestors of one another. e.g.:

• /foo/
• /foo/bar/

The LSP server (Flow) is only valid inside a subfolder (e.g. /foo/bar/) and only applicable to files within.

Previously, (with https://github.com/sublimelsp/LSP/pull/81) I was able to just set my project's first folder entry to /foo/bar/ to have LSP plugin start the LSP server in the correct directory.

Now that it uses workspaceFolders instead, I have no way of controlling the order of these folders and as a result LSP plugin tries to start the server in /foo/ and fails.

I was able to temporarily fix it for myself by modifying sorted_workspace_folders in workspace.py to comment out the sorting logic:

def sorted_workspace_folders(folders: List[str], file_path: str) -> List[WorkspaceFolder]:
    sorted_folders = []  # type: List[WorkspaceFolder]
    for folder in folders:
        # if file_path and file_path.startswith(folder):
        #     sorted_folders.insert(0, WorkspaceFolder.from_path(folder))
        # else:
            sorted_folders.append(WorkspaceFolder.from_path(folder))
    return sorted_folders

This sorting logic probably needs to handle the case where multiple folders satisfy the prefix path requirement. Longest prefix length should probably win.

[rwols]

The way things work now are as follows:

1. If a language server is workspace-aware, there'll only ever be one process of that language server running, serving all of your folders for the entire .sublime-project. The rootUri property in the initialize request is essentially meaningless. The language server will utilize the workspaces property instead.
2. If a language server is workspace-unaware, then for each folder in your .sublime-project file, a process of that language server is started, with the rootUri property in the initialize request set to the associated folder.

What that means for you, is that there'll be two Flow processes running. One for /foo and one for /foo/bar. This is what that sorted_workspace_folders function is for: once we determine if the language server is workspace-aware or workspace-unaware, we throw away all folders except for the "designated" first one in case the language server is workspace-unaware.

So in addition to handling the case where multiple folders satisfy the prefix path, what we probably want is also a way to disable starting a language server entirely for specific folders. This last problem is somewhat intrusive, and I don't know how it would look in practice. I'd like to defer this problem for later.

This entire problem could also be solved on the Flow side, if it were to handle workspaces. The Flow language server could iterate over the provided workspaces, and find its designated folder, ignoring all other folders.

All in all, the definitive bug is "longest prefix path should win", but you'll still experience some bogus diagnostics since two Flow instances will be started once that is fixed.

[olegbl]

All in all, the definitive bug is "longest prefix path should win", but you'll still experience some bogus diagnostics since two Flow instances will be started once that is fixed.

Well, the one for /foo/ wouldn't start since there's no .flowconfig in there so that should be fine.

If a language server is workspace-unaware, then for each folder in your .sublime-project file, a process of that language server is started, with the rootUri property in the initialize request set to the associated folder.

It doesn't seem like it works this way currently. It only tries to start Flow in one folder (/foo/), then crashes (server didn't respond in 3 seconds) and disables LSP for that window.

[tomv564]

I'm not super excited about optimising LSP for this less-than-optimal scenario (starting multiple servers and gracefully letting them fail).

Why not write the needed code to check for .flowconfig files instead? This is exactly the use case for a LanguageHandler.

[olegbl]

It's kind of strange to expect the user to be able to go modify the LSP server of a product they're using (not developing).

It's much easier to manually fix the code for sublimelsp which is exactly what I did. It's just going to be annoying to keep re-doing that after every update.

The fix here is even very simple. (Sort the list by length of matching prefix instead of always bumping the last entry that has a matching prefix to the top.)

e.g.:

def sorted_workspace_folders(folders: List[str], file_path: str) -> List[WorkspaceFolder]:
    def sortFn(folder):
        return -len(folder) if (file_path and file_path.startswith(folder)) else 0
    def mapFn(folder):
      return WorkspaceFolder.from_path(folder)
    return list(map(mapFn, sorted(folders, key=sortFn)))

Also, I'm not sure Flow is doing anything wrong asides from missing workspace directories support. It shouldn't successfully start if there's no .flowconfig. SublimeLSP is simply choosing the wrong working directory for the active file.

[rwols]

What @tomv564 meant is that you write a file in Packages/User/flow.py containing something like this (untested):

from LSP.plugin.core.handlers import LanguageHandler
import os


def contains_flowconfig(p):
    q = os.path.dirname(p)
    if p == q:
        return False
    if os.path.exists(os.path.join(p, ".flowconfig")):
        return True
    return contains_flowconfig(q)


class Flow(LanguageHandler):

    def on_start(self, config, window):
        # unfortunately, the file_path + workspace folder is not passed to this method
        file_path = window.active_view().file_path()
        return file_path is not None and contains_flowconfig(os.path.dirname(file_path))

    name = "flow"

    def config(self):
        return ClientConfig(
            name=self.name,
            command=["flow", "lsp"],
            tcp_port=None,
            languages=[
                  LanguageConfig(
                       languageId="js",
                       syntax="Packages/JavaScript/JavaScript.sublime-syntax",
                       scopes=["source.js"])],
           enabled=True