Rework Core.Options and infer all server capabilities from handlers #196
Conversation
The capabilities should be all infered
Also pull out the logic into its own function
| -- | CodeActionKinds that this server may return. | ||
| -- The list of kinds may be generic, such as `CodeActionKind.Refactor`, or the server | ||
| -- may list out every specific kind they provide. | ||
| , codeActionKinds :: Maybe [J.CodeActionKind] |
There was a problem hiding this comment.
I think the API for this isn’t quite right (but that’s not new to this PR):
The LSP spec states
The
CodeActionOptionsreturn type is only valid if the client signals code action literal support via the propertytextDocument.codeAction.codeActionLiteralSupport.
However, the current API requires you to specify the options before you can inspect the capabilities so you can’t actually specify this properly.
There was a problem hiding this comment.
Good catch. I think we can just return CodeActionOptionsStatic True in the case when the client capabilities do not declare support for code action literals, and pass on the code action options in the case that they do
| (map singleton <$> completionAllCommitCharacters o) | ||
| | otherwise = Nothing | ||
|
|
||
| codeActionProvider |
There was a problem hiding this comment.
This breaks ghcide in a way that I don’t know how to fix. We currently set this to CodeActionOptionsStatic True and as mentioned above, I can’t set the individual flags properly since I learn the capabilities after I need to specify the options. Maybe for now, we could use something like
isJust $ codeActionHandler h = maybe (J.CodeActionOptionsStatic True) (CodeActionOptions . Just) (codeActionKinds o)
There was a problem hiding this comment.
That sounds reasonable. I left it this way as it is possible to return a CodeActionOptions with a null CodeActionKinds inside it (which doesn't make much sense given the previous comment), so I'm happy enough to default to the static capability if the kinds in the LSP options are set to Nothing
Also fix some providers being errenously set
Previously we were using a mixture of inferring server capabilities based on wether or not there was a handler registered for it, and wether or not the user of the library had set options for it.
This changes it to the former, so that Core.Options only contains options that are needed to tweak client behaviour, and all the actual capabilities are determined based on wether or not there is a handler set.
It also adds some stricter types, and throws errors if the user of the library forgets to set some mandatory options when a specific handler is set, for example executeCommand and documentOnTypeFormatting.
Closes #195