Open michaelpj opened 10 months ago
I'm also beginning to bump into this issue with semantic token legends. In this case the server-supplied legend needs to be chosen based on the client capabilities, and then stored somewhere for use when calling makeSemanticTokens
.
The simplest change I can think of to enable this would be:
ServerDefinition
's doInitialize
function chooses the server's legend, and stores it in the state a
.chooseCapabilities :: ServerCapabilities -> m ServerCapabilities
to ServerDefinition
, that reads from the LSP state and returns the modified capabilities.The main ugliness here is that that the signature of chooseCapabilities
allows you to send messages before initialisation (which you shouldn't!), but I don't think that's really avoidable.
So the way I'm currently thinking of approaching this is a bit bigger, namely via https://github.com/haskell/lsp/issues/583
But I think there's an intermediary state that would help, namely:
staticHandlers
field in ServerDefinition
doInitialize
return a ServerCapabilities
and a Handlers
also
inferServerCapabilities
themselves to get the ServerCapabilities
to start withI think this pushes us in the right direction and would let people set up the ServerCapabilities
as they like.
I'm in the middle of something else, but I'd welcome a PR for that change, I think it should be fairly localised!
Ahh, sorry! Had seen that issue, but hadn't quite thought about how it might relate to this one.
With this change, are you proposing removing all the capability-related information from Options
then, and removing that as an argument from inferServerCapabilities
, or would that stay for now?
Hmm yes, I think we could basically get rid of Options
entirely, in fact? I'm mildly unsure. Perhaps it's nicer for people to fill in a defined list of possible tweaks and feed it to inferServerCapabilities
, rather than poking individual bits of the capability structure themselves. But I would like to at least try the other approach, since I think it might be fine and it's both forwards-compatible and good for offering control the user.
Great, thanks!
Hmm yes, I think we could basically get rid of
Options
entirely, in fact?
There's a couple of options (server info, progress delays), which aren't part of server capabilities. Those could stay as-is, or be merged into ServerDefinition
.
Yes, those make sense to stay as options indeed.
Sorry for the lack of updates. I've a branch which makes this change, but having done so, I'm now less sure it makes sense in isolation from #583.
The main pain point here is that ServerCapabilities
matches the JSON representation of the server capabilities, rather than being especially easy to manipulate. For instance, code action options are defined as Maybe (Bool |? CodeActionOptions)
, so if you wanted to write a function that sets the code action kinds (as a replacement for optCodeActionKinds
), you either need to handle all the different cases, or just write a partial function.
Yeah, it's definitely less fun to work with ServerCapabilities
itself. I just added this which helps with getting to the capability field, but modifying it is indeed not that pleasant. I think you probably can do this nicely with sufficient lens magic, I should try and figure out an example!
So you could write a lens (or traversal) that promotes the boolean to the full options[^1]. For example (excuse the terrible names):
newtype ServerCapabilities' = ServerCapabilities' { unServerCapabilities' :: ServerCapabilities }
serverCapabilities :: Lens' ServerCapabilities' ServerCapabilities
serverCapabilities = lens unServerCapabilities' (const ServerCapabilities')
instance L.HasCodeActionProvider ServerCapabilities' (Maybe CodeActionOptions) where
codeActionProvider = serverCapabilities . lens get set where
get caps = case caps ^. L.codeActionProvider of
Nothing -> Nothing
Just (InL False) -> Nothing
Just (InL True) -> Just $ CodeActionOptions Nothing Nothing Nothing
Just (InR opts) -> Just opts
set caps val = caps & L.codeActionProvider .~ (InR <$> val)
[^1]: This wouldn't follow the lens laws (as over l id /= id
), but it preserves the meaning of the value (even if not the exact structure), so it's not the end of the world.
Right, and also for some of the others we should have sub/supertype lenses: https://github.com/haskell/lsp/issues/589
There are a few things I think we also need to express:
Nothing
fields, ideally we would express this with a typeclassTrue
into that. (What about if it's false? we want to overwrite that in some circumstances but not others, because we can't actually represent "no capability" in another way :( )Nothing
Then maybe we can have some optic that turns a big |?
of things into a canonical thing. I'm not sure I've quite got a handle on it yet, but I think there should be something sensible we can pull out of all this...
At the moment we infer the
ServerCapabilities
from theHandlers
andOptions
. The only way for servers to alter the capabilities that we infer is if we provide a handle for doing that viaOptions
. In some ways that's nice and discoverable, but it might be easier just to let the server directly modify the inferredServerCapbilities
before we send it back.In many cases I think it would just be fiddly to expose things via
Options
. For example, most server capabilities for methods allow you to opt-in to client-initiated progress. At the moment we just always disable that and rely on server-initiated progress, but it would be reasonable for a server to want to opt in... but even then you probably only want to do it for certain methods. So we could have amethodsToOptInToClientInitiatedProgress
option... or we could just let the server modify theServerCapabilities
.