Enable TypeDefinition Requests #1169
Conversation
| @@ -966,6 +966,7 @@ hieOptions :: [T.Text] -> Core.Options | |||
| hieOptions commandIds = | |||
| def { Core.textDocumentSync = Just syncOptions | |||
| , Core.completionProvider = Just (J.CompletionOptions (Just True) (Just ["."])) | |||
| , Core.typeDefinitionProvider = Just (J.GotoOptionsStatic True) | |||
There was a problem hiding this comment.
Is J.GotoOptionsStatic True the right thing?
There was a problem hiding this comment.
I looked the haskell-lsp docs and was none the wiser afterwards.
There was a problem hiding this comment.
I know that the protocol allows to pass a link instead of a file as response, which would be nice for hoogle packages. Should we look into this?
There was a problem hiding this comment.
What would that entail?
What do you mean with hoogle packages? Documentation?
There was a problem hiding this comment.
I’ll investigate a bit more, I think it would entail showing the local Html page build with haddock or potentially a link to the haddock page for the type, which we can get from hoogle
There was a problem hiding this comment.
This would also be interesting for findDefinition.
There was a problem hiding this comment.
Also, if it worked project wide, but I think, this is a different problem.
There was a problem hiding this comment.
I would expect the typedefinitionprovider flag to be set only if there is a handler for the gototypedefinition request, rather than having to be set explicitly.
So there may be a shortcoming in haskell-lsp.
There was a problem hiding this comment.
This is the correct usage of the LSP options from what I can remember (to let the client know what features the server supports and other stuff. We should probably go through flick on all the options for the stuff we already support at some point too)
As a side point, shouldn’t the field be called server capabilities? Perhaps I am confusing it with something else
|
So is this ready to merge? The feature looks like it's working on small projects. |
|
@bubba @fendor I would expect this capability to be set the same way it is done for go to definition, as per /~https://github.com/alanz/haskell-lsp/blob/master/src/Language/Haskell/LSP/Core.hs#L620 |
|
@alanz why are the capabilities of hie set via haskell-lsp? I thought, haskell-lsp is more general than hie. |
|
@fendor because some of them are mechanical consequences. If we have a typedefinitionrequesthandler, it means we have the capability, so should set it. If this were correctly set in haskell-lsp from the beginning, this PR would not be necessary. |
|
So, this is a mistake in haskell-lsp that should be fixed and this commit should be reverted after it is done? |
|
yes |
It works in VsCode with simple examples.