Plugins

[alanz]

We now have a proof of concept plugin setup, demonstrated in Ide.Plugin.Example.

What are the next steps?
Should the plugins currently homed in ghcide be moved here?
Should this be used only for migrating plugins from hie (formatters, hlint, etc)?

[jneira]

Well, i think all should be handled the same way at the end but maybe we can start with hie's ones and add value faster.

[alanz]

Yes, I think the formatters are probably the best place to start, and in terms of those start with Ormolu.

[EncodePanda]

I and @lukasz-golebiewski were working in Bristol on stylish-haskell plugin (which we have almost ready, I think PR will land this weekend) in HIE. Should we port it here right away?

[alanz]

@EncodePanda may as well

[EncodePanda]

we will start in HIE, and if that goes through we will add support in haskell-ide

[ndmitchell]

The two plugins in the ghcide repo are Completions and CodeActions. I'm ambivalent about where they live in the long term, but having them in ghcide isn't causing problems, let's us iterate the ghcide Plugin API, and they have great tests where they are - so I'd probably not move them until (at the earliest) after the first release of this project.

I know @cocreature had views too.

[cocreature]

For now, I would prefer to keep completions and code actions in ghcide. Partially because right now, we depend only on ghcide in DAML but not ide and we want to use those plugins and partially because I think it’s somewhat helpful to have a bit more testing surface in ghcide itself to see the impact on plugins caused by changes in ghcide.

That said, if this does turn out to cause issues or there are compelling arguments for moving them over, I’m not opposed to changing this at some point.

[alanz]

@cocreature I tend to agree that they stay where they are for now.

I guess the question will become more relevant if the e.g. completions functionality already in hie is different / better, and we want to make that available here.

[pepeiborra]

It seems to me that the current plugin architecture is not really extensible: it cannot handle two plugins of the same type. E.g. we cannot have two code action plugins, the first plugin that produces a response wins. Fixing this will require some architectural changes in ghcide.

Has anyone thought about this yet?
How much do we care about it?

[pepeiborra]

I guess the question will become more relevant if the e.g. completions functionality already in hie is different / better, and we want to make that available here.

The completions functionality in ghcide was ported from hie by @serras, so that shouldn't be an issue

[alanz]

How much do we care about it?

Quite a bit. The general model in hie has been to be able to specify providers, which get chained together. So multiple sources of diagnostics, code actions, etc.

The current PR (#42) for formatters shows how to select one of n where that actually makes sense.

And these are the two "styles" we should support, in my opinion.

[ndmitchell]

The plugins were designed to be extensible - the important point is that https://hackage.haskell.org/package/ghcide-0.1.0/docs/Development-IDE-LSP-Server.html#t:PartialHandlers gets the existing handlers as an argument, so a partial handler for completions can both answer and get someone else to answer in the same call. It's certainly not been used that way to date, so may not work in practice, but that was the design...

[pepeiborra]

The plugins were designed to be extensible - the important point is that https://hackage.haskell.org/package/ghcide-0.1.0/docs/Development-IDE-LSP-Server.html#t:PartialHandlers gets the existing handlers as an argument, so a partial handler for completions can both answer and get someone else to answer in the same call. It's certainly not been used that way to date, so may not work in practice, but that was the design...

Right, the problem is that Handlers are not composable. Composing handlers 'A' and 'B' using their Semigroup instance yields two independent responses where we would want a single, composed response, and there is no other way of composing handlers as far as I can see.

[ndmitchell]

The idea was NOT to compose them using their Semigroup instance - there's a reason PartialHandler is not opaque. There's the semigroup composition (overriding) and the by-hand call on to the other handlers and do your own stuff to put them back together.

[alanz]

I think it would be a good idea to put together a toy example chaining two handlers, for say code actions.

[pepeiborra]

All right, let me try to explain.

The current definition of codeActionPlugin in ghcide throws away the existing codeActionHandler:

codeActionPlugin :: (LSP.LspFuncs c -> IdeState -> TextDocumentIdentifier -> Range -> CodeActionContext -> IO (Either ResponseError [CAResult])) -> Plugin c
codeActionPlugin f = Plugin mempty $ PartialHandlers $ \WithMessage{..} x -> return x{
    LSP.codeActionHandler = withResponse RspCodeAction g
    }
    where
      g lsp state (CodeActionParams a b c _) = fmap List <$> f lsp state a b c

I thought that this was an oversight, easily fixed by composing with the existing handler using the Semigroup instance:

codeActionPlugin f = Plugin mempty $ PartialHandlers $ \WithMessage{..} x -> return x{
    LSP.codeActionHandler = withResponse RspCodeAction g `composeHandler` LSP.codeActionHandler x
    }
    where
      g lsp state (CodeActionParams a b c _) = fmap List <$> f lsp state a b c

composeHandler :: Handler b -> Handler b -> Handler b
composeHandler = (<>)

I think that's what Neil is suggesting too, unfortunately it doesn't work because Handler itself is not composable.
The type of Handler is:

type Handler b = b -> IO ()

-- pseudocode to show what (<>) means
instance Semigroup (Handler b) where
  h1 <> h2 = \b -> h1 b >> h2 b

Code action handlers are usually defined using withResponse, which is implemented in the ghcide LanguageServer module by (indirectly) sending an immediate response to the lsp client. So the code above ends up sending potentially two responses, and it's up to the client to decide what to do with them. VSCode seems to ignore the second one.

[ndmitchell]

Ah yeah, that's a design that works for the first few type synonyms, but not all the way down. This part is layered on top of Ghcide, so changing it shouldn't be a big deal, once we can figure out the right API.

[serras]

The completions functionality in ghcide was ported from hie by @serras, so that shouldn't be an issue

There were some additions, though, in how documentation is obtained and shown.

[jneira]

In that design maybe it would be nice that plugins could transform and augment the result of previous ones, although the order of plugin application will matter.

[alanz]

@ndmitchell @pepeiborra I am going to give this a shot, with some example plugins in hls.

Then we can bikeshed the final API

[alanz]

Right, the problem is that Handlers are not composable. Composing handlers 'A' and 'B' using their Semigroup instance yields two independent responses where we would want a single, composed response, and there is no other way of composing handlers as far as I can see.

The way we do it in hie is to effectively have a Semigroup operation on the results of each handler, and we only send the response once, at the end, when all the relevant handlers have run.

How likely are we to want to actually override a "lower" instance of a handler, given we can assemble the handler stack we want in the Main.hs?

[alanz]

Because asking each plugin to be "well-behaved" and to pass on the results from the prior one in the chain seems like asking for trouble.

[alanz]

In hie we have

data PluginDescriptor =
  PluginDescriptor { pluginId                 :: PluginId
                   , pluginName               :: T.Text
                   , pluginDesc               :: T.Text
                   , pluginCommands           :: [PluginCommand]
                   , pluginCodeActionProvider :: Maybe CodeActionProvider
                   , pluginDiagnosticProvider :: Maybe DiagnosticProvider
                   , pluginHoverProvider      :: Maybe HoverProvider
                   , pluginSymbolProvider     :: Maybe SymbolProvider
                   , pluginFormattingProvider :: Maybe FormattingProvider
                   } deriving (Generic)

This allows the LSP Server loop to deal with each of those categories in the way that makes sense. The pluginName and pluginDesc are for some mythical future when a UI allows you to decide which plugins to enable. They probably do not belong.

I am going to try to layer something like this on top of what already exists in ghcide, and see where it takes me.

[alanz]

FYI, the germ of this just landed, via

haskell-language-server/exe/Main.hs

Lines 70 to 71 in 80bc2e8

	formatterPlugins [("ormolu", Ormolu.provider)
	,("floskell", Floskell.provider)] <>

and

https://github.com/haskell/haskell-language-server/blob/master/src/Ide/Plugin/Formatter.hs

[pepeiborra]

A possible solution to the problem of having 2 code actions plugins generating 2 responses is to use the lsp protocol support for partial results: https://microsoft.github.io/language-server-protocol/specifications/specification-current/#partialResults

I'm going to prototype this approach and will revert back to this thread.

UPDATE: turns out that VSCode does not support partial results yet - microsoft/vscode-languageserver-node#528

[alanz]

Ok. I also have a draft concept in my branch on my repo. Currently work in progress, but gives the separation I am keen on, and can end up similar to the current plugin descriptor in hie. But I am traveling this week, so progress is stalled.

…

On Tue, 25 Feb 2020, 03:49 Pepe Iborra, ***@***.***> wrote: A possible solution to the problem of having 2 code actions plugins generating 2 responses is to use the lsp protocol support for partial results: https://microsoft.github.io/language-server-protocol/specifications/specification-current/#partialResults I'm going to prototype this approach and will revert back to this thread — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#25?email_source=notifications&email_token=AADEAB2SA3QX2P2I3EFXQQLRESBK5A5CNFSM4KN5DYVKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEM2K4KA#issuecomment-590655016>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADEAB4IQRREESUTA7QE333RESBK5ANCNFSM4KN5DYVA> .

[alanz]

See also #164

[jneira]

@alanz maybe we could close this one?

[alanz]

@jneira I am happy to do that. Do we have docs around this? I guess @pepeiborra blog posts count as that.

[jneira]

We could add more specific issues with changes in the design or need of documentation