Adopt workspaceFolders

[baronfel]

Per ionide/ionide-vscode-fsharp#1204 we should move off of using rootPath/rootUri directly.

Updated thoughts

We should do this, but it's a large task so we will take it in stages:

Stage 1 - remove use of deprecated members, keep behavior the same as today

• update FSAC to not use RootPath/RootUri directly, in favor of the workspacePaths[0].Uri - this should be functionally the same as today, but stop our use of the deprecated members
• update Ionide to do the same

Stage 2 - allow FSAC to handle not being the first workspacePath

• update FSAC to probe the N workspacePaths for the first workspacePath with F# code in it and then fixate on that one - this keeps us in the 'single workspace' mode but enables users with many other workspaces to start working with us
• update Ionide to do the same

Stage 3 - update FSAC to support multiple workspaces, with AdaptiveServerStates per workspace

• update FSAC to support multiple proper workspaces, managing a set of AdaptiveServerState instances per workspace root
• update Ionide to do the same

First version of this description

Here's some initial investigation:

I've been looking at handling workspaceFolders (omnisharp has a not-completely-terrible thing that helped me get an idea for how it could be done), but it's completely going to up-end the initialization assumptions we've made so far. A brief explanation is in order. Right now, there
are three primary initialization flows, all of which I call the single folder model:

manual-init

in this model, the call stack looks like:

• initialize
• workspacePeek
• workspaceLoad

at this point the projects load, the user can do interactions as normal

automatic-init-single-interesting

in this model, the call stack looks like:

• initialize (with the AutomaticWorkspaceInit flag set)
  • the initialize logic calls workspacePeek and workspace load and there's only one directory or solution file found
• that one thing is loaded

at this point the projects load, the user can do interactions as normal

automatic-init-multiple-interesting

in this model, the call stack looks like:

• initialize (with the AutomaticWorkspaceInit flag set)
  • the initialize logic calls workspacePeek and workspace load and there are multiple potential interesting directories/solutions
• nothing is loaded, some workspace load notifications are fired and the user has to choose a workspace

In all of these cases, the intialize payload has a RootPath/RootUri, and there's a block of F#-specific settings under the FSharp property key. We set a rootpath in the Commands at this point, and we eventually set the workspaceRoot (some mutable state) in the LSP itself.

multi-workspace

In a multi-workspace situation, none of that holds. The callstack looks like the following:

• initialize (with RootPath/RootUri set, workspaceFolders set, and no Fsharp settings)
• workspaceDidChangeConfiguration with the new configuration now including FSharp settings

This means that we need to extract out the actual initialization code from the initialize call and conditionally apply it, as well as likely listen to workspaceChanged events to set the workspace root.

[baronfel]

Note that I'm not actually suggesting supporting multiple roots in FSAC, but just altering our initialization routines to handle them, eventually deciding upon the one root.

[baronfel]

From reading https://github.com/microsoft/vscode/wiki/Adopting-Multi-Root-Workspace-APIs#eliminating-rootpath a bit more, we might have to change our settings to support being set at the resource scope in order to get them to show up at the initialize call instead of waiting for workspace-specific settings via the configuration change event. Lots more details here

[baronfel]

Ok, taking a look at this today and summarizing some thoughts. Today our behavior in a multi-workspace scenario is very bad - we only look inside the first workspace, which often can not even be .NET code.

When a workspace is defined (either anonymous or loaded from a saved workspaces config file), the LSP server is still initialized via only a single initialize call. This single call will have a property workspaceFolders that looks like this:

    "workspaceFolders": [
        {
            "uri": "file:///e%3A/Code/FsAutoComplete",
            "name": "FsAutoComplete"
        },
        {
            "uri": "file:///e%3A/Code/ionide-vscode-fsharp",
            "name": "ionide-vscode-fsharp"
        }
    ]

in addition, the rootPath/rootUri will be set (for compat reasons):

"rootPath": "e:\\Code\\FsAutoComplete",
"rootUri": "file:///e%3A/Code/FsAutoComplete",

I think we have three modes to think about here:

No workspace

This appears as a single workspace in the initialize, so we can consider these two the same

Single workspace

We can update FSAC's initialize handler and config update handler(s) our code to use the single workspace that is present as the 'root'. Similarly, Ionide's custom logic for inferring a root should do the same.

Multi-workspace

In this case we have two scenarios to think about - one in which only a single workspace has a solution file or F# code, and one in which multiple workspaces have a solution file or F# code.

Single sub-workspace has F# code

In this case we can continue as normal - we can have the workspacePeek and FSAC AutomaticInit routines probe each workspace root, discard the ones that have no interesting solution files or projects, and if only one workspace has any interesting items we can treat that workspace as the inferred root from that point onwards

Multiple sub-workspaces have F# code

In this case we have some complications to consider. I am interested in two main touch-points:

• how workspacePeek probing should behave
• how inferring a RootPath for the AdaptiveServerState should behave

For workspacePeek probing, we should consider a model where potential workspaces are inferred on a per-LSP-workspace basis. So instead of a single set of results like today, the Response would be an array of responses, each one mapped to a specific LSP workspace. Within a specific LSP workspace inference could continue as today, which decisions about which specific solution/project should be opened being made in each workspace. Since inside a VSCode workspace settings are shared across open workspaces, we would need an alternative to FSharp.workspacePath that could be set per-parent-workspace. Possibly just an array of { workspaceName: string; rootProjectOrSolution: string }.

For inferring a RootPath for the AdaptiveServerState, I think this could be more problematic. AdaptiveServerState.RootPath is kind of a big deal - it acts as a pivot for many derived settings that are then implicitly used through other members of AdaptiveServerState. As far as I see it we can take two approaches to the State - either internalize the concept of multiple workspace roots in a singleton AdaptiveServerState, or instead change the AdaptiveFSharpLspServer to track multiple AdaptiveServerState instances - one per workspace that contains F# code. To me this second option seems more attractive. It allows for more easily reasoning about the information held by the AdaptiveServerState at the cost of the FSharpLspServer's APIs needing to map from an incoming TextDocumentUri to the AdaptiveServerState instance belonging to the workspace that contains that file.

@TheAngryByrd / @Krzysztof-Cieslak, do you have any thoughts or commentary on this? Right now my inclination is to go the multiple-AdaptiveServerState-instances route.

[TheAngryByrd]

AdaptiveFSharpLspServer to track multiple AdaptiveServerState instances - one per workspace that contains F# code... To me this second option seems more attractive

I agree. I think it makes sense have a AdaptiveServerState per workspace. It does seem like the mapping between textdocument/didX might be challenging. I think we'll have to expose all files loaded by msbuild in an AdaptiveServerState and filter based on that because even if we get a didChange event, it's not guaranteed that file will actually be under a workspace so doing something like StartsWith would create issues.