-
-
Notifications
You must be signed in to change notification settings - Fork 390
Starting to sketch out IDE-level plugin modularity #45
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
It seems diagnostics work by generating a rule, and custom |
I'm afraid I don't see how this works. I thought the PartialHandlers couldn't be combined? But you seem to be using them as is? Where does the merging for code lens happen, for example? |
@ndmitchell this is in a very incomplete state at the moment, and I had to stop for a week. Working on it at the moment, will let you know when it makes sense to look at it again. |
src/Ide/Plugin.hs
Outdated
-- | Map a set of plugins to the underlying ghcide engine. Main point is | ||
-- IdePlugins are arranged by kind of operation, 'Plugin' is arranged by message | ||
-- category ('Notifaction', 'Request' etc). | ||
asGhcIdePlugin :: IdePlugins -> Plugin Config | ||
asGhcIdePlugin mp = | ||
mkPlugin executeCommandPlugins (Just . pluginCommands) <> | ||
mkPlugin codeActionPlugins pluginCodeActionProvider <> | ||
-- diagnostics from pluginDiagnosticProvider | ||
mkPlugin hoverPlugins pluginHoverProvider <> | ||
-- symbols via pluginSymbolProvider | ||
mkPlugin formatterPlugins pluginFormattingProvider | ||
-- completions | ||
where | ||
justs (p, Just x) = [(p, x)] | ||
justs (_, Nothing) = [] | ||
|
||
ls = Map.toList (ipMap mp) | ||
|
||
mkPlugin :: ([(PluginId, b)] -> t) -> (PluginDescriptor -> Maybe b) -> t | ||
mkPlugin maker selector | ||
= maker $ concatMap (\(pid, p) -> justs (pid, selector p)) ls | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the key part of this approach.
This is by no means polished, but provides a skeleton for how we should proceed. My intention is to merge it soon, so it provides a context for others to be able to build. |
Address haskell#25 Currently WIP
Tests do not pass, but want to rebase, so marking a checkpoint.
Proof of concept, seems to work in big pieces, needs huge amount of cleanup.
Currently (for me) neither ghcide nor haskell-language-server can resolve a simple project. Assume some sort of underlying hie-bios issue, will wait.
Demonstrated by adding missing pragmas derived from GHC error messages.
It piggy-backs existing args from ghcide, probably a bad idea
Starting to look pretty solid.
Currently suspect the range we get to format the whole file is wrong, second time around.
Moving PluginCommand into hls.
Conclusion is that getDocumentContents is returning junk, doing the idempotent test manually on vscode works as expected, but ends up with junk at the end of the file in the test. 2020-03-12 21:11:05.79062259 [ThreadId 38] - Formatter.doFormatting: contents= "{-# LANGUAGE NoImplicitPrelude #-} module Format where foo :: Int -> Int foo 3 = 2 foo x = x bar :: String -> IO String bar s = do x <- return \"hello\" return \"asdf\" " 2020-03-12 21:11:07.896114974 [ThreadId 7] - <--2--{"result":[{"range":{"start":{"line":0,"character":0},"end":{"line":9,"character":0}},"newText": "{-# LANGUAGE NoImplicitPrelude #-} module Format where foo :: Int -> Int foo 3 = 2 foo x = x bar :: String -> IO String bar s = do x <- return \"hello\" return \"asdf\" "}],"jsonrpc":"2.0","id":1} 2020-03-12 21:11:07.897123428 [ThreadId 5] - ---> {"jsonrpc":"2.0","method":"textDocument/didChange","params":{"textDocument":{"version":0,"uri":"file:///home/alanz/mysrc/github/alanz/haskell-language-server/test/testdata/Format.hs"},"contentChanges":[{"text": "{-# LANGUAGE NoImplicitPrelude #-} module Format where foo :: Int -> Int foo 3 = 2 foo x = x bar :: String -> IO String bar s = do x <- return \"hello\" return \"asdf\" ","range":{"start":{"line":0,"character":0},"end":{"line":9,"character":0}}}]}} ------------------------------------------------------- 2020-03-12 21:11:07.899375044 [ThreadId 213] - Formatter.doFormatting: contents="{-# LANGUAGE NoImplicitPrelude #-} module Format where foo :: Int -> Int foo 3 = 2 foo x = x bar :: String -> IO String bar s = do x <- return \"hello\" return \"asdf\" " 2020-03-12 21:11:07.902320214 [ThreadId 7] - <--2--{"result":[{"range":{"start":{"line":0,"character":0},"end":{"line":9,"character":0}},"newText":"{-# LANGUAGE NoImplicitPrelude #-} module Format where foo :: Int -> Int foo 3 = 2 foo x = x bar :: String -> IO String bar s = do x <- return \"hello\" return \"asdf\" "}],"jsonrpc":"2.0","id":2} 2020-03-12 21:11:07.902812215 [ThreadId 5] - ---> {"jsonrpc":"2.0","method":"textDocument/didChange","params":{"textDocument":{"version":0,"uri":"file:///home/alanz/mysrc/github/alanz/haskell-language-server/test/testdata/Format.hs"},"contentChanges":[{"text": "{-# LANGUAGE NoImplicitPrelude #-} module Format where foo :: Int -> Int foo 3 = 2 foo x = x bar :: String -> IO String bar s = do x <- return \"hello\" return \"asdf\" ","range":{"start":{"line":0,"character":0},"end":{"line":9,"character":0}}}]}} -------------------------------- hieCommand: haskell-language-server --lsp -d -l test-logs/hie-stack-8.6.5.yaml.log HIE cache is warmed up Format formatting provider formatting is idempotent FAILED [1] Failures: test/functional/FormatSpec.hs:64:42: 1) Format, formatting provider, formatting is idempotent expected: "{-# LANGUAGE NoImplicitPrelude #-}\n\nmodule Format where\n\nfoo :: Int -> Int\nfoo 3 = 2\nfoo x = x\n\nbar :: String -> IO String\nbar s = do\n x <- return \"hello\"\n return \"asdf\"\n" but got: "{-# LANGUAGE NoImplicitPrelude #-}\n\nmodule Format where\n\nfoo :: Int -> Int\nfoo 3 = 2\nfoo x = x\n\nbar :: String -> IO String\nbar s = do\n x <- return \"hello\"\n return \"asdf\"\nbar s = do\n x <- return \"hello\"\n return \"asdf\"\n" To rerun use: --match "/Format/formatting provider/formatting is idempotent/" Randomized with seed 1814425400
The CompletionProvider handler type should probably be extended to include a WithSnippets parameter, and the prefix.
It seems lsp-test is not applying edits properly, doing the same sequence in vscode results in the correct result.
Otherwise the tests crash.
The basic idea looks reasonable. I think a |
We use the plugin id when constructing a command name to be sent to the client. These end up being the concatenation of the server process id, the plugin id, and the exposed command. For me the critical thing is that the plugin id can be configured from the Main module when assembling a list of plugins for the particular instantiation, so we do not have to worry about command name clashes. If there is some simpler way to do this, we can move in that direction. This mirrors the approach we are currently taking in hie, so also makes it easier to port existing plugins. |
* Test for #45 * Remove redundant symbols from imports Fixes #45 * Update src/Development/IDE/LSP/CodeAction.hs Co-Authored-By: Andreas Herrmann <[email protected]> * Apply suggestions from code review Co-Authored-By: Andreas Herrmann <[email protected]> * Add regex-tdfa extra deps to ghc-lib build * Fix for GHC 8.4 (error message prints qualified binding) GHC ticket #14881 changed this to print identifiers unqualified * dropBindingsFromImportLine: make total Co-authored-by: Andreas Herrmann <[email protected]>
Address #25
Currently WIP