Skip to content
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

Extract ghcide completions and code actions into HLS plugins #724

Closed
pepeiborra opened this issue Dec 6, 2020 · 9 comments · Fixed by #1257
Closed

Extract ghcide completions and code actions into HLS plugins #724

pepeiborra opened this issue Dec 6, 2020 · 9 comments · Fixed by #1257

Comments

@pepeiborra
Copy link
Collaborator

pepeiborra commented Dec 6, 2020

The completions code in ghcide is structured as a single provider that serves multiple contexts (pragmas, imports, types, values, etc.). The static set of dependencies is particularly suboptimal: no completions for pragmas unless the file type checks (haskell/ghcide#631)! It would be advisable to break this down into a family of providers that can all contribute completions depending on the context.

Haskell-language-server has a plugin model that solves exactly this problem. Rather than duplicating this model in ghcide, it would make sense to extract HLS plugins instead. It would constitute a loss of functionality for ghcide users, but note that we are considering to remove the ghcide executable (#935). If that doesn't go ahead, we could always extend the current executable with the ability to run HLS plugins as discussed in #380

Similarly, the code actions code in ghcide should be properly extracted out as HLS plugins, although this seems lower priority. The code is already loosely structured as a bunch of providers, 16 in total, and does not have the shortcomings listed above for completions. Still, an HLS plugin translation would bring certain benefits -decoupling, parallelism and isolation- and hopefully no downsides.

@gdevanla
Copy link
Contributor

gdevanla commented Dec 9, 2020

@pepeiborra I feel this is a good project I can focus my efforts on. If no one else has taken a stab at it, I can start work on this. I like the idea very much. I did feel the Completions code was doing to many things all at once.

Would this necessarily mean, the Completions and CodeActions section will be completely moved out of ghcide, except for say the implementation of the Rule types?

Based on my understanding, this is a multi-step process, which needs co-ordination for removal from ghcide and addition to respective hls plugins. Do you have any initial thoughts on how we should be approaching this on a per task basis. Perhaps, we could have a list of TODO items under this task, to accomplish what we need. I look forward to other suggestions as well. Meanwhile, I can prepare my own notes on the changes needed.

That said, I am happy to co-ordinate with others here, on a separate branch to get through these changes as well.

@pepeiborra
Copy link
Collaborator Author

By all means, do work on this if you are interested!

My mental model is:

  • I would start with the completions. If it goes well continue with the code actions. There's no need to micromanage if you are the only person working on it

  • Extract all the code to one or more packages which depend on his-plugin-api and include one or more plugins each. This package would live in the haskell-language-server repo. Something like:

    • hls-ghc-completions-plugins: A cabal package containing three (or more) HLS completion providers
      • "local-completions" provider: An HLS provider for local completions and the LocalCompletions rule
      • "non-local-completions" provider: An HLS provider for non-local completions and the NonLocalCompletions rule
      • "import-completions" provider: similar

@gdevanla
Copy link
Contributor

Awesome. Thanks for elaborating on that. I had a similar mental model in mind.

@pepeiborra
Copy link
Collaborator Author

pepeiborra commented Dec 26, 2020

I have been giving this a bit more thought and realised that we probably want to keep the completions code in this repository, so that we can continue to leverage the benchmark suite as well as preserve the history of the code, just wrap them as HLS plugins following the plan outlined by @jneira in #380 (comment):

I would say that all ghcide plugins should be separated from the lib, make them follow the haskell plugin definition (introducing hls-plugin-api as dependency) and make ghcide executable depends on those plugins (like we did in hls)
I would keep them in the ghcide repo to make them available to daml, that maybe would need be refactored to acommodate that change (not sure about that)

The problem is that I do not know how to do this without incurring in accidental complexity. Because his-plugin-api depends on ghcide, ghcide cannot depend on hls-plugin-api without creating a cycle in the Cabal graph (Cabal solver works at the package level, not at the component level, see haskell/cabal#1575) . One has to split ghcide in two packages ghcide-types and ghcide-exe. Another option is to move his-plugin-api into ghcide. I'm inclined to agree with @wz1000 about merging the two repos.

@ndmitchell
Copy link
Collaborator

I'm inclined to agree with @wz1000 about merging the two repos.

It now seems an inevitable terminal state as a result of the changes that are happening, and Ghcide will end up disappearing. I'm OK with that.

@pepeiborra
Copy link
Collaborator Author

pepeiborra commented Dec 27, 2020

A better alternative is removing the dependency on ghcide from his-plugin-api. This is not only possible but also quite reasonable, since all the ghcide bits (the providers and the asGhcIdePlugin adapter) can be moved ghcide itself. Then hls-plugin-api becomes a pure types package which both hls and ghcide depend on.

I'm inclined to agree with @wz1000 about merging the two repos.

It now seems an inevitable terminal state as a result of the changes that are happening, and Ghcide will end up disappearing. I'm OK with that.

I think it's inevitable that the two repos are merged, but I don't see ghcide the library going away any time soon.

@jneira
Copy link
Member

jneira commented Dec 27, 2020

This is not only possible but also quite reasonable, since all the ghcide bits (the providers and the asGhcIdePlugin adapter) can be moved ghcide itself. Then hls-plugin-api becomes a pure types package which both hls and ghcide depend on.

Sounds good, also it would be definitely better for hls-plugin-api downstream packages out there

@gdevanla
Copy link
Contributor

@pepeiborra Just so that I understand. With this change, I am assuming we retain the completion code inside ghcide and then make an effort to split up local/non-local/other completions inside this repo itself. Is that correct?

This also means that we don't need a separate completions plugin residing inside hls.

@pepeiborra
Copy link
Collaborator Author

@pepeiborra Just so that I understand. With this change, I am assuming we retain the completion code inside ghcide and then make an effort to split up local/non-local/other completions inside this repo itself. Is that correct?

This also means that we don't need a separate completions plugin residing inside hls.

The plan is still to break the completion code apart from ghcide and into HLS providers. But the HLS providers will live inside ghcide.cabal

pepeiborra referenced this issue Dec 28, 2020
This PR includes changes both to ghcide and HLS to implement the reorg described in https://github.com/haskell/ghcide/issues/936#issuecomment-751437853

To summarise:

- `hls-plugin-api` no longer depends on ghcide.
- `ghcide` now depends on `hls-plugin-api` and exposes:
  - The ghcide HLS plugin
  - The `asGhcIdePlugin` adaptor

The goals are:
- to be able to break the `ghcide` HLS plugin down
- to rewrite exe:ghcide on top of the HLS plugin model.

The ghcide side is reviewed in haskell/ghcide#963

If this change is accepted there are two further considerations:
- This would be a good moment to merge the 2 repos, so that there is no history loss.
- `hls-plugin-api` will need to be released to Hackage prior to merging haskell/ghcide#963
pepeiborra referenced this issue Dec 29, 2020
This PR includes changes both to ghcide and HLS to implement the reorg described in https://github.com/haskell/ghcide/issues/936#issuecomment-751437853

To summarise:

- `hls-plugin-api` no longer depends on ghcide.
- `ghcide` now depends on `hls-plugin-api` and exposes:
  - The ghcide HLS plugin
  - The `asGhcIdePlugin` adaptor

The goals are:
- to be able to break the `ghcide` HLS plugin down
- to rewrite exe:ghcide on top of the HLS plugin model.

The ghcide side is reviewed in haskell/ghcide#963

If this change is accepted there are two further considerations:
- This would be a good moment to merge the 2 repos, so that there is no history loss.
- `hls-plugin-api` will need to be released to Hackage prior to merging haskell/ghcide#963
pepeiborra referenced this issue Dec 29, 2020
This PR includes changes both to ghcide and HLS to implement the reorg described in https://github.com/haskell/ghcide/issues/936#issuecomment-751437853

To summarise:

- `hls-plugin-api` no longer depends on ghcide.
- `ghcide` now depends on `hls-plugin-api` and exposes:
  - The ghcide HLS plugin
  - The `asGhcIdePlugin` adaptor

The goals are:
- to be able to break the `ghcide` HLS plugin down
- to rewrite exe:ghcide on top of the HLS plugin model.

The ghcide side is reviewed in haskell/ghcide#963

If this change is accepted there are two further considerations:
- This would be a good moment to merge the 2 repos, so that there is no history loss.
- `hls-plugin-api` will need to be released to Hackage prior to merging haskell/ghcide#963
pepeiborra referenced this issue Dec 29, 2020
This PR includes changes both to ghcide and HLS to implement the reorg described in https://github.com/haskell/ghcide/issues/936#issuecomment-751437853

To summarise:

- `hls-plugin-api` no longer depends on ghcide.
- `ghcide` now depends on `hls-plugin-api` and exposes:
  - The ghcide HLS plugin
  - The `asGhcIdePlugin` adaptor

The goals are:
- to be able to break the `ghcide` HLS plugin down
- to rewrite exe:ghcide on top of the HLS plugin model.

The ghcide side is reviewed in haskell/ghcide#963

If this change is accepted there are two further considerations:
- This would be a good moment to merge the 2 repos, so that there is no history loss.
- `hls-plugin-api` will need to be released to Hackage prior to merging haskell/ghcide#963
@pepeiborra pepeiborra changed the title Consider extracting completions and code actions into HLS plugins Extract ghcide completions and code actions into HLS plugins Dec 30, 2020
@pepeiborra pepeiborra transferred this issue from haskell/ghcide Dec 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants