Allow users to specify whether to use cabal's multi-repl feature
#4179
Conversation
fendor
force-pushed
the
wip/configure-multi-repl
branch
2 times, most recently
from
99563a5
to
35a22ad
Compare
|
The only thing is kind of bad is that changing the load style requires a restart of the language server. We could track when the config changes and unload all components instead. |
fendor
force-pushed
the
wip/configure-multi-repl
branch
2 times, most recently
from
989d7c1
to
dce9b8b
Compare
Yeah, we already do something like this when e.g. we have a change to a cabal file. Can we do the same here? |
Yeah, I believe we can. |
|
I'm skeptical about the utility of the |
Yes, after some more discussion, I agree that the
|
|
Then isn't this really just a flag saying "use cabal multi-repl if available: yes/no"? I continue to be a little suspicious of pretending that it's more general than it is. |
|
Yes it is, and the docs can make this clear, but there is nothing that is specific to cabal cradles right now. |
fendor
force-pushed
the
wip/configure-multi-repl
branch
6 times, most recently
from
d3029e3
to
a47b398
Compare
hls-plugin-api/src/Ide/Types.hs
Outdated
| @@ -194,6 +197,7 @@ instance Default Config where | |||
| -- , cabalFormattingProvider = "cabal-fmt" | |||
| -- this string value needs to kept in sync with the value provided in HlsPlugins | |||
| , maxCompletions = 40 | |||
| , sessionLoading = SessionLoadSingleComponent | |||
There was a problem hiding this comment.
I do think we should make the default multi-component. We want people to benefit from this without knowing they have to turn it on! The main thing is that they can turn it off if they need to.
There was a problem hiding this comment.
People have raised the concern that this is basically an untested cabal feature that may contain many hard to understand bugs, so we should disable it by default, and then enable it by default in the next release.
| @@ -461,6 +469,7 @@ loadSessionWithOptions recorder SessionLoadingOptions{..} dir = do | |||
| runningCradle <- newVar dummyAs :: IO (Var (Async (IdeResult HscEnvEq,[FilePath]))) | |||
|
|
|||
| return $ do | |||
| clientConfig <- getClientConfigAction | |||
There was a problem hiding this comment.
I'm not sure this actually works? This will get the client config at this point, but we consult it inside consultCradle which is an IO function that maybe runs later? So might we just always see a stale version of the config? I think we might need to get the config in IO inside consultCradle.
There was a problem hiding this comment.
It does work, in combination with invalidating the GhcSessionIO rule.
This new dependency seems to be working: https://github.com/haskell/haskell-language-server/pull/4179/files#diff-65a56362c2b3593800cd8369e6cfab1e92c347a9be97d1f0b8793f0e5dd9d524R711
And it doesn't invalidate the whole session on config changes.
fendor
force-pushed
the
wip/configure-multi-repl
branch
4 times, most recently
from
8d700ea
to
095c04b
Compare
We add an option to `Config` that allows clients to specify how HLS
should load components.
We support two loading strategies:
* SessionLoadSingleComponent: Always load only a single component
when a new component is discovered.
* SessionLoadMultipleComponents: Always allow the cradle to load
multiple components at once. This might not be always possible,
e.g., if the tool doesn't support multiple components loading.
The cradle decides how to handle these situations.
By default, we use the conservative `SessionLoadSingleComponent` mode.
Additionally, changing the config at run-time leads to a reload of the
GHC session, allowing users to switch between the modes without
restarting the full server.
fendor
force-pushed
the
wip/configure-multi-repl
branch
from
095c04b
to
036a537
Compare
We add an option to
Configthat allows clients to specify how HLS should load components.The three options are:
This allows users to opt-out of
cabalmulti-replwhenever they want and vice versa.Closes #4178
Depends on haskell/hie-bios#433
Sibling PR for vscode-haskell: haskell/vscode-haskell#1077