Ghcide: discussion : plugin architecture

[alanz]

Now that haskell-language-server exists, and we have given some thought and effort to using ghcide as the building block for maintaining project artifacts, it might be useful to have a discussion about the role of plugins.

The underlying haskell-lsp library exposes a Handlers data structure which represents every message the language server can receive, and can provide a specific handler for it. Unused handlers can be set to Nothing, and so ignored.

ghcide interposes a PartialHandlers structure, which can be populated with individual message handlers without reference to the underlying haskell-lsp Handlers. These can be combined monoidally, with the one drawback that if more than one partial handler attempts to process a given message in the haskell-lsp Handlers structure, the last one defined in the monoidal chain will "win" and mask the others.

In some cases this is fine, but when we want to freely combine plugins from different sources, each providing some useful information to the end user, this model breaks down.

A case in point is hover information. The haskell-language-server approach mirrors that in haskell-ide-engine, where there can be multiple hover providers, andthere is a top level handler which runs the provider from each plugin to get get potential hover information, and then combines the results into a single hover result message.

Similarly for exposing commands for the workspace/executeCommand handler, which routes the incoming command to the appropriate plugin handler for processing.

Right now, with https://github.com/haskell/haskell-language-server/pull/58 and #490 we are able to work around this architecture in haskell-language-server.

But perhaps we should consider changing the way it is represented in ghcide, to simplify it there. I do not have any firm preconceptions about what should go there, but to me the simplest case would be to be able to provide a haskell-lsp Handlers structure populated appropriately, together with whatever rules need to be made active.

This is a discussion issue, please share your thoughts.

cc @cocreature @ndmitchell @mpickering @pepeiborra @jneira @fendor and anyone else with an opinion.

[mpickering]

I will annoyingly point out that this isn't an issue with the reflex backend as each notification corresponds to an Event, which everyone can subscribe to and process in any way they see fit.

[cocreature]

@mpickering I’m not sure that’s really accurate. For an LSP request you can only send one response. If all plugins operate independently you end up sending multiple requests.

[alanz]

I agree. It is about managing the interface between ghcide (shake or reflex based) and haskell-lsp, in such a way that each message type in the protocol gets handled in a well defined way, which can included having a "combining" handler where more than one plugin can generate a response.

[mpickering]

@mpickering I’m not sure that’s really accurate. For an LSP request you can only send one response. If all plugins operate independently you end up sending multiple requests.

That is a good point Moritz, I suppose it is important to distinguish between notifications which require a response and those which don't.

[ndmitchell]

The design of PartialHandlers was based on fundamental assumptions that just aren't true, so it doesn't work as designed. Coming up with a new design is therefore required.

One potential design would be for each plugin to have Request -> Maybe (Action Response) and then we could use Shake to run all the responses using the parallelism architecture of the graph, and then define some kind of combining monoid for those responses that can be combined.

[alanz]

The design of PartialHandlers was based on fundamental assumptions that just aren't true, so it doesn't work as designed. Coming up with a new design is therefore required.

This was my understanding too, and hence this discussion.

[jneira]

Could have sense let plugins transform the previous response and let them decide if it should be replaced or augmented (Request -> Response -> Action Response)? No supervisor over plugin application will be needed.
It would be more similar to a Monad than a Monoid and order will matter. That can be seen as a advantage or a drawback.

[alanz]

It would depend on how the accumulation is done, and what extra info is required from any earlier plugins in the chain, if any.

In my opinion it is better to have a more general mechanism.

[pepeiborra]

My gut feeling is that the programming model offered by PartialHandlers feels right. The only obstacle is that VSCode doesn't accept multiple responses because it doesn't implement lsp partial results

Since partial results might become supported in the future, it might make sense to try and bridge this gap in ghcide itself until it is no longer necessary to do so, and avoid replacing by another abstraction that might prove inadequate in that hypothetical future.

I did some work to add partial results to haskell-lsp, lsp-test and ghcide. See the ghcide branch here including an example with two code action plugins: https://github.com/pepeiborra/ghcide/tree/partial-results

[alanz]

The only obstacle is that VSCode doesn't accept multiple responses because it doesn't implement lsp partial results

FYI The linked spec item refers to reporting progress.

[pepeiborra]

The only obstacle is that VSCode doesn't accept multiple responses because it doesn't implement lsp partial results

FYI The linked spec item refers to reporting progress.

Not sure what you mean. The link points to https://microsoft.github.io/language-server-protocol/specification#partialResults which describes the mechanism for multiple server responses for a single client request. But as said, not even VSCode implements this mechanism currently, so ghcide would need to collect all the responses from PartialResults handlers and merge them into a single one until the mechanism is implemented on the client side.

[alanz]

oops, I did not read it clearly enough, the intro paragraph is about the progress stuff.

[Avi-D-coder]

@pepeiborra It will take even longer to be implemented in other editor's language clients.

[pepeiborra]

@pepeiborra It will take even longer to be implemented in other editor's language clients.

That wouldn't be a problem if ghcide bridges the gap by concating the responses from all plugins when the client doesn't support partial responses.

[alanz]

Well, I am looking at doing something in hls similar to what we have in hie, where we can have some kind of control over how we combine responses, which could include suppressing some if others are present.

The current approach can work, and is working in the branch I have.

And I guess it also brings into question the future breakdown of functionality between the various components: Does haskell-language-server need to exist?

[pepeiborra]

Well, I am looking at doing something in hls similar to what we have in hie, where we can have some kind of control over how we combine responses, which could include suppressing some if others are present.

The current approach can work, and is working in the branch I have.

And I guess it also brings into question the future breakdown of functionality between the various components: Does haskell-language-server need to exist?

You are right, there is a bit of a duplication of efforts here. I think the higher level FooProvider approach is perfect for hls. It has clear semantics and it makes it very easy to contribute. And it makes complete sense for hls to exist and be the Haskell lsp server for the masses.

Is there any reason not to leave PartialHandlers as it is and move all the plugin definitions to hls?

[alanz]

Is there any reason not to leave PartialHandlers as it is and move all the plugin definitions to hls?

This is actually the conclusion I am coming to as well.

And FYI I have most of the plugins exposed and operational in the tip of https://github.com/haskell/haskell-language-server/pull/57, which is built on top of @pepeiborra 's interface files and @mpickering 's multi-component support.

[jneira]

Afaiu we have a final conclusion here and there is nothing left to be done so we could close this, if nobody disagree